From 336051e316745a19c985dcc86d031b23e3d89813 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 22:31:12 -0500 Subject: [PATCH 01/11] 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, } From 0b61e50621d505087550efc4704a4b055f8d8755 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 22:40:06 -0500 Subject: [PATCH 02/11] packer/rpc: implement new warnings interfaces --- packer/rpc/build.go | 22 +++++++++++++----- packer/rpc/build_test.go | 47 +++++++++++++++++++++++++++----------- packer/rpc/builder.go | 24 +++++++++++++------ packer/rpc/builder_test.go | 4 ++-- 4 files changed, 69 insertions(+), 28 deletions(-) diff --git a/packer/rpc/build.go b/packer/rpc/build.go index 8f21b4ce9..2df03a6ab 100644 --- a/packer/rpc/build.go +++ b/packer/rpc/build.go @@ -21,6 +21,11 @@ type BuildRunArgs struct { UiRPCAddress string } +type BuildPrepareResponse struct { + Warnings []string + Error error +} + func Build(client *rpc.Client) *build { return &build{client} } @@ -30,12 +35,13 @@ func (b *build) Name() (result string) { return } -func (b *build) Prepare(v map[string]string) (err error) { - if cerr := b.client.Call("Build.Prepare", v, &err); cerr != nil { - return cerr +func (b *build) Prepare(v map[string]string) ([]string, error) { + var resp BuildPrepareResponse + if cerr := b.client.Call("Build.Prepare", v, &resp); cerr != nil { + return nil, cerr } - return + return resp.Warnings, resp.Error } func (b *build) Run(ui packer.Ui, cache packer.Cache) ([]packer.Artifact, error) { @@ -86,8 +92,12 @@ func (b *BuildServer) Name(args *interface{}, reply *string) error { return nil } -func (b *BuildServer) Prepare(v map[string]string, reply *error) error { - *reply = b.build.Prepare(v) +func (b *BuildServer) Prepare(v map[string]string, resp *BuildPrepareResponse) error { + warnings, err := b.build.Prepare(v) + *resp = BuildPrepareResponse{ + Warnings: warnings, + Error: err, + } return nil } diff --git a/packer/rpc/build_test.go b/packer/rpc/build_test.go index bee9b6796..3cb5e1fb2 100644 --- a/packer/rpc/build_test.go +++ b/packer/rpc/build_test.go @@ -4,21 +4,23 @@ import ( "errors" "github.com/mitchellh/packer/packer" "net/rpc" + "reflect" "testing" ) var testBuildArtifact = &testArtifact{} type testBuild struct { - nameCalled bool - prepareCalled bool - prepareVars map[string]string - runCalled bool - runCache packer.Cache - runUi packer.Ui - setDebugCalled bool - setForceCalled bool - cancelCalled bool + nameCalled bool + prepareCalled bool + prepareVars map[string]string + prepareWarnings []string + runCalled bool + runCache packer.Cache + runUi packer.Ui + setDebugCalled bool + setForceCalled bool + cancelCalled bool errRunResult bool } @@ -28,10 +30,10 @@ func (b *testBuild) Name() string { return "name" } -func (b *testBuild) Prepare(v map[string]string) error { +func (b *testBuild) Prepare(v map[string]string) ([]string, error) { b.prepareCalled = true b.prepareVars = v - return nil + return b.prepareWarnings, nil } func (b *testBuild) Run(ui packer.Ui, cache packer.Cache) ([]packer.Artifact, error) { @@ -58,7 +60,7 @@ func (b *testBuild) Cancel() { b.cancelCalled = true } -func TestBuildRPC(t *testing.T) { +func buildRPCClient(t *testing.T) (*testBuild, packer.Build) { // Create the interface to test b := new(testBuild) @@ -72,7 +74,11 @@ func TestBuildRPC(t *testing.T) { if err != nil { t.Fatalf("err: %s", err) } - bClient := Build(client) + return b, Build(client) +} + +func TestBuild(t *testing.T) { + b, bClient := buildRPCClient(t) // Test Name bClient.Name() @@ -157,6 +163,21 @@ func TestBuildRPC(t *testing.T) { } } +func TestBuildPrepare_Warnings(t *testing.T) { + b, bClient := buildRPCClient(t) + + expected := []string{"foo"} + b.prepareWarnings = expected + + warnings, err := bClient.Prepare(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + if !reflect.DeepEqual(warnings, expected) { + t.Fatalf("bad: %#v", warnings) + } +} + func TestBuild_ImplementsBuild(t *testing.T) { var _ packer.Build = Build(nil) } diff --git a/packer/rpc/builder.go b/packer/rpc/builder.go index 5e2c20d77..42f92c4d4 100644 --- a/packer/rpc/builder.go +++ b/packer/rpc/builder.go @@ -29,6 +29,11 @@ type BuilderRunArgs struct { ResponseAddress string } +type BuilderPrepareResponse struct { + Warnings []string + Error error +} + type BuilderRunResponse struct { Err error RPCAddress string @@ -38,13 +43,14 @@ func Builder(client *rpc.Client) *builder { return &builder{client} } -func (b *builder) Prepare(config ...interface{}) (err error) { - cerr := b.client.Call("Builder.Prepare", &BuilderPrepareArgs{config}, &err) +func (b *builder) Prepare(config ...interface{}) ([]string, error) { + var resp BuilderPrepareResponse + cerr := b.client.Call("Builder.Prepare", &BuilderPrepareArgs{config}, &resp) if cerr != nil { - err = cerr + return nil, cerr } - return + return resp.Warnings, resp.Error } func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { @@ -108,12 +114,16 @@ func (b *builder) Cancel() { } } -func (b *BuilderServer) Prepare(args *BuilderPrepareArgs, reply *error) error { - err := b.builder.Prepare(args.Configs...) +func (b *BuilderServer) Prepare(args *BuilderPrepareArgs, reply *BuilderPrepareResponse) error { + warnings, err := b.builder.Prepare(args.Configs...) if err != nil { - *reply = NewBasicError(err) + err = NewBasicError(err) } + *reply = BuilderPrepareResponse{ + Warnings: warnings, + Error: err, + } return nil } diff --git a/packer/rpc/builder_test.go b/packer/rpc/builder_test.go index 6ce8df78e..b56c8afaf 100644 --- a/packer/rpc/builder_test.go +++ b/packer/rpc/builder_test.go @@ -23,10 +23,10 @@ type testBuilder struct { nilRunResult bool } -func (b *testBuilder) Prepare(config ...interface{}) error { +func (b *testBuilder) Prepare(config ...interface{}) ([]string, error) { b.prepareCalled = true b.prepareConfig = config - return nil + return nil, nil } func (b *testBuilder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { From 230cc9738e80c6d80dbdb5f629ec56b16dba0037 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 22:47:23 -0500 Subject: [PATCH 03/11] packer/rpc: use packer.MockBuilder for tests --- packer/builder_mock.go | 15 ++++ packer/rpc/builder_test.go | 138 ++++++++++++++------------------- packer/rpc/environment_test.go | 4 +- 3 files changed, 76 insertions(+), 81 deletions(-) diff --git a/packer/builder_mock.go b/packer/builder_mock.go index c580456e0..bfa0a0e47 100644 --- a/packer/builder_mock.go +++ b/packer/builder_mock.go @@ -1,11 +1,17 @@ package packer +import ( + "errors" +) + // MockBuilder is an implementation of Builder that can be used for tests. // 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 PrepareWarnings []string + RunErrResult bool + RunNilResult bool PrepareCalled bool PrepareConfig []interface{} @@ -27,6 +33,15 @@ func (tb *MockBuilder) Run(ui Ui, h Hook, c Cache) (Artifact, error) { tb.RunHook = h tb.RunUi = ui tb.RunCache = c + + if tb.RunErrResult { + return nil, errors.New("foo") + } + + if tb.RunNilResult { + return nil, nil + } + return &MockArtifact{ IdValue: tb.ArtifactId, }, nil diff --git a/packer/rpc/builder_test.go b/packer/rpc/builder_test.go index b56c8afaf..402cb6920 100644 --- a/packer/rpc/builder_test.go +++ b/packer/rpc/builder_test.go @@ -1,7 +1,6 @@ package rpc import ( - "errors" "github.com/mitchellh/packer/packer" "net/rpc" "reflect" @@ -10,47 +9,8 @@ import ( var testBuilderArtifact = &testArtifact{} -type testBuilder struct { - prepareCalled bool - prepareConfig []interface{} - runCalled bool - runCache packer.Cache - runHook packer.Hook - runUi packer.Ui - cancelCalled bool - - errRunResult bool - nilRunResult bool -} - -func (b *testBuilder) Prepare(config ...interface{}) ([]string, error) { - b.prepareCalled = true - b.prepareConfig = config - return nil, nil -} - -func (b *testBuilder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { - b.runCache = cache - b.runCalled = true - b.runHook = hook - b.runUi = ui - - if b.errRunResult { - return nil, errors.New("foo") - } else if b.nilRunResult { - return nil, nil - } else { - return testBuilderArtifact, nil - } -} - -func (b *testBuilder) Cancel() { - b.cancelCalled = true -} - -func TestBuilderRPC(t *testing.T) { - // Create the interface to test - b := new(testBuilder) +func builderRPCClient(t *testing.T) (*packer.MockBuilder, packer.Builder) { + b := new(packer.MockBuilder) // Start the server server := rpc.NewServer() @@ -62,18 +22,26 @@ func TestBuilderRPC(t *testing.T) { if err != nil { t.Fatalf("err: %s", err) } + return b, Builder(client) +} + +func TestBuilderPrepare(t *testing.T) { + b, bClient := builderRPCClient(t) // Test Prepare config := 42 - bClient := Builder(client) bClient.Prepare(config) - if !b.prepareCalled { + if !b.PrepareCalled { t.Fatal("should be called") } - if !reflect.DeepEqual(b.prepareConfig, []interface{}{42}) { - t.Fatalf("bad: %#v", b.prepareConfig) + if !reflect.DeepEqual(b.PrepareConfig, []interface{}{42}) { + t.Fatalf("bad: %#v", b.PrepareConfig) } +} + +func TestBuilderRun(t *testing.T) { + b, bClient := builderRPCClient(t) // Test Run cache := new(testCache) @@ -84,59 +52,71 @@ func TestBuilderRPC(t *testing.T) { t.Fatalf("err: %s", err) } - if !b.runCalled { + if !b.RunCalled { t.Fatal("run should be called") } - if b.runCalled { - b.runCache.Lock("foo") - if !cache.lockCalled { - t.Fatal("should be called") - } - - b.runHook.Run("foo", nil, nil, nil) - if !hook.RunCalled { - t.Fatal("should be called") - } - - b.runUi.Say("format") - if !ui.sayCalled { - t.Fatal("say should be called") - } - - if ui.sayMessage != "format" { - t.Fatalf("bad: %s", ui.sayMessage) - } - - if artifact.Id() != testBuilderArtifact.Id() { - t.Fatalf("bad: %s", artifact.Id()) - } + b.RunCache.Lock("foo") + if !cache.lockCalled { + t.Fatal("should be called") } - // Test run with nil result - b.nilRunResult = true - artifact, err = bClient.Run(ui, hook, cache) + b.RunHook.Run("foo", nil, nil, nil) + if !hook.RunCalled { + t.Fatal("should be called") + } + + b.RunUi.Say("format") + if !ui.sayCalled { + t.Fatal("say should be called") + } + + if ui.sayMessage != "format" { + t.Fatalf("bad: %s", ui.sayMessage) + } + + if artifact.Id() != testBuilderArtifact.Id() { + t.Fatalf("bad: %s", artifact.Id()) + } +} + +func TestBuilderRun_nilResult(t *testing.T) { + b, bClient := builderRPCClient(t) + b.RunNilResult = true + + cache := new(testCache) + hook := &packer.MockHook{} + ui := &testUi{} + artifact, err := bClient.Run(ui, hook, cache) if artifact != nil { t.Fatalf("bad: %#v", artifact) } if err != nil { t.Fatalf("bad: %#v", err) } +} - // Test with an error - b.errRunResult = true - b.nilRunResult = false - artifact, err = bClient.Run(ui, hook, cache) +func TestBuilderRun_ErrResult(t *testing.T) { + b, bClient := builderRPCClient(t) + b.RunErrResult = true + + cache := new(testCache) + hook := &packer.MockHook{} + ui := &testUi{} + artifact, err := bClient.Run(ui, hook, cache) if artifact != nil { t.Fatalf("bad: %#v", artifact) } if err == nil { t.Fatal("should have error") } +} + +func TestBuilderCancel(t *testing.T) { + b, bClient := builderRPCClient(t) - // Test Cancel bClient.Cancel() - if !b.cancelCalled { + if !b.CancelCalled { t.Fatal("cancel should be called") } } diff --git a/packer/rpc/environment_test.go b/packer/rpc/environment_test.go index 39d020ce6..cb80929ec 100644 --- a/packer/rpc/environment_test.go +++ b/packer/rpc/environment_test.go @@ -7,7 +7,7 @@ import ( "testing" ) -var testEnvBuilder = &testBuilder{} +var testEnvBuilder = &packer.MockBuilder{} var testEnvCache = &testCache{} var testEnvUi = &testUi{} @@ -90,7 +90,7 @@ func TestEnvironmentRPC(t *testing.T) { } builder.Prepare(nil) - if !testEnvBuilder.prepareCalled { + if !testEnvBuilder.PrepareCalled { t.Fatal("should be called") } From b2b125d83b55dc16cbd518ad167f316e9bfe92b7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 22:49:10 -0500 Subject: [PATCH 04/11] packer/rpc: test warnings with builders --- packer/rpc/builder_test.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/packer/rpc/builder_test.go b/packer/rpc/builder_test.go index 402cb6920..fa20a4b85 100644 --- a/packer/rpc/builder_test.go +++ b/packer/rpc/builder_test.go @@ -30,7 +30,14 @@ func TestBuilderPrepare(t *testing.T) { // Test Prepare config := 42 - bClient.Prepare(config) + warnings, err := bClient.Prepare(config) + if err != nil { + t.Fatalf("bad: %s", err) + } + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } + if !b.PrepareCalled { t.Fatal("should be called") } @@ -40,6 +47,22 @@ func TestBuilderPrepare(t *testing.T) { } } +func TestBuilderPrepare_Warnings(t *testing.T) { + b, bClient := builderRPCClient(t) + + expected := []string{"foo"} + b.PrepareWarnings = expected + + // Test Prepare + warnings, err := bClient.Prepare(nil) + if err != nil { + t.Fatalf("bad: %s", err) + } + if !reflect.DeepEqual(warnings, expected) { + t.Fatalf("bad: %#v", warnings) + } +} + func TestBuilderRun(t *testing.T) { b, bClient := builderRPCClient(t) From 82cbf13f8250cd8c567a5c733777c5ace090ea35 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 22:51:26 -0500 Subject: [PATCH 05/11] packer/plugin: new Builder interface --- packer/plugin/builder.go | 2 +- packer/plugin/builder_test.go | 13 ------------- packer/plugin/plugin_test.go | 2 +- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/packer/plugin/builder.go b/packer/plugin/builder.go index b8999ff94..a0a89d57b 100644 --- a/packer/plugin/builder.go +++ b/packer/plugin/builder.go @@ -10,7 +10,7 @@ type cmdBuilder struct { client *Client } -func (b *cmdBuilder) Prepare(config ...interface{}) error { +func (b *cmdBuilder) Prepare(config ...interface{}) ([]string, error) { defer func() { r := recover() b.checkExit(r, nil) diff --git a/packer/plugin/builder_test.go b/packer/plugin/builder_test.go index e3356f50f..23a80999f 100644 --- a/packer/plugin/builder_test.go +++ b/packer/plugin/builder_test.go @@ -1,23 +1,10 @@ package plugin import ( - "github.com/mitchellh/packer/packer" "os/exec" "testing" ) -type helperBuilder byte - -func (helperBuilder) Prepare(...interface{}) error { - return nil -} - -func (helperBuilder) Run(packer.Ui, packer.Hook, packer.Cache) (packer.Artifact, error) { - return nil, nil -} - -func (helperBuilder) Cancel() {} - func TestBuilder_NoExist(t *testing.T) { c := NewClient(&ClientConfig{Cmd: exec.Command("i-should-not-exist")}) defer c.Kill() diff --git a/packer/plugin/plugin_test.go b/packer/plugin/plugin_test.go index 10c3f9d5c..d80aceaf7 100644 --- a/packer/plugin/plugin_test.go +++ b/packer/plugin/plugin_test.go @@ -54,7 +54,7 @@ func TestHelperProcess(*testing.T) { fmt.Printf("%s1|:1234\n", APIVersion) <-make(chan int) case "builder": - ServeBuilder(new(helperBuilder)) + ServeBuilder(new(packer.MockBuilder)) case "command": ServeCommand(new(helperCommand)) case "hook": From 3cd7379d1f626908552d9d58b08f3f8112f8a048 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 22:56:54 -0500 Subject: [PATCH 06/11] builder/amazon/*: warnings --- builder/amazon/chroot/builder.go | 10 +-- builder/amazon/chroot/builder_test.go | 40 ++++++++--- builder/amazon/ebs/builder.go | 10 +-- builder/amazon/ebs/builder_test.go | 25 +++++-- builder/amazon/instance/builder.go | 10 +-- builder/amazon/instance/builder_test.go | 90 ++++++++++++++++++++----- 6 files changed, 139 insertions(+), 46 deletions(-) diff --git a/builder/amazon/chroot/builder.go b/builder/amazon/chroot/builder.go index b0a9df638..e898f4173 100644 --- a/builder/amazon/chroot/builder.go +++ b/builder/amazon/chroot/builder.go @@ -45,15 +45,15 @@ type Builder struct { runner multistep.Runner } -func (b *Builder) Prepare(raws ...interface{}) error { +func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { md, err := common.DecodeConfig(&b.config, raws...) if err != nil { - return err + return nil, err } b.config.tpl, err = packer.NewConfigTemplate() if err != nil { - return err + return nil, err } b.config.tpl.UserVars = b.config.PackerUserVars b.config.tpl.Funcs(awscommon.TemplateFuncs) @@ -140,11 +140,11 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if errs != nil && len(errs.Errors) > 0 { - return errs + return nil, errs } log.Println(common.ScrubConfig(b.config, b.config.AccessKey, b.config.SecretKey)) - return nil + return nil, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/amazon/chroot/builder_test.go b/builder/amazon/chroot/builder_test.go index 6bdf7dc28..291651477 100644 --- a/builder/amazon/chroot/builder_test.go +++ b/builder/amazon/chroot/builder_test.go @@ -26,7 +26,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test good config["ami_name"] = "foo" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -34,7 +37,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test bad config["ami_name"] = "foo {{" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -42,7 +48,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test bad delete(config, "ami_name") b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -53,7 +62,10 @@ func TestBuilderPrepare_ChrootMounts(t *testing.T) { config := testConfig() config["chroot_mounts"] = nil - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Errorf("err: %s", err) } @@ -61,7 +73,10 @@ func TestBuilderPrepare_ChrootMounts(t *testing.T) { config["chroot_mounts"] = [][]string{ []string{"bad"}, } - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -71,13 +86,19 @@ func TestBuilderPrepare_SourceAmi(t *testing.T) { config := testConfig() config["source_ami"] = "" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } config["source_ami"] = "foo" - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Errorf("err: %s", err) } @@ -88,7 +109,10 @@ func TestBuilderPrepare_CommandWrapper(t *testing.T) { config := testConfig() config["command_wrapper"] = "echo hi; {{.Command}}" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Errorf("err: %s", err) } diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index f38c6826c..a590b96e7 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -33,15 +33,15 @@ type Builder struct { runner multistep.Runner } -func (b *Builder) Prepare(raws ...interface{}) error { +func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { md, err := common.DecodeConfig(&b.config, raws...) if err != nil { - return err + return nil, err } b.config.tpl, err = packer.NewConfigTemplate() if err != nil { - return err + return nil, err } b.config.tpl.UserVars = b.config.PackerUserVars b.config.tpl.Funcs(awscommon.TemplateFuncs) @@ -53,11 +53,11 @@ func (b *Builder) Prepare(raws ...interface{}) error { errs = packer.MultiErrorAppend(errs, b.config.RunConfig.Prepare(b.config.tpl)...) if errs != nil && len(errs.Errors) > 0 { - return errs + return nil, errs } log.Println(common.ScrubConfig(b.config, b.config.AccessKey, b.config.SecretKey)) - return nil + return nil, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/amazon/ebs/builder_test.go b/builder/amazon/ebs/builder_test.go index 3ad6d17b6..6f777afa9 100644 --- a/builder/amazon/ebs/builder_test.go +++ b/builder/amazon/ebs/builder_test.go @@ -31,7 +31,10 @@ func TestBuilder_Prepare_BadType(t *testing.T) { "access_key": []string{}, } - err := b.Prepare(c) + warnings, err := b.Prepare(c) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatalf("prepare should fail") } @@ -43,7 +46,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test good config["ami_name"] = "foo" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -51,7 +57,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test bad config["ami_name"] = "foo {{" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -59,7 +68,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test bad delete(config, "ami_name") b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -71,7 +83,10 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) { // Add a random key config["i_should_not_be_valid"] = true - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } diff --git a/builder/amazon/instance/builder.go b/builder/amazon/instance/builder.go index 8b1a5f6af..e44ac31a9 100644 --- a/builder/amazon/instance/builder.go +++ b/builder/amazon/instance/builder.go @@ -45,15 +45,15 @@ type Builder struct { runner multistep.Runner } -func (b *Builder) Prepare(raws ...interface{}) error { +func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { md, err := common.DecodeConfig(&b.config, raws...) if err != nil { - return err + return nil, err } b.config.tpl, err = packer.NewConfigTemplate() if err != nil { - return err + return nil, err } b.config.tpl.UserVars = b.config.PackerUserVars b.config.tpl.Funcs(awscommon.TemplateFuncs) @@ -156,11 +156,11 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if errs != nil && len(errs.Errors) > 0 { - return errs + return nil, errs } log.Println(common.ScrubConfig(b.config, b.config.AccessKey, b.config.SecretKey)) - return nil + return nil, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/amazon/instance/builder_test.go b/builder/amazon/instance/builder_test.go index c61a6f73d..e15c45131 100644 --- a/builder/amazon/instance/builder_test.go +++ b/builder/amazon/instance/builder_test.go @@ -40,19 +40,28 @@ func TestBuilderPrepare_AccountId(t *testing.T) { config := testConfig() config["account_id"] = "" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } config["account_id"] = "foo" - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Errorf("err: %s", err) } config["account_id"] = "0123-0456-7890" - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("err: %s", err) } @@ -68,7 +77,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test good config["ami_name"] = "foo" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -76,7 +88,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test bad config["ami_name"] = "foo {{" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -84,7 +99,10 @@ func TestBuilderPrepare_AMIName(t *testing.T) { // Test bad delete(config, "ami_name") b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -95,7 +113,10 @@ func TestBuilderPrepare_BundleDestination(t *testing.T) { config := testConfig() config["bundle_destination"] = "" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("err: %s", err) } @@ -110,7 +131,10 @@ func TestBuilderPrepare_BundlePrefix(t *testing.T) { config := testConfig() config["bundle_prefix"] = "" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("err: %s", err) } @@ -126,7 +150,10 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) { // Add a random key config["i_should_not_be_valid"] = true - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -137,13 +164,19 @@ func TestBuilderPrepare_S3Bucket(t *testing.T) { config := testConfig() config["s3_bucket"] = "" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } config["s3_bucket"] = "foo" - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Errorf("err: %s", err) } @@ -154,13 +187,19 @@ func TestBuilderPrepare_X509CertPath(t *testing.T) { config := testConfig() config["x509_cert_path"] = "" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } config["x509_cert_path"] = "i/am/a/file/that/doesnt/exist" - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Error("should have error") } @@ -172,7 +211,10 @@ func TestBuilderPrepare_X509CertPath(t *testing.T) { defer os.Remove(tf.Name()) config["x509_cert_path"] = tf.Name() - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -183,13 +225,19 @@ func TestBuilderPrepare_X509KeyPath(t *testing.T) { config := testConfig() config["x509_key_path"] = "" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } config["x509_key_path"] = "i/am/a/file/that/doesnt/exist" - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Error("should have error") } @@ -201,7 +249,10 @@ func TestBuilderPrepare_X509KeyPath(t *testing.T) { defer os.Remove(tf.Name()) config["x509_key_path"] = tf.Name() - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -212,7 +263,10 @@ func TestBuilderPrepare_X509UploadPath(t *testing.T) { config := testConfig() config["x509_upload_path"] = "" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } From a6150e6596ef938b15286609555db63523e33c80 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 23:03:59 -0500 Subject: [PATCH 07/11] builder/all: update to warnings --- builder/digitalocean/builder.go | 10 +- builder/digitalocean/builder_test.go | 125 +++++++++--- builder/openstack/builder.go | 10 +- builder/openstack/builder_test.go | 25 ++- builder/virtualbox/builder.go | 10 +- builder/virtualbox/builder_test.go | 280 +++++++++++++++++++++------ builder/vmware/builder.go | 10 +- builder/vmware/builder_test.go | 225 ++++++++++++++++----- 8 files changed, 544 insertions(+), 151 deletions(-) diff --git a/builder/digitalocean/builder.go b/builder/digitalocean/builder.go index acf92c240..ae6564eb9 100644 --- a/builder/digitalocean/builder.go +++ b/builder/digitalocean/builder.go @@ -49,15 +49,15 @@ type Builder struct { runner multistep.Runner } -func (b *Builder) Prepare(raws ...interface{}) error { +func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { md, err := common.DecodeConfig(&b.config, raws...) if err != nil { - return err + return nil, err } b.config.tpl, err = packer.NewConfigTemplate() if err != nil { - return err + return nil, err } b.config.tpl.UserVars = b.config.PackerUserVars @@ -161,11 +161,11 @@ func (b *Builder) Prepare(raws ...interface{}) error { b.config.stateTimeout = stateTimeout if errs != nil && len(errs.Errors) > 0 { - return errs + return nil, errs } common.ScrubConfig(b.config, b.config.ClientID, b.config.APIKey) - return nil + return nil, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/digitalocean/builder_test.go b/builder/digitalocean/builder_test.go index cd19f0f31..1d5deea00 100644 --- a/builder/digitalocean/builder_test.go +++ b/builder/digitalocean/builder_test.go @@ -34,7 +34,10 @@ func TestBuilder_Prepare_BadType(t *testing.T) { "api_key": []string{}, } - err := b.Prepare(c) + warnings, err := b.Prepare(c) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatalf("prepare should fail") } @@ -46,7 +49,10 @@ func TestBuilderPrepare_APIKey(t *testing.T) { // Test good config["api_key"] = "foo" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -58,7 +64,10 @@ func TestBuilderPrepare_APIKey(t *testing.T) { // Test bad delete(config, "api_key") b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -67,7 +76,10 @@ func TestBuilderPrepare_APIKey(t *testing.T) { delete(config, "api_key") os.Setenv("DIGITALOCEAN_API_KEY", "foo") defer os.Setenv("DIGITALOCEAN_API_KEY", "") - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -79,7 +91,10 @@ func TestBuilderPrepare_ClientID(t *testing.T) { // Test good config["client_id"] = "foo" - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -91,7 +106,10 @@ func TestBuilderPrepare_ClientID(t *testing.T) { // Test bad delete(config, "client_id") b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -100,7 +118,10 @@ func TestBuilderPrepare_ClientID(t *testing.T) { delete(config, "client_id") os.Setenv("DIGITALOCEAN_CLIENT_ID", "foo") defer os.Setenv("DIGITALOCEAN_CLIENT_ID", "") - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -112,7 +133,10 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) { // Add a random key config["i_should_not_be_valid"] = true - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -123,7 +147,10 @@ func TestBuilderPrepare_RegionID(t *testing.T) { config := testConfig() // Test default - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -135,7 +162,10 @@ func TestBuilderPrepare_RegionID(t *testing.T) { // Test set config["region_id"] = 2 b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -150,7 +180,10 @@ func TestBuilderPrepare_SizeID(t *testing.T) { config := testConfig() // Test default - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -162,7 +195,10 @@ func TestBuilderPrepare_SizeID(t *testing.T) { // Test set config["size_id"] = 67 b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -177,7 +213,10 @@ func TestBuilderPrepare_ImageID(t *testing.T) { config := testConfig() // Test default - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -189,7 +228,10 @@ func TestBuilderPrepare_ImageID(t *testing.T) { // Test set config["size_id"] = 2 b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -204,7 +246,10 @@ func TestBuilderPrepare_SSHUsername(t *testing.T) { config := testConfig() // Test default - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -216,7 +261,10 @@ func TestBuilderPrepare_SSHUsername(t *testing.T) { // Test set config["ssh_username"] = "foo" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -231,7 +279,10 @@ func TestBuilderPrepare_SSHTimeout(t *testing.T) { config := testConfig() // Test default - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -243,7 +294,10 @@ func TestBuilderPrepare_SSHTimeout(t *testing.T) { // Test set config["ssh_timeout"] = "30s" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -251,7 +305,10 @@ func TestBuilderPrepare_SSHTimeout(t *testing.T) { // Test bad config["ssh_timeout"] = "tubes" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -263,7 +320,10 @@ func TestBuilderPrepare_StateTimeout(t *testing.T) { config := testConfig() // Test default - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -275,7 +335,10 @@ func TestBuilderPrepare_StateTimeout(t *testing.T) { // Test set config["state_timeout"] = "5m" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -283,7 +346,10 @@ func TestBuilderPrepare_StateTimeout(t *testing.T) { // Test bad config["state_timeout"] = "tubes" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err == nil { t.Fatal("should have error") } @@ -295,7 +361,10 @@ func TestBuilderPrepare_SnapshotName(t *testing.T) { config := testConfig() // Test default - err := b.Prepare(config) + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -307,7 +376,10 @@ func TestBuilderPrepare_SnapshotName(t *testing.T) { // Test set config["snapshot_name"] = "foobarbaz" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -315,7 +387,10 @@ func TestBuilderPrepare_SnapshotName(t *testing.T) { // Test set with template config["snapshot_name"] = "{{timestamp}}" b = Builder{} - err = b.Prepare(config) + warnings, err = b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } if err != nil { t.Fatalf("should not have error: %s", err) } diff --git a/builder/openstack/builder.go b/builder/openstack/builder.go index e1db7c5d5..3455f087c 100644 --- a/builder/openstack/builder.go +++ b/builder/openstack/builder.go @@ -29,15 +29,15 @@ type Builder struct { runner multistep.Runner } -func (b *Builder) Prepare(raws ...interface{}) error { +func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { md, err := common.DecodeConfig(&b.config, raws...) if err != nil { - return err + return nil, err } b.config.tpl, err = packer.NewConfigTemplate() if err != nil { - return err + return nil, err } b.config.tpl.UserVars = b.config.PackerUserVars @@ -48,11 +48,11 @@ func (b *Builder) Prepare(raws ...interface{}) error { errs = packer.MultiErrorAppend(errs, b.config.RunConfig.Prepare(b.config.tpl)...) if errs != nil && len(errs.Errors) > 0 { - return errs + return nil, errs } log.Println(common.ScrubConfig(b.config, b.config.Password)) - return nil + return nil, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/openstack/builder_test.go b/builder/openstack/builder_test.go index cf7ca2927..badf9784d 100644 --- a/builder/openstack/builder_test.go +++ b/builder/openstack/builder_test.go @@ -32,7 +32,10 @@ func TestBuilder_Prepare_BadType(t *testing.T) { "password": []string{}, } - err := b.Prepare(c) + warns, err := b.Prepare(c) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatalf("prepare should fail") } @@ -44,7 +47,10 @@ func TestBuilderPrepare_ImageName(t *testing.T) { // Test good config["image_name"] = "foo" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -52,7 +58,10 @@ func TestBuilderPrepare_ImageName(t *testing.T) { // Test bad config["image_name"] = "foo {{" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -60,7 +69,10 @@ func TestBuilderPrepare_ImageName(t *testing.T) { // Test bad delete(config, "image_name") b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -72,7 +84,10 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) { // Add a random key config["i_should_not_be_valid"] = true - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } diff --git a/builder/virtualbox/builder.go b/builder/virtualbox/builder.go index 6aa8da108..2744375d2 100644 --- a/builder/virtualbox/builder.go +++ b/builder/virtualbox/builder.go @@ -72,15 +72,15 @@ type config struct { tpl *packer.ConfigTemplate } -func (b *Builder) Prepare(raws ...interface{}) error { +func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { md, err := common.DecodeConfig(&b.config, raws...) if err != nil { - return err + return nil, err } b.config.tpl, err = packer.NewConfigTemplate() if err != nil { - return err + return nil, err } b.config.tpl.UserVars = b.config.PackerUserVars @@ -364,10 +364,10 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if errs != nil && len(errs.Errors) > 0 { - return errs + return nil, errs } - return nil + return nil, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/virtualbox/builder_test.go b/builder/virtualbox/builder_test.go index 6a32d875f..f5f263524 100644 --- a/builder/virtualbox/builder_test.go +++ b/builder/virtualbox/builder_test.go @@ -60,7 +60,10 @@ func TestBuilder_ImplementsBuilder(t *testing.T) { func TestBuilderPrepare_Defaults(t *testing.T) { var b Builder config := testConfig() - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -104,7 +107,10 @@ func TestBuilderPrepare_BootWait(t *testing.T) { // Test a default boot_wait delete(config, "boot_wait") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -115,7 +121,10 @@ func TestBuilderPrepare_BootWait(t *testing.T) { // Test with a bad boot_wait config["boot_wait"] = "this is not good" - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -123,7 +132,10 @@ func TestBuilderPrepare_BootWait(t *testing.T) { // Test with a good one config["boot_wait"] = "5s" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -134,7 +146,10 @@ func TestBuilderPrepare_DiskSize(t *testing.T) { config := testConfig() delete(config, "disk_size") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("bad err: %s", err) } @@ -145,7 +160,10 @@ func TestBuilderPrepare_DiskSize(t *testing.T) { config["disk_size"] = 60000 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -160,7 +178,10 @@ func TestBuilderPrepare_FloppyFiles(t *testing.T) { config := testConfig() delete(config, "floppy_files") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("bad err: %s", err) } @@ -171,7 +192,10 @@ func TestBuilderPrepare_FloppyFiles(t *testing.T) { config["floppy_files"] = []string{"foo", "bar"} b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -188,7 +212,10 @@ func TestBuilderPrepare_GuestAdditionsMode(t *testing.T) { // test default mode delete(config, "guest_additions_mode") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("bad err: %s", err) } @@ -196,7 +223,10 @@ func TestBuilderPrepare_GuestAdditionsMode(t *testing.T) { // Test another mode config["guest_additions_mode"] = "attach" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -208,7 +238,10 @@ func TestBuilderPrepare_GuestAdditionsMode(t *testing.T) { // Test bad mode config["guest_additions_mode"] = "teleport" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should error") } @@ -219,7 +252,10 @@ func TestBuilderPrepare_GuestAdditionsPath(t *testing.T) { config := testConfig() delete(config, "guest_additions_path") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("bad err: %s", err) } @@ -230,7 +266,10 @@ func TestBuilderPrepare_GuestAdditionsPath(t *testing.T) { config["guest_additions_path"] = "foo" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -245,7 +284,10 @@ func TestBuilderPrepare_GuestAdditionsSHA256(t *testing.T) { config := testConfig() delete(config, "guest_additions_sha256") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("bad err: %s", err) } @@ -256,7 +298,10 @@ func TestBuilderPrepare_GuestAdditionsSHA256(t *testing.T) { config["guest_additions_sha256"] = "FOO" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -271,7 +316,10 @@ func TestBuilderPrepare_GuestAdditionsURL(t *testing.T) { config := testConfig() config["guest_additions_url"] = "" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -282,7 +330,10 @@ func TestBuilderPrepare_GuestAdditionsURL(t *testing.T) { config["guest_additions_url"] = "http://www.packer.io" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Errorf("should not have error: %s", err) } @@ -294,7 +345,10 @@ func TestBuilderPrepare_HardDriveInterface(t *testing.T) { // Test a default boot_wait delete(config, "hard_drive_interface") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -306,7 +360,10 @@ func TestBuilderPrepare_HardDriveInterface(t *testing.T) { // Test with a bad config["hard_drive_interface"] = "fake" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -314,7 +371,10 @@ func TestBuilderPrepare_HardDriveInterface(t *testing.T) { // Test with a good config["hard_drive_interface"] = "sata" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -327,7 +387,10 @@ func TestBuilderPrepare_HTTPPort(t *testing.T) { // Bad config["http_port_min"] = 1000 config["http_port_max"] = 500 - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -335,7 +398,10 @@ func TestBuilderPrepare_HTTPPort(t *testing.T) { // Bad config["http_port_min"] = -500 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -344,7 +410,10 @@ func TestBuilderPrepare_HTTPPort(t *testing.T) { config["http_port_min"] = 500 config["http_port_max"] = 1000 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -356,7 +425,10 @@ func TestBuilderPrepare_Format(t *testing.T) { // Bad config["format"] = "illegal value" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -364,7 +436,10 @@ func TestBuilderPrepare_Format(t *testing.T) { // Good config["format"] = "ova" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -372,7 +447,10 @@ func TestBuilderPrepare_Format(t *testing.T) { // Good config["format"] = "ovf" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -384,7 +462,10 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) { // Add a random key config["i_should_not_be_valid"] = true - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -396,7 +477,10 @@ func TestBuilderPrepare_ISOChecksum(t *testing.T) { // Test bad config["iso_checksum"] = "" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -404,7 +488,10 @@ func TestBuilderPrepare_ISOChecksum(t *testing.T) { // Test good config["iso_checksum"] = "FOo" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -420,7 +507,10 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { // Test bad config["iso_checksum_type"] = "" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -428,7 +518,10 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { // Test good config["iso_checksum_type"] = "mD5" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -440,7 +533,10 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { // Test unknown config["iso_checksum_type"] = "fake" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -455,7 +551,10 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { // Test both epty config["iso_url"] = "" b = Builder{} - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -463,7 +562,10 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { // Test iso_url set config["iso_url"] = "http://www.packer.io" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Errorf("should not have error: %s", err) } @@ -477,7 +579,10 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { config["iso_url"] = "http://www.packer.io" config["iso_urls"] = []string{"http://www.packer.io"} b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -490,7 +595,10 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { } b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Errorf("should not have error: %s", err) } @@ -517,7 +625,10 @@ func TestBuilderPrepare_OutputDir(t *testing.T) { config["output_directory"] = dir b = Builder{} - err = b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -525,7 +636,10 @@ func TestBuilderPrepare_OutputDir(t *testing.T) { // Test with a good one config["output_directory"] = "i-hope-i-dont-exist" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -537,7 +651,10 @@ func TestBuilderPrepare_ShutdownTimeout(t *testing.T) { // Test with a bad value config["shutdown_timeout"] = "this is not good" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -545,7 +662,10 @@ func TestBuilderPrepare_ShutdownTimeout(t *testing.T) { // Test with a good one config["shutdown_timeout"] = "5s" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -559,7 +679,10 @@ func TestBuilderPrepare_SSHHostPort(t *testing.T) { config["ssh_host_port_min"] = 1000 config["ssh_host_port_max"] = 500 b = Builder{} - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -567,7 +690,10 @@ func TestBuilderPrepare_SSHHostPort(t *testing.T) { // Bad config["ssh_host_port_min"] = -500 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -576,7 +702,10 @@ func TestBuilderPrepare_SSHHostPort(t *testing.T) { config["ssh_host_port_min"] = 500 config["ssh_host_port_max"] = 1000 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -588,14 +717,20 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) { config["ssh_key_path"] = "" b = Builder{} - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } config["ssh_key_path"] = "/i/dont/exist" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -614,7 +749,10 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) { config["ssh_key_path"] = tf.Name() b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -625,7 +763,10 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) { tf.Write([]byte(testPem)) config["ssh_key_path"] = tf.Name() b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -637,14 +778,20 @@ func TestBuilderPrepare_SSHUser(t *testing.T) { config["ssh_username"] = "" b = Builder{} - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } config["ssh_username"] = "exists" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -656,7 +803,10 @@ func TestBuilderPrepare_SSHWaitTimeout(t *testing.T) { // Test a default boot_wait delete(config, "ssh_wait_timeout") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -668,7 +818,10 @@ func TestBuilderPrepare_SSHWaitTimeout(t *testing.T) { // Test with a bad value config["ssh_wait_timeout"] = "this is not good" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -676,7 +829,10 @@ func TestBuilderPrepare_SSHWaitTimeout(t *testing.T) { // Test with a good one config["ssh_wait_timeout"] = "5s" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -688,7 +844,10 @@ func TestBuilderPrepare_VBoxManage(t *testing.T) { // Test with empty delete(config, "vboxmanage") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -703,7 +862,10 @@ func TestBuilderPrepare_VBoxManage(t *testing.T) { } b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -723,7 +885,10 @@ func TestBuilderPrepare_VBoxVersionFile(t *testing.T) { // Test empty delete(config, "virtualbox_version_file") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -735,7 +900,10 @@ func TestBuilderPrepare_VBoxVersionFile(t *testing.T) { // Test with a good one config["virtualbox_version_file"] = "foo" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } diff --git a/builder/vmware/builder.go b/builder/vmware/builder.go index 25657c30d..c5a657b68 100644 --- a/builder/vmware/builder.go +++ b/builder/vmware/builder.go @@ -66,15 +66,15 @@ type config struct { tpl *packer.ConfigTemplate } -func (b *Builder) Prepare(raws ...interface{}) error { +func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { md, err := common.DecodeConfig(&b.config, raws...) if err != nil { - return err + return nil, err } b.config.tpl, err = packer.NewConfigTemplate() if err != nil { - return err + return nil, err } b.config.tpl.UserVars = b.config.PackerUserVars @@ -327,10 +327,10 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if errs != nil && len(errs.Errors) > 0 { - return errs + return nil, errs } - return nil + return nil, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/vmware/builder_test.go b/builder/vmware/builder_test.go index 5d97f36f1..7d627d09d 100644 --- a/builder/vmware/builder_test.go +++ b/builder/vmware/builder_test.go @@ -64,7 +64,10 @@ func TestBuilderPrepare_BootWait(t *testing.T) { // Test a default boot_wait delete(config, "boot_wait") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -75,7 +78,10 @@ func TestBuilderPrepare_BootWait(t *testing.T) { // Test with a bad boot_wait config["boot_wait"] = "this is not good" - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -83,7 +89,10 @@ func TestBuilderPrepare_BootWait(t *testing.T) { // Test with a good one config["boot_wait"] = "5s" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -95,7 +104,10 @@ func TestBuilderPrepare_ISOChecksum(t *testing.T) { // Test bad config["iso_checksum"] = "" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -103,7 +115,10 @@ func TestBuilderPrepare_ISOChecksum(t *testing.T) { // Test good config["iso_checksum"] = "FOo" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -119,7 +134,10 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { // Test bad config["iso_checksum_type"] = "" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -127,7 +145,10 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { // Test good config["iso_checksum_type"] = "mD5" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -139,7 +160,10 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { // Test unknown config["iso_checksum_type"] = "fake" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -147,7 +171,10 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) { func TestBuilderPrepare_Defaults(t *testing.T) { var b Builder config := testConfig() - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -174,7 +201,10 @@ func TestBuilderPrepare_DiskSize(t *testing.T) { config := testConfig() delete(config, "disk_size") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("bad err: %s", err) } @@ -185,7 +215,10 @@ func TestBuilderPrepare_DiskSize(t *testing.T) { config["disk_size"] = 60000 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -200,7 +233,10 @@ func TestBuilderPrepare_FloppyFiles(t *testing.T) { config := testConfig() delete(config, "floppy_files") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("bad err: %s", err) } @@ -211,7 +247,10 @@ func TestBuilderPrepare_FloppyFiles(t *testing.T) { config["floppy_files"] = []string{"foo", "bar"} b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -229,7 +268,10 @@ func TestBuilderPrepare_HTTPPort(t *testing.T) { // Bad config["http_port_min"] = 1000 config["http_port_max"] = 500 - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -237,7 +279,10 @@ func TestBuilderPrepare_HTTPPort(t *testing.T) { // Bad config["http_port_min"] = -500 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -246,7 +291,10 @@ func TestBuilderPrepare_HTTPPort(t *testing.T) { config["http_port_min"] = 500 config["http_port_max"] = 1000 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -258,7 +306,10 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) { // Add a random key config["i_should_not_be_valid"] = true - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -273,7 +324,10 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { // Test both epty config["iso_url"] = "" b = Builder{} - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -281,7 +335,10 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { // Test iso_url set config["iso_url"] = "http://www.packer.io" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Errorf("should not have error: %s", err) } @@ -295,7 +352,10 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { config["iso_url"] = "http://www.packer.io" config["iso_urls"] = []string{"http://www.packer.io"} b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -308,7 +368,10 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) { } b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Errorf("should not have error: %s", err) } @@ -335,7 +398,10 @@ func TestBuilderPrepare_OutputDir(t *testing.T) { config["output_directory"] = dir b = Builder{} - err = b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -343,7 +409,10 @@ func TestBuilderPrepare_OutputDir(t *testing.T) { // Test with a good one config["output_directory"] = "i-hope-i-dont-exist" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -355,7 +424,10 @@ func TestBuilderPrepare_ShutdownTimeout(t *testing.T) { // Test with a bad value config["shutdown_timeout"] = "this is not good" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -363,7 +435,10 @@ func TestBuilderPrepare_ShutdownTimeout(t *testing.T) { // Test with a good one config["shutdown_timeout"] = "5s" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -375,14 +450,20 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) { config["ssh_key_path"] = "" b = Builder{} - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } config["ssh_key_path"] = "/i/dont/exist" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -401,7 +482,10 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) { config["ssh_key_path"] = tf.Name() b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -412,7 +496,10 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) { tf.Write([]byte(testPem)) config["ssh_key_path"] = tf.Name() b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -423,14 +510,20 @@ func TestBuilderPrepare_SSHUser(t *testing.T) { config := testConfig() config["ssh_username"] = "" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } config["ssh_username"] = "exists" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -442,7 +535,10 @@ func TestBuilderPrepare_SSHPort(t *testing.T) { // Test with a bad value delete(config, "ssh_port") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("bad err: %s", err) } @@ -454,7 +550,10 @@ func TestBuilderPrepare_SSHPort(t *testing.T) { // Test with a good one config["ssh_port"] = 44 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -470,7 +569,10 @@ func TestBuilderPrepare_SSHWaitTimeout(t *testing.T) { // Test with a bad value config["ssh_wait_timeout"] = "this is not good" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -478,7 +580,10 @@ func TestBuilderPrepare_SSHWaitTimeout(t *testing.T) { // Test with a good one config["ssh_wait_timeout"] = "5s" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -490,7 +595,10 @@ func TestBuilderPrepare_ToolsUploadPath(t *testing.T) { // Test a default delete(config, "tools_upload_path") - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("err: %s", err) } @@ -502,7 +610,10 @@ func TestBuilderPrepare_ToolsUploadPath(t *testing.T) { // Test with a bad value config["tools_upload_path"] = "{{{nope}" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -510,7 +621,10 @@ func TestBuilderPrepare_ToolsUploadPath(t *testing.T) { // Test with a good one config["tools_upload_path"] = "hey" b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -522,7 +636,10 @@ func TestBuilderPrepare_VMXTemplatePath(t *testing.T) { // Test bad config["vmx_template_path"] = "/i/dont/exist/forreal" - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -541,7 +658,10 @@ func TestBuilderPrepare_VMXTemplatePath(t *testing.T) { config["vmx_template_path"] = tf.Name() b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -560,7 +680,10 @@ func TestBuilderPrepare_VMXTemplatePath(t *testing.T) { config["vmx_template_path"] = tf2.Name() b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -573,7 +696,10 @@ func TestBuilderPrepare_VNCPort(t *testing.T) { // Bad config["vnc_port_min"] = 1000 config["vnc_port_max"] = 500 - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -581,7 +707,10 @@ func TestBuilderPrepare_VNCPort(t *testing.T) { // Bad config["vnc_port_min"] = -500 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err == nil { t.Fatal("should have error") } @@ -590,7 +719,10 @@ func TestBuilderPrepare_VNCPort(t *testing.T) { config["vnc_port_min"] = 500 config["vnc_port_max"] = 1000 b = Builder{} - err = b.Prepare(config) + warns, err = b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } @@ -605,7 +737,10 @@ func TestBuilderPrepare_VMXData(t *testing.T) { "two": "bar", } - err := b.Prepare(config) + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } if err != nil { t.Fatalf("should not have error: %s", err) } From 5d45d9b728f91741895843483ab3b71f9e44dcbb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 23:09:30 -0500 Subject: [PATCH 08/11] command/validate: output warnings --- command/validate/command.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/command/validate/command.go b/command/validate/command.go index c95b026d4..37dd1dd84 100644 --- a/command/validate/command.go +++ b/command/validate/command.go @@ -62,6 +62,7 @@ func (c Command) Run(env packer.Environment, args []string) int { } errs := make([]error, 0) + warnings := make(map[string][]string) // The component finder for our builds components := &packer.ComponentFinder{ @@ -81,7 +82,10 @@ func (c Command) Run(env packer.Environment, args []string) int { // Check the configuration of all builds for _, b := range builds { log.Printf("Preparing build: %s", b.Name()) - err := b.Prepare(userVars) + warns, err := b.Prepare(userVars) + if len(warns) > 0 { + warnings[b.Name()] = warns + } if err != nil { errs = append(errs, fmt.Errorf("Errors validating build '%s'. %s", b.Name(), err)) } @@ -100,6 +104,21 @@ func (c Command) Run(env packer.Environment, args []string) int { return 1 } + if len(warnings) > 0 { + env.Ui().Say("Template validation succeeded, but there were some warnings.") + env.Ui().Say("These are ONLY WARNINGS, and Packer will attempt to build the") + env.Ui().Say("template despite them, but they should be paid attention to.\n") + + for build, warns := range warnings { + env.Ui().Say(fmt.Sprintf("Warnings for build '%s':\n", build)) + for _, warning := range warns { + env.Ui().Say(fmt.Sprintf("* %s", warning)) + } + } + + return 0 + } + env.Ui().Say("Template validated successfully.") return 0 } From 87e88dc8471d42b799a3111ecbf3bcbe7bf7ef5d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 23:09:58 -0500 Subject: [PATCH 09/11] command/build: get command passing --- command/build/command.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/command/build/command.go b/command/build/command.go index b0e387c04..1c0f7b2f1 100644 --- a/command/build/command.go +++ b/command/build/command.go @@ -113,7 +113,8 @@ func (c Command) Run(env packer.Environment, args []string) int { log.Printf("Preparing build: %s", b.Name()) b.SetDebug(cfgDebug) b.SetForce(cfgForce) - err := b.Prepare(userVars) + // TODO(mitchellh): Handle warnings + _, err := b.Prepare(userVars) if err != nil { env.Ui().Error(err.Error()) return 1 From 9acaa97a32cbf628a450b81aa52343b913ce7f42 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 23:17:21 -0500 Subject: [PATCH 10/11] builder/virtualbox,vmware: warning if shutdown_command is not specified --- builder/virtualbox/builder.go | 16 ++++++++++++---- builder/virtualbox/builder_test.go | 16 ++++++++++++++++ builder/vmware/builder.go | 14 +++++++++++--- builder/vmware/builder_test.go | 16 ++++++++++++++++ 4 files changed, 55 insertions(+), 7 deletions(-) diff --git a/builder/virtualbox/builder.go b/builder/virtualbox/builder.go index 2744375d2..5df6c30eb 100644 --- a/builder/virtualbox/builder.go +++ b/builder/virtualbox/builder.go @@ -84,8 +84,9 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { } b.config.tpl.UserVars = b.config.PackerUserVars - // Accumulate any errors + // Accumulate any errors and warnings errs := common.CheckUnusedConfig(md) + warnings := make([]string, 0) if b.config.DiskSize == 0 { b.config.DiskSize = 40000 @@ -363,11 +364,18 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { } } - if errs != nil && len(errs.Errors) > 0 { - return nil, errs + // Warnings + if b.config.ShutdownCommand == "" { + warnings = append(warnings, + "A shutdown_command was not specified. Without a shutdown command, Packer\n"+ + "will forcibly halt the virtual machine, which may result in data loss.") } - return nil, nil + if errs != nil && len(errs.Errors) > 0 { + return warnings, errs + } + + return warnings, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/virtualbox/builder_test.go b/builder/virtualbox/builder_test.go index f5f263524..942bc059f 100644 --- a/builder/virtualbox/builder_test.go +++ b/builder/virtualbox/builder_test.go @@ -43,6 +43,7 @@ func testConfig() map[string]interface{} { "iso_checksum": "foo", "iso_checksum_type": "md5", "iso_url": "http://www.google.com/", + "shutdown_command": "yes", "ssh_username": "foo", packer.BuildNameConfigKey: "foo", @@ -645,6 +646,21 @@ func TestBuilderPrepare_OutputDir(t *testing.T) { } } +func TestBuilderPrepare_ShutdownCommand(t *testing.T) { + var b Builder + config := testConfig() + delete(config, "shutdown_command") + + warns, err := b.Prepare(config) + if err != nil { + t.Fatalf("bad: %s", err) + } + + if len(warns) != 1 { + t.Fatalf("bad: %#v", warns) + } +} + func TestBuilderPrepare_ShutdownTimeout(t *testing.T) { var b Builder config := testConfig() diff --git a/builder/vmware/builder.go b/builder/vmware/builder.go index c5a657b68..f75014459 100644 --- a/builder/vmware/builder.go +++ b/builder/vmware/builder.go @@ -80,6 +80,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { // Accumulate any errors errs := common.CheckUnusedConfig(md) + warnings := make([]string, 0) if b.config.DiskName == "" { b.config.DiskName = "disk" @@ -326,11 +327,18 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs, fmt.Errorf("vnc_port_min must be less than vnc_port_max")) } - if errs != nil && len(errs.Errors) > 0 { - return nil, errs + // Warnings + if b.config.ShutdownCommand == "" { + warnings = append(warnings, + "A shutdown_command was not specified. Without a shutdown command, Packer\n"+ + "will forcibly halt the virtual machine, which may result in data loss.") } - return nil, nil + if errs != nil && len(errs.Errors) > 0 { + return warnings, errs + } + + return warnings, nil } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { diff --git a/builder/vmware/builder_test.go b/builder/vmware/builder_test.go index 7d627d09d..893970768 100644 --- a/builder/vmware/builder_test.go +++ b/builder/vmware/builder_test.go @@ -44,6 +44,7 @@ func testConfig() map[string]interface{} { "iso_checksum": "foo", "iso_checksum_type": "md5", "iso_url": "http://www.packer.io", + "shutdown_command": "foo", "ssh_username": "foo", packer.BuildNameConfigKey: "foo", @@ -418,6 +419,21 @@ func TestBuilderPrepare_OutputDir(t *testing.T) { } } +func TestBuilderPrepare_ShutdownCommand(t *testing.T) { + var b Builder + config := testConfig() + delete(config, "shutdown_command") + + warns, err := b.Prepare(config) + if err != nil { + t.Fatalf("bad: %s", err) + } + + if len(warns) != 1 { + t.Fatalf("bad: %#v", warns) + } +} + func TestBuilderPrepare_ShutdownTimeout(t *testing.T) { var b Builder config := testConfig() From 05e61e1a07d66bfcffff451b63d926a71339a932 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Nov 2013 23:20:27 -0500 Subject: [PATCH 11/11] command/build: output warnings --- command/build/command.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/command/build/command.go b/command/build/command.go index 1c0f7b2f1..4a1e7c283 100644 --- a/command/build/command.go +++ b/command/build/command.go @@ -113,12 +113,20 @@ func (c Command) Run(env packer.Environment, args []string) int { log.Printf("Preparing build: %s", b.Name()) b.SetDebug(cfgDebug) b.SetForce(cfgForce) - // TODO(mitchellh): Handle warnings - _, err := b.Prepare(userVars) + + warnings, err := b.Prepare(userVars) if err != nil { env.Ui().Error(err.Error()) return 1 } + if len(warnings) > 0 { + ui := buildUis[b.Name()] + ui.Say(fmt.Sprintf("Warnings for build '%s':\n", b.Name())) + for _, warning := range warnings { + ui.Say(fmt.Sprintf("* %s", warning)) + } + ui.Say("") + } } // Run all the builds in parallel and wait for them to complete