vSphere StepAddCDRom tests (#9955)

builder/vsphere/driver/driver_test.go

@@ -104,3 +104,11 @@ func NewSimulatorDriver(s *simulator.Server) (*VCenterDriver, error) {
 	}
 	return d, nil
 }
+
+//Simulator shortcut to choose any pre created VM.
+func ChooseSimulatorPreCreatedVM(driverSim *VCenterDriver) VirtualMachine {
+	machine := simulator.Map.Any("VirtualMachine").(*simulator.VirtualMachine)
+	ref := machine.Reference()
+	vm := driverSim.NewVM(&ref)
+	return vm
+}

builder/vsphere/driver/vm.go

@@ -32,8 +32,8 @@ type VirtualMachine interface {
 	Configure(config *HardwareConfig) error
 	Customize(spec types.CustomizationSpec) error
 	ResizeDisk(diskSize int64) error
-	PowerOn() error
 	WaitForIP(ctx context.Context, ipNet *net.IPNet) (string, error)
+	PowerOn() error
 	PowerOff() error
 	IsPoweredOff() (bool, error)
 	StartShutdown() error
@@ -43,7 +43,6 @@ type VirtualMachine interface {
 	ImportOvfToContentLibrary(ovf vcenter.OVF) error
 	ImportToContentLibrary(template vcenter.Template) error
 	GetDir() (string, error)
-	AddCdrom(controllerType string, datastoreIsoPath string) error
 	AddFloppy(imgPath string) error
 	SetBootOrder(order []string) error
 	RemoveDevice(keepFiles bool, device ...types.BaseVirtualDevice) error
@@ -53,6 +52,13 @@ type VirtualMachine interface {
 	CreateDescriptor(m *ovf.Manager, cdp types.OvfCreateDescriptorParams) (*types.OvfCreateDescriptorResult, error)
 	NewOvfManager() *ovf.Manager
 	GetOvfExportOptions(m *ovf.Manager) ([]types.OvfOptionInfo, error)
+
+	AddCdrom(controllerType string, datastoreIsoPath string) error
+	CreateCdrom(c *types.VirtualController) (*types.VirtualCdrom, error)
+	RemoveCdroms() error
+	EjectCdroms() error
+	AddSATAController() error
+	FindSATAController() (*types.VirtualAHCIController, error)
 }
 
 type VirtualMachineDriver struct {

builder/vsphere/driver/vm_cdrom_test.go

@@ -0,0 +1,197 @@
+package driver
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/vmware/govmomi/simulator"
+	"github.com/vmware/govmomi/vim25/types"
+)
+
+func TestVirtualMachineDriver_FindAndAddSATAController(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())
+	}
+
+	vm := ChooseSimulatorPreCreatedVM(driverSim)
+
+	_, err = vm.FindSATAController()
+	if err != nil && !strings.Contains(err.Error(), "no available SATA controller") {
+		t.Fatalf("unexpected error: %s", err.Error())
+	}
+	if err == nil {
+		t.Fatalf("vm should not have sata controller")
+	}
+
+	if err := vm.AddSATAController(); err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+
+	sc, err := vm.FindSATAController()
+	if err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+	if sc == nil {
+		t.Fatalf("SATA controller wasn't added properly")
+	}
+}
+
+func TestVirtualMachineDriver_CreateAndRemoveCdrom(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.
+	vm := ChooseSimulatorPreCreatedVM(driverSim)
+
+	// Add SATA Controller
+	if err := vm.AddSATAController(); err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+
+	// Verify if controller was created
+	sc, err := vm.FindSATAController()
+	if err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+	if sc == nil {
+		t.Fatalf("SATA controller wasn't added properly")
+	}
+
+	// Create CDROM
+	controller := sc.GetVirtualController()
+	cdrom, err := vm.CreateCdrom(controller)
+	if err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+	if cdrom == nil {
+		t.Fatalf("CDrom wasn't created properly")
+	}
+
+	// Verify if CDROM was created
+	devices, err := vm.Devices()
+	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))
+	}
+
+	// Remove CDROM
+	err = vm.RemoveCdroms()
+	if err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+	// Verify if CDROM was removed
+	devices, err = vm.Devices()
+	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))
+	}
+}
+
+func TestVirtualMachineDriver_EjectCdrom(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.
+	vm := ChooseSimulatorPreCreatedVM(driverSim)
+
+	// Add SATA Controller
+	if err := vm.AddSATAController(); err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+
+	// Verify if controller was created
+	sc, err := vm.FindSATAController()
+	if err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+	if sc == nil {
+		t.Fatalf("SATA controller wasn't added properly")
+	}
+
+	// Create CDROM
+	controller := sc.GetVirtualController()
+	cdrom, err := vm.CreateCdrom(controller)
+	if err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+	if cdrom == nil {
+		t.Fatalf("CDrom wasn't created properly")
+	}
+
+	// Verify if CDROM was created
+	devices, err := vm.Devices()
+	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))
+	}
+
+	// Remove CDROM
+	err = vm.EjectCdroms()
+	if err != nil {
+		t.Fatalf("should not fail: %s", err.Error())
+	}
+	// Verify if CDROM was removed
+	devices, err = vm.Devices()
+	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))
+	}
+	cd, ok := cdroms[0].(*types.VirtualCdrom)
+	if !ok {
+		t.Fatalf("Wrong cdrom type")
+	}
+	if diff := cmp.Diff(cd.Backing, &types.VirtualCdromRemotePassthroughBackingInfo{}); diff != "" {
+		t.Fatalf("Wrong cdrom backing info: %s", diff)
+	}
+	if diff := cmp.Diff(cd.Connectable, &types.VirtualDeviceConnectInfo{}); diff != "" {
+		t.Fatalf("Wrong cdrom connect info: %s", diff)
+	}
+}

