fix: use cdroms len instead of ISOPaths len (#394)

* common: factor out a function to list cdrom devices
* common: use cdroms len instead of ISOPaths len
* driver/vm_mock: nil-out added ISO paths in EjectCdroms
* docs: Add the documentation for `reattach_cdroms` to the 
generated documentation.


---------

Signed-off-by: Ryan Johnson <ryan.johnson@broadcom.com>
Co-authored-by: Ryan Johnson <ryan.johnson@broadcom.com>

Author: Hi-Angel

.web-docs/components/builder/vsphere-iso/README.md

@@ -702,6 +702,17 @@ iso_paths = [
 <!-- End of code generated from the comments of the RemoveCDRomConfig struct in builder/vsphere/common/step_remove_cdrom.go; -->
 
 
+<!-- Code generated from the comments of the ReattachCDRomConfig struct in builder/vsphere/common/step_reattach_cdrom.go; DO NOT EDIT MANUALLY -->
+
+- `reattach_cdroms` (int) - Reattach one or more configured CD-ROM devices. Range: 1-4.
+  You can reattach up to 4 CD-ROM devices to the final build artifact.
+  If set to 0, `reattach_cdroms` is ignored and the step is skipped.
+  When set to a value in the range, `remove_cdrom` is ignored and
+  the CD-ROM devices are kept without any attached media.
+
+<!-- End of code generated from the comments of the ReattachCDRomConfig struct in builder/vsphere/common/step_reattach_cdrom.go; -->
+
+
 <!-- Code generated from the comments of the CDConfig struct in multistep/commonsteps/extra_iso_config.go; DO NOT EDIT MANUALLY -->
 
 An iso (CD) containing custom files can be made available for your build.

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")),

builder/vsphere/common/step_reattach_cdrom.go

@@ -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 {

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) {
@@ -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,
 		},
@@ -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,
 		},
@@ -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,
 		},

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 {

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{}

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))
 	}

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
 }
 
@@ -280,6 +295,7 @@ func (vm *VirtualMachineMock) ReattachCDRoms() error {
 
 func (vm *VirtualMachineMock) EjectCdroms() error {
 	vm.EjectCdromsCalled = true
+	vm.AddCdromPaths = nil
 	return vm.EjectCdromsErr
 }
 

docs/builders/vsphere-iso.mdx

@@ -168,6 +168,8 @@ iso_paths = [
 
 @include 'builder/vsphere/common/RemoveCDRomConfig-not-required.mdx'
 
+@include 'builder/vsphere/common/ReattachCDRomConfig-not-required.mdx'
+
 @include 'packer-plugin-sdk/multistep/commonsteps/CDConfig.mdx'
 
 ## Optional: