diff --git a/builder/vsphere/common/common_test.go b/builder/vsphere/common/common_test.go new file mode 100644 index 000000000..6242ca4b6 --- /dev/null +++ b/builder/vsphere/common/common_test.go @@ -0,0 +1,19 @@ +package common + +import ( + "bytes" + "strings" + + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +func basicStateBag(errorBuffer *strings.Builder) *multistep.BasicStateBag { + state := new(multistep.BasicStateBag) + state.Put("ui", &packer.BasicUi{ + Reader: new(bytes.Buffer), + Writer: new(bytes.Buffer), + ErrorWriter: errorBuffer, + }) + return state +} diff --git a/builder/vsphere/common/step_hardware.go b/builder/vsphere/common/step_hardware.go index 5f16a79b7..98e36d0d6 100644 --- a/builder/vsphere/common/step_hardware.go +++ b/builder/vsphere/common/step_hardware.go @@ -65,7 +65,7 @@ type StepConfigureHardware struct { func (s *StepConfigureHardware) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { ui := state.Get("ui").(packer.Ui) - vm := state.Get("vm").(*driver.VirtualMachineDriver) + vm := state.Get("vm").(driver.VirtualMachine) if *s.Config != (HardwareConfig{}) { ui.Say("Customizing hardware...") diff --git a/builder/vsphere/common/step_hardware_test.go b/builder/vsphere/common/step_hardware_test.go new file mode 100644 index 000000000..08eb22c50 --- /dev/null +++ b/builder/vsphere/common/step_hardware_test.go @@ -0,0 +1,181 @@ +package common + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/packer/builder/vsphere/driver" + "github.com/hashicorp/packer/helper/multistep" +) + +func TestHardwareConfig_Prepare(t *testing.T) { + tc := []struct { + name string + config *HardwareConfig + fail bool + expectedErrMsg string + }{ + { + name: "Validate empty config", + config: &HardwareConfig{}, + fail: false, + }, + { + name: "Validate RAMReservation RAMReserveAll cannot be used together", + config: &HardwareConfig{ + RAMReservation: 2, + RAMReserveAll: true, + }, + fail: true, + expectedErrMsg: "'RAM_reservation' and 'RAM_reserve_all' cannot be used together", + }, + { + name: "Invalid firmware", + config: &HardwareConfig{ + Firmware: "invalid", + }, + fail: true, + expectedErrMsg: "'firmware' must be '', 'bios', 'efi' or 'efi-secure'", + }, + { + name: "Validate 'bios' firmware", + config: &HardwareConfig{ + Firmware: "bios", + }, + fail: false, + }, + { + name: "Validate 'efi' firmware", + config: &HardwareConfig{ + Firmware: "efi", + }, + fail: false, + }, + { + name: "Validate 'efi-secure' firmware", + config: &HardwareConfig{ + Firmware: "efi-secure", + }, + fail: false, + }, + } + for _, c := range tc { + t.Run(c.name, func(t *testing.T) { + errs := c.config.Prepare() + if c.fail { + if len(errs) == 0 { + t.Fatalf("Config preprare should fail") + } + if errs[0].Error() != c.expectedErrMsg { + t.Fatalf("Expected error message: %s but was '%s'", c.expectedErrMsg, errs[0].Error()) + } + } else { + if len(errs) != 0 { + t.Fatalf("Config preprare should not fail") + } + } + }) + } +} + +func TestStepConfigureHardware_Run(t *testing.T) { + tc := []struct { + name string + step *StepConfigureHardware + action multistep.StepAction + configureError error + configureCalled bool + hardwareConfig *driver.HardwareConfig + }{ + { + name: "Configure hardware", + step: basicStepConfigureHardware(), + action: multistep.ActionContinue, + configureError: nil, + configureCalled: true, + hardwareConfig: driverHardwareConfigFromConfig(basicStepConfigureHardware().Config), + }, + { + name: "Don't configure hardware when config is empty", + step: &StepConfigureHardware{Config: &HardwareConfig{}}, + action: multistep.ActionContinue, + configureError: nil, + configureCalled: false, + }, + { + name: "Halt when configure return error", + step: basicStepConfigureHardware(), + action: multistep.ActionHalt, + configureError: errors.New("failed to configure"), + configureCalled: true, + hardwareConfig: driverHardwareConfigFromConfig(basicStepConfigureHardware().Config), + }, + } + for _, c := range tc { + t.Run(c.name, func(t *testing.T) { + state := basicStateBag(nil) + vmMock := new(driver.VirtualMachineMock) + vmMock.ConfigureError = c.configureError + state.Put("vm", vmMock) + + action := c.step.Run(context.TODO(), state) + if action != c.action { + t.Fatalf("expected action '%v' but actual action was '%v'", c.action, action) + } + if vmMock.ConfigureCalled != c.configureCalled { + t.Fatalf("expecting vm.Configure called to %t but was %t", c.configureCalled, vmMock.ConfigureCalled) + } + if diff := cmp.Diff(vmMock.ConfigureHardwareConfig, c.hardwareConfig); diff != "" { + t.Fatalf("wrong driver.HardwareConfig: %s", diff) + } + + err, ok := state.GetOk("error") + containsError := c.configureError != nil + if containsError != ok { + t.Fatalf("Contain error - expecting %t but was %t", containsError, ok) + } + if containsError { + if !strings.Contains(err.(error).Error(), c.configureError.Error()) { + t.Fatalf("Destroy should fail with error message '%s' but failed with '%s'", c.configureError.Error(), err.(error).Error()) + } + } + }) + } +} + +func basicStepConfigureHardware() *StepConfigureHardware { + return &StepConfigureHardware{ + Config: &HardwareConfig{ + CPUs: 1, + CpuCores: 1, + CPUReservation: 1, + CPULimit: 4000, + RAM: 1024, + RAMReserveAll: true, + Firmware: "efi-secure", + ForceBIOSSetup: true, + }, + } +} + +func driverHardwareConfigFromConfig(config *HardwareConfig) *driver.HardwareConfig { + return &driver.HardwareConfig{ + CPUs: config.CPUs, + CpuCores: config.CpuCores, + CPUReservation: config.CPUReservation, + CPULimit: config.CPULimit, + RAM: config.RAM, + RAMReservation: config.RAMReservation, + RAMReserveAll: config.RAMReserveAll, + NestedHV: config.NestedHV, + CpuHotAddEnabled: config.CpuHotAddEnabled, + MemoryHotAddEnabled: config.MemoryHotAddEnabled, + VideoRAM: config.VideoRAM, + VGPUProfile: config.VGPUProfile, + Firmware: config.Firmware, + ForceBIOSSetup: config.ForceBIOSSetup, + } +} diff --git a/builder/vsphere/driver/driver_test.go b/builder/vsphere/driver/driver_test.go index ea36ca337..a3818a34a 100644 --- a/builder/vsphere/driver/driver_test.go +++ b/builder/vsphere/driver/driver_test.go @@ -62,7 +62,7 @@ func NewSimulatorServer(model *simulator.Model) (*simulator.Server, error) { return model.Service.NewServer(), nil } -func NewSimulatorDriver(s *simulator.Server) (Driver, error) { +func NewSimulatorDriver(s *simulator.Server) (*VCenterDriver, error) { ctx := context.TODO() user := &url.Userinfo{} s.URL.User = user diff --git a/builder/vsphere/driver/vm_mock.go b/builder/vsphere/driver/vm_mock.go index cd7f12422..4af694d9d 100644 --- a/builder/vsphere/driver/vm_mock.go +++ b/builder/vsphere/driver/vm_mock.go @@ -16,6 +16,10 @@ import ( type VirtualMachineMock struct { DestroyError error DestroyCalled bool + + ConfigureError error + ConfigureCalled bool + ConfigureHardwareConfig *HardwareConfig } func (vm *VirtualMachineMock) Info(params ...string) (*mo.VirtualMachine, error) { @@ -51,6 +55,11 @@ func (vm *VirtualMachineMock) Destroy() error { } func (vm *VirtualMachineMock) Configure(config *HardwareConfig) error { + vm.ConfigureCalled = true + vm.ConfigureHardwareConfig = config + if vm.ConfigureError != nil { + return vm.ConfigureError + } return nil } diff --git a/builder/vsphere/driver/vm_test.go b/builder/vsphere/driver/vm_test.go new file mode 100644 index 000000000..e9d8a8917 --- /dev/null +++ b/builder/vsphere/driver/vm_test.go @@ -0,0 +1,74 @@ +package driver + +import ( + "testing" + + "github.com/vmware/govmomi/simulator" + "github.com/vmware/govmomi/vim25/methods" + "github.com/vmware/govmomi/vim25/soap" + "github.com/vmware/govmomi/vim25/types" +) + +// ReconfigureFail changes the behavior of simulator.VirtualMachine +type ReconfigureFail struct { + *simulator.VirtualMachine +} + +// Override simulator.VirtualMachine.ReconfigVMTask to inject faults +func (vm *ReconfigureFail) ReconfigVMTask(req *types.ReconfigVM_Task) soap.HasFault { + task := simulator.CreateTask(req.This, "reconfigure", func(*simulator.Task) (types.AnyType, types.BaseMethodFault) { + return nil, &types.TaskInProgress{} + }) + + return &methods.ReconfigVM_TaskBody{ + Res: &types.ReconfigVM_TaskResponse{ + Returnval: task.Run(), + }, + } +} + +func TestVirtualMachineDriver_Configure(t *testing.T) { + model := simulator.VPX() + model.Machine = 1 + defer model.Remove() + + s, err := NewSimulatorServer(model) + if err != nil { + t.Fatalf("should not fail: %s", err.Error()) + } + defer s.Close() + + driverSim, err := NewSimulatorDriver(s) + if err != nil { + t.Fatalf("should not fail: %s", err.Error()) + } + + //Simulator shortcut to choose any pre created VM. + machine := simulator.Map.Any("VirtualMachine").(*simulator.VirtualMachine) + ref := machine.Reference() + vm := driverSim.NewVM(&ref) + + // Happy test + hardwareConfig := &HardwareConfig{ + CPUs: 1, + CpuCores: 1, + CPUReservation: 2500, + CPULimit: 1, + RAM: 1024, + RAMReserveAll: true, + VideoRAM: 512, + VGPUProfile: "grid_m10-8q", + Firmware: "efi-secure", + ForceBIOSSetup: true, + } + if err = vm.Configure(hardwareConfig); err != nil { + t.Fatalf("should not fail: %s", err.Error()) + } + + //Fail test + //Wrap the existing vm object with the mocked reconfigure task which will return a fault + simulator.Map.Put(&ReconfigureFail{machine}) + if err = vm.Configure(&HardwareConfig{}); err == nil { + t.Fatalf("Configure should fail") + } +} diff --git a/builder/vsphere/iso/step_create_test.go b/builder/vsphere/iso/step_create_test.go index 814ff6877..dbe530778 100644 --- a/builder/vsphere/iso/step_create_test.go +++ b/builder/vsphere/iso/step_create_test.go @@ -160,7 +160,7 @@ func TestStepCreateVM_Run(t *testing.T) { t.Fatalf("driver.CreateVM should be called.") } if diff := cmp.Diff(driverMock.CreateConfig, driverCreateConfig(step.Config, step.Location)); diff != "" { - t.Fatalf("wrog driver.CreateConfig: %s", diff) + t.Fatalf("wrong driver.CreateConfig: %s", diff) } vm, ok := state.GetOk("vm") if !ok {