Update storage status

This patch updates how a VM's storage usage is surfaced. Now
the usage is surfaced in a much more human-friendly manner:

* Total used that counts against namespace storage quota
* Total used by non-PVC disks
* Total used by VM's non-disk files

api/v1alpha1/virtualmachine_conversion_test.go

@@ -1699,9 +1699,11 @@ func TestVirtualMachineConversion(t *testing.T) {
 			hub := vmopv1.VirtualMachine{
 				Status: vmopv1.VirtualMachineStatus{
 					Storage: &vmopv1.VirtualMachineStorageStatus{
-						Committed:   ptrOf(resource.MustParse("10Gi")),
-						Uncommitted: ptrOf(resource.MustParse("20Gi")),
-						Unshared:    ptrOf(resource.MustParse("9Gi")),
+						Usage: &vmopv1.VirtualMachineStorageStatusUsage{
+							Total: ptrOf(resource.MustParse("20Gi")),
+							Disks: ptrOf(resource.MustParse("10Gi")),
+							Other: ptrOf(resource.MustParse("10Gi")),
+						},
 					},
 				},
 			}

api/v1alpha2/virtualmachine_conversion_test.go

@@ -280,9 +280,11 @@ func TestVirtualMachineConversion(t *testing.T) {
 			hub := vmopv1.VirtualMachine{
 				Status: vmopv1.VirtualMachineStatus{
 					Storage: &vmopv1.VirtualMachineStorageStatus{
-						Committed:   ptrOf(resource.MustParse("10Gi")),
-						Uncommitted: ptrOf(resource.MustParse("20Gi")),
-						Unshared:    ptrOf(resource.MustParse("9Gi")),
+						Usage: &vmopv1.VirtualMachineStorageStatusUsage{
+							Total: ptrOf(resource.MustParse("20Gi")),
+							Disks: ptrOf(resource.MustParse("10Gi")),
+							Other: ptrOf(resource.MustParse("10Gi")),
+						},
 					},
 				},
 			}

api/v1alpha3/virtualmachine_storage_types.go

@@ -166,21 +166,29 @@ func SortVirtualMachineVolumeStatuses(s []VirtualMachineVolumeStatus) {
 // VirtualMachineStorageStatus defines the observed state of a VirtualMachine's
 // storage.
 type VirtualMachineStorageStatus struct {
+	// +optional
+
+	// Usage describes the observed amount of storage used by a VirtualMachine.
+	Usage *VirtualMachineStorageStatusUsage `json:"usage,omitempty"`
+}
 
+type VirtualMachineStorageStatusUsage struct {
 	// +optional
 
-	// Committed is the total storage space committed to this VirtualMachine.
-	Committed *resource.Quantity `json:"committed,omitempty"`
+	// Total describes the total storage space used by a VirtualMachine that
+	// counts against the Namespace's storage quota.
+	Total *resource.Quantity `json:"total,omitempty"`
 
 	// +optional
 
-	// Uncommitted is the total storage space potentially used by this
-	// VirtualMachine.
-	Uncommitted *resource.Quantity `json:"uncommitted,omitempty"`
+	// Disks describes the total storage space used by a VirtualMachine's
+	// non-PVC disks.
+	Disks *resource.Quantity `json:"disks,omitempty"`
 
 	// +optional
 
-	// Unshared is the total storage space occupied by this VirtualMachine that
-	// is not shared with any other VirtualMachine.
-	Unshared *resource.Quantity `json:"unshared,omitempty"`
+	// Other describes the total storage space used by the VirtualMachine's
+	// non disk files, ex. the configuration file, swap space, logs, snapshots,
+	// etc.
+	Other *resource.Quantity `json:"other,omitempty"`
 }

config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml

@@ -5422,32 +5422,39 @@ spec:
                 description: Storage describes the observed state of the VirtualMachine's
                   storage.
                 properties:
-                  committed:
-                    anyOf:
-                    - type: integer
-                    - type: string
-                    description: Committed is the total storage space committed to
-                      this VirtualMachine.
-                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                    x-kubernetes-int-or-string: true
-                  uncommitted:
-                    anyOf:
-                    - type: integer
-                    - type: string
-                    description: |-
-                      Uncommitted is the total storage space potentially used by this
-                      VirtualMachine.
-                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                    x-kubernetes-int-or-string: true
-                  unshared:
-                    anyOf:
-                    - type: integer
-                    - type: string
-                    description: |-
-                      Unshared is the total storage space occupied by this VirtualMachine that
-                      is not shared with any other VirtualMachine.
-                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                    x-kubernetes-int-or-string: true
+                  usage:
+                    description: Usage describes the observed amount of storage used
+                      by a VirtualMachine.
+                    properties:
+                      disks:
+                        anyOf:
+                        - type: integer
+                        - type: string
+                        description: |-
+                          Disks describes the total storage space used by a VirtualMachine's
+                          non-PVC disks.
+                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                        x-kubernetes-int-or-string: true
+                      other:
+                        anyOf:
+                        - type: integer
+                        - type: string
+                        description: |-
+                          Other describes the total storage space used by the VirtualMachine's
+                          non disk files, ex. the configuration file, swap space, logs, snapshots,
+                          etc.
+                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                        x-kubernetes-int-or-string: true
+                      total:
+                        anyOf:
+                        - type: integer
+                        - type: string
+                        description: |-
+                          Total describes the total storage space used by a VirtualMachine that
+                          counts against the Namespace's storage quota.
+                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                        x-kubernetes-int-or-string: true
+                    type: object
                 type: object
               uniqueID:
                 description: |-

controllers/virtualmachine/storagepolicyusage/storagepolicyusage_controller.go

@@ -182,21 +182,8 @@ func reportUsed(
 	vm vmopv1.VirtualMachine,
 	total *resource.Quantity) {
 
-	// The VM is created, and therefore the VM's actual usage should be
-	// reported as its Used value.
-	if vm.Status.Storage.Unshared != nil {
-		total.Add(*vm.Status.Storage.Unshared)
-	}
-
-	// Subtract the PVC usage/limit from the total used value as PVCs
-	// are counted separately.
-	for j := range vm.Status.Volumes {
-		v := vm.Status.Volumes[j]
-		if v.Type == vmopv1.VirtualMachineStorageDiskTypeManaged {
-			if v.Used != nil {
-				total.Sub(*v.Used)
-			}
-		}
+	if vm.Status.Storage != nil {
+		total.Add(*vm.Status.Storage.Usage.Total)
 	}
 }
 

controllers/virtualmachine/storagepolicyusage/storagepolicyusage_controller_intg_test.go

@@ -135,22 +135,8 @@ func intgTestsReconcile() {
 							},
 						},
 						Storage: &vmopv1.VirtualMachineStorageStatus{
-							Committed:   ptr.To(resource.MustParse("10Gi")),
-							Uncommitted: ptr.To(resource.MustParse("20Gi")),
-							Unshared:    ptr.To(resource.MustParse("5Gi")),
-						},
-						Volumes: []vmopv1.VirtualMachineVolumeStatus{
-							{
-								Name:  "vm-1",
-								Type:  vmopv1.VirtualMachineStorageDiskTypeClassic,
-								Limit: ptr.To(resource.MustParse("10Gi")),
-								Used:  ptr.To(resource.MustParse("5Gi")),
-							},
-							{
-								Name:  "vol-1",
-								Type:  vmopv1.VirtualMachineStorageDiskTypeManaged,
-								Limit: ptr.To(resource.MustParse("10Gi")),
-								Used:  ptr.To(resource.MustParse("5Gi")),
+							Usage: &vmopv1.VirtualMachineStorageStatusUsage{
+								Total: ptr.To(resource.MustParse("20Gi")),
 							},
 						},
 					},
@@ -175,9 +161,9 @@ func intgTestsReconcile() {
 							},
 						},
 						Storage: &vmopv1.VirtualMachineStorageStatus{
-							Committed:   ptr.To(resource.MustParse("5Gi")),
-							Uncommitted: ptr.To(resource.MustParse("10Gi")),
-							Unshared:    ptr.To(resource.MustParse("2Gi")),
+							Usage: &vmopv1.VirtualMachineStorageStatusUsage{
+								Total: ptr.To(resource.MustParse("50Gi")),
+							},
 						},
 					},
 				},

