From 0cd71745981bf71bfb4e2c673dbf0db11cf910f4 Mon Sep 17 00:00:00 2001 From: Lars Karlitski Date: Wed, 28 Oct 2020 00:24:56 +0100 Subject: [PATCH] worker: deprecate the local target Add "image_name" and "stream_optimized" fields to the osbuild job as replacement for the local target options. The former signifies the name of the uploaded artifact and whether an artifact should be uploaded at all (only weldr API). The latter will be deprecated at some point, when osbuild itself can make streamoptimized vmdk images. This change separates what have always been two distinct concepts: artifacts that are reported back to the composer node (in practice always running on the same machine), and upload targets to clouds and such. Separating them makes it easier to add job types that only allow one upload target while keeping artifacts. Keep the local target around, so that jobs that are scheduled can still be run after an upgrade. --- cmd/osbuild-worker/main.go | 30 +++++++++++++++++++++++++----- internal/cloudapi/server.go | 5 ++++- internal/kojiapi/server.go | 33 +++++++++++++++++++-------------- internal/weldr/api.go | 16 ++++++---------- internal/weldr/api_test.go | 16 ---------------- internal/worker/client.go | 12 +++++------- internal/worker/json.go | 6 ++++-- internal/worker/server.go | 9 +-------- internal/worker/server_test.go | 4 ++-- 9 files changed, 66 insertions(+), 65 deletions(-) diff --git a/cmd/osbuild-worker/main.go b/cmd/osbuild-worker/main.go index f619f0914..e2550bdb3 100644 --- a/cmd/osbuild-worker/main.go +++ b/cmd/osbuild-worker/main.go @@ -119,14 +119,14 @@ func RunJob(job worker.Job, store string, kojiServers map[string]koji.GSSAPICred } }() - manifest, targets, err := job.OSBuildArgs() + args, err := job.OSBuildArgs() if err != nil { return nil, err } start_time := time.Now() - result, err := RunOSBuild(manifest, store, outputDirectory, os.Stderr) + result, err := RunOSBuild(args.Manifest, store, outputDirectory, os.Stderr) if err != nil { return nil, err } @@ -138,9 +138,29 @@ func RunJob(job worker.Job, store string, kojiServers map[string]koji.GSSAPICred return result, nil } + if args.ImageName != "" { + var f *os.File + imagePath := path.Join(outputDirectory, args.ImageName) + if args.StreamOptimized { + f, err = vmware.OpenAsStreamOptimizedVmdk(imagePath) + if err != nil { + return nil, err + } + } else { + f, err = os.Open(imagePath) + if err != nil { + return nil, err + } + } + err = job.UploadArtifact(args.ImageName, f) + if err != nil { + return nil, err + } + } + var r []error - for _, t := range targets { + for _, t := range args.Targets { switch options := t.Options.(type) { case *target.LocalTargetOptions: var f *os.File @@ -322,12 +342,12 @@ func RunJob(job worker.Job, store string, kojiServers map[string]koji.GSSAPICred } func FailJob(job worker.Job, kojiServers map[string]koji.GSSAPICredentials) { - _, targets, err := job.OSBuildArgs() + args, err := job.OSBuildArgs() if err != nil { panic(err) } - for _, t := range targets { + for _, t := range args.Targets { switch options := t.Options.(type) { case *target.KojiTargetOptions: // Koji for some reason needs TLS renegotiation enabled. diff --git a/internal/cloudapi/server.go b/internal/cloudapi/server.go index 680931267..65e6d3579 100644 --- a/internal/cloudapi/server.go +++ b/internal/cloudapi/server.go @@ -188,7 +188,10 @@ func (server *Server) Compose(w http.ResponseWriter, r *http.Request) { return } - id, err := server.workers.Enqueue(ir.arch, ir.manifest, targets) + id, err := server.workers.Enqueue(ir.arch, &worker.OSBuildJob{ + Manifest: ir.manifest, + Targets: targets, + }) if err != nil { http.Error(w, "Failed to enqueue manifest", http.StatusInternalServerError) return diff --git a/internal/kojiapi/server.go b/internal/kojiapi/server.go index 7bdb47dfa..e3d205c53 100644 --- a/internal/kojiapi/server.go +++ b/internal/kojiapi/server.go @@ -178,20 +178,25 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Could not initialize build with koji: %v", err)) } - id, err := h.server.workers.Enqueue(ir.arch, ir.manifest, []*target.Target{ - target.NewKojiTarget(&target.KojiTargetOptions{ - BuildID: uint64(buildInfo.BuildID), - TaskID: uint64(request.Koji.TaskId), - Token: buildInfo.Token, - Name: request.Name, - Version: request.Version, - Release: request.Release, - Filename: ir.filename, - UploadDirectory: "osbuild-composer-koji-" + uuid.New().String(), - Server: request.Koji.Server, - KojiFilename: ir.kojiFilename, - }), - }) + job := worker.OSBuildJob{ + Manifest: ir.manifest, + Targets: []*target.Target{ + target.NewKojiTarget(&target.KojiTargetOptions{ + BuildID: uint64(buildInfo.BuildID), + TaskID: uint64(request.Koji.TaskId), + Token: buildInfo.Token, + Name: request.Name, + Version: request.Version, + Release: request.Release, + Filename: ir.filename, + UploadDirectory: "osbuild-composer-koji-" + uuid.New().String(), + Server: request.Koji.Server, + KojiFilename: ir.kojiFilename, + }), + }, + } + + id, err := h.server.workers.Enqueue(ir.arch, &job) if err != nil { // This is a programming errror. panic(err) diff --git a/internal/weldr/api.go b/internal/weldr/api.go index b1ea63982..bf99e1d29 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -1799,15 +1799,6 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request targets = append(targets, t) } - targets = append(targets, target.NewLocalTarget( - &target.LocalTargetOptions{ - ComposeId: composeID, - ImageBuildId: 0, - Filename: imageType.Filename(), - StreamOptimized: imageType.Name() == "vmdk", // TODO: move conversion to osbuild - }, - )) - bp := api.store.GetBlueprintCommitted(cr.BlueprintName) if bp == nil { errors := responseError{ @@ -1870,7 +1861,12 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request } else { var jobId uuid.UUID - jobId, err = api.workers.Enqueue(api.arch.Name(), manifest, targets) + jobId, err = api.workers.Enqueue(api.arch.Name(), &worker.OSBuildJob{ + Manifest: manifest, + Targets: targets, + ImageName: imageType.Filename(), + StreamOptimized: imageType.Name() == "vmdk", // https://github.com/osbuild/osbuild/issues/528 + }) if err == nil { err = api.store.PushCompose(composeID, manifest, imageType, bp, size, targets, jobId) } diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go index 31d3ed8dd..2057a1b50 100644 --- a/internal/weldr/api_test.go +++ b/internal/weldr/api_test.go @@ -474,15 +474,6 @@ func TestCompose(t *testing.T) { QueueStatus: common.IBWaiting, ImageType: imgType, Manifest: manifest, - Targets: []*target.Target{ - { - // skip Uuid and Created fields - they are ignored - Name: "org.osbuild.local", - Options: &target.LocalTargetOptions{ - Filename: "test.img", - }, - }, - }, }, } expectedComposeLocalAndAws := &store.Compose{ @@ -512,13 +503,6 @@ func TestCompose(t *testing.T) { Key: "imagekey", }, }, - { - // skip Uuid and Created fields - they are ignored - Name: "org.osbuild.local", - Options: &target.LocalTargetOptions{ - Filename: "test.img", - }, - }, }, }, } diff --git a/internal/worker/client.go b/internal/worker/client.go index 88e7f0700..f48fd10d9 100644 --- a/internal/worker/client.go +++ b/internal/worker/client.go @@ -14,9 +14,7 @@ import ( "github.com/google/uuid" "github.com/osbuild/osbuild-composer/internal/common" - "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/osbuild" - "github.com/osbuild/osbuild-composer/internal/target" "github.com/osbuild/osbuild-composer/internal/worker/api" ) @@ -27,7 +25,7 @@ type Client struct { type Job interface { Id() uuid.UUID - OSBuildArgs() (distro.Manifest, []*target.Target, error) + OSBuildArgs() (*OSBuildJob, error) Update(result *osbuild.Result) error Canceled() (bool, error) UploadArtifact(name string, reader io.Reader) error @@ -140,18 +138,18 @@ func (j *job) Id() uuid.UUID { return j.id } -func (j *job) OSBuildArgs() (distro.Manifest, []*target.Target, error) { +func (j *job) OSBuildArgs() (*OSBuildJob, error) { if j.jobType != "osbuild" { - return nil, nil, errors.New("not an osbuild job") + return nil, errors.New("not an osbuild job") } var args OSBuildJob err := json.Unmarshal(j.args, &args) if err != nil { - return nil, nil, fmt.Errorf("error parsing osbuild job arguments: %v", err) + return nil, fmt.Errorf("error parsing osbuild job arguments: %v", err) } - return args.Manifest, args.Targets, nil + return &args, nil } func (j *job) Update(result *osbuild.Result) error { diff --git a/internal/worker/json.go b/internal/worker/json.go index 977adc6a4..3cd8120f4 100644 --- a/internal/worker/json.go +++ b/internal/worker/json.go @@ -15,8 +15,10 @@ import ( // type OSBuildJob struct { - Manifest distro.Manifest `json:"manifest"` - Targets []*target.Target `json:"targets,omitempty"` + Manifest distro.Manifest `json:"manifest"` + Targets []*target.Target `json:"targets,omitempty"` + ImageName string `json:"image_name,omitempty"` + StreamOptimized bool `json:"stream_optimized,omitempty"` } type OSBuildJobResult struct { diff --git a/internal/worker/server.go b/internal/worker/server.go index 324f29bf0..15d1550e3 100644 --- a/internal/worker/server.go +++ b/internal/worker/server.go @@ -18,9 +18,7 @@ import ( "github.com/google/uuid" "github.com/labstack/echo/v4" - "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/jobqueue" - "github.com/osbuild/osbuild-composer/internal/target" "github.com/osbuild/osbuild-composer/internal/worker/api" ) @@ -86,12 +84,7 @@ func (s *Server) ServeHTTP(writer http.ResponseWriter, request *http.Request) { s.server.Handler.ServeHTTP(writer, request) } -func (s *Server) Enqueue(arch string, manifest distro.Manifest, targets []*target.Target) (uuid.UUID, error) { - job := OSBuildJob{ - Manifest: manifest, - Targets: targets, - } - +func (s *Server) Enqueue(arch string, job *OSBuildJob) (uuid.UUID, error) { return s.jobs.Enqueue("osbuild:"+arch, job, nil) } diff --git a/internal/worker/server_test.go b/internal/worker/server_test.go index c89836e53..8f392424b 100644 --- a/internal/worker/server_test.go +++ b/internal/worker/server_test.go @@ -64,7 +64,7 @@ func TestCreate(t *testing.T) { } server := worker.NewServer(nil, testjobqueue.New(), "") - _, err = server.Enqueue(arch.Name(), manifest, nil) + _, err = server.Enqueue(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) require.NoError(t, err) test.TestRoute(t, server, false, "POST", "/api/worker/v1/jobs", `{"types":["osbuild"],"arch":"x86_64"}`, http.StatusCreated, @@ -87,7 +87,7 @@ func TestCancel(t *testing.T) { } server := worker.NewServer(nil, testjobqueue.New(), "") - jobId, err := server.Enqueue(arch.Name(), manifest, nil) + jobId, err := server.Enqueue(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) require.NoError(t, err) token, j, _, err := server.RequestOSBuildJob(context.Background(), arch.Name())