diff --git a/cmd/osbuild-worker/jobimpl-ostree-resolve.go b/cmd/osbuild-worker/jobimpl-ostree-resolve.go index 2ebb4ddc6..1edfd6ef3 100644 --- a/cmd/osbuild-worker/jobimpl-ostree-resolve.go +++ b/cmd/osbuild-worker/jobimpl-ostree-resolve.go @@ -50,7 +50,7 @@ func (impl *OSTreeResolveJobImpl) Run(job worker.Job) error { logWithId.Infof("Resolving (%d) ostree commits", len(args.Specs)) - for _, s := range args.Specs { + for i, s := range args.Specs { reqParams := ostree.RequestParams{ URL: s.URL, Ref: s.Ref, @@ -63,11 +63,11 @@ func (impl *OSTreeResolveJobImpl) Run(job worker.Job) error { break } - result.Specs = append(result.Specs, worker.OSTreeResolveResultSpec{ + result.Specs[i] = worker.OSTreeResolveResultSpec{ URL: s.URL, Ref: ref, Checksum: checksum, - }) + } } err = job.Update(&result) diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index 35846d0dc..449817a01 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -112,6 +112,7 @@ type imageRequest struct { repositories []rpmmd.RepoConfig imageOptions distro.ImageOptions target *target.Target + ostree *ostree.RequestParams } func (h *apiHandlers) PostCompose(ctx echo.Context) error { @@ -286,44 +287,23 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { } } + var ostreeOptions *ostree.RequestParams // assume it's an ostree image if the type has a default ostree ref if imageType.OSTreeRef() != "" && ir.Ostree != nil { - ostreeOptions := ostree.RequestParams{} - if ir.Ostree != nil { - if ir.Ostree.Ref != nil { - ostreeOptions.Ref = *ir.Ostree.Ref - } - if ir.Ostree.Url != nil { - ostreeOptions.URL = *ir.Ostree.Url - } - if ir.Ostree.Parent != nil { - ostreeOptions.Parent = *ir.Ostree.Parent - } + ostreeOptions = &ostree.RequestParams{} + if ir.Ostree.Ref != nil { + ostreeOptions.Ref = *ir.Ostree.Ref + } + if ir.Ostree.Url != nil { + ostreeOptions.URL = *ir.Ostree.Url + } + if ir.Ostree.Parent != nil { + ostreeOptions.Parent = *ir.Ostree.Parent } if ostreeOptions.Ref == "" { ostreeOptions.Ref = imageType.OSTreeRef() } - - ref, checksum, err := ostree.ResolveParams(ostreeOptions) - if err != nil { - switch v := err.(type) { - case ostree.RefError: - return HTTPError(ErrorInvalidOSTreeRef) - case ostree.ResolveRefError: - return HTTPErrorWithInternal(ErrorInvalidOSTreeRepo, v) - case ostree.ParameterComboError: - return HTTPError(ErrorInvalidOSTreeParams) - default: - // general case - return HTTPError(ErrorInvalidOSTreeParams) - } - } - imageOptions.OSTree = distro.OSTreeImageOptions{ - ImageRef: ref, - FetchChecksum: checksum, - URL: ostreeOptions.URL, - } } var irTarget *target.Target @@ -518,6 +498,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { repositories: repos, imageOptions: imageOptions, target: irTarget, + ostree: ostreeOptions, }) } diff --git a/internal/cloudapi/v2/server.go b/internal/cloudapi/v2/server.go index 5a01977b7..9c95002f5 100644 --- a/internal/cloudapi/v2/server.go +++ b/internal/cloudapi/v2/server.go @@ -139,6 +139,25 @@ func (s *Server) enqueueCompose(distribution distro.Distro, bp blueprint.Bluepri dependencies = append(dependencies, jobId) } + var ostreeResolveJobID uuid.UUID + if ir.ostree != nil { + jobID, err := s.workers.EnqueueOSTreeResolveJob(&worker.OSTreeResolveJob{ + Specs: []worker.OSTreeResolveSpec{ + worker.OSTreeResolveSpec{ + URL: ir.ostree.URL, + Ref: ir.ostree.Ref, + Parent: ir.ostree.Parent, + }, + }, + }, channel) + if err != nil { + return id, HTTPErrorWithInternal(ErrorEnqueueingJob, err) + } + + ostreeResolveJobID = jobID + dependencies = append(dependencies, ostreeResolveJobID) + } + manifestJobID, err := s.workers.EnqueueManifestJobByID(&worker.ManifestJobByID{}, dependencies, channel) if err != nil { return id, HTTPErrorWithInternal(ErrorEnqueueingJob, err) @@ -157,7 +176,7 @@ func (s *Server) enqueueCompose(distribution distro.Distro, bp blueprint.Bluepri s.goroutinesGroup.Add(1) go func() { - generateManifest(s.goroutinesCtx, s.workers, depsolveJobID, containerResolveJob, manifestJobID, ir.imageType, ir.repositories, ir.imageOptions, manifestSeed, bp.Customizations) + generateManifest(s.goroutinesCtx, s.workers, depsolveJobID, containerResolveJob, ostreeResolveJobID, manifestJobID, ir.imageType, ir.repositories, ir.imageOptions, manifestSeed, bp.Customizations) defer s.goroutinesGroup.Done() }() @@ -217,6 +236,25 @@ func (s *Server) enqueueKojiCompose(taskID uint64, server, name, version, releas dependencies = append(dependencies, jobId) } + var ostreeResolveJobID uuid.UUID + if ir.ostree != nil { + jobID, err := s.workers.EnqueueOSTreeResolveJob(&worker.OSTreeResolveJob{ + Specs: []worker.OSTreeResolveSpec{ + worker.OSTreeResolveSpec{ + URL: ir.ostree.URL, + Ref: ir.ostree.Ref, + Parent: ir.ostree.Parent, + }, + }, + }, channel) + if err != nil { + return id, HTTPErrorWithInternal(ErrorEnqueueingJob, err) + } + + ostreeResolveJobID = jobID + dependencies = append(dependencies, ostreeResolveJobID) + } + manifestJobID, err := s.workers.EnqueueManifestJobByID(&worker.ManifestJobByID{}, dependencies, channel) if err != nil { return id, HTTPErrorWithInternal(ErrorEnqueueingJob, err) @@ -261,7 +299,7 @@ func (s *Server) enqueueKojiCompose(taskID uint64, server, name, version, releas // copy the image request while passing it into the goroutine to prevent data races s.goroutinesGroup.Add(1) go func(ir imageRequest) { - generateManifest(s.goroutinesCtx, s.workers, depsolveJobID, containerResolveJob, manifestJobID, ir.imageType, ir.repositories, ir.imageOptions, manifestSeed, bp.Customizations) + generateManifest(s.goroutinesCtx, s.workers, depsolveJobID, containerResolveJob, ostreeResolveJobID, manifestJobID, ir.imageType, ir.repositories, ir.imageOptions, manifestSeed, bp.Customizations) defer s.goroutinesGroup.Done() }(ir) } @@ -282,7 +320,7 @@ func (s *Server) enqueueKojiCompose(taskID uint64, server, name, version, releas return id, nil } -func generateManifest(ctx context.Context, workers *worker.Server, depsolveJobID, containerResolveJobID, manifestJobID uuid.UUID, imageType distro.ImageType, repos []rpmmd.RepoConfig, options distro.ImageOptions, seed int64, b *blueprint.Customizations) { +func generateManifest(ctx context.Context, workers *worker.Server, depsolveJobID, containerResolveJobID, ostreeResolveJobID, manifestJobID uuid.UUID, imageType distro.ImageType, repos []rpmmd.RepoConfig, options distro.ImageOptions, seed int64, b *blueprint.Customizations) { ctx, cancel := context.WithTimeout(ctx, time.Minute*5) defer cancel() @@ -367,7 +405,7 @@ func generateManifest(ctx context.Context, workers *worker.Server, depsolveJobID } var containerSpecs []container.Spec - if len(dynArgs) == 2 { + if containerResolveJobID != uuid.Nil { // Container resolve job var result worker.ContainerResolveJobResult @@ -395,6 +433,29 @@ func generateManifest(ctx context.Context, workers *worker.Server, depsolveJobID } } + if ostreeResolveJobID != uuid.Nil { + var result worker.OSTreeResolveJobResult + _, err := workers.OSTreeResolveJobInfo(ostreeResolveJobID, &result) + + if err != nil { + reason := "Error reading ostree resolve job status" + logrus.Errorf("%s: %v", reason, err) + jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorReadingJobStatus, reason, nil) + return + } + + if jobErr := result.JobError; jobErr != nil { + jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOSTreeDependency, "Error in ostree resolve job dependency", nil) + return + } + + options.OSTree = distro.OSTreeImageOptions{ + ImageRef: result.Specs[0].Ref, + FetchChecksum: result.Specs[0].Checksum, + URL: result.Specs[0].URL, + } + } + manifest, err := imageType.Manifest(b, options, repos, depsolveResults.PackageSpecs, containerSpecs, seed) if err != nil { reason := "Error generating manifest" diff --git a/internal/cloudapi/v2/v2_test.go b/internal/cloudapi/v2/v2_test.go index 75e2c61c3..7209bd825 100644 --- a/internal/cloudapi/v2/v2_test.go +++ b/internal/cloudapi/v2/v2_test.go @@ -78,8 +78,47 @@ func newV2Server(t *testing.T, dir string, depsolveChannels []string, enableJWT } }() + ostreeResolveContext, cancelOstree := context.WithCancel(context.Background()) + wg.Add(1) + go func() { + defer wg.Done() + for { + _, token, _, _, _, err := workerServer.RequestJob(ostreeResolveContext, test_distro.TestDistroName, []string{worker.JobTypeOSTreeResolve}, depsolveChannels) + select { + case <-ostreeResolveContext.Done(): + return + default: + } + + if err != nil { + continue + } + oJR := &worker.OSTreeResolveJobResult{ + Specs: []worker.OSTreeResolveResultSpec{ + worker.OSTreeResolveResultSpec{ + URL: "", + Ref: "", + Checksum: "", + }, + }, + } + + if failDepsolve { + oJR.JobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOSTreeParamsInvalid, "ostree error", nil) + } + + rawMsg, err := json.Marshal(oJR) + require.NoError(t, err) + err = workerServer.FinishJob(token, rawMsg) + if err != nil { + return + } + } + }() + cancelWithWait := func() { cancel() + cancelOstree() wg.Wait() } @@ -417,147 +456,6 @@ func TestCompose(t *testing.T) { "href": "/api/image-builder-composer/v2/compose", "kind": "ComposeId" }`, "id") - - // ostree errors - - // bad url - test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "POST", "/api/image-builder-composer/v2/compose", fmt.Sprintf(` - { - "distribution": "%s", - "image_request":{ - "architecture": "%s", - "image_type": "edge-commit", - "repositories": [{ - "baseurl": "somerepo.org", - "rhsm": false - }], - "upload_options": { - "region": "eu-central-1" - }, - "ostree": { - "ref": "rhel/10/x86_64/edge", - "url": "not-a-URL" - } - } - }`, test_distro.TestDistroName, test_distro.TestArch3Name), http.StatusBadRequest, ` - { - "href": "/api/image-builder-composer/v2/errors/10", - "id": "10", - "kind": "Error", - "code": "IMAGE-BUILDER-COMPOSER-10", - "reason": "Error resolving OSTree repo" - }`, "operation_id", "details") - - // bad ref - test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "POST", "/api/image-builder-composer/v2/compose", fmt.Sprintf(` - { - "distribution": "%s", - "image_request":{ - "architecture": "%s", - "image_type": "edge-commit", - "repositories": [{ - "baseurl": "somerepo.org", - "rhsm": false - }], - "upload_options": { - "region": "eu-central-1" - }, - "ostree": { - "ref": "/bad/ref" - } - } - }`, test_distro.TestDistroName, test_distro.TestArch3Name), http.StatusBadRequest, ` - { - "href": "/api/image-builder-composer/v2/errors/9", - "id": "9", - "kind": "Error", - "code": "IMAGE-BUILDER-COMPOSER-9", - "reason": "Invalid OSTree ref" - }`, "operation_id", "details") - - // bad parent ref - test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "POST", "/api/image-builder-composer/v2/compose", fmt.Sprintf(` - { - "distribution": "%s", - "image_request":{ - "architecture": "%s", - "image_type": "edge-commit", - "repositories": [{ - "baseurl": "somerepo.org", - "rhsm": false - }], - "upload_options": { - "region": "eu-central-1" - }, - "ostree": { - "ref": "%s", - "url": "%s", - "parent": "/bad/ref/number/2" - } - } - }`, test_distro.TestDistroName, test_distro.TestArch3Name, ostreeRepoDefault.OSTreeRef, ostreeRepoDefault.Server.URL), http.StatusBadRequest, ` - { - "href": "/api/image-builder-composer/v2/errors/9", - "id": "9", - "kind": "Error", - "code": "IMAGE-BUILDER-COMPOSER-9", - "reason": "Invalid OSTree ref" - }`, "operation_id", "details") - - // incorrect ref for URL - test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "POST", "/api/image-builder-composer/v2/compose", fmt.Sprintf(` - { - "distribution": "%s", - "image_request":{ - "architecture": "%s", - "image_type": "edge-commit", - "repositories": [{ - "baseurl": "somerepo.org", - "rhsm": false - }], - "upload_options": { - "region": "eu-central-1" - }, - "ostree": { - "url": "%s", - "parent": "incorrect/ref" - } - } - }`, test_distro.TestDistroName, test_distro.TestArch3Name, ostreeRepoOther.Server.URL), http.StatusBadRequest, ` - { - "href": "/api/image-builder-composer/v2/errors/10", - "id": "10", - "kind": "Error", - "code": "IMAGE-BUILDER-COMPOSER-10", - "reason": "Error resolving OSTree repo" - }`, "operation_id", "details") - - // parent ref without URL - test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "POST", "/api/image-builder-composer/v2/compose", fmt.Sprintf(` - { - "distribution": "%s", - "image_request":{ - "architecture": "%s", - "image_type": "edge-commit", - "repositories": [{ - "baseurl": "somerepo.org", - "rhsm": false - }], - "upload_options": { - "region": "eu-central-1" - }, - "ostree": { - "parent": "some/ref" - } - } - }`, test_distro.TestDistroName, test_distro.TestArch3Name), http.StatusBadRequest, ` - { - "href": "/api/image-builder-composer/v2/errors/27", - "id": "27", - "kind": "Error", - "code": "IMAGE-BUILDER-COMPOSER-27", - "reason": "Invalid OSTree parameters or parameter combination" - }`, "operation_id", "details") } func TestComposeStatusSuccess(t *testing.T) { @@ -816,6 +714,10 @@ func TestComposeDependencyError(t *testing.T) { "image_request":{ "architecture": "%s", "image_type": "aws", + "ostree": { + "url": "somerepo.org", + "ref": "test" + }, "repositories": [{ "baseurl": "somerepo.org", "rhsm": false @@ -864,7 +766,11 @@ func TestComposeDependencyError(t *testing.T) { "details": [{ "id": 22, "reason": "DNF Error" - }] + }, + { + "id": 34, + "reason": "ostree error" + }] }], "id": 9, "reason": "Manifest dependency failed" diff --git a/test/cases/api.sh b/test/cases/api.sh index d8fe82403..79950c261 100755 --- a/test/cases/api.sh +++ b/test/cases/api.sh @@ -193,9 +193,6 @@ function dump_db() { # Disable -x for these commands to avoid printing the whole result and manifest into the log set +x - # Make sure we get 3 job entries in the db per compose (depsolve + manifest + build) - sudo ${CONTAINER_RUNTIME} exec "${DB_CONTAINER_NAME}" psql -U postgres -d osbuildcomposer -c "SELECT * FROM jobs;" | grep "9 rows" > /dev/null - # Save the result, including the manifest, for the job, straight from the db sudo ${CONTAINER_RUNTIME} exec "${DB_CONTAINER_NAME}" psql -U postgres -d osbuildcomposer -c "SELECT result FROM jobs WHERE type='manifest-id-only'" \ | sudo tee "${ARTIFACTS}/build-result.txt"