controllers/virtualmachine/storagepolicyusage/storagepolicyusage_controller_unit_test.go

@@ -296,22 +296,8 @@ func unitTestsReconcile() {
 							},
 						},
 						Storage: &vmopv1.VirtualMachineStorageStatus{
-							Committed:   ptr.To(resource.MustParse("10Gi")),
-							Uncommitted: ptr.To(resource.MustParse("20Gi")),
-							Unshared:    ptr.To(resource.MustParse("5Gi")),
-						},
-						Volumes: []vmopv1.VirtualMachineVolumeStatus{
-							{
-								Name:  "vm-1",
-								Type:  vmopv1.VirtualMachineStorageDiskTypeClassic,
-								Limit: ptr.To(resource.MustParse("20Gi")),
-								Used:  ptr.To(resource.MustParse("10Gi")),
-							},
-							{
-								Name:  "vol-1",
-								Type:  vmopv1.VirtualMachineStorageDiskTypeManaged,
-								Limit: ptr.To(resource.MustParse("10Gi")),
-								Used:  ptr.To(resource.MustParse("2Gi")),
+							Usage: &vmopv1.VirtualMachineStorageStatusUsage{
+								Total: ptr.To(resource.MustParse("20Gi")),
 							},
 						},
 					},
@@ -336,22 +322,8 @@ func unitTestsReconcile() {
 							},
 						},
 						Storage: &vmopv1.VirtualMachineStorageStatus{
-							Committed:   ptr.To(resource.MustParse("5Gi")),
-							Uncommitted: ptr.To(resource.MustParse("10Gi")),
-							Unshared:    ptr.To(resource.MustParse("2Gi")),
-						},
-						Volumes: []vmopv1.VirtualMachineVolumeStatus{
-							{
-								Name:  "vm-2",
-								Type:  vmopv1.VirtualMachineStorageDiskTypeClassic,
-								Limit: ptr.To(resource.MustParse("10Gi")),
-								Used:  ptr.To(resource.MustParse("1Gi")),
-							},
-							{
-								Name:  "vol-2",
-								Type:  vmopv1.VirtualMachineStorageDiskTypeManaged,
-								Limit: ptr.To(resource.MustParse("10Gi")),
-								Used:  ptr.To(resource.MustParse("0.5Gi")),
+							Usage: &vmopv1.VirtualMachineStorageStatusUsage{
+								Total: ptr.To(resource.MustParse("50Gi")),
 							},
 						},
 					},

pkg/providers/vsphere/vmlifecycle/update_status.go

@@ -742,8 +742,8 @@ func updateGuestNetworkStatus(vm *vmopv1.VirtualMachine, gi *vimtypes.GuestInfo)
 // updateStorageStatus updates the status for all storage-related fields.
 func updateStorageStatus(vm *vmopv1.VirtualMachine, moVM mo.VirtualMachine) {
 	updateChangeBlockTracking(vm, moVM)
-	updateStorageUsage(vm, moVM)
 	updateVolumeStatus(vm, moVM)
+	updateStorageUsage(vm, moVM)
 }
 
 func updateChangeBlockTracking(vm *vmopv1.VirtualMachine, moVM mo.VirtualMachine) {
@@ -755,21 +755,68 @@ func updateChangeBlockTracking(vm *vmopv1.VirtualMachine, moVM mo.VirtualMachine
 }
 
 func updateStorageUsage(vm *vmopv1.VirtualMachine, moVM mo.VirtualMachine) {
-	if moVM.Summary.Storage == nil {
+	// Get the storage consumed by non-disks.
+	var other int64
+	if moVM.LayoutEx != nil {
+		for i := range moVM.LayoutEx.File {
+			f := moVM.LayoutEx.File[i]
+			switch vimtypes.VirtualMachineFileLayoutExFileType(f.Type) {
+			case vimtypes.VirtualMachineFileLayoutExFileTypeDiskDescriptor,
+				vimtypes.VirtualMachineFileLayoutExFileTypeDiskExtent,
+				vimtypes.VirtualMachineFileLayoutExFileTypeDigestDescriptor,
+				vimtypes.VirtualMachineFileLayoutExFileTypeDigestExtent:
+
+				// Skip disks
+
+			default:
+				other += f.UniqueSize
+			}
+		}
+	}
+
+	// Get the storage consumed by disks.
+	var disks *resource.Quantity
+	for i := range vm.Status.Volumes {
+		v := vm.Status.Volumes[i]
+		if v.Type == vmopv1.VirtualMachineStorageDiskTypeClassic {
+			if disks == nil {
+				disks = ptr.To(v.Used.DeepCopy())
+			} else if v.Used != nil {
+				disks.Add(*v.Used)
+			}
+		}
+	}
+
+	if disks == nil && other == 0 {
 		return
 	}
+
 	if vm.Status.Storage == nil {
 		vm.Status.Storage = &vmopv1.VirtualMachineStorageStatus{}
 	}
+	if vm.Status.Storage.Usage == nil {
+		vm.Status.Storage.Usage = &vmopv1.VirtualMachineStorageStatusUsage{}
+	}
 
-	vm.Status.Storage.Committed = BytesToResourceGiB(
-		moVM.Summary.Storage.Committed)
+	vm.Status.Storage.Usage.Disks = nil
+	vm.Status.Storage.Usage.Other = nil
+	vm.Status.Storage.Usage.Total = &resource.Quantity{}
 
-	vm.Status.Storage.Uncommitted = BytesToResourceGiB(
-		moVM.Summary.Storage.Uncommitted)
+	if disks != nil {
+		vm.Status.Storage.Usage.Disks = disks
+		vm.Status.Storage.Usage.Total.Add(*disks)
+	}
+
+	if other > 0 {
+		o := BytesToResourceGiB(other)
+		vm.Status.Storage.Usage.Other = o
+		vm.Status.Storage.Usage.Total.Add(*o)
+	}
 
-	vm.Status.Storage.Unshared = BytesToResourceGiB(
-		moVM.Summary.Storage.Unshared)
+	if t := vm.Status.Storage.Usage.Total; t != nil {
+		i, _ := vm.Status.Storage.Usage.Total.AsInt64()
+		vm.Status.Storage.Usage.Total = BytesToResourceGiB(i)
+	}
 }
 
 func updateVolumeStatus(vm *vmopv1.VirtualMachine, moVM mo.VirtualMachine) {