particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

osbuild-worker: implement structured errors

Browse source 
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.
This commit is contained in:
Gianluca Zuccarelli 2021-12-23 13:45:30 +00:00 • committed by Sanne Raymaekers
parent daf24f8db3
commit cc981b887a
8 changed files with 142 additions and 68 deletions
Download patch file Download diff file Expand all files Collapse all files

19
cmd/osbuild-worker/jobimpl-depsolve.go
View file

@ -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)

9
cmd/osbuild-worker/jobimpl-koji-finalize.go
View file

@ -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
}
}

3
cmd/osbuild-worker/jobimpl-koji-init.go
View file

@ -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)

5
cmd/osbuild-worker/jobimpl-osbuild-koji.go
View file

@ -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())
}
}
}

84
cmd/osbuild-worker/jobimpl-osbuild.go
View file

@ -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
}
}