From 4093c352d36ef7109130342f02e620934b4b08c1 Mon Sep 17 00:00:00 2001 From: Konstantin Kharlamov Date: Thu, 21 Mar 2024 16:08:51 +0300 Subject: [PATCH] common: factor out a function to list cdrom devices It's a pattern that re-appears in the code, let's deduplicate that. --- builder/vsphere/common/step_add_cdrom_test.go | 5 +++++ .../vsphere/common/step_reattach_cdrom_test.go | 4 ++++ builder/vsphere/driver/vm.go | 10 ++++++++++ builder/vsphere/driver/vm_cdrom.go | 6 ++---- builder/vsphere/driver/vm_cdrom_test.go | 12 ++++-------- builder/vsphere/driver/vm_mock.go | 15 +++++++++++++++ 6 files changed, 40 insertions(+), 12 deletions(-) diff --git a/builder/vsphere/common/step_add_cdrom_test.go b/builder/vsphere/common/step_add_cdrom_test.go index 18018cfdd..22d88a799 100644 --- a/builder/vsphere/common/step_add_cdrom_test.go +++ b/builder/vsphere/common/step_add_cdrom_test.go @@ -12,6 +12,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/packer-plugin-sdk/multistep" "github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/driver" + "github.com/vmware/govmomi/object" ) func TestCDRomConfig_Prepare(t *testing.T) { @@ -97,6 +98,7 @@ func TestStepAddCDRom_Run(t *testing.T) { AddCdromCalledTimes: 3, AddCdromTypes: []string{"sata", "sata", "sata"}, AddCdromPaths: []string{"remote/path", "iso/path", "cd/path"}, + CdromDevicesList: object.VirtualDeviceList{nil, nil, nil}, }, fail: false, errMessage: "", @@ -156,6 +158,7 @@ func TestStepAddCDRom_Run(t *testing.T) { AddCdromCalledTimes: 1, AddCdromTypes: []string{"ide"}, AddCdromPaths: []string{"iso/path"}, + CdromDevicesList: object.VirtualDeviceList{nil}, }, fail: false, errMessage: "", @@ -176,6 +179,7 @@ func TestStepAddCDRom_Run(t *testing.T) { AddCdromCalledTimes: 1, AddCdromTypes: []string{""}, AddCdromPaths: []string{"iso/path"}, + CdromDevicesList: object.VirtualDeviceList{nil}, }, fail: true, errMessage: fmt.Sprintf("error mounting an image 'iso/path': %v", fmt.Errorf("AddCdrom error")), @@ -194,6 +198,7 @@ func TestStepAddCDRom_Run(t *testing.T) { AddCdromCalledTimes: 1, AddCdromTypes: []string{""}, AddCdromPaths: []string{"remote/path"}, + CdromDevicesList: object.VirtualDeviceList{nil}, }, fail: true, errMessage: fmt.Sprintf("error mounting an image 'remote/path': %v", fmt.Errorf("AddCdrom error")), diff --git a/builder/vsphere/common/step_reattach_cdrom_test.go b/builder/vsphere/common/step_reattach_cdrom_test.go index 86cb7bd42..cac8b4a32 100644 --- a/builder/vsphere/common/step_reattach_cdrom_test.go +++ b/builder/vsphere/common/step_reattach_cdrom_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/packer-plugin-sdk/multistep" "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/driver" + "github.com/vmware/govmomi/object" ) func TestStepReattachCDRom_Run(t *testing.T) { @@ -43,6 +44,7 @@ func TestStepReattachCDRom_Run(t *testing.T) { }, expectedVmMock: &driver.VirtualMachineMock{ EjectCdromsCalled: true, + CdromDevicesList: object.VirtualDeviceList{nil, nil, nil, nil, nil, nil}, ReattachCDRomsCalled: true, FindSATAControllerCalled: true, AddCdromCalledTimes: 6, @@ -73,12 +75,14 @@ func TestStepReattachCDRom_Run(t *testing.T) { expectedAction: multistep.ActionContinue, vmMock: &driver.VirtualMachineMock{ ReattachCDRomsCalled: true, + CdromDevicesList: object.VirtualDeviceList{nil, nil, nil, nil}, }, expectedVmMock: &driver.VirtualMachineMock{ RemoveNCdromsCalled: true, EjectCdromsCalled: true, ReattachCDRomsCalled: true, AddCdromCalledTimes: 0, + CdromDevicesList: object.VirtualDeviceList{nil, nil}, }, fail: false, }, diff --git a/builder/vsphere/driver/vm.go b/builder/vsphere/driver/vm.go index 5d0456a69..99aeec1ca 100644 --- a/builder/vsphere/driver/vm.go +++ b/builder/vsphere/driver/vm.go @@ -27,6 +27,7 @@ import ( type VirtualMachine interface { Info(params ...string) (*mo.VirtualMachine, error) Devices() (object.VirtualDeviceList, error) + CdromDevices() (object.VirtualDeviceList, error) FloppyDevices() (object.VirtualDeviceList, error) Clone(ctx context.Context, config *CloneConfig) (VirtualMachine, error) updateVAppConfig(ctx context.Context, newProps map[string]string) (*types.VmConfigSpec, error) @@ -313,6 +314,15 @@ func (vm *VirtualMachineDriver) FloppyDevices() (object.VirtualDeviceList, error return floppies, nil } +func (vm *VirtualMachineDriver) CdromDevices() (object.VirtualDeviceList, error) { + device, err := vm.Devices() + if err != nil { + return device, err + } + floppies := device.SelectByType((*types.VirtualCdrom)(nil)) + return floppies, nil +} + func (vm *VirtualMachineDriver) Clone(ctx context.Context, config *CloneConfig) (VirtualMachine, error) { folder, err := vm.driver.FindFolder(config.Folder) if err != nil { diff --git a/builder/vsphere/driver/vm_cdrom.go b/builder/vsphere/driver/vm_cdrom.go index 9c802976e..a794f7a56 100644 --- a/builder/vsphere/driver/vm_cdrom.go +++ b/builder/vsphere/driver/vm_cdrom.go @@ -80,11 +80,10 @@ func (vm *VirtualMachineDriver) RemoveNCdroms(n int) error { if n == 0 { return nil } - devices, err := vm.Devices() + cdroms, err := vm.CdromDevices() if err != nil { return err } - cdroms := devices.SelectByType((*types.VirtualCdrom)(nil)) if (n < 0) || (n > len(cdroms)) { return fmt.Errorf("invalid number: n must be between 0 and %d", len(cdroms)) } @@ -98,11 +97,10 @@ func (vm *VirtualMachineDriver) RemoveNCdroms(n int) error { } func (vm *VirtualMachineDriver) EjectCdroms() error { - devices, err := vm.Devices() + cdroms, err := vm.CdromDevices() if err != nil { return err } - cdroms := devices.SelectByType((*types.VirtualCdrom)(nil)) for _, cd := range cdroms { c := cd.(*types.VirtualCdrom) c.Backing = &types.VirtualCdromRemotePassthroughBackingInfo{} diff --git a/builder/vsphere/driver/vm_cdrom_test.go b/builder/vsphere/driver/vm_cdrom_test.go index f4c6d19bf..407681b17 100644 --- a/builder/vsphere/driver/vm_cdrom_test.go +++ b/builder/vsphere/driver/vm_cdrom_test.go @@ -75,11 +75,10 @@ func TestVirtualMachineDriver_CreateAndRemoveCdrom(t *testing.T) { } // Verify if CDROM was created - devices, err := vm.Devices() + cdroms, err := vm.CdromDevices() if err != nil { t.Fatalf("should not fail: %s", err.Error()) } - cdroms := devices.SelectByType((*types.VirtualCdrom)(nil)) if len(cdroms) != 1 { t.Fatalf("unexpected numbers of cdrom: %d", len(cdroms)) } @@ -90,11 +89,10 @@ func TestVirtualMachineDriver_CreateAndRemoveCdrom(t *testing.T) { t.Fatalf("should not fail: %s", err.Error()) } // Verify if CDROM was removed - devices, err = vm.Devices() + cdroms, err = vm.CdromDevices() if err != nil { t.Fatalf("should not fail: %s", err.Error()) } - cdroms = devices.SelectByType((*types.VirtualCdrom)(nil)) if len(cdroms) != 0 { t.Fatalf("unexpected numbers of cdrom: %d", len(cdroms)) } @@ -134,11 +132,10 @@ func TestVirtualMachineDriver_EjectCdrom(t *testing.T) { } // Verify if CDROM was created - devices, err := vm.Devices() + cdroms, err := vm.CdromDevices() if err != nil { t.Fatalf("should not fail: %s", err.Error()) } - cdroms := devices.SelectByType((*types.VirtualCdrom)(nil)) if len(cdroms) != 1 { t.Fatalf("unexpected numbers of cdrom: %d", len(cdroms)) } @@ -149,11 +146,10 @@ func TestVirtualMachineDriver_EjectCdrom(t *testing.T) { t.Fatalf("should not fail: %s", err.Error()) } // Verify if CDROM was removed - devices, err = vm.Devices() + cdroms, err = vm.CdromDevices() if err != nil { t.Fatalf("should not fail: %s", err.Error()) } - cdroms = devices.SelectByType((*types.VirtualCdrom)(nil)) if len(cdroms) != 1 { t.Fatalf("unexpected numbers of cdrom: %d", len(cdroms)) } diff --git a/builder/vsphere/driver/vm_mock.go b/builder/vsphere/driver/vm_mock.go index 3c1637202..6a9ab72ca 100644 --- a/builder/vsphere/driver/vm_mock.go +++ b/builder/vsphere/driver/vm_mock.go @@ -53,6 +53,10 @@ type VirtualMachineMock struct { FloppyDevicesReturn object.VirtualDeviceList FloppyDevicesCalled bool + CdromDevicesErr error + CdromDevicesList object.VirtualDeviceList + CdromDevicesCalled bool + RemoveDeviceErr error RemoveDeviceCalled bool RemoveDeviceKeepFiles bool @@ -88,6 +92,11 @@ func (vm *VirtualMachineMock) FloppyDevices() (object.VirtualDeviceList, error) return vm.FloppyDevicesReturn, vm.FloppyDevicesErr } +func (vm *VirtualMachineMock) CdromDevices() (object.VirtualDeviceList, error) { + vm.CdromDevicesCalled = true + return vm.CdromDevicesList, vm.CdromDevicesErr +} + func (vm *VirtualMachineMock) Clone(ctx context.Context, config *CloneConfig) (VirtualMachine, error) { vm.CloneCalled = true vm.CloneConfig = config @@ -191,6 +200,7 @@ func (vm *VirtualMachineMock) GetDir() (string, error) { func (vm *VirtualMachineMock) AddCdrom(controllerType string, isoPath string) error { vm.AddCdromCalledTimes++ vm.AddCdromTypes = append(vm.AddCdromTypes, controllerType) + vm.CdromDevicesList = append(vm.CdromDevicesList, nil) if isoPath != "" { vm.AddCdromPaths = append(vm.AddCdromPaths, isoPath) } @@ -266,11 +276,16 @@ func (vm *VirtualMachineMock) CreateCdrom(c *types.VirtualController) (*types.Vi func (vm *VirtualMachineMock) RemoveCdroms() error { vm.RemoveCdromsCalled = true + vm.CdromDevicesList = nil return vm.RemoveCdromsErr } func (vm *VirtualMachineMock) RemoveNCdroms(n_cdroms int) error { vm.RemoveNCdromsCalled = true + if n_cdroms == 0 { + return nil + } + vm.CdromDevicesList = vm.CdromDevicesList[:n_cdroms] return vm.RemoveNCdromsErr }