diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index c672faa4b..a2979cd84 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -596,11 +596,11 @@ func (h *apiHandlers) GetComposeStatus(ctx echo.Context, id string) error { if err != nil { return HTTPError(ErrorMalformedOSBuildJobResult) } - var buildJobResults []worker.OSBuildKojiJobResult + var buildJobResults []worker.OSBuildJobResult var buildJobStatuses []ImageStatus for i := 1; i < len(deps); i++ { - var buildJobResult worker.OSBuildKojiJobResult - buildJobStatus, _, err := h.server.workers.OSBuildKojiJobStatus(deps[i], &buildJobResult) + var buildJobResult worker.OSBuildJobResult + buildJobStatus, _, err := h.server.workers.OSBuildJobStatus(deps[i], &buildJobResult) if err != nil { return HTTPError(ErrorMalformedOSBuildJobResult) } @@ -668,7 +668,7 @@ func imageStatusFromOSBuildJobStatus(js *worker.JobStatus, result *worker.OSBuil return ImageStatusValueFailure } -func imageStatusFromKojiJobStatus(js *worker.JobStatus, initResult *worker.KojiInitJobResult, buildResult *worker.OSBuildKojiJobResult) ImageStatusValue { +func imageStatusFromKojiJobStatus(js *worker.JobStatus, initResult *worker.KojiInitJobResult, buildResult *worker.OSBuildJobResult) ImageStatusValue { if js.Canceled { return ImageStatusValueFailure } @@ -712,7 +712,7 @@ func composeStatusFromOSBuildJobStatus(js *worker.JobStatus, result *worker.OSBu return ComposeStatusValueFailure } -func composeStatusFromKojiJobStatus(js *worker.JobStatus, initResult *worker.KojiInitJobResult, buildResults []worker.OSBuildKojiJobResult, result *worker.KojiFinalizeJobResult) ComposeStatusValue { +func composeStatusFromKojiJobStatus(js *worker.JobStatus, initResult *worker.KojiInitJobResult, buildResults []worker.OSBuildJobResult, result *worker.KojiFinalizeJobResult) ComposeStatusValue { if js.Canceled { return ComposeStatusValueFailure } @@ -866,6 +866,7 @@ func (h *apiHandlers) GetComposeLogs(ctx echo.Context, id string) error { } var buildResultBlobs []interface{} + resp := &ComposeLogs{ ObjectReference: ObjectReference{ Href: fmt.Sprintf("/api/image-builder-composer/v2/composes/%v/logs", jobId), @@ -889,12 +890,33 @@ func (h *apiHandlers) GetComposeLogs(ctx echo.Context, id string) error { } for i := 1; i < len(deps); i++ { - var buildResult worker.OSBuildKojiJobResult - _, _, err = h.server.workers.OSBuildKojiJobStatus(deps[i], &buildResult) + buildJobType, err := h.server.workers.JobType(deps[i]) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } - buildResultBlobs = append(buildResultBlobs, buildResult) + + switch buildJobType { + // TODO: remove eventually. Kept for backward compatibility + case worker.JobTypeOSBuildKoji: + var buildResult worker.OSBuildKojiJobResult + _, _, err = h.server.workers.OSBuildKojiJobStatus(deps[i], &buildResult) + if err != nil { + return HTTPErrorWithInternal(ErrorComposeNotFound, err) + } + buildResultBlobs = append(buildResultBlobs, buildResult) + + case worker.JobTypeOSBuild: + var buildResult worker.OSBuildJobResult + _, _, err = h.server.workers.OSBuildJobStatus(deps[i], &buildResult) + if err != nil { + return HTTPErrorWithInternal(ErrorComposeNotFound, err) + } + buildResultBlobs = append(buildResultBlobs, buildResult) + + default: + return HTTPErrorWithInternal(ErrorInvalidJobType, + fmt.Errorf("unexpected job type in koji compose dependencies: %q", buildJobType)) + } } resp.Koji = &KojiLogs{ @@ -964,25 +986,60 @@ func (h *apiHandlers) GetComposeManifests(ctx echo.Context, id string) error { } for i := 1; i < len(deps); i++ { - var buildJob worker.OSBuildKojiJob - err = h.server.workers.OSBuildKojiJob(deps[i], &buildJob) + buildJobType, err := h.server.workers.JobType(deps[i]) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } var manifest distro.Manifest - if len(buildJob.Manifest) != 0 { - manifest = buildJob.Manifest - } else { - _, buildDeps, err := h.server.workers.OSBuildKojiJobStatus(deps[i], &worker.OSBuildKojiJobResult{}) + + switch buildJobType { + // TODO: remove eventually. Kept for backward compatibility + case worker.JobTypeOSBuildKoji: + var buildJob worker.OSBuildKojiJob + err = h.server.workers.OSBuildKojiJob(deps[i], &buildJob) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } - manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, buildDeps) - if err != nil { - return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err)) + + if len(buildJob.Manifest) != 0 { + manifest = buildJob.Manifest + } else { + _, buildDeps, err := h.server.workers.OSBuildKojiJobStatus(deps[i], &worker.OSBuildKojiJobResult{}) + if err != nil { + return HTTPErrorWithInternal(ErrorComposeNotFound, err) + } + manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, buildDeps) + if err != nil { + return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err)) + } + manifest = manifestResult.Manifest } - manifest = manifestResult.Manifest + + case worker.JobTypeOSBuild: + var buildJob worker.OSBuildJob + err = h.server.workers.OSBuildJob(deps[i], &buildJob) + if err != nil { + return HTTPErrorWithInternal(ErrorComposeNotFound, err) + } + + if len(buildJob.Manifest) != 0 { + manifest = buildJob.Manifest + } else { + _, buildDeps, err := h.server.workers.OSBuildJobStatus(deps[i], &worker.OSBuildJobResult{}) + if err != nil { + return HTTPErrorWithInternal(ErrorComposeNotFound, err) + } + manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, buildDeps) + if err != nil { + return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err)) + } + manifest = manifestResult.Manifest + } + + default: + return HTTPErrorWithInternal(ErrorInvalidJobType, + fmt.Errorf("unexpected job type in koji compose dependencies: %q", buildJobType)) } manifestBlobs = append(manifestBlobs, manifest) } diff --git a/internal/cloudapi/v2/server.go b/internal/cloudapi/v2/server.go index 003b424f1..055370789 100644 --- a/internal/cloudapi/v2/server.go +++ b/internal/cloudapi/v2/server.go @@ -206,17 +206,20 @@ func (s *Server) enqueueKojiCompose(taskID uint64, server, name, version, releas ir.arch.Name(), splitExtension(ir.imageType.Filename()), ) - buildID, err := s.workers.EnqueueOSBuildKojiAsDependency(ir.arch.Name(), &worker.OSBuildKojiJob{ + buildID, err := s.workers.EnqueueOSBuildAsDependency(ir.arch.Name(), &worker.OSBuildJob{ ImageName: ir.imageType.Filename(), Exports: ir.imageType.Exports(), PipelineNames: &worker.PipelineNames{ Build: ir.imageType.BuildPipelines(), Payload: ir.imageType.PayloadPipelines(), }, - KojiServer: server, - KojiDirectory: kojiDirectory, - KojiFilename: kojiFilename, - }, manifestJobID, initID, channel) + Targets: []*target.Target{target.NewKojiTarget(&target.KojiTargetOptions{ + Server: server, + UploadDirectory: kojiDirectory, + Filename: kojiFilename, + })}, + ManifestDynArgsIdx: common.IntToPtr(1), + }, []uuid.UUID{initID, manifestJobID}, channel) if err != nil { return id, HTTPErrorWithInternal(ErrorEnqueueingJob, err) } diff --git a/internal/cloudapi/v2/v2_koji_test.go b/internal/cloudapi/v2/v2_koji_test.go index 08f1fdd98..4d4e1df18 100644 --- a/internal/cloudapi/v2/v2_koji_test.go +++ b/internal/cloudapi/v2/v2_koji_test.go @@ -17,6 +17,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/kojiapi/api" "github.com/osbuild/osbuild-composer/internal/osbuild2" osbuild "github.com/osbuild/osbuild-composer/internal/osbuild2" + "github.com/osbuild/osbuild-composer/internal/target" "github.com/osbuild/osbuild-composer/internal/test" "github.com/osbuild/osbuild-composer/internal/worker" "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" @@ -34,7 +35,7 @@ func TestKojiCompose(t *testing.T) { type kojiCase struct { initResult worker.KojiInitJobResult - buildResult worker.OSBuildKojiJobResult + buildResult worker.OSBuildJobResult finalizeResult worker.KojiFinalizeJobResult composeReplyCode int composeReply string @@ -48,11 +49,13 @@ func TestKojiCompose(t *testing.T) { BuildID: 42, Token: `"foobar"`, }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, + buildResult: worker.OSBuildJobResult{ + Arch: test_distro.TestArchName, + HostOS: test_distro.TestDistroName, + TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ + ImageMD5: "browns", + ImageSize: 42, + })}, OSBuildOutput: &osbuild.Result{ Success: true, }, @@ -83,11 +86,13 @@ func TestKojiCompose(t *testing.T) { initResult: worker.KojiInitJobResult{ KojiError: "failure", }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, + buildResult: worker.OSBuildJobResult{ + Arch: test_distro.TestArchName, + HostOS: test_distro.TestDistroName, + TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ + ImageMD5: "browns", + ImageSize: 42, + })}, OSBuildOutput: &osbuild.Result{ Success: true, }, @@ -118,11 +123,13 @@ func TestKojiCompose(t *testing.T) { JobError: clienterrors.WorkerClientError(clienterrors.ErrorKojiInit, "Koji init error"), }, }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, + buildResult: worker.OSBuildJobResult{ + Arch: test_distro.TestArchName, + HostOS: test_distro.TestDistroName, + TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ + ImageMD5: "browns", + ImageSize: 42, + })}, OSBuildOutput: &osbuild.Result{ Success: true, }, @@ -152,11 +159,13 @@ func TestKojiCompose(t *testing.T) { BuildID: 42, Token: `"foobar"`, }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, + buildResult: worker.OSBuildJobResult{ + Arch: test_distro.TestArchName, + HostOS: test_distro.TestDistroName, + TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ + ImageMD5: "browns", + ImageSize: 42, + })}, OSBuildOutput: &osbuild.Result{ Success: false, }, @@ -188,58 +197,13 @@ func TestKojiCompose(t *testing.T) { BuildID: 42, Token: `"foobar"`, }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, - OSBuildOutput: &osbuild.Result{ - Success: true, - }, - KojiError: "failure", - }, - composeReplyCode: http.StatusCreated, - composeReply: `{"href":"/api/image-builder-composer/v2/compose", "kind":"ComposeId"}`, - composeStatus: `{ - "kind": "ComposeStatus", - "image_status": { - "status": "failure", - "error": { - "details": null, - "id": 18, - "reason": "failure" - } - }, - "image_statuses": [ - { - "status": "failure", - "error": { - "details": null, - "id": 18, - "reason": "failure" - } - }, - { - "status": "success" - } - ], - "koji_status": { - "build_id": 42 - }, - "status": "failure" - }`, - }, - // #5 - { - initResult: worker.KojiInitJobResult{ - BuildID: 42, - Token: `"foobar"`, - }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, + buildResult: worker.OSBuildJobResult{ + Arch: test_distro.TestArchName, + HostOS: test_distro.TestDistroName, + TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ + ImageMD5: "browns", + ImageSize: 42, + })}, OSBuildOutput: &osbuild.Result{ Success: true, }, @@ -278,17 +242,19 @@ func TestKojiCompose(t *testing.T) { "status": "failure" }`, }, - // #6 + // #5 { initResult: worker.KojiInitJobResult{ BuildID: 42, Token: `"foobar"`, }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, + buildResult: worker.OSBuildJobResult{ + Arch: test_distro.TestArchName, + HostOS: test_distro.TestDistroName, + TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ + ImageMD5: "browns", + ImageSize: 42, + })}, OSBuildOutput: &osbuild.Result{ Success: true, }, @@ -317,17 +283,19 @@ func TestKojiCompose(t *testing.T) { "status": "failure" }`, }, - // #7 + // #6 { initResult: worker.KojiInitJobResult{ BuildID: 42, Token: `"foobar"`, }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, + buildResult: worker.OSBuildJobResult{ + Arch: test_distro.TestArchName, + HostOS: test_distro.TestDistroName, + TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ + ImageMD5: "browns", + ImageSize: 42, + })}, OSBuildOutput: &osbuild.Result{ Success: true, }, @@ -358,17 +326,19 @@ func TestKojiCompose(t *testing.T) { "status": "failure" }`, }, - // #8 + // #7 { initResult: worker.KojiInitJobResult{ BuildID: 42, Token: `"foobar"`, }, - buildResult: worker.OSBuildKojiJobResult{ - Arch: test_distro.TestArchName, - HostOS: test_distro.TestDistroName, - ImageHash: "browns", - ImageSize: 42, + buildResult: worker.OSBuildJobResult{ + Arch: test_distro.TestArchName, + HostOS: test_distro.TestDistroName, + TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ + ImageMD5: "browns", + ImageSize: 42, + })}, OSBuildOutput: &osbuild.Result{ Success: true, }, @@ -497,16 +467,17 @@ func TestKojiCompose(t *testing.T) { // handle build jobs for i := 0; i < len(buildJobIDs); i++ { - jobID, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuildKoji}, []string{""}) + jobID, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) - require.Equal(t, worker.JobTypeOSBuildKoji, jobType) + require.Equal(t, worker.JobTypeOSBuild, jobType) - var osbuildJob worker.OSBuildKojiJob + var osbuildJob worker.OSBuildJob err = json.Unmarshal(rawJob, &osbuildJob) require.NoError(t, err) - require.Equal(t, "koji.example.com", osbuildJob.KojiServer) + jobTarget := osbuildJob.Targets[0].Options.(*target.KojiTargetOptions) + require.Equal(t, "koji.example.com", jobTarget.Server) require.Equal(t, "test.img", osbuildJob.ImageName) - require.NotEmpty(t, osbuildJob.KojiDirectory) + require.NotEmpty(t, jobTarget.UploadDirectory) var buildJobResult string switch jobID { @@ -618,16 +589,18 @@ func TestKojiJobTypeValidation(t *testing.T) { manifest, err := json.Marshal(osbuild2.Manifest{}) require.NoErrorf(t, err, "error marshalling empty Manifest to JSON") - buildJobs := make([]worker.OSBuildKojiJob, nImages) + buildJobs := make([]worker.OSBuildJob, nImages) buildJobIDs := make([]uuid.UUID, nImages) filenames := make([]string, nImages) for idx := 0; idx < nImages; idx++ { fname := fmt.Sprintf("image-file-%04d", idx) - buildJob := worker.OSBuildKojiJob{ - ImageName: fmt.Sprintf("build-job-%04d", idx), - KojiServer: "test-server", - KojiDirectory: "koji-server-test-dir", - KojiFilename: fname, + buildJob := worker.OSBuildJob{ + ImageName: fmt.Sprintf("build-job-%04d", idx), + Targets: []*target.Target{target.NewKojiTarget(&target.KojiTargetOptions{ + Server: "test-server", + UploadDirectory: "koji-server-test-dir", + Filename: fname, + })}, // Add an empty manifest as a static job argument to make the test pass. // Becasue of a bug in the API, the test was passing even without // any manifest being attached to the job (static or dynamic). @@ -635,7 +608,7 @@ func TestKojiJobTypeValidation(t *testing.T) { // TODO: use dependent depsolve and manifests jobs instead Manifest: manifest, } - buildID, err := workers.EnqueueOSBuildKoji(fmt.Sprintf("fake-arch-%d", idx), &buildJob, initID, "") + buildID, err := workers.EnqueueOSBuildAsDependency(fmt.Sprintf("fake-arch-%d", idx), &buildJob, []uuid.UUID{initID}, "") require.NoError(t, err) buildJobs[idx] = buildJob @@ -669,7 +642,7 @@ func TestKojiJobTypeValidation(t *testing.T) { test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%s%s", initID, path), ``, http.StatusNotFound, `{"code":"IMAGE-BUILDER-COMPOSER-26","href":"/api/image-builder-composer/v2/errors/26","id":"26","kind":"Error","reason":"Requested job has invalid type"}`, `operation_id`) for _, buildID := range buildJobIDs { - test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%s%s", buildID, path), ``, http.StatusNotFound, `{"code":"IMAGE-BUILDER-COMPOSER-26","href":"/api/image-builder-composer/v2/errors/26","id":"26","kind":"Error","reason":"Requested job has invalid type"}`, `operation_id`) + test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%s%s", buildID, path), ``, http.StatusOK, "*") } badID := uuid.New() diff --git a/internal/worker/json.go b/internal/worker/json.go index 83e32636d..05d3cd877 100644 --- a/internal/worker/json.go +++ b/internal/worker/json.go @@ -78,7 +78,7 @@ type OSBuildKojiJobResult struct { PipelineNames *PipelineNames `json:"pipeline_names,omitempty"` ImageHash string `json:"image_hash"` ImageSize uint64 `json:"image_size"` - KojiError string `json:"koji_error"` + KojiError string `json:"koji_error"` // not set by any code other than unit tests JobResult }