From 55d3854033804d9226f4ca07559ce0bf8b37435c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Thu, 13 Feb 2020 11:19:06 +0100 Subject: [PATCH] targets/local: drop Location field When support for osbuild result was added into osbuild-composer it was in a bit hacky way - localtarget's location was reused as a path for the result. This didn't make much sense because we want to store the result even when image build has no localtarget. Several past commits made store less dependant on the localtarget. The responsibility for "holding the paths" to build artifacts was gradually switched from the localtarget to the store while still maintaining backwards compatibility - localtarget.Location still pointed at the correct location. This commit finishes the switch: local target now has no Location field. The store is now fully responsible for managing the artifacts and paths to them. LocalTarget is now just a simple "switch" - if image build has it, then worker uploads an image into the store and it's then available for download using the weldr API. --- internal/compose/compose.go | 8 +++--- internal/mocks/rpmmd/fixtures.go | 4 +-- internal/store/store.go | 48 ++++++++++++++++++++++++++------ internal/target/local_target.go | 1 - internal/weldr/api.go | 31 ++++----------------- 5 files changed, 50 insertions(+), 42 deletions(-) diff --git a/internal/compose/compose.go b/internal/compose/compose.go index cd7621a60..d08c56b0a 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -71,14 +71,14 @@ func (ib *ImageBuild) DeepCopy() ImageBuild { } } -func (ib *ImageBuild) GetLocalTarget() *target.LocalTargetOptions { +func (ib *ImageBuild) HasLocalTarget() bool { for _, t := range ib.Targets { - if localTarget, ok := t.Options.(*target.LocalTargetOptions); ok { - return localTarget + if _, ok := t.Options.(*target.LocalTargetOptions); ok { + return true } } - return nil + return false } // A Compose represent the task of building a set of images from a single blueprint. diff --git a/internal/mocks/rpmmd/fixtures.go b/internal/mocks/rpmmd/fixtures.go index 11976cccd..60f7b07cb 100644 --- a/internal/mocks/rpmmd/fixtures.go +++ b/internal/mocks/rpmmd/fixtures.go @@ -75,9 +75,7 @@ func createBaseStoreFixture() *store.Store { ImageName: "localimage", Created: date, Status: common.IBWaiting, - Options: &target.LocalTargetOptions{ - Location: "/tmp/localimage", - }, + Options: &target.LocalTargetOptions{}, } var awsTarget = &target.Target{ diff --git a/internal/store/store.go b/internal/store/store.go index 060358f32..4ef5b67fa 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -461,6 +461,42 @@ func (s *Store) GetImageBuildResult(composeId uuid.UUID, imageBuildId int) (io.R return os.Open(s.getImageBuildDirectory(composeId, imageBuildId) + "/result.json") } +func (s *Store) GetImageBuildImage(composeId uuid.UUID, imageBuildId int) (io.ReadCloser, int64, error) { + c, ok := s.Composes[composeId] + + if !ok { + return nil, 0, &NotFoundError{"compose does not exist"} + } + + imageBuild := c.ImageBuilds[imageBuildId] + if !imageBuild.HasLocalTarget() { + return nil, 0, &NoLocalTargetError{"compose does not have local target"} + } + + compatString, _ := imageBuild.ImageType.ToCompatString() + filename, _, err := s.distro.FilenameFromType(compatString) + if err != nil { + panic(err) + } + + path := fmt.Sprintf("%s/%s", s.getImageBuildDirectory(composeId, imageBuildId), filename) + + f, err := os.Open(path) + + if err != nil { + return nil, 0, err + } + + fileInfo, err := f.Stat() + + if err != nil { + return nil, 0, err + } + + return f, fileInfo.Size(), err + +} + func (s *Store) getComposeDirectory(composeID uuid.UUID) string { return fmt.Sprintf("%s/outputs/%s", *s.stateDir, composeID.String()) } @@ -487,9 +523,7 @@ func (s *Store) PushCompose(composeID uuid.UUID, bp *blueprint.Blueprint, checks } targets = append(targets, target.NewLocalTarget( - &target.LocalTargetOptions{ - Location: outputDir, - }, + &target.LocalTargetOptions{}, )) } @@ -749,9 +783,7 @@ func (s *Store) AddImageToImageUpload(composeID uuid.UUID, imageBuildID int, rea } imageBuild := currentCompose.ImageBuilds[imageBuildID] - localTarget := imageBuild.GetLocalTarget() - - if localTarget == nil { + if !imageBuild.HasLocalTarget() { return &NoLocalTargetError{fmt.Sprintf("image upload requested for compse %s and image build %d but it has no local target", composeID.String(), imageBuildID)} } @@ -762,8 +794,8 @@ func (s *Store) AddImageToImageUpload(composeID uuid.UUID, imageBuildID int, rea return &InvalidRequestError{err.Error()} } - path := fmt.Sprintf("%s/%s", localTarget.Location, filename) - f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + path := fmt.Sprintf("%s/%s", s.getImageBuildDirectory(composeID, imageBuildID), filename) + f, err := os.Create(path) if err != nil { return err diff --git a/internal/target/local_target.go b/internal/target/local_target.go index 35209d524..eb2fa9dbc 100644 --- a/internal/target/local_target.go +++ b/internal/target/local_target.go @@ -1,7 +1,6 @@ package target type LocalTargetOptions struct { - Location string `json:"location"` } func (LocalTargetOptions) isTargetOptions() {} diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 8664d87f8..80287a9f5 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -11,7 +11,6 @@ import ( "net" "net/http" "net/url" - "os" "sort" "strconv" "strings" @@ -1566,17 +1565,6 @@ func (api *API) composeImageHandler(writer http.ResponseWriter, request *http.Re } imageBuild := compose.ImageBuilds[0] - localTarget := imageBuild.GetLocalTarget() - - if localTarget == nil { - errors := responseError{ - ID: "BadCompose", - Msg: fmt.Sprintf("Compose %s is ill-formed: it doesn't contain LocalTarget", uuidString), - } - statusResponseError(writer, http.StatusInternalServerError, errors) - return - } - imageType, _ := imageBuild.ImageType.ToCompatString() imageName, imageMime, err := api.distro.FilenameFromType(imageType) @@ -1589,7 +1577,9 @@ func (api *API) composeImageHandler(writer http.ResponseWriter, request *http.Re return } - file, err := os.Open(localTarget.Location + "/" + imageName) + reader, fileSize, err := api.store.GetImageBuildImage(uuid, 0) + + // TODO: this might return misleading error if err != nil { errors := responseError{ ID: "BuildMissingFile", @@ -1599,22 +1589,11 @@ func (api *API) composeImageHandler(writer http.ResponseWriter, request *http.Re return } - stat, err := file.Stat() - - if err != nil { - errors := responseError{ - ID: "BuildStatFailed", - Msg: fmt.Sprintf("Cannot stat image for build %s!", uuidString), - } - statusResponseError(writer, http.StatusBadRequest, errors) - return - } - writer.Header().Set("Content-Disposition", "attachment; filename="+uuid.String()+"-"+imageName) writer.Header().Set("Content-Type", imageMime) - writer.Header().Set("Content-Length", fmt.Sprintf("%d", stat.Size())) + writer.Header().Set("Content-Length", fmt.Sprintf("%d", fileSize)) - io.Copy(writer, file) + io.Copy(writer, reader) } func (api *API) composeLogsHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {