Fix a case when deleting element when looping (vmware-tanzu#1385)

When deleting stale entries in volume status, store the indexes first, 
then deletes them in a way without shifting indexes before deleting next one.

Author: lubronzhan

pkg/providers/vsphere/vmlifecycle/update_status.go

@@ -1317,17 +1317,29 @@ func updateVolumeStatus(vmCtx pkgctx.VirtualMachineContext) {
 			existingDisksInStatus[vol.DiskUUID] = i
 		}
 	}
+	// Collect indices to delete first.
+	indicesToDelete := []int{}
 	for _, di := range info.Disks {
 		if volSpec, ok := info.Volumes[di.Target.String()]; ok {
 			if diskIndex, ok := existingDisksInStatus[di.UUID]; ok {
 				if volSpec.Name != vm.Status.Volumes[diskIndex].Name {
-					vmCtx.VM.Status.Volumes = slices.Delete(
-						vmCtx.VM.Status.Volumes, diskIndex, diskIndex+1)
+					indicesToDelete = append(indicesToDelete, diskIndex)
 					delete(existingDisksInStatus, di.UUID)
 				}
 			}
 		}
 	}
+	// Delete in reverse order so that we don't shift indices before deleting
+	// next one.
+	slices.Sort(indicesToDelete)
+	for i := len(indicesToDelete) - 1; i >= 0; i-- {
+		vm.Status.Volumes = slices.Delete(
+			vm.Status.Volumes,
+			indicesToDelete[i],
+			indicesToDelete[i]+1,
+		)
+	}
+	// Update existingDisksInStatus with new indexes.
 	for i := range vm.Status.Volumes {
 		if vol := vm.Status.Volumes[i]; vol.DiskUUID != "" {
 			existingDisksInStatus[vol.DiskUUID] = i

pkg/providers/vsphere/vmlifecycle/update_status_test.go

@@ -1742,7 +1742,7 @@ var _ = Describe("UpdateStatus", func() {
 				})
 			})
 
-			When("vm.status.volumes has a stale (not in spec) classic disk", func() {
+			When("vm.status.volumes has two stale (not in spec) classic disks", func() {
 				BeforeEach(func() {
 					vmCtx.VM.Spec.Volumes = []vmopv1.VirtualMachineVolume{
 						{
@@ -1754,17 +1754,38 @@ var _ = Describe("UpdateStatus", func() {
 							ControllerBusNumber: ptr.To[int32](0),
 							UnitNumber:          ptr.To[int32](3),
 						},
+						{
+							Name: pkgutil.GeneratePVCName("disk", "104"),
+							VirtualMachineVolumeSource: vmopv1.VirtualMachineVolumeSource{
+								PersistentVolumeClaim: &vmopv1.PersistentVolumeClaimVolumeSource{},
+							},
+							ControllerType:      vmopv1.VirtualControllerTypeSCSI,
+							ControllerBusNumber: ptr.To[int32](0),
+							UnitNumber:          ptr.To[int32](4),
+						},
 					}
 					vmCtx.VM.Status.Volumes = []vmopv1.VirtualMachineVolumeStatus{
 						{
-							Name:      "my-old-name",
+							Name:      "my-old-name-1",
 							DiskUUID:  "100",
 							Type:      vmopv1.VolumeTypeClassic,
 							Attached:  true,
 							Limit:     kubeutil.BytesToResource(10 * oneGiBInBytes),
 							Requested: kubeutil.BytesToResource(10 * oneGiBInBytes),
 						},
+						{
+							Name:      "my-old-name-2",
+							DiskUUID:  "104",
+							Type:      vmopv1.VolumeTypeClassic,
+							Attached:  true,
+							Limit:     kubeutil.BytesToResource(10 * oneGiBInBytes),
+							Requested: kubeutil.BytesToResource(10 * oneGiBInBytes),
+						},
 					}
+					// Update disk with uuid 104 with same controller info in spec.
+					vd := vmCtx.MoVM.Config.Hardware.Device[5].(*vimtypes.VirtualDisk)
+					vd.ControllerKey = 200
+					vd.UnitNumber = ptr.To[int32](4)
 				})
 				Specify("status.volumes no longer includes the stale classic disk", func() {
 					Expect(vmCtx.VM.Status.Volumes).To(Equal([]vmopv1.VirtualMachineVolumeStatus{