From ebed9e53fb9336f2c7dfe5fcb6d42d4f3a12fb55 Mon Sep 17 00:00:00 2001 From: Trevor Suarez Date: Tue, 3 Nov 2015 12:30:55 -0500 Subject: [PATCH 1/7] Adding new "Options" configuration parameter for the puppet-masterless provisioner, to allow for specifying additional options to pass to the execute command --- provisioner/puppet-masterless/provisioner.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/provisioner/puppet-masterless/provisioner.go b/provisioner/puppet-masterless/provisioner.go index 546224a54..b3dc5117f 100644 --- a/provisioner/puppet-masterless/provisioner.go +++ b/provisioner/puppet-masterless/provisioner.go @@ -22,6 +22,9 @@ type Config struct { // The command used to execute Puppet. ExecuteCommand string `mapstructure:"execute_command"` + // Additional options to pass when executing Puppet + Options []string + // Additional facts to set when executing Puppet Facter map[string]string @@ -62,6 +65,7 @@ type ExecuteTemplate struct { ManifestFile string ManifestDir string Sudo bool + Options string } func (p *Provisioner) Prepare(raws ...interface{}) error { @@ -86,6 +90,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { "{{if ne .HieraConfigPath \"\"}}--hiera_config='{{.HieraConfigPath}}' {{end}}" + "{{if ne .ManifestDir \"\"}}--manifestdir='{{.ManifestDir}}' {{end}}" + "--detailed-exitcodes " + + "{{.Options}} " + "{{.ManifestFile}}" } @@ -218,6 +223,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { ModulePath: strings.Join(modulePaths, ":"), Sudo: !p.config.PreventSudo, WorkingDir: p.config.WorkingDir, + Options: strings.Join(p.config.Options, " "), } command, err := interpolate.Render(p.config.ExecuteCommand, &p.config.ctx) if err != nil { From e41f0bb9f56c26e4ab221df43c7fabd8424dd8fb Mon Sep 17 00:00:00 2001 From: Trevor Suarez Date: Tue, 3 Nov 2015 13:55:40 -0500 Subject: [PATCH 2/7] Adding documentation for the new configuration parameter --- .../docs/provisioners/puppet-masterless.html.markdown | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/website/source/docs/provisioners/puppet-masterless.html.markdown b/website/source/docs/provisioners/puppet-masterless.html.markdown index 7ef13265e..c283c7a46 100644 --- a/website/source/docs/provisioners/puppet-masterless.html.markdown +++ b/website/source/docs/provisioners/puppet-masterless.html.markdown @@ -59,6 +59,12 @@ Optional parameters: variables](/docs/templates/configuration-templates.html) available. See below for more information. +- `options` (array of strings) - This is an array of additional options to + pass to the puppet command when executing puppet. This allows for + customization of the `execute_command` without having to completely replace + or include it's contents, making forward-compatible customizations much + easier. + - `facter` (object of key/value strings) - Additional [facts](http://puppetlabs.com/puppet/related-projects/facter) to make available when Puppet is running. From 84e1b387c4c5adb69dafe5f949150ad7cc0ed593 Mon Sep 17 00:00:00 2001 From: Trevor Suarez Date: Tue, 3 Nov 2015 14:36:04 -0500 Subject: [PATCH 3/7] New test for preparing the new config parameter --- .../puppet-masterless/provisioner_test.go | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/provisioner/puppet-masterless/provisioner_test.go b/provisioner/puppet-masterless/provisioner_test.go index 42ddd9d7a..5416447a2 100644 --- a/provisioner/puppet-masterless/provisioner_test.go +++ b/provisioner/puppet-masterless/provisioner_test.go @@ -1,10 +1,11 @@ package puppetmasterless import ( - "github.com/mitchellh/packer/packer" "io/ioutil" "os" "testing" + + "github.com/mitchellh/packer/packer" ) func testConfig() map[string]interface{} { @@ -177,3 +178,32 @@ func TestProvisionerPrepare_facterFacts(t *testing.T) { t.Fatalf("err: Default facts are not set in the Puppet provisioner!") } } + +func TestProvisionerPrepare_options(t *testing.T) { + config := testConfig() + + delete(config, "options") + p := new(Provisioner) + err := p.Prepare(config) + if err != nil { + t.Fatalf("err: %s", err) + } + + // Test with malformed fact + config["options"] = "{{}}" + p = new(Provisioner) + err = p.Prepare(config) + if err == nil { + t.Fatal("should be an error") + } + + config["options"] = []string{ + "arg", + } + + p = new(Provisioner) + err = p.Prepare(config) + if err != nil { + t.Fatalf("err: %s", err) + } +} From 4ea7e3473ddcb95930097d9335a3f391ed645167 Mon Sep 17 00:00:00 2001 From: Trevor Suarez Date: Tue, 3 Nov 2015 14:59:55 -0500 Subject: [PATCH 4/7] Testing the new options argument during the actual call to `Provision()` --- .../puppet-masterless/provisioner_test.go | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/provisioner/puppet-masterless/provisioner_test.go b/provisioner/puppet-masterless/provisioner_test.go index 5416447a2..10048551c 100644 --- a/provisioner/puppet-masterless/provisioner_test.go +++ b/provisioner/puppet-masterless/provisioner_test.go @@ -3,6 +3,7 @@ package puppetmasterless import ( "io/ioutil" "os" + "strings" "testing" "github.com/mitchellh/packer/packer" @@ -207,3 +208,34 @@ func TestProvisionerPrepare_options(t *testing.T) { t.Fatalf("err: %s", err) } } + +func TestProvisionerProvision_options(t *testing.T) { + config := testConfig() + ui := &packer.MachineReadableUi{ + Writer: ioutil.Discard, + } + comm := new(packer.MockCommunicator) + + options := []string{ + "--some-arg=yup", + "--some-other-arg", + } + config["options"] = options + + p := new(Provisioner) + err := p.Prepare(config) + if err != nil { + t.Fatalf("err: %s", err) + } + + err = p.Provision(ui, comm) + if err != nil { + t.Fatalf("err: %s", err) + } + + expectedArgs := strings.Join(options, " ") + + if !strings.Contains(comm.StartCmd.Command, expectedArgs) { + t.Fatalf("Command %q doesn't contain the expected arguments %q", comm.StartCmd.Command, expectedArgs) + } +} From 627a8fe819cfd0db9bd2072465e719283de20774 Mon Sep 17 00:00:00 2001 From: Trevor Suarez Date: Tue, 3 Nov 2015 17:55:03 -0500 Subject: [PATCH 5/7] Renaming the config parameter from "options" to "extra_arguments" --- provisioner/puppet-masterless/provisioner.go | 10 +++++----- .../puppet-masterless/provisioner_test.go | 16 ++++++++-------- .../provisioners/puppet-masterless.html.markdown | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/provisioner/puppet-masterless/provisioner.go b/provisioner/puppet-masterless/provisioner.go index b3dc5117f..4e35d1a94 100644 --- a/provisioner/puppet-masterless/provisioner.go +++ b/provisioner/puppet-masterless/provisioner.go @@ -22,8 +22,8 @@ type Config struct { // The command used to execute Puppet. ExecuteCommand string `mapstructure:"execute_command"` - // Additional options to pass when executing Puppet - Options []string + // Additional arguments to pass when executing Puppet + ExtraArguments []string `mapstructure:"extra_arguments"` // Additional facts to set when executing Puppet Facter map[string]string @@ -65,7 +65,7 @@ type ExecuteTemplate struct { ManifestFile string ManifestDir string Sudo bool - Options string + ExtraArguments string } func (p *Provisioner) Prepare(raws ...interface{}) error { @@ -90,7 +90,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { "{{if ne .HieraConfigPath \"\"}}--hiera_config='{{.HieraConfigPath}}' {{end}}" + "{{if ne .ManifestDir \"\"}}--manifestdir='{{.ManifestDir}}' {{end}}" + "--detailed-exitcodes " + - "{{.Options}} " + + "{{.ExtraArguments}} " + "{{.ManifestFile}}" } @@ -223,7 +223,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { ModulePath: strings.Join(modulePaths, ":"), Sudo: !p.config.PreventSudo, WorkingDir: p.config.WorkingDir, - Options: strings.Join(p.config.Options, " "), + ExtraArguments: strings.Join(p.config.ExtraArguments, " "), } command, err := interpolate.Render(p.config.ExecuteCommand, &p.config.ctx) if err != nil { diff --git a/provisioner/puppet-masterless/provisioner_test.go b/provisioner/puppet-masterless/provisioner_test.go index 10048551c..9355897b4 100644 --- a/provisioner/puppet-masterless/provisioner_test.go +++ b/provisioner/puppet-masterless/provisioner_test.go @@ -180,10 +180,10 @@ func TestProvisionerPrepare_facterFacts(t *testing.T) { } } -func TestProvisionerPrepare_options(t *testing.T) { +func TestProvisionerPrepare_extraArguments(t *testing.T) { config := testConfig() - delete(config, "options") + delete(config, "extra_arguments") p := new(Provisioner) err := p.Prepare(config) if err != nil { @@ -191,14 +191,14 @@ func TestProvisionerPrepare_options(t *testing.T) { } // Test with malformed fact - config["options"] = "{{}}" + config["extra_arguments"] = "{{}}" p = new(Provisioner) err = p.Prepare(config) if err == nil { t.Fatal("should be an error") } - config["options"] = []string{ + config["extra_arguments"] = []string{ "arg", } @@ -209,18 +209,18 @@ func TestProvisionerPrepare_options(t *testing.T) { } } -func TestProvisionerProvision_options(t *testing.T) { +func TestProvisionerProvision_extraArguments(t *testing.T) { config := testConfig() ui := &packer.MachineReadableUi{ Writer: ioutil.Discard, } comm := new(packer.MockCommunicator) - options := []string{ + extraArguments := []string{ "--some-arg=yup", "--some-other-arg", } - config["options"] = options + config["extra_arguments"] = extraArguments p := new(Provisioner) err := p.Prepare(config) @@ -233,7 +233,7 @@ func TestProvisionerProvision_options(t *testing.T) { t.Fatalf("err: %s", err) } - expectedArgs := strings.Join(options, " ") + expectedArgs := strings.Join(extraArguments, " ") if !strings.Contains(comm.StartCmd.Command, expectedArgs) { t.Fatalf("Command %q doesn't contain the expected arguments %q", comm.StartCmd.Command, expectedArgs) diff --git a/website/source/docs/provisioners/puppet-masterless.html.markdown b/website/source/docs/provisioners/puppet-masterless.html.markdown index c283c7a46..7995a0ee2 100644 --- a/website/source/docs/provisioners/puppet-masterless.html.markdown +++ b/website/source/docs/provisioners/puppet-masterless.html.markdown @@ -59,7 +59,7 @@ Optional parameters: variables](/docs/templates/configuration-templates.html) available. See below for more information. -- `options` (array of strings) - This is an array of additional options to +- `extra_arguments` (array of strings) - This is an array of additional options to pass to the puppet command when executing puppet. This allows for customization of the `execute_command` without having to completely replace or include it's contents, making forward-compatible customizations much From 6ca02286d4875c2713579d411c94822eab485fa0 Mon Sep 17 00:00:00 2001 From: Trevor Suarez Date: Tue, 3 Nov 2015 18:18:24 -0500 Subject: [PATCH 6/7] Test for when the config parameter isn't passed --- .../puppet-masterless/provisioner_test.go | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/provisioner/puppet-masterless/provisioner_test.go b/provisioner/puppet-masterless/provisioner_test.go index 9355897b4..b2cc3adb9 100644 --- a/provisioner/puppet-masterless/provisioner_test.go +++ b/provisioner/puppet-masterless/provisioner_test.go @@ -183,6 +183,7 @@ func TestProvisionerPrepare_facterFacts(t *testing.T) { func TestProvisionerPrepare_extraArguments(t *testing.T) { config := testConfig() + // Test with missing parameter delete(config, "extra_arguments") p := new(Provisioner) err := p.Prepare(config) @@ -190,7 +191,7 @@ func TestProvisionerPrepare_extraArguments(t *testing.T) { t.Fatalf("err: %s", err) } - // Test with malformed fact + // Test with malformed value config["extra_arguments"] = "{{}}" p = new(Provisioner) err = p.Prepare(config) @@ -198,6 +199,7 @@ func TestProvisionerPrepare_extraArguments(t *testing.T) { t.Fatal("should be an error") } + // Test with valid values config["extra_arguments"] = []string{ "arg", } @@ -222,6 +224,7 @@ func TestProvisionerProvision_extraArguments(t *testing.T) { } config["extra_arguments"] = extraArguments + // Test with valid values p := new(Provisioner) err := p.Prepare(config) if err != nil { @@ -238,4 +241,24 @@ func TestProvisionerProvision_extraArguments(t *testing.T) { if !strings.Contains(comm.StartCmd.Command, expectedArgs) { t.Fatalf("Command %q doesn't contain the expected arguments %q", comm.StartCmd.Command, expectedArgs) } + + // Test with missing parameter + delete(config, "extra_arguments") + + p = new(Provisioner) + err = p.Prepare(config) + if err != nil { + t.Fatalf("err: %s", err) + } + + err = p.Provision(ui, comm) + if err != nil { + t.Fatalf("err: %s", err) + } + + // Check the expected `extra_arguments` position for an empty value + splitCommand := strings.Split(comm.StartCmd.Command, " ") + if "" == splitCommand[len(splitCommand)-2] { + t.Fatalf("Command %q contains an extra-space which may cause arg parsing issues", comm.StartCmd.Command) + } } From f006a83c95bde7c3c4ec77609ca0acef513d24b0 Mon Sep 17 00:00:00 2001 From: Trevor Suarez Date: Tue, 3 Nov 2015 18:19:03 -0500 Subject: [PATCH 7/7] Fixing the bug found in the tests --- provisioner/puppet-masterless/provisioner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provisioner/puppet-masterless/provisioner.go b/provisioner/puppet-masterless/provisioner.go index 4e35d1a94..6eaac474b 100644 --- a/provisioner/puppet-masterless/provisioner.go +++ b/provisioner/puppet-masterless/provisioner.go @@ -90,7 +90,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { "{{if ne .HieraConfigPath \"\"}}--hiera_config='{{.HieraConfigPath}}' {{end}}" + "{{if ne .ManifestDir \"\"}}--manifestdir='{{.ManifestDir}}' {{end}}" + "--detailed-exitcodes " + - "{{.ExtraArguments}} " + + "{{if ne .ExtraArguments \"\"}}{{.ExtraArguments}} {{end}}" + "{{.ManifestFile}}" }