builder/vsphere/driver/vm_mock.go

@@ -20,6 +20,18 @@ type VirtualMachineMock struct {
 	ConfigureError          error
 	ConfigureCalled         bool
 	ConfigureHardwareConfig *HardwareConfig
+
+	FindSATAControllerCalled bool
+	FindSATAControllerErr    error
+
+	AddSATAControllerCalled bool
+	AddSATAControllerErr    error
+
+	AddCdromCalled      bool
+	AddCdromCalledTimes int
+	AddCdromErr         error
+	AddCdromTypes       []string
+	AddCdromPaths       []string
 }
 
 func (vm *VirtualMachineMock) Info(params ...string) (*mo.VirtualMachine, error) {
@@ -115,8 +127,12 @@ func (vm *VirtualMachineMock) GetDir() (string, error) {
 	return "", nil
 }
 
-func (vm *VirtualMachineMock) AddCdrom(controllerType string, datastoreIsoPath string) error {
+func (vm *VirtualMachineMock) AddCdrom(cdromType string, isoPath string) error {
-	return nil
+	vm.AddCdromCalledTimes++
+	vm.AddCdromCalled = true
+	vm.AddCdromTypes = append(vm.AddCdromTypes, cdromType)
+	vm.AddCdromPaths = append(vm.AddCdromPaths, isoPath)
+	return vm.AddCdromErr
 }
 
 func (vm *VirtualMachineMock) AddFloppy(imgPath string) error {
@@ -154,3 +170,25 @@ func (vm *VirtualMachineMock) NewOvfManager() *ovf.Manager {
 func (vm *VirtualMachineMock) GetOvfExportOptions(m *ovf.Manager) ([]types.OvfOptionInfo, error) {
 	return nil, nil
 }
+
+func (vm *VirtualMachineMock) AddSATAController() error {
+	vm.AddSATAControllerCalled = true
+	return vm.AddSATAControllerErr
+}
+
+func (vm *VirtualMachineMock) FindSATAController() (*types.VirtualAHCIController, error) {
+	vm.FindSATAControllerCalled = true
+	return nil, vm.FindSATAControllerErr
+}
+
+func (vm *VirtualMachineMock) CreateCdrom(c *types.VirtualController) (*types.VirtualCdrom, error) {
+	return nil, nil
+}
+
+func (vm *VirtualMachineMock) RemoveCdroms() error {
+	return nil
+}
+
+func (vm *VirtualMachineMock) EjectCdroms() error {
+	return nil
+}

builder/vsphere/iso/step_add_cdrom.go

@@ -42,7 +42,7 @@ func (c *CDRomConfig) Prepare() []error {
 
 func (s *StepAddCDRom) 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.CdromType == "sata" {
 		if _, err := vm.FindSATAController(); err == driver.ErrNoSataController {
@@ -63,11 +63,8 @@ func (s *StepAddCDRom) Run(_ context.Context, state multistep.StateBag) multiste
 	}
 
 	// Add our custom CD, if it exists
-	cd_path, ok := state.Get("cd_path").(string)
+	if cd_path, _ := state.Get("cd_path").(string); cd_path != "" {
-	if ok {
+		s.Config.ISOPaths = append(s.Config.ISOPaths, cd_path)
-		if cd_path != "" {
-			s.Config.ISOPaths = append(s.Config.ISOPaths, cd_path)
-		}
 	}
 
 	if len(s.Config.ISOPaths) > 0 {

builder/vsphere/iso/step_add_cdrom_test.go

@@ -0,0 +1,235 @@
+package iso
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/hashicorp/packer/builder/vsphere/driver"
+	"github.com/hashicorp/packer/helper/multistep"
+)
+
+func TestCDRomConfig_Prepare(t *testing.T) {
+	// Data validation
+	tc := []struct {
+		name           string
+		config         *CDRomConfig
+		fail           bool
+		expectedErrMsg string
+	}{
+		{
+			name:           "Should not fail for empty config",
+			config:         new(CDRomConfig),
+			fail:           false,
+			expectedErrMsg: "",
+		},
+		{
+			name:           "Valid cdroom type ide",
+			config:         &CDRomConfig{CdromType: "ide"},
+			fail:           false,
+			expectedErrMsg: "",
+		},
+		{
+			name:           "Valid cdroom type sata",
+			config:         &CDRomConfig{CdromType: "ide"},
+			fail:           false,
+			expectedErrMsg: "",
+		},
+		{
+			name:           "Invalid cdroom type",
+			config:         &CDRomConfig{CdromType: "invalid"},
+			fail:           true,
+			expectedErrMsg: "'cdrom_type' must be 'ide' or 'sata'",
+		},
+	}
+
+	for _, c := range tc {
+		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 TestStepAddCDRom_Run(t *testing.T) {
+	tc := []struct {
+		name           string
+		state          *multistep.BasicStateBag
+		step           *StepAddCDRom
+		vmMock         *driver.VirtualMachineMock
+		expectedAction multistep.StepAction
+		expectedVmMock *driver.VirtualMachineMock
+		fail           bool
+		errMessage     string
+	}{
+		{
+			name:  "CDRom SATA type with all iso paths set",
+			state: cdAndIsoRemotePathStateBag(),
+			step: &StepAddCDRom{
+				Config: &CDRomConfig{
+					CdromType: "sata",
+					ISOPaths:  []string{"iso/path"},
+				},
+			},
+			vmMock:         new(driver.VirtualMachineMock),
+			expectedAction: multistep.ActionContinue,
+			expectedVmMock: &driver.VirtualMachineMock{
+				FindSATAControllerCalled: true,
+				AddCdromCalled:           true,
+				AddCdromCalledTimes:      3,
+				AddCdromTypes:            []string{"sata", "sata", "sata"},
+				AddCdromPaths:            []string{"remote/path", "iso/path", "cd/path"},
+			},
+			fail:       false,
+			errMessage: "",
+		},
+		{
+			name:  "Add SATA Controller",
+			state: basicStateBag(),
+			step: &StepAddCDRom{
+				Config: &CDRomConfig{
+					CdromType: "sata",
+				},
+			},
+			vmMock: &driver.VirtualMachineMock{
+				FindSATAControllerErr: driver.ErrNoSataController,
+			},
+			expectedAction: multistep.ActionContinue,
+			expectedVmMock: &driver.VirtualMachineMock{
+				FindSATAControllerCalled: true,
+				FindSATAControllerErr:    driver.ErrNoSataController,
+				AddSATAControllerCalled:  true,
+			},
+			fail:       false,
+			errMessage: "",
+		},
+		{
+			name:  "Fail to add SATA Controller",
+			state: basicStateBag(),
+			step: &StepAddCDRom{
+				Config: &CDRomConfig{
+					CdromType: "sata",
+				},
+			},
+			vmMock: &driver.VirtualMachineMock{
+				FindSATAControllerErr: driver.ErrNoSataController,
+				AddSATAControllerErr:  fmt.Errorf("AddSATAController error"),
+			},
+			expectedAction: multistep.ActionHalt,
+			expectedVmMock: &driver.VirtualMachineMock{
+				FindSATAControllerCalled: true,
+				AddSATAControllerCalled:  true,
+			},
+			fail:       true,
+			errMessage: fmt.Sprintf("error adding SATA controller: %v", fmt.Errorf("AddSATAController error")),
+		},
+		{
+			name:  "IDE CDRom Type and Iso Path set",
+			state: basicStateBag(),
+			step: &StepAddCDRom{
+				Config: &CDRomConfig{
+					CdromType: "ide",
+					ISOPaths:  []string{"iso/path"},
+				},
+			},
+			vmMock:         new(driver.VirtualMachineMock),
+			expectedAction: multistep.ActionContinue,
+			expectedVmMock: &driver.VirtualMachineMock{
+				AddCdromCalled:      true,
+				AddCdromCalledTimes: 1,
+				AddCdromTypes:       []string{"ide"},
+				AddCdromPaths:       []string{"iso/path"},
+			},
+			fail:       false,
+			errMessage: "",
+		},
+		{
+			name:  "Fail to add cdrom from ISOPaths",
+			state: basicStateBag(),
+			step: &StepAddCDRom{
+				Config: &CDRomConfig{
+					ISOPaths: []string{"iso/path"},
+				},
+			},
+			vmMock: &driver.VirtualMachineMock{
+				AddCdromErr: fmt.Errorf("AddCdrom error"),
+			},
+			expectedAction: multistep.ActionHalt,
+			expectedVmMock: &driver.VirtualMachineMock{
+				AddCdromCalled:      true,
+				AddCdromCalledTimes: 1,
+				AddCdromTypes:       []string{""},
+				AddCdromPaths:       []string{"iso/path"},
+			},
+			fail:       true,
+			errMessage: fmt.Sprintf("error mounting an image 'iso/path': %v", fmt.Errorf("AddCdrom error")),
+		},
+		{
+			name:  "Fail to add cdrom from state iso_remote_path",
+			state: isoRemotePathStateBag(),
+			step: &StepAddCDRom{
+				Config: new(CDRomConfig),
+			},
+			vmMock: &driver.VirtualMachineMock{
+				AddCdromErr: fmt.Errorf("AddCdrom error"),
+			},
+			expectedAction: multistep.ActionHalt,
+			expectedVmMock: &driver.VirtualMachineMock{
+				AddCdromCalled:      true,
+				AddCdromCalledTimes: 1,
+				AddCdromTypes:       []string{""},
+				AddCdromPaths:       []string{"remote/path"},
+			},
+			fail:       true,
+			errMessage: fmt.Sprintf("error mounting an image 'remote/path': %v", fmt.Errorf("AddCdrom error")),
+		},
+	}
+
+	for _, c := range tc {
+		t.Run(c.name, func(t *testing.T) {
+			c.state.Put("vm", c.vmMock)
+			if action := c.step.Run(context.TODO(), c.state); action != c.expectedAction {
+				t.Fatalf("unexpected action %v", action)
+			}
+			err, ok := c.state.Get("error").(error)
+			if ok {
+				if err.Error() != c.errMessage {
+					t.Fatalf("unexpected error %s", err.Error())
+				}
+			} else {
+				if c.fail {
+					t.Fatalf("expected to fail but it didn't")
+				}
+			}
+
+			if diff := cmp.Diff(c.vmMock, c.expectedVmMock,
+				cmpopts.IgnoreInterfaces(struct{ error }{})); diff != "" {
+				t.Fatalf("unexpected VirtualMachine calls: %s", diff)
+			}
+		})
+	}
+}
+
+func cdAndIsoRemotePathStateBag() *multistep.BasicStateBag {
+	state := basicStateBag()
+	state.Put("iso_remote_path", "remote/path")
+	state.Put("cd_path", "cd/path")
+	return state
+}
+
+func isoRemotePathStateBag() *multistep.BasicStateBag {
+	state := basicStateBag()
+	state.Put("iso_remote_path", "remote/path")
+	return state
+}