Addresses review comments

Signed-off-by: Sagar Muchhal <[email protected]>
vmware-tanzu · Sep 18, 2023 · fa1d625 · fa1d625
1 parent 076c972
commit fa1d625
Show file tree

Hide file tree

Showing 5 changed files with 5 additions and 48 deletions.
diff --git a/pkg/vmprovider/providers/vsphere/session/session_vm_status.go b/pkg/vmprovider/providers/vsphere/session/session_vm_status.go
@@ -109,6 +109,7 @@ func (s *Session) updateVMStatus(
 	vm.Status.UniqueID = resVM.MoRef().Value
 	vm.Status.BiosUUID = summary.Config.Uuid
 	vm.Status.InstanceUUID = summary.Config.InstanceUuid
+	vm.Status.HardwareVersion = util.ParseVirtualHardwareVersion(summary.Config.HwVersion)
 
 	if host := summary.Runtime.Host; host != nil {
 		hostSystem := object.NewHostSystem(s.Client.VimClient(), *host)
@@ -166,7 +167,5 @@ func (s *Session) updateVMStatus(
 		}
 	}
 
-	vm.Status.HardwareVersion = util.ParseVirtualHardwareVersion(summary.Config.HwVersion)
-
 	return k8serrors.NewAggregate(errs)
 }
diff --git a/pkg/vmprovider/providers/vsphere/session/session_vm_update.go b/pkg/vmprovider/providers/vsphere/session/session_vm_update.go
@@ -457,19 +457,6 @@ func UpdateConfigSpecDeviceGroups(
 	}
 }
 
-// UpdateConfigSpecHardwareVersion sets the hardware version in the ConfigSpec
-// if the current hardware version is lower than the specified minimum version.
-func UpdateConfigSpecHardwareVersion(
-	config *vimTypes.VirtualMachineConfigInfo,
-	configSpec *vimTypes.VirtualMachineConfigSpec,
-	minHardwareVersion int32) {
-
-	existingVersion := util.ParseVirtualHardwareVersion(config.Version)
-	if existingVersion < minHardwareVersion {
-		util.EnsureMinHardwareVersionInConfigSpec(configSpec, minHardwareVersion)
-	}
-}
-
 // updateConfigSpec overlays the VM Class spec with the provided ConfigSpec to form a desired
 // ConfigSpec that will be used to reconfigure the VM.
 func updateConfigSpec(
@@ -491,7 +478,7 @@ func updateConfigSpec(
 		UpdateConfigSpecMemoryAllocation(config, configSpec, &vmClassSpec)
 		// Only in this case, the VM's hardware version is not set to the
 		// minimum version from the VM.Spec, if any.
-		UpdateConfigSpecHardwareVersion(config, configSpec, vmCtx.VM.Spec.MinHardwareVersion)
+		util.EnsureMinHardwareVersionInConfigSpec(configSpec, vmCtx.VM.Spec.MinHardwareVersion)
 	}
 
 	UpdateConfigSpecAnnotation(config, configSpec)

diff --git a/pkg/vmprovider/providers/vsphere/session/session_vm_update_test.go b/pkg/vmprovider/providers/vsphere/session/session_vm_update_test.go
@@ -386,37 +386,6 @@ var _ = Describe("Update ConfigSpec", func() {
 		})
 	})
 
-	Context("Hardware Version", func() {
-		var minHardwareVersion int32
-
-		JustBeforeEach(func() {
-			session.UpdateConfigSpecHardwareVersion(config, configSpec, minHardwareVersion)
-		})
-
-		Context("Existing hardware version is lower than min version", func() {
-			BeforeEach(func() {
-				config.Version = "vmx-12"
-				minHardwareVersion = 15
-			})
-
-			It("updates the hardware version", func() {
-				Expect(configSpec.Version).To(Equal("vmx-15"))
-			})
-		})
-
-		Context("Existing hardware version is greater than min version", func() {
-			BeforeEach(func() {
-				config.Version = "vmx-17"
-				minHardwareVersion = 16
-			})
-
-			It("does not update the hardware version", func() {
-				Expect(configSpec.Version).NotTo(Equal("vmx-16"))
-			})
-		})
-
-	})
-
 	Context("ExtraConfig", func() {
 		var vmClassSpec *vmopv1.VirtualMachineClassSpec
 		var classConfigSpec *vimTypes.VirtualMachineConfigSpec

diff --git a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go
@@ -692,6 +692,7 @@ func (v validator) validateImmutableFields(ctx *context.WebhookRequestContext, v
 	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ClassName, oldVM.Spec.ClassName, specPath.Child("className"))...)
 	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.StorageClass, oldVM.Spec.StorageClass, specPath.Child("storageClass"))...)
 	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ResourcePolicyName, oldVM.Spec.ResourcePolicyName, specPath.Child("resourcePolicyName"))...)
+	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.MinHardwareVersion, oldVM.Spec.MinHardwareVersion, specPath.Child("minHardwareVersion"))...)
 
 	return allErrs
 }

diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go
@@ -622,13 +622,14 @@ func (v validator) validateUpdatesWhenPoweredOn(ctx *context.WebhookRequestConte
 	return allErrs
 }
 
-func (v validator) validateImmutableFields(ctx *context.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList {
+func (v validator) validateImmutableFields(_ *context.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList {
 	var allErrs field.ErrorList
 	specPath := field.NewPath("spec")
 
 	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ImageName, oldVM.Spec.ImageName, specPath.Child("imageName"))...)
 	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ClassName, oldVM.Spec.ClassName, specPath.Child("className"))...)
 	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.StorageClass, oldVM.Spec.StorageClass, specPath.Child("storageClass"))...)
+	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.MinHardwareVersion, oldVM.Spec.MinHardwareVersion, specPath.Child("minHardwareVersion"))...)
 	// TODO: More checks.
 
 	// TODO: Allow privilege?