Support VM deployment from image status (friendly) name (vmware-tanzu…

…#223)

This PR updates the VirtualMachine mutation webhook to replace the `vm.spec.imageName` if it's a friendly image name that matches a single namespace or cluster scope image. If multiple images are found, the mutator denies the VM creation request. The mutator also indexes the VirtualMachineImage and ClusterVirtualMachineImage objects by their `.status.imageName` fields to enable efficient querying.

config/rbac/role.yaml

@@ -217,8 +217,10 @@ rules:
   - clustervirtualmachineimages/status
   verbs:
   - get
+  - list
   - patch
   - update
+  - watch
 - apiGroups:
   - vmoperator.vmware.com
   resources:
@@ -326,8 +328,10 @@ rules:
   - virtualmachineimages/status
   verbs:
   - get
+  - list
   - patch
   - update
+  - watch
 - apiGroups:
   - vmoperator.vmware.com
   resources:

controllers/contentlibrary/v1alpha1/utils/utils.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2022 VMware, Inc. All Rights Reserved.
+// Copyright (c) 2022-2023 VMware, Inc. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package utils
@@ -17,22 +17,24 @@ import (
 	vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1"
 )
 
-// GetImageFieldNameFromItem returns the Image field name in format of "vmi-<uuid>"
-// by using the same identifier from the given Item name in "clitem-<uuid>".
+// GetImageFieldNameFromItem returns the image field name in the format of
+// "vmi-<uuid>" by using the identifier from the given item name "clitem-<uuid>".
 func GetImageFieldNameFromItem(itemName string) (string, error) {
 	if !strings.HasPrefix(itemName, ItemFieldNamePrefix) {
-		return "", fmt.Errorf("item name doesn't start with %q", ItemFieldNamePrefix)
+		return "", fmt.Errorf("item name %q doesn't start with %q",
+			itemName, ItemFieldNamePrefix)
 	}
+
 	itemNameSplit := strings.Split(itemName, "-")
 	if len(itemNameSplit) < 2 {
-		return "", fmt.Errorf("item name doesn't have an identifier after %s-", ItemFieldNamePrefix)
+		return "", fmt.Errorf("item name %q doesn't contain a uuid", itemName)
 	}
 
 	uuid := strings.Join(itemNameSplit[1:], "-")
 	return fmt.Sprintf("%s-%s", ImageFieldNamePrefix, uuid), nil
 }
 
-// IsItemReady returns if the given item conditions contain a Ready condition with status True.
+// IsItemReady checks if an item is ready by iterating through its conditions.
 func IsItemReady(itemConditions imgregv1a1.Conditions) bool {
 	var isReady bool
 	for _, condition := range itemConditions {
@@ -45,23 +47,42 @@ func IsItemReady(itemConditions imgregv1a1.Conditions) bool {
 	return isReady
 }
 
-// GetVMImageSpecStatus returns the VirtualMachineImage Spec and Status fields in an image-registry service enabled cluster.
-// It first tries to get the namespace scope VM Image; if not found, it looks up the cluster scope VM Image.
-func GetVMImageSpecStatus(ctx context.Context, ctrlClient client.Client, imageName, namespace string) (
-	spec *vmopv1.VirtualMachineImageSpec, status *vmopv1.VirtualMachineImageStatus, err error) {
-
+// GetVMImageSpecStatus retrieves the VirtualMachineImageSpec and
+// VirtualMachineImageStatus for a given image name and namespace.
+// It first checks if a namespace scope image exists by the resource name,
+// and if not, checks if a cluster scope image exists by the resource name.
+func GetVMImageSpecStatus(
+	ctx context.Context,
+	ctrlClient client.Client,
+	imageName, namespace string) (
+	*vmopv1.VirtualMachineImageSpec,
+	*vmopv1.VirtualMachineImageStatus,
+	error) {
+	// Check if a namespace scope image exists by the resource name.
 	vmi := vmopv1.VirtualMachineImage{}
-	if err = ctrlClient.Get(ctx, client.ObjectKey{Name: imageName, Namespace: namespace}, &vmi); err == nil {
-		spec = &vmi.Spec
-		status = &vmi.Status
-	} else if apierrors.IsNotFound(err) {
-		// Namespace scope image is not found. Check if a cluster scope image with this name exists.
-		cvmi := vmopv1.ClusterVirtualMachineImage{}
-		if err = ctrlClient.Get(ctx, client.ObjectKey{Name: imageName}, &cvmi); err == nil {
-			spec = &cvmi.Spec
-			status = &cvmi.Status
-		}
+	key := client.ObjectKey{Name: imageName, Namespace: namespace}
+	err := ctrlClient.Get(ctx, key, &vmi)
+	if err == nil {
+		return &vmi.Spec, &vmi.Status, nil
+	}
+	if !apierrors.IsNotFound(err) {
+		return nil, nil, err
+	}
+
+	// Check if a cluster scope image exists by the resource name.
+	cvmi := vmopv1.ClusterVirtualMachineImage{}
+	key = client.ObjectKey{Name: imageName}
+	err = ctrlClient.Get(ctx, key, &cvmi)
+	if err == nil {
+		return &cvmi.Spec, &cvmi.Status, nil
+	}
+	if !apierrors.IsNotFound(err) {
+		return nil, nil, err
 	}
 
-	return spec, status, err
+	err = fmt.Errorf(
+		"cannot find a namespace or cluster scope VM image for resource name %q",
+		imageName,
+	)
+	return nil, nil, err
 }

pkg/lib/env.go

@@ -14,6 +14,7 @@ import (
 
 const (
 	TrueString                    = "true"
+	FalseString                   = "false"
 	VmopNamespaceEnv              = "POD_NAMESPACE"
 	WcpFaultDomainsFSS            = "FSS_WCP_FAULTDOMAINS"
 	VMServiceV1Alpha2FSS          = "FSS_WCP_VMSERVICE_V1ALPHA2"

pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go

@@ -349,14 +349,13 @@ func resolveVMImage(
 
 	imageSpec, imageStatus, err := clutils.GetVMImageSpecStatus(vmCtx, k8sClient, imageName, vmCtx.VM.Namespace)
 	if err != nil {
-		imageNotFoundMsg := fmt.Sprintf("Failed to get the VM's image: %s", imageName)
 		conditions.MarkFalse(vmCtx.VM,
 			vmopv1.VirtualMachinePrereqReadyCondition,
 			vmopv1.VirtualMachineImageNotFoundReason,
 			vmopv1.ConditionSeverityError,
-			imageNotFoundMsg,
+			fmt.Sprintf("Failed to get the VM's image: %s", imageName),
 		)
-		return nil, nil, errors.Wrap(err, imageNotFoundMsg)
+		return nil, nil, err
 	}
 
 	// Do not return the VM image status if the current image condition is not satisfied.

pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go

@@ -6,7 +6,7 @@ package vsphere_test
 import (
 	goctx "context"
 	"fmt"
-	"sync/atomic"
+	"os"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -56,7 +56,6 @@ func vmUtilTests() {
 	})
 
 	Context("GetVirtualMachineClass", func() {
-		oldNamespacedVMClassFSSEnabledFunc := lib.IsNamespacedVMClassFSSEnabled
 
 		When("WCP_Namespaced_VM_Class FSS is disabled", func() {
 			var (
@@ -68,13 +67,11 @@ func vmUtilTests() {
 				vmClass, vmClassBinding = builder.DummyVirtualMachineClassAndBinding("dummy-vm-class", vmCtx.VM.Namespace)
 				vmCtx.VM.Spec.ClassName = vmClass.Name
 
-				lib.IsNamespacedVMClassFSSEnabled = func() bool {
-					return false
-				}
+				Expect(os.Setenv(lib.NamespacedVMClassFSS, lib.FalseString)).To(Succeed())
 			})
 
 			AfterEach(func() {
-				lib.IsNamespacedVMClassFSSEnabled = oldNamespacedVMClassFSSEnabledFunc
+				Expect(os.Unsetenv(lib.NamespacedVMClassFSS)).To(Succeed())
 			})
 
 			Context("VirtualMachineClass custom resource doesn't exist", func() {
@@ -165,13 +162,11 @@ func vmUtilTests() {
 				vmClass.Namespace = vmCtx.VM.Namespace
 				vmCtx.VM.Spec.ClassName = vmClass.Name
 
-				lib.IsNamespacedVMClassFSSEnabled = func() bool {
-					return true
-				}
+				Expect(os.Setenv(lib.NamespacedVMClassFSS, lib.TrueString)).To(Succeed())
 			})
 
 			AfterEach(func() {
-				lib.IsNamespacedVMClassFSSEnabled = oldNamespacedVMClassFSSEnabledFunc
+				Expect(os.Unsetenv(lib.NamespacedVMClassFSS)).To(Succeed())
 			})
 
 			Context("VirtualMachineClass custom resource doesn't exist", func() {
@@ -233,9 +228,11 @@ func vmUtilTests() {
 
 				vmCtx.VM.Spec.ImageName = vmImage.Name
 
-				lib.IsWCPVMImageRegistryEnabled = func() bool {
-					return false
-				}
+				Expect(os.Setenv(lib.VMImageRegistryFSS, lib.FalseString)).To(Succeed())
+			})
+
+			AfterEach(func() {
+				Expect(os.Unsetenv(lib.VMImageRegistryFSS)).To(Succeed())
 			})
 
 			When("VirtualMachineImage does not exist", func() {
@@ -348,7 +345,6 @@ func vmUtilTests() {
 		})
 
 		When("WCPVMImageRegistry FSS is enabled", func() {
-
 			var (
 				cl             *imgregv1a1.ContentLibrary
 				nsVMImage      *vmopv1.VirtualMachineImage
@@ -371,25 +367,31 @@ func vmUtilTests() {
 					Name: clusterCL.Name,
 				}
 
-				lib.IsWCPVMImageRegistryEnabled = func() bool {
-					return true
-				}
+				Expect(os.Setenv(lib.VMImageRegistryFSS, lib.TrueString)).To(Succeed())
+			})
+
+			AfterEach(func() {
+				Expect(os.Unsetenv(lib.VMImageRegistryFSS)).To(Succeed())
 			})
 
 			When("Neither cluster or namespace scoped VM image exists", func() {
 
+				BeforeEach(func() {
+					vmCtx.VM.Spec.ImageName = "non-existing-image"
+				})
+
 				It("returns error and sets condition", func() {
 					_, _, _, err := vsphere.GetVMImageAndContentLibraryUUID(vmCtx, k8sClient)
 					Expect(err).To(HaveOccurred())
-					expectedErrMsg := fmt.Sprintf("Failed to get the VM's image: %s", vmCtx.VM.Spec.ImageName)
+					expectedErrMsg := fmt.Sprintf("cannot find a namespace or cluster scope VM image for resource name %q", vmCtx.VM.Spec.ImageName)
 					Expect(err.Error()).To(ContainSubstring(expectedErrMsg))
 
 					expectedCondition := vmopv1.Conditions{
 						*conditions.FalseCondition(
 							vmopv1.VirtualMachinePrereqReadyCondition,
 							vmopv1.VirtualMachineImageNotFoundReason,
 							vmopv1.ConditionSeverityError,
-							expectedErrMsg),
+							fmt.Sprintf("Failed to get the VM's image: %s", vmCtx.VM.Spec.ImageName)),
 					}
 					Expect(vmCtx.VM.Status.Conditions).To(conditions.MatchConditions(expectedCondition))
 				})
@@ -481,7 +483,7 @@ func vmUtilTests() {
 				})
 			})
 
-			When("Namespace scoped VirtualMachineImage exists and ready", func() {
+			When("Namespace scoped VirtualMachineImage is ready and VM specifies an image CR name", func() {
 				BeforeEach(func() {
 					conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderReadyCondition)
 					conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition)
@@ -490,15 +492,18 @@ func vmUtilTests() {
 					vmCtx.VM.Spec.ImageName = nsVMImage.Name
 				})
 
-				It("returns success", func() {
-					_, imageStatus, uuid, err := vsphere.GetVMImageAndContentLibraryUUID(vmCtx, k8sClient)
+				It("returns expected namespace image spec, status, and CL uuid", func() {
+					imageSpec, imageStatus, uuid, err := vsphere.GetVMImageAndContentLibraryUUID(vmCtx, k8sClient)
 					Expect(err).ToNot(HaveOccurred())
+					Expect(imageSpec).ToNot(BeNil())
+					Expect(imageSpec.ImageID).To(Equal(nsVMImage.Spec.ImageID))
 					Expect(imageStatus).ToNot(BeNil())
+					Expect(imageStatus.ImageName).To(Equal(nsVMImage.Status.ImageName))
 					Expect(uuid).To(Equal(string(cl.Spec.UUID)))
 				})
 			})
 
-			When("ClusterVirtualMachineImage exists and ready", func() {
+			When("ClusterVirtualMachineImage is ready and VM specifies an image CR name", func() {
 				BeforeEach(func() {
 					conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderReadyCondition)
 					conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition)
@@ -507,10 +512,13 @@ func vmUtilTests() {
 					vmCtx.VM.Spec.ImageName = clusterVMImage.Name
 				})
 
-				It("returns success", func() {
-					_, imageStatus, uuid, err := vsphere.GetVMImageAndContentLibraryUUID(vmCtx, k8sClient)
+				It("returns expected cluster image spec, status, and CL uuid", func() {
+					imageSpec, imageStatus, uuid, err := vsphere.GetVMImageAndContentLibraryUUID(vmCtx, k8sClient)
 					Expect(err).ToNot(HaveOccurred())
+					Expect(imageSpec).ToNot(BeNil())
+					Expect(imageSpec.ImageID).To(Equal(clusterVMImage.Spec.ImageID))
 					Expect(imageStatus).ToNot(BeNil())
+					Expect(imageStatus.ImageName).To(Equal(clusterVMImage.Status.ImageName))
 					Expect(uuid).To(Equal(string(clusterCL.Spec.UUID)))
 				})
 			})
@@ -684,8 +692,7 @@ func vmUtilTests() {
 	Context("AddInstanceStorageVolumes", func() {
 
 		var (
-			vmClass            *vmopv1.VirtualMachineClass
-			instanceStorageFSS uint32
+			vmClass *vmopv1.VirtualMachineClass
 		)
 
 		expectInstanceStorageVolumes := func(
@@ -713,28 +720,29 @@ func vmUtilTests() {
 		}
 
 		BeforeEach(func() {
-			lib.IsInstanceStorageFSSEnabled = func() bool {
-				return atomic.LoadUint32(&instanceStorageFSS) != 0
-			}
-
 			vmClass = builder.DummyVirtualMachineClass()
 		})
 
 		AfterEach(func() {
-			atomic.StoreUint32(&instanceStorageFSS, 0)
+			Expect(os.Unsetenv(lib.InstanceStorageFSS)).To(Succeed())
 		})
 
-		It("Instance Storage FSS is disabled", func() {
-			atomic.StoreUint32(&instanceStorageFSS, 0)
+		When("Instance Storage FSS is disabled", func() {
 
-			err := vsphere.AddInstanceStorageVolumes(vmCtx, vmClass)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(instancestorage.FilterVolumes(vmCtx.VM)).To(BeEmpty())
+			BeforeEach(func() {
+				Expect(os.Setenv(lib.InstanceStorageFSS, lib.FalseString)).To(Succeed())
+			})
+
+			It("VM Class does not contain instance storage volumes", func() {
+				err := vsphere.AddInstanceStorageVolumes(vmCtx, vmClass)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(instancestorage.FilterVolumes(vmCtx.VM)).To(BeEmpty())
+			})
 		})
 
 		When("InstanceStorage FFS is enabled", func() {
 			BeforeEach(func() {
-				atomic.StoreUint32(&instanceStorageFSS, 1)
+				Expect(os.Setenv(lib.InstanceStorageFSS, lib.TrueString)).To(Succeed())
 			})
 
 			It("VM Class does not contain instance storage volumes", func() {