Update CD-ROM API fields and related webhook code (#659)

This PR updates CD-ROM API fields and docs for clarity and improves the VM webhook for ISO VM deployment. Specifically, this patch includes: - Fix `AllowGuestControl` and `Connected` fields with boolean pointers. - Update VM mutating webhook to not set the default CD-ROM name and re-populate `VirtualMachineImage` kind if previously set to default. - Update VM validating webhook to require `guestID` if VM has a CD-ROM and handle duplicates across namespace and cluster scope images (since they will point to the same backing file name).
vmware-tanzu · Aug 12, 2024 · a72c3b4 · a72c3b4
1 parent 00878bd
commit a72c3b4
Show file tree

Hide file tree

Showing 19 changed files with 477 additions and 179 deletions.
diff --git a/api/v1alpha1/virtualmachine_conversion_test.go b/api/v1alpha1/virtualmachine_conversion_test.go
@@ -224,8 +224,8 @@ func TestVirtualMachineConversion(t *testing.T) {
 							Name: "my-cdrom-image",
 							Kind: "VirtualMachineImage",
 						},
-						AllowGuestControl: true,
-						Connected:         true,
+						AllowGuestControl: ptrOf(true),
+						Connected:         ptrOf(true),
 					},
 				},
 			},

diff --git a/api/v1alpha2/virtualmachine_conversion_test.go b/api/v1alpha2/virtualmachine_conversion_test.go
@@ -246,8 +246,8 @@ func TestVirtualMachineConversion(t *testing.T) {
 							Name: "my-cdrom-image",
 							Kind: "VirtualMachineImage",
 						},
-						AllowGuestControl: true,
-						Connected:         true,
+						AllowGuestControl: ptrOf(true),
+						Connected:         ptrOf(true),
 					},
 				},
 			},

