Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use cdroms len instead of ISOPaths len #394

Merged
merged 4 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions builder/vsphere/common/step_add_cdrom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand All @@ -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")),
Expand All @@ -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")),
Expand Down
7 changes: 6 additions & 1 deletion builder/vsphere/common/step_reattach_cdrom.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ func (s *StepReattachCDRom) Run(_ context.Context, state multistep.StateBag) mul

// Add the CD-ROM devices to the image based on the value of `reattach_cdroms`.
// A valid ISO path is required for this step. The media will subsequently be ejected.
nAttachableCdroms := ReattachCDRom - len(s.CDRomConfig.ISOPaths)
cdroms, err := vm.CdromDevices()
if err != nil {
state.Put("error listing cdrom devices: %v", err)
return multistep.ActionHalt
}
nAttachableCdroms := ReattachCDRom - len(cdroms)
if nAttachableCdroms < 0 {
err = vm.RemoveNCdroms(int(math.Abs(float64(nAttachableCdroms))))
if err != nil {
Expand Down
18 changes: 12 additions & 6 deletions builder/vsphere/common/step_reattach_cdrom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -40,17 +41,16 @@ func TestStepReattachCDRom_Run(t *testing.T) {
expectedAction: multistep.ActionContinue,
vmMock: &driver.VirtualMachineMock{
ReattachCDRomsCalled: true,
CdromDevicesList: object.VirtualDeviceList{nil},
},
expectedVmMock: &driver.VirtualMachineMock{
EjectCdromsCalled: true,
CdromDevicesCalled: true,
CdromDevicesList: object.VirtualDeviceList{nil, nil, nil, nil},
ReattachCDRomsCalled: true,
FindSATAControllerCalled: true,
AddCdromCalledTimes: 6,
AddCdromTypes: []string{
"sata", "sata",
"sata", "sata",
"sata", "sata",
},
AddCdromCalledTimes: 3,
AddCdromTypes: []string{"sata", "sata", "sata"},
},
fail: false,
},
Expand All @@ -73,12 +73,15 @@ 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,
CdromDevicesCalled: true,
ReattachCDRomsCalled: true,
AddCdromCalledTimes: 0,
CdromDevicesList: object.VirtualDeviceList{nil, nil},
},
fail: false,
},
Expand All @@ -99,11 +102,14 @@ func TestStepReattachCDRom_Run(t *testing.T) {
expectedAction: multistep.ActionContinue,
vmMock: &driver.VirtualMachineMock{
ReattachCDRomsCalled: true,
CdromDevicesList: object.VirtualDeviceList{nil, nil},
},
expectedVmMock: &driver.VirtualMachineMock{
EjectCdromsCalled: true,
CdromDevicesCalled: true,
ReattachCDRomsCalled: true,
AddCdromCalledTimes: 0,
CdromDevicesList: object.VirtualDeviceList{nil, nil},
},
fail: false,
},
Expand Down
10 changes: 10 additions & 0 deletions builder/vsphere/driver/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions builder/vsphere/driver/vm_cdrom.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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{}
Expand Down
12 changes: 4 additions & 8 deletions builder/vsphere/driver/vm_cdrom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand Down
16 changes: 16 additions & 0 deletions builder/vsphere/driver/vm_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand All @@ -280,6 +295,7 @@ func (vm *VirtualMachineMock) ReattachCDRoms() error {

func (vm *VirtualMachineMock) EjectCdroms() error {
vm.EjectCdromsCalled = true
vm.AddCdromPaths = nil
return vm.EjectCdromsErr
}

Expand Down
Loading