From 905db043c4b66c8b7c9da71f4a4a19fa6d737435 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 10 Jan 2019 11:37:41 +0100 Subject: [PATCH 1/5] command.TestBuildOnlyFileCommaFlags: create some files using post processors --- command/build_test.go | 21 ++++++++++++------- .../test-fixtures/build-only/template.json | 10 +++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/command/build_test.go b/command/build_test.go index f5ec395f8..e0a22b268 100644 --- a/command/build_test.go +++ b/command/build_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/packer/builder/file" "github.com/hashicorp/packer/packer" + shell_local "github.com/hashicorp/packer/post-processor/shell-local" ) func TestBuildOnlyFileCommaFlags(t *testing.T) { @@ -111,14 +112,15 @@ func TestBuildExceptFileCommaFlags(t *testing.T) { fatalCommand(t, c.Meta) } - if fileExists("chocolate.txt") { - t.Error("Expected NOT to find chocolate.txt") + for _, f := range []string{"chocolate.txt"} { + if fileExists(f) { + t.Errorf("Expected NOT to find %s", f) + } } - if !fileExists("vanilla.txt") { - t.Error("Expected to find vanilla.txt") - } - if !fileExists("cherry.txt") { - t.Error("Expected to find cherry.txt") + for _, f := range []string{"vanilla.txt", "cherry.txt", "apple.txt", "peach.txt"} { + if !fileExists(f) { + t.Errorf("Expected to find %s", f) + } } } @@ -137,6 +139,9 @@ func testCoreConfigBuilder(t *testing.T) *packer.CoreConfig { Builder: func(n string) (packer.Builder, error) { return &file.Builder{}, nil }, + PostProcessor: func(n string) (packer.PostProcessor, error) { + return &shell_local.PostProcessor{}, nil + }, } return &packer.CoreConfig{ Components: components, @@ -159,4 +164,6 @@ func cleanup() { os.RemoveAll("chocolate.txt") os.RemoveAll("vanilla.txt") os.RemoveAll("cherry.txt") + os.RemoveAll("apple.txt") + os.RemoveAll("peach.txt") } diff --git a/command/test-fixtures/build-only/template.json b/command/test-fixtures/build-only/template.json index ee89d635e..e5b0d6d09 100644 --- a/command/test-fixtures/build-only/template.json +++ b/command/test-fixtures/build-only/template.json @@ -18,5 +18,15 @@ "content":"cherry", "target":"cherry.txt" } + ], + "post-processors": [ + { + "type": "shell-local", + "inline": ["touch apple.txt"] + }, + { + "type": "shell-local", + "inline": ["touch peach.txt"] + } ] } From 4bf3cd44fc7e1f35529360d8b3cb13a914f6021f Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 10 Jan 2019 15:27:02 +0100 Subject: [PATCH 2/5] allow to skip a post processor --- command/build_test.go | 6 ++--- command/meta.go | 22 +++++++++---------- .../test-fixtures/build-only/template.json | 2 ++ packer/core.go | 21 ++++++++++++++---- template/parse.go | 3 ++- template/template.go | 3 ++- 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/command/build_test.go b/command/build_test.go index e0a22b268..7cdf4a9a4 100644 --- a/command/build_test.go +++ b/command/build_test.go @@ -102,7 +102,7 @@ func TestBuildExceptFileCommaFlags(t *testing.T) { } args := []string{ - "-except=chocolate", + "-except=chocolate,apple", filepath.Join(testFixture("build-only"), "template.json"), } @@ -112,12 +112,12 @@ func TestBuildExceptFileCommaFlags(t *testing.T) { fatalCommand(t, c.Meta) } - for _, f := range []string{"chocolate.txt"} { + for _, f := range []string{"chocolate.txt", "apple.txt"} { if fileExists(f) { t.Errorf("Expected NOT to find %s", f) } } - for _, f := range []string{"vanilla.txt", "cherry.txt", "apple.txt", "peach.txt"} { + for _, f := range []string{"vanilla.txt", "cherry.txt", "peach.txt"} { if !fileExists(f) { t.Errorf("Expected to find %s", f) } diff --git a/command/meta.go b/command/meta.go index b8dfdfbf5..5407c802e 100644 --- a/command/meta.go +++ b/command/meta.go @@ -6,8 +6,8 @@ import ( "fmt" "io" - "github.com/hashicorp/packer/helper/flag-kv" - "github.com/hashicorp/packer/helper/flag-slice" + kvflag "github.com/hashicorp/packer/helper/flag-kv" + sliceflag "github.com/hashicorp/packer/helper/flag-slice" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template" ) @@ -31,9 +31,7 @@ type Meta struct { Version string // These are set by command-line flags - flagBuildExcept []string - flagBuildOnly []string - flagVars map[string]string + flagVars map[string]string } // Core returns the core for the given template given the configured @@ -59,7 +57,7 @@ func (m *Meta) BuildNames(c *packer.Core) []string { // TODO: test // Filter the "only" - if len(m.flagBuildOnly) > 0 { + if len(m.CoreConfig.Only) > 0 { // Build a set of all the available names nameSet := make(map[string]struct{}) for _, n := range c.BuildNames() { @@ -67,8 +65,8 @@ func (m *Meta) BuildNames(c *packer.Core) []string { } // Build our result set which we pre-allocate some sane number - result := make([]string, 0, len(m.flagBuildOnly)) - for _, n := range m.flagBuildOnly { + result := make([]string, 0, len(m.CoreConfig.Only)) + for _, n := range m.CoreConfig.Only { if _, ok := nameSet[n]; ok { result = append(result, n) } @@ -78,10 +76,10 @@ func (m *Meta) BuildNames(c *packer.Core) []string { } // Filter the "except" - if len(m.flagBuildExcept) > 0 { + if len(m.CoreConfig.Except) > 0 { // Build a set of the things we don't want nameSet := make(map[string]struct{}) - for _, n := range m.flagBuildExcept { + for _, n := range m.CoreConfig.Except { nameSet[n] = struct{}{} } @@ -111,8 +109,8 @@ func (m *Meta) FlagSet(n string, fs FlagSetFlags) *flag.FlagSet { // FlagSetBuildFilter tells us to enable the settings for selecting // builds we care about. if fs&FlagSetBuildFilter != 0 { - f.Var((*sliceflag.StringFlag)(&m.flagBuildExcept), "except", "") - f.Var((*sliceflag.StringFlag)(&m.flagBuildOnly), "only", "") + f.Var((*sliceflag.StringFlag)(&m.CoreConfig.Except), "except", "") + f.Var((*sliceflag.StringFlag)(&m.CoreConfig.Only), "only", "") } // FlagSetVars tells us what variables to use diff --git a/command/test-fixtures/build-only/template.json b/command/test-fixtures/build-only/template.json index e5b0d6d09..036990227 100644 --- a/command/test-fixtures/build-only/template.json +++ b/command/test-fixtures/build-only/template.json @@ -21,10 +21,12 @@ ], "post-processors": [ { + "name": "apple", "type": "shell-local", "inline": ["touch apple.txt"] }, { + "name": "peach", "type": "shell-local", "inline": ["touch peach.txt"] } diff --git a/packer/core.go b/packer/core.go index 86994ca29..c81745148 100644 --- a/packer/core.go +++ b/packer/core.go @@ -4,8 +4,8 @@ import ( "fmt" "sort" - "github.com/hashicorp/go-multierror" - "github.com/hashicorp/go-version" + multierror "github.com/hashicorp/go-multierror" + version "github.com/hashicorp/go-version" "github.com/hashicorp/packer/template" "github.com/hashicorp/packer/template/interpolate" ) @@ -20,6 +20,9 @@ type Core struct { builds map[string]*template.Builder version string secrets []string + + except []string + only []string } // CoreConfig is the structure for initializing a new Core. Once a CoreConfig @@ -30,6 +33,10 @@ type CoreConfig struct { Variables map[string]string SensitiveVariables []string Version string + + // These are set by command-line flags + Except []string + Only []string } // The function type used to lookup Builder implementations. @@ -61,6 +68,8 @@ func NewCore(c *CoreConfig) (*Core, error) { components: c.Components, variables: c.Variables, version: c.Version, + only: c.Only, + except: c.Except, } if err := result.validate(); err != nil { @@ -126,7 +135,7 @@ func (c *Core) Build(n string) (Build, error) { provisioners := make([]coreBuildProvisioner, 0, len(c.Template.Provisioners)) for _, rawP := range c.Template.Provisioners { // If we're skipping this, then ignore it - if rawP.Skip(rawName) { + if rawP.OnlyExcept.Skip(rawName) { continue } @@ -172,7 +181,11 @@ func (c *Core) Build(n string) (Build, error) { current := make([]coreBuildPostProcessor, 0, len(rawPs)) for _, rawP := range rawPs { // If we skip, ignore - if rawP.Skip(rawName) { + rawP.OnlyExcept.Except = append(rawP.OnlyExcept.Except, c.except...) + if rawP.OnlyExcept.Skip(rawName) { + continue + } + if rawP.OnlyExcept.Skip(rawP.Name) { continue } diff --git a/template/parse.go b/template/parse.go index a2d337300..9f37929a3 100644 --- a/template/parse.go +++ b/template/parse.go @@ -11,7 +11,7 @@ import ( "sort" "strings" - "github.com/hashicorp/go-multierror" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/packer/packer/tmp" "github.com/mitchellh/mapstructure" ) @@ -149,6 +149,7 @@ func (r *rawTemplate) Template() (*Template, error) { delete(c, "only") delete(c, "keep_input_artifact") delete(c, "type") + delete(c, "name") if len(c) > 0 { pp.Config = c } diff --git a/template/template.go b/template/template.go index 73bc094e4..3a0a372aa 100644 --- a/template/template.go +++ b/template/template.go @@ -5,7 +5,7 @@ import ( "fmt" "time" - "github.com/hashicorp/go-multierror" + multierror "github.com/hashicorp/go-multierror" ) // Template represents the parsed template that is used to configure @@ -40,6 +40,7 @@ type Builder struct { type PostProcessor struct { OnlyExcept `mapstructure:",squash"` + Name string Type string KeepInputArtifact bool `mapstructure:"keep_input_artifact"` Config map[string]interface{} From 61ade0e127e4b030c5706fa98f23bc50884faaf8 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 10 Jan 2019 15:32:54 +0100 Subject: [PATCH 3/5] allow to use --only with post-processors --- command/build_test.go | 4 ++++ packer/core.go | 1 + 2 files changed, 5 insertions(+) diff --git a/command/build_test.go b/command/build_test.go index 7cdf4a9a4..e334f0055 100644 --- a/command/build_test.go +++ b/command/build_test.go @@ -76,6 +76,7 @@ func TestBuildOnlyFileMultipleFlags(t *testing.T) { args := []string{ "-only=chocolate", "-only=cherry", + "-only=apple", filepath.Join(testFixture("build-only"), "template.json"), } @@ -94,6 +95,9 @@ func TestBuildOnlyFileMultipleFlags(t *testing.T) { if !fileExists("cherry.txt") { t.Error("Expected to find cherry.txt") } + if !fileExists("apple.txt") { + t.Error("Expected to find apple.txt") + } } func TestBuildExceptFileCommaFlags(t *testing.T) { diff --git a/packer/core.go b/packer/core.go index c81745148..5c68b348b 100644 --- a/packer/core.go +++ b/packer/core.go @@ -182,6 +182,7 @@ func (c *Core) Build(n string) (Build, error) { for _, rawP := range rawPs { // If we skip, ignore rawP.OnlyExcept.Except = append(rawP.OnlyExcept.Except, c.except...) + rawP.OnlyExcept.Only = append(rawP.OnlyExcept.Only, c.only...) if rawP.OnlyExcept.Skip(rawName) { continue } From 75af18661f918a81e1d95ed3e5d365c5e9f7c369 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 11 Jan 2019 11:23:42 +0100 Subject: [PATCH 4/5] Document --except & --only flags for post-processors --- contrib/zsh-completion/_packer | 2 +- website/source/docs/commands/build.html.md | 15 ++++++++------- .../source/docs/templates/provisioners.html.md | 8 ++++---- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/contrib/zsh-completion/_packer b/contrib/zsh-completion/_packer index 38df34236..f0ae47fd9 100644 --- a/contrib/zsh-completion/_packer +++ b/contrib/zsh-completion/_packer @@ -14,7 +14,7 @@ _packer () { '-force[Force a build to continue if artifacts exist, deletes existing artifacts.]' '-machine-readable[Produce machine-readable output.]' '-color=[(false) Disable color output. (Default: color)]' - '-except=[(foo,bar,baz) Build all builds other than these.]' + '-except=[(foo,bar,baz) Run all builds and post-procesors other than these.]' '-on-error=[(cleanup,abort,ask) If the build fails do: clean up (default), abort, or ask.]' '-only=[(foo,bar,baz) Only build the given builds by name.]' '-parallel=[(false) Disable parallelization. (Default: parallel)]' diff --git a/website/source/docs/commands/build.html.md b/website/source/docs/commands/build.html.md index 4788b1c12..131b22587 100644 --- a/website/source/docs/commands/build.html.md +++ b/website/source/docs/commands/build.html.md @@ -26,10 +26,10 @@ artifacts that are created will be outputted at the end of the build. will stop between each step, waiting for keyboard input before continuing. This will allow the user to inspect state and so on. -- `-except=foo,bar,baz` - Builds all the builds except those with the given - comma-separated names. Build names by default are the names of their - builders, unless a specific `name` attribute is specified within the - configuration. +- `-except=foo,bar,baz` - Run all the builds and post-processors except + those with the given comma-separated names. Build and post-processor names + by default are their type, unless a specific `name` attribute is specified + within the configuration. - `-force` - Forces a builder to run when artifacts from a previous build prevent a build from running. The exact behavior of a forced build is left @@ -44,9 +44,10 @@ artifacts that are created will be outputted at the end of the build. presents a prompt and waits for you to decide to clean up, abort, or retry the failed step. -- `-only=foo,bar,baz` - Only build the builds with the given comma-separated - names. Build names by default are the names of their builders, unless a - specific `name` attribute is specified within the configuration. +- `-only=foo,bar,baz` - Only run the builds and post-processors with the + given comma-separated names. Build and post-processor names by default are + their type, unless a specific `name` attribute is specified within the + configuration. - `-parallel=false` - Disable parallelization of multiple builders (on by default). diff --git a/website/source/docs/templates/provisioners.html.md b/website/source/docs/templates/provisioners.html.md index 1a13948c9..d4cd5e555 100644 --- a/website/source/docs/templates/provisioners.html.md +++ b/website/source/docs/templates/provisioners.html.md @@ -77,10 +77,10 @@ effectively the same: } ``` -The values within `only` or `except` are *build names*, not builder types. If -you recall, build names by default are just their builder type, but if you -specify a custom `name` parameter, then you should use that as the value -instead of the type. +The values within `only` or `except` can be *build or post-processor names*, +not builder types. If you recall, build and post-processor names by default are +just their builder type, but if you specify a custom `name` parameter, then you +should use that as the value instead of the type. ## Build-Specific Overrides From 58245f2557380880326096bef1c704a2f368cf74 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 11 Jan 2019 14:06:34 +0100 Subject: [PATCH 5/5] break a chain of post-processors when one is skipped & make `-only` "blind" to post-processors * to avoid trouble * other arrays of post processors might still be there ! * add docs * update tests --- command/build_test.go | 51 +++++++++-------- .../test-fixtures/build-only/template.json | 55 +++++++++++-------- packer/core.go | 3 +- website/source/docs/commands/build.html.md | 18 +++--- .../docs/templates/provisioners.html.md | 9 +-- 5 files changed, 73 insertions(+), 63 deletions(-) diff --git a/command/build_test.go b/command/build_test.go index e334f0055..5e56d81f1 100644 --- a/command/build_test.go +++ b/command/build_test.go @@ -27,12 +27,13 @@ func TestBuildOnlyFileCommaFlags(t *testing.T) { fatalCommand(t, c.Meta) } - if !fileExists("chocolate.txt") { - t.Error("Expected to find chocolate.txt") - } - if !fileExists("vanilla.txt") { - t.Error("Expected to find vanilla.txt") + for _, f := range []string{"chocolate.txt", "vanilla.txt", + "apple.txt", "peach.txt", "pear.txt"} { + if !fileExists(f) { + t.Errorf("Expected to find %s", f) + } } + if fileExists("cherry.txt") { t.Error("Expected NOT to find cherry.txt") } @@ -57,14 +58,10 @@ func TestBuildStdin(t *testing.T) { fatalCommand(t, c.Meta) } - if !fileExists("chocolate.txt") { - t.Error("Expected to find chocolate.txt") - } - if !fileExists("vanilla.txt") { - t.Error("Expected to find vanilla.txt") - } - if !fileExists("cherry.txt") { - t.Error("Expected to find cherry.txt") + for _, f := range []string{"vanilla.txt", "cherry.txt", "chocolate.txt"} { + if !fileExists(f) { + t.Errorf("Expected to find %s", f) + } } } @@ -76,7 +73,9 @@ func TestBuildOnlyFileMultipleFlags(t *testing.T) { args := []string{ "-only=chocolate", "-only=cherry", - "-only=apple", + "-only=apple", // ignored + "-only=peach", // ignored + "-only=pear", // ignored filepath.Join(testFixture("build-only"), "template.json"), } @@ -86,17 +85,16 @@ func TestBuildOnlyFileMultipleFlags(t *testing.T) { fatalCommand(t, c.Meta) } - if !fileExists("chocolate.txt") { - t.Error("Expected to find chocolate.txt") - } - if fileExists("vanilla.txt") { - t.Error("Expected NOT to find vanilla.txt") - } - if !fileExists("cherry.txt") { - t.Error("Expected to find cherry.txt") + for _, f := range []string{"vanilla.txt"} { + if fileExists(f) { + t.Errorf("Expected NOT to find %s", f) + } } - if !fileExists("apple.txt") { - t.Error("Expected to find apple.txt") + for _, f := range []string{"chocolate.txt", "cherry.txt", + "apple.txt", "peach.txt", "pear.txt"} { + if !fileExists(f) { + t.Errorf("Expected to find %s", f) + } } } @@ -116,12 +114,12 @@ func TestBuildExceptFileCommaFlags(t *testing.T) { fatalCommand(t, c.Meta) } - for _, f := range []string{"chocolate.txt", "apple.txt"} { + for _, f := range []string{"chocolate.txt", "apple.txt", "peach.txt"} { if fileExists(f) { t.Errorf("Expected NOT to find %s", f) } } - for _, f := range []string{"vanilla.txt", "cherry.txt", "peach.txt"} { + for _, f := range []string{"vanilla.txt", "cherry.txt", "pear.txt"} { if !fileExists(f) { t.Errorf("Expected to find %s", f) } @@ -170,4 +168,5 @@ func cleanup() { os.RemoveAll("cherry.txt") os.RemoveAll("apple.txt") os.RemoveAll("peach.txt") + os.RemoveAll("pear.txt") } diff --git a/command/test-fixtures/build-only/template.json b/command/test-fixtures/build-only/template.json index 036990227..af09b6cbf 100644 --- a/command/test-fixtures/build-only/template.json +++ b/command/test-fixtures/build-only/template.json @@ -1,34 +1,43 @@ { "builders": [ { - "name":"chocolate", - "type":"file", - "content":"chocolate", - "target":"chocolate.txt" + "name": "chocolate", + "type": "file", + "content": "chocolate", + "target": "chocolate.txt" }, { - "name":"vanilla", - "type":"file", - "content":"vanilla", - "target":"vanilla.txt" + "name": "vanilla", + "type": "file", + "content": "vanilla", + "target": "vanilla.txt" }, { - "name":"cherry", - "type":"file", - "content":"cherry", - "target":"cherry.txt" + "name": "cherry", + "type": "file", + "content": "cherry", + "target": "cherry.txt" } ], "post-processors": [ - { - "name": "apple", - "type": "shell-local", - "inline": ["touch apple.txt"] - }, - { - "name": "peach", - "type": "shell-local", - "inline": ["touch peach.txt"] - } + [ + { + "name": "apple", + "type": "shell-local", + "inline": [ "touch apple.txt" ] + }, + { + "name": "peach", + "type": "shell-local", + "inline": [ "touch peach.txt" ] + } + ], + [ + { + "name": "pear", + "type": "shell-local", + "inline": [ "touch pear.txt" ] + } + ] ] -} +} \ No newline at end of file diff --git a/packer/core.go b/packer/core.go index 5c68b348b..feec80d24 100644 --- a/packer/core.go +++ b/packer/core.go @@ -182,12 +182,11 @@ func (c *Core) Build(n string) (Build, error) { for _, rawP := range rawPs { // If we skip, ignore rawP.OnlyExcept.Except = append(rawP.OnlyExcept.Except, c.except...) - rawP.OnlyExcept.Only = append(rawP.OnlyExcept.Only, c.only...) if rawP.OnlyExcept.Skip(rawName) { continue } if rawP.OnlyExcept.Skip(rawP.Name) { - continue + break } // Get the post-processor diff --git a/website/source/docs/commands/build.html.md b/website/source/docs/commands/build.html.md index 131b22587..61ee8c3a9 100644 --- a/website/source/docs/commands/build.html.md +++ b/website/source/docs/commands/build.html.md @@ -26,10 +26,12 @@ artifacts that are created will be outputted at the end of the build. will stop between each step, waiting for keyboard input before continuing. This will allow the user to inspect state and so on. -- `-except=foo,bar,baz` - Run all the builds and post-processors except - those with the given comma-separated names. Build and post-processor names - by default are their type, unless a specific `name` attribute is specified - within the configuration. +- `-except=foo,bar,baz` - Run all the builds and post-processors except those + with the given comma-separated names. Build and post-processor names by + default are their type, unless a specific `name` attribute is specified + within the configuration. Any post-processor following a skipped + post-processor will not run. Because post-processors can be nested in + arrays a differ post-processor chain can still run. - `-force` - Forces a builder to run when artifacts from a previous build prevent a build from running. The exact behavior of a forced build is left @@ -44,10 +46,10 @@ artifacts that are created will be outputted at the end of the build. presents a prompt and waits for you to decide to clean up, abort, or retry the failed step. -- `-only=foo,bar,baz` - Only run the builds and post-processors with the - given comma-separated names. Build and post-processor names by default are - their type, unless a specific `name` attribute is specified within the - configuration. +- `-only=foo,bar,baz` - Only run the builds with the given comma-separated + names. Build names by default are their type, unless a specific `name` + attribute is specified within the configuration. `-only` does not apply to + post-processors. - `-parallel=false` - Disable parallelization of multiple builders (on by default). diff --git a/website/source/docs/templates/provisioners.html.md b/website/source/docs/templates/provisioners.html.md index d4cd5e555..b74929562 100644 --- a/website/source/docs/templates/provisioners.html.md +++ b/website/source/docs/templates/provisioners.html.md @@ -77,10 +77,11 @@ effectively the same: } ``` -The values within `only` or `except` can be *build or post-processor names*, -not builder types. If you recall, build and post-processor names by default are -just their builder type, but if you specify a custom `name` parameter, then you -should use that as the value instead of the type. +The values within `only` or `except` are *build names*, not builder types. If +you recall, build names by default are just their builder type, but if you +specify a custom `name` parameter, then you should use that as the value +instead of the type. +Values within `except` could also be a *post-processor* name. ## Build-Specific Overrides