From dabe7ac5846b1ce44cf6249a8aaf5ce2903679bb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 18 Jun 2013 22:45:53 -0700 Subject: [PATCH] packer: keep_input_artifact will keep prior artifact in a PP [GH-19] --- packer/artifact_test.go | 17 +++- packer/build.go | 58 +++++++++++--- packer/build_test.go | 142 +++++++++++++++++++++++++++++----- packer/builder_test.go | 4 +- packer/post_processor_test.go | 3 +- packer/template.go | 10 ++- packer/template_test.go | 7 +- 7 files changed, 201 insertions(+), 40 deletions(-) diff --git a/packer/artifact_test.go b/packer/artifact_test.go index 9c16db6c1..ab6f39fb4 100644 --- a/packer/artifact_test.go +++ b/packer/artifact_test.go @@ -1,6 +1,9 @@ package packer -type TestArtifact struct{} +type TestArtifact struct { + id string + destroyCalled bool +} func (*TestArtifact) BuilderId() string { return "bid" @@ -10,14 +13,20 @@ func (*TestArtifact) Files() []string { return []string{"a", "b"} } -func (*TestArtifact) Id() string { - return "id" +func (a *TestArtifact) Id() string { + id := a.id + if id == "" { + id = "id" + } + + return id } func (*TestArtifact) String() string { return "string" } -func (*TestArtifact) Destroy() error { +func (a *TestArtifact) Destroy() error { + a.destroyCalled = true return nil } diff --git a/packer/build.go b/packer/build.go index 448e20658..807b1e362 100644 --- a/packer/build.go +++ b/packer/build.go @@ -60,8 +60,9 @@ type coreBuild struct { // Keeps track of the post-processor and the configuration of the // post-processor used within a build. type coreBuildPostProcessor struct { - processor PostProcessor - config interface{} + processor PostProcessor + config interface{} + keepInputArtifact bool } // Keeps track of the provisioner and the configuration of the provisioner @@ -153,21 +154,44 @@ func (b *coreBuild) Run(ui Ui, cache Cache) ([]Artifact, error) { artifacts := make([]Artifact, 0, 1) builderArtifact, err := b.builder.Run(ui, hook, cache) - if builderArtifact != nil { - artifacts = append(artifacts, builderArtifact) - } - if err != nil { return artifacts, err } errors := make([]error, 0) + keepOriginalArtifact := len(b.postProcessors) == 0 // Run the post-processors PostProcessorRunSeqLoop: for _, ppSeq := range b.postProcessors { artifact := builderArtifact - for _, corePP := range ppSeq { + + var priorArtifact Artifact + for i, corePP := range ppSeq { + if i == 0 { + // This is the first post-processor. We handle deleting + // previous artifacts a bit different because multiple + // post-processors may be using the original and need it. + if !keepOriginalArtifact { + keepOriginalArtifact = corePP.keepInputArtifact + } + } else { + if priorArtifact == nil { + errors = append(errors, fmt.Errorf("Post-processor returned nil artifact mid-chain.")) + continue PostProcessorRunSeqLoop + } + + // We have a prior artifact. If we want to keep it, we append + // it to the results list. Otherwise, we destroy it. + if corePP.keepInputArtifact { + artifacts = append(artifacts, priorArtifact) + } else { + if err := priorArtifact.Destroy(); err != nil { + errors = append(errors, fmt.Errorf("Failed cleaning up prior artifact: %s", err)) + } + } + } + var err error artifact, err = corePP.processor.PostProcess(ui, artifact) if err != nil { @@ -175,9 +199,23 @@ PostProcessorRunSeqLoop: continue PostProcessorRunSeqLoop } - if artifact != nil { - artifacts = append(artifacts, artifact) - } + priorArtifact = artifact + } + + // Add on the last artifact to the results + if priorArtifact != nil { + artifacts = append(artifacts, priorArtifact) + } + } + + if keepOriginalArtifact { + artifacts = append(artifacts, nil) + copy(artifacts[1:], artifacts) + artifacts[0] = builderArtifact + } else { + log.Printf("Deleting original artifact for build '%s'", b.name) + if err := builderArtifact.Destroy(); err != nil { + errors = append(errors, fmt.Errorf("Error destroying builder artifact: %s", err)) } } diff --git a/packer/build_test.go b/packer/build_test.go index 4362359b7..fc6bea083 100644 --- a/packer/build_test.go +++ b/packer/build_test.go @@ -2,13 +2,14 @@ package packer import ( "cgl.tideland.biz/asserts" + "reflect" "testing" ) -func testBuild() Build { +func testBuild() *coreBuild { return &coreBuild{ name: "test", - builder: &TestBuilder{}, + builder: &TestBuilder{artifactId: "b"}, builderConfig: 42, hooks: map[string][]Hook{ "foo": []Hook{&TestHook{}}, @@ -18,7 +19,7 @@ func testBuild() Build { }, postProcessors: [][]coreBuildPostProcessor{ []coreBuildPostProcessor{ - coreBuildPostProcessor{&TestPostProcessor{}, 42}, + coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp"}, 42, true}, }, }, } @@ -41,19 +42,18 @@ func TestBuild_Prepare(t *testing.T) { debugFalseConfig := map[string]interface{}{DebugConfigKey: false} build := testBuild() - coreB := build.(*coreBuild) - builder := coreB.builder.(*TestBuilder) + builder := build.builder.(*TestBuilder) build.Prepare() assert.True(builder.prepareCalled, "prepare should be called") assert.Equal(builder.prepareConfig, []interface{}{42, debugFalseConfig}, "prepare config should be 42") - coreProv := coreB.provisioners[0] + coreProv := build.provisioners[0] prov := coreProv.provisioner.(*TestProvisioner) assert.True(prov.prepCalled, "prepare should be called") assert.Equal(prov.prepConfigs, []interface{}{42, debugFalseConfig}, "prepare should be called with proper config") - corePP := coreB.postProcessors[0][0] + corePP := build.postProcessors[0][0] pp := corePP.processor.(*TestPostProcessor) assert.True(pp.configCalled, "config should be called") assert.Equal(pp.configVal, 42, "config should have right value") @@ -85,15 +85,14 @@ func TestBuild_Prepare_Debug(t *testing.T) { debugConfig := map[string]interface{}{DebugConfigKey: true} build := testBuild() - coreB := build.(*coreBuild) - builder := coreB.builder.(*TestBuilder) + builder := build.builder.(*TestBuilder) build.SetDebug(true) build.Prepare() assert.True(builder.prepareCalled, "prepare should be called") assert.Equal(builder.prepareConfig, []interface{}{42, debugConfig}, "prepare config should be 42") - coreProv := coreB.provisioners[0] + coreProv := build.provisioners[0] prov := coreProv.provisioner.(*TestProvisioner) assert.True(prov.prepCalled, "prepare should be called") assert.Equal(prov.prepConfigs, []interface{}{42, debugConfig}, "prepare should be called with proper config") @@ -111,10 +110,8 @@ func TestBuild_Run(t *testing.T) { assert.Nil(err, "should not error") assert.Equal(len(artifacts), 2, "should have two artifacts") - coreB := build.(*coreBuild) - // Verify builder was run - builder := coreB.builder.(*TestBuilder) + builder := build.builder.(*TestBuilder) assert.True(builder.runCalled, "run should be called") assert.Equal(builder.runUi, ui, "run should be called with ui") @@ -122,20 +119,129 @@ func TestBuild_Run(t *testing.T) { dispatchHook := builder.runHook dispatchHook.Run("foo", nil, nil, 42) - hook := coreB.hooks["foo"][0].(*TestHook) + hook := build.hooks["foo"][0].(*TestHook) assert.True(hook.runCalled, "run should be called") assert.Equal(hook.runData, 42, "should have correct data") // Verify provisioners run dispatchHook.Run(HookProvision, nil, nil, 42) - prov := coreB.provisioners[0].provisioner.(*TestProvisioner) + prov := build.provisioners[0].provisioner.(*TestProvisioner) assert.True(prov.provCalled, "provision should be called") // Verify post-processor was run - pp := coreB.postProcessors[0][0].processor.(*TestPostProcessor) + pp := build.postProcessors[0][0].processor.(*TestPostProcessor) assert.True(pp.ppCalled, "post processor should be called") } +func TestBuild_Run_Artifacts(t *testing.T) { + cache := &TestCache{} + ui := testUi() + + // Test case: Test that with no post-processors, we only get the + // main build. + build := testBuild() + build.postProcessors = [][]coreBuildPostProcessor{} + + build.Prepare() + artifacts, err := build.Run(ui, cache) + if err != nil { + t.Fatalf("err: %s", err) + } + + expectedIds := []string{"b"} + artifactIds := make([]string, len(artifacts)) + for i, artifact := range artifacts { + artifactIds[i] = artifact.Id() + } + + if !reflect.DeepEqual(artifactIds, expectedIds) { + t.Fatalf("unexpected ids: %#v", artifactIds) + } + + // Test case: Test that with a single post-processor that doesn't keep + // inputs, only that post-processors results are returned. + build = testBuild() + build.postProcessors = [][]coreBuildPostProcessor{ + []coreBuildPostProcessor{ + coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp"}, 42, false}, + }, + } + + build.Prepare() + artifacts, err = build.Run(ui, cache) + if err != nil { + t.Fatalf("err: %s", err) + } + + expectedIds = []string{"pp"} + artifactIds = make([]string, len(artifacts)) + for i, artifact := range artifacts { + artifactIds[i] = artifact.Id() + } + + if !reflect.DeepEqual(artifactIds, expectedIds) { + t.Fatalf("unexpected ids: %#v", artifactIds) + } + + // Test case: Test that with multiple post-processors, as long as one + // keeps the original, the original is kept. + build = testBuild() + build.postProcessors = [][]coreBuildPostProcessor{ + []coreBuildPostProcessor{ + coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp1"}, 42, false}, + }, + []coreBuildPostProcessor{ + coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp2"}, 42, true}, + }, + } + + build.Prepare() + artifacts, err = build.Run(ui, cache) + if err != nil { + t.Fatalf("err: %s", err) + } + + expectedIds = []string{"b", "pp1", "pp2"} + artifactIds = make([]string, len(artifacts)) + for i, artifact := range artifacts { + artifactIds[i] = artifact.Id() + } + + if !reflect.DeepEqual(artifactIds, expectedIds) { + t.Fatalf("unexpected ids: %#v", artifactIds) + } + + // Test case: Test that with sequences, intermediaries are kept if they + // want to be. + build = testBuild() + build.postProcessors = [][]coreBuildPostProcessor{ + []coreBuildPostProcessor{ + coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp1a"}, 42, false}, + coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp1b"}, 42, true}, + }, + []coreBuildPostProcessor{ + coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp2a"}, 42, false}, + coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp2b"}, 42, false}, + }, + } + + build.Prepare() + artifacts, err = build.Run(ui, cache) + if err != nil { + t.Fatalf("err: %s", err) + } + + expectedIds = []string{"pp1a", "pp1b", "pp2b"} + artifactIds = make([]string, len(artifacts)) + for i, artifact := range artifacts { + artifactIds[i] = artifact.Id() + } + + if !reflect.DeepEqual(artifactIds, expectedIds) { + t.Fatalf("unexpected ids: %#v", artifactIds) + } +} + func TestBuild_RunBeforePrepare(t *testing.T) { assert := asserts.NewTestingAsserts(t, true) @@ -154,8 +260,6 @@ func TestBuild_Cancel(t *testing.T) { build := testBuild() build.Cancel() - coreB := build.(*coreBuild) - - builder := coreB.builder.(*TestBuilder) + builder := build.builder.(*TestBuilder) assert.True(builder.cancelCalled, "cancel should be called") } diff --git a/packer/builder_test.go b/packer/builder_test.go index 4ddb8d691..09e1f1825 100644 --- a/packer/builder_test.go +++ b/packer/builder_test.go @@ -1,6 +1,8 @@ package packer type TestBuilder struct { + artifactId string + prepareCalled bool prepareConfig []interface{} runCalled bool @@ -21,7 +23,7 @@ func (tb *TestBuilder) Run(ui Ui, h Hook, c Cache) (Artifact, error) { tb.runHook = h tb.runUi = ui tb.runCache = c - return new(TestArtifact), nil + return &TestArtifact{id: tb.artifactId}, nil } func (tb *TestBuilder) Cancel() { diff --git a/packer/post_processor_test.go b/packer/post_processor_test.go index 9a483f0a2..476bbe762 100644 --- a/packer/post_processor_test.go +++ b/packer/post_processor_test.go @@ -1,6 +1,7 @@ package packer type TestPostProcessor struct { + artifactId string configCalled bool configVal interface{} ppCalled bool @@ -18,5 +19,5 @@ func (pp *TestPostProcessor) PostProcess(ui Ui, a Artifact) (Artifact, error) { pp.ppCalled = true pp.ppArtifact = a pp.ppUi = ui - return new(TestArtifact), nil + return &TestArtifact{id: pp.artifactId}, nil } diff --git a/packer/template.go b/packer/template.go index f2a743e6c..de10b437d 100644 --- a/packer/template.go +++ b/packer/template.go @@ -41,8 +41,9 @@ type rawBuilderConfig struct { // configuration. It contains the type of the post processor as well as the // raw configuration that is handed to the post-processor for it to process. type rawPostProcessorConfig struct { - Type string - rawConfig interface{} + Type string + KeepInputArtifact bool `mapstructure:"keep_input_artifact"` + rawConfig interface{} } // rawProvisionerConfig represents a raw, unprocessed provisioner configuration. @@ -300,8 +301,9 @@ func (t *Template) Build(name string, components *ComponentFinder) (b Build, err } current[i] = coreBuildPostProcessor{ - processor: pp, - config: rawPP.rawConfig, + processor: pp, + config: rawPP.rawConfig, + keepInputArtifact: rawPP.KeepInputArtifact, } } diff --git a/packer/template_test.go b/packer/template_test.go index ac89cb901..96b0b760a 100644 --- a/packer/template_test.go +++ b/packer/template_test.go @@ -447,7 +447,10 @@ func TestTemplate_Build(t *testing.T) { "post-processors": [ "simple", - ["simple", "simple"] + [ + "simple", + { "type": "simple", "keep_input_artifact": true } + ] ] } ` @@ -497,6 +500,8 @@ func TestTemplate_Build(t *testing.T) { assert.Equal(len(coreBuild.postProcessors), 2, "should have pps") assert.Equal(len(coreBuild.postProcessors[0]), 1, "should have correct number") assert.Equal(len(coreBuild.postProcessors[1]), 2, "should have correct number") + assert.False(coreBuild.postProcessors[1][0].keepInputArtifact, "shoule be correct") + assert.True(coreBuild.postProcessors[1][1].keepInputArtifact, "shoule be correct") } func TestTemplate_Build_ProvisionerOverride(t *testing.T) {