diff --git a/api/v1alpha3/virtualmachine_types.go b/api/v1alpha3/virtualmachine_types.go
@@ -276,9 +276,10 @@ type VirtualMachineImageRef struct {
 type VirtualMachineCdromSpec struct {
 	// +kubebuilder:validation:Pattern="^[a-z0-9]{2,}$"
 
-	// Name describes the unique and immutable name of this CD-ROM device.
-	// If omitted or empty, the controller will generate it as "cdrom" + "num",
-	// where "num" is the device's index in the spec.cdrom list.
+	// Name consists of at least two lowercase letters or digits of this CD-ROM.
+	// It must be unique among all CD-ROM devices attached to the VM.
+	//
+	// This field is immutable when the VM is powered on.
 	Name string `json:"name"`
 
 	// Image describes the reference to an ISO type VirtualMachineImage or
@@ -303,8 +304,11 @@ type VirtualMachineCdromSpec struct {
 	// disconnected from the VM. If the CD-ROM device already exists, it is
 	// updated to a disconnected state.
 	//
+	// Please note that disconnecting a CD-ROM during guest OS installation may
+	// not work since the CD-ROM might be locked by the guest.
+	//
 	// Defaults to true if omitted.
-	Connected bool `json:"connected,omitempty"`
+	Connected *bool `json:"connected,omitempty"`
 
 	// +optional
 	// +kubebuilder:default=true
@@ -313,7 +317,7 @@ type VirtualMachineCdromSpec struct {
 	// may be used to connect/disconnect the CD-ROM device.
 	//
 	// Defaults to true if omitted.
-	AllowGuestControl bool `json:"allowGuestControl,omitempty"`
+	AllowGuestControl *bool `json:"allowGuestControl,omitempty"`
 }
 
 // VirtualMachineSpec defines the desired state of a VirtualMachine.
@@ -326,7 +330,9 @@ type VirtualMachineSpec struct {
 	//
 	// Each CD-ROM device requires a reference to an ISO-type
 	// VirtualMachineImage or ClusterVirtualMachineImage resource as backing.
-	// More than one CD-ROM device with the backing image is disallowed.
+	//
+	// Multiple CD-ROM devices using the same backing image, regardless of image
+	// kinds (namespace or cluster scope), are not allowed.
 	//
 	// CD-ROM devices can be added, updated, or removed when the VM is powered
 	// off. When the VM is powered on, only the connection state of existing
@@ -590,18 +596,19 @@ type VirtualMachineSpec struct {
 	//
 	// The logic that determines the guest ID is as follows:
 	//
-	// 1. If this field is set, then its value is used.
-	// 2. Otherwise, if the VirtualMachineClass used to deploy the VM contains a
-	//    non-empty guest ID, then it is used.
-	// 3. Finally, if this field is still undetermined, and the VM is deployed
-	// from an OVF template that defines a guest ID, then that value is used.
+	// If this field is set, then its value is used.
+	// Otherwise, if the VM is deployed from an OVF template that defines a
+	// guest ID, then that value is used.
+	// The guest ID from VirtualMachineClass used to deploy the VM is ignored.
 	//
 	// Please refer to https://bit.ly/4elnjP3 for a complete list of supported
 	// guest operating system identifiers.
 	//
 	// Please note that this field is immutable after the VM is powered on.
 	// To change the guest ID after the VM is powered on, the VM must be powered
 	// off and then powered on again with the updated guest ID spec.
+	//
+	// This field is required when the VM has any CD-ROM devices attached.
 	GuestID string `json:"guestID,omitempty"`
 }
 

diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go
diff --git a/config/crd/bases/vmoperator.vmware.com_virtualmachinereplicasets.yaml b/config/crd/bases/vmoperator.vmware.com_virtualmachinereplicasets.yaml
@@ -993,7 +993,10 @@ spec:
 
                           Each CD-ROM device requires a reference to an ISO-type
                           VirtualMachineImage or ClusterVirtualMachineImage resource as backing.
-                          More than one CD-ROM device with the backing image is disallowed.
+
+
+                          Multiple CD-ROM devices using the same backing image, regardless of image
+                          kinds (namespace or cluster scope), are not allowed.
 
 
                           CD-ROM devices can be added, updated, or removed when the VM is powered
@@ -1028,6 +1031,10 @@ spec:
                                 updated to a disconnected state.
 
 
+                                Please note that disconnecting a CD-ROM during guest OS installation may
+                                not work since the CD-ROM might be locked by the guest.
+
+
                                 Defaults to true if omitted.
                               type: boolean
                             image:
@@ -1059,9 +1066,11 @@ spec:
                               type: object
                             name:
                               description: |-
-                                Name describes the unique and immutable name of this CD-ROM device.
-                                If omitted or empty, the controller will generate it as "cdrom" + "num",
-                                where "num" is the device's index in the spec.cdrom list.
+                                Name consists of at least two lowercase letters or digits of this CD-ROM.
+                                It must be unique among all CD-ROM devices attached to the VM.
+
+
+                                This field is immutable when the VM is powered on.
                               pattern: ^[a-z0-9]{2,}$
                               type: string
                           required:
@@ -1091,11 +1100,10 @@ spec:
                           The logic that determines the guest ID is as follows:
 
 
-                          1. If this field is set, then its value is used.
-                          2. Otherwise, if the VirtualMachineClass used to deploy the VM contains a
-                             non-empty guest ID, then it is used.
-                          3. Finally, if this field is still undetermined, and the VM is deployed
-                          from an OVF template that defines a guest ID, then that value is used.
+                          If this field is set, then its value is used.
+                          Otherwise, if the VM is deployed from an OVF template that defines a
+                          guest ID, then that value is used.
+                          The guest ID from VirtualMachineClass used to deploy the VM is ignored.
 
 
                           Please refer to https://bit.ly/4elnjP3 for a complete list of supported
@@ -1105,6 +1113,9 @@ spec:
                           Please note that this field is immutable after the VM is powered on.
                           To change the guest ID after the VM is powered on, the VM must be powered
                           off and then powered on again with the updated guest ID spec.
+
+
+                          This field is required when the VM has any CD-ROM devices attached.
                         type: string
                       image:
                         description: |-

diff --git a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml
@@ -3980,7 +3980,10 @@ spec:
 
                   Each CD-ROM device requires a reference to an ISO-type
                   VirtualMachineImage or ClusterVirtualMachineImage resource as backing.
-                  More than one CD-ROM device with the backing image is disallowed.
+
+
+                  Multiple CD-ROM devices using the same backing image, regardless of image
+                  kinds (namespace or cluster scope), are not allowed.
 
 
                   CD-ROM devices can be added, updated, or removed when the VM is powered
@@ -4015,6 +4018,10 @@ spec:
                         updated to a disconnected state.
 
 
+                        Please note that disconnecting a CD-ROM during guest OS installation may
+                        not work since the CD-ROM might be locked by the guest.
+
+
                         Defaults to true if omitted.
                       type: boolean
                     image:
@@ -4046,9 +4053,11 @@ spec:
                       type: object
                     name:
                       description: |-
-                        Name describes the unique and immutable name of this CD-ROM device.
-                        If omitted or empty, the controller will generate it as "cdrom" + "num",
-                        where "num" is the device's index in the spec.cdrom list.
+                        Name consists of at least two lowercase letters or digits of this CD-ROM.
+                        It must be unique among all CD-ROM devices attached to the VM.
+
+
+                        This field is immutable when the VM is powered on.
                       pattern: ^[a-z0-9]{2,}$
                       type: string
                   required:
@@ -4078,11 +4087,10 @@ spec:
                   The logic that determines the guest ID is as follows:
 
 
-                  1. If this field is set, then its value is used.
-                  2. Otherwise, if the VirtualMachineClass used to deploy the VM contains a
-                     non-empty guest ID, then it is used.
-                  3. Finally, if this field is still undetermined, and the VM is deployed
-                  from an OVF template that defines a guest ID, then that value is used.
+                  If this field is set, then its value is used.
+                  Otherwise, if the VM is deployed from an OVF template that defines a
+                  guest ID, then that value is used.
+                  The guest ID from VirtualMachineClass used to deploy the VM is ignored.
 
 
                   Please refer to https://bit.ly/4elnjP3 for a complete list of supported
@@ -4092,6 +4100,9 @@ spec:
                   Please note that this field is immutable after the VM is powered on.
                   To change the guest ID after the VM is powered on, the VM must be powered
                   off and then powered on again with the updated guest ID spec.
+
+
+                  This field is required when the VM has any CD-ROM devices attached.
                 type: string
               image:
                 description: |-

diff --git a/docs/ref/api/v1alpha3.md b/docs/ref/api/v1alpha3.md
@@ -692,9 +692,11 @@ _Appears in:_
 
 | Field | Description |
 | --- | --- |
-| `name` _string_ | Name describes the unique and immutable name of this CD-ROM device.
-If omitted or empty, the controller will generate it as "cdrom" + "num",
-where "num" is the device's index in the spec.cdrom list. |
+| `name` _string_ | Name consists of at least two lowercase letters or digits of this CD-ROM.
+It must be unique among all CD-ROM devices attached to the VM.
+
+
+This field is immutable when the VM is powered on. |
 | `image` _[VirtualMachineImageRef](#virtualmachineimageref)_ | Image describes the reference to an ISO type VirtualMachineImage or
 ClusterVirtualMachineImage resource used as the backing for the CD-ROM.
 If the image kind is omitted, it defaults to VirtualMachineImage.
@@ -717,6 +719,10 @@ disconnected from the VM. If the CD-ROM device already exists, it is
 updated to a disconnected state.
 
 
+Please note that disconnecting a CD-ROM during guest OS installation may
+not work since the CD-ROM might be locked by the guest.
+
+
 Defaults to true if omitted. |
 | `allowGuestControl` _boolean_ | AllowGuestControl describes whether or not a web console connection
 may be used to connect/disconnect the CD-ROM device.
@@ -822,6 +828,21 @@ _Appears in:_
 
 
 
+### VirtualMachineImageDiskInfo
+
+
+
+VirtualMachineImageDiskInfo describes information about any disks associated with
+this image.
+
+_Appears in:_
+- [VirtualMachineImageStatus](#virtualmachineimagestatus)
+
+| Field | Description |
+| --- | --- |
+| `capacity` _[Quantity](#quantity)_ | Capacity is the virtual disk capacity in bytes. |
+| `size` _[Quantity](#quantity)_ | Size is the estimated populated size of the virtual disk in bytes. |
+
 ### VirtualMachineImageOSInfo
 
 
@@ -942,6 +963,7 @@ image. |
 | `vmwareSystemProperties` _KeyValuePair array_ | VMwareSystemProperties describes the observed VMware system properties defined for
 this image. |
 | `productInfo` _[VirtualMachineImageProductInfo](#virtualmachineimageproductinfo)_ | ProductInfo describes the observed product information for this image. |
+| `disks` _[VirtualMachineImageDiskInfo](#virtualmachineimagediskinfo) array_ | Disks describes the observed disk information for this image. |
 | `providerContentVersion` _string_ | ProviderContentVersion describes the content version from the provider item
 that this image corresponds to. If the provider of this image is a Content
 Library, this will be the version of the corresponding Content Library item. |
@@ -2090,7 +2112,10 @@ _Appears in:_
 
 Each CD-ROM device requires a reference to an ISO-type
 VirtualMachineImage or ClusterVirtualMachineImage resource as backing.
-More than one CD-ROM device with the backing image is disallowed.
+
+
+Multiple CD-ROM devices using the same backing image, regardless of image
+kinds (namespace or cluster scope), are not allowed.
 
 
 CD-ROM devices can be added, updated, or removed when the VM is powered
@@ -2303,11 +2328,10 @@ default value for spec.bootstrap.cloudInit.instanceID if it is omitted. |
 The logic that determines the guest ID is as follows:
 
 
-1. If this field is set, then its value is used.
-2. Otherwise, if the VirtualMachineClass used to deploy the VM contains a
-   non-empty guest ID, then it is used.
-3. Finally, if this field is still undetermined, and the VM is deployed
-from an OVF template that defines a guest ID, then that value is used.
+If this field is set, then its value is used.
+Otherwise, if the VM is deployed from an OVF template that defines a
+guest ID, then that value is used.
+The guest ID from VirtualMachineClass used to deploy the VM is ignored.
 
 
 Please refer to https://bit.ly/4elnjP3 for a complete list of supported
@@ -2316,7 +2340,10 @@ guest operating system identifiers.
 
 Please note that this field is immutable after the VM is powered on.
 To change the guest ID after the VM is powered on, the VM must be powered
-off and then powered on again with the updated guest ID spec. |
+off and then powered on again with the updated guest ID spec.
+
+
+This field is required when the VM has any CD-ROM devices attached. |
 
 ### VirtualMachineStatus
 

diff --git a/pkg/providers/vsphere/session/session_vm_update_test.go b/pkg/providers/vsphere/session/session_vm_update_test.go
@@ -1553,8 +1553,8 @@ var _ = Describe("UpdateVirtualMachine", func() {
 									Name: vmiName,
 									Kind: vmiKind,
 								},
-								AllowGuestControl: true,
-								Connected:         true,
+								AllowGuestControl: ptr.To(true),
+								Connected:         ptr.To(true),
 							},
 						}
 					})

diff --git a/pkg/providers/vsphere/vmprovider_vm_utils_test.go b/pkg/providers/vsphere/vmprovider_vm_utils_test.go
@@ -207,10 +207,10 @@ func vmUtilTests() {
 		)
 
 		BeforeEach(func() {
-			nsVMImage = builder.DummyVirtualMachineImage(builder.DummyVMIID)
+			nsVMImage = builder.DummyVirtualMachineImage(builder.DummyVMIName)
 			nsVMImage.Namespace = vmCtx.VM.Namespace
 			conditions.MarkTrue(nsVMImage, vmopv1.ReadyConditionType)
-			clusterVMImage = builder.DummyClusterVirtualMachineImage(builder.DummyVMIID)
+			clusterVMImage = builder.DummyClusterVirtualMachineImage(builder.DummyCVMIName)
 			conditions.MarkTrue(clusterVMImage, vmopv1.ReadyConditionType)
 		})
 

diff --git a/pkg/util/vmopv1/resize_overwrite_test.go b/pkg/util/vmopv1/resize_overwrite_test.go
@@ -91,6 +91,8 @@ var _ = Describe("OverwriteResizeConfigSpec", func() {
 	vmAdvSpec := func(advSpec vmopv1.VirtualMachineAdvancedSpec) vmopv1.VirtualMachine {
 		vm := builder.DummyVirtualMachine()
 		vm.Spec.Advanced = &advSpec
+		// Empty guestID to avoid it being set in the returned ConfigSpec.
+		vm.Spec.GuestID = ""
 		return *vm
 	}