From 55ae8038523db36e2d637763d7f75d1383b51076 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Sun, 9 Feb 2020 13:08:22 -0600 Subject: [PATCH 01/17] Add Hyper-V support for Gen-1 boot order with ISO --- builder/hyperv/common/config.go | 7 +++++++ builder/hyperv/common/driver.go | 2 +- builder/hyperv/common/driver_ps_4.go | 4 ++-- builder/hyperv/common/step_mount_dvddrive.go | 3 ++- builder/hyperv/iso/builder.go | 3 ++- builder/hyperv/vmcx/builder.go | 3 ++- common/powershell/hyperv/hyperv.go | 12 ++++++++++-- 7 files changed, 26 insertions(+), 8 deletions(-) diff --git a/builder/hyperv/common/config.go b/builder/hyperv/common/config.go index 152868e1d..1cba3e393 100644 --- a/builder/hyperv/common/config.go +++ b/builder/hyperv/common/config.go @@ -148,6 +148,13 @@ type CommonConfig struct { // built. When this value is set to true, the machine will start without a // console. Headless bool `mapstructure:"headless" required:"false"` + // Over time the Hyper-V builder has been modified to change the original + // boot order that is used when an ISO is mounted. Hyper-V's default is to + // boot from the CD first, the original Hyper-V builder included code to + // codify this setting when the primary ISO is mounted, that code was eventually + // modified to place the IDE adapter before the the CD (only in generation 1). + // Setting this value to true, forces the original method of operation. + LegacyGen1BootOrder bool `mapstructure:"legacy_gen1_boot_order" required:"false"` } func (c *CommonConfig) Prepare(ctx *interpolate.Context, pc *common.PackerConfig) ([]error, []string) { diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index 29b282611..545596bee 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -111,7 +111,7 @@ type Driver interface { MountDvdDrive(string, string, uint, uint) error - SetBootDvdDrive(string, uint, uint, uint) error + SetBootDvdDrive(string, uint, uint, uint, bool) error UnmountDvdDrive(string, uint, uint) error diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index d365ae2e7..5b072a72e 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -263,8 +263,8 @@ func (d *HypervPS4Driver) MountDvdDrive(vmName string, path string, controllerNu } func (d *HypervPS4Driver) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, - generation uint) error { - return hyperv.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, generation) + generation uint, legacyGen1BootOrder bool) error { + return hyperv.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, generation, legacyGen1BootOrder) } func (d *HypervPS4Driver) UnmountDvdDrive(vmName string, controllerNumber uint, controllerLocation uint) error { diff --git a/builder/hyperv/common/step_mount_dvddrive.go b/builder/hyperv/common/step_mount_dvddrive.go index 88b084c6c..3f54bcbcb 100644 --- a/builder/hyperv/common/step_mount_dvddrive.go +++ b/builder/hyperv/common/step_mount_dvddrive.go @@ -13,6 +13,7 @@ import ( type StepMountDvdDrive struct { Generation uint + LegacyGen1BootOrder bool } func (s *StepMountDvdDrive) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { @@ -58,7 +59,7 @@ func (s *StepMountDvdDrive) Run(ctx context.Context, state multistep.StateBag) m state.Put("os.dvd.properties", dvdControllerProperties) ui.Say(fmt.Sprintf("Setting boot drive to os dvd drive %s ...", isoPath)) - err = driver.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, s.Generation) + err = driver.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, s.Generation, s.LegacyGen1BootOrder) if err != nil { err := fmt.Errorf(errorMsg, err) state.Put("error", err) diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 2990ea9c6..a39da7b7b 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -242,7 +242,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack &hypervcommon.StepEnableIntegrationService{}, &hypervcommon.StepMountDvdDrive{ - Generation: b.config.Generation, + Generation: b.config.Generation, + LegacyGen1BootOrder: b.config.LegacyGen1BootOrder, }, &hypervcommon.StepMountFloppydrive{ Generation: b.config.Generation, diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index d07bf8b02..f151354a6 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -282,7 +282,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack &hypervcommon.StepEnableIntegrationService{}, &hypervcommon.StepMountDvdDrive{ - Generation: b.config.Generation, + Generation: b.config.Generation, + LegacyGen1BootOrder: b.config.LegacyGen1BootOrder, }, &hypervcommon.StepMountFloppydrive{ Generation: b.config.Generation, diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index a80624609..6aa8726c0 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -156,13 +156,21 @@ Hyper-V\Set-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -Cont return err } -func SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, generation uint) error { +func SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, generation uint, legacyGen1BootOrder bool) error { if generation < 2 { - script := ` + var script string + if (legacyGen1BootOrder) { + script = ` +param([string]$vmName) +Hyper-V\Set-VMBios -VMName $vmName -StartupOrder @("CD","IDE","LegacyNetworkAdapter","Floppy") +` + } else { + script = ` param([string]$vmName) Hyper-V\Set-VMBios -VMName $vmName -StartupOrder @("IDE","CD","LegacyNetworkAdapter","Floppy") ` + } var ps powershell.PowerShellCmd err := ps.Run(script, vmName) return err From 71527325975d2ed43c7851be7bf9c83a0f0e2c9d Mon Sep 17 00:00:00 2001 From: William Brooks Date: Sun, 9 Feb 2020 14:26:46 -0600 Subject: [PATCH 02/17] changing SetBootDvdDrive messaging to be truthful --- builder/hyperv/common/step_mount_dvddrive.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/builder/hyperv/common/step_mount_dvddrive.go b/builder/hyperv/common/step_mount_dvddrive.go index 3f54bcbcb..1efb4eeaa 100644 --- a/builder/hyperv/common/step_mount_dvddrive.go +++ b/builder/hyperv/common/step_mount_dvddrive.go @@ -58,7 +58,12 @@ func (s *StepMountDvdDrive) Run(ctx context.Context, state multistep.StateBag) m state.Put("os.dvd.properties", dvdControllerProperties) - ui.Say(fmt.Sprintf("Setting boot drive to os dvd drive %s ...", isoPath)) + if ((s.Generation == 1) && (!s.LegacyGen1BootOrder)) { + ui.Say("Setting boot drive to IDE and then CD drive. Use legacy_gen1_boot_order to override.") + } else { + ui.Say(fmt.Sprintf("Setting boot drive to os dvd drive %s ...", isoPath)) + } + err = driver.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, s.Generation, s.LegacyGen1BootOrder) if err != nil { err := fmt.Errorf(errorMsg, err) From 31622b50ac9d52b2699f91fbbc31463b9b2a0485 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Sun, 9 Feb 2020 15:22:36 -0600 Subject: [PATCH 03/17] applied fmt --- builder/hyperv/common/config.go | 2 +- builder/hyperv/common/step_mount_dvddrive.go | 4 ++-- common/powershell/hyperv/hyperv.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/builder/hyperv/common/config.go b/builder/hyperv/common/config.go index 1cba3e393..142d399ef 100644 --- a/builder/hyperv/common/config.go +++ b/builder/hyperv/common/config.go @@ -150,7 +150,7 @@ type CommonConfig struct { Headless bool `mapstructure:"headless" required:"false"` // Over time the Hyper-V builder has been modified to change the original // boot order that is used when an ISO is mounted. Hyper-V's default is to - // boot from the CD first, the original Hyper-V builder included code to + // boot from the CD first, the original Hyper-V builder included code to // codify this setting when the primary ISO is mounted, that code was eventually // modified to place the IDE adapter before the the CD (only in generation 1). // Setting this value to true, forces the original method of operation. diff --git a/builder/hyperv/common/step_mount_dvddrive.go b/builder/hyperv/common/step_mount_dvddrive.go index 1efb4eeaa..777a288b6 100644 --- a/builder/hyperv/common/step_mount_dvddrive.go +++ b/builder/hyperv/common/step_mount_dvddrive.go @@ -12,7 +12,7 @@ import ( ) type StepMountDvdDrive struct { - Generation uint + Generation uint LegacyGen1BootOrder bool } @@ -58,7 +58,7 @@ func (s *StepMountDvdDrive) Run(ctx context.Context, state multistep.StateBag) m state.Put("os.dvd.properties", dvdControllerProperties) - if ((s.Generation == 1) && (!s.LegacyGen1BootOrder)) { + if (s.Generation == 1) && (!s.LegacyGen1BootOrder) { ui.Say("Setting boot drive to IDE and then CD drive. Use legacy_gen1_boot_order to override.") } else { ui.Say(fmt.Sprintf("Setting boot drive to os dvd drive %s ...", isoPath)) diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index 6aa8726c0..a60322bdf 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -160,7 +160,7 @@ func SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation ui if generation < 2 { var script string - if (legacyGen1BootOrder) { + if legacyGen1BootOrder { script = ` param([string]$vmName) Hyper-V\Set-VMBios -VMName $vmName -StartupOrder @("CD","IDE","LegacyNetworkAdapter","Floppy") From 9891e75f75696d514baff262e8aacd9eb25a8051 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Sun, 9 Feb 2020 15:39:35 -0600 Subject: [PATCH 04/17] update hyper-v driver_mock.go --- builder/hyperv/common/driver_mock.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 4005b35e2..4320f932a 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -229,12 +229,13 @@ type DriverMock struct { MountDvdDrive_ControllerLocation uint MountDvdDrive_Err error - SetBootDvdDrive_Called bool - SetBootDvdDrive_VmName string - SetBootDvdDrive_ControllerNumber uint - SetBootDvdDrive_ControllerLocation uint - SetBootDvdDrive_Generation uint - SetBootDvdDrive_Err error + SetBootDvdDrive_Called bool + SetBootDvdDrive_VmName string + SetBootDvdDrive_ControllerNumber uint + SetBootDvdDrive_ControllerLocation uint + SetBootDvdDrive_Generation uint + SetBootDvdDrive_LegacyGen1BootOrder bool + SetBootDvdDrive_Err error UnmountDvdDrive_Called bool UnmountDvdDrive_VmName string @@ -566,12 +567,13 @@ func (d *DriverMock) MountDvdDrive(vmName string, path string, controllerNumber } func (d *DriverMock) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, - generation uint) error { + generation uint, legacyGen1BootOrder bool) error { d.SetBootDvdDrive_Called = true d.SetBootDvdDrive_VmName = vmName d.SetBootDvdDrive_ControllerNumber = controllerNumber d.SetBootDvdDrive_ControllerLocation = controllerLocation d.SetBootDvdDrive_Generation = generation + d.SetBootDvdDrive_LegacyGen1BootOrder = legacyGen1BootOrder return d.SetBootDvdDrive_Err } From 61f5f867ebd7492ee30a37b6fc2e68b09a4b5c80 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Sun, 9 Feb 2020 16:41:07 -0600 Subject: [PATCH 05/17] make generate --- builder/hyperv/iso/builder.hcl2spec.go | 2 ++ builder/hyperv/vmcx/builder.hcl2spec.go | 2 ++ .../hyperv/common/_CommonConfig-not-required.html.md | 7 +++++++ 3 files changed, 11 insertions(+) diff --git a/builder/hyperv/iso/builder.hcl2spec.go b/builder/hyperv/iso/builder.hcl2spec.go index d06b4b3af..5753a3f41 100644 --- a/builder/hyperv/iso/builder.hcl2spec.go +++ b/builder/hyperv/iso/builder.hcl2spec.go @@ -97,6 +97,7 @@ type FlatConfig struct { SkipCompaction *bool `mapstructure:"skip_compaction" required:"false" cty:"skip_compaction"` SkipExport *bool `mapstructure:"skip_export" required:"false" cty:"skip_export"` Headless *bool `mapstructure:"headless" required:"false" cty:"headless"` + LegacyGen1BootOrder *bool `mapstructure:"legacy_gen1_boot_order" required:"false" cty:"legacy_gen1_boot_order"` ShutdownCommand *string `mapstructure:"shutdown_command" required:"false" cty:"shutdown_command"` ShutdownTimeout *string `mapstructure:"shutdown_timeout" required:"false" cty:"shutdown_timeout"` DiskSize *uint `mapstructure:"disk_size" required:"false" cty:"disk_size"` @@ -205,6 +206,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "skip_compaction": &hcldec.AttrSpec{Name: "skip_compaction", Type: cty.Bool, Required: false}, "skip_export": &hcldec.AttrSpec{Name: "skip_export", Type: cty.Bool, Required: false}, "headless": &hcldec.AttrSpec{Name: "headless", Type: cty.Bool, Required: false}, + "legacy_gen1_boot_order": &hcldec.AttrSpec{Name: "legacy_gen1_boot_order", Type: cty.Bool, Required: false}, "shutdown_command": &hcldec.AttrSpec{Name: "shutdown_command", Type: cty.String, Required: false}, "shutdown_timeout": &hcldec.AttrSpec{Name: "shutdown_timeout", Type: cty.String, Required: false}, "disk_size": &hcldec.AttrSpec{Name: "disk_size", Type: cty.Number, Required: false}, diff --git a/builder/hyperv/vmcx/builder.hcl2spec.go b/builder/hyperv/vmcx/builder.hcl2spec.go index 628436bf0..e2c028e73 100644 --- a/builder/hyperv/vmcx/builder.hcl2spec.go +++ b/builder/hyperv/vmcx/builder.hcl2spec.go @@ -97,6 +97,7 @@ type FlatConfig struct { SkipCompaction *bool `mapstructure:"skip_compaction" required:"false" cty:"skip_compaction"` SkipExport *bool `mapstructure:"skip_export" required:"false" cty:"skip_export"` Headless *bool `mapstructure:"headless" required:"false" cty:"headless"` + LegacyGen1BootOrder *bool `mapstructure:"legacy_gen1_boot_order" required:"false" cty:"legacy_gen1_boot_order"` ShutdownCommand *string `mapstructure:"shutdown_command" required:"false" cty:"shutdown_command"` ShutdownTimeout *string `mapstructure:"shutdown_timeout" required:"false" cty:"shutdown_timeout"` CloneFromVMCXPath *string `mapstructure:"clone_from_vmcx_path" cty:"clone_from_vmcx_path"` @@ -207,6 +208,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "skip_compaction": &hcldec.AttrSpec{Name: "skip_compaction", Type: cty.Bool, Required: false}, "skip_export": &hcldec.AttrSpec{Name: "skip_export", Type: cty.Bool, Required: false}, "headless": &hcldec.AttrSpec{Name: "headless", Type: cty.Bool, Required: false}, + "legacy_gen1_boot_order": &hcldec.AttrSpec{Name: "legacy_gen1_boot_order", Type: cty.Bool, Required: false}, "shutdown_command": &hcldec.AttrSpec{Name: "shutdown_command", Type: cty.String, Required: false}, "shutdown_timeout": &hcldec.AttrSpec{Name: "shutdown_timeout", Type: cty.String, Required: false}, "clone_from_vmcx_path": &hcldec.AttrSpec{Name: "clone_from_vmcx_path", Type: cty.String, Required: false}, diff --git a/website/source/partials/builder/hyperv/common/_CommonConfig-not-required.html.md b/website/source/partials/builder/hyperv/common/_CommonConfig-not-required.html.md index de742a737..e1ebf434b 100644 --- a/website/source/partials/builder/hyperv/common/_CommonConfig-not-required.html.md +++ b/website/source/partials/builder/hyperv/common/_CommonConfig-not-required.html.md @@ -109,4 +109,11 @@ machines by launching a GUI that shows the console of the machine being built. When this value is set to true, the machine will start without a console. + +- `legacy_gen1_boot_order` (bool) - Over time the Hyper-V builder has been modified to change the original + boot order that is used when an ISO is mounted. Hyper-V's default is to + boot from the CD first, the original Hyper-V builder included code to + codify this setting when the primary ISO is mounted, that code was eventually + modified to place the IDE adapter before the the CD (only in generation 1). + Setting this value to true, forces the original method of operation. \ No newline at end of file From d7300f46357085aac54098b45a22438a3ba407ba Mon Sep 17 00:00:00 2001 From: William Brooks Date: Fri, 21 Feb 2020 01:01:09 -0600 Subject: [PATCH 06/17] Remove legacy_boot and replace with first_boot_device (initial) --- builder/hyperv/common/config.go | 22 ++- builder/hyperv/common/driver.go | 4 +- builder/hyperv/common/driver_ps_4.go | 9 +- builder/hyperv/common/step_mount_dvddrive.go | 30 ++-- .../common/step_set_first_boot_device.go | 140 ++++++++++++++++++ builder/hyperv/iso/builder.go | 7 +- builder/hyperv/vmcx/builder.go | 7 +- common/powershell/hyperv/hyperv.go | 62 ++++++-- 8 files changed, 247 insertions(+), 34 deletions(-) create mode 100644 builder/hyperv/common/step_set_first_boot_device.go diff --git a/builder/hyperv/common/config.go b/builder/hyperv/common/config.go index 142d399ef..949266454 100644 --- a/builder/hyperv/common/config.go +++ b/builder/hyperv/common/config.go @@ -148,13 +148,21 @@ type CommonConfig struct { // built. When this value is set to true, the machine will start without a // console. Headless bool `mapstructure:"headless" required:"false"` - // Over time the Hyper-V builder has been modified to change the original - // boot order that is used when an ISO is mounted. Hyper-V's default is to - // boot from the CD first, the original Hyper-V builder included code to - // codify this setting when the primary ISO is mounted, that code was eventually - // modified to place the IDE adapter before the the CD (only in generation 1). - // Setting this value to true, forces the original method of operation. - LegacyGen1BootOrder bool `mapstructure:"legacy_gen1_boot_order" required:"false"` + // When configured, determines the device or device type that is given preferential + // treatment when choosing a boot device. + // + // For Generation 1: + // - `IDE` + // - `CD` *or* `DVD` + // - `Floppy` + // - `NET` + // + // For Generation 2: + // - `IDE:x:y` + // - `SCSI:x:y` + // - `CD` *or* `DVD` + // - `NET` + FirstBootDevice string `mapstructure:"first_boot_device" required:"false"` } func (c *CommonConfig) Prepare(ctx *interpolate.Context, pc *common.PackerConfig) ([]error, []string) { diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index 545596bee..9731d0572 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -111,7 +111,9 @@ type Driver interface { MountDvdDrive(string, string, uint, uint) error - SetBootDvdDrive(string, uint, uint, uint, bool) error + SetBootDvdDrive(string, uint, uint, uint) error + + SetFirstBootDevice(string, string, uint, uint, uint) error UnmountDvdDrive(string, uint, uint) error diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index 5b072a72e..c1738830d 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -263,8 +263,13 @@ func (d *HypervPS4Driver) MountDvdDrive(vmName string, path string, controllerNu } func (d *HypervPS4Driver) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, - generation uint, legacyGen1BootOrder bool) error { - return hyperv.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, generation, legacyGen1BootOrder) + generation uint) error { + return hyperv.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, generation) +} + +func (d *HypervPS4Driver) SetFirstBootDevice(vmName string, controllerType string, controllerNumber uint, + controllerLocation uint, generation uint) error { + return hyperv.SetFirstBootDevice(vmName, controllerType, controllerNumber, controllerLocation, generation) } func (d *HypervPS4Driver) UnmountDvdDrive(vmName string, controllerNumber uint, controllerLocation uint) error { diff --git a/builder/hyperv/common/step_mount_dvddrive.go b/builder/hyperv/common/step_mount_dvddrive.go index 777a288b6..faa2a49c7 100644 --- a/builder/hyperv/common/step_mount_dvddrive.go +++ b/builder/hyperv/common/step_mount_dvddrive.go @@ -13,7 +13,7 @@ import ( type StepMountDvdDrive struct { Generation uint - LegacyGen1BootOrder bool + FirstBootDevice string } func (s *StepMountDvdDrive) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { @@ -58,18 +58,24 @@ func (s *StepMountDvdDrive) Run(ctx context.Context, state multistep.StateBag) m state.Put("os.dvd.properties", dvdControllerProperties) - if (s.Generation == 1) && (!s.LegacyGen1BootOrder) { - ui.Say("Setting boot drive to IDE and then CD drive. Use legacy_gen1_boot_order to override.") - } else { - ui.Say(fmt.Sprintf("Setting boot drive to os dvd drive %s ...", isoPath)) - } + // the "first_boot_device" setting has precedence over the legacy boot order + // configuration, but only if its been assigned a value. + + if s.FirstBootDevice == "" { + + if s.Generation > 1 { + // only print this message for Gen2, it's not a true statement for Gen1 VMs + ui.Say(fmt.Sprintf("Setting boot drive to os dvd drive %s ...", isoPath)) + } + + err = driver.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, s.Generation) + if err != nil { + err := fmt.Errorf(errorMsg, err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } - err = driver.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, s.Generation, s.LegacyGen1BootOrder) - if err != nil { - err := fmt.Errorf(errorMsg, err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt } ui.Say(fmt.Sprintf("Mounting os dvd drive %s ...", isoPath)) diff --git a/builder/hyperv/common/step_set_first_boot_device.go b/builder/hyperv/common/step_set_first_boot_device.go new file mode 100644 index 000000000..f17e9a178 --- /dev/null +++ b/builder/hyperv/common/step_set_first_boot_device.go @@ -0,0 +1,140 @@ +package common + +import ( + "context" + "fmt" + "regexp" + "strconv" + "strings" + + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +type StepSetFirstBootDevice struct { + Generation uint + FirstBootDevice string +} + +func ParseBootDeviceIdentifier(deviceIdentifier string, generation uint) (string, uint, uint, error) { + + captureExpression := "^(FLOPPY|IDE|NET)|(CD|DVD)$" + if generation > 1 { + captureExpression = "^((IDE|SCSI):(\\d+):(\\d+))|(DVD|CD)|(NET)$" + } + + r, err := regexp.Compile(captureExpression) + if err != nil { + return "", 0, 0, err + } + + // match against the appropriate set of values.. we force to uppercase to ensure that + // all devices are always in the same case + + identifierMatches := r.FindStringSubmatch(strings.ToUpper(deviceIdentifier)) + if identifierMatches == nil { + return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device or device group identifier.", deviceIdentifier) + } + + switch { + + // CD or DVD are always returned as "CD" + case ((generation == 1) && (identifierMatches[2] != "")) || ((generation > 1) && (identifierMatches[5] != "")): + return "CD", 0, 0, nil + + // generation 1 only has FLOPPY, IDE or NET remaining.. + case (generation == 1): + return identifierMatches[0], 0, 0, nil + + // generation 2, check for IDE or SCSI and parse location and number + case (identifierMatches[2] != ""): + { + + var controllerLocation int64 + var controllerNumber int64 + + // NOTE: controllerNumber and controllerLocation cannot be negative, the regex expression + // would not have matched if either number was signed + + controllerNumber, err = strconv.ParseInt(identifierMatches[3], 10, 8) + if err == nil { + + controllerLocation, err = strconv.ParseInt(identifierMatches[4], 10, 8) + if err == nil { + + return identifierMatches[2], uint(controllerNumber), uint(controllerLocation), nil + + } + + } + + return "", 0, 0, err + + } + + // only "NET" left on generation 2 + default: + return "NET", 0, 0, nil + + } + +} + +func (s *StepSetFirstBootDevice) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + + driver := state.Get("driver").(Driver) + ui := state.Get("ui").(packer.Ui) + vmName := state.Get("vmName").(string) + + if s.FirstBootDevice != "" { + + controllerType, controllerNumber, controllerLocation, err := ParseBootDeviceIdentifier(s.FirstBootDevice, s.Generation) + if err == nil { + + switch { + + case controllerType == "CD": + { + // the "DVD" controller is special, we only apply the setting if we actually mounted + // an ISO and only if that was mounted as the "IsoUrl" not a secondary ISO. + + dvdControllerState := state.Get("os.dvd.properties") + if dvdControllerState == nil { + + ui.Say("First Boot Device is DVD, but no primary ISO mounted. Ignoring.") + return multistep.ActionContinue + + } + + ui.Say(fmt.Sprintf("Setting boot device to %q", s.FirstBootDevice)) + dvdController := dvdControllerState.(DvdControllerProperties) + err = driver.SetFirstBootDevice(vmName, controllerType, dvdController.ControllerNumber, dvdController.ControllerLocation, s.Generation) + + } + + default: + { + // anything else, we just pass as is.. + ui.Say(fmt.Sprintf("Setting boot device to %q", s.FirstBootDevice)) + err = driver.SetFirstBootDevice(vmName, controllerType, controllerNumber, controllerLocation, s.Generation) + } + } + + } + + if err != nil { + err := fmt.Errorf("Error setting first boot device: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + + } + + } + + return multistep.ActionContinue +} + +func (s *StepSetFirstBootDevice) Cleanup(state multistep.StateBag) { + // do nothing +} diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index a39da7b7b..5735cbbfe 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -243,7 +243,7 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack &hypervcommon.StepMountDvdDrive{ Generation: b.config.Generation, - LegacyGen1BootOrder: b.config.LegacyGen1BootOrder, + FirstBootDevice: b.config.FirstBootDevice, }, &hypervcommon.StepMountFloppydrive{ Generation: b.config.Generation, @@ -265,6 +265,11 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack SwitchVlanId: b.config.SwitchVlanId, }, + &hypervcommon.StepSetFirstBootDevice{ + Generation: b.config.Generation, + FirstBootDevice: b.config.FirstBootDevice, + }, + &hypervcommon.StepRun{ Headless: b.config.Headless, SwitchName: b.config.SwitchName, diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index f151354a6..6eed0a3c3 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -283,7 +283,7 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack &hypervcommon.StepMountDvdDrive{ Generation: b.config.Generation, - LegacyGen1BootOrder: b.config.LegacyGen1BootOrder, + FirstBootDevice: b.config.FirstBootDevice, }, &hypervcommon.StepMountFloppydrive{ Generation: b.config.Generation, @@ -305,6 +305,11 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack SwitchVlanId: b.config.SwitchVlanId, }, + &hypervcommon.StepSetFirstBootDevice{ + Generation: b.config.Generation, + FirstBootDevice: b.config.FirstBootDevice, + }, + &hypervcommon.StepRun{ Headless: b.config.Headless, SwitchName: b.config.SwitchName, diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index a60322bdf..bebc4119f 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -156,21 +156,13 @@ Hyper-V\Set-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -Cont return err } -func SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, generation uint, legacyGen1BootOrder bool) error { +func SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, generation uint) error { if generation < 2 { - var script string - if legacyGen1BootOrder { - script = ` -param([string]$vmName) -Hyper-V\Set-VMBios -VMName $vmName -StartupOrder @("CD","IDE","LegacyNetworkAdapter","Floppy") -` - } else { - script = ` + script := ` param([string]$vmName) Hyper-V\Set-VMBios -VMName $vmName -StartupOrder @("IDE","CD","LegacyNetworkAdapter","Floppy") ` - } var ps powershell.PowerShellCmd err := ps.Run(script, vmName) return err @@ -188,6 +180,56 @@ Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $vmDvdDrive -ErrorAction } } +func SetFirstBootDeviceGen1(vmName string, controllerType string) error { + + // for Generation 1 VMs, we read the value of the VM's boot order, strip the value specified in + // controllerType and insert that value back at the beginning of the list. + // + // controllerType must be 'NET', 'DVD', 'IDE' or 'FLOPPY' (case sensitive) + // The 'NET' value is always replaced with 'LegacyNetworkAdapter' + + if (controllerType == "NET") { + controllerType = "LegacyNetworkAdapter" + } + + script := ` +param([string] $vmName, [string] $controllerType) + $vmBootOrder = Hyper-V\Get-VMBios -VMName $vmName | Select-Object -ExpandProperty StartupOrder | Where-Object { $_ -ne $controllerType } + Hyper-V\Set-VMBios -VMName $vmName -StartupOrder (@($controllerType) + $vmBootOrder) +` + + var ps powershell.PowerShellCmd + err := ps.Run(script, vmName, controllerType) + return err +} + +func SetFirstBootDeviceGen2(vmName string, controllerType string, controllerNumber uint, controllerLocation uint) error { + + + +// script := ` +// param([string]$vmName,[int]$controllerNumber,[int]$controllerLocation) +// $vmDvdDrive = Hyper-V\Get-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -ControllerLocation $controllerLocation +// if (!$vmDvdDrive) {throw 'unable to find dvd drive'} +// Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $vmDvdDrive -ErrorAction SilentlyContinue +// ` + +// script := ` +// param([string] $vmName, [string] $controllerType, ) +//` + + return nil +} + +func SetFirstBootDevice(vmName string, controllerType string, controllerNumber uint, controllerLocation uint, generation uint) error { + + if generation == 1 { + return SetFirstBootDeviceGen1(vmName, controllerType) + } else { + return SetFirstBootDeviceGen2(vmName, controllerType, controllerNumber, controllerLocation) + } +} + func DeleteDvdDrive(vmName string, controllerNumber uint, controllerLocation uint) error { var script = ` param([string]$vmName,[int]$controllerNumber,[int]$controllerLocation) From 5e1e4ec7016e38fe69f5892d2da00d30bc0c9288 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Fri, 21 Feb 2020 01:07:58 -0600 Subject: [PATCH 07/17] fmt && generate --- builder/hyperv/common/config.go | 4 ++-- builder/hyperv/common/step_mount_dvddrive.go | 4 ++-- builder/hyperv/iso/builder.go | 4 ++-- builder/hyperv/iso/builder.hcl2spec.go | 4 ++-- builder/hyperv/vmcx/builder.go | 4 ++-- builder/hyperv/vmcx/builder.hcl2spec.go | 4 ++-- common/powershell/hyperv/hyperv.go | 22 +++++++++---------- .../common/_CommonConfig-not-required.html.md | 20 ++++++++++++----- 8 files changed, 36 insertions(+), 30 deletions(-) diff --git a/builder/hyperv/common/config.go b/builder/hyperv/common/config.go index 949266454..c97d0ffcd 100644 --- a/builder/hyperv/common/config.go +++ b/builder/hyperv/common/config.go @@ -150,9 +150,9 @@ type CommonConfig struct { Headless bool `mapstructure:"headless" required:"false"` // When configured, determines the device or device type that is given preferential // treatment when choosing a boot device. - // + // // For Generation 1: - // - `IDE` + // - `IDE` // - `CD` *or* `DVD` // - `Floppy` // - `NET` diff --git a/builder/hyperv/common/step_mount_dvddrive.go b/builder/hyperv/common/step_mount_dvddrive.go index faa2a49c7..f0eeaba61 100644 --- a/builder/hyperv/common/step_mount_dvddrive.go +++ b/builder/hyperv/common/step_mount_dvddrive.go @@ -12,8 +12,8 @@ import ( ) type StepMountDvdDrive struct { - Generation uint - FirstBootDevice string + Generation uint + FirstBootDevice string } func (s *StepMountDvdDrive) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 5735cbbfe..31905e01f 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -242,8 +242,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack &hypervcommon.StepEnableIntegrationService{}, &hypervcommon.StepMountDvdDrive{ - Generation: b.config.Generation, - FirstBootDevice: b.config.FirstBootDevice, + Generation: b.config.Generation, + FirstBootDevice: b.config.FirstBootDevice, }, &hypervcommon.StepMountFloppydrive{ Generation: b.config.Generation, diff --git a/builder/hyperv/iso/builder.hcl2spec.go b/builder/hyperv/iso/builder.hcl2spec.go index 5753a3f41..a7f2b18aa 100644 --- a/builder/hyperv/iso/builder.hcl2spec.go +++ b/builder/hyperv/iso/builder.hcl2spec.go @@ -97,7 +97,7 @@ type FlatConfig struct { SkipCompaction *bool `mapstructure:"skip_compaction" required:"false" cty:"skip_compaction"` SkipExport *bool `mapstructure:"skip_export" required:"false" cty:"skip_export"` Headless *bool `mapstructure:"headless" required:"false" cty:"headless"` - LegacyGen1BootOrder *bool `mapstructure:"legacy_gen1_boot_order" required:"false" cty:"legacy_gen1_boot_order"` + FirstBootDevice *string `mapstructure:"first_boot_device" required:"false" cty:"first_boot_device"` ShutdownCommand *string `mapstructure:"shutdown_command" required:"false" cty:"shutdown_command"` ShutdownTimeout *string `mapstructure:"shutdown_timeout" required:"false" cty:"shutdown_timeout"` DiskSize *uint `mapstructure:"disk_size" required:"false" cty:"disk_size"` @@ -206,7 +206,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "skip_compaction": &hcldec.AttrSpec{Name: "skip_compaction", Type: cty.Bool, Required: false}, "skip_export": &hcldec.AttrSpec{Name: "skip_export", Type: cty.Bool, Required: false}, "headless": &hcldec.AttrSpec{Name: "headless", Type: cty.Bool, Required: false}, - "legacy_gen1_boot_order": &hcldec.AttrSpec{Name: "legacy_gen1_boot_order", Type: cty.Bool, Required: false}, + "first_boot_device": &hcldec.AttrSpec{Name: "first_boot_device", Type: cty.String, Required: false}, "shutdown_command": &hcldec.AttrSpec{Name: "shutdown_command", Type: cty.String, Required: false}, "shutdown_timeout": &hcldec.AttrSpec{Name: "shutdown_timeout", Type: cty.String, Required: false}, "disk_size": &hcldec.AttrSpec{Name: "disk_size", Type: cty.Number, Required: false}, diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index 6eed0a3c3..17a878142 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -282,8 +282,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack &hypervcommon.StepEnableIntegrationService{}, &hypervcommon.StepMountDvdDrive{ - Generation: b.config.Generation, - FirstBootDevice: b.config.FirstBootDevice, + Generation: b.config.Generation, + FirstBootDevice: b.config.FirstBootDevice, }, &hypervcommon.StepMountFloppydrive{ Generation: b.config.Generation, diff --git a/builder/hyperv/vmcx/builder.hcl2spec.go b/builder/hyperv/vmcx/builder.hcl2spec.go index e2c028e73..113cd677e 100644 --- a/builder/hyperv/vmcx/builder.hcl2spec.go +++ b/builder/hyperv/vmcx/builder.hcl2spec.go @@ -97,7 +97,7 @@ type FlatConfig struct { SkipCompaction *bool `mapstructure:"skip_compaction" required:"false" cty:"skip_compaction"` SkipExport *bool `mapstructure:"skip_export" required:"false" cty:"skip_export"` Headless *bool `mapstructure:"headless" required:"false" cty:"headless"` - LegacyGen1BootOrder *bool `mapstructure:"legacy_gen1_boot_order" required:"false" cty:"legacy_gen1_boot_order"` + FirstBootDevice *string `mapstructure:"first_boot_device" required:"false" cty:"first_boot_device"` ShutdownCommand *string `mapstructure:"shutdown_command" required:"false" cty:"shutdown_command"` ShutdownTimeout *string `mapstructure:"shutdown_timeout" required:"false" cty:"shutdown_timeout"` CloneFromVMCXPath *string `mapstructure:"clone_from_vmcx_path" cty:"clone_from_vmcx_path"` @@ -208,7 +208,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "skip_compaction": &hcldec.AttrSpec{Name: "skip_compaction", Type: cty.Bool, Required: false}, "skip_export": &hcldec.AttrSpec{Name: "skip_export", Type: cty.Bool, Required: false}, "headless": &hcldec.AttrSpec{Name: "headless", Type: cty.Bool, Required: false}, - "legacy_gen1_boot_order": &hcldec.AttrSpec{Name: "legacy_gen1_boot_order", Type: cty.Bool, Required: false}, + "first_boot_device": &hcldec.AttrSpec{Name: "first_boot_device", Type: cty.String, Required: false}, "shutdown_command": &hcldec.AttrSpec{Name: "shutdown_command", Type: cty.String, Required: false}, "shutdown_timeout": &hcldec.AttrSpec{Name: "shutdown_timeout", Type: cty.String, Required: false}, "clone_from_vmcx_path": &hcldec.AttrSpec{Name: "clone_from_vmcx_path", Type: cty.String, Required: false}, diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index bebc4119f..3ace245a3 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -188,7 +188,7 @@ func SetFirstBootDeviceGen1(vmName string, controllerType string) error { // controllerType must be 'NET', 'DVD', 'IDE' or 'FLOPPY' (case sensitive) // The 'NET' value is always replaced with 'LegacyNetworkAdapter' - if (controllerType == "NET") { + if controllerType == "NET" { controllerType = "LegacyNetworkAdapter" } @@ -205,18 +205,16 @@ param([string] $vmName, [string] $controllerType) func SetFirstBootDeviceGen2(vmName string, controllerType string, controllerNumber uint, controllerLocation uint) error { + // script := ` + // param([string]$vmName,[int]$controllerNumber,[int]$controllerLocation) + // $vmDvdDrive = Hyper-V\Get-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -ControllerLocation $controllerLocation + // if (!$vmDvdDrive) {throw 'unable to find dvd drive'} + // Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $vmDvdDrive -ErrorAction SilentlyContinue + // ` - -// script := ` -// param([string]$vmName,[int]$controllerNumber,[int]$controllerLocation) -// $vmDvdDrive = Hyper-V\Get-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -ControllerLocation $controllerLocation -// if (!$vmDvdDrive) {throw 'unable to find dvd drive'} -// Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $vmDvdDrive -ErrorAction SilentlyContinue -// ` - -// script := ` -// param([string] $vmName, [string] $controllerType, ) -//` + // script := ` + // param([string] $vmName, [string] $controllerType, ) + //` return nil } diff --git a/website/source/partials/builder/hyperv/common/_CommonConfig-not-required.html.md b/website/source/partials/builder/hyperv/common/_CommonConfig-not-required.html.md index e1ebf434b..74f9914c5 100644 --- a/website/source/partials/builder/hyperv/common/_CommonConfig-not-required.html.md +++ b/website/source/partials/builder/hyperv/common/_CommonConfig-not-required.html.md @@ -110,10 +110,18 @@ built. When this value is set to true, the machine will start without a console. -- `legacy_gen1_boot_order` (bool) - Over time the Hyper-V builder has been modified to change the original - boot order that is used when an ISO is mounted. Hyper-V's default is to - boot from the CD first, the original Hyper-V builder included code to - codify this setting when the primary ISO is mounted, that code was eventually - modified to place the IDE adapter before the the CD (only in generation 1). - Setting this value to true, forces the original method of operation. +- `first_boot_device` (string) - When configured, determines the device or device type that is given preferential + treatment when choosing a boot device. + + For Generation 1: + - `IDE` + - `CD` *or* `DVD` + - `Floppy` + - `NET` + + For Generation 2: + - `IDE:x:y` + - `SCSI:x:y` + - `CD` *or* `DVD` + - `NET` \ No newline at end of file From dd8f4370c6e222923409ad78e65458b6aed5d5c1 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Fri, 21 Feb 2020 01:30:59 -0600 Subject: [PATCH 08/17] initial support for gen2 and fix driver_mock --- builder/hyperv/common/driver_mock.go | 9 ++++++- common/powershell/hyperv/hyperv.go | 35 ++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 4320f932a..882d37398 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -234,9 +234,16 @@ type DriverMock struct { SetBootDvdDrive_ControllerNumber uint SetBootDvdDrive_ControllerLocation uint SetBootDvdDrive_Generation uint - SetBootDvdDrive_LegacyGen1BootOrder bool SetBootDvdDrive_Err error + SetFirstBootDevice_Called bool + SetFirstBootDevice_VmName string + SetFirstBootDevice_ControllerType string + SetFirstBootDevice_ControllerNumber uint + SetFirstBootDevice_ControllerLocation uint + SetFirstBootDevice_Generation uint + SetFirstBootDevice_Err error + UnmountDvdDrive_Called bool UnmountDvdDrive_VmName string UnmountDvdDrive_ControllerNumber uint diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index 3ace245a3..a51d171a7 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -205,18 +205,33 @@ param([string] $vmName, [string] $controllerType) func SetFirstBootDeviceGen2(vmName string, controllerType string, controllerNumber uint, controllerLocation uint) error { - // script := ` - // param([string]$vmName,[int]$controllerNumber,[int]$controllerLocation) - // $vmDvdDrive = Hyper-V\Get-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -ControllerLocation $controllerLocation - // if (!$vmDvdDrive) {throw 'unable to find dvd drive'} - // Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $vmDvdDrive -ErrorAction SilentlyContinue - // ` + script := ` +param ([string] $vmName, [string] $controllerType, [int] $controllerNumber, [int] $controllerLocation) +` - // script := ` - // param([string] $vmName, [string] $controllerType, ) - //` + switch { - return nil + case controllerType == "CD": + // for CDs we have to use Get-VMDvdDrive to find the device + script += `$bootDevice = Hyper-V\Get-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -ControllerLocation $controllerLocation -ErrorAction SilentlyContinue` + + case controllerType == "NET": + // for "NET" device, we select the first network adapter on the VM + script += `$bootDevice = Hyper-V\Get-VMNetworkAdapter -VMName $vmName -ErrorAction SilentlyContinue | Select-Object -First 1` + + default: + script += `$bootDevice = Hyper-V\Get-VMHardDiskDrive -VMName $vmName -ControllerType $controllerType -ControllerNumber $controllerNumber -ControllerLocation controllerLocation -ErrorAction SilentlyContinue` + + } + + script += ` +if ($bootDevice -ne $null) { throw 'unable to find boot device' } +Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $bootDevice +` + + var ps powershell.PowerShellCmd + err := ps.Run(script, vmName, controllerType, strconv.FormatInt(int64(controllerNumber), 10), strconv.FormatInt(int64(controllerLocation), 10)) + return err } func SetFirstBootDevice(vmName string, controllerType string, controllerNumber uint, controllerLocation uint, generation uint) error { From 0a0fbfc33b09d1d731707703837474b607c2d6b6 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Fri, 21 Feb 2020 01:31:18 -0600 Subject: [PATCH 09/17] fmt --- builder/hyperv/common/driver_mock.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 882d37398..64cb14fa5 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -229,12 +229,12 @@ type DriverMock struct { MountDvdDrive_ControllerLocation uint MountDvdDrive_Err error - SetBootDvdDrive_Called bool - SetBootDvdDrive_VmName string - SetBootDvdDrive_ControllerNumber uint - SetBootDvdDrive_ControllerLocation uint - SetBootDvdDrive_Generation uint - SetBootDvdDrive_Err error + SetBootDvdDrive_Called bool + SetBootDvdDrive_VmName string + SetBootDvdDrive_ControllerNumber uint + SetBootDvdDrive_ControllerLocation uint + SetBootDvdDrive_Generation uint + SetBootDvdDrive_Err error SetFirstBootDevice_Called bool SetFirstBootDevice_VmName string From 2de731453f4f1bdb6941b46766aca02f1d7d5434 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Fri, 21 Feb 2020 01:43:03 -0600 Subject: [PATCH 10/17] really fix driver_mock this time --- builder/hyperv/common/driver_mock.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 64cb14fa5..eb633bcd3 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -580,10 +580,20 @@ func (d *DriverMock) SetBootDvdDrive(vmName string, controllerNumber uint, contr d.SetBootDvdDrive_ControllerNumber = controllerNumber d.SetBootDvdDrive_ControllerLocation = controllerLocation d.SetBootDvdDrive_Generation = generation - d.SetBootDvdDrive_LegacyGen1BootOrder = legacyGen1BootOrder return d.SetBootDvdDrive_Err } +func (d *DriverMock) SetFirstBootDevice(vmName string, controllerType string, controllerNumber uint, + controllerLocation uint, generation uint) error { + d.SetFirstBootDevice_Called = true + d.SetFirstBootDevice_VmName = vmName + d.SetFirstBootDevice_ControllerType = controllerType + d.SetFirstBootDevice_ControllerNumber = controllerNumber + d.SetFirstBootDevice_ControllerLocation = controllerLocation + d.SetFirstBootDevice.Generation = generation + return d.SetFirstBootDevice_Err +} + func (d *DriverMock) UnmountDvdDrive(vmName string, controllerNumber uint, controllerLocation uint) error { d.UnmountDvdDrive_Called = true d.UnmountDvdDrive_VmName = vmName From 11fae173085fa804fea752dab282efa9b5351149 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Fri, 21 Feb 2020 01:44:49 -0600 Subject: [PATCH 11/17] again, really fix driver_mock --- builder/hyperv/common/driver_mock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index eb633bcd3..4ca8d7ed0 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -590,7 +590,7 @@ func (d *DriverMock) SetFirstBootDevice(vmName string, controllerType string, co d.SetFirstBootDevice_ControllerType = controllerType d.SetFirstBootDevice_ControllerNumber = controllerNumber d.SetFirstBootDevice_ControllerLocation = controllerLocation - d.SetFirstBootDevice.Generation = generation + d.SetFirstBootDevice_Generation = generation return d.SetFirstBootDevice_Err } From 723a9eba9a531f979946d99e26f9e5c563bf3d0b Mon Sep 17 00:00:00 2001 From: William Brooks Date: Fri, 21 Feb 2020 01:53:13 -0600 Subject: [PATCH 12/17] really, really, really fix driver_mock --- builder/hyperv/common/driver_mock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 4ca8d7ed0..e585f3513 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -574,7 +574,7 @@ func (d *DriverMock) MountDvdDrive(vmName string, path string, controllerNumber } func (d *DriverMock) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, - generation uint, legacyGen1BootOrder bool) error { + generation uint) error { d.SetBootDvdDrive_Called = true d.SetBootDvdDrive_VmName = vmName d.SetBootDvdDrive_ControllerNumber = controllerNumber From 7f38ce06863ea0e63ddeeebbae47c2120d7ae25c Mon Sep 17 00:00:00 2001 From: William Brooks Date: Fri, 21 Feb 2020 11:09:49 -0600 Subject: [PATCH 13/17] test and correct gen2 SetFirstBootDeviceGen2 scripts --- common/powershell/hyperv/hyperv.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index a51d171a7..b57fcaa3a 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -205,28 +205,34 @@ param([string] $vmName, [string] $controllerType) func SetFirstBootDeviceGen2(vmName string, controllerType string, controllerNumber uint, controllerLocation uint) error { - script := ` -param ([string] $vmName, [string] $controllerType, [int] $controllerNumber, [int] $controllerLocation) -` + script := `param ([string] $vmName, [string] $controllerType, [int] $controllerNumber, [int] $controllerLocation)` switch { case controllerType == "CD": // for CDs we have to use Get-VMDvdDrive to find the device - script += `$bootDevice = Hyper-V\Get-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -ControllerLocation $controllerLocation -ErrorAction SilentlyContinue` + script += ` +$vmDevice = Hyper-V\Get-VMDvdDrive -VMName $vmName -ControllerNumber $controllerNumber -ControllerLocation $controllerLocation -ErrorAction SilentlyContinue` case controllerType == "NET": // for "NET" device, we select the first network adapter on the VM - script += `$bootDevice = Hyper-V\Get-VMNetworkAdapter -VMName $vmName -ErrorAction SilentlyContinue | Select-Object -First 1` + script += ` +$vmDevice = Hyper-V\Get-VMNetworkAdapter -VMName $vmName -ErrorAction SilentlyContinue | Select-Object -First 1` default: - script += `$bootDevice = Hyper-V\Get-VMHardDiskDrive -VMName $vmName -ControllerType $controllerType -ControllerNumber $controllerNumber -ControllerLocation controllerLocation -ErrorAction SilentlyContinue` + script += ` +$vmDevice = @(Hyper-V\Get-VMIdeController -VMName $vmName -ErrorAction SilentlyContinue) + + @(Hyper-V\Get-VMScsiController -VMName $vmName -ErrorAction SilentlyContinue) | + Select-Object -ExpandProperty Drives | + Where-Object { $_.ControllerType -eq $controllerType } | + Where-Object { ($_.ControllerNumber -eq $controllerNumber) -and ($_.ControllerLocation -eq $controllerLocation) } +` } script += ` -if ($bootDevice -ne $null) { throw 'unable to find boot device' } -Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $bootDevice +if ($vmDevice -eq $null) { throw 'unable to find boot device' } +Hyper-V\Set-VMFirmware -VMName $vmName -FirstBootDevice $vmDevice ` var ps powershell.PowerShellCmd From b5b190b1f0d663f8bca595fd9246f8a1a9797be8 Mon Sep 17 00:00:00 2001 From: William Date: Sat, 22 Feb 2020 00:29:05 -0600 Subject: [PATCH 14/17] Bootdevice testing (#3) Start step_first_boot_device_test and rejigger ParseBootDeviceIdentifier to avoid regex where reasonable. --- .../common/step_set_first_boot_device.go | 94 +++++++++++-------- .../common/step_set_first_boot_device_test.go | 74 +++++++++++++++ 2 files changed, 131 insertions(+), 37 deletions(-) create mode 100644 builder/hyperv/common/step_set_first_boot_device_test.go diff --git a/builder/hyperv/common/step_set_first_boot_device.go b/builder/hyperv/common/step_set_first_boot_device.go index f17e9a178..397b76669 100644 --- a/builder/hyperv/common/step_set_first_boot_device.go +++ b/builder/hyperv/common/step_set_first_boot_device.go @@ -18,66 +18,86 @@ type StepSetFirstBootDevice struct { func ParseBootDeviceIdentifier(deviceIdentifier string, generation uint) (string, uint, uint, error) { - captureExpression := "^(FLOPPY|IDE|NET)|(CD|DVD)$" - if generation > 1 { - captureExpression = "^((IDE|SCSI):(\\d+):(\\d+))|(DVD|CD)|(NET)$" - } + // all input strings are forced to upperCase for comparison, I believe this is + // safe as all of our values are 7bit ASCII clean. - r, err := regexp.Compile(captureExpression) - if err != nil { - return "", 0, 0, err + lookupDeviceIdentifier := strings.ToUpper(deviceIdentifier) + + if (generation == 1) { + + // Gen1 values are a simple set of if/then/else values, which we coalesce into a map + // here for simplicity + + lookupTable := map[string]string { + "FLOPPY": "FLOPPY", + "IDE": "IDE", + "NET": "NET", + "CD": "CD", + "DVD": "CD", + } + + controllerType, isDefined := lookupTable[lookupDeviceIdentifier] + if (!isDefined) { + + return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device group identifier.", deviceIdentifier) + + } + + // success + return controllerType, 0, 0, nil } - // match against the appropriate set of values.. we force to uppercase to ensure that - // all devices are always in the same case + // everything else is treated as generation 2... the first set of lookups covers + // the simple options.. - identifierMatches := r.FindStringSubmatch(strings.ToUpper(deviceIdentifier)) - if identifierMatches == nil { - return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device or device group identifier.", deviceIdentifier) + lookupTable := map[string]string { + "CD": "CD", + "DVD": "CD", + "NET": "NET", } - switch { + controllerType, isDefined := lookupTable[lookupDeviceIdentifier] + if (isDefined) { - // CD or DVD are always returned as "CD" - case ((generation == 1) && (identifierMatches[2] != "")) || ((generation > 1) && (identifierMatches[5] != "")): - return "CD", 0, 0, nil + // these types do not require controllerNumber or controllerLocation + return controllerType, 0, 0, nil - // generation 1 only has FLOPPY, IDE or NET remaining.. - case (generation == 1): - return identifierMatches[0], 0, 0, nil + } - // generation 2, check for IDE or SCSI and parse location and number - case (identifierMatches[2] != ""): - { + // not a simple option, check for a controllerType:controllerNumber:controllerLocation formatted + // device.. - var controllerLocation int64 - var controllerNumber int64 + r, err := regexp.Compile("^(IDE|SCSI):(\\d+):(\\d+)$") + if err != nil { + return "", 0, 0, err + } - // NOTE: controllerNumber and controllerLocation cannot be negative, the regex expression - // would not have matched if either number was signed + controllerMatch := r.FindStringSubmatch(lookupDeviceIdentifier) + if controllerMatch != nil { - controllerNumber, err = strconv.ParseInt(identifierMatches[3], 10, 8) - if err == nil { + var controllerLocation int64 + var controllerNumber int64 - controllerLocation, err = strconv.ParseInt(identifierMatches[4], 10, 8) - if err == nil { + // NOTE: controllerNumber and controllerLocation cannot be negative, the regex expression + // would not have matched if either number was signed - return identifierMatches[2], uint(controllerNumber), uint(controllerLocation), nil + controllerNumber, err = strconv.ParseInt(controllerMatch[2], 10, 8) + if err == nil { - } + controllerLocation, err = strconv.ParseInt(controllerMatch[3], 10, 8) + if err == nil { - } + return controllerMatch[1], uint(controllerNumber), uint(controllerLocation), nil - return "", 0, 0, err + } } - // only "NET" left on generation 2 - default: - return "NET", 0, 0, nil + return "", 0, 0, err } + return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device identifier.", deviceIdentifier) } func (s *StepSetFirstBootDevice) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { diff --git a/builder/hyperv/common/step_set_first_boot_device_test.go b/builder/hyperv/common/step_set_first_boot_device_test.go new file mode 100644 index 000000000..fd663aeb6 --- /dev/null +++ b/builder/hyperv/common/step_set_first_boot_device_test.go @@ -0,0 +1,74 @@ +package common + +import ( + "testing" + + "github.com/hashicorp/packer/helper/multistep" +) + +func TestStepSetFirstBootDevice_impl(t *testing.T) { + var _ multistep.Step = new(StepSetFirstBootDevice) +} + +func TestStepSetFirstBootDevice(t *testing.T) { +// t.Fatal("Fail IT!") +} + +type parseBootDeviceIdentifierTest struct { + generation uint + deviceIdentifier string + controllerType string + controllerNumber uint + controllerLocation uint + shouldError bool +} + +func TestStepSetFirstBootDevice_ParseIdentifier(t *testing.T) { + + identifierTests := [...]parseBootDeviceIdentifierTest{ + {1, "IDE", "IDE", 0, 0, false}, + {1, "idE", "IDE", 0, 0, false}, + {1, "CD", "CD", 0, 0, false}, + {1, "cD", "CD", 0, 0, false}, + {1, "DVD", "CD", 0, 0, false}, + {1, "Dvd", "CD", 0, 0, false}, + {1, "FLOPPY", "FLOPPY", 0, 0, false}, + {1, "FloppY", "FLOPPY", 0, 0, false}, + {1, "NET", "NET", 0, 0, false}, + {1, "net", "NET", 0, 0, false}, + {1, "", "", 0, 0, true}, + {1, "bad", "", 0, 0, true}, + {1, "IDE:0:0", "", 0, 0, true}, + {1, "SCSI:0:0", "", 0, 0, true}, + } + + for _, identifierTest := range identifierTests { + + controllerType, controllerNumber, controllerLocation, err := ParseBootDeviceIdentifier( + identifierTest.deviceIdentifier, + identifierTest.generation) + + if (err != nil) != identifierTest.shouldError { + + t.Fatalf("Test %q (gen %v): shouldError: %v but err: %v", identifierTest.deviceIdentifier, + identifierTest.generation, identifierTest.shouldError, err) + + } + + switch { + + case controllerType != identifierTest.controllerType: + t.Fatalf("Test %q (gen %v): controllerType: %q != %q", identifierTest.deviceIdentifier, identifierTest.generation, + identifierTest.controllerType, controllerType) + + case controllerNumber != identifierTest.controllerNumber: + t.Fatalf("Test %q (gen %v): controllerNumber: %v != %v", identifierTest.deviceIdentifier, identifierTest.generation, + identifierTest.controllerNumber, controllerNumber) + + case controllerLocation != identifierTest.controllerLocation: + t.Fatalf("Test %q (gen %v): controllerLocation: %v != %v", identifierTest.deviceIdentifier, identifierTest.generation, + identifierTest.controllerLocation, controllerLocation) + + } + } +} \ No newline at end of file From d246d0c82a09dba69ea893530d41d1b8ca6e9c06 Mon Sep 17 00:00:00 2001 From: William Date: Sat, 22 Feb 2020 02:03:42 -0600 Subject: [PATCH 15/17] More testing (#4) Additional testing for SetFirstBootDevice and checking configuration in config.Prepare() --- builder/hyperv/common/config.go | 7 + .../common/step_set_first_boot_device.go | 20 +-- .../common/step_set_first_boot_device_test.go | 160 ++++++++++++++---- 3 files changed, 145 insertions(+), 42 deletions(-) diff --git a/builder/hyperv/common/config.go b/builder/hyperv/common/config.go index c97d0ffcd..5e592ed74 100644 --- a/builder/hyperv/common/config.go +++ b/builder/hyperv/common/config.go @@ -283,6 +283,13 @@ func (c *CommonConfig) Prepare(ctx *interpolate.Context, pc *common.PackerConfig } } + if c.FirstBootDevice != "" { + _, _, _, err := ParseBootDeviceIdentifier(c.FirstBootDevice, c.Generation) + if err != nil { + errs = append(errs, fmt.Errorf("first_boot_device: %s", err)) + } + } + if c.EnableVirtualizationExtensions { if c.EnableDynamicMemory { warning := fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " + diff --git a/builder/hyperv/common/step_set_first_boot_device.go b/builder/hyperv/common/step_set_first_boot_device.go index 397b76669..da950c123 100644 --- a/builder/hyperv/common/step_set_first_boot_device.go +++ b/builder/hyperv/common/step_set_first_boot_device.go @@ -23,21 +23,21 @@ func ParseBootDeviceIdentifier(deviceIdentifier string, generation uint) (string lookupDeviceIdentifier := strings.ToUpper(deviceIdentifier) - if (generation == 1) { + if generation == 1 { // Gen1 values are a simple set of if/then/else values, which we coalesce into a map // here for simplicity - lookupTable := map[string]string { + lookupTable := map[string]string{ "FLOPPY": "FLOPPY", - "IDE": "IDE", - "NET": "NET", - "CD": "CD", - "DVD": "CD", + "IDE": "IDE", + "NET": "NET", + "CD": "CD", + "DVD": "CD", } controllerType, isDefined := lookupTable[lookupDeviceIdentifier] - if (!isDefined) { + if !isDefined { return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device group identifier.", deviceIdentifier) @@ -50,14 +50,14 @@ func ParseBootDeviceIdentifier(deviceIdentifier string, generation uint) (string // everything else is treated as generation 2... the first set of lookups covers // the simple options.. - lookupTable := map[string]string { - "CD": "CD", + lookupTable := map[string]string{ + "CD": "CD", "DVD": "CD", "NET": "NET", } controllerType, isDefined := lookupTable[lookupDeviceIdentifier] - if (isDefined) { + if isDefined { // these types do not require controllerNumber or controllerLocation return controllerType, 0, 0, nil diff --git a/builder/hyperv/common/step_set_first_boot_device_test.go b/builder/hyperv/common/step_set_first_boot_device_test.go index fd663aeb6..264363e98 100644 --- a/builder/hyperv/common/step_set_first_boot_device_test.go +++ b/builder/hyperv/common/step_set_first_boot_device_test.go @@ -1,58 +1,86 @@ package common import ( + "context" "testing" "github.com/hashicorp/packer/helper/multistep" ) -func TestStepSetFirstBootDevice_impl(t *testing.T) { - var _ multistep.Step = new(StepSetFirstBootDevice) -} - -func TestStepSetFirstBootDevice(t *testing.T) { -// t.Fatal("Fail IT!") -} - type parseBootDeviceIdentifierTest struct { generation uint deviceIdentifier string controllerType string controllerNumber uint controllerLocation uint - shouldError bool + failInParse bool // true if ParseBootDeviceIdentifier should return an error + haltStep bool // true if Step.Run should return Halt action + shouldCallSet bool // true if driver.SetFirstBootDevice should have been called + setDvdProps bool // true to set DvdDeviceProperties state } -func TestStepSetFirstBootDevice_ParseIdentifier(t *testing.T) { +var parseIdentifierTests = [...]parseBootDeviceIdentifierTest{ + {1, "IDE", "IDE", 0, 0, false, false, true, false}, + {1, "idE", "IDE", 0, 0, false, false, true, false}, + {1, "CD", "CD", 0, 0, false, false, false, false}, + {1, "CD", "CD", 0, 0, false, false, true, true}, + {1, "cD", "CD", 0, 0, false, false, false, false}, + {1, "DVD", "CD", 0, 0, false, false, false, false}, + {1, "DVD", "CD", 0, 0, false, false, true, true}, + {1, "Dvd", "CD", 0, 0, false, false, false, false}, + {1, "FLOPPY", "FLOPPY", 0, 0, false, false, true, false}, + {1, "FloppY", "FLOPPY", 0, 0, false, false, true, false}, + {1, "NET", "NET", 0, 0, false, false, true, false}, + {1, "net", "NET", 0, 0, false, false, true, false}, + {1, "", "", 0, 0, true, false, false, false}, + {1, "bad", "", 0, 0, true, true, false, false}, + {1, "IDE:0:0", "", 0, 0, true, true, true, false}, + {1, "SCSI:0:0", "", 0, 0, true, true, true, false}, + {2, "IDE", "", 0, 0, true, true, true, false}, + {2, "idE", "", 0, 0, true, true, true, false}, + {2, "CD", "CD", 0, 0, false, false, false, false}, + {2, "CD", "CD", 0, 0, false, false, true, true}, + {2, "cD", "CD", 0, 0, false, false, false, false}, + {2, "DVD", "CD", 0, 0, false, false, false, false}, + {2, "DVD", "CD", 0, 0, false, false, true, true}, + {2, "Dvd", "CD", 0, 0, false, false, false, false}, + {2, "FLOPPY", "", 0, 0, true, true, true, false}, + {2, "FloppY", "", 0, 0, true, true, true, false}, + {2, "NET", "NET", 0, 0, false, false, true, false}, + {2, "net", "NET", 0, 0, false, false, true, false}, + {2, "", "", 0, 0, true, false, false, false}, + {2, "bad", "", 0, 0, true, true, false, false}, + {2, "IDE:0:0", "IDE", 0, 0, false, false, true, false}, + {2, "SCSI:0:0", "SCSI", 0, 0, false, false, true, false}, + {2, "Ide:0:0", "IDE", 0, 0, false, false, true, false}, + {2, "sCsI:0:0", "SCSI", 0, 0, false, false, true, false}, + {2, "IDEscsi:0:0", "", 0, 0, true, true, false, false}, + {2, "SCSIide:0:0", "", 0, 0, true, true, false, false}, + {2, "IDE:0", "", 0, 0, true, true, false, false}, + {2, "SCSI:0", "", 0, 0, true, true, false, false}, + {2, "IDE:0:a", "", 0, 0, true, true, false, false}, + {2, "SCSI:0:a", "", 0, 0, true, true, false, false}, + {2, "IDE:0:653", "", 0, 0, true, true, false, false}, + {2, "SCSI:-10:0", "", 0, 0, true, true, false, false}, +} - identifierTests := [...]parseBootDeviceIdentifierTest{ - {1, "IDE", "IDE", 0, 0, false}, - {1, "idE", "IDE", 0, 0, false}, - {1, "CD", "CD", 0, 0, false}, - {1, "cD", "CD", 0, 0, false}, - {1, "DVD", "CD", 0, 0, false}, - {1, "Dvd", "CD", 0, 0, false}, - {1, "FLOPPY", "FLOPPY", 0, 0, false}, - {1, "FloppY", "FLOPPY", 0, 0, false}, - {1, "NET", "NET", 0, 0, false}, - {1, "net", "NET", 0, 0, false}, - {1, "", "", 0, 0, true}, - {1, "bad", "", 0, 0, true}, - {1, "IDE:0:0", "", 0, 0, true}, - {1, "SCSI:0:0", "", 0, 0, true}, - } +func TestStepSetFirstBootDevice_impl(t *testing.T) { + var _ multistep.Step = new(StepSetFirstBootDevice) +} + +func TestStepSetFirstBootDevice_ParseIdentifier(t *testing.T) { - for _, identifierTest := range identifierTests { + for _, identifierTest := range parseIdentifierTests { controllerType, controllerNumber, controllerLocation, err := ParseBootDeviceIdentifier( identifierTest.deviceIdentifier, identifierTest.generation) - if (err != nil) != identifierTest.shouldError { + if (err != nil) != identifierTest.failInParse { + + t.Fatalf("Test %q (gen %v): failInParse: %v but err: %v", identifierTest.deviceIdentifier, + identifierTest.generation, identifierTest.failInParse, err) - t.Fatalf("Test %q (gen %v): shouldError: %v but err: %v", identifierTest.deviceIdentifier, - identifierTest.generation, identifierTest.shouldError, err) - } switch { @@ -71,4 +99,72 @@ func TestStepSetFirstBootDevice_ParseIdentifier(t *testing.T) { } } -} \ No newline at end of file +} + +func TestStepSetFirstBootDevice(t *testing.T) { + + step := new(StepSetFirstBootDevice) + + for _, identifierTest := range parseIdentifierTests { + + state := testState(t) + driver := state.Get("driver").(*DriverMock) + + // requires the vmName state value + vmName := "foo" + state.Put("vmName", vmName) + + // pretend that we mounted a DVD somewhere (CD:0:0) + if identifierTest.setDvdProps { + var dvdControllerProperties DvdControllerProperties + dvdControllerProperties.ControllerNumber = 0 + dvdControllerProperties.ControllerLocation = 0 + dvdControllerProperties.Existing = false + state.Put("os.dvd.properties", dvdControllerProperties) + } + + step.Generation = identifierTest.generation + step.FirstBootDevice = identifierTest.deviceIdentifier + + action := step.Run(context.Background(), state) + if (action != multistep.ActionContinue) != identifierTest.haltStep { + t.Fatalf("Test %q (gen %v): Bad action: %v", identifierTest.deviceIdentifier, identifierTest.generation, action) + } + + if identifierTest.haltStep { + + if _, ok := state.GetOk("error"); !ok { + t.Fatalf("Test %q (gen %v): Should have error", identifierTest.deviceIdentifier, identifierTest.generation) + } + + // don't perform the remaining checks.. + continue + + } else { + + if _, ok := state.GetOk("error"); ok { + t.Fatalf("Test %q (gen %v): Should NOT have error", identifierTest.deviceIdentifier, identifierTest.generation) + } + + } + + if driver.SetFirstBootDevice_Called != identifierTest.shouldCallSet { + if identifierTest.shouldCallSet { + t.Fatalf("Test %q (gen %v): Should have called SetFirstBootDevice", identifierTest.deviceIdentifier, identifierTest.generation) + } + + t.Fatalf("Test %q (gen %v): Should NOT have called SetFirstBootDevice", identifierTest.deviceIdentifier, identifierTest.generation) + } + + if (driver.SetFirstBootDevice_Called) && + ((driver.SetFirstBootDevice_VmName != vmName) || + (driver.SetFirstBootDevice_ControllerType != identifierTest.controllerType) || + (driver.SetFirstBootDevice_ControllerNumber != identifierTest.controllerNumber) || + (driver.SetFirstBootDevice_ControllerLocation != identifierTest.controllerLocation) || + (driver.SetFirstBootDevice_Generation != identifierTest.generation)) { + + t.Fatalf("Test %q (gen %v): Called SetFirstBootDevice with unexpected arguments.", identifierTest.deviceIdentifier, identifierTest.generation) + + } + } +} From efa9d94d8ef59c0d2ba19bc06c8417a87f1ccc09 Mon Sep 17 00:00:00 2001 From: William Brooks Date: Sat, 22 Feb 2020 02:22:59 -0600 Subject: [PATCH 16/17] use raw string around regex to make go lint happy --- builder/hyperv/common/step_set_first_boot_device.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/hyperv/common/step_set_first_boot_device.go b/builder/hyperv/common/step_set_first_boot_device.go index da950c123..e58820902 100644 --- a/builder/hyperv/common/step_set_first_boot_device.go +++ b/builder/hyperv/common/step_set_first_boot_device.go @@ -67,7 +67,7 @@ func ParseBootDeviceIdentifier(deviceIdentifier string, generation uint) (string // not a simple option, check for a controllerType:controllerNumber:controllerLocation formatted // device.. - r, err := regexp.Compile("^(IDE|SCSI):(\\d+):(\\d+)$") + r, err := regexp.Compile(`^(IDE|SCSI):(\\d+):(\\d+)$`) if err != nil { return "", 0, 0, err } From 8e0ed663187d0ded59c14d955af5e37375a5f9fd Mon Sep 17 00:00:00 2001 From: William Brooks Date: Sat, 22 Feb 2020 02:40:41 -0600 Subject: [PATCH 17/17] Fix raw string breaks escaping --- builder/hyperv/common/step_set_first_boot_device.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/hyperv/common/step_set_first_boot_device.go b/builder/hyperv/common/step_set_first_boot_device.go index e58820902..c8b56f8f0 100644 --- a/builder/hyperv/common/step_set_first_boot_device.go +++ b/builder/hyperv/common/step_set_first_boot_device.go @@ -67,7 +67,7 @@ func ParseBootDeviceIdentifier(deviceIdentifier string, generation uint) (string // not a simple option, check for a controllerType:controllerNumber:controllerLocation formatted // device.. - r, err := regexp.Compile(`^(IDE|SCSI):(\\d+):(\\d+)$`) + r, err := regexp.Compile(`^(IDE|SCSI):(\d+):(\d+)$`) if err != nil { return "", 0, 0, err }