From 6af2fd5bd08826e8e1a9d78afc017853db58150b Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Wed, 28 Oct 2015 16:10:27 -0400 Subject: [PATCH 01/18] Update Packer Debug Docs for Cloud-Init Issue Update documentation to make official note of the cloud-init issue with ubuntu AMIs. --- .../source/docs/other/debugging.html.markdown | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/website/source/docs/other/debugging.html.markdown b/website/source/docs/other/debugging.html.markdown index 2ca5ab7d1..33a515091 100644 --- a/website/source/docs/other/debugging.html.markdown +++ b/website/source/docs/other/debugging.html.markdown @@ -58,10 +58,36 @@ any logging to be enabled. ### Debugging Packer in Powershell/Windows In Windows you can set the detailed logs environmental variable `PACKER_LOG` or -the log variable `PACKER_LOG_PATH` using powershell environment variables. For example: +the log variable `PACKER_LOG_PATH` using powershell environment variables. For +example: $env:PACKER_LOG=1 $env:PACKER_LOG_PATH="packerlog.txt" If you find a bug with Packer, please include the detailed log by using a service such as [gist](http://gist.github.com). + +## Issues Installing Ubuntu Packages + +Issues may arise using and building Ubuntu AMIs where common packages that +*should* be installed from Ubuntu's Main repository are not found during a +provisioner step: + + amazon-ebs: No candidate version found for build-essential + amazon-ebs: No candidate version found for build-essential + +This, obviously can cause problems where a build is unable to finish +successfully as the proper packages cannot be provisioned correctly. The problem +arises when cloud-init has not finished fully running on the source AMI by the +time that packer starts any provisioning steps. + +Adding the following provisioner to the packer template, allows for the +cloud-init process to fully finish before packer starts provisioning the source +AMI. + + { + "type": "shell", + "inline": [ + "while [ ! -f /var/lib/cloud/instance/boot-finished ]; do echo 'Waiting for cloud-init...'; sleep 1; done" + ] + } From 8e1cc16ab5c8a0d3c926dc8b7feaf73e4b6769b8 Mon Sep 17 00:00:00 2001 From: Vasiliy Tolstov Date: Thu, 29 Oct 2015 10:54:25 +0000 Subject: [PATCH 02/18] add convert step for qcow2 image format https://ext4.wiki.kernel.org/index.php/Ext4_VM_Images does not recommends to dd zero file and deletes it, but in case of enabling discards and qcow2 image we can recreate qcow2 file with less used space. Also qemu-img able to enable compression for qcow2 files, that sometimes may be useful because it natively supported by qemu. Signed-off-by: Vasiliy Tolstov --- builder/qemu/builder.go | 8 +++ builder/qemu/step_convert_disk.go | 67 +++++++++++++++++++ .../source/docs/builders/qemu.html.markdown | 6 ++ 3 files changed, 81 insertions(+) create mode 100644 builder/qemu/step_convert_disk.go diff --git a/builder/qemu/builder.go b/builder/qemu/builder.go index eb38ba0c7..2e7e107d7 100644 --- a/builder/qemu/builder.go +++ b/builder/qemu/builder.go @@ -88,6 +88,8 @@ type Config struct { DiskSize uint `mapstructure:"disk_size"` DiskCache string `mapstructure:"disk_cache"` DiskDiscard string `mapstructure:"disk_discard"` + SkipCompaction bool `mapstructure:"skip_compaction"` + DiskCompression bool `mapstructure:"disk_compression"` FloppyFiles []string `mapstructure:"floppy_files"` Format string `mapstructure:"format"` Headless bool `mapstructure:"headless"` @@ -242,6 +244,11 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs, errors.New("invalid format, only 'qcow2' or 'raw' are allowed")) } + if b.config.Format != "qcow2" { + b.config.SkipCompaction = true + b.config.DiskCompression = false + } + if _, ok := accels[b.config.Accelerator]; !ok { errs = packer.MultiErrorAppend( errs, errors.New("invalid accelerator, only 'kvm', 'tcg', 'xen', or 'none' are allowed")) @@ -364,6 +371,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe }, new(common.StepProvision), new(stepShutdown), + new(stepConvertDisk), } // Setup the state bag diff --git a/builder/qemu/step_convert_disk.go b/builder/qemu/step_convert_disk.go new file mode 100644 index 000000000..dcf5e97de --- /dev/null +++ b/builder/qemu/step_convert_disk.go @@ -0,0 +1,67 @@ +package qemu + +import ( + "fmt" + "path/filepath" + + "github.com/mitchellh/multistep" + "github.com/mitchellh/packer/packer" + + "os" +) + +// This step converts the virtual disk that was used as the +// hard drive for the virtual machine. +type stepConvertDisk struct{} + +func (s *stepConvertDisk) Run(state multistep.StateBag) multistep.StepAction { + config := state.Get("config").(*Config) + driver := state.Get("driver").(Driver) + diskName := state.Get("disk_filename").(string) + ui := state.Get("ui").(packer.Ui) + + if config.SkipCompaction && !config.DiskCompression { + return multistep.ActionContinue + } + + name := diskName + ".convert" + + sourcePath := filepath.Join(config.OutputDir, diskName) + targetPath := filepath.Join(config.OutputDir, name) + + command := []string{ + "convert", + "-q", + } + + if config.DiskCompression { + command = append(command, "-c") + } + + command = append(command, []string{ + "-f", config.Format, + "-O", config.Format, + sourcePath, + targetPath, + }..., + ) + + ui.Say("Converting hard drive...") + if err := driver.QemuImg(command...); err != nil { + err := fmt.Errorf("Error converting hard drive: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + if err := os.Rename(targetPath, sourcePath); err != nil { + err := fmt.Errorf("Error moving converted hard drive: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + return multistep.ActionContinue +} + +func (s *stepConvertDisk) Cleanup(state multistep.StateBag) {} diff --git a/website/source/docs/builders/qemu.html.markdown b/website/source/docs/builders/qemu.html.markdown index 13ad4bda2..5cfc767d3 100644 --- a/website/source/docs/builders/qemu.html.markdown +++ b/website/source/docs/builders/qemu.html.markdown @@ -136,6 +136,12 @@ builder. - `disk_size` (integer) - The size, in megabytes, of the hard disk to create for the VM. By default, this is 40000 (about 40 GB). +- `skip_compaction` (boolean) - Packer compacts the QCOW2 image using `qemu-img convert`. + Set this option to `true` to disable compacting. Defaults to `false`. + +- `disk_compression` (boolean) - Apply compression to the QCOW2 disk file + using `qemu-img convert`. Defaults to `false`. + - `floppy_files` (array of strings) - A list of files to place onto a floppy disk that is attached when the VM is booted. This is most useful for unattended Windows installs, which look for an `Autounattend.xml` file on From 8682dec178221c2a66d7b3518bc81ff58e44f3d3 Mon Sep 17 00:00:00 2001 From: Luke Amdor Date: Fri, 30 Oct 2015 13:58:56 -0500 Subject: [PATCH 03/18] aws: build after upstream breaking change see https://github.com/aws/aws-sdk-go/commit/1a69d069352edadd48f12b0f2c5f375de4a636d7 --- builder/amazon/chroot/builder.go | 4 +++- builder/amazon/common/artifact.go | 4 +++- builder/amazon/common/step_ami_region_copy.go | 5 ++++- builder/amazon/common/step_create_tags.go | 9 ++++++--- builder/amazon/common/step_modify_ami_attributes.go | 7 +++++-- builder/amazon/ebs/builder.go | 4 +++- builder/amazon/instance/builder.go | 4 +++- 7 files changed, 27 insertions(+), 10 deletions(-) diff --git a/builder/amazon/chroot/builder.go b/builder/amazon/chroot/builder.go index 636de8df7..637b1e40f 100644 --- a/builder/amazon/chroot/builder.go +++ b/builder/amazon/chroot/builder.go @@ -9,6 +9,7 @@ import ( "log" "runtime" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mitchellh/multistep" awscommon "github.com/mitchellh/packer/builder/amazon/common" @@ -131,7 +132,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, err } - ec2conn := ec2.New(config) + session := session.New(config) + ec2conn := ec2.New(session) wrappedCommand := func(command string) (string, error) { ctx := b.config.ctx diff --git a/builder/amazon/common/artifact.go b/builder/amazon/common/artifact.go index 8eed0134d..76f4e03e0 100644 --- a/builder/amazon/common/artifact.go +++ b/builder/amazon/common/artifact.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mitchellh/packer/packer" ) @@ -72,7 +73,8 @@ func (a *Artifact) Destroy() error { Credentials: a.Conn.Config.Credentials, Region: aws.String(region), } - regionConn := ec2.New(regionConfig) + sess := session.New(regionConfig) + regionConn := ec2.New(sess) input := &ec2.DeregisterImageInput{ ImageId: &imageId, diff --git a/builder/amazon/common/step_ami_region_copy.go b/builder/amazon/common/step_ami_region_copy.go index 0cf4d40fa..fa955ac7b 100644 --- a/builder/amazon/common/step_ami_region_copy.go +++ b/builder/amazon/common/step_ami_region_copy.go @@ -6,6 +6,7 @@ import ( "sync" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mitchellh/multistep" @@ -87,7 +88,9 @@ func amiRegionCopy(state multistep.StateBag, config *AccessConfig, name string, } awsConfig.Region = aws.String(target) - regionconn := ec2.New(awsConfig) + sess := session.New(awsConfig) + regionconn := ec2.New(sess) + resp, err := regionconn.CopyImage(&ec2.CopyImageInput{ SourceRegion: &source, SourceImageId: &imageId, diff --git a/builder/amazon/common/step_create_tags.go b/builder/amazon/common/step_create_tags.go index 7f62d2657..2ac39dc02 100644 --- a/builder/amazon/common/step_create_tags.go +++ b/builder/amazon/common/step_create_tags.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" @@ -33,11 +34,13 @@ func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction { // Declare list of resources to tag resourceIds := []*string{&ami} - - regionconn := ec2.New(&aws.Config{ + awsConfig := aws.Config{ Credentials: ec2conn.Config.Credentials, Region: aws.String(region), - }) + } + session := session.New(&awsConfig) + + regionconn := ec2.New(session) // Retrieve image list for given AMI imageResp, err := regionconn.DescribeImages(&ec2.DescribeImagesInput{ diff --git a/builder/amazon/common/step_modify_ami_attributes.go b/builder/amazon/common/step_modify_ami_attributes.go index e8e7de589..31d60a392 100644 --- a/builder/amazon/common/step_modify_ami_attributes.go +++ b/builder/amazon/common/step_modify_ami_attributes.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" @@ -88,10 +89,12 @@ func (s *StepModifyAMIAttributes) Run(state multistep.StateBag) multistep.StepAc for region, ami := range amis { ui.Say(fmt.Sprintf("Modifying attributes on AMI (%s)...", ami)) - regionconn := ec2.New(&aws.Config{ + awsConfig := aws.Config{ Credentials: ec2conn.Config.Credentials, Region: aws.String(region), - }) + } + session := session.New(&awsConfig) + regionconn := ec2.New(session) for name, input := range options { ui.Message(fmt.Sprintf("Modifying: %s", name)) input.ImageId = &ami diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index 26a525bcb..b9277a5ab 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -9,6 +9,7 @@ import ( "fmt" "log" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mitchellh/multistep" awscommon "github.com/mitchellh/packer/builder/amazon/common" @@ -68,7 +69,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, err } - ec2conn := ec2.New(config) + session := session.New(config) + ec2conn := ec2.New(session) // Setup the state bag and initial state for the steps state := new(multistep.BasicStateBag) diff --git a/builder/amazon/instance/builder.go b/builder/amazon/instance/builder.go index 6ca908230..6b1726efb 100644 --- a/builder/amazon/instance/builder.go +++ b/builder/amazon/instance/builder.go @@ -9,6 +9,7 @@ import ( "os" "strings" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mitchellh/multistep" awscommon "github.com/mitchellh/packer/builder/amazon/common" @@ -159,7 +160,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, err } - ec2conn := ec2.New(config) + session := session.New(config) + ec2conn := ec2.New(session) // Setup the state bag and initial state for the steps state := new(multistep.BasicStateBag) From 62bcf6c7ce8a69181b07efa104a93ccf10f944ed Mon Sep 17 00:00:00 2001 From: Jason Martin Date: Fri, 30 Oct 2015 13:19:44 -0700 Subject: [PATCH 04/18] Add IAM policy for copying images The ec2:CopyImage privilege is required in order to make a cross-region AMI when using the `ami_regions` option for the amazon-ebs builder. --- website/source/docs/builders/amazon.html.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/source/docs/builders/amazon.html.markdown b/website/source/docs/builders/amazon.html.markdown index a85e22d1a..ae9c5fcb8 100644 --- a/website/source/docs/builders/amazon.html.markdown +++ b/website/source/docs/builders/amazon.html.markdown @@ -101,6 +101,7 @@ Packer to work: "ec2:DeleteSecurityGroup", "ec2:AuthorizeSecurityGroupIngress", "ec2:CreateImage", + "ec2:CopyImage", "ec2:RunInstances", "ec2:TerminateInstances", "ec2:StopInstances", From 07079a5905ee6dde84fa726fc4c3e116a37a2bf1 Mon Sep 17 00:00:00 2001 From: Yuya Kusakabe Date: Sat, 31 Oct 2015 19:32:40 +0900 Subject: [PATCH 05/18] Fix #2892 --- builder/vmware/iso/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/vmware/iso/builder.go b/builder/vmware/iso/builder.go index a9f2ae9b8..7958b5760 100755 --- a/builder/vmware/iso/builder.go +++ b/builder/vmware/iso/builder.go @@ -40,7 +40,7 @@ type Config struct { DiskSize uint `mapstructure:"disk_size"` DiskTypeId string `mapstructure:"disk_type_id"` FloppyFiles []string `mapstructure:"floppy_files"` - Format string `mapstruture:"format"` + Format string `mapstructure:"format"` GuestOSType string `mapstructure:"guest_os_type"` Version string `mapstructure:"version"` VMName string `mapstructure:"vm_name"` From ca19688316b63cb9c4a449b94bd4dbc369d5d177 Mon Sep 17 00:00:00 2001 From: Mark Peek Date: Sat, 31 Oct 2015 11:04:50 -0700 Subject: [PATCH 06/18] aws: fix test breakage due to upstream breaking change #2891 --- builder/amazon/ebs/builder_acc_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builder/amazon/ebs/builder_acc_test.go b/builder/amazon/ebs/builder_acc_test.go index 890b3228a..47da06c10 100644 --- a/builder/amazon/ebs/builder_acc_test.go +++ b/builder/amazon/ebs/builder_acc_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mitchellh/packer/builder/amazon/common" builderT "github.com/mitchellh/packer/helper/builder/testing" @@ -160,7 +161,8 @@ func testEC2Conn() (*ec2.EC2, error) { return nil, err } - return ec2.New(config), nil + session := session.New(config) + return ec2.New(session), nil } const testBuilderAccBasic = ` From 31dd989e2efef42a30a1f2f8ea6c0364f3805adc Mon Sep 17 00:00:00 2001 From: Mark Peek Date: Sat, 31 Oct 2015 18:15:19 -0700 Subject: [PATCH 07/18] Add qcow2 shrink/compress tests for #2748 --- builder/qemu/builder_test.go | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/builder/qemu/builder_test.go b/builder/qemu/builder_test.go index d2a1d1eba..f885c570b 100644 --- a/builder/qemu/builder_test.go +++ b/builder/qemu/builder_test.go @@ -132,6 +132,48 @@ func TestBuilderPrepare_BootWait(t *testing.T) { } } +func TestBuilderPrepare_DiskCompaction(t *testing.T) { + var b Builder + config := testConfig() + + // Bad + config["skip_compaction"] = false + config["disk_compression"] = true + config["format"] = "img" + warns, err := b.Prepare(config) + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + if err == nil { + t.Fatal("should have error") + } + if b.config.SkipCompaction != true { + t.Fatalf("SkipCompaction should be true") + } + if b.config.DiskCompression != false { + t.Fatalf("DiskCompression should be false") + } + + // Good + config["skip_compaction"] = false + config["disk_compression"] = true + config["format"] = "qcow2" + b = Builder{} + 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) + } + if b.config.SkipCompaction != false { + t.Fatalf("SkipCompaction should be false") + } + if b.config.DiskCompression != true { + t.Fatalf("DiskCompression should be true") + } +} + func TestBuilderPrepare_DiskSize(t *testing.T) { var b Builder config := testConfig() From b34525358d21a2f32cce0bdf76e446c75bde9388 Mon Sep 17 00:00:00 2001 From: Sergio Rodriguez Date: Mon, 2 Nov 2015 16:35:07 -0500 Subject: [PATCH 08/18] add "disable_sudo" configuration reference --- website/source/docs/provisioners/salt-masterless.html.markdown | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/source/docs/provisioners/salt-masterless.html.markdown b/website/source/docs/provisioners/salt-masterless.html.markdown index adb1c4bb3..7e69f2351 100644 --- a/website/source/docs/provisioners/salt-masterless.html.markdown +++ b/website/source/docs/provisioners/salt-masterless.html.markdown @@ -38,6 +38,9 @@ Optional: has more detailed usage instructions. By default, no arguments are sent to the script. +- `disable_sudo` (boolean) - By default, the bootstrap install command is prefixed with `sudo`. When using a + Docker builder, you will likely want to pass `true` since `sudo` is often not pre-installed. + - `remote_pillar_roots` (string) - The path to your remote [pillar roots](http://docs.saltstack.com/ref/configuration/master.html#pillar-configuration). default: `/srv/pillar`. From 61c98bb701d528dd4232f310d7638619e5ba2ba8 Mon Sep 17 00:00:00 2001 From: Barrie Bremner Date: Tue, 3 Nov 2015 14:48:21 +0000 Subject: [PATCH 09/18] Doc change only: misspelling of 'termination' --- website/source/docs/builders/amazon-ebs.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/builders/amazon-ebs.html.markdown b/website/source/docs/builders/amazon-ebs.html.markdown index effa07be2..860040ee7 100644 --- a/website/source/docs/builders/amazon-ebs.html.markdown +++ b/website/source/docs/builders/amazon-ebs.html.markdown @@ -278,7 +278,7 @@ Here is an example using the optional AMI tags. This will add the tags -> **Note:** Packer uses pre-built AMIs as the source for building images. These source AMIs may include volumes that are not flagged to be destroyed on -termiation of the instance building the new image. Packer will attempt to clean +termination of the instance building the new image. Packer will attempt to clean up all residual volumes that are not designated by the user to remain after termination. If you need to preserve those source volumes, you can overwrite the termination setting by specifying `delete_on_termination=false` in the From ebed9e53fb9336f2c7dfe5fcb6d42d4f3a12fb55 Mon Sep 17 00:00:00 2001 From: Trevor Suarez Date: Tue, 3 Nov 2015 12:30:55 -0500 Subject: [PATCH 10/18] 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 11/18] 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 12/18] 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 13/18] 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 14/18] 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 15/18] 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 16/18] 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}}" } From 38612d45a923c6f276524cfe6d8164668b79edf5 Mon Sep 17 00:00:00 2001 From: Rickard von Essen Date: Wed, 4 Nov 2015 15:29:26 +0100 Subject: [PATCH 17/18] Make all scripts portable regardless of where bash is installed. --- scripts/build.sh | 2 +- scripts/dist.sh | 2 +- scripts/upload.sh | 2 +- scripts/website_push.sh | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/build.sh b/scripts/build.sh index dcd9bd7c8..2b5048ecb 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # # This script builds the application from source for multiple platforms. set -e diff --git a/scripts/dist.sh b/scripts/dist.sh index 9533ef285..ba5e1b2c0 100755 --- a/scripts/dist.sh +++ b/scripts/dist.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -e # Get the parent directory of where this script is. diff --git a/scripts/upload.sh b/scripts/upload.sh index 9854a3b0e..284724bab 100755 --- a/scripts/upload.sh +++ b/scripts/upload.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -e # Get the parent directory of where this script is. diff --git a/scripts/website_push.sh b/scripts/website_push.sh index 95168f977..a51c9c9b1 100755 --- a/scripts/website_push.sh +++ b/scripts/website_push.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Set the tmpdir if [ -z "$TMPDIR" ]; then From 1c7a8553021c6facc63cbc5e69f84c5ebe060382 Mon Sep 17 00:00:00 2001 From: Mark Peek Date: Wed, 4 Nov 2015 12:36:00 -0800 Subject: [PATCH 18/18] Switch osext package from mitchellh -> kardianos #2842 --- config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.go b/config.go index 3a4c41ec9..07a64d0af 100644 --- a/config.go +++ b/config.go @@ -10,7 +10,7 @@ import ( "runtime" "strings" - "github.com/mitchellh/osext" + "github.com/kardianos/osext" "github.com/mitchellh/packer/command" "github.com/mitchellh/packer/packer" "github.com/mitchellh/packer/packer/plugin"