From 6d51d285cfeec4cd3b74433f590acfdc166ce16f Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Fri, 12 Mar 2021 16:45:26 +0100 Subject: [PATCH] GCP: accept context from the caller in all methods Modify all relevant methods in the internal GCP library to accept context from the caller. Modify all places which call the internal GCP library methods to pass the context. Signed-off-by: Tomas Hozza --- cmd/cloud-cleaner/main.go | 13 ++++++---- cmd/osbuild-upload-gcp/main.go | 13 ++++++---- cmd/osbuild-worker/jobimpl-osbuild.go | 12 ++++++---- internal/cloud/gcp/compute.go | 34 +++++++++++++++++---------- internal/cloud/gcp/storage.go | 15 ++++-------- 5 files changed, 49 insertions(+), 38 deletions(-) diff --git a/cmd/cloud-cleaner/main.go b/cmd/cloud-cleaner/main.go index 6fc80e446..411171bd0 100644 --- a/cmd/cloud-cleaner/main.go +++ b/cmd/cloud-cleaner/main.go @@ -3,6 +3,7 @@ package main import ( + "context" "crypto/sha256" "fmt" "log" @@ -59,10 +60,12 @@ func cleanupGCP(testID string, wg *sync.WaitGroup) { return } + ctx := context.Background() + // Try to delete potentially running instance log.Printf("[GCP] ๐Ÿงน Deleting VM instance %s in %s. "+ "This should fail if the test succedded.", GCPInstance, GCPZone) - err = g.ComputeInstanceDelete(GCPZone, GCPInstance) + err = g.ComputeInstanceDelete(ctx, GCPZone, GCPInstance) if err != nil { log.Printf("[GCP] Error: %v", err) } @@ -70,7 +73,7 @@ func cleanupGCP(testID string, wg *sync.WaitGroup) { // Try to clean up storage of cache objects after image import job log.Println("[GCP] ๐Ÿงน Cleaning up cache objects from storage after image " + "import. This should fail if the test succedded.") - cacheObjects, errs := g.StorageImageImportCleanup(GCPImage) + cacheObjects, errs := g.StorageImageImportCleanup(ctx, GCPImage) for _, err = range errs { log.Printf("[GCP] Error: %v", err) } @@ -79,12 +82,12 @@ func cleanupGCP(testID string, wg *sync.WaitGroup) { } // Try to find the potentially uploaded Storage objects using custom metadata - objects, err := g.StorageListObjectsByMetadata(GCPBucket, map[string]string{gcp.MetadataKeyImageName: GCPImage}) + objects, err := g.StorageListObjectsByMetadata(ctx, GCPBucket, map[string]string{gcp.MetadataKeyImageName: GCPImage}) if err != nil { log.Printf("[GCP] Error: %v", err) } for _, obj := range objects { - if err = g.StorageObjectDelete(obj.Bucket, obj.Name); err != nil { + if err = g.StorageObjectDelete(ctx, obj.Bucket, obj.Name); err != nil { log.Printf("[GCP] Error: %v", err) } log.Printf("[GCP] ๐Ÿงน Deleted object %s/%s related to build of image %s", obj.Bucket, obj.Name, GCPImage) @@ -92,7 +95,7 @@ func cleanupGCP(testID string, wg *sync.WaitGroup) { // Try to delete the imported image log.Printf("[GCP] ๐Ÿงน Deleting image %s. This should fail if the test succedded.", GCPImage) - err = g.ComputeImageDelete(GCPImage) + err = g.ComputeImageDelete(ctx, GCPImage) if err != nil { log.Printf("[GCP] Error: %v", err) } diff --git a/cmd/osbuild-upload-gcp/main.go b/cmd/osbuild-upload-gcp/main.go index 54431bb88..069136614 100644 --- a/cmd/osbuild-upload-gcp/main.go +++ b/cmd/osbuild-upload-gcp/main.go @@ -1,6 +1,7 @@ package main import ( + "context" "flag" "fmt" "io/ioutil" @@ -60,10 +61,12 @@ func main() { log.Fatalf("[GCP] Failed to create new GCP object: %v", err) } + ctx := context.Background() + // Upload image to the Storage if !skipUpload { log.Printf("[GCP] ๐Ÿš€ Uploading image to: %s/%s", bucketName, objectName) - _, err := g.StorageObjectUpload(imageFile, bucketName, objectName, + _, err := g.StorageObjectUpload(ctx, imageFile, bucketName, objectName, map[string]string{gcp.MetadataKeyImageName: imageName}) if err != nil { log.Fatalf("[GCP] Uploading image failed: %v", err) @@ -73,7 +76,7 @@ func main() { // Import Image to Compute Node if !skipImport { log.Printf("[GCP] ๐Ÿ“ฅ Importing image into Compute Node as '%s'", imageName) - imageBuild, importErr := g.ComputeImageImport(bucketName, objectName, imageName, osFamily, region) + imageBuild, importErr := g.ComputeImageImport(ctx, bucketName, objectName, imageName, osFamily, region) if imageBuild != nil { log.Printf("[GCP] ๐Ÿ“œ Image import log URL: %s", imageBuild.LogUrl) log.Printf("[GCP] ๐ŸŽ‰ Image import finished with status: %s", imageBuild.Status) @@ -81,11 +84,11 @@ func main() { // Cleanup storage before checking for errors log.Printf("[GCP] ๐Ÿงน Deleting uploaded image file: %s/%s", bucketName, objectName) - if err = g.StorageObjectDelete(bucketName, objectName); err != nil { + if err = g.StorageObjectDelete(ctx, bucketName, objectName); err != nil { log.Printf("[GCP] Encountered error while deleting object: %v", err) } - deleted, errs := g.StorageImageImportCleanup(imageName) + deleted, errs := g.StorageImageImportCleanup(ctx, imageName) for _, d := range deleted { log.Printf("[GCP] ๐Ÿงน Deleted image import job file '%s'", d) } @@ -103,7 +106,7 @@ func main() { // Share the imported Image with specified accounts using IAM policy if len(shareWith) > 0 { log.Printf("[GCP] ๐Ÿ”— Sharing the image with: %+v", shareWith) - err = g.ComputeImageShare(imageName, []string(shareWith)) + err = g.ComputeImageShare(ctx, imageName, []string(shareWith)) if err != nil { log.Fatalf("[GCP] Sharing image failed: %s", err) } diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index 42ef0b6c3..79bca4218 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/cmd/osbuild-worker/jobimpl-osbuild.go @@ -271,6 +271,8 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { osbuildJobResult.Success = true osbuildJobResult.UploadStatus = "success" case *target.GCPTargetOptions: + ctx := context.Background() + g, err := gcp.New(impl.GCPCreds) if err != nil { appendTargetError(osbuildJobResult, err) @@ -278,7 +280,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { } log.Printf("[GCP] ๐Ÿš€ Uploading image to: %s/%s", options.Bucket, options.Object) - _, err = g.StorageObjectUpload(path.Join(outputDirectory, options.Filename), + _, err = g.StorageObjectUpload(ctx, path.Join(outputDirectory, options.Filename), options.Bucket, options.Object, map[string]string{gcp.MetadataKeyImageName: args.Targets[0].ImageName}) if err != nil { appendTargetError(osbuildJobResult, err) @@ -286,7 +288,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { } log.Printf("[GCP] ๐Ÿ“ฅ Importing image into Compute Node as '%s'", args.Targets[0].ImageName) - imageBuild, importErr := g.ComputeImageImport(options.Bucket, options.Object, args.Targets[0].ImageName, options.Os, options.Region) + imageBuild, importErr := g.ComputeImageImport(ctx, options.Bucket, options.Object, args.Targets[0].ImageName, options.Os, options.Region) if imageBuild != nil { log.Printf("[GCP] ๐Ÿ“œ Image import log URL: %s", imageBuild.LogUrl) log.Printf("[GCP] ๐ŸŽ‰ Image import finished with status: %s", imageBuild.Status) @@ -294,11 +296,11 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { // Cleanup storage before checking for errors log.Printf("[GCP] ๐Ÿงน Deleting uploaded image file: %s/%s", options.Bucket, options.Object) - if err = g.StorageObjectDelete(options.Bucket, options.Object); err != nil { + if err = g.StorageObjectDelete(ctx, options.Bucket, options.Object); err != nil { log.Printf("[GCP] Encountered error while deleting object: %v", err) } - deleted, errs := g.StorageImageImportCleanup(args.Targets[0].ImageName) + deleted, errs := g.StorageImageImportCleanup(ctx, args.Targets[0].ImageName) for _, d := range deleted { log.Printf("[GCP] ๐Ÿงน Deleted image import job file '%s'", d) } @@ -315,7 +317,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { if len(options.ShareWithAccounts) > 0 { log.Printf("[GCP] ๐Ÿ”— Sharing the image with: %+v", options.ShareWithAccounts) - err = g.ComputeImageShare(args.Targets[0].ImageName, options.ShareWithAccounts) + err = g.ComputeImageShare(ctx, args.Targets[0].ImageName, options.ShareWithAccounts) if err != nil { appendTargetError(osbuildJobResult, err) return nil diff --git a/internal/cloud/gcp/compute.go b/internal/cloud/gcp/compute.go index 80b5638d6..a0b21d85f 100644 --- a/internal/cloud/gcp/compute.go +++ b/internal/cloud/gcp/compute.go @@ -43,8 +43,7 @@ import ( // // Uses: // - Cloud Build API -func (g *GCP) ComputeImageImport(bucket, object, imageName, os, region string) (*cloudbuildpb.Build, error) { - ctx := context.Background() +func (g *GCP) ComputeImageImport(ctx context.Context, bucket, object, imageName, os, region string) (*cloudbuildpb.Build, error) { cloudbuildClient, err := cloudbuild.NewClient(ctx, option.WithCredentials(g.creds)) if err != nil { return nil, fmt.Errorf("failed to get Cloud Build client: %v", err) @@ -105,6 +104,24 @@ func (g *GCP) ComputeImageImport(bucket, object, imageName, os, region string) ( // Wait for the build to finish for { + select { + case <-time.After(30 * time.Second): + // Just check the build status below + case <-ctx.Done(): + // cancel the build + cancelBuildReq := &cloudbuildpb.CancelBuildRequest{ + ProjectId: imageBuild.ProjectId, + Id: imageBuild.Id, + } + // since the provided ctx has been canceled, create a new one to cancel the build + ctx = context.Background() + // !NOTE: Cancelling the build leaves behind all resources that it created and didn't manage to clean up + imageBuild, err = cloudbuildClient.CancelBuild(ctx, cancelBuildReq) + if err != nil { + return imageBuild, fmt.Errorf("failed to cancel the image import build job: %v", err) + } + } + imageBuild, err = cloudbuildClient.GetBuild(ctx, getBuldReq) if err != nil { return imageBuild, fmt.Errorf("failed to get the build info: %v", err) @@ -113,7 +130,6 @@ func (g *GCP) ComputeImageImport(bucket, object, imageName, os, region string) ( if imageBuild.Status != cloudbuildpb.Build_WORKING && imageBuild.Status != cloudbuildpb.Build_QUEUED { break } - time.Sleep(time.Second * 30) } if imageBuild.Status != cloudbuildpb.Build_SUCCESS { @@ -148,9 +164,7 @@ func (g *GCP) ComputeImageURL(imageName string) string { // // Uses: // - Compute Engine API -func (g *GCP) ComputeImageShare(imageName string, shareWith []string) error { - ctx := context.Background() - +func (g *GCP) ComputeImageShare(ctx context.Context, imageName string, shareWith []string) error { computeService, err := compute.NewService(ctx, option.WithCredentials(g.creds)) if err != nil { return fmt.Errorf("failed to get Compute Engine client: %v", err) @@ -210,9 +224,7 @@ func (g *GCP) ComputeImageShare(imageName string, shareWith []string) error { // // Uses: // - Compute Engine API -func (g *GCP) ComputeImageDelete(image string) error { - ctx := context.Background() - +func (g *GCP) ComputeImageDelete(ctx context.Context, image string) error { computeService, err := compute.NewService(ctx, option.WithCredentials(g.creds)) if err != nil { return fmt.Errorf("failed to get Compute Engine client: %v", err) @@ -229,9 +241,7 @@ func (g *GCP) ComputeImageDelete(image string) error { // // Uses: // - Compute Engine API -func (g *GCP) ComputeInstanceDelete(zone, instance string) error { - ctx := context.Background() - +func (g *GCP) ComputeInstanceDelete(ctx context.Context, zone, instance string) error { computeService, err := compute.NewService(ctx, option.WithCredentials(g.creds)) if err != nil { return fmt.Errorf("failed to get Compute Engine client: %v", err) diff --git a/internal/cloud/gcp/storage.go b/internal/cloud/gcp/storage.go index b5aa63bb3..6b3d548c7 100644 --- a/internal/cloud/gcp/storage.go +++ b/internal/cloud/gcp/storage.go @@ -31,9 +31,7 @@ const ( // // Uses: // - Storage API -func (g *GCP) StorageObjectUpload(filename, bucket, object string, metadata map[string]string) (*storage.ObjectAttrs, error) { - ctx := context.Background() - +func (g *GCP) StorageObjectUpload(ctx context.Context, filename, bucket, object string, metadata map[string]string) (*storage.ObjectAttrs, error) { storageClient, err := storage.NewClient(ctx, option.WithCredentials(g.creds)) if err != nil { return nil, fmt.Errorf("failed to get Storage client: %v", err) @@ -85,9 +83,7 @@ func (g *GCP) StorageObjectUpload(filename, bucket, object string, metadata map[ // // Uses: // - Storage API -func (g *GCP) StorageObjectDelete(bucket, object string) error { - ctx := context.Background() - +func (g *GCP) StorageObjectDelete(ctx context.Context, bucket, object string) error { storageClient, err := storage.NewClient(ctx, option.WithCredentials(g.creds)) if err != nil { return fmt.Errorf("failed to get Storage client: %v", err) @@ -114,12 +110,10 @@ func (g *GCP) StorageObjectDelete(bucket, object string) error { // Uses: // - Compute Engine API // - Storage API -func (g *GCP) StorageImageImportCleanup(imageName string) ([]string, []error) { +func (g *GCP) StorageImageImportCleanup(ctx context.Context, imageName string) ([]string, []error) { var deletedObjects []string var errors []error - ctx := context.Background() - storageClient, err := storage.NewClient(ctx, option.WithCredentials(g.creds)) if err != nil { errors = append(errors, fmt.Errorf("failed to get Storage client: %v", err)) @@ -205,9 +199,8 @@ func (g *GCP) StorageImageImportCleanup(imageName string) ([]string, []error) { // // Uses: // - Storage API -func (g *GCP) StorageListObjectsByMetadata(bucket string, metadata map[string]string) ([]*storage.ObjectAttrs, error) { +func (g *GCP) StorageListObjectsByMetadata(ctx context.Context, bucket string, metadata map[string]string) ([]*storage.ObjectAttrs, error) { var matchedObjectAttr []*storage.ObjectAttrs - ctx := context.Background() storageClient, err := storage.NewClient(ctx, option.WithCredentials(g.creds)) if err != nil {