From cc981b887a686bbc7d9e8529b659ee65b7f4b05d Mon Sep 17 00:00:00 2001 From: Gianluca Zuccarelli Date: Thu, 23 Dec 2021 13:45:30 +0000 Subject: [PATCH] osbuild-worker: implement structured errors Implement the structured errors as defined by the worker client. Every error for each of the job types now returns a structured error with a reason and a specific error code. This will make it possible to differentiate between 4xx errors and 5xx errors. This commit refactors the way errors are implemented in the workers, but maintains backwards compatability in composer by checking for both kinds of errors. --- cmd/osbuild-worker/jobimpl-depsolve.go | 19 ++++- cmd/osbuild-worker/jobimpl-koji-finalize.go | 9 ++- cmd/osbuild-worker/jobimpl-koji-init.go | 3 +- cmd/osbuild-worker/jobimpl-osbuild-koji.go | 5 +- cmd/osbuild-worker/jobimpl-osbuild.go | 84 ++++++++++++--------- internal/cloudapi/v2/v2.go | 34 ++++++--- internal/kojiapi/server.go | 15 ++-- internal/worker/server.go | 41 +++++++++- 8 files changed, 142 insertions(+), 68 deletions(-) diff --git a/cmd/osbuild-worker/jobimpl-depsolve.go b/cmd/osbuild-worker/jobimpl-depsolve.go index 85018974b..49a703092 100644 --- a/cmd/osbuild-worker/jobimpl-depsolve.go +++ b/cmd/osbuild-worker/jobimpl-depsolve.go @@ -5,6 +5,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/rpmmd" "github.com/osbuild/osbuild-composer/internal/worker" + "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" ) type DepsolveJobImpl struct { @@ -40,13 +41,23 @@ func (impl *DepsolveJobImpl) Run(job worker.Job) error { var result worker.DepsolveJobResult result.PackageSpecs, err = impl.depsolve(args.PackageSets, args.Repos, args.ModulePlatformID, args.Arch, args.Releasever, args.PackageSetsRepositories) if err != nil { - switch err.(type) { + switch e := err.(type) { case *rpmmd.DNFError: - result.ErrorType = worker.DepsolveErrorType + // Error originates from dnf-json (the http call dnf-json wasn't StatusOK) + switch e.Kind { + case "DepsolveError": + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDNFDepsolveError, err.Error()) + case "MarkingError": + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDNFMarkingError, err.Error()) + default: + // This still has the kind/reason format but a kind that's returned + // by dnf-json and not explicitly handled here. + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDNFOtherError, err.Error()) + } case error: - result.ErrorType = worker.OtherErrorType + // Error originates from internal/rpmmd, not from dnf-json + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorRPMMDError, err.Error()) } - result.Error = err.Error() } err = job.Update(&result) diff --git a/cmd/osbuild-worker/jobimpl-koji-finalize.go b/cmd/osbuild-worker/jobimpl-koji-finalize.go index baf39bafb..48271187b 100644 --- a/cmd/osbuild-worker/jobimpl-koji-finalize.go +++ b/cmd/osbuild-worker/jobimpl-koji-finalize.go @@ -12,6 +12,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/rpmmd" "github.com/osbuild/osbuild-composer/internal/upload/koji" "github.com/osbuild/osbuild-composer/internal/worker" + "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" ) type KojiFinalizeJobImpl struct { @@ -113,7 +114,7 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error { // Update the status immediately and bail out. var result worker.KojiFinalizeJobResult if err != nil { - result.KojiError = err.Error() + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorKojiFailedDependency, err.Error()) } err = job.Update(&result) if err != nil { @@ -193,7 +194,7 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error { var result worker.KojiFinalizeJobResult err = impl.kojiImport(args.Server, build, buildRoots, images, args.KojiDirectory, initArgs.Token) if err != nil { - result.KojiError = err.Error() + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorKojiFinalize, err.Error()) } err = job.Update(&result) @@ -227,12 +228,12 @@ func extractDynamicArgs(job worker.Job) (*worker.KojiInitJobResult, []worker.OSB // Returns true if any of koji-finalize dependencies failed. func hasFailedDependency(kojiInitResult worker.KojiInitJobResult, osbuildKojiResults []worker.OSBuildKojiJobResult) bool { - if kojiInitResult.KojiError != "" { + if kojiInitResult.JobError != nil { return true } for _, r := range osbuildKojiResults { - if !r.OSBuildOutput.Success || r.KojiError != "" { + if !r.OSBuildOutput.Success || r.JobError != nil { return true } } diff --git a/cmd/osbuild-worker/jobimpl-koji-init.go b/cmd/osbuild-worker/jobimpl-koji-init.go index 4df408587..ee4b46587 100644 --- a/cmd/osbuild-worker/jobimpl-koji-init.go +++ b/cmd/osbuild-worker/jobimpl-koji-init.go @@ -10,6 +10,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/upload/koji" "github.com/osbuild/osbuild-composer/internal/worker" + "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" ) type KojiInitJobImpl struct { @@ -64,7 +65,7 @@ func (impl *KojiInitJobImpl) Run(job worker.Job) error { var result worker.KojiInitJobResult result.Token, result.BuildID, err = impl.kojiInit(args.Server, args.Name, args.Version, args.Release) if err != nil { - result.KojiError = err.Error() + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorKojiInit, err.Error()) } err = job.Update(&result) diff --git a/cmd/osbuild-worker/jobimpl-osbuild-koji.go b/cmd/osbuild-worker/jobimpl-osbuild-koji.go index e24c7554b..5a955867c 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild-koji.go +++ b/cmd/osbuild-worker/jobimpl-osbuild-koji.go @@ -13,6 +13,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/upload/koji" "github.com/osbuild/osbuild-composer/internal/worker" + "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" "github.com/sirupsen/logrus" ) @@ -86,7 +87,7 @@ func (impl *OSBuildKojiJobImpl) Run(job worker.Job) error { return err } - if initArgs.KojiError == "" { + if initArgs.JobError != nil { exports := args.Exports if len(exports) == 0 { // job did not define exports, likely coming from an older version of composer @@ -112,7 +113,7 @@ func (impl *OSBuildKojiJobImpl) Run(job worker.Job) error { } result.ImageHash, result.ImageSize, err = impl.kojiUpload(f, args.KojiServer, args.KojiDirectory, args.KojiFilename) if err != nil { - result.KojiError = err.Error() + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorKojiBuild, err.Error()) } } } diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index 255558e66..39d811812 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/cmd/osbuild-worker/jobimpl-osbuild.go @@ -20,6 +20,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/upload/koji" "github.com/osbuild/osbuild-composer/internal/upload/vmware" "github.com/osbuild/osbuild-composer/internal/worker" + "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" ) type OSBuildJobImpl struct { @@ -31,12 +32,6 @@ type OSBuildJobImpl struct { AWSCreds string } -func appendTargetError(res *worker.OSBuildJobResult, err error) { - errStr := err.Error() - logrus.Errorf("target failed: %s", errStr) - res.TargetErrors = append(res.TargetErrors, errStr) -} - // Returns an *awscloud.AWS object with the credentials of the request. If they // are not accessible, then try to use the one obtained in the worker // configuration. @@ -48,6 +43,23 @@ func (impl *OSBuildJobImpl) getAWS(region string, accessId string, secret string } } +func validateResult(result *worker.OSBuildJobResult, jobID string) { + logWithId := logrus.WithField("jobId", jobID) + if result.JobError != nil { + logWithId.Errorf("osbuild job failed: %s", result.JobError.Reason) + return + } + // if the job failed, but the JobError is + // nil, we still need to handle this as an error + if !result.OSBuildOutput.Success { + reason := "osbuild job was unsuccessful" + logWithId.Errorf("osbuild job failed: %s", reason) + result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, reason) + return + } + result.Success = true +} + func (impl *OSBuildJobImpl) Run(job worker.Job) error { logWithId := logrus.WithField("jobId", job.Id().String()) // Initialize variable needed for reporting back to osbuild-composer. @@ -63,6 +75,8 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { // In all cases it is necessary to report result back to osbuild-composer worker API. defer func() { + validateResult(osbuildJobResult, job.Id().String()) + err := job.Update(osbuildJobResult) if err != nil { logWithId.Errorf("Error reporting job result: %v", err) @@ -95,7 +109,8 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { } // skip the job if the manifest generation failed - if manifestJR.Error != "" { + if manifestJR.JobError != nil { + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorManifestDependency, "Manifest dependency failed") return nil } args.Manifest = manifestJR.Manifest @@ -125,6 +140,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { osbuildJobResult.OSBuildOutput, err = RunOSBuild(args.Manifest, impl.Store, outputDirectory, exports, os.Stderr) // First handle the case when "running" osbuild failed if err != nil { + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "osbuild build failed") return err } @@ -152,6 +168,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { // Second handle the case when the build failed, but osbuild finished successfully if !osbuildJobResult.OSBuildOutput.Success { + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "osbuild build failed") return nil } @@ -200,7 +217,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { tempDirectory, err := ioutil.TempDir(impl.Output, job.Id().String()+"-vmware-*") if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, err.Error()) return nil } @@ -216,13 +233,13 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { imagePath := path.Join(tempDirectory, imageName) err = os.Symlink(streamOptimizedPath, imagePath) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, err.Error()) return nil } err = vmware.UploadImage(credentials, imagePath) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, err.Error()) return nil } @@ -231,7 +248,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { case *target.AWSTargetOptions: a, err := impl.getAWS(options.Region, options.AccessKeyID, options.SecretAccessKey, options.SessionToken) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, err.Error()) return nil } @@ -242,18 +259,18 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { _, err = a.Upload(path.Join(outputDirectory, exportPath, options.Filename), options.Bucket, key) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, err.Error()) return nil } ami, err := a.Register(args.Targets[0].ImageName, options.Bucket, key, options.ShareWithAccounts, common.CurrentArch()) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorImportingImage, err.Error()) return nil } if ami == nil { - appendTargetError(osbuildJobResult, fmt.Errorf("No ami returned")) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorImportingImage, "No ami returned") return nil } @@ -267,7 +284,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { case *target.AWSS3TargetOptions: a, err := impl.getAWS(options.Region, options.AccessKeyID, options.SecretAccessKey, options.SessionToken) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, err.Error()) return nil } @@ -279,12 +296,12 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { _, err = a.Upload(path.Join(outputDirectory, exportPath, options.Filename), options.Bucket, key) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, err.Error()) return nil } url, err := a.S3ObjectPresignedURL(options.Bucket, key) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, err.Error()) return nil } @@ -295,7 +312,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { case *target.AzureTargetOptions: azureStorageClient, err := azure.NewStorageClient(options.StorageAccount, options.StorageAccessKey) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, err.Error()) return err } @@ -313,7 +330,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { ) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, err.Error()) return nil } @@ -324,7 +341,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { g, err := gcp.New(impl.GCPCreds) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, err.Error()) return nil } @@ -332,7 +349,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { _, err = g.StorageObjectUpload(ctx, path.Join(outputDirectory, exportPath, options.Filename), options.Bucket, options.Object, map[string]string{gcp.MetadataKeyImageName: args.Targets[0].ImageName}) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, err.Error()) return nil } @@ -360,7 +377,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { // check error from ComputeImageImport() if importErr != nil { - appendTargetError(osbuildJobResult, importErr) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorImportingImage, importErr.Error()) return nil } logWithId.Infof("[GCP] 💿 Image URL: %s", g.ComputeImageURL(args.Targets[0].ImageName)) @@ -369,7 +386,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { logWithId.Infof("[GCP] 🔗 Sharing the image with: %+v", options.ShareWithAccounts) err = g.ComputeImageShare(ctx, args.Targets[0].ImageName, options.ShareWithAccounts) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, err.Error()) return nil } } @@ -385,13 +402,13 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { ctx := context.Background() if impl.AzureCreds == nil { - appendTargetError(osbuildJobResult, fmt.Errorf("osbuild job has org.osbuild.azure.image target but this worker doesn't have azure credentials")) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, "osbuild job has org.osbuild.azure.image target but this worker doesn't have azure credentials") return nil } c, err := azure.NewClient(*impl.AzureCreds, options.TenantID) if err != nil { - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidTargetConfig, err.Error()) return nil } logWithId.Info("[Azure] 🔑 Logged in Azure") @@ -408,7 +425,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { storageAccountTag, ) if err != nil { - appendTargetError(osbuildJobResult, fmt.Errorf("searching for a storage account failed: %v", err)) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidTargetConfig, fmt.Sprintf("searching for a storage account failed: %v", err)) return nil } @@ -426,7 +443,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { storageAccountTag, ) if err != nil { - appendTargetError(osbuildJobResult, fmt.Errorf("creating a new storage account failed: %v", err)) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidTargetConfig, fmt.Sprintf("creating a new storage account failed: %v", err)) return nil } } @@ -439,13 +456,13 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { storageAccount, ) if err != nil { - appendTargetError(osbuildJobResult, fmt.Errorf("retrieving the storage account key failed: %v", err)) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidTargetConfig, fmt.Sprintf("retrieving the storage account key failed: %v", err)) return nil } azureStorageClient, err := azure.NewStorageClient(storageAccount, storageAccessKey) if err != nil { - appendTargetError(osbuildJobResult, fmt.Errorf("creating the storage client failed: %v", err)) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidTargetConfig, fmt.Sprintf("creating the storage client failed: %v", err)) return nil } @@ -454,7 +471,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { logWithId.Info("[Azure] 📦 Ensuring that we have a storage container") err = azureStorageClient.CreateStorageContainerIfNotExist(ctx, storageAccount, storageContainer) if err != nil { - appendTargetError(osbuildJobResult, fmt.Errorf("cannot create a storage container: %v", err)) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidTargetConfig, fmt.Sprintf("cannot create a storage container: %v", err)) return nil } @@ -474,7 +491,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { azure.DefaultUploadThreads, ) if err != nil { - appendTargetError(osbuildJobResult, fmt.Errorf("uploading the image failed: %v", err)) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, fmt.Sprintf("uploading the image failed: %v", err)) return nil } @@ -490,7 +507,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { options.Location, ) if err != nil { - appendTargetError(osbuildJobResult, fmt.Errorf("registering the image failed: %v", err)) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorImportingImage, fmt.Sprintf("registering the image failed: %v", err)) return nil } @@ -503,8 +520,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { osbuildJobResult.Success = true osbuildJobResult.UploadStatus = "success" default: - err = fmt.Errorf("invalid target type: %s", args.Targets[0].Name) - appendTargetError(osbuildJobResult, err) + osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidTarget, fmt.Sprintf("invalid target type: %s", args.Targets[0].Name)) return nil } } diff --git a/internal/cloudapi/v2/v2.go b/internal/cloudapi/v2/v2.go index fdcfc7fe5..4537e8b91 100644 --- a/internal/cloudapi/v2/v2.go +++ b/internal/cloudapi/v2/v2.go @@ -28,6 +28,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/rpmmd" "github.com/osbuild/osbuild-composer/internal/target" "github.com/osbuild/osbuild-composer/internal/worker" + "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" ) // Server represents the state of the cloud Server @@ -426,7 +427,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { manifestJobContext, manifestCancel := context.WithTimeout(context.Background(), time.Minute*5) // start 1 goroutine which requests datajob type - go func(workers *worker.Server, manifestJobID uuid.UUID, b *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, seed int64) { + go func(workers *worker.Server, manifestJobID uuid.UUID, b *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, seed int64, depsolveJobID uuid.UUID) { defer manifestCancel() // wait until job is in a pending state var token uuid.UUID @@ -454,12 +455,11 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { var jobResult *worker.ManifestJobByIDResult = &worker.ManifestJobByIDResult{ Manifest: nil, - Error: "", } defer func() { - if jobResult.Error != "" { - logrus.Errorf("Error in manifest job %v: %v", manifestJobID, jobResult.Error) + if jobResult.JobError != nil { + logrus.Errorf("Error in manifest job %v: %v", manifestJobID, jobResult.JobError.Reason) } result, err := json.Marshal(jobResult) @@ -474,34 +474,44 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { }() if len(dynArgs) == 0 { - jobResult.Error = "No dynamic arguments" + reason := "No dynamic arguments" + jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorNoDynamicArgs, reason) return } var depsolveResults worker.DepsolveJobResult err = json.Unmarshal(dynArgs[0], &depsolveResults) if err != nil { - jobResult.Error = "Error parsing dynamic arguments" + reason := "Error parsing dynamic arguments" + jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorParsingDynamicArgs, reason) return } - if depsolveResults.Error != "" { - if depsolveResults.ErrorType == worker.DepsolveErrorType { - jobResult.Error = "Error in depsolve job dependency input, bad request" + _, _, err = workers.JobStatus(depsolveJobID, &depsolveResults) + if err != nil { + reason := "Error reading depsolve status" + jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorReadingJobStatus, reason) + return + } + + if jobErr := depsolveResults.JobError; jobErr != nil { + if jobErr.ID == clienterrors.ErrorDNFDepsolveError || jobErr.ID == clienterrors.ErrorDNFMarkingError { + jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDepsolveDependency, "Error in depsolve job dependency input, bad package set requested") return } - jobResult.Error = "Error in depsolve job dependency" + jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDepsolveDependency, "Error in depsolve job dependency") return } manifest, err := imageType.Manifest(b, options, repos, depsolveResults.PackageSpecs, seed) if err != nil { - jobResult.Error = "Error generating manifest" + reason := "Error generating manifest" + jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorManifestGeneration, reason) return } jobResult.Manifest = manifest - }(h.server.workers, manifestJobID, blueprintCustoms, imageOptions, repositories, manifestSeed) + }(h.server.workers, manifestJobID, blueprintCustoms, imageOptions, repositories, manifestSeed, depsolveJobID) return ctx.JSON(http.StatusCreated, &ComposeId{ ObjectReference: ObjectReference{ diff --git a/internal/kojiapi/server.go b/internal/kojiapi/server.go index 769a7f51b..5f396a6fe 100644 --- a/internal/kojiapi/server.go +++ b/internal/kojiapi/server.go @@ -216,8 +216,9 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { } time.Sleep(500 * time.Millisecond) } - if initResult.KojiError != "" { - return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Could not initialize build with koji: %v", initResult.KojiError)) + + if initResult.JobError != nil { + return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Could not initialize build with koji: %v", initResult.JobError.Reason)) } return ctx.JSON(http.StatusCreated, &api.ComposeResponse{ @@ -254,7 +255,7 @@ func composeStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiIni return api.ComposeStatusValuePending } - if initResult.KojiError != "" { + if initResult.JobError != nil { return api.ComposeStatusValueFailure } @@ -262,12 +263,12 @@ func composeStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiIni if buildResult.OSBuildOutput != nil && !buildResult.OSBuildOutput.Success { return api.ComposeStatusValueFailure } - if buildResult.KojiError != "" { + if buildResult.JobError != nil { return api.ComposeStatusValueFailure } } - if result.KojiError != "" { + if result.JobError != nil { return api.ComposeStatusValueFailure } @@ -279,7 +280,7 @@ func imageStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiInitJ return api.ImageStatusValueFailure } - if initResult.KojiError != "" { + if initResult.JobError != nil { return api.ImageStatusValueFailure } @@ -291,7 +292,7 @@ func imageStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiInitJ return api.ImageStatusValueBuilding } - if buildResult.OSBuildOutput != nil && buildResult.OSBuildOutput.Success && buildResult.KojiError == "" { + if buildResult.OSBuildOutput != nil && buildResult.OSBuildOutput.Success && buildResult.JobError == nil { return api.ImageStatusValueSuccess } diff --git a/internal/worker/server.go b/internal/worker/server.go index 23faf5689..b33955e13 100644 --- a/internal/worker/server.go +++ b/internal/worker/server.go @@ -22,6 +22,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/jobqueue" "github.com/osbuild/osbuild-composer/internal/worker/api" + "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" ) type Server struct { @@ -131,11 +132,43 @@ func (s *Server) JobStatus(id uuid.UUID, result interface{}) (*JobStatus, []uuid } } - // For backwards compatibility: OSBuildJobResult didn't use to have a - // top-level `Success` flag. Override it here by looking into the job. - if r, ok := result.(*OSBuildJobResult); ok { + switch r := result.(type) { + case *KojiInitJobResult: + if r.JobError == nil && r.KojiError != "" { + r.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOldResultCompatible, r.KojiError) + } + case *KojiFinalizeJobResult: + if r.JobError == nil && r.KojiError != "" { + r.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOldResultCompatible, r.KojiError) + } + case *OSBuildKojiJobResult: + if r.JobError == nil && r.KojiError != "" { + r.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOldResultCompatible, r.KojiError) + } + case *ManifestJobByIDResult: + if r.JobError == nil && r.Error != "" { + r.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOldResultCompatible, r.Error) + } + case *DepsolveJobResult: + if r.JobError == nil && r.Error != "" { + if r.ErrorType == DepsolveErrorType { + r.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDNFDepsolveError, r.Error) + } else { + r.JobError = clienterrors.WorkerClientError(clienterrors.ErrorRPMMDError, r.Error) + } + } + case *OSBuildJobResult: + if r.JobError == nil && len(r.TargetErrors) > 0 || (r.OSBuildOutput != nil && len(r.OSBuildOutput.Error) > 0) { + if r.OSBuildOutput != nil && len(r.OSBuildOutput.Error) > 0 { + r.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOldResultCompatible, string(r.OSBuildOutput.Error)) + } else if len(r.TargetErrors) > 0 { + r.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOldResultCompatible, r.TargetErrors[0]) + } + } + // For backwards compatibility: OSBuildJobResult didn't use to have a + // top-level `Success` flag. Override it here by looking into the job. if !r.Success && r.OSBuildOutput != nil { - r.Success = r.OSBuildOutput.Success && len(r.TargetErrors) == 0 + r.Success = r.OSBuildOutput.Success && r.JobError == nil } }