diff --git a/builder/vsphere/clone/step_clone.go b/builder/vsphere/clone/step_clone.go index d29fefc61..c0def3269 100644 --- a/builder/vsphere/clone/step_clone.go +++ b/builder/vsphere/clone/step_clone.go @@ -75,7 +75,7 @@ type StepCloneVM struct { func (s *StepCloneVM) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { ui := state.Get("ui").(packersdk.Ui) - d := state.Get("driver").(*driver.VCenterDriver) + d := state.Get("driver").(driver.Driver) vmPath := path.Join(s.Location.Folder, s.Location.VMName) err := d.PreCleanVM(ui, vmPath, s.Force) @@ -102,17 +102,18 @@ func (s *StepCloneVM) Run(ctx context.Context, state multistep.StateBag) multist } vm, err := template.Clone(ctx, &driver.CloneConfig{ - Name: s.Location.VMName, - Folder: s.Location.Folder, - Cluster: s.Location.Cluster, - Host: s.Location.Host, - ResourcePool: s.Location.ResourcePool, - Datastore: s.Location.Datastore, - LinkedClone: s.Config.LinkedClone, - Network: s.Config.Network, - MacAddress: s.Config.MacAddress, - Annotation: s.Config.Notes, - VAppProperties: s.Config.VAppConfig.Properties, + Name: s.Location.VMName, + Folder: s.Location.Folder, + Cluster: s.Location.Cluster, + Host: s.Location.Host, + ResourcePool: s.Location.ResourcePool, + Datastore: s.Location.Datastore, + LinkedClone: s.Config.LinkedClone, + Network: s.Config.Network, + MacAddress: s.Config.MacAddress, + Annotation: s.Config.Notes, + VAppProperties: s.Config.VAppConfig.Properties, + PrimaryDiskSize: s.Config.DiskSize, StorageConfig: driver.StorageConfig{ DiskControllerType: s.Config.StorageConfig.DiskControllerType, Storage: disks, @@ -127,14 +128,6 @@ func (s *StepCloneVM) Run(ctx context.Context, state multistep.StateBag) multist } state.Put("vm", vm) - if s.Config.DiskSize > 0 { - err = vm.ResizeDisk(s.Config.DiskSize) - if err != nil { - state.Put("error", err) - return multistep.ActionHalt - } - } - return multistep.ActionContinue } diff --git a/builder/vsphere/clone/step_clone_test.go b/builder/vsphere/clone/step_clone_test.go new file mode 100644 index 000000000..9b23ea443 --- /dev/null +++ b/builder/vsphere/clone/step_clone_test.go @@ -0,0 +1,251 @@ +package clone + +import ( + "bytes" + "context" + "path" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/packer-plugin-sdk/multistep" + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer/builder/vsphere/common" + "github.com/hashicorp/packer/builder/vsphere/driver" +) + +func TestCreateConfig_Prepare(t *testing.T) { + tc := []struct { + name string + config *CloneConfig + fail bool + expectedErrMsg string + }{ + { + name: "Valid config", + config: &CloneConfig{ + Template: "template name", + StorageConfig: common.StorageConfig{ + DiskControllerType: []string{"test"}, + Storage: []common.DiskConfig{ + { + DiskSize: 0, + }, + }, + }, + }, + fail: true, + expectedErrMsg: "storage[0].'disk_size' is required", + }, + { + name: "Storage validate disk_size", + config: &CloneConfig{ + StorageConfig: common.StorageConfig{ + Storage: []common.DiskConfig{ + { + DiskSize: 0, + DiskThinProvisioned: true, + }, + }, + }, + }, + fail: true, + expectedErrMsg: "storage[0].'disk_size' is required", + }, + { + name: "Storage validate disk_controller_index", + config: &CloneConfig{ + StorageConfig: common.StorageConfig{ + Storage: []common.DiskConfig{ + { + DiskSize: 32768, + DiskControllerIndex: 3, + }, + }, + }, + }, + fail: true, + expectedErrMsg: "storage[0].'disk_controller_index' references an unknown disk controller", + }, + { + name: "Validate template is set", + config: &CloneConfig{ + StorageConfig: common.StorageConfig{ + DiskControllerType: []string{"test"}, + Storage: []common.DiskConfig{ + { + DiskSize: 32768, + }, + }, + }, + }, + fail: true, + expectedErrMsg: "'template' is required", + }, + { + name: "Validate LinkedClone and DiskSize set at the same time", + config: &CloneConfig{ + Template: "template name", + LinkedClone: true, + DiskSize: 32768, + StorageConfig: common.StorageConfig{ + DiskControllerType: []string{"test"}, + Storage: []common.DiskConfig{ + { + DiskSize: 32768, + }, + }, + }, + }, + fail: true, + expectedErrMsg: "'linked_clone' and 'disk_size' cannot be used together", + }, + { + name: "Validate MacAddress and Network not set at the same time", + config: &CloneConfig{ + Template: "template name", + MacAddress: "some mac address", + StorageConfig: common.StorageConfig{ + DiskControllerType: []string{"test"}, + Storage: []common.DiskConfig{ + { + DiskSize: 32768, + }, + }, + }, + }, + fail: true, + expectedErrMsg: "'network' is required when 'mac_address' is specified", + }, + } + + 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: %s", errs[0]) + } + } + }) + } +} + +func TestStepCreateVM_Run(t *testing.T) { + state := new(multistep.BasicStateBag) + state.Put("ui", &packersdk.BasicUi{ + Reader: new(bytes.Buffer), + Writer: new(bytes.Buffer), + }) + driverMock := driver.NewDriverMock() + state.Put("driver", driverMock) + step := basicStepCloneVM() + step.Force = true + vmPath := path.Join(step.Location.Folder, step.Location.VMName) + vmMock := new(driver.VirtualMachineMock) + driverMock.VM = vmMock + + if action := step.Run(context.TODO(), state); action == multistep.ActionHalt { + t.Fatalf("Should not halt.") + } + + // Pre clean VM + if !driverMock.PreCleanVMCalled { + t.Fatalf("driver.PreCleanVM should be called.") + } + if driverMock.PreCleanForce != step.Force { + t.Fatalf("Force PreCleanVM should be %t but was %t.", step.Force, driverMock.PreCleanForce) + } + if driverMock.PreCleanVMPath != vmPath { + t.Fatalf("VM path expected to be %s but was %s", vmPath, driverMock.PreCleanVMPath) + } + + if !driverMock.FindVMCalled { + t.Fatalf("driver.FindVM should be called.") + } + if !vmMock.CloneCalled { + t.Fatalf("vm.Clone should be called.") + } + + if diff := cmp.Diff(vmMock.CloneConfig, driverCreateConfig(step.Config, step.Location)); diff != "" { + t.Fatalf("wrong driver.CreateConfig: %s", diff) + } + vm, ok := state.GetOk("vm") + if !ok { + t.Fatal("state must contain the VM") + } + if vm != driverMock.VM { + t.Fatalf("state doesn't contain the created VM.") + } +} + +func basicStepCloneVM() *StepCloneVM { + step := &StepCloneVM{ + Config: createConfig(), + Location: basicLocationConfig(), + } + return step +} + +func basicLocationConfig() *common.LocationConfig { + return &common.LocationConfig{ + VMName: "test-vm", + Folder: "test-folder", + Cluster: "test-cluster", + Host: "test-host", + ResourcePool: "test-resource-pool", + Datastore: "test-datastore", + } +} + +func createConfig() *CloneConfig { + return &CloneConfig{ + Template: "template name", + StorageConfig: common.StorageConfig{ + DiskControllerType: []string{"pvscsi"}, + Storage: []common.DiskConfig{ + { + DiskSize: 32768, + DiskThinProvisioned: true, + }, + }, + }, + } +} + +func driverCreateConfig(config *CloneConfig, location *common.LocationConfig) *driver.CloneConfig { + var disks []driver.Disk + for _, disk := range config.StorageConfig.Storage { + disks = append(disks, driver.Disk{ + DiskSize: disk.DiskSize, + DiskEagerlyScrub: disk.DiskEagerlyScrub, + DiskThinProvisioned: disk.DiskThinProvisioned, + ControllerIndex: disk.DiskControllerIndex, + }) + } + + return &driver.CloneConfig{ + StorageConfig: driver.StorageConfig{ + DiskControllerType: config.StorageConfig.DiskControllerType, + Storage: disks, + }, + Annotation: config.Notes, + Name: location.VMName, + Folder: location.Folder, + Cluster: location.Cluster, + Host: location.Host, + ResourcePool: location.ResourcePool, + Datastore: location.Datastore, + LinkedClone: config.LinkedClone, + Network: config.Network, + MacAddress: config.MacAddress, + VAppProperties: config.VAppConfig.Properties, + PrimaryDiskSize: config.DiskSize, + } +} diff --git a/builder/vsphere/driver/driver_mock.go b/builder/vsphere/driver/driver_mock.go index f8fd6ad79..03fa736fb 100644 --- a/builder/vsphere/driver/driver_mock.go +++ b/builder/vsphere/driver/driver_mock.go @@ -24,6 +24,9 @@ type DriverMock struct { CreateVMCalled bool CreateConfig *CreateConfig VM VirtualMachine + + FindVMCalled bool + FindVMName string } func NewDriverMock() *DriverMock { @@ -45,7 +48,12 @@ func (d *DriverMock) NewVM(ref *types.ManagedObjectReference) VirtualMachine { } func (d *DriverMock) FindVM(name string) (VirtualMachine, error) { - return nil, nil + d.FindVMCalled = true + if d.VM == nil { + d.VM = new(VirtualMachineMock) + } + d.FindVMName = name + return d.VM, d.FindDatastoreErr } func (d *DriverMock) FindCluster(name string) (*Cluster, error) { diff --git a/builder/vsphere/driver/vm.go b/builder/vsphere/driver/vm.go index ff4b8beb2..0c739d108 100644 --- a/builder/vsphere/driver/vm.go +++ b/builder/vsphere/driver/vm.go @@ -32,7 +32,7 @@ type VirtualMachine interface { Destroy() error Configure(config *HardwareConfig) error Customize(spec types.CustomizationSpec) error - ResizeDisk(diskSize int64) error + ResizeDisk(diskSize int64) ([]types.BaseVirtualDeviceConfigSpec, error) WaitForIP(ctx context.Context, ipNet *net.IPNet) (string, error) PowerOn() error PowerOff() error @@ -68,18 +68,19 @@ type VirtualMachineDriver struct { } type CloneConfig struct { - Name string - Folder string - Cluster string - Host string - ResourcePool string - Datastore string - LinkedClone bool - Network string - MacAddress string - Annotation string - VAppProperties map[string]string - StorageConfig StorageConfig + Name string + Folder string + Cluster string + Host string + ResourcePool string + Datastore string + LinkedClone bool + Network string + MacAddress string + Annotation string + VAppProperties map[string]string + PrimaryDiskSize int64 + StorageConfig StorageConfig } type HardwareConfig struct { @@ -339,6 +340,15 @@ func (vm *VirtualMachineDriver) Clone(ctx context.Context, config *CloneConfig) if err != nil { return nil, err } + + if config.PrimaryDiskSize > 0 { + deviceResizeSpec, err := vm.ResizeDisk(config.PrimaryDiskSize) + if err != nil { + return nil, fmt.Errorf("failed to resize primary disk: %s", err.Error()) + } + configSpec.DeviceChange = append(configSpec.DeviceChange, deviceResizeSpec...) + } + virtualDisks := devices.SelectByType((*types.VirtualDisk)(nil)) virtualControllers := devices.SelectByType((*types.VirtualController)(nil)) @@ -349,7 +359,7 @@ func (vm *VirtualMachineDriver) Clone(ctx context.Context, config *CloneConfig) storageConfigSpec, err := config.StorageConfig.AddStorageDevices(existingDevices) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to add storage devices: %s", err.Error()) } configSpec.DeviceChange = append(configSpec.DeviceChange, storageConfigSpec...) @@ -597,35 +607,25 @@ func (vm *VirtualMachineDriver) Customize(spec types.CustomizationSpec) error { return task.Wait(vm.driver.ctx) } -func (vm *VirtualMachineDriver) ResizeDisk(diskSize int64) error { - var confSpec types.VirtualMachineConfigSpec - +func (vm *VirtualMachineDriver) ResizeDisk(diskSize int64) ([]types.BaseVirtualDeviceConfigSpec, error) { devices, err := vm.vm.Device(vm.driver.ctx) if err != nil { - return err + return nil, err } disk, err := findDisk(devices) if err != nil { - return err + return nil, err } disk.CapacityInKB = diskSize * 1024 - confSpec.DeviceChange = []types.BaseVirtualDeviceConfigSpec{ + return []types.BaseVirtualDeviceConfigSpec{ &types.VirtualDeviceConfigSpec{ Device: disk, Operation: types.VirtualDeviceConfigSpecOperationEdit, }, - } - - task, err := vm.vm.Reconfigure(vm.driver.ctx, confSpec) - if err != nil { - return err - } - - _, err = task.WaitForResult(vm.driver.ctx, nil) - return err + }, nil } func (vm *VirtualMachineDriver) PowerOn() error { diff --git a/builder/vsphere/driver/vm_mock.go b/builder/vsphere/driver/vm_mock.go index 5251b2030..daf62b3c9 100644 --- a/builder/vsphere/driver/vm_mock.go +++ b/builder/vsphere/driver/vm_mock.go @@ -55,6 +55,9 @@ type VirtualMachineMock struct { RemoveCdromsCalled bool RemoveCdromsErr error + CloneCalled bool + CloneConfig *CloneConfig + CloneError error } func (vm *VirtualMachineMock) Info(params ...string) (*mo.VirtualMachine, error) { @@ -71,7 +74,9 @@ func (vm *VirtualMachineMock) FloppyDevices() (object.VirtualDeviceList, error) } func (vm *VirtualMachineMock) Clone(ctx context.Context, config *CloneConfig) (VirtualMachine, error) { - return nil, nil + vm.CloneCalled = true + vm.CloneConfig = config + return vm, vm.CloneError } func (vm *VirtualMachineMock) updateVAppConfig(ctx context.Context, newProps map[string]string) (*types.VmConfigSpec, error) { @@ -107,8 +112,8 @@ func (vm *VirtualMachineMock) Customize(spec types.CustomizationSpec) error { return nil } -func (vm *VirtualMachineMock) ResizeDisk(diskSize int64) error { - return nil +func (vm *VirtualMachineMock) ResizeDisk(diskSize int64) ([]types.BaseVirtualDeviceConfigSpec, error) { + return nil, nil } func (vm *VirtualMachineMock) PowerOn() error { diff --git a/builder/vsphere/driver/vm_test.go b/builder/vsphere/driver/vm_test.go index d598a0dab..186538974 100644 --- a/builder/vsphere/driver/vm_test.go +++ b/builder/vsphere/driver/vm_test.go @@ -1,6 +1,7 @@ package driver import ( + "context" "testing" "github.com/vmware/govmomi/simulator" @@ -61,7 +62,7 @@ func TestVirtualMachineDriver_Configure(t *testing.T) { } } -func TestVirtualMachineDriver_CreateVM(t *testing.T) { +func TestVirtualMachineDriver_CreateVMWithMultipleDisks(t *testing.T) { sim, err := NewVCenterSimulator() if err != nil { t.Fatalf("should not fail: %s", err.Error()) @@ -71,10 +72,9 @@ func TestVirtualMachineDriver_CreateVM(t *testing.T) { _, datastore := sim.ChooseSimulatorPreCreatedDatastore() config := &CreateConfig{ - Annotation: "mock annotation", - Name: "mock name", - Host: "DC0_H0", - Datastore: datastore.Name, + Name: "mock name", + Host: "DC0_H0", + Datastore: datastore.Name, NICs: []NIC{ { Network: "VM Network", @@ -98,8 +98,90 @@ func TestVirtualMachineDriver_CreateVM(t *testing.T) { }, } - _, err = sim.driver.CreateVM(config) + vm, err := sim.driver.CreateVM(config) if err != nil { t.Fatalf("unexpected error %s", err.Error()) } + + devices, err := vm.Devices() + if err != nil { + t.Fatalf("unexpected error %s", err.Error()) + } + + var disks []*types.VirtualDisk + for _, device := range devices { + switch d := device.(type) { + case *types.VirtualDisk: + disks = append(disks, d) + } + } + + if len(disks) != 2 { + t.Fatalf("unexpected number of devices") + } +} + +func TestVirtualMachineDriver_CloneWithPrimaryDiskResize(t *testing.T) { + sim, err := NewVCenterSimulator() + if err != nil { + t.Fatalf("should not fail: %s", err.Error()) + } + defer sim.Close() + + _, datastore := sim.ChooseSimulatorPreCreatedDatastore() + vm, _ := sim.ChooseSimulatorPreCreatedVM() + + config := &CloneConfig{ + Name: "mock name", + Host: "DC0_H0", + Datastore: datastore.Name, + PrimaryDiskSize: 204800, + StorageConfig: StorageConfig{ + DiskControllerType: []string{"pvscsi"}, + Storage: []Disk{ + { + DiskSize: 3072, + DiskThinProvisioned: true, + ControllerIndex: 0, + }, + { + DiskSize: 20480, + DiskThinProvisioned: true, + ControllerIndex: 0, + }, + }, + }, + } + + clonedVM, err := vm.Clone(context.TODO(), config) + if err != nil { + t.Fatalf("unexpected error %s", err.Error()) + } + + devices, err := clonedVM.Devices() + if err != nil { + t.Fatalf("unexpected error %s", err.Error()) + } + + var disks []*types.VirtualDisk + for _, device := range devices { + switch d := device.(type) { + case *types.VirtualDisk: + disks = append(disks, d) + } + } + + if len(disks) != 3 { + t.Fatalf("unexpected number of devices") + } + + if disks[0].CapacityInKB != config.PrimaryDiskSize*1024 { + t.Fatalf("unexpected disk size for primary disk: %d", disks[0].CapacityInKB) + } + if disks[1].CapacityInKB != config.StorageConfig.Storage[0].DiskSize*1024 { + t.Fatalf("unexpected disk size for primary disk: %d", disks[1].CapacityInKB) + } + if disks[2].CapacityInKB != config.StorageConfig.Storage[1].DiskSize*1024 { + t.Fatalf("unexpected disk size for primary disk: %d", disks[2].CapacityInKB) + } }