Merge pull request vmware-tanzu#455 from akutz/feature/configspec-by-…

…value

✨ ConfigSpec by value

pkg/util/configspec.go

@@ -16,7 +16,7 @@ import (
 // MarshalConfigSpecToXML returns a byte slice of the provided ConfigSpec
 // marshalled to an XML string.
 func MarshalConfigSpecToXML(
-	configSpec *vimTypes.VirtualMachineConfigSpec) ([]byte, error) {
+	configSpec vimTypes.VirtualMachineConfigSpec) ([]byte, error) {
 
 	start := xml.StartElement{
 		Name: xml.Name{
@@ -53,17 +53,17 @@ func MarshalConfigSpecToXML(
 // UnmarshalConfigSpecFromXML returns a ConfigSpec object from a byte-slice of
 // the ConfigSpec marshaled as an XML string.
 func UnmarshalConfigSpecFromXML(
-	data []byte) (*vimTypes.VirtualMachineConfigSpec, error) {
+	data []byte) (vimTypes.VirtualMachineConfigSpec, error) {
 
-	configSpec := &vimTypes.VirtualMachineConfigSpec{}
+	var configSpec vimTypes.VirtualMachineConfigSpec
 
 	// Instantiate a new XML decoder in order to specify the lookup table used
 	// by GoVmomi to transform XML types to Golang types.
 	dec := xml.NewDecoder(bytes.NewReader(data))
 	dec.TypeFunc = vimTypes.TypeFunc()
 
 	if err := dec.Decode(&configSpec); err != nil {
-		return nil, err
+		return vimTypes.VirtualMachineConfigSpec{}, err
 	}
 
 	return configSpec, nil
@@ -72,19 +72,19 @@ func UnmarshalConfigSpecFromXML(
 // UnmarshalConfigSpecFromBase64XML returns a ConfigSpec object from a
 // byte-slice of the ConfigSpec marshaled as a base64-encoded, XML string.
 func UnmarshalConfigSpecFromBase64XML(
-	src []byte) (*vimTypes.VirtualMachineConfigSpec, error) {
+	src []byte) (vimTypes.VirtualMachineConfigSpec, error) {
 
 	data, err := Base64Decode(src)
 	if err != nil {
-		return nil, err
+		return vimTypes.VirtualMachineConfigSpec{}, err
 	}
 	return UnmarshalConfigSpecFromXML(data)
 }
 
 // MarshalConfigSpecToJSON returns a byte slice of the provided ConfigSpec
 // marshaled to a JSON string.
 func MarshalConfigSpecToJSON(
-	configSpec *vimTypes.VirtualMachineConfigSpec) ([]byte, error) {
+	configSpec vimTypes.VirtualMachineConfigSpec) ([]byte, error) {
 
 	var w bytes.Buffer
 	enc := vimTypes.NewJSONEncoder(&w)
@@ -97,15 +97,15 @@ func MarshalConfigSpecToJSON(
 // UnmarshalConfigSpecFromJSON returns a ConfigSpec object from a byte-slice of
 // the ConfigSpec marshaled as a JSON string.
 func UnmarshalConfigSpecFromJSON(
-	data []byte) (*vimTypes.VirtualMachineConfigSpec, error) {
+	data []byte) (vimTypes.VirtualMachineConfigSpec, error) {
 
 	var configSpec vimTypes.VirtualMachineConfigSpec
 
 	dec := vimTypes.NewJSONDecoder(bytes.NewReader(data))
 	if err := dec.Decode(&configSpec); err != nil {
-		return nil, err
+		return vimTypes.VirtualMachineConfigSpec{}, err
 	}
-	return &configSpec, nil
+	return configSpec, nil
 }
 
 // DevicesFromConfigSpec returns a slice of devices from the ConfigSpec's

pkg/util/configspec_test.go

@@ -108,17 +108,17 @@ var _ = Describe("DevicesFromConfigSpec", func() {
 var _ = Describe("ConfigSpec Util", func() {
 	Context("MarshalConfigSpecToXML", func() {
 		It("marshals and unmarshal to the same spec", func() {
-			inputSpec := &vimTypes.VirtualMachineConfigSpec{Name: "dummy-VM"}
+			inputSpec := vimTypes.VirtualMachineConfigSpec{Name: "dummy-VM"}
 			bytes, err := util.MarshalConfigSpecToXML(inputSpec)
 			Expect(err).ShouldNot(HaveOccurred())
 			var outputSpec vimTypes.VirtualMachineConfigSpec
 			err = xml.Unmarshal(bytes, &outputSpec)
 			Expect(err).ShouldNot(HaveOccurred())
-			Expect(reflect.DeepEqual(inputSpec, &outputSpec)).To(BeTrue())
+			Expect(reflect.DeepEqual(inputSpec, outputSpec)).To(BeTrue())
 		})
 
 		It("marshals spec correctly to expected base64 encoded XML", func() {
-			inputSpec := &vimTypes.VirtualMachineConfigSpec{Name: "dummy-VM"}
+			inputSpec := vimTypes.VirtualMachineConfigSpec{Name: "dummy-VM"}
 			bytes, err := util.MarshalConfigSpecToXML(inputSpec)
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(base64.StdEncoding.EncodeToString(bytes)).To(Equal("PG9iaiB4bWxuczp2aW0yNT0idXJuOnZpbTI1I" +

pkg/vmprovider/providers/vsphere2/placement/cluster_placement.go

@@ -111,11 +111,11 @@ func CloneVMRelocateSpec(
 func PlaceVMForCreate(
 	ctx goctx.Context,
 	cluster *object.ClusterComputeResource,
-	configSpec *types.VirtualMachineConfigSpec) ([]Recommendation, error) {
+	configSpec types.VirtualMachineConfigSpec) ([]Recommendation, error) {
 
 	placementSpec := types.PlacementSpec{
 		PlacementType: string(types.PlacementSpecPlacementTypeCreate),
-		ConfigSpec:    configSpec,
+		ConfigSpec:    &configSpec,
 	}
 
 	resp, err := cluster.PlaceVm(ctx, placementSpec)
@@ -147,18 +147,17 @@ func ClusterPlaceVMForCreate(
 	vmCtx context.VirtualMachineContextA2,
 	vcClient *vim25.Client,
 	resourcePoolsMoRefs []types.ManagedObjectReference,
-	configSpec *types.VirtualMachineConfigSpec,
+	configSpec types.VirtualMachineConfigSpec,
 	needsHost bool) ([]Recommendation, error) {
 
 	// Work around PlaceVmsXCluster bug that crashes vpxd when ConfigSpec.Files is nil.
-	cs := *configSpec
-	cs.Files = new(types.VirtualMachineFileInfo)
+	configSpec.Files = new(types.VirtualMachineFileInfo)
 
 	placementSpec := types.PlaceVmsXClusterSpec{
 		ResourcePools: resourcePoolsMoRefs,
 		VmPlacementSpecs: []types.PlaceVmsXClusterSpecVmPlacementSpec{
 			{
-				ConfigSpec: cs,
+				ConfigSpec: configSpec,
 			},
 		},
 		HostRecommRequired: &needsHost,

pkg/vmprovider/providers/vsphere2/placement/zone_placement.go

@@ -162,7 +162,7 @@ func getPlacementRecommendations(
 	vmCtx context.VirtualMachineContextA2,
 	vcClient *vim25.Client,
 	candidates map[string][]string,
-	configSpec *types.VirtualMachineConfigSpec) map[string][]Recommendation {
+	configSpec types.VirtualMachineConfigSpec) map[string][]Recommendation {
 
 	recommendations := map[string][]Recommendation{}
 
@@ -211,7 +211,7 @@ func getZonalPlacementRecommendations(
 	vmCtx context.VirtualMachineContextA2,
 	vcClient *vim25.Client,
 	candidates map[string][]string,
-	configSpec *types.VirtualMachineConfigSpec,
+	configSpec types.VirtualMachineConfigSpec,
 	needsHost bool) map[string][]Recommendation {
 
 	rpMOToZone := map[types.ManagedObjectReference]string{}
@@ -286,7 +286,7 @@ func Placement(
 	vmCtx context.VirtualMachineContextA2,
 	client ctrlclient.Client,
 	vcClient *vim25.Client,
-	configSpec *types.VirtualMachineConfigSpec,
+	configSpec types.VirtualMachineConfigSpec,
 	childRPName string) (*Result, error) {
 
 	existingRes, zonePlacement, instanceStoragePlacement := doesVMNeedPlacement(vmCtx)

pkg/vmprovider/providers/vsphere2/placement/zone_placement_test.go

@@ -72,7 +72,7 @@ func vcSimPlacement() {
 
 		vm         *vmopv1.VirtualMachine
 		vmCtx      context.VirtualMachineContextA2
-		configSpec *types.VirtualMachineConfigSpec
+		configSpec types.VirtualMachineConfigSpec
 	)
 
 	BeforeEach(func() {
@@ -82,7 +82,7 @@ func vcSimPlacement() {
 		vm.Name = "placement-test"
 
 		// Other than the name ConfigSpec contents don't matter for vcsim.
-		configSpec = &types.VirtualMachineConfigSpec{
+		configSpec = types.VirtualMachineConfigSpec{
 			Name: vm.Name,
 		}
 	})

pkg/vmprovider/providers/vsphere2/session/session_vm_update.go

@@ -40,7 +40,7 @@ type VMUpdateArgs struct {
 
 	BootstrapData vmlifecycle.BootstrapData
 
-	ConfigSpec *vimTypes.VirtualMachineConfigSpec
+	ConfigSpec vimTypes.VirtualMachineConfigSpec
 
 	NetworkResults network2.NetworkInterfaceResults
 }
@@ -519,10 +519,10 @@ func updateConfigSpec(
 	UpdateConfigSpecAnnotation(config, configSpec)
 	UpdateConfigSpecManagedBy(config, configSpec)
 	UpdateConfigSpecExtraConfig(
-		vmCtx, config, configSpec, updateArgs.ConfigSpec,
+		vmCtx, config, configSpec, &updateArgs.ConfigSpec,
 		&vmClassSpec, vmCtx.VM, updateArgs.ExtraConfig)
 	UpdateConfigSpecChangeBlockTracking(
-		vmCtx, config, configSpec, updateArgs.ConfigSpec, vmCtx.VM.Spec)
+		vmCtx, config, configSpec, &updateArgs.ConfigSpec, vmCtx.VM.Spec)
 	UpdateConfigSpecFirmware(config, configSpec, vmCtx.VM)
 
 	return configSpec
@@ -558,7 +558,7 @@ func (s *Session) prePowerOnVMConfigSpec(
 	configSpec.DeviceChange = append(configSpec.DeviceChange, ethCardDeviceChanges...)
 
 	var expectedPCIDevices []vimTypes.BaseVirtualDevice
-	if configSpecDevs := util.DevicesFromConfigSpec(updateArgs.ConfigSpec); len(configSpecDevs) > 0 {
+	if configSpecDevs := util.DevicesFromConfigSpec(&updateArgs.ConfigSpec); len(configSpecDevs) > 0 {
 		pciPassthruFromConfigSpec := util.SelectVirtualPCIPassthrough(configSpecDevs)
 		expectedPCIDevices = virtualmachine.CreatePCIDevicesFromConfigSpec(pciPassthruFromConfigSpec)
 	}
@@ -746,7 +746,7 @@ func (s *Session) prepareVMForPowerOn(
 	cfg *vimTypes.VirtualMachineConfigInfo,
 	updateArgs *VMUpdateArgs) error {
 
-	netIfList, err := s.ensureNetworkInterfaces(vmCtx, updateArgs.ConfigSpec)
+	netIfList, err := s.ensureNetworkInterfaces(vmCtx, &updateArgs.ConfigSpec)
 	if err != nil {
 		return err
 	}

pkg/vmprovider/providers/vsphere2/virtualmachine/configspec.go

@@ -22,19 +22,10 @@ import (
 // the config we build here.
 func CreateConfigSpec(
 	vmCtx context.VirtualMachineContextA2,
-	vmClassConfigSpec *types.VirtualMachineConfigSpec,
+	configSpec types.VirtualMachineConfigSpec,
 	vmClassSpec *vmopv1.VirtualMachineClassSpec,
 	vmImageStatus *vmopv1.VirtualMachineImageStatus,
-	minFreq uint64) *types.VirtualMachineConfigSpec {
-
-	configSpec := types.VirtualMachineConfigSpec{}
-
-	// If there is a class ConfigSpec, then that is our initial ConfigSpec. Note that our caller
-	// will synthesize a class ConfigSpec from the class's Hardware.Devices if the class doesn't
-	// have a ConfigSpec.
-	if vmClassConfigSpec != nil {
-		configSpec = *vmClassConfigSpec
-	}
+	minFreq uint64) types.VirtualMachineConfigSpec {
 
 	configSpec.Name = vmCtx.VM.Name
 	if configSpec.Annotation == "" {
@@ -111,7 +102,7 @@ func CreateConfigSpec(
 		}
 	}
 
-	return &configSpec
+	return configSpec
 }
 
 func determineHardwareVersion(
@@ -156,15 +147,16 @@ func determineHardwareVersion(
 	return max(vmMinVersion, minVerFromDevs)
 }
 
-// CreateConfigSpecForPlacement creates a ConfigSpec that is suitable for Placement.
-// baseConfigSpec will likely be - or at least derived from - the ConfigSpec returned by CreateConfigSpec above.
+// CreateConfigSpecForPlacement creates a ConfigSpec that is suitable for
+// Placement. configSpec will likely be - or at least derived from - the
+// ConfigSpec returned by CreateConfigSpec above.
 func CreateConfigSpecForPlacement(
 	vmCtx context.VirtualMachineContextA2,
-	baseConfigSpec *types.VirtualMachineConfigSpec,
-	storageClassesToIDs map[string]string) *types.VirtualMachineConfigSpec {
+	configSpec types.VirtualMachineConfigSpec,
+	storageClassesToIDs map[string]string) types.VirtualMachineConfigSpec {
 
-	deviceChangeCopy := make([]types.BaseVirtualDeviceConfigSpec, 0, len(baseConfigSpec.DeviceChange))
-	for _, devChange := range baseConfigSpec.DeviceChange {
+	deviceChangeCopy := make([]types.BaseVirtualDeviceConfigSpec, 0, len(configSpec.DeviceChange))
+	for _, devChange := range configSpec.DeviceChange {
 		if spec := devChange.GetVirtualDeviceConfigSpec(); spec != nil {
 			// VC PlaceVmsXCluster() has issues when the ConfigSpec has EthCards so return to the
 			// prior status quo until those issues get sorted out.
@@ -175,7 +167,6 @@ func CreateConfigSpecForPlacement(
 		deviceChangeCopy = append(deviceChangeCopy, devChange)
 	}
 
-	configSpec := *baseConfigSpec
 	configSpec.DeviceChange = deviceChangeCopy
 
 	// Add a dummy disk for placement: PlaceVmsXCluster expects there to always be at least one disk.
@@ -228,19 +219,19 @@ func CreateConfigSpecForPlacement(
 	//  - any way to do the cluster modules for anti-affinity?
 	//  - whatever else I'm forgetting
 
-	return &configSpec
+	return configSpec
 }
 
 // ConfigSpecFromVMClassDevices creates a ConfigSpec that adds the standalone hardware devices from
 // the VMClass if any. This ConfigSpec will be used as the class ConfigSpec to CreateConfigSpec, with
 // the rest of the class fields - like CPU count - applied on top.
-func ConfigSpecFromVMClassDevices(vmClassSpec *vmopv1.VirtualMachineClassSpec) *types.VirtualMachineConfigSpec {
+func ConfigSpecFromVMClassDevices(vmClassSpec *vmopv1.VirtualMachineClassSpec) types.VirtualMachineConfigSpec {
 	devsFromClass := CreatePCIDevicesFromVMClass(vmClassSpec.Hardware.Devices)
 	if len(devsFromClass) == 0 {
-		return nil
+		return types.VirtualMachineConfigSpec{}
 	}
 
-	configSpec := &types.VirtualMachineConfigSpec{}
+	var configSpec types.VirtualMachineConfigSpec
 	for _, dev := range devsFromClass {
 		configSpec.DeviceChange = append(configSpec.DeviceChange, &types.VirtualDeviceConfigSpec{
 			Operation: types.VirtualDeviceConfigSpecOperationAdd,

pkg/vmprovider/providers/vsphere2/virtualmachine/configspec_test.go

@@ -30,8 +30,8 @@ var _ = Describe("CreateConfigSpec", func() {
 		vmClassSpec     *vmopv1.VirtualMachineClassSpec
 		vmImageStatus   *vmopv1.VirtualMachineImageStatus
 		minCPUFreq      uint64
-		configSpec      *vimTypes.VirtualMachineConfigSpec
-		classConfigSpec *vimTypes.VirtualMachineConfigSpec
+		configSpec      vimTypes.VirtualMachineConfigSpec
+		classConfigSpec vimTypes.VirtualMachineConfigSpec
 		pvcVolume       = vmopv1.VirtualMachineVolume{
 			Name: "vmware",
 			VirtualMachineVolumeSource: vmopv1.VirtualMachineVolumeSource{
@@ -75,7 +75,7 @@ var _ = Describe("CreateConfigSpec", func() {
 
 	Context("No VM Class ConfigSpec", func() {
 		BeforeEach(func() {
-			classConfigSpec = nil
+			classConfigSpec = vimTypes.VirtualMachineConfigSpec{}
 		})
 
 		When("Basic ConfigSpec assertions", func() {
@@ -118,7 +118,7 @@ var _ = Describe("CreateConfigSpec", func() {
 
 	Context("VM Class ConfigSpec", func() {
 		BeforeEach(func() {
-			classConfigSpec = &vimTypes.VirtualMachineConfigSpec{
+			classConfigSpec = vimTypes.VirtualMachineConfigSpec{
 				Name:       "dont-use-this-dummy-VM",
 				Annotation: "test-annotation",
 				DeviceChange: []vimTypes.BaseVirtualDeviceConfigSpec{
@@ -319,12 +319,12 @@ var _ = Describe("CreateConfigSpecForPlacement", func() {
 	var (
 		vmCtx               context.VirtualMachineContextA2
 		storageClassesToIDs map[string]string
-		baseConfigSpec      *vimTypes.VirtualMachineConfigSpec
-		configSpec          *vimTypes.VirtualMachineConfigSpec
+		baseConfigSpec      vimTypes.VirtualMachineConfigSpec
+		configSpec          vimTypes.VirtualMachineConfigSpec
 	)
 
 	BeforeEach(func() {
-		baseConfigSpec = &vimTypes.VirtualMachineConfigSpec{}
+		baseConfigSpec = vimTypes.VirtualMachineConfigSpec{}
 		storageClassesToIDs = map[string]string{}
 
 		vm := builder.DummyVirtualMachineA2()
@@ -345,7 +345,7 @@ var _ = Describe("CreateConfigSpecForPlacement", func() {
 
 	Context("Returns expected ConfigSpec", func() {
 		BeforeEach(func() {
-			baseConfigSpec = &vimTypes.VirtualMachineConfigSpec{
+			baseConfigSpec = vimTypes.VirtualMachineConfigSpec{
 				Name:       "dummy-VM",
 				Annotation: "test-annotation",
 				NumCPUs:    42,
@@ -422,7 +422,7 @@ var _ = Describe("CreateConfigSpecForPlacement", func() {
 
 	Context("Removes VirtualEthernetCards without a backing", func() {
 		BeforeEach(func() {
-			baseConfigSpec = &vimTypes.VirtualMachineConfigSpec{
+			baseConfigSpec = vimTypes.VirtualMachineConfigSpec{
 				Name: "dummy-VM",
 				DeviceChange: []vimTypes.BaseVirtualDeviceConfigSpec{
 					&vimTypes.VirtualDeviceConfigSpec{
@@ -447,7 +447,7 @@ var _ = Describe("ConfigSpecFromVMClassDevices", func() {
 
 	var (
 		vmClassSpec *vmopv1.VirtualMachineClassSpec
-		configSpec  *vimTypes.VirtualMachineConfigSpec
+		configSpec  vimTypes.VirtualMachineConfigSpec
 	)
 
 	Context("when Class specifies GPU/DDPIO in Hardware", func() {

pkg/vmprovider/providers/vsphere2/vmlifecycle/create.go

@@ -17,7 +17,7 @@ type CreateArgs struct {
 	UseContentLibrary bool
 	ProviderItemID    string
 
-	ConfigSpec          *types.VirtualMachineConfigSpec
+	ConfigSpec          types.VirtualMachineConfigSpec
 	StorageProvisioning string
 	FolderMoID          string
 	ResourcePoolMoID    string

pkg/vmprovider/providers/vsphere2/vmlifecycle/create_clone.go

@@ -59,7 +59,7 @@ func createCloneSpec(
 	srcVM *object.VirtualMachine) (*vimtypes.VirtualMachineCloneSpec, error) {
 
 	cloneSpec := &vimtypes.VirtualMachineCloneSpec{
-		Config: createArgs.ConfigSpec,
+		Config: &createArgs.ConfigSpec,
 		Memory: ptr.To(false), // No full memory clones.
 	}