From e787efef5061cf56342821717bc04dc09bcbc113 Mon Sep 17 00:00:00 2001 From: Bryan Venteicher Date: Wed, 31 Dec 2025 12:37:49 -0600 Subject: [PATCH] Don't always backfill VM Status.Zone from label When either is unset, or they have different values, lookup the VM's zone and assign the zone to the label and status. To avoid a lookup if both the label and status are set to the same value, we'll just trust what is there. --- .../vsphere/vmlifecycle/update_status.go | 13 ++- .../vsphere/vmlifecycle/update_status_test.go | 84 +++++++++++++++++++ 2 files changed, 90 insertions(+), 7 deletions(-) diff --git a/pkg/providers/vsphere/vmlifecycle/update_status.go b/pkg/providers/vsphere/vmlifecycle/update_status.go index 5cc2e1f7b..77aa181e5 100644 --- a/pkg/providers/vsphere/vmlifecycle/update_status.go +++ b/pkg/providers/vsphere/vmlifecycle/update_status.go @@ -361,14 +361,16 @@ func reconcileStatusZone( var errs []error - zoneName := vmCtx.VM.Labels[corev1.LabelTopologyZone] - if zoneName == "" { + zoneLabel := vmCtx.VM.Labels[corev1.LabelTopologyZone] + zoneStatus := vmCtx.VM.Status.Zone + + if zoneLabel == "" || zoneLabel != zoneStatus { clusterMoRef, err := vcenter.GetResourcePoolOwnerMoRef( vmCtx, vcVM.Client(), vmCtx.MoVM.ResourcePool.Value) if err != nil { errs = append(errs, err) } else { - zoneName, err = topology.LookupZoneForClusterMoID( + zoneName, err := topology.LookupZoneForClusterMoID( vmCtx, k8sClient, clusterMoRef.Value) if err != nil { errs = append(errs, err) @@ -377,14 +379,11 @@ func reconcileStatusZone( vmCtx.VM.Labels = map[string]string{} } vmCtx.VM.Labels[corev1.LabelTopologyZone] = zoneName + vmCtx.VM.Status.Zone = zoneName } } } - if zoneName != "" { - vmCtx.VM.Status.Zone = zoneName - } - return errs } diff --git a/pkg/providers/vsphere/vmlifecycle/update_status_test.go b/pkg/providers/vsphere/vmlifecycle/update_status_test.go index 5f21e03f5..aded41cfa 100644 --- a/pkg/providers/vsphere/vmlifecycle/update_status_test.go +++ b/pkg/providers/vsphere/vmlifecycle/update_status_test.go @@ -54,8 +54,11 @@ var _ = Describe("UpdateStatus", func() { BeforeEach(func() { ctx = suite.NewTestContextForVCSim(builder.VCSimTestConfig{}) + nsInfo := ctx.CreateWorkloadNamespace() + vm := builder.DummyVirtualMachine() vm.Name = "update-status-test" + vm.Namespace = nsInfo.Namespace vmCtx = pkgctx.VirtualMachineContext{ Context: ctx, @@ -67,6 +70,14 @@ var _ = Describe("UpdateStatus", func() { vcVM, err = ctx.Finder.VirtualMachine(ctx, "DC0_C0_RP0_VM0") Expect(err).ToNot(HaveOccurred()) + nsRP := ctx.GetResourcePoolForNamespace(nsInfo.Namespace, "", "") + task, err := vcVM.Relocate(ctx, vimtypes.VirtualMachineRelocateSpec{ + Folder: ptr.To(nsInfo.Folder.Reference()), + Pool: ptr.To(nsRP.Reference()), + }, vimtypes.VirtualMachineMovePriorityDefaultPriority) + Expect(err).ToNot(HaveOccurred()) + Expect(task.Wait(ctx)).To(Succeed()) + // Initialize with the expected properties. Tests can overwrite this if needed. Expect(vcVM.Properties( ctx, @@ -2470,6 +2481,79 @@ var _ = Describe("UpdateStatus", func() { }) }) + Context("Zone", func() { + var zoneName string + + BeforeEach(func() { + delete(vmCtx.VM.Labels, corev1.LabelTopologyZone) + vmCtx.VM.Status.Zone = "" + + zoneName = ctx.GetFirstZoneName() + }) + + assertZones := func() { + GinkgoHelper() + Expect(vmCtx.VM.Labels).To(HaveKeyWithValue(corev1.LabelTopologyZone, zoneName)) + Expect(vmCtx.VM.Status.Zone).To(Equal(zoneName)) + } + + Context("Neither label and status are set", func() { + It("Sets Zone", assertZones) + }) + + Context("Label and status are both set", func() { + BeforeEach(func() { + vmCtx.VM.Labels[corev1.LabelTopologyZone] = zoneName + vmCtx.VM.Status.Zone = zoneName + }) + It("Zone still set", assertZones) + }) + + Context("Label is set but status is not", func() { + BeforeEach(func() { + vmCtx.VM.Labels[corev1.LabelTopologyZone] = zoneName + }) + It("Sets Zone", assertZones) + }) + + Context("Label is not set but status is", func() { + BeforeEach(func() { + vmCtx.VM.Status.Zone = zoneName + }) + It("Sets Zone", assertZones) + }) + + Context("Label is not set but status is", func() { + BeforeEach(func() { + vmCtx.VM.Status.Zone = zoneName + }) + It("Sets Zone", assertZones) + }) + + Context("Label is not set but status is", func() { + BeforeEach(func() { + vmCtx.VM.Status.Zone = zoneName + }) + It("Sets Zone", assertZones) + }) + + Context("Label is set to incorrect value", func() { + BeforeEach(func() { + vmCtx.VM.Labels[corev1.LabelTopologyZone] = "bogus" + vmCtx.VM.Status.Zone = zoneName + }) + It("Sets Zone", assertZones) + }) + + Context("Status is set to incorrect value", func() { + BeforeEach(func() { + vmCtx.VM.Labels[corev1.LabelTopologyZone] = zoneName + vmCtx.VM.Status.Zone = "bogus" + }) + It("Sets Zone", assertZones) + }) + }) + Context("Controllers", func() { When("moVM.Config is nil", func() { BeforeEach(func() {