From cbcfceef10ef69c544fb1b1e98f971adc1fded40 Mon Sep 17 00:00:00 2001 From: Sagar Muchhal Date: Mon, 22 May 2023 14:05:34 -0700 Subject: [PATCH] Adds minimum h/w version to VirtualMachine CRD Introduces a new field on the VirtualMachine CRD which allows the user to define the minimum hardware version for the provisioned VM. The behavior of the field is as follows: - If the VMClassAsConfig FSS is enabled, it will override the hardware version derived from the VMClass's ConfigSpec if it is lower than the minimum version field. Otherwise, the value from the ConfigSpec is honored. - If the VMClassAsConfig FSS is disabled, it will reconfigure the VM post creation before the power on operation to ensure that the h/w version is atleast set to the minimum version. This means, it can override the hardware version of the OVA with a hardware version value lower than this field. Signed-off-by: Sagar Muchhal --- api/v1alpha1/virtualmachine_types.go | 47 +++++++++++++ api/v1alpha2/virtualmachine_types.go | 49 ++++++++++++- ...vmoperator.vmware.com_virtualmachines.yaml | 68 ++++++++++++++++++- pkg/util/configspec.go | 18 +++++ pkg/util/configspec_test.go | 39 +++++++++++ pkg/util/hardware_version.go | 29 ++++++++ pkg/util/hardware_version_test.go | 28 ++++++++ .../contentlibrary/content_library_utils.go | 25 +------ .../content_library_utils_test.go | 18 ----- .../vsphere/session/session_vm_status.go | 2 + .../providers/vsphere/vmprovider_vm.go | 5 +- .../providers/vsphere/vmprovider_vm_test.go | 34 ++++++++++ .../vsphere2/vmlifecycle/update_status.go | 2 + .../vmlifecycle/update_status_test.go | 21 ++++++ .../providers/vsphere2/vmprovider_vm.go | 2 + .../providers/vsphere2/vmprovider_vm_test.go | 30 ++++++++ test/builder/util.go | 12 ++-- test/builder/utila2.go | 11 +-- .../validation/virtualmachine_validator.go | 2 + .../virtualmachine_validator_intg_test.go | 12 ++++ .../validation/virtualmachine_validator.go | 4 +- .../virtualmachine_validator_intg_test.go | 12 ++++ 22 files changed, 414 insertions(+), 56 deletions(-) create mode 100644 pkg/util/hardware_version.go create mode 100644 pkg/util/hardware_version_test.go diff --git a/api/v1alpha1/virtualmachine_types.go b/api/v1alpha1/virtualmachine_types.go index b562f50e7..3d20e64e7 100644 --- a/api/v1alpha1/virtualmachine_types.go +++ b/api/v1alpha1/virtualmachine_types.go @@ -486,6 +486,44 @@ type VirtualMachineSpec struct { // AdvancedOptions describes a set of optional, advanced options for configuring a VirtualMachine AdvancedOptions *VirtualMachineAdvancedOptions `json:"advancedOptions,omitempty"` + + // MinHardwareVersion specifies the desired minimum hardware version + // for this VM. + // + // Usually the VM's hardware version is derived from: + // 1. the VirtualMachineClass used to deploy the VM provided by the ClassName field + // 2. the datacenter/cluster/host default hardware version + // Setting this field will ensure that the hardware version of the VM + // is at least set to the specified value. To enforce this, it will override + // the value from the VirtualMachineClass. + // + // This field is never updated to reflect the derived hardware version. + // Instead, VirtualMachineStatus.HardwareVersion surfaces + // the observed hardware version. + // + // Please note, setting this field's value to N ensures a VM's hardware + // version is equal to or greater than N. For example, if a VM's observed + // hardware version is 10 and this field's value is 13, then the VM will be + // upgraded to hardware version 13. However, if the observed hardware + // version is 17 and this field's value is 13, no change will occur. + // + // Several features are hardware version dependent, for example: + // + // * NVMe Controllers >= 14 + // * Dynamic Direct Path I/O devices >= 17 + // + // Please refer to https://kb.vmware.com/s/article/1003746 for a list of VM + // hardware versions. + // + // It is important to remember that a VM's hardware version may not be + // downgraded and upgrading a VM deployed from an image based on an older + // hardware version to a more recent one may result in unpredictable + // behavior. In other words, please be careful when choosing to upgrade a + // VM to a newer hardware version. + // + // +optional + // +kubebuilder:validation:Minimum=13 + MinHardwareVersion int32 `json:"minHardwareVersion,omitempty"` } // VirtualMachineAdvancedOptions describes a set of optional, advanced options for configuring a VirtualMachine. @@ -602,6 +640,15 @@ type VirtualMachineStatus struct { // LastRestartTime describes the last time the VM was restarted. // +optional LastRestartTime *metav1.Time `json:"lastRestartTime,omitempty"` + + // HardwareVersion describes the VirtualMachine resource's observed + // hardware version. + // + // Please refer to VirtualMachineSpec.MinHardwareVersion for more + // information on the topic of a VM's hardware version. + // + // +optional + HardwareVersion int32 `json:"hardwareVersion,omitempty"` } func (vm *VirtualMachine) GetConditions() Conditions { diff --git a/api/v1alpha2/virtualmachine_types.go b/api/v1alpha2/virtualmachine_types.go index 843253780..b1546dafe 100644 --- a/api/v1alpha2/virtualmachine_types.go +++ b/api/v1alpha2/virtualmachine_types.go @@ -174,7 +174,7 @@ type VirtualMachineSpec struct { // +optional ImageName string `json:"imageName,omitempty"` - // Class describes the name of the VirtualMachineClass resource used to + // ClassName describes the name of the VirtualMachineClass resource used to // deploy this VM. // // This field is optional in the cases where there exists a sensible @@ -335,6 +335,44 @@ type VirtualMachineSpec struct { // // +optional Reserved VirtualMachineReservedSpec `json:"reserved,omitempty"` + + // MinHardwareVersion specifies the desired minimum hardware version + // for this VM. + // + // Usually the VM's hardware version is derived from: + // 1. the VirtualMachineClass used to deploy the VM provided by the ClassName field + // 2. the datacenter/cluster/host default hardware version + // Setting this field will ensure that the hardware version of the VM + // is at least set to the specified value. To enforce this, it will override + // the value from the VirtualMachineClass. + // + // This field is never updated to reflect the derived hardware version. + // Instead, VirtualMachineStatus.HardwareVersion surfaces + // the observed hardware version. + // + // Please note, setting this field's value to N ensures a VM's hardware + // version is equal to or greater than N. For example, if a VM's observed + // hardware version is 10 and this field's value is 13, then the VM will be + // upgraded to hardware version 13. However, if the observed hardware + // version is 17 and this field's value is 13, no change will occur. + // + // Several features are hardware version dependent, for example: + // + // * NVMe Controllers >= 14 + // * Dynamic Direct Path I/O devices >= 17 + // + // Please refer to https://kb.vmware.com/s/article/1003746 for a list of VM + // hardware versions. + // + // It is important to remember that a VM's hardware version may not be + // downgraded and upgrading a VM deployed from an image based on an older + // hardware version to a more recent one may result in unpredictable + // behavior. In other words, please be careful when choosing to upgrade a + // VM to a newer hardware version. + // + // +optional + // +kubebuilder:validation:Minimum=13 + MinHardwareVersion int32 `json:"minHardwareVersion,omitempty"` } // VirtualMachineReservedSpec describes a set of VM configuration options @@ -455,6 +493,15 @@ type VirtualMachineStatus struct { // // +optional LastRestartTime *metav1.Time `json:"lastRestartTime,omitempty"` + + // HardwareVersion describes the VirtualMachine resource's observed + // hardware version. + // + // Please refer to VirtualMachineSpec.MinHardwareVersion for more + // information on the topic of a VM's hardware version. + // + // +optional + HardwareVersion int32 `json:"hardwareVersion,omitempty"` } // +kubebuilder:object:root=true diff --git a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml index 12a4f4341..5cf3895a5 100644 --- a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml +++ b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml @@ -104,6 +104,33 @@ spec: be introspected to discover identifying attributes that may help users to identify the desired image to use. type: string + minHardwareVersion: + description: "MinHardwareVersion specifies the desired minimum hardware + version for this VM. \n Usually the VM's hardware version is derived + from: 1. the VirtualMachineClass used to deploy the VM provided + by the ClassName field 2. the datacenter/cluster/host default hardware + version Setting this field will ensure that the hardware version + of the VM is at least set to the specified value. To enforce this, + it will override the value from the VirtualMachineClass. \n This + field is never updated to reflect the derived hardware version. + Instead, VirtualMachineStatus.HardwareVersion surfaces the observed + hardware version. \n Please note, setting this field's value to + N ensures a VM's hardware version is equal to or greater than N. + For example, if a VM's observed hardware version is 10 and this + field's value is 13, then the VM will be upgraded to hardware version + 13. However, if the observed hardware version is 17 and this field's + value is 13, no change will occur. \n Several features are hardware + version dependent, for example: \n * NVMe Controllers \t\t + >= 14 * Dynamic Direct Path I/O devices >= 17 \n Please refer to + https://kb.vmware.com/s/article/1003746 for a list of VM hardware + versions. \n It is important to remember that a VM's hardware version + may not be downgraded and upgrading a VM deployed from an image + based on an older hardware version to a more recent one may result + in unpredictable behavior. In other words, please be careful when + choosing to upgrade a VM to a newer hardware version." + format: int32 + minimum: 13 + type: integer networkInterfaces: description: NetworkInterfaces describes a list of VirtualMachineNetworkInterfaces to be configured on the VirtualMachine instance. Each of these VirtualMachineNetworkInterfaces @@ -494,6 +521,12 @@ spec: - type type: object type: array + hardwareVersion: + description: "HardwareVersion describes the VirtualMachine resource's + observed hardware version. \n Please refer to VirtualMachineSpec.MinHardwareVersion + for more information on the topic of a VM's hardware version." + format: int32 + type: integer host: description: Host describes the hostname or IP address of the infrastructure host that the VirtualMachine is executing on. @@ -1500,7 +1533,7 @@ spec: type: object type: object className: - description: "Class describes the name of the VirtualMachineClass + description: "ClassName describes the name of the VirtualMachineClass resource used to deploy this VM. \n This field is optional in the cases where there exists a sensible default value, such as when there is a single VirtualMachineClass resource available in the @@ -1518,6 +1551,33 @@ spec: default value, such as when there is a single VirtualMachineImage resource available in the same Namespace as the VM being deployed." type: string + minHardwareVersion: + description: "MinHardwareVersion specifies the desired minimum hardware + version for this VM. \n Usually the VM's hardware version is derived + from: 1. the VirtualMachineClass used to deploy the VM provided + by the ClassName field 2. the datacenter/cluster/host default hardware + version Setting this field will ensure that the hardware version + of the VM is at least set to the specified value. To enforce this, + it will override the value from the VirtualMachineClass. \n This + field is never updated to reflect the derived hardware version. + Instead, VirtualMachineStatus.HardwareVersion surfaces the observed + hardware version. \n Please note, setting this field's value to + N ensures a VM's hardware version is equal to or greater than N. + For example, if a VM's observed hardware version is 10 and this + field's value is 13, then the VM will be upgraded to hardware version + 13. However, if the observed hardware version is 17 and this field's + value is 13, no change will occur. \n Several features are hardware + version dependent, for example: \n * NVMe Controllers \t\t + >= 14 * Dynamic Direct Path I/O devices >= 17 \n Please refer to + https://kb.vmware.com/s/article/1003746 for a list of VM hardware + versions. \n It is important to remember that a VM's hardware version + may not be downgraded and upgrading a VM deployed from an image + based on an older hardware version to a more recent one may result + in unpredictable behavior. In other words, please be careful when + choosing to upgrade a VM to a newer hardware version." + format: int32 + minimum: 13 + type: integer network: description: "Network describes the desired network configuration for the VM. \n Please note this value may be omitted entirely and @@ -2203,6 +2263,12 @@ spec: - type type: object type: array + hardwareVersion: + description: "HardwareVersion describes the VirtualMachine resource's + observed hardware version. \n Please refer to VirtualMachineSpec.MinHardwareVersion + for more information on the topic of a VM's hardware version." + format: int32 + type: integer host: description: Host describes the hostname or IP address of the infrastructure host where the VM is executed. diff --git a/pkg/util/configspec.go b/pkg/util/configspec.go index 8a62d364b..041f65cd6 100644 --- a/pkg/util/configspec.go +++ b/pkg/util/configspec.go @@ -5,6 +5,7 @@ package util import ( "bytes" + "fmt" "reflect" "github.com/vmware/govmomi/vim25" @@ -215,3 +216,20 @@ func MergeExtraConfig(extraConfig []vimTypes.BaseOptionValue, newMap map[string] } return merged } + +// EnsureMinHardwareVersionInConfigSpec ensures that the hardware version in the ConfigSpec +// is at least equal to the passed minimum hardware version value. +func EnsureMinHardwareVersionInConfigSpec(configSpec *vimTypes.VirtualMachineConfigSpec, minVersion int32) { + if minVersion == 0 { + return + } + + configSpecHwVersion := int32(0) + if configSpec.Version != "" { + configSpecHwVersion = ParseVirtualHardwareVersion(configSpec.Version) + } + if minVersion > configSpecHwVersion { + configSpecHwVersion = minVersion + } + configSpec.Version = fmt.Sprintf("vmx-%d", configSpecHwVersion) +} diff --git a/pkg/util/configspec_test.go b/pkg/util/configspec_test.go index 9d895f53c..bc631bf7b 100644 --- a/pkg/util/configspec_test.go +++ b/pkg/util/configspec_test.go @@ -165,6 +165,45 @@ var _ = Describe("ConfigSpec Util", func() { Expect(cmp.Diff(cs2, cs3)).To(BeEmpty()) }) }) + + Context("EnsureMinHardwareVersionInConfigSpec", func() { + When("minimum hardware version is unset", func() { + It("does not change the existing value of the configSpec's version", func() { + configSpec := &vimTypes.VirtualMachineConfigSpec{Version: "vmx-15"} + util.EnsureMinHardwareVersionInConfigSpec(configSpec, 0) + + Expect(configSpec.Version).To(Equal("vmx-15")) + }) + + It("does not set the configSpec's version", func() { + configSpec := &vimTypes.VirtualMachineConfigSpec{} + util.EnsureMinHardwareVersionInConfigSpec(configSpec, 0) + + Expect(configSpec.Version).To(BeEmpty()) + }) + }) + + It("overrides the hardware version if the existing version is lesser", func() { + configSpec := &vimTypes.VirtualMachineConfigSpec{Version: "vmx-15"} + util.EnsureMinHardwareVersionInConfigSpec(configSpec, 17) + + Expect(configSpec.Version).To(Equal("vmx-17")) + }) + + It("sets the hardware version if the existing version is unset", func() { + configSpec := &vimTypes.VirtualMachineConfigSpec{} + util.EnsureMinHardwareVersionInConfigSpec(configSpec, 16) + + Expect(configSpec.Version).To(Equal("vmx-16")) + }) + + It("overrides the hardware version if the existing version is set incorrectly", func() { + configSpec := &vimTypes.VirtualMachineConfigSpec{Version: "foo"} + util.EnsureMinHardwareVersionInConfigSpec(configSpec, 17) + + Expect(configSpec.Version).To(Equal("vmx-17")) + }) + }) }) var _ = Describe("RemoveDevicesFromConfigSpec", func() { diff --git a/pkg/util/hardware_version.go b/pkg/util/hardware_version.go new file mode 100644 index 000000000..416d5a2f3 --- /dev/null +++ b/pkg/util/hardware_version.go @@ -0,0 +1,29 @@ +// Copyright (c) 2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package util + +import ( + "regexp" + "strconv" +) + +var vmxRe = regexp.MustCompile(`vmx-(\d+)`) + +// ParseVirtualHardwareVersion parses the virtual hardware version +// For eg. "vmx-15" returns 15. +func ParseVirtualHardwareVersion(vmxVersion string) int32 { + // obj matches the full string and the submatch (\d+) + // and return a []string with values + obj := vmxRe.FindStringSubmatch(vmxVersion) + if len(obj) != 2 { + return 0 + } + + version, err := strconv.ParseInt(obj[1], 10, 32) + if err != nil { + return 0 + } + + return int32(version) +} diff --git a/pkg/util/hardware_version_test.go b/pkg/util/hardware_version_test.go new file mode 100644 index 000000000..b9b8a2e54 --- /dev/null +++ b/pkg/util/hardware_version_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package util_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/vmware-tanzu/vm-operator/pkg/util" +) + +var _ = Describe("ParseVirtualHardwareVersion", func() { + It("empty hardware string", func() { + vmxHwVersionString := "" + Expect(util.ParseVirtualHardwareVersion(vmxHwVersionString)).To(BeZero()) + }) + + It("invalid hardware string", func() { + vmxHwVersionString := "blah" + Expect(util.ParseVirtualHardwareVersion(vmxHwVersionString)).To(BeZero()) + }) + + It("valid hardware version string eg. vmx-15", func() { + vmxHwVersionString := "vmx-15" + Expect(util.ParseVirtualHardwareVersion(vmxHwVersionString)).To(Equal(int32(15))) + }) +}) diff --git a/pkg/vmprovider/providers/vsphere/contentlibrary/content_library_utils.go b/pkg/vmprovider/providers/vsphere/contentlibrary/content_library_utils.go index e9ef8e03d..84b3f1d16 100644 --- a/pkg/vmprovider/providers/vsphere/contentlibrary/content_library_utils.go +++ b/pkg/vmprovider/providers/vsphere/contentlibrary/content_library_utils.go @@ -8,8 +8,6 @@ import ( "fmt" "io" "net/url" - "regexp" - "strconv" "strings" "github.com/vmware/govmomi/ovf" @@ -24,29 +22,10 @@ import ( vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" "github.com/vmware-tanzu/vm-operator/pkg/conditions" "github.com/vmware-tanzu/vm-operator/pkg/lib" + "github.com/vmware-tanzu/vm-operator/pkg/util" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/constants" ) -var vmxRe = regexp.MustCompile(`vmx-(\d+)`) - -// ParseVirtualHardwareVersion parses the virtual hardware version -// For eg. "vmx-15" returns 15. -func ParseVirtualHardwareVersion(vmxVersion string) int32 { - // obj matches the full string and the submatch (\d+) - // and return a []string with values - obj := vmxRe.FindStringSubmatch(vmxVersion) - if len(obj) != 2 { - return 0 - } - - version, err := strconv.ParseInt(obj[1], 10, 32) - if err != nil { - return 0 - } - - return int32(version) -} - // LibItemToVirtualMachineImage converts a given library item and its attributes to return a // VirtualMachineImage that represents a k8s-native view of the item. func LibItemToVirtualMachineImage( @@ -170,7 +149,7 @@ func updateImageSpecWithOvfVirtualSystem(imageSpec *vmopv1.VirtualMachineImageSp if virtualHwSection := ovfVirtualSystem.VirtualHardware; len(virtualHwSection) > 0 { hw := virtualHwSection[0] if hw.System != nil && hw.System.VirtualSystemType != nil { - hwVersion = ParseVirtualHardwareVersion(*hw.System.VirtualSystemType) + hwVersion = util.ParseVirtualHardwareVersion(*hw.System.VirtualSystemType) } } diff --git a/pkg/vmprovider/providers/vsphere/contentlibrary/content_library_utils_test.go b/pkg/vmprovider/providers/vsphere/contentlibrary/content_library_utils_test.go index 802a70f43..33f3a95a3 100644 --- a/pkg/vmprovider/providers/vsphere/contentlibrary/content_library_utils_test.go +++ b/pkg/vmprovider/providers/vsphere/contentlibrary/content_library_utils_test.go @@ -16,30 +16,12 @@ import ( "k8s.io/utils/pointer" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" - "github.com/vmware-tanzu/vm-operator/pkg/conditions" "github.com/vmware-tanzu/vm-operator/pkg/lib" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/constants" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/contentlibrary" ) -var _ = Describe("ParseVirtualHardwareVersion", func() { - It("empty hardware string", func() { - vmxHwVersionString := "" - Expect(contentlibrary.ParseVirtualHardwareVersion(vmxHwVersionString)).To(BeZero()) - }) - - It("invalid hardware string", func() { - vmxHwVersionString := "blah" - Expect(contentlibrary.ParseVirtualHardwareVersion(vmxHwVersionString)).To(BeZero()) - }) - - It("valid hardware version string eg. vmx-15", func() { - vmxHwVersionString := "vmx-15" - Expect(contentlibrary.ParseVirtualHardwareVersion(vmxHwVersionString)).To(Equal(int32(15))) - }) -}) - var _ = Describe("LibItemToVirtualMachineImage", func() { const ( versionKey = "vmware-system-version" diff --git a/pkg/vmprovider/providers/vsphere/session/session_vm_status.go b/pkg/vmprovider/providers/vsphere/session/session_vm_status.go index 23518b513..7bcdf7b98 100644 --- a/pkg/vmprovider/providers/vsphere/session/session_vm_status.go +++ b/pkg/vmprovider/providers/vsphere/session/session_vm_status.go @@ -16,6 +16,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/lib" "github.com/vmware-tanzu/vm-operator/pkg/topology" + "github.com/vmware-tanzu/vm-operator/pkg/util" res "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/resources" ) @@ -108,6 +109,7 @@ func (s *Session) updateVMStatus( vm.Status.UniqueID = resVM.MoRef().Value vm.Status.BiosUUID = summary.Config.Uuid vm.Status.InstanceUUID = summary.Config.InstanceUuid + vm.Status.HardwareVersion = util.ParseVirtualHardwareVersion(summary.Config.HwVersion) if host := summary.Runtime.Host; host != nil { hostSystem := object.NewHostSystem(s.Client.VimClient(), *host) diff --git a/pkg/vmprovider/providers/vsphere/vmprovider_vm.go b/pkg/vmprovider/providers/vsphere/vmprovider_vm.go index a1a4f9bf3..84b743390 100644 --- a/pkg/vmprovider/providers/vsphere/vmprovider_vm.go +++ b/pkg/vmprovider/providers/vsphere/vmprovider_vm.go @@ -26,7 +26,6 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/util" vcclient "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/client" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/constants" - "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/contentlibrary" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/instancestorage" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/network" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/placement" @@ -214,7 +213,7 @@ func (vs *vSphereVMProvider) GetVirtualMachineHardwareVersion( return 0, err } - return contentlibrary.ParseVirtualHardwareVersion(o.Config.Version), nil + return util.ParseVirtualHardwareVersion(o.Config.Version), nil } func (vs *vSphereVMProvider) createVirtualMachine( @@ -675,6 +674,8 @@ func (vs *vSphereVMProvider) vmCreateGenConfigSpec( createArgs.ConfigSpec.Version = fmt.Sprintf("vmx-%d", version) } } + + util.EnsureMinHardwareVersionInConfigSpec(createArgs.ConfigSpec, vmCtx.VM.Spec.MinHardwareVersion) } func (vs *vSphereVMProvider) vmCreateValidateArgs( diff --git a/pkg/vmprovider/providers/vsphere/vmprovider_vm_test.go b/pkg/vmprovider/providers/vsphere/vmprovider_vm_test.go index 0c3d3aa2b..f71d5de7c 100644 --- a/pkg/vmprovider/providers/vsphere/vmprovider_vm_test.go +++ b/pkg/vmprovider/providers/vsphere/vmprovider_vm_test.go @@ -854,6 +854,7 @@ func vmTests() { Context("VM spec has a PVC", func() { BeforeEach(func() { + vm.Spec.MinHardwareVersion = 15 vm.Spec.Volumes = []vmopv1.VirtualMachineVolume{ { Name: "dummy-vol", @@ -876,11 +877,44 @@ func vmTests() { It("creates a VM with a hardware version minimum supported for PVCs", func() { var o mo.VirtualMachine Expect(vcVM.Properties(ctx, vcVM.Reference(), nil, &o)).To(Succeed()) + // The min version required for PVCs is honored even when the min h/w + // version is set to a different value. + Expect(o.Config.Version).NotTo(Equal("vmx-15")) Expect(o.Config.Version).To(Equal(fmt.Sprintf("vmx-%d", constants.MinSupportedHWVersionForPVC))) }) }) }) + Context("VM Class Config specifies a hardware version", func() { + BeforeEach(func() { + configSpec = &types.VirtualMachineConfigSpec{Version: "vmx-14"} + }) + + When("The minimum hardware version on the VMSpec is greater than VMClass", func() { + BeforeEach(func() { + vm.Spec.MinHardwareVersion = 15 + }) + + It("updates the VM to minimum hardware version from the Spec", func() { + var o mo.VirtualMachine + Expect(vcVM.Properties(ctx, vcVM.Reference(), nil, &o)).To(Succeed()) + Expect(o.Config.Version).To(Equal("vmx-15")) + }) + }) + + When("The minimum hardware version on the VMSpec is less than VMClass", func() { + BeforeEach(func() { + vm.Spec.MinHardwareVersion = 13 + }) + + It("uses the hardware version from the VMClass", func() { + var o mo.VirtualMachine + Expect(vcVM.Properties(ctx, vcVM.Reference(), nil, &o)).To(Succeed()) + Expect(o.Config.Version).To(Equal("vmx-14")) + }) + }) + }) + Context("VMClassAsConfig FSS is Enabled", func() { BeforeEach(func() { diff --git a/pkg/vmprovider/providers/vsphere2/vmlifecycle/update_status.go b/pkg/vmprovider/providers/vsphere2/vmlifecycle/update_status.go index 21dc3c7c4..330128201 100644 --- a/pkg/vmprovider/providers/vsphere2/vmlifecycle/update_status.go +++ b/pkg/vmprovider/providers/vsphere2/vmlifecycle/update_status.go @@ -21,6 +21,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/lib" "github.com/vmware-tanzu/vm-operator/pkg/topology" + "github.com/vmware-tanzu/vm-operator/pkg/util" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere2/virtualmachine" ) @@ -74,6 +75,7 @@ func UpdateStatus( vm.Status.BiosUUID = summary.Config.Uuid vm.Status.InstanceUUID = summary.Config.InstanceUuid vm.Status.Network = getGuestNetworkStatus(vmMO.Guest) + vm.Status.HardwareVersion = util.ParseVirtualHardwareVersion(summary.Config.HwVersion) vm.Status.Host, err = getRuntimeHostHostname(vmCtx, vcVM, summary.Runtime.Host) if err != nil { diff --git a/pkg/vmprovider/providers/vsphere2/vmlifecycle/update_status_test.go b/pkg/vmprovider/providers/vsphere2/vmlifecycle/update_status_test.go index f9050ea97..900768f97 100644 --- a/pkg/vmprovider/providers/vsphere2/vmlifecycle/update_status_test.go +++ b/pkg/vmprovider/providers/vsphere2/vmlifecycle/update_status_test.go @@ -126,6 +126,27 @@ var _ = Describe("UpdateStatus", func() { }) }) }) + + Context("Copies values to the VM status", func() { + biosUUID, instanceUUID := "f7c371d6-2003-5a48-9859-3bc9a8b0890", "6132d223-1566-5921-bc3b-df91ece09a4d" + BeforeEach(func() { + vmMO.Summary = types.VirtualMachineSummary{ + Config: types.VirtualMachineConfigSummary{ + Uuid: biosUUID, + InstanceUuid: instanceUUID, + HwVersion: "vmx-19", + }, + } + }) + + It("sets the summary config values in the status", func() { + status := vmCtx.VM.Status + Expect(status).NotTo(BeNil()) + Expect(status.BiosUUID).To(Equal(biosUUID)) + Expect(status.InstanceUUID).To(Equal(instanceUUID)) + Expect(status.HardwareVersion).To(Equal(int32(19))) + }) + }) }) var _ = Describe("VirtualMachineTools Status to VM Status Condition", func() { diff --git a/pkg/vmprovider/providers/vsphere2/vmprovider_vm.go b/pkg/vmprovider/providers/vsphere2/vmprovider_vm.go index c3046d060..41de29915 100644 --- a/pkg/vmprovider/providers/vsphere2/vmprovider_vm.go +++ b/pkg/vmprovider/providers/vsphere2/vmprovider_vm.go @@ -899,6 +899,8 @@ func (vs *vSphereVMProvider) vmCreateGenConfigSpec( } } + util.EnsureMinHardwareVersionInConfigSpec(createArgs.ConfigSpec, vmCtx.VM.Spec.MinHardwareVersion) + err := vs.vmCreateGenConfigSpecExtraConfig(vmCtx, createArgs) if err != nil { return err diff --git a/pkg/vmprovider/providers/vsphere2/vmprovider_vm_test.go b/pkg/vmprovider/providers/vsphere2/vmprovider_vm_test.go index cfb000f8d..692bd4dcd 100644 --- a/pkg/vmprovider/providers/vsphere2/vmprovider_vm_test.go +++ b/pkg/vmprovider/providers/vsphere2/vmprovider_vm_test.go @@ -852,6 +852,36 @@ func vmTests() { }) }) + Context("VM Class Config specifies a hardware version", func() { + BeforeEach(func() { + configSpec = &types.VirtualMachineConfigSpec{Version: "vmx-14"} + }) + + When("The minimum hardware version on the VMSpec is greater than VMClass", func() { + BeforeEach(func() { + vm.Spec.MinHardwareVersion = 15 + }) + + It("updates the VM to minimum hardware version from the Spec", func() { + var o mo.VirtualMachine + Expect(vcVM.Properties(ctx, vcVM.Reference(), nil, &o)).To(Succeed()) + Expect(o.Config.Version).To(Equal("vmx-15")) + }) + }) + + When("The minimum hardware version on the VMSpec is less than VMClass", func() { + BeforeEach(func() { + vm.Spec.MinHardwareVersion = 13 + }) + + It("uses the hardware version from the VMClass", func() { + var o mo.VirtualMachine + Expect(vcVM.Properties(ctx, vcVM.Reference(), nil, &o)).To(Succeed()) + Expect(o.Config.Version).To(Equal("vmx-14")) + }) + }) + }) + Context("VMClassAsConfig FSS is Enabled", func() { BeforeEach(func() { diff --git a/test/builder/util.go b/test/builder/util.go index 196a1124d..60cdcf9c3 100644 --- a/test/builder/util.go +++ b/test/builder/util.go @@ -29,6 +29,7 @@ import ( "sigs.k8s.io/yaml" imgregv1a1 "github.com/vmware-tanzu/image-registry-operator-api/api/v1alpha1" + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" topologyv1 "github.com/vmware-tanzu/vm-operator/external/tanzu-topology/api/v1alpha1" "github.com/vmware-tanzu/vm-operator/pkg/lib" @@ -247,11 +248,12 @@ func DummyVirtualMachine() *vmopv1.VirtualMachine { Annotations: map[string]string{}, }, Spec: vmopv1.VirtualMachineSpec{ - ImageName: DummyImageName, - ClassName: DummyClassName, - PowerState: vmopv1.VirtualMachinePoweredOn, - PowerOffMode: vmopv1.VirtualMachinePowerOpModeHard, - SuspendMode: vmopv1.VirtualMachinePowerOpModeHard, + ImageName: DummyImageName, + ClassName: DummyClassName, + PowerState: vmopv1.VirtualMachinePoweredOn, + PowerOffMode: vmopv1.VirtualMachinePowerOpModeHard, + SuspendMode: vmopv1.VirtualMachinePowerOpModeHard, + MinHardwareVersion: 13, NetworkInterfaces: []vmopv1.VirtualMachineNetworkInterface{ { NetworkName: DummyNetworkName, diff --git a/test/builder/utila2.go b/test/builder/utila2.go index 141d93cf5..97e9a4739 100644 --- a/test/builder/utila2.go +++ b/test/builder/utila2.go @@ -139,11 +139,12 @@ func DummyVirtualMachineA2() *vmopv1.VirtualMachine { Annotations: map[string]string{}, }, Spec: vmopv1.VirtualMachineSpec{ - ImageName: DummyImageName, - ClassName: DummyClassName, - PowerState: vmopv1.VirtualMachinePowerStateOn, - PowerOffMode: vmopv1.VirtualMachinePowerOpModeHard, - SuspendMode: vmopv1.VirtualMachinePowerOpModeHard, + ImageName: DummyImageName, + ClassName: DummyClassName, + PowerState: vmopv1.VirtualMachinePowerStateOn, + PowerOffMode: vmopv1.VirtualMachinePowerOpModeHard, + SuspendMode: vmopv1.VirtualMachinePowerOpModeHard, + MinHardwareVersion: 13, Volumes: []vmopv1.VirtualMachineVolume{ { Name: DummyVolumeName, diff --git a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go index 775dc01d6..b179281bc 100644 --- a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go +++ b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go @@ -146,6 +146,7 @@ func (v validator) ValidateDelete(*context.WebhookRequestContext) admission.Resp // - ClassName // - StorageClass // - ResourcePolicyName +// - Minimum VM Hardware Version // Following fields can only be updated when the VM is powered off. // - Ports @@ -715,6 +716,7 @@ func (v validator) validateImmutableFields(ctx *context.WebhookRequestContext, v allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ClassName, oldVM.Spec.ClassName, specPath.Child("className"))...) allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.StorageClass, oldVM.Spec.StorageClass, specPath.Child("storageClass"))...) allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ResourcePolicyName, oldVM.Spec.ResourcePolicyName, specPath.Child("resourcePolicyName"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.MinHardwareVersion, oldVM.Spec.MinHardwareVersion, specPath.Child("minHardwareVersion"))...) return allErrs } diff --git a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_intg_test.go b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_intg_test.go index 9c205cecf..d3c93e1c2 100644 --- a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_intg_test.go +++ b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_intg_test.go @@ -163,6 +163,18 @@ func intgTestsValidateUpdate() { }) }) + When("update is performed with changed minimum hardware version", func() { + BeforeEach(func() { + ctx.vm.Spec.MinHardwareVersion += 2 + }) + It("should deny the request", func() { + Expect(err).To(HaveOccurred()) + expectedPath := field.NewPath("spec", "minHardwareVersion") + Expect(err.Error()).To(ContainSubstring(expectedPath.String())) + Expect(err.Error()).To(ContainSubstring(immutableFieldMsg)) + }) + }) + Context("VirtualMachine update while VM is powered on", func() { BeforeEach(func() { ctx.vm.Spec.PowerState = "poweredOn" diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go index 59b3a9925..e848b14bc 100644 --- a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go @@ -135,6 +135,7 @@ func (v validator) ValidateDelete(*context.WebhookRequestContext) admission.Resp // - ClassName // - StorageClass // - ResourcePolicyName +// - Minimum VM Hardware Version // // Following fields can only be changed when the VM is powered off. // - Bootstrap @@ -738,13 +739,14 @@ func (v validator) validateUpdatesWhenPoweredOn(ctx *context.WebhookRequestConte return allErrs } -func (v validator) validateImmutableFields(ctx *context.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList { +func (v validator) validateImmutableFields(_ *context.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList { var allErrs field.ErrorList specPath := field.NewPath("spec") allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ImageName, oldVM.Spec.ImageName, specPath.Child("imageName"))...) allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ClassName, oldVM.Spec.ClassName, specPath.Child("className"))...) allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.StorageClass, oldVM.Spec.StorageClass, specPath.Child("storageClass"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.MinHardwareVersion, oldVM.Spec.MinHardwareVersion, specPath.Child("minHardwareVersion"))...) // TODO: More checks. // TODO: Allow privilege? diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_intg_test.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_intg_test.go index e241ec925..08898c837 100644 --- a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_intg_test.go +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_intg_test.go @@ -128,6 +128,18 @@ func intgTestsValidateUpdate() { }) }) + When("update is performed with changed minimum hardware version", func() { + BeforeEach(func() { + ctx.vm.Spec.MinHardwareVersion += 2 + }) + It("should deny the request", func() { + Expect(err).To(HaveOccurred()) + expectedPath := field.NewPath("spec", "minHardwareVersion") + Expect(err.Error()).To(ContainSubstring(expectedPath.String())) + Expect(err.Error()).To(ContainSubstring(immutableFieldMsg)) + }) + }) + Context("VirtualMachine update while VM is powered on", func() { BeforeEach(func() { ctx.vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOn