From 336051e316745a19c985dcc86d031b23e3d89813 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 22:31:12 -0500 Subject: [PATCH] packer: builder prepare can return warnings --- packer/build.go | 11 +++-- packer/build_test.go | 95 +++++++++++++++++++++++++------------- packer/builder.go | 5 +- packer/builder_mock.go | 7 +-- packer/builder_test.go | 30 ------------ packer/environment_test.go | 4 +- packer/template_test.go | 6 +-- 7 files changed, 82 insertions(+), 76 deletions(-) diff --git a/packer/build.go b/packer/build.go index 7e405c546..982dce40b 100644 --- a/packer/build.go +++ b/packer/build.go @@ -38,8 +38,9 @@ type Build interface { Name() string // Prepare configures the various components of this build and reports - // any errors in doing so (such as syntax errors, validation errors, etc.) - Prepare(v map[string]string) error + // any errors in doing so (such as syntax errors, validation errors, etc.). + // It also reports any warnings. + Prepare(v map[string]string) ([]string, error) // Run runs the actual builder, returning an artifact implementation // of what is built. If anything goes wrong, an error is returned. @@ -115,7 +116,7 @@ func (b *coreBuild) Name() string { // Prepare prepares the build by doing some initialization for the builder // and any hooks. This _must_ be called prior to Run. The parameter is the // overrides for the variables within the template (if any). -func (b *coreBuild) Prepare(userVars map[string]string) (err error) { +func (b *coreBuild) Prepare(userVars map[string]string) (warn []string, err error) { b.l.Lock() defer b.l.Unlock() @@ -154,7 +155,7 @@ func (b *coreBuild) Prepare(userVars map[string]string) (err error) { // If there were any problem with variables, return an error right // away because we can't be certain anything else will actually work. if len(varErrs) > 0 { - return &MultiError{ + return nil, &MultiError{ Errors: varErrs, } } @@ -168,7 +169,7 @@ func (b *coreBuild) Prepare(userVars map[string]string) (err error) { } // Prepare the builder - err = b.builder.Prepare(b.builderConfig, packerConfig) + warn, err = b.builder.Prepare(b.builderConfig, packerConfig) if err != nil { log.Printf("Build '%s' prepare failure: %s\n", b.name, err) return diff --git a/packer/build_test.go b/packer/build_test.go index 7ada5d7d8..fce6cf382 100644 --- a/packer/build_test.go +++ b/packer/build_test.go @@ -8,7 +8,7 @@ import ( func testBuild() *coreBuild { return &coreBuild{ name: "test", - builder: &TestBuilder{artifactId: "b"}, + builder: &MockBuilder{ArtifactId: "b"}, builderConfig: 42, builderType: "foo", hooks: map[string][]Hook{ @@ -26,10 +26,6 @@ func testBuild() *coreBuild { } } -func testBuilder() *TestBuilder { - return &TestBuilder{} -} - func testDefaultPackerConfig() map[string]interface{} { return map[string]interface{}{ BuildNameConfigKey: "test", @@ -50,14 +46,14 @@ func TestBuild_Prepare(t *testing.T) { packerConfig := testDefaultPackerConfig() build := testBuild() - builder := build.builder.(*TestBuilder) + builder := build.builder.(*MockBuilder) build.Prepare(nil) - if !builder.prepareCalled { + if !builder.PrepareCalled { t.Fatal("should be called") } - if !reflect.DeepEqual(builder.prepareConfig, []interface{}{42, packerConfig}) { - t.Fatalf("bad: %#v", builder.prepareConfig) + if !reflect.DeepEqual(builder.PrepareConfig, []interface{}{42, packerConfig}) { + t.Fatalf("bad: %#v", builder.PrepareConfig) } coreProv := build.provisioners[0] @@ -81,7 +77,11 @@ func TestBuild_Prepare(t *testing.T) { func TestBuild_Prepare_Twice(t *testing.T) { build := testBuild() - if err := build.Prepare(nil); err != nil { + warn, err := build.Prepare(nil) + if len(warn) > 0 { + t.Fatalf("bad: %#v", warn) + } + if err != nil { t.Fatalf("bad error: %s", err) } @@ -99,20 +99,36 @@ func TestBuild_Prepare_Twice(t *testing.T) { build.Prepare(nil) } +func TestBuildPrepare_BuilderWarniings(t *testing.T) { + expected := []string{"foo"} + + build := testBuild() + builder := build.builder.(*MockBuilder) + builder.PrepareWarnings = expected + + warn, err := build.Prepare(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + if !reflect.DeepEqual(warn, expected) { + t.Fatalf("bad: %#v", warn) + } +} + func TestBuild_Prepare_Debug(t *testing.T) { packerConfig := testDefaultPackerConfig() packerConfig[DebugConfigKey] = true build := testBuild() - builder := build.builder.(*TestBuilder) + builder := build.builder.(*MockBuilder) build.SetDebug(true) build.Prepare(nil) - if !builder.prepareCalled { + if !builder.PrepareCalled { t.Fatalf("should be called") } - if !reflect.DeepEqual(builder.prepareConfig, []interface{}{42, packerConfig}) { - t.Fatalf("bad: %#v", builder.prepareConfig) + if !reflect.DeepEqual(builder.PrepareConfig, []interface{}{42, packerConfig}) { + t.Fatalf("bad: %#v", builder.PrepareConfig) } coreProv := build.provisioners[0] @@ -133,19 +149,22 @@ func TestBuildPrepare_variables_default(t *testing.T) { build := testBuild() build.variables["foo"] = coreBuildVariable{Default: "bar"} - builder := build.builder.(*TestBuilder) + builder := build.builder.(*MockBuilder) - err := build.Prepare(nil) + warn, err := build.Prepare(nil) + if len(warn) > 0 { + t.Fatalf("bad: %#v", warn) + } if err != nil { t.Fatalf("err: %s", err) } - if !builder.prepareCalled { + if !builder.PrepareCalled { t.Fatal("prepare should be called") } - if !reflect.DeepEqual(builder.prepareConfig[1], packerConfig) { - t.Fatalf("prepare bad: %#v", builder.prepareConfig[1]) + if !reflect.DeepEqual(builder.PrepareConfig[1], packerConfig) { + t.Fatalf("prepare bad: %#v", builder.PrepareConfig[1]) } } @@ -153,7 +172,10 @@ func TestBuildPrepare_variables_nonexist(t *testing.T) { build := testBuild() build.variables["foo"] = coreBuildVariable{Default: "bar"} - err := build.Prepare(map[string]string{"bar": "baz"}) + warn, err := build.Prepare(map[string]string{"bar": "baz"}) + if len(warn) > 0 { + t.Fatalf("bad: %#v", warn) + } if err == nil { t.Fatal("should have had error") } @@ -167,19 +189,22 @@ func TestBuildPrepare_variables_override(t *testing.T) { build := testBuild() build.variables["foo"] = coreBuildVariable{Default: "bar"} - builder := build.builder.(*TestBuilder) + builder := build.builder.(*MockBuilder) - err := build.Prepare(map[string]string{"foo": "baz"}) + warn, err := build.Prepare(map[string]string{"foo": "baz"}) + if len(warn) > 0 { + t.Fatalf("bad: %#v", warn) + } if err != nil { t.Fatalf("err: %s", err) } - if !builder.prepareCalled { + if !builder.PrepareCalled { t.Fatal("prepare should be called") } - if !reflect.DeepEqual(builder.prepareConfig[1], packerConfig) { - t.Fatalf("prepare bad: %#v", builder.prepareConfig[1]) + if !reflect.DeepEqual(builder.PrepareConfig[1], packerConfig) { + t.Fatalf("prepare bad: %#v", builder.PrepareConfig[1]) } } @@ -187,7 +212,10 @@ func TestBuildPrepare_variablesRequired(t *testing.T) { build := testBuild() build.variables["foo"] = coreBuildVariable{Required: true} - err := build.Prepare(map[string]string{}) + warn, err := build.Prepare(map[string]string{}) + if len(warn) > 0 { + t.Fatalf("bad: %#v", warn) + } if err == nil { t.Fatal("should have had error") } @@ -195,7 +223,10 @@ func TestBuildPrepare_variablesRequired(t *testing.T) { // Test with setting the value build = testBuild() build.variables["foo"] = coreBuildVariable{Required: true} - err = build.Prepare(map[string]string{"foo": ""}) + warn, err = build.Prepare(map[string]string{"foo": ""}) + if len(warn) > 0 { + t.Fatalf("bad: %#v", warn) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -216,13 +247,13 @@ func TestBuild_Run(t *testing.T) { } // Verify builder was run - builder := build.builder.(*TestBuilder) - if !builder.runCalled { + builder := build.builder.(*MockBuilder) + if !builder.RunCalled { t.Fatal("should be called") } // Verify hooks are disapatchable - dispatchHook := builder.runHook + dispatchHook := builder.RunHook dispatchHook.Run("foo", nil, nil, 42) hook := build.hooks["foo"][0].(*MockHook) @@ -402,8 +433,8 @@ func TestBuild_Cancel(t *testing.T) { build := testBuild() build.Cancel() - builder := build.builder.(*TestBuilder) - if !builder.cancelCalled { + builder := build.builder.(*MockBuilder) + if !builder.CancelCalled { t.Fatal("cancel should be called") } } diff --git a/packer/builder.go b/packer/builder.go index c2e945572..369b9a53e 100644 --- a/packer/builder.go +++ b/packer/builder.go @@ -22,7 +22,10 @@ type Builder interface { // // Each of the configuration values should merge into the final // configuration. - Prepare(...interface{}) error + // + // Prepare should return a list of warnings along with any errors + // that occured while preparing. + Prepare(...interface{}) ([]string, error) // Run is where the actual build should take place. It takes a Build and a Ui. Run(ui Ui, hook Hook, cache Cache) (Artifact, error) diff --git a/packer/builder_mock.go b/packer/builder_mock.go index 72167bd80..c580456e0 100644 --- a/packer/builder_mock.go +++ b/packer/builder_mock.go @@ -4,7 +4,8 @@ package packer // You can set some fake return values and you can keep track of what // methods were called on the builder. It is fairly basic. type MockBuilder struct { - ArtifactId string + ArtifactId string + PrepareWarnings []string PrepareCalled bool PrepareConfig []interface{} @@ -15,10 +16,10 @@ type MockBuilder struct { CancelCalled bool } -func (tb *MockBuilder) Prepare(config ...interface{}) error { +func (tb *MockBuilder) Prepare(config ...interface{}) ([]string, error) { tb.PrepareCalled = true tb.PrepareConfig = config - return nil + return tb.PrepareWarnings, nil } func (tb *MockBuilder) Run(ui Ui, h Hook, c Cache) (Artifact, error) { diff --git a/packer/builder_test.go b/packer/builder_test.go index 09e1f1825..d7c610c15 100644 --- a/packer/builder_test.go +++ b/packer/builder_test.go @@ -1,31 +1 @@ package packer - -type TestBuilder struct { - artifactId string - - prepareCalled bool - prepareConfig []interface{} - runCalled bool - runCache Cache - runHook Hook - runUi Ui - cancelCalled bool -} - -func (tb *TestBuilder) Prepare(config ...interface{}) error { - tb.prepareCalled = true - tb.prepareConfig = config - return nil -} - -func (tb *TestBuilder) Run(ui Ui, h Hook, c Cache) (Artifact, error) { - tb.runCalled = true - tb.runHook = h - tb.runUi = ui - tb.runCache = c - return &TestArtifact{id: tb.artifactId}, nil -} - -func (tb *TestBuilder) Cancel() { - tb.cancelCalled = true -} diff --git a/packer/environment_test.go b/packer/environment_test.go index 1a4751281..b5ac7b9c8 100644 --- a/packer/environment_test.go +++ b/packer/environment_test.go @@ -17,7 +17,7 @@ func init() { } func testComponentFinder() *ComponentFinder { - builderFactory := func(n string) (Builder, error) { return testBuilder(), nil } + builderFactory := func(n string) (Builder, error) { return new(MockBuilder), nil } ppFactory := func(n string) (PostProcessor, error) { return new(TestPostProcessor), nil } provFactory := func(n string) (Provisioner, error) { return new(MockProvisioner), nil } return &ComponentFinder{ @@ -97,7 +97,7 @@ func TestEnvironment_NilComponents(t *testing.T) { } func TestEnvironment_Builder(t *testing.T) { - builder := &TestBuilder{} + builder := &MockBuilder{} builders := make(map[string]Builder) builders["foo"] = builder diff --git a/packer/template_test.go b/packer/template_test.go index 57ba31e47..d5299f4ac 100644 --- a/packer/template_test.go +++ b/packer/template_test.go @@ -9,7 +9,7 @@ import ( ) func testTemplateComponentFinder() *ComponentFinder { - builder := testBuilder() + builder := new(MockBuilder) pp := new(TestPostProcessor) provisioner := &MockProvisioner{} @@ -706,7 +706,7 @@ func TestTemplate_Build(t *testing.T) { t.Fatalf("err: %s", err) } - builder := testBuilder() + builder := new(MockBuilder) builderMap := map[string]Builder{ "test-builder": builder, } @@ -1194,7 +1194,7 @@ func TestTemplate_Build_ProvisionerOverride(t *testing.T) { t.Fatalf("bad raw: %#v", RawConfig) } - builder := testBuilder() + builder := new(MockBuilder) builderMap := map[string]Builder{ "test-builder": builder, }