From 4f72e1c2d579134557e061820d434b565e873a62 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Fri, 10 Nov 2023 11:04:13 -0500 Subject: [PATCH] lib: change CreateImage prototype The CreateImage and CreateImageFromRaw functions fropm the GCE driver were very similar and contained a lot of duplicated code, for very little changes in between the two calls. Instead of having both with a lot of parameters, we simplify the function to accept an image specification, as exposed by the GCE client library, so the driver only takes care of performing the call, and manage its lifecycle through channels, then returning the image after it's created. --- builder/googlecompute/step_create_image.go | 26 +++++- .../googlecompute/step_create_image_test.go | 29 +++--- lib/common/driver.go | 5 +- lib/common/driver_gce.go | 88 +------------------ lib/common/driver_mock.go | 77 ++++++---------- .../googlecompute-import/post-processor.go | 21 ++++- 6 files changed, 82 insertions(+), 164 deletions(-) diff --git a/builder/googlecompute/step_create_image.go b/builder/googlecompute/step_create_image.go index 39e1b80a..96c8d351 100644 --- a/builder/googlecompute/step_create_image.go +++ b/builder/googlecompute/step_create_image.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/packer-plugin-googlecompute/lib/common" "github.com/hashicorp/packer-plugin-sdk/multistep" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "google.golang.org/api/compute/v1" ) // StepCreateImage represents a Packer build step that creates GCE machine @@ -47,10 +48,27 @@ func (s *StepCreateImage) Run(ctx context.Context, state multistep.StateBag) mul ui.Say("Creating image...") - imageCh, errCh := driver.CreateImage( - config.ImageProjectId, config.ImageName, config.ImageDescription, config.ImageFamily, config.Zone, - config.DiskName, config.ImageLabels, config.ImageLicenses, config.ImageGuestOsFeatures, - config.ImageEncryptionKey.ComputeType(), config.ImageStorageLocations) + sourceDiskURI := fmt.Sprintf("/compute/v1/projects/%s/zones/%s/disks/%s", config.ProjectId, config.Zone, config.DiskName) + + imageFeatures := make([]*compute.GuestOsFeature, 0, len(config.ImageGuestOsFeatures)) + for _, v := range config.ImageGuestOsFeatures { + imageFeatures = append(imageFeatures, &compute.GuestOsFeature{ + Type: v, + }) + } + imagePayload := &compute.Image{ + Description: config.ImageDescription, + Name: config.ImageName, + Family: config.ImageFamily, + Labels: config.ImageLabels, + Licenses: config.ImageLicenses, + GuestOsFeatures: imageFeatures, + ImageEncryptionKey: config.ImageEncryptionKey.ComputeType(), + SourceDisk: sourceDiskURI, + SourceType: "RAW", + StorageLocations: config.ImageStorageLocations, + } + imageCh, errCh := driver.CreateImage(config.ImageProjectId, imagePayload) var err error select { case err = <-errCh: diff --git a/builder/googlecompute/step_create_image_test.go b/builder/googlecompute/step_create_image_test.go index 00a6a468..ba249baa 100644 --- a/builder/googlecompute/step_create_image_test.go +++ b/builder/googlecompute/step_create_image_test.go @@ -25,12 +25,8 @@ func TestStepCreateImage(t *testing.T) { c := state.Get("config").(*Config) d := state.Get("driver").(*common.DriverMock) - // These are the values of the image the driver will return. - d.CreateImageResultProjectId = "test-project" - d.CreateImageResultSizeGb = 100 - d.CreateImageFeatures = []string{ - "UEFI_COMPATIBLE", - } + d.CreateImageReturnSelfLink = "https://selflink/compute/v1/test" + d.CreateImageReturnDiskSize = 420 // run the step action := step.Run(context.Background(), state) @@ -42,21 +38,16 @@ func TestStepCreateImage(t *testing.T) { assert.True(t, ok, "Image in state is not an Image.") // Verify created Image results. - assert.Equal(t, image.Name, c.ImageName, "Created image does not match config name.") - assert.Equal(t, image.ProjectId, d.CreateImageResultProjectId, "Created image project does not match driver project.") - assert.Equal(t, image.SizeGb, d.CreateImageResultSizeGb, "Created image size does not match the size returned by the driver.") - assert.Equal(t, len(image.GuestOsFeatures), len(d.CreateImageFeatures), "Created image features does not match config features.") + assert.Equal(t, c.ImageName, image.Name, "Created image does not match config name.") + assert.Equal(t, len(c.ImageGuestOsFeatures), len(image.GuestOsFeatures), "Created image features does not match config.") + assert.Equal(t, c.ImageLabels, image.Labels, "Created image labels does not match config.") + assert.Equal(t, c.ImageLicenses, image.Licenses, "Created image licenses does not match config.") + assert.Equal(t, c.ProjectId, image.ProjectId, "Created image project ID does not match config.") + assert.Equal(t, d.CreateImageReturnSelfLink, image.SelfLink, "Created image selflink does not match config") + assert.Equal(t, d.CreateImageReturnDiskSize, image.SizeGb, "Created image disk size does not match config") // Verify proper args passed to driver.CreateImage. - assert.Equal(t, d.CreateImageName, c.ImageName, "Incorrect image name passed to driver.") - assert.Equal(t, d.CreateImageDesc, c.ImageDescription, "Incorrect image description passed to driver.") - assert.Equal(t, d.CreateImageFamily, c.ImageFamily, "Incorrect image family passed to driver.") - assert.Equal(t, d.CreateImageZone, c.Zone, "Incorrect image zone passed to driver.") - assert.Equal(t, d.CreateImageDisk, c.DiskName, "Incorrect disk passed to driver.") - assert.Equal(t, d.CreateImageLabels, c.ImageLabels, "Incorrect image_labels passed to driver.") - assert.Equal(t, d.CreateImageLicenses, c.ImageLicenses, "Incorrect image_licenses passed to driver.") - assert.Equal(t, d.CreateImageEncryptionKey, c.ImageEncryptionKey.ComputeType(), "Incorrect image_encryption_key passed to driver.") - assert.Equal(t, d.CreateImageStorageLocations, c.ImageStorageLocations, "Incorrect image_storage_locations passed to driver.") + assert.Equal(t, c.ProjectId, d.CreateImageProjectId, "Incorrect project ID passed to driver.") } func TestStepCreateImage_errorOnChannel(t *testing.T) { diff --git a/lib/common/driver.go b/lib/common/driver.go index f9804628..2554f5b4 100644 --- a/lib/common/driver.go +++ b/lib/common/driver.go @@ -22,10 +22,7 @@ type Driver interface { // CreateImage creates an image from the given disk in Google Compute // Engine. - CreateImage(project, name, description, family, zone, disk string, image_labels map[string]string, image_licenses []string, image_guest_os_features []string, image_encryption_key *compute.CustomerEncryptionKey, imageStorageLocation []string) (<-chan *Image, <-chan error) - - // CreateImageFromRaw creates an image in GCE from a raw disk image. - CreateImageFromRaw(project, name, description, family, rawImageLocation string, image_labels map[string]string, image_guest_os_features []string, shielded_vm_state_config *compute.InitialStateConfig, imageStorageLocation []string, architecture string) (<-chan *Image, <-chan error) + CreateImage(project string, imageSpec *compute.Image) (<-chan *Image, <-chan error) // DeleteImage deletes the image with the given name. DeleteImage(project, name string) <-chan error diff --git a/lib/common/driver_gce.go b/lib/common/driver_gce.go index 83d1f8ea..3243093e 100644 --- a/lib/common/driver_gce.go +++ b/lib/common/driver_gce.go @@ -200,92 +200,10 @@ func NewDriverGCE(config GCEDriverConfig) (Driver, error) { }, nil } -func (d *driverGCE) CreateImage(project, name, description, family, zone, disk string, image_labels map[string]string, image_licenses []string, image_guest_os_features []string, image_encryption_key *compute.CustomerEncryptionKey, imageStorageLocations []string) (<-chan *Image, <-chan error) { - - image_features := make([]*compute.GuestOsFeature, 0, len(image_guest_os_features)) - for _, v := range image_guest_os_features { - image_features = append(image_features, &compute.GuestOsFeature{ - Type: v, - }) - } - gce_image := &compute.Image{ - Description: description, - Name: name, - Family: family, - Labels: image_labels, - Licenses: image_licenses, - GuestOsFeatures: image_features, - ImageEncryptionKey: image_encryption_key, - SourceDisk: fmt.Sprintf("%sprojects/%s/zones/%s/disks/%s", d.service.BasePath, d.projectId, zone, disk), - SourceType: "RAW", - StorageLocations: imageStorageLocations, - } - +func (d *driverGCE) CreateImage(project string, imageSpec *compute.Image) (<-chan *Image, <-chan error) { imageCh := make(chan *Image, 1) errCh := make(chan error, 1) - op, err := d.service.Images.Insert(project, gce_image).Do() - if err != nil { - errCh <- err - } else { - go func() { - err = waitForState(errCh, "DONE", d.refreshGlobalOp(project, op)) - if err != nil { - close(imageCh) - errCh <- err - return - } - var image *Image - image, err = d.GetImageFromProject(project, name, false) - if err != nil { - close(imageCh) - errCh <- err - return - } - imageCh <- image - close(imageCh) - }() - } - - return imageCh, errCh -} - -func (d *driverGCE) CreateImageFromRaw( - project string, - rawImageURL string, - imageName string, - imageDescription string, - imageFamily string, - imageLabels map[string]string, - imageGuestOsFeatures []string, - shieldedVMStateConfig *compute.InitialStateConfig, - imageStorageLocations []string, - imageArchitecture string, -) (<-chan *Image, <-chan error) { - // Build up the imageFeatures - imageFeatures := make([]*compute.GuestOsFeature, len(imageGuestOsFeatures)) - for _, v := range imageGuestOsFeatures { - imageFeatures = append(imageFeatures, &compute.GuestOsFeature{ - Type: v, - }) - } - - gceImage := &compute.Image{ - Architecture: imageArchitecture, - Description: imageDescription, - Family: imageFamily, - GuestOsFeatures: imageFeatures, - Labels: imageLabels, - Name: imageName, - RawDisk: &compute.ImageRawDisk{Source: rawImageURL}, - SourceType: "RAW", - ShieldedInstanceInitialState: shieldedVMStateConfig, - StorageLocations: imageStorageLocations, - } - - imageCh := make(chan *Image, 1) - errCh := make(chan error, 1) - - op, err := d.service.Images.Insert(project, gceImage).Do() + op, err := d.service.Images.Insert(project, imageSpec).Do() if err != nil { errCh <- err } else { @@ -297,7 +215,7 @@ func (d *driverGCE) CreateImageFromRaw( return } var image *Image - image, err = d.GetImageFromProject(project, imageName, false) + image, err = d.GetImageFromProject(project, imageSpec.Name, false) if err != nil { close(imageCh) errCh <- err diff --git a/lib/common/driver_mock.go b/lib/common/driver_mock.go index 752ab0eb..fdf6fe7f 100644 --- a/lib/common/driver_mock.go +++ b/lib/common/driver_mock.go @@ -19,22 +19,12 @@ type DriverMock struct { CreateDiskResultCh <-chan *compute.Disk CreateDiskErrCh <-chan error - CreateImageProjectId string - CreateImageName string - CreateImageDesc string - CreateImageFamily string - CreateImageEncryptionKey *compute.CustomerEncryptionKey - CreateImageLabels map[string]string - CreateImageLicenses []string - CreateImageFeatures []string - CreateImageStorageLocations []string - CreateImageZone string - CreateImageDisk string - CreateImageResultProjectId string - CreateImageResultSelfLink string - CreateImageResultSizeGb int64 - CreateImageErrCh <-chan error - CreateImageResultCh <-chan *Image + CreateImageProjectId string + CreateImageSpec *compute.Image + CreateImageReturnDiskSize int64 + CreateImageReturnSelfLink string + CreateImageErrCh <-chan error + CreateImageResultCh <-chan *Image DeleteProjectId string DeleteImageName string @@ -127,46 +117,31 @@ type DriverMock struct { UploadToBucketError error } -func (d *DriverMock) CreateImage(project, name, description, family, zone, disk string, image_labels map[string]string, image_licenses []string, image_features []string, image_encryption_key *compute.CustomerEncryptionKey, imageStorageLocations []string) (<-chan *Image, <-chan error) { +func (d *DriverMock) CreateImage(project string, imageSpec *compute.Image) (<-chan *Image, <-chan error) { d.CreateImageProjectId = project - d.CreateImageName = name - d.CreateImageDesc = description - d.CreateImageFamily = family - d.CreateImageLabels = image_labels - d.CreateImageLicenses = image_licenses - d.CreateImageFeatures = image_features - d.CreateImageStorageLocations = imageStorageLocations - d.CreateImageZone = zone - d.CreateImageDisk = disk - d.CreateImageEncryptionKey = image_encryption_key - if d.CreateImageResultProjectId == "" { - d.CreateImageResultProjectId = "test" - } - if d.CreateImageResultSelfLink == "" { - d.CreateImageResultSelfLink = fmt.Sprintf( - "http://content.googleapis.com/compute/v1/%s/global/licenses/test", - d.CreateImageResultProjectId) - } - if d.CreateImageResultSizeGb == 0 { - d.CreateImageResultSizeGb = 10 - } - imageFeatures := make([]*compute.GuestOsFeature, 0, len(image_features)) - for _, v := range image_features { - imageFeatures = append(imageFeatures, &compute.GuestOsFeature{ - Type: v, - }) - } + d.CreateImageSpec = imageSpec resultCh := d.CreateImageResultCh if resultCh == nil { ch := make(chan *Image, 1) + + selfLink := d.CreateImageReturnSelfLink + if selfLink == "" { + selfLink = fmt.Sprintf("http://content.googleapis.com/compute/v1/%s/global/licenses/test", d.CreateImageProjectId) + } + + diskSizeGb := d.CreateImageReturnDiskSize + if diskSizeGb == 0 { + diskSizeGb = 25 + } + ch <- &Image{ - GuestOsFeatures: imageFeatures, - Labels: d.CreateImageLabels, - Licenses: d.CreateImageLicenses, - Name: name, - ProjectId: d.CreateImageResultProjectId, - SelfLink: d.CreateImageResultSelfLink, - SizeGb: d.CreateImageResultSizeGb, + GuestOsFeatures: imageSpec.GuestOsFeatures, + Labels: imageSpec.Labels, + Licenses: imageSpec.Licenses, + Name: imageSpec.Name, + ProjectId: d.CreateImageProjectId, + SelfLink: selfLink, + SizeGb: diskSizeGb, } close(ch) resultCh = ch diff --git a/post-processor/googlecompute-import/post-processor.go b/post-processor/googlecompute-import/post-processor.go index c5c1e576..e1848eef 100644 --- a/post-processor/googlecompute-import/post-processor.go +++ b/post-processor/googlecompute-import/post-processor.go @@ -217,7 +217,26 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packersdk.Ui, artifa var retArtifact *Artifact var retErr error - imageCh, errCh := driver.CreateImageFromRaw(p.config.ProjectId, rawImageGcsPath, p.config.ImageName, p.config.ImageDescription, p.config.ImageFamily, p.config.ImageLabels, p.config.ImageGuestOsFeatures, shieldedVMStateConfig, p.config.ImageStorageLocations, p.config.ImageArchitecture) + imageFeatures := make([]*compute.GuestOsFeature, 0, len(p.config.ImageGuestOsFeatures)) + for _, v := range p.config.ImageGuestOsFeatures { + imageFeatures = append(imageFeatures, &compute.GuestOsFeature{ + Type: v, + }) + } + imageSpec := &compute.Image{ + Architecture: p.config.ImageArchitecture, + Description: p.config.ImageDescription, + Family: p.config.ImageFamily, + GuestOsFeatures: imageFeatures, + Labels: p.config.ImageLabels, + Name: p.config.ImageName, + RawDisk: &compute.ImageRawDisk{Source: rawImageGcsPath}, + SourceType: "RAW", + ShieldedInstanceInitialState: shieldedVMStateConfig, + StorageLocations: p.config.ImageStorageLocations, + } + + imageCh, errCh := driver.CreateImage(p.config.ProjectId, imageSpec) select { case img := <-imageCh: retArtifact = &Artifact{