From b54b82d3acd0f145fa96596c54f7cedea0c7f2cf Mon Sep 17 00:00:00 2001 From: Scott Crunkleton Date: Wed, 7 Sep 2016 19:00:30 -0700 Subject: [PATCH] Some googlecompute fixes and cleanup. Addresses https://github.com/mitchellh/packer/issues/3829. Changes: - startup scripts don't run for Windows since it is isn't implemented yet. - startup scripts use instance metadata instead of serial port output to flag when they are done. - added licenses to Image data type (to check if an Image is a Windows Image). - added GetImage and GetImageFromProject to googlecompute Drivers. - changed some of the builder/googlecompute tests to use github.com/stretchr/testify/assert. Tests: - (in the Packer directory) `go test .`, `go test ./builder/googlecompute`, and `go test ./post-processor/googlecompute-export` - manual run of `packer build packer_template.json` with the following files --packer_template.json-- { "builders": [ { "type": "googlecompute", "account_file": "creds.json", "project_id": "google.com:packer-test", "source_image": "debian-8-jessie-v20160629", "zone": "us-central1-a", "startup_script_file": "startup_script.sh", "metadata": { "startup-script": "#!/bin/sh\necho \"This should be overwritten.\"", "startup-script-log-dest": "gs://packer-test.google.com.a.appspot.com/startup-script.log" }, "image_name": "test-packer-modifications", "ssh_username": "foo" } ], "post-processors": [ { "type": "googlecompute-export", "paths": [ "gs://packer-test.google.com.a.appspot.com/foo.tar.gz", "gs://packer-test.google.com.a.appspot.com/bar.tar.gz" ], "keep_input_artifact": true } ] } --startup_script.sh-- \#!/bin/sh echo "Hi, my name is Scott. I'm waiting 60 seconds!" >> /scott sleep 60 echo "I'm done waiting!" >> /scott --- builder/googlecompute/artifact.go | 2 +- builder/googlecompute/builder.go | 2 +- builder/googlecompute/driver.go | 27 +++-- builder/googlecompute/driver_gce.go | 110 +++++++++++------- builder/googlecompute/driver_mock.go | 96 ++++++++++----- builder/googlecompute/image.go | 22 ++++ builder/googlecompute/image_test.go | 26 +++++ builder/googlecompute/startup.go | 36 +++--- .../step_check_existing_image.go | 8 +- builder/googlecompute/step_create_image.go | 4 +- .../googlecompute/step_create_image_test.go | 76 +++++------- builder/googlecompute/step_create_instance.go | 85 ++++++++------ .../step_create_instance_test.go | 100 +++++++--------- builder/googlecompute/step_test.go | 3 +- .../step_wait_instance_startup.go | 19 ++- .../step_wait_instance_startup_test.go | 30 ++--- 16 files changed, 378 insertions(+), 268 deletions(-) create mode 100644 builder/googlecompute/image.go create mode 100644 builder/googlecompute/image_test.go diff --git a/builder/googlecompute/artifact.go b/builder/googlecompute/artifact.go index 86d8aa9e1..7738daa50 100644 --- a/builder/googlecompute/artifact.go +++ b/builder/googlecompute/artifact.go @@ -7,7 +7,7 @@ import ( // Artifact represents a GCE image as the result of a Packer build. type Artifact struct { - image Image + image *Image driver Driver config *Config } diff --git a/builder/googlecompute/builder.go b/builder/googlecompute/builder.go index d4930e033..d1c733629 100644 --- a/builder/googlecompute/builder.go +++ b/builder/googlecompute/builder.go @@ -93,7 +93,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } artifact := &Artifact{ - image: state.Get("image").(Image), + image: state.Get("image").(*Image), driver: driver, config: b.config, } diff --git a/builder/googlecompute/driver.go b/builder/googlecompute/driver.go index 8c0e53aef..183ef294d 100644 --- a/builder/googlecompute/driver.go +++ b/builder/googlecompute/driver.go @@ -4,13 +4,9 @@ package googlecompute // with GCE. The Driver interface exists mostly to allow a mock implementation // to be used to test the steps. type Driver interface { - // ImageExists returns true if the specified image exists. If an error - // occurs calling the API, this method returns false. - ImageExists(name string) bool - // CreateImage creates an image from the given disk in Google Compute // Engine. - CreateImage(name, description, family, zone, disk string) (<-chan Image, <-chan error) + CreateImage(name, description, family, zone, disk string) (<-chan *Image, <-chan error) // DeleteImage deletes the image with the given name. DeleteImage(name string) <-chan error @@ -21,6 +17,15 @@ type Driver interface { // DeleteDisk deletes the disk with the given name. DeleteDisk(zone, name string) (<-chan error, error) + // GetImage gets an image; tries the default and public projects. + GetImage(name string) (*Image, error) + + // GetImageFromProject gets an image from a specific project. + GetImageFromProject(project, name string) (*Image, error) + + // GetInstanceMetadata gets a metadata variable for the instance, name. + GetInstanceMetadata(zone, name, key string) (string, error) + // GetInternalIP gets the GCE-internal IP address for the instance. GetInternalIP(zone, name string) (string, error) @@ -30,6 +35,10 @@ type Driver interface { // GetSerialPortOutput gets the Serial Port contents for the instance. GetSerialPortOutput(zone, name string) (string, error) + // ImageExists returns true if the specified image exists. If an error + // occurs calling the API, this method returns false. + ImageExists(name string) bool + // RunInstance takes the given config and launches an instance. RunInstance(*InstanceConfig) (<-chan error, error) @@ -37,18 +46,12 @@ type Driver interface { WaitForInstance(state, zone, name string) <-chan error } -type Image struct { - Name string - ProjectId string - SizeGb int64 -} - type InstanceConfig struct { Address string Description string DiskSizeGb int64 DiskType string - Image Image + Image *Image MachineType string Metadata map[string]string Name string diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 1c0292e07..31f8e2b03 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -5,6 +5,7 @@ import ( "log" "net/http" "runtime" + "strings" "github.com/mitchellh/packer/packer" "github.com/mitchellh/packer/version" @@ -13,7 +14,6 @@ import ( "golang.org/x/oauth2/google" "golang.org/x/oauth2/jwt" "google.golang.org/api/compute/v1" - "strings" ) // driverGCE is a Driver implementation that actually talks to GCE. @@ -88,15 +88,8 @@ func NewDriverGCE(ui packer.Ui, p string, a *AccountFile) (Driver, error) { }, nil } -func (d *driverGCE) ImageExists(name string) bool { - _, err := d.service.Images.Get(d.projectId, name).Do() - // The API may return an error for reasons other than the image not - // existing, but this heuristic is sufficient for now. - return err == nil -} - -func (d *driverGCE) CreateImage(name, description, family, zone, disk string) (<-chan Image, <-chan error) { - image := &compute.Image{ +func (d *driverGCE) CreateImage(name, description, family, zone, disk string) (<-chan *Image, <-chan error) { + gce_image := &compute.Image{ Description: description, Name: name, Family: family, @@ -104,9 +97,9 @@ func (d *driverGCE) CreateImage(name, description, family, zone, disk string) (< SourceType: "RAW", } - imageCh := make(chan Image, 1) + imageCh := make(chan *Image, 1) errCh := make(chan error, 1) - op, err := d.service.Images.Insert(d.projectId, image).Do() + op, err := d.service.Images.Insert(d.projectId, gce_image).Do() if err != nil { errCh <- err } else { @@ -114,17 +107,17 @@ func (d *driverGCE) CreateImage(name, description, family, zone, disk string) (< err = waitForState(errCh, "DONE", d.refreshGlobalOp(op)) if err != nil { close(imageCh) + errCh <- err + return } - image, err = d.getImage(name, d.projectId) + var image *Image + image, err = d.GetImageFromProject(d.projectId, name) if err != nil { close(imageCh) errCh <- err + return } - imageCh <- Image{ - Name: name, - ProjectId: d.projectId, - SizeGb: image.DiskSizeGb, - } + imageCh <- image close(imageCh) }() } @@ -166,6 +159,57 @@ func (d *driverGCE) DeleteDisk(zone, name string) (<-chan error, error) { return errCh, nil } +func (d *driverGCE) GetImage(name string) (*Image, error) { + projects := []string{d.projectId, "centos-cloud", "coreos-cloud", "debian-cloud", "google-containers", "opensuse-cloud", "rhel-cloud", "suse-cloud", "ubuntu-os-cloud", "windows-cloud"} + var errs error + for _, project := range projects { + image, err := d.GetImageFromProject(project, name) + if err != nil { + errs = packer.MultiErrorAppend(errs, err) + } + if image != nil { + return image, nil + } + } + + return nil, fmt.Errorf( + "Could not find image, %s, in projects, %s: %s", name, + projects, errs) +} + +func (d *driverGCE) GetImageFromProject(project, name string) (*Image, error) { + image, err := d.service.Images.Get(project, name).Do() + + if err != nil { + return nil, err + } else if image == nil || image.SelfLink == "" { + return nil, fmt.Errorf("Image, %s, could not be found in project: %s", name, project) + } else { + return &Image{ + Licenses: image.Licenses, + Name: image.Name, + ProjectId: project, + SelfLink: image.SelfLink, + SizeGb: image.DiskSizeGb, + }, nil + } +} + +func (d *driverGCE) GetInstanceMetadata(zone, name, key string) (string, error) { + instance, err := d.service.Instances.Get(d.projectId, zone, name).Do() + if err != nil { + return "", err + } + + for _, item := range instance.Metadata.Items { + if item.Key == key { + return *item.Value, nil + } + } + + return "", fmt.Errorf("Instance metadata key, %s, not found.", key) +} + func (d *driverGCE) GetNatIP(zone, name string) (string, error) { instance, err := d.service.Instances.Get(d.projectId, zone, name).Do() if err != nil { @@ -211,6 +255,13 @@ func (d *driverGCE) GetSerialPortOutput(zone, name string) (string, error) { return output.Contents, nil } +func (d *driverGCE) ImageExists(name string) bool { + _, err := d.GetImageFromProject(d.projectId, name) + // The API may return an error for reasons other than the image not + // existing, but this heuristic is sufficient for now. + return err == nil +} + func (d *driverGCE) RunInstance(c *InstanceConfig) (<-chan error, error) { // Get the zone d.ui.Message(fmt.Sprintf("Loading zone: %s", c.Zone)) @@ -219,13 +270,6 @@ func (d *driverGCE) RunInstance(c *InstanceConfig) (<-chan error, error) { return nil, err } - // Get the image - d.ui.Message(fmt.Sprintf("Loading image: %s in project %s", c.Image.Name, c.Image.ProjectId)) - image, err := d.getImage(c.Image.Name, c.Image.ProjectId) - if err != nil { - return nil, err - } - // Get the machine type d.ui.Message(fmt.Sprintf("Loading machine type: %s", c.MachineType)) machineType, err := d.service.MachineTypes.Get( @@ -302,7 +346,7 @@ func (d *driverGCE) RunInstance(c *InstanceConfig) (<-chan error, error) { Boot: true, AutoDelete: false, InitializeParams: &compute.AttachedDiskInitializeParams{ - SourceImage: image.SelfLink, + SourceImage: c.Image.SelfLink, DiskSizeGb: c.DiskSizeGb, DiskType: fmt.Sprintf("zones/%s/diskTypes/%s", zone.Name, c.DiskType), }, @@ -355,20 +399,6 @@ func (d *driverGCE) WaitForInstance(state, zone, name string) <-chan error { return errCh } -func (d *driverGCE) getImage(name, projectId string) (image *compute.Image, err error) { - projects := []string{projectId, "centos-cloud", "coreos-cloud", "debian-cloud", "google-containers", "opensuse-cloud", "rhel-cloud", "suse-cloud", "ubuntu-os-cloud", "windows-cloud"} - for _, project := range projects { - image, err = d.service.Images.Get(project, name).Do() - if err == nil && image != nil && image.SelfLink != "" { - return - } - image = nil - } - - err = fmt.Errorf("Image %s could not be found in any of these projects: %s", name, projects) - return -} - func (d *driverGCE) refreshInstanceState(zone, name string) stateRefreshFunc { return func() (string, error) { instance, err := d.service.Instances.Get(d.projectId, zone, name).Do() diff --git a/builder/googlecompute/driver_mock.go b/builder/googlecompute/driver_mock.go index 90ab726c5..b911aa6d6 100644 --- a/builder/googlecompute/driver_mock.go +++ b/builder/googlecompute/driver_mock.go @@ -1,20 +1,21 @@ package googlecompute +import "fmt" + // DriverMock is a Driver implementation that is a mocked out so that // it can be used for tests. type DriverMock struct { - ImageExistsName string - ImageExistsResult bool - - CreateImageName string - CreateImageDesc string - CreateImageFamily string - CreateImageZone string - CreateImageDisk string - CreateImageProjectId string - CreateImageSizeGb int64 - CreateImageErrCh <-chan error - CreateImageResultCh <-chan Image + CreateImageName string + CreateImageDesc string + CreateImageFamily string + CreateImageZone string + CreateImageDisk string + CreateImageResultLicenses []string + CreateImageResultProjectId string + CreateImageResultSelfLink string + CreateImageResultSizeGb int64 + CreateImageErrCh <-chan error + CreateImageResultCh <-chan *Image DeleteImageName string DeleteImageErrCh <-chan error @@ -29,6 +30,21 @@ type DriverMock struct { DeleteDiskErrCh <-chan error DeleteDiskErr error + GetImageName string + GetImageResult *Image + GetImageErr error + + GetImageFromProjectProject string + GetImageFromProjectName string + GetImageFromProjectResult *Image + GetImageFromProjectErr error + + GetInstanceMetadataZone string + GetInstanceMetadataName string + GetInstanceMetadataKey string + GetInstanceMetadataResult string + GetInstanceMetadataErr error + GetNatIPZone string GetNatIPName string GetNatIPResult string @@ -44,6 +60,9 @@ type DriverMock struct { GetSerialPortOutputResult string GetSerialPortOutputErr error + ImageExistsName string + ImageExistsResult bool + RunInstanceConfig *InstanceConfig RunInstanceErrCh <-chan error RunInstanceErr error @@ -54,31 +73,33 @@ type DriverMock struct { WaitForInstanceErrCh <-chan error } -func (d *DriverMock) ImageExists(name string) bool { - d.ImageExistsName = name - return d.ImageExistsResult -} - -func (d *DriverMock) CreateImage(name, description, family, zone, disk string) (<-chan Image, <-chan error) { +func (d *DriverMock) CreateImage(name, description, family, zone, disk string) (<-chan *Image, <-chan error) { d.CreateImageName = name d.CreateImageDesc = description d.CreateImageFamily = family d.CreateImageZone = zone d.CreateImageDisk = disk - if d.CreateImageSizeGb == 0 { - d.CreateImageSizeGb = 10 + if d.CreateImageResultProjectId == "" { + d.CreateImageResultProjectId = "test" } - if d.CreateImageProjectId == "" { - d.CreateImageProjectId = "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 } resultCh := d.CreateImageResultCh if resultCh == nil { - ch := make(chan Image, 1) - ch <- Image{ + ch := make(chan *Image, 1) + ch <- &Image{ + Licenses: d.CreateImageResultLicenses, Name: name, - ProjectId: d.CreateImageProjectId, - SizeGb: d.CreateImageSizeGb, + ProjectId: d.CreateImageResultProjectId, + SelfLink: d.CreateImageResultSelfLink, + SizeGb: d.CreateImageResultSizeGb, } close(ch) resultCh = ch @@ -135,6 +156,24 @@ func (d *DriverMock) DeleteDisk(zone, name string) (<-chan error, error) { return resultCh, d.DeleteDiskErr } +func (d *DriverMock) GetImage(name string) (*Image, error) { + d.GetImageName = name + return d.GetImageResult, d.GetImageErr +} + +func (d *DriverMock) GetImageFromProject(project, name string) (*Image, error) { + d.GetImageFromProjectProject = project + d.GetImageFromProjectName = name + return d.GetImageFromProjectResult, d.GetImageFromProjectErr +} + +func (d *DriverMock) GetInstanceMetadata(zone, name, key string) (string, error) { + d.GetInstanceMetadataZone = zone + d.GetInstanceMetadataName = name + d.GetInstanceMetadataKey = key + return d.GetInstanceMetadataResult, d.GetInstanceMetadataErr +} + func (d *DriverMock) GetNatIP(zone, name string) (string, error) { d.GetNatIPZone = zone d.GetNatIPName = name @@ -153,6 +192,11 @@ func (d *DriverMock) GetSerialPortOutput(zone, name string) (string, error) { return d.GetSerialPortOutputResult, d.GetSerialPortOutputErr } +func (d *DriverMock) ImageExists(name string) bool { + d.ImageExistsName = name + return d.ImageExistsResult +} + func (d *DriverMock) RunInstance(c *InstanceConfig) (<-chan error, error) { d.RunInstanceConfig = c diff --git a/builder/googlecompute/image.go b/builder/googlecompute/image.go new file mode 100644 index 000000000..11fe7df55 --- /dev/null +++ b/builder/googlecompute/image.go @@ -0,0 +1,22 @@ +package googlecompute + +import ( + "strings" +) + +type Image struct { + Licenses []string + Name string + ProjectId string + SelfLink string + SizeGb int64 +} + +func (i *Image) IsWindows() bool { + for _, license := range i.Licenses { + if strings.Contains(license, "windows") { + return true + } + } + return false +} diff --git a/builder/googlecompute/image_test.go b/builder/googlecompute/image_test.go new file mode 100644 index 000000000..10998a1de --- /dev/null +++ b/builder/googlecompute/image_test.go @@ -0,0 +1,26 @@ +package googlecompute + +import( + "testing" + "fmt" + + "github.com/stretchr/testify/assert" +) + +func StubImage(name, project string, licenses []string, sizeGb int64) *Image { + return &Image{ + Licenses: licenses, + Name: name, + ProjectId: project, + SelfLink: fmt.Sprintf("https://www.googleapis.com/compute/v1/projects/%s/global/images/%s", project, name), + SizeGb: sizeGb, + } +} + +func TestImage_IsWindows(t *testing.T) { + i := StubImage("foo", "foo-project", []string{"license-foo", "license-bar"}, 100) + assert.False(t, i.IsWindows()) + + i = StubImage("foo", "foo-project", []string{"license-foo", "windows-license"}, 100) + assert.True(t, i.IsWindows()) +} diff --git a/builder/googlecompute/startup.go b/builder/googlecompute/startup.go index 308e33e18..5b550fe39 100644 --- a/builder/googlecompute/startup.go +++ b/builder/googlecompute/startup.go @@ -1,37 +1,40 @@ package googlecompute import ( - "encoding/base64" "fmt" ) -const StartupScriptStartLog string = "Packer startup script starting." -const StartupScriptDoneLog string = "Packer startup script done." const StartupScriptKey string = "startup-script" +const StartupScriptStatusKey string = "startup-script-status" const StartupWrappedScriptKey string = "packer-wrapped-startup-script" -// We have to encode StartupScriptDoneLog because we use it as a sentinel value to indicate -// that the user-provided startup script is done. If we pass StartupScriptDoneLog as-is, it -// will be printed early in the instance console log (before the startup script even runs; -// we print out instance creation metadata which contains this wrapper script). -var StartupScriptDoneLogBase64 string = base64.StdEncoding.EncodeToString([]byte(StartupScriptDoneLog)) +const StartupScriptStatusDone string = "done" +const StartupScriptStatusError string = "error" +const StartupScriptStatusNotDone string = "notdone" -var StartupScript string = fmt.Sprintf(`#!/bin/bash -echo %s +var StartupScriptLinux string = fmt.Sprintf(`#!/bin/bash +echo "Packer startup script starting." RETVAL=0 +BASEMETADATAURL=http://metadata/computeMetadata/v1/instance/ GetMetadata () { - echo "$(curl -f -H "Metadata-Flavor: Google" http://metadata/computeMetadata/v1/instance/attributes/$1 2> /dev/null)" + echo "$(curl -f -H "Metadata-Flavor: Google" ${BASEMETADATAURL}/${1} 2> /dev/null)" } -STARTUPSCRIPT=$(GetMetadata %s) +ZONE=$(GetMetadata zone | grep -oP "[^/]*$") + +SetMetadata () { + gcloud compute instances add-metadata ${HOSTNAME} --metadata ${1}=${2} --zone ${ZONE} +} + +STARTUPSCRIPT=$(GetMetadata attributes/%s) STARTUPSCRIPTPATH=/packer-wrapped-startup-script if [ -f "/var/log/startupscript.log" ]; then STARTUPSCRIPTLOGPATH=/var/log/startupscript.log else STARTUPSCRIPTLOGPATH=/var/log/daemon.log fi -STARTUPSCRIPTLOGDEST=$(GetMetadata startup-script-log-dest) +STARTUPSCRIPTLOGDEST=$(GetMetadata attributes/startup-script-log-dest) if [[ ! -z $STARTUPSCRIPT ]]; then echo "Executing user-provided startup script..." @@ -48,6 +51,9 @@ if [[ ! -z $STARTUPSCRIPT ]]; then rm ${STARTUPSCRIPTPATH} fi -echo $(echo %s | base64 --decode) +echo "Packer startup script done." +SetMetadata %s %s exit $RETVAL -`, StartupScriptStartLog, StartupWrappedScriptKey, StartupScriptDoneLogBase64) +`, StartupWrappedScriptKey, StartupScriptStatusKey, StartupScriptStatusDone) + +var StartupScriptWindows string = "" diff --git a/builder/googlecompute/step_check_existing_image.go b/builder/googlecompute/step_check_existing_image.go index 2a3365739..9b87e8337 100644 --- a/builder/googlecompute/step_check_existing_image.go +++ b/builder/googlecompute/step_check_existing_image.go @@ -13,14 +13,14 @@ type StepCheckExistingImage int // Run executes the Packer build step that checks if the image already exists. func (s *StepCheckExistingImage) Run(state multistep.StateBag) multistep.StepAction { - config := state.Get("config").(*Config) - driver := state.Get("driver").(Driver) + c := state.Get("config").(*Config) + d := state.Get("driver").(Driver) ui := state.Get("ui").(packer.Ui) ui.Say("Checking image does not exist...") - exists := driver.ImageExists(config.ImageName) + exists := d.ImageExists(c.ImageName) if exists { - err := fmt.Errorf("Image %s already exists", config.ImageName) + err := fmt.Errorf("Image %s already exists", c.ImageName) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt diff --git a/builder/googlecompute/step_create_image.go b/builder/googlecompute/step_create_image.go index f7e531d43..c8d5b4834 100644 --- a/builder/googlecompute/step_create_image.go +++ b/builder/googlecompute/step_create_image.go @@ -24,7 +24,9 @@ func (s *StepCreateImage) Run(state multistep.StateBag) multistep.StepAction { ui.Say("Creating image...") - imageCh, errCh := driver.CreateImage(config.ImageName, config.ImageDescription, config.ImageFamily, config.Zone, config.DiskName) + imageCh, errCh := driver.CreateImage( + config.ImageName, config.ImageDescription, config.ImageFamily, config.Zone, + config.DiskName) 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 c956ec5e3..0b5a096d3 100644 --- a/builder/googlecompute/step_create_image_test.go +++ b/builder/googlecompute/step_create_image_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/mitchellh/multistep" + "github.com/stretchr/testify/assert" ) func TestStepCreateImage_impl(t *testing.T) { @@ -16,52 +17,35 @@ func TestStepCreateImage(t *testing.T) { step := new(StepCreateImage) defer step.Cleanup(state) - config := state.Get("config").(*Config) - driver := state.Get("driver").(*DriverMock) - driver.CreateImageProjectId = "createimage-project" - driver.CreateImageSizeGb = 100 + c := state.Get("config").(*Config) + d := state.Get("driver").(*DriverMock) + + // These are the values of the image the driver will return. + d.CreateImageResultLicenses = []string{"test-license"} + d.CreateImageResultProjectId = "test-project" + d.CreateImageResultSizeGb = 100 // run the step - if action := step.Run(state); action != multistep.ActionContinue { - t.Fatalf("bad action: %#v", action) - } + action := step.Run(state) + assert.Equal(t, action, multistep.ActionContinue, "Step did not pass.") uncastImage, ok := state.GetOk("image") - if !ok { - t.Fatal("should have image") - } - image, ok := uncastImage.(Image) - if !ok { - t.Fatal("image is not an Image") - } + assert.True(t, ok, "State does not have resulting image.") + image, ok := uncastImage.(*Image) + assert.True(t, ok, "Image in state is not an Image.") // Verify created Image results. - if image.Name != config.ImageName { - t.Fatalf("Created image name, %s, does not match config name, %s.", image.Name, config.ImageName) - } - if driver.CreateImageProjectId != image.ProjectId { - t.Fatalf("Created image project ID, %s, does not match driver project ID, %s.", image.ProjectId, driver.CreateImageProjectId) - } - if driver.CreateImageSizeGb != image.SizeGb { - t.Fatalf("Created image size, %d, does not match the expected test value, %d.", image.SizeGb, driver.CreateImageSizeGb) - } + assert.Equal(t, image.Licenses, d.CreateImageResultLicenses, "Created image licenses don't match the licenses returned by the driver.") + 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.") // Verify proper args passed to driver.CreateImage. - if driver.CreateImageName != config.ImageName { - t.Fatalf("bad: %#v", driver.CreateImageName) - } - if driver.CreateImageDesc != config.ImageDescription { - t.Fatalf("bad: %#v", driver.CreateImageDesc) - } - if driver.CreateImageFamily != config.ImageFamily { - t.Fatalf("bad: %#v", driver.CreateImageFamily) - } - if driver.CreateImageZone != config.Zone { - t.Fatalf("bad: %#v", driver.CreateImageZone) - } - if driver.CreateImageDisk != config.DiskName { - t.Fatalf("bad: %#v", driver.CreateImageDisk) - } + 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.") } func TestStepCreateImage_errorOnChannel(t *testing.T) { @@ -76,14 +60,10 @@ func TestStepCreateImage_errorOnChannel(t *testing.T) { driver.CreateImageErrCh = errCh // run the step - if action := step.Run(state); action != multistep.ActionHalt { - t.Fatalf("bad action: %#v", action) - } - - if _, ok := state.GetOk("error"); !ok { - t.Fatal("should have error") - } - if _, ok := state.GetOk("image_name"); ok { - t.Fatal("should NOT have image") - } + action := step.Run(state) + assert.Equal(t, action, multistep.ActionHalt, "Step should not have passed.") + _, ok := state.GetOk("error") + assert.True(t, ok, "State should have an error.") + _, ok = state.GetOk("image_name") + assert.False(t, ok, "State should not have a resulting image.") } diff --git a/builder/googlecompute/step_create_instance.go b/builder/googlecompute/step_create_instance.go index bf4a013e8..f3844ef67 100644 --- a/builder/googlecompute/step_create_instance.go +++ b/builder/googlecompute/step_create_instance.go @@ -15,82 +15,97 @@ type StepCreateInstance struct { Debug bool } -func (config *Config) getImage() Image { - project := config.ProjectId - if config.SourceImageProjectId != "" { - project = config.SourceImageProjectId - } - return Image{Name: config.SourceImage, ProjectId: project} -} - -func (config *Config) getInstanceMetadata(sshPublicKey string) (map[string]string, error) { +func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) (map[string]string, error) { instanceMetadata := make(map[string]string) var err error // Copy metadata from config. - for k, v := range config.Metadata { + for k, v := range c.Metadata { instanceMetadata[k] = v } // Merge any existing ssh keys with our public key. sshMetaKey := "sshKeys" - sshKeys := fmt.Sprintf("%s:%s", config.Comm.SSHUsername, sshPublicKey) + sshKeys := fmt.Sprintf("%s:%s", c.Comm.SSHUsername, sshPublicKey) if confSshKeys, exists := instanceMetadata[sshMetaKey]; exists { sshKeys = fmt.Sprintf("%s\n%s", sshKeys, confSshKeys) } instanceMetadata[sshMetaKey] = sshKeys // Wrap any startup script with our own startup script. - if config.StartupScriptFile != "" { + if c.StartupScriptFile != "" { var content []byte - content, err = ioutil.ReadFile(config.StartupScriptFile) + content, err = ioutil.ReadFile(c.StartupScriptFile) instanceMetadata[StartupWrappedScriptKey] = string(content) } else if wrappedStartupScript, exists := instanceMetadata[StartupScriptKey]; exists { instanceMetadata[StartupWrappedScriptKey] = wrappedStartupScript } - instanceMetadata[StartupScriptKey] = StartupScript + if sourceImage.IsWindows() { + // Windows startup script support is not yet implemented. + // Mark the startup script as done. + instanceMetadata[StartupScriptKey] = StartupScriptWindows + instanceMetadata[StartupScriptStatusKey] = StartupScriptStatusDone + } else { + instanceMetadata[StartupScriptKey] = StartupScriptLinux + instanceMetadata[StartupScriptStatusKey] = StartupScriptStatusNotDone + } return instanceMetadata, err } +func getImage(c *Config, d Driver) (*Image, error) { + if c.SourceImageProjectId == "" { + return d.GetImage(c.SourceImage) + } else { + return d.GetImageFromProject(c.SourceImageProjectId, c.SourceImage) + } +} + // Run executes the Packer build step that creates a GCE instance. func (s *StepCreateInstance) Run(state multistep.StateBag) multistep.StepAction { - config := state.Get("config").(*Config) - driver := state.Get("driver").(Driver) + c := state.Get("config").(*Config) + d := state.Get("driver").(Driver) sshPublicKey := state.Get("ssh_public_key").(string) ui := state.Get("ui").(packer.Ui) + sourceImage, err := getImage(c, d) + if err != nil { + err := fmt.Errorf("Error getting source image for instance creation: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + ui.Say("Creating instance...") - name := config.InstanceName + name := c.InstanceName var errCh <-chan error - var err error var metadata map[string]string - metadata, err = config.getInstanceMetadata(sshPublicKey) - errCh, err = driver.RunInstance(&InstanceConfig{ - Address: config.Address, + metadata, err = c.createInstanceMetadata(sourceImage, sshPublicKey) + errCh, err = d.RunInstance(&InstanceConfig{ + Address: c.Address, Description: "New instance created by Packer", - DiskSizeGb: config.DiskSizeGb, - DiskType: config.DiskType, - Image: config.getImage(), - MachineType: config.MachineType, + DiskSizeGb: c.DiskSizeGb, + DiskType: c.DiskType, + Image: sourceImage, + MachineType: c.MachineType, Metadata: metadata, Name: name, - Network: config.Network, - OmitExternalIP: config.OmitExternalIP, - Preemptible: config.Preemptible, - Region: config.Region, - ServiceAccountEmail: config.Account.ClientEmail, - Subnetwork: config.Subnetwork, - Tags: config.Tags, - Zone: config.Zone, + Network: c.Network, + OmitExternalIP: c.OmitExternalIP, + Preemptible: c.Preemptible, + Region: c.Region, + ServiceAccountEmail: c.Account.ClientEmail, + Subnetwork: c.Subnetwork, + Tags: c.Tags, + Zone: c.Zone, }) if err == nil { ui.Message("Waiting for creation operation to complete...") select { case err = <-errCh: - case <-time.After(config.stateTimeout): + case <-time.After(c.stateTimeout): err = errors.New("time out while waiting for instance to create") } } @@ -106,7 +121,7 @@ func (s *StepCreateInstance) Run(state multistep.StateBag) multistep.StepAction if s.Debug { if name != "" { - ui.Message(fmt.Sprintf("Instance: %s started in %s", name, config.Zone)) + ui.Message(fmt.Sprintf("Instance: %s started in %s", name, c.Zone)) } } diff --git a/builder/googlecompute/step_create_instance_test.go b/builder/googlecompute/step_create_instance_test.go index f2ccbf57c..689eb3a35 100644 --- a/builder/googlecompute/step_create_instance_test.go +++ b/builder/googlecompute/step_create_instance_test.go @@ -2,9 +2,11 @@ package googlecompute import ( "errors" - "github.com/mitchellh/multistep" "testing" "time" + + "github.com/mitchellh/multistep" + "github.com/stretchr/testify/assert" ) func TestStepCreateInstance_impl(t *testing.T) { @@ -18,36 +20,25 @@ func TestStepCreateInstance(t *testing.T) { state.Put("ssh_public_key", "key") - config := state.Get("config").(*Config) - driver := state.Get("driver").(*DriverMock) + c := state.Get("config").(*Config) + d := state.Get("driver").(*DriverMock) + d.GetImageResult = StubImage("test-image", "test-project", []string{}, 100) // run the step - if action := step.Run(state); action != multistep.ActionContinue { - t.Fatalf("bad action: %#v", action) - } + assert.Equal(t, step.Run(state), multistep.ActionContinue, "Step should have passed and continued.") // Verify state nameRaw, ok := state.GetOk("instance_name") - if !ok { - t.Fatal("should have instance name") - } + assert.True(t, ok, "State should have an instance name.") // cleanup step.Cleanup(state) - if driver.DeleteInstanceName != nameRaw.(string) { - t.Fatal("should've deleted instance") - } - if driver.DeleteInstanceZone != config.Zone { - t.Fatalf("bad instance zone: %#v", driver.DeleteInstanceZone) - } - - if driver.DeleteDiskName != config.InstanceName { - t.Fatal("should've deleted disk") - } - if driver.DeleteDiskZone != config.Zone { - t.Fatalf("bad disk zone: %#v", driver.DeleteDiskZone) - } + // Check args passed to the driver. + assert.Equal(t, d.DeleteInstanceName, nameRaw.(string), "Incorrect instance name passed to driver.") + assert.Equal(t, d.DeleteInstanceZone, c.Zone, "Incorrect instance zone passed to driver.") + assert.Equal(t, d.DeleteDiskName, c.InstanceName, "Incorrect disk name passed to driver.") + assert.Equal(t, d.DeleteDiskZone, c.Zone, "Incorrect disk zone passed to driver.") } func TestStepCreateInstance_error(t *testing.T) { @@ -57,21 +48,18 @@ func TestStepCreateInstance_error(t *testing.T) { state.Put("ssh_public_key", "key") - driver := state.Get("driver").(*DriverMock) - driver.RunInstanceErr = errors.New("error") + d := state.Get("driver").(*DriverMock) + d.RunInstanceErr = errors.New("error") + d.GetImageResult = StubImage("test-image", "test-project", []string{}, 100) // run the step - if action := step.Run(state); action != multistep.ActionHalt { - t.Fatalf("bad action: %#v", action) - } + assert.Equal(t, step.Run(state), multistep.ActionHalt, "Step should have failed and halted.") // Verify state - if _, ok := state.GetOk("error"); !ok { - t.Fatal("should have error") - } - if _, ok := state.GetOk("instance_name"); ok { - t.Fatal("should NOT have instance name") - } + _, ok := state.GetOk("error") + assert.True(t, ok, "State should have an error.") + _, ok = state.GetOk("instance_name") + assert.False(t, ok, "State should not have an instance name.") } func TestStepCreateInstance_errorOnChannel(t *testing.T) { @@ -79,26 +67,23 @@ func TestStepCreateInstance_errorOnChannel(t *testing.T) { step := new(StepCreateInstance) defer step.Cleanup(state) + state.Put("ssh_public_key", "key") + errCh := make(chan error, 1) errCh <- errors.New("error") - state.Put("ssh_public_key", "key") - - driver := state.Get("driver").(*DriverMock) - driver.RunInstanceErrCh = errCh + d := state.Get("driver").(*DriverMock) + d.RunInstanceErrCh = errCh + d.GetImageResult = StubImage("test-image", "test-project", []string{}, 100) // run the step - if action := step.Run(state); action != multistep.ActionHalt { - t.Fatalf("bad action: %#v", action) - } + assert.Equal(t, step.Run(state), multistep.ActionHalt, "Step should have failed and halted.") // Verify state - if _, ok := state.GetOk("error"); !ok { - t.Fatal("should have error") - } - if _, ok := state.GetOk("instance_name"); ok { - t.Fatal("should NOT have instance name") - } + _, ok := state.GetOk("error") + assert.True(t, ok, "State should have an error.") + _, ok = state.GetOk("instance_name") + assert.False(t, ok, "State should not have an instance name.") } func TestStepCreateInstance_errorTimeout(t *testing.T) { @@ -106,30 +91,27 @@ func TestStepCreateInstance_errorTimeout(t *testing.T) { step := new(StepCreateInstance) defer step.Cleanup(state) + state.Put("ssh_public_key", "key") + errCh := make(chan error, 1) go func() { <-time.After(10 * time.Millisecond) errCh <- nil }() - state.Put("ssh_public_key", "key") - config := state.Get("config").(*Config) config.stateTimeout = 1 * time.Microsecond - driver := state.Get("driver").(*DriverMock) - driver.RunInstanceErrCh = errCh + d := state.Get("driver").(*DriverMock) + d.RunInstanceErrCh = errCh + d.GetImageResult = StubImage("test-image", "test-project", []string{}, 100) // run the step - if action := step.Run(state); action != multistep.ActionHalt { - t.Fatalf("bad action: %#v", action) - } + assert.Equal(t, step.Run(state), multistep.ActionHalt, "Step should have failed and halted.") // Verify state - if _, ok := state.GetOk("error"); !ok { - t.Fatal("should have error") - } - if _, ok := state.GetOk("instance_name"); ok { - t.Fatal("should NOT have instance name") - } + _, ok := state.GetOk("error") + assert.True(t, ok, "State should have an error.") + _, ok = state.GetOk("instance_name") + assert.False(t, ok, "State should not have an instance name.") } diff --git a/builder/googlecompute/step_test.go b/builder/googlecompute/step_test.go index cba0dbac9..4c1ceff55 100644 --- a/builder/googlecompute/step_test.go +++ b/builder/googlecompute/step_test.go @@ -2,9 +2,10 @@ package googlecompute import ( "bytes" + "testing" + "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" - "testing" ) func testState(t *testing.T) multistep.StateBag { diff --git a/builder/googlecompute/step_wait_instance_startup.go b/builder/googlecompute/step_wait_instance_startup.go index 3fa162732..7d7e6777f 100644 --- a/builder/googlecompute/step_wait_instance_startup.go +++ b/builder/googlecompute/step_wait_instance_startup.go @@ -1,8 +1,8 @@ package googlecompute import( + "errors" "fmt" - "strings" "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" @@ -10,7 +10,8 @@ import( type StepWaitInstanceStartup int -// Run reads the instance serial port output and looks for the log entry indicating the startup script finished. +// Run reads the instance metadata and looks for the log entry +// indicating the startup script finished. func (s *StepWaitInstanceStartup) Run(state multistep.StateBag) multistep.StepAction { config := state.Get("config").(*Config) driver := state.Get("driver").(Driver) @@ -21,14 +22,20 @@ func (s *StepWaitInstanceStartup) Run(state multistep.StateBag) multistep.StepAc // Keep checking the serial port output to see if the startup script is done. err := Retry(10, 60, 0, func() (bool, error) { - output, err := driver.GetSerialPortOutput(config.Zone, instanceName) + status, err := driver.GetInstanceMetadata(config.Zone, + instanceName, StartupScriptStatusKey) if err != nil { - err := fmt.Errorf("Error getting serial port output: %s", err) + err := fmt.Errorf("Error getting startup script status: %s", err) return false, err } - - done := strings.Contains(output, StartupScriptDoneLog) + + if status == StartupScriptStatusError { + err = errors.New("Startup script error.") + return false, err + } + + done := status == StartupScriptStatusDone if !done { ui.Say("Startup script not finished yet. Waiting...") } diff --git a/builder/googlecompute/step_wait_instance_startup_test.go b/builder/googlecompute/step_wait_instance_startup_test.go index da6d896d3..1013a10df 100644 --- a/builder/googlecompute/step_wait_instance_startup_test.go +++ b/builder/googlecompute/step_wait_instance_startup_test.go @@ -3,36 +3,28 @@ package googlecompute import ( "github.com/mitchellh/multistep" "testing" + "github.com/stretchr/testify/assert" ) func TestStepWaitInstanceStartup(t *testing.T) { state := testState(t) step := new(StepWaitInstanceStartup) - config := state.Get("config").(*Config) - driver := state.Get("driver").(*DriverMock) + c := state.Get("config").(*Config) + d := state.Get("driver").(*DriverMock) testZone := "test-zone" testInstanceName := "test-instance-name" - config.Zone = testZone + c.Zone = testZone state.Put("instance_name", testInstanceName) - // The done log triggers step completion. - driver.GetSerialPortOutputResult = StartupScriptDoneLog + + // This step stops when it gets Done back from the metadata. + d.GetInstanceMetadataResult = StartupScriptStatusDone // Run the step. - if action := step.Run(state); action != multistep.ActionContinue { - t.Fatalf("StepWaitInstanceStartup did not return a Continue action: %#v", action) - } + assert.Equal(t, step.Run(state), multistep.ActionContinue, "Step should have passed and continued.") - // Check that GetSerialPortOutput was called properly. - if driver.GetSerialPortOutputZone != testZone { - t.Fatalf( - "GetSerialPortOutput wrong zone. Expected: %s, Actual: %s", driver.GetSerialPortOutputZone, - testZone) - } - if driver.GetSerialPortOutputName != testInstanceName { - t.Fatalf( - "GetSerialPortOutput wrong instance name. Expected: %s, Actual: %s", driver.GetSerialPortOutputName, - testInstanceName) - } + // Check that GetInstanceMetadata was called properly. + assert.Equal(t, d.GetInstanceMetadataZone, testZone, "Incorrect zone passed to GetInstanceMetadata.") + assert.Equal(t, d.GetInstanceMetadataName, testInstanceName, "Incorrect instance name passed to GetInstanceMetadata.") } \ No newline at end of file