From 926c8bbaa6d9a4a64521cbaae74447ba641cc801 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 13 Jun 2019 17:23:21 +0200 Subject: [PATCH] refactor ShutdownConfig into a single struct --- builder/hyperv/iso/builder.go | 17 +++---- builder/hyperv/vmcx/builder.go | 17 +++---- builder/parallels/common/shutdown_config.go | 41 ----------------- .../parallels/common/shutdown_config_test.go | 45 ------------------- builder/parallels/iso/builder.go | 3 +- builder/parallels/pvm/config.go | 3 +- builder/qemu/builder.go | 44 +++++------------- builder/qemu/step_shutdown.go | 6 +-- builder/vmware/common/shutdown_config.go | 39 ---------------- builder/vmware/common/shutdown_config_test.go | 45 ------------------- builder/vmware/iso/config.go | 29 ++++++------ builder/vmware/vmx/config.go | 25 ++++++----- .../shutdowncommand/config.go | 4 +- .../shutdowncommand/config_test.go | 2 +- 14 files changed, 66 insertions(+), 254 deletions(-) delete mode 100644 builder/parallels/common/shutdown_config.go delete mode 100644 builder/parallels/common/shutdown_config_test.go delete mode 100644 builder/vmware/common/shutdown_config.go delete mode 100644 builder/vmware/common/shutdown_config_test.go rename builder/hyperv/common/shutdown_config.go => common/shutdowncommand/config.go (96%) rename builder/hyperv/common/shutdown_config_test.go => common/shutdowncommand/config_test.go (97%) diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 89d78344d..f7c6a4d7d 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/packer/common/bootcommand" powershell "github.com/hashicorp/packer/common/powershell" "github.com/hashicorp/packer/common/powershell/hyperv" + "github.com/hashicorp/packer/common/shutdowncommand" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" @@ -52,14 +53,14 @@ type Builder struct { } type Config struct { - common.PackerConfig `mapstructure:",squash"` - common.HTTPConfig `mapstructure:",squash"` - common.ISOConfig `mapstructure:",squash"` - common.FloppyConfig `mapstructure:",squash"` - bootcommand.BootConfig `mapstructure:",squash"` - hypervcommon.OutputConfig `mapstructure:",squash"` - hypervcommon.SSHConfig `mapstructure:",squash"` - hypervcommon.ShutdownConfig `mapstructure:",squash"` + common.PackerConfig `mapstructure:",squash"` + common.HTTPConfig `mapstructure:",squash"` + common.ISOConfig `mapstructure:",squash"` + common.FloppyConfig `mapstructure:",squash"` + bootcommand.BootConfig `mapstructure:",squash"` + hypervcommon.OutputConfig `mapstructure:",squash"` + hypervcommon.SSHConfig `mapstructure:",squash"` + shutdowncommand.ShutdownConfig `mapstructure:",squash"` // The size, in megabytes, of the hard disk to create // for the VM. By default, this is 40 GB. DiskSize uint `mapstructure:"disk_size" required:"false"` diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index 03e0e2cd6..324f26c68 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/packer/common/bootcommand" powershell "github.com/hashicorp/packer/common/powershell" "github.com/hashicorp/packer/common/powershell/hyperv" + "github.com/hashicorp/packer/common/shutdowncommand" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" @@ -42,14 +43,14 @@ type Builder struct { } type Config struct { - common.PackerConfig `mapstructure:",squash"` - common.HTTPConfig `mapstructure:",squash"` - common.ISOConfig `mapstructure:",squash"` - common.FloppyConfig `mapstructure:",squash"` - bootcommand.BootConfig `mapstructure:",squash"` - hypervcommon.OutputConfig `mapstructure:",squash"` - hypervcommon.SSHConfig `mapstructure:",squash"` - hypervcommon.ShutdownConfig `mapstructure:",squash"` + common.PackerConfig `mapstructure:",squash"` + common.HTTPConfig `mapstructure:",squash"` + common.ISOConfig `mapstructure:",squash"` + common.FloppyConfig `mapstructure:",squash"` + bootcommand.BootConfig `mapstructure:",squash"` + hypervcommon.OutputConfig `mapstructure:",squash"` + hypervcommon.SSHConfig `mapstructure:",squash"` + shutdowncommand.ShutdownConfig `mapstructure:",squash"` // The amount, in megabytes, of RAM to assign to the // VM. By default, this is 1 GB. RamSize uint `mapstructure:"memory" required:"false"` diff --git a/builder/parallels/common/shutdown_config.go b/builder/parallels/common/shutdown_config.go deleted file mode 100644 index 3bf491460..000000000 --- a/builder/parallels/common/shutdown_config.go +++ /dev/null @@ -1,41 +0,0 @@ -//go:generate struct-markdown - -package common - -import ( - "fmt" - "time" - - "github.com/hashicorp/packer/template/interpolate" -) - -// ShutdownConfig contains the configuration for VM shutdown. -type ShutdownConfig struct { - // The command to use to gracefully shut down the - // machine once all the provisioning is done. By default this is an empty - // string, which tells Packer to just forcefully shut down the machine. - ShutdownCommand string `mapstructure:"shutdown_command" required:"false"` - // The amount of time to wait after executing the - // shutdown_command for the virtual machine to actually shut down. If it - // doesn't shut down in this time, it is an error. By default, the timeout is - // "5m", or five minutes. - RawShutdownTimeout string `mapstructure:"shutdown_timeout" required:"false"` - - ShutdownTimeout time.Duration `` -} - -// Prepare sets default values to the VM shutdown configuration. -func (c *ShutdownConfig) Prepare(ctx *interpolate.Context) []error { - if c.RawShutdownTimeout == "" { - c.RawShutdownTimeout = "5m" - } - - var errs []error - var err error - c.ShutdownTimeout, err = time.ParseDuration(c.RawShutdownTimeout) - if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing shutdown_timeout: %s", err)) - } - - return errs -} diff --git a/builder/parallels/common/shutdown_config_test.go b/builder/parallels/common/shutdown_config_test.go deleted file mode 100644 index 5da613a19..000000000 --- a/builder/parallels/common/shutdown_config_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package common - -import ( - "testing" - "time" -) - -func testShutdownConfig() *ShutdownConfig { - return &ShutdownConfig{} -} - -func TestShutdownConfigPrepare_ShutdownCommand(t *testing.T) { - var c *ShutdownConfig - var errs []error - - c = testShutdownConfig() - errs = c.Prepare(testConfigTemplate(t)) - if len(errs) > 0 { - t.Fatalf("err: %#v", errs) - } -} - -func TestShutdownConfigPrepare_ShutdownTimeout(t *testing.T) { - var c *ShutdownConfig - var errs []error - - // Test with a bad value - c = testShutdownConfig() - c.RawShutdownTimeout = "this is not good" - errs = c.Prepare(testConfigTemplate(t)) - if len(errs) == 0 { - t.Fatalf("should have error") - } - - // Test with a good one - c = testShutdownConfig() - c.RawShutdownTimeout = "5s" - errs = c.Prepare(testConfigTemplate(t)) - if len(errs) > 0 { - t.Fatalf("err: %#v", errs) - } - if c.ShutdownTimeout != 5*time.Second { - t.Fatalf("bad: %s", c.ShutdownTimeout) - } -} diff --git a/builder/parallels/iso/builder.go b/builder/parallels/iso/builder.go index f55314352..63b4aa618 100644 --- a/builder/parallels/iso/builder.go +++ b/builder/parallels/iso/builder.go @@ -10,6 +10,7 @@ import ( parallelscommon "github.com/hashicorp/packer/builder/parallels/common" "github.com/hashicorp/packer/common" "github.com/hashicorp/packer/common/bootcommand" + "github.com/hashicorp/packer/common/shutdowncommand" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" @@ -35,7 +36,7 @@ type Config struct { parallelscommon.PrlctlConfig `mapstructure:",squash"` parallelscommon.PrlctlPostConfig `mapstructure:",squash"` parallelscommon.PrlctlVersionConfig `mapstructure:",squash"` - parallelscommon.ShutdownConfig `mapstructure:",squash"` + shutdowncommand.ShutdownConfig `mapstructure:",squash"` parallelscommon.SSHConfig `mapstructure:",squash"` parallelscommon.ToolsConfig `mapstructure:",squash"` // The size, in megabytes, of the hard disk to create diff --git a/builder/parallels/pvm/config.go b/builder/parallels/pvm/config.go index 46ba559af..aa1fe3e45 100644 --- a/builder/parallels/pvm/config.go +++ b/builder/parallels/pvm/config.go @@ -9,6 +9,7 @@ import ( parallelscommon "github.com/hashicorp/packer/builder/parallels/common" "github.com/hashicorp/packer/common" "github.com/hashicorp/packer/common/bootcommand" + "github.com/hashicorp/packer/common/shutdowncommand" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -23,7 +24,7 @@ type Config struct { parallelscommon.PrlctlPostConfig `mapstructure:",squash"` parallelscommon.PrlctlVersionConfig `mapstructure:",squash"` parallelscommon.SSHConfig `mapstructure:",squash"` - parallelscommon.ShutdownConfig `mapstructure:",squash"` + shutdowncommand.ShutdownConfig `mapstructure:",squash"` bootcommand.BootConfig `mapstructure:",squash"` parallelscommon.ToolsConfig `mapstructure:",squash"` // The path to a PVM directory that acts as the source diff --git a/builder/qemu/builder.go b/builder/qemu/builder.go index b36469f31..ea5857968 100644 --- a/builder/qemu/builder.go +++ b/builder/qemu/builder.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/packer/common" "github.com/hashicorp/packer/common/bootcommand" + "github.com/hashicorp/packer/common/shutdowncommand" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" @@ -91,12 +92,13 @@ type Builder struct { } type Config struct { - common.PackerConfig `mapstructure:",squash"` - common.HTTPConfig `mapstructure:",squash"` - common.ISOConfig `mapstructure:",squash"` - bootcommand.VNCConfig `mapstructure:",squash"` - Comm communicator.Config `mapstructure:",squash"` - common.FloppyConfig `mapstructure:",squash"` + common.PackerConfig `mapstructure:",squash"` + common.HTTPConfig `mapstructure:",squash"` + common.ISOConfig `mapstructure:",squash"` + bootcommand.VNCConfig `mapstructure:",squash"` + shutdowncommand.ShutdownConfig `mapstructure:",squash"` + Comm communicator.Config `mapstructure:",squash"` + common.FloppyConfig `mapstructure:",squash"` // Use iso from provided url. Qemu must support // curl block device. This defaults to `false`. ISOSkipCache bool `mapstructure:"iso_skip_cache" required:"false"` @@ -268,16 +270,6 @@ type Config struct { // some platforms. For example qemu-kvm, or qemu-system-i386 may be a // better choice for some systems. QemuBinary string `mapstructure:"qemu_binary" required:"false"` - // The command to use to gracefully shut down the - // machine once all the provisioning is done. By default this is an empty - // string, which tells Packer to just forcefully shut down the machine unless a - // shutdown command takes place inside script so this may safely be omitted. It - // is important to add a shutdown_command. By default Packer halts the virtual - // machine and the file system may not be sync'd. Thus, changes made in a - // provisioner might not be saved. If one or more scripts require a reboot it is - // suggested to leave this blank since reboots may fail and specify the final - // shutdown command in your last script. - ShutdownCommand string `mapstructure:"shutdown_command" required:"false"` // The minimum and // maximum port to use for the SSH port on the host machine which is forwarded // to the SSH port on the guest machine. Because Packer often runs in parallel, @@ -312,14 +304,8 @@ type Config struct { // TODO(mitchellh): deprecate RunOnce bool `mapstructure:"run_once"` - // The amount of time to wait after executing the - // shutdown_command for the virtual machine to actually shut down. If it - // doesn't shut down in this time, it is an error. By default, the timeout is - // 5m or five minutes. - RawShutdownTimeout string `mapstructure:"shutdown_timeout" required:"false"` - shutdownTimeout time.Duration `` - ctx interpolate.Context + ctx interpolate.Context } func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { @@ -340,6 +326,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { var errs *packer.MultiError warnings := make([]string, 0) + errs = packer.MultiErrorAppend(errs, b.config.ShutdownConfig.Prepare(&b.config.ctx)...) + if b.config.DiskSize == 0 { b.config.DiskSize = 40960 } @@ -508,16 +496,6 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { } } - if b.config.RawShutdownTimeout == "" { - b.config.RawShutdownTimeout = "5m" - } - - b.config.shutdownTimeout, err = time.ParseDuration(b.config.RawShutdownTimeout) - if err != nil { - errs = packer.MultiErrorAppend( - errs, fmt.Errorf("Failed parsing shutdown_timeout: %s", err)) - } - if b.config.SSHHostPortMin > b.config.SSHHostPortMax { errs = packer.MultiErrorAppend( errs, errors.New("ssh_host_port_min must be less than ssh_host_port_max")) diff --git a/builder/qemu/step_shutdown.go b/builder/qemu/step_shutdown.go index 804dc721d..b883e97e1 100644 --- a/builder/qemu/step_shutdown.go +++ b/builder/qemu/step_shutdown.go @@ -33,7 +33,7 @@ func (s *stepShutdown) Run(ctx context.Context, state multistep.StateBag) multis cancelCh := make(chan struct{}, 1) go func() { defer close(cancelCh) - <-time.After(config.shutdownTimeout) + <-time.After(config.ShutdownTimeout) }() ui.Say("Waiting for shutdown...") if ok := driver.WaitForShutdown(cancelCh); ok { @@ -63,10 +63,10 @@ func (s *stepShutdown) Run(ctx context.Context, state multistep.StateBag) multis cancelCh := make(chan struct{}, 1) go func() { defer close(cancelCh) - <-time.After(config.shutdownTimeout) + <-time.After(config.ShutdownTimeout) }() - log.Printf("Waiting max %s for shutdown to complete", config.shutdownTimeout) + log.Printf("Waiting max %s for shutdown to complete", config.ShutdownTimeout) if ok := driver.WaitForShutdown(cancelCh); !ok { err := errors.New("Timeout while waiting for machine to shut down.") state.Put("error", err) diff --git a/builder/vmware/common/shutdown_config.go b/builder/vmware/common/shutdown_config.go deleted file mode 100644 index fd5d665fe..000000000 --- a/builder/vmware/common/shutdown_config.go +++ /dev/null @@ -1,39 +0,0 @@ -//go:generate struct-markdown - -package common - -import ( - "fmt" - "time" - - "github.com/hashicorp/packer/template/interpolate" -) - -type ShutdownConfig struct { - // The command to use to gracefully shut down the - // machine once all the provisioning is done. By default this is an empty - // string, which tells Packer to just forcefully shut down the machine. - ShutdownCommand string `mapstructure:"shutdown_command" required:"false"` - // The amount of time to wait after executing the - // shutdown_command for the virtual machine to actually shut down. If it - // doesn't shut down in this time, it is an error. By default, the timeout is - // 5m or five minutes. - RawShutdownTimeout string `mapstructure:"shutdown_timeout" required:"false"` - - ShutdownTimeout time.Duration `` -} - -func (c *ShutdownConfig) Prepare(ctx *interpolate.Context) []error { - if c.RawShutdownTimeout == "" { - c.RawShutdownTimeout = "5m" - } - - var errs []error - var err error - c.ShutdownTimeout, err = time.ParseDuration(c.RawShutdownTimeout) - if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing shutdown_timeout: %s", err)) - } - - return errs -} diff --git a/builder/vmware/common/shutdown_config_test.go b/builder/vmware/common/shutdown_config_test.go deleted file mode 100644 index 5da613a19..000000000 --- a/builder/vmware/common/shutdown_config_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package common - -import ( - "testing" - "time" -) - -func testShutdownConfig() *ShutdownConfig { - return &ShutdownConfig{} -} - -func TestShutdownConfigPrepare_ShutdownCommand(t *testing.T) { - var c *ShutdownConfig - var errs []error - - c = testShutdownConfig() - errs = c.Prepare(testConfigTemplate(t)) - if len(errs) > 0 { - t.Fatalf("err: %#v", errs) - } -} - -func TestShutdownConfigPrepare_ShutdownTimeout(t *testing.T) { - var c *ShutdownConfig - var errs []error - - // Test with a bad value - c = testShutdownConfig() - c.RawShutdownTimeout = "this is not good" - errs = c.Prepare(testConfigTemplate(t)) - if len(errs) == 0 { - t.Fatalf("should have error") - } - - // Test with a good one - c = testShutdownConfig() - c.RawShutdownTimeout = "5s" - errs = c.Prepare(testConfigTemplate(t)) - if len(errs) > 0 { - t.Fatalf("err: %#v", errs) - } - if c.ShutdownTimeout != 5*time.Second { - t.Fatalf("bad: %s", c.ShutdownTimeout) - } -} diff --git a/builder/vmware/iso/config.go b/builder/vmware/iso/config.go index f11521e26..b07720d9b 100644 --- a/builder/vmware/iso/config.go +++ b/builder/vmware/iso/config.go @@ -11,26 +11,27 @@ import ( vmwcommon "github.com/hashicorp/packer/builder/vmware/common" "github.com/hashicorp/packer/common" "github.com/hashicorp/packer/common/bootcommand" + "github.com/hashicorp/packer/common/shutdowncommand" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" ) type Config struct { - common.PackerConfig `mapstructure:",squash"` - common.HTTPConfig `mapstructure:",squash"` - common.ISOConfig `mapstructure:",squash"` - common.FloppyConfig `mapstructure:",squash"` - bootcommand.VNCConfig `mapstructure:",squash"` - vmwcommon.DriverConfig `mapstructure:",squash"` - vmwcommon.HWConfig `mapstructure:",squash"` - vmwcommon.OutputConfig `mapstructure:",squash"` - vmwcommon.RunConfig `mapstructure:",squash"` - vmwcommon.ShutdownConfig `mapstructure:",squash"` - vmwcommon.SSHConfig `mapstructure:",squash"` - vmwcommon.ToolsConfig `mapstructure:",squash"` - vmwcommon.VMXConfig `mapstructure:",squash"` - vmwcommon.ExportConfig `mapstructure:",squash"` + common.PackerConfig `mapstructure:",squash"` + common.HTTPConfig `mapstructure:",squash"` + common.ISOConfig `mapstructure:",squash"` + common.FloppyConfig `mapstructure:",squash"` + bootcommand.VNCConfig `mapstructure:",squash"` + vmwcommon.DriverConfig `mapstructure:",squash"` + vmwcommon.HWConfig `mapstructure:",squash"` + vmwcommon.OutputConfig `mapstructure:",squash"` + vmwcommon.RunConfig `mapstructure:",squash"` + shutdowncommand.ShutdownConfig `mapstructure:",squash"` + vmwcommon.SSHConfig `mapstructure:",squash"` + vmwcommon.ToolsConfig `mapstructure:",squash"` + vmwcommon.VMXConfig `mapstructure:",squash"` + vmwcommon.ExportConfig `mapstructure:",squash"` // The size(s) of any additional // hard disks for the VM in megabytes. If this is not specified then the VM // will only contain a primary hard disk. The builder uses expandable, not diff --git a/builder/vmware/vmx/config.go b/builder/vmware/vmx/config.go index 17c7bb3f4..c5a42b0e5 100644 --- a/builder/vmware/vmx/config.go +++ b/builder/vmware/vmx/config.go @@ -9,6 +9,7 @@ import ( vmwcommon "github.com/hashicorp/packer/builder/vmware/common" "github.com/hashicorp/packer/common" "github.com/hashicorp/packer/common/bootcommand" + "github.com/hashicorp/packer/common/shutdowncommand" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -16,18 +17,18 @@ import ( // Config is the configuration structure for the builder. type Config struct { - common.PackerConfig `mapstructure:",squash"` - common.HTTPConfig `mapstructure:",squash"` - common.FloppyConfig `mapstructure:",squash"` - bootcommand.VNCConfig `mapstructure:",squash"` - vmwcommon.DriverConfig `mapstructure:",squash"` - vmwcommon.OutputConfig `mapstructure:",squash"` - vmwcommon.RunConfig `mapstructure:",squash"` - vmwcommon.ShutdownConfig `mapstructure:",squash"` - vmwcommon.SSHConfig `mapstructure:",squash"` - vmwcommon.ToolsConfig `mapstructure:",squash"` - vmwcommon.VMXConfig `mapstructure:",squash"` - vmwcommon.ExportConfig `mapstructure:",squash"` + common.PackerConfig `mapstructure:",squash"` + common.HTTPConfig `mapstructure:",squash"` + common.FloppyConfig `mapstructure:",squash"` + bootcommand.VNCConfig `mapstructure:",squash"` + vmwcommon.DriverConfig `mapstructure:",squash"` + vmwcommon.OutputConfig `mapstructure:",squash"` + vmwcommon.RunConfig `mapstructure:",squash"` + shutdowncommand.ShutdownConfig `mapstructure:",squash"` + vmwcommon.SSHConfig `mapstructure:",squash"` + vmwcommon.ToolsConfig `mapstructure:",squash"` + vmwcommon.VMXConfig `mapstructure:",squash"` + vmwcommon.ExportConfig `mapstructure:",squash"` // By default Packer creates a 'full' clone of // the virtual machine specified in source_path. The resultant virtual // machine is fully independant from the parent it was cloned from. diff --git a/builder/hyperv/common/shutdown_config.go b/common/shutdowncommand/config.go similarity index 96% rename from builder/hyperv/common/shutdown_config.go rename to common/shutdowncommand/config.go index ef028ee51..a3b78ac8d 100644 --- a/builder/hyperv/common/shutdown_config.go +++ b/common/shutdowncommand/config.go @@ -1,6 +1,4 @@ -//go:generate struct-markdown - -package common +package shutdowncommand import ( "fmt" diff --git a/builder/hyperv/common/shutdown_config_test.go b/common/shutdowncommand/config_test.go similarity index 97% rename from builder/hyperv/common/shutdown_config_test.go rename to common/shutdowncommand/config_test.go index 5da613a19..2af7b8737 100644 --- a/builder/hyperv/common/shutdown_config_test.go +++ b/common/shutdowncommand/config_test.go @@ -1,4 +1,4 @@ -package common +package shutdowncommand import ( "testing"