Merge pull request vmware-tanzu#916 from bryanv/bryanv/classless-vm-resize

🐛 Handle resize for VMs that were classless

Author: bryanv

pkg/providers/vsphere/vmprovider_vm_resize_test.go

@@ -212,6 +212,31 @@ func vmResizeTests() {
 					})
 				})
 
+				It("Resizes VM that was classless", func() {
+					newCS := configSpec
+					newCS.NumCPUs = 42
+					newCS.MemoryMB = 8192
+					newVMClass := createVMClass(newCS)
+					vm.Spec.ClassName = newVMClass.Name
+
+					// Simulate what the VM mutation webhook would do by setting the LRA to an empty class name.
+					delete(vm.Annotations, vmopv1util.LastResizedAnnotationKey)
+					Expect(vmopv1util.SetLastResizedAnnotationClassName(vm, "")).To(Succeed())
+					Expect(vm.Annotations).To(HaveKey(vmopv1util.LastResizedAnnotationKey))
+
+					vcVM, err := createOrUpdateAndGetVcVM(ctx, vmProvider, vm)
+					Expect(err).ToNot(HaveOccurred())
+
+					var o mo.VirtualMachine
+					Expect(vcVM.Properties(ctx, vcVM.Reference(), nil, &o)).To(Succeed())
+					Expect(o.Summary.Runtime.PowerState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOff))
+					Expect(o.Config.Hardware.NumCPU).To(BeEquivalentTo(newCS.NumCPUs))
+					Expect(o.Config.Hardware.MemoryMB).To(BeEquivalentTo(newCS.MemoryMB))
+					assertExpectedNoReservationFields(o)
+
+					assertExpectedResizedClassFields(vm, newVMClass)
+				})
+
 				Context("CPU/Memory Reservations", func() {
 
 					Context("No reservations", func() {
@@ -402,6 +427,32 @@ func vmResizeTests() {
 
 						assertExpectedResizedClassFields(vm, newVMClass)
 					})
+
+					It("Resizes VM that was classless", func() {
+						newCS := configSpec
+						newCS.NumCPUs = 42
+						newCS.MemoryMB = 8192
+						newVMClass := createVMClass(newCS)
+						vm.Spec.ClassName = newVMClass.Name
+
+						// Simulate what the VM mutation webhook would do by setting the LRA to an emtpy class name.
+						delete(vm.Annotations, vmopv1util.LastResizedAnnotationKey)
+						Expect(vmopv1util.SetLastResizedAnnotationClassName(vm, "")).To(Succeed())
+						Expect(vm.Annotations).To(HaveKey(vmopv1util.LastResizedAnnotationKey))
+
+						vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOn
+						vcVM, err := createOrUpdateAndGetVcVM(ctx, vmProvider, vm)
+						Expect(err).ToNot(HaveOccurred())
+
+						var o mo.VirtualMachine
+						Expect(vcVM.Properties(ctx, vcVM.Reference(), nil, &o)).To(Succeed())
+						Expect(o.Summary.Runtime.PowerState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOn))
+						Expect(o.Config.Hardware.NumCPU).To(BeEquivalentTo(newCS.NumCPUs))
+						Expect(o.Config.Hardware.MemoryMB).To(BeEquivalentTo(newCS.MemoryMB))
+						assertExpectedNoReservationFields(o)
+
+						assertExpectedResizedClassFields(vm, newVMClass)
+					})
 				})
 
 				Context("Without Same Class Resize Annotation", func() {

pkg/util/vmopv1/resize.go

@@ -58,6 +58,9 @@ func MustSetLastResizedAnnotation(
 }
 
 // SetLastResizedAnnotationClassName sets the resize annotation to just of the class name.
+// This is called from the VM mutation wehbook to record the prior class name of a VM
+// that does not already have the annotation (so that ResizeNeeded() will return true).
+// Note className may be empty if the VM was classless.
 func SetLastResizedAnnotationClassName(
 	vm *vmopv1.VirtualMachine,
 	className string) error {
@@ -107,7 +110,7 @@ func ResizeNeeded(
 	vmClass vmopv1.VirtualMachineClass) bool {
 
 	lra, exists := getLastResizeAnnotation(vm)
-	if !exists || lra.Name == "" {
+	if !exists {
 		// The VM does not have the last resized annotation. Most likely this is an existing VM
 		// and the VM Spec.ClassName hasn't been changed (because the mutation webhook sets the
 		// annotation when it changes). However, if the same class opt-in annotation is set, do

pkg/util/vmopv1/resize_test.go

@@ -54,6 +54,24 @@ var _ = Describe("SetLastResizedAnnotationClassName", func() {
 		vmClass.Generation = 42
 	})
 
+	When("Classless", func() {
+		It("Sets expected annotation", func() {
+			err := vmopv1util.SetLastResizedAnnotationClassName(vm, "")
+			Expect(err).ToNot(HaveOccurred())
+			val, ok := vm.Annotations[vmopv1util.LastResizedAnnotationKey]
+			Expect(ok).To(BeTrue())
+			Expect(val).To(MatchJSON(`{"name":""}`))
+
+			By("GetLastResizedAnnotation also returns expected values", func() {
+				className, uid, generation, ok := vmopv1util.GetLastResizedAnnotation(*vm)
+				Expect(ok).To(BeTrue())
+				Expect(className).To(BeEmpty())
+				Expect(uid).To(BeEmpty())
+				Expect(generation).To(BeZero())
+			})
+		})
+	})
+
 	It("Sets expected annotation", func() {
 		err := vmopv1util.SetLastResizedAnnotationClassName(vm, vmClass.Name)
 		Expect(err).ToNot(HaveOccurred())
@@ -96,6 +114,20 @@ var _ = Describe("GetLastResizedAnnotation", func() {
 		})
 	})
 
+	When("Resize annotation is present from classless VM", func() {
+		BeforeEach(func() {
+			Expect(vmopv1util.SetLastResizedAnnotationClassName(vm, ""))
+		})
+
+		It("Returns expected values", func() {
+			className, uid, generation, ok := vmopv1util.GetLastResizedAnnotation(*vm)
+			Expect(ok).To(BeTrue())
+			Expect(className).To(BeEmpty())
+			Expect(uid).To(BeEmpty())
+			Expect(generation).To(BeZero())
+		})
+	})
+
 	When("Resize annotation is present", func() {
 		BeforeEach(func() {
 			Expect(vmopv1util.SetLastResizedAnnotation(vm, vmClass)).To(Succeed())
@@ -144,6 +176,30 @@ var _ = Describe("ResizeNeeded", func() {
 		})
 	})
 
+	Context("Resize annotation with just ClassName is set", func() {
+		Context("Resize annotation is present via non-classless VM", func() {
+			BeforeEach(func() {
+				Expect(vmopv1util.SetLastResizedAnnotationClassName(&vm, "my-old-class")).To(Succeed())
+				vm.Spec.ClassName = "my-new-class"
+			})
+
+			It("returns true", func() {
+				Expect(vmopv1util.ResizeNeeded(vm, vmClass)).To(BeTrue())
+			})
+		})
+
+		Context("Resize annotation is present via classless VM", func() {
+			BeforeEach(func() {
+				Expect(vmopv1util.SetLastResizedAnnotationClassName(&vm, "")).To(Succeed())
+				vm.Spec.ClassName = "my-new-class"
+			})
+
+			It("returns true", func() {
+				Expect(vmopv1util.ResizeNeeded(vm, vmClass)).To(BeTrue())
+			})
+		})
+	})
+
 	Context("Resize annotation is present", func() {
 		BeforeEach(func() {
 			Expect(vmopv1util.SetLastResizedAnnotation(&vm, vmClass)).To(Succeed())

webhooks/virtualmachine/mutation/virtualmachine_mutator_unit_test.go

@@ -1148,6 +1148,23 @@ func unitTestsMutating() {
 					})
 				})
 
+				When("existing vm is classless", func() {
+					It("set last-resize annotation", func() {
+						oldVM.Spec.ClassName = ""
+						ctx.vm.Spec.ClassName = newClassName
+
+						updated, err := mutation.SetLastResizeAnnotation(&ctx.WebhookRequestContext, ctx.vm, oldVM)
+						Expect(err).ToNot(HaveOccurred())
+						Expect(updated).To(BeTrue())
+
+						className, uid, gen, exists := vmopv1util.GetLastResizedAnnotation(*ctx.vm)
+						Expect(exists).To(BeTrue())
+						Expect(className).To(Equal(oldVM.Spec.ClassName))
+						Expect(uid).To(BeEmpty())
+						Expect(gen).To(BeZero())
+					})
+				})
+
 				When("vm already has last-resize annotation", func() {
 					It("annotation is not changed", func() {
 						vmClass := builder.DummyVirtualMachineClass("my-class")