From 6d6b3e1ac2ae03546269744d788f5e631fb0f810 Mon Sep 17 00:00:00 2001 From: Samit Pal Date: Mon, 6 Jul 2015 09:32:08 +0000 Subject: [PATCH 1/3] The default image name in the code has a bug. It is being set to packer-{{timestamp}}, the {{timestamp}} part needs to be interpolated. Without the interpolation the GCE builder fails with the following error ==> googlecompute: Creating image... ==> googlecompute: Error waiting for image: googleapi: Error 400: Invalid value for field 'resource.name': 'packer-{{timestamp}}'. Must be a match of regex '(?:[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?)', invalid --- builder/googlecompute/config.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index eebea011f..dc049aa1c 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -73,7 +73,13 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { } if c.ImageName == "" { - c.ImageName = "packer-{{timestamp}}" + img, err := interpolate.Render("packer-{{timestamp}}", nil) + if err != nil { + panic(err) + } + + // Default to packer-{{ unix timestamp (utc) }} + c.ImageName = img } if c.InstanceName == "" { From bd6c31c2d902822909b6a4c6cc94e689ebdc7a41 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 7 Jul 2015 16:18:31 -0600 Subject: [PATCH 2/3] Added TestImageName and moved private methods to the bottom of the file --- builder/googlecompute/config_test.go | 88 ++++++++++++++++------------ 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index 93997912e..581c1425b 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -2,48 +2,10 @@ package googlecompute import ( "io/ioutil" + "strings" "testing" ) -func testConfig(t *testing.T) map[string]interface{} { - return map[string]interface{}{ - "account_file": testAccountFile(t), - "project_id": "hashicorp", - "source_image": "foo", - "zone": "us-east-1a", - } -} - -func testConfigStruct(t *testing.T) *Config { - c, warns, errs := NewConfig(testConfig(t)) - if len(warns) > 0 { - t.Fatalf("bad: %#v", len(warns)) - } - if errs != nil { - t.Fatalf("bad: %#v", errs) - } - - return c -} - -func testConfigErr(t *testing.T, warns []string, err error, extra string) { - if len(warns) > 0 { - t.Fatalf("bad: %#v", warns) - } - if err == nil { - t.Fatalf("should error: %s", extra) - } -} - -func testConfigOk(t *testing.T, warns []string, err error) { - if len(warns) > 0 { - t.Fatalf("bad: %#v", warns) - } - if err != nil { - t.Fatalf("bad: %s", err) - } -} - func TestConfigPrepare(t *testing.T) { cases := []struct { Key string @@ -181,6 +143,54 @@ func TestConfigDefaults(t *testing.T) { } } +func TestImageName(t *testing.T) { + c, _, _ := NewConfig(testConfig(t)) + if strings.Contains(c.ImageName, "{{timestamp}}") { + t.Errorf("ImageName should be interpolated; found %s", c.ImageName) + } +} + +// Helper stuff below + +func testConfig(t *testing.T) map[string]interface{} { + return map[string]interface{}{ + "account_file": testAccountFile(t), + "project_id": "hashicorp", + "source_image": "foo", + "zone": "us-east-1a", + } +} + +func testConfigStruct(t *testing.T) *Config { + c, warns, errs := NewConfig(testConfig(t)) + if len(warns) > 0 { + t.Fatalf("bad: %#v", len(warns)) + } + if errs != nil { + t.Fatalf("bad: %#v", errs) + } + + return c +} + +func testConfigErr(t *testing.T, warns []string, err error, extra string) { + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err == nil { + t.Fatalf("should error: %s", extra) + } +} + +func testConfigOk(t *testing.T, warns []string, err error) { + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err != nil { + t.Fatalf("bad: %s", err) + } +} + func testAccountFile(t *testing.T) string { tf, err := ioutil.TempFile("", "packer") if err != nil { From 1c71eaaa91cf43f24eac59d412907c0bcd58cc37 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 7 Jul 2015 17:12:21 -0600 Subject: [PATCH 3/3] Change panic to multierror --- builder/googlecompute/config.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index dc049aa1c..317d64ace 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -59,6 +59,8 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { return nil, nil, err } + var errs *packer.MultiError + // Set defaults. if c.Network == "" { c.Network = "default" @@ -75,11 +77,10 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { if c.ImageName == "" { img, err := interpolate.Render("packer-{{timestamp}}", nil) if err != nil { - panic(err) + errs = packer.MultiErrorAppend(errs, + fmt.Errorf("Unable to parse image name: %s ", err)) + c.ImageName = img } - - // Default to packer-{{ unix timestamp (utc) }} - c.ImageName = img } if c.InstanceName == "" { @@ -102,7 +103,6 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { c.Comm.SSHUsername = "root" } - var errs *packer.MultiError if es := c.Comm.Prepare(&c.ctx); len(es) > 0 { errs = packer.MultiErrorAppend(errs, es...) }