diff --git a/controllers/contentlibrary/v1alpha1/utils/utils.go b/controllers/contentlibrary/v1alpha1/utils/utils.go index f647c5fda..f446b6663 100644 --- a/controllers/contentlibrary/v1alpha1/utils/utils.go +++ b/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-" -// by using the same identifier from the given Item name in "clitem-". +// GetImageFieldNameFromItem returns the image field name in the format of +// "vmi-" by using the identifier from the given item name "clitem-". 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,68 @@ 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 namespace scope image exists by the status name. +// If neither is found, it checks a cluster scope image with the same order. +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 namespace scope image exists by the image status name. + vmiList := vmopv1.VirtualMachineImageList{} + err = ctrlClient.List(ctx, &vmiList, client.InNamespace(namespace)) + if err == nil { + for _, vmi := range vmiList.Items { + if vmi.Status.ImageName == imageName { + return &vmi.Spec, &vmi.Status, nil + } } } + if err != nil && !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 + } + + // Check if a cluster scope image exists by the image status name. + cvmiList := vmopv1.ClusterVirtualMachineImageList{} + err = ctrlClient.List(ctx, &cvmiList) + if err == nil { + for _, cvmi := range cvmiList.Items { + if cvmi.Status.ImageName == imageName { + return &cvmi.Spec, &cvmi.Status, nil + } + } + } + if err != nil && !apierrors.IsNotFound(err) { + return nil, nil, err + } - return spec, status, err + err = fmt.Errorf("no Virtual Machine image can be found for %q", imageName) + return nil, nil, err } diff --git a/pkg/lib/env.go b/pkg/lib/env.go index a1e6594f6..2435fb281 100644 --- a/pkg/lib/env.go +++ b/pkg/lib/env.go @@ -15,6 +15,7 @@ import ( const ( trueString = "true" TrueString = "true" + FalseString = "false" VmopNamespaceEnv = "POD_NAMESPACE" WcpFaultDomainsFSS = "FSS_WCP_FAULTDOMAINS" VMServiceV1Alpha2FSS = "FSS_WCP_VMSERVICE_V1ALPHA2" diff --git a/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go b/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go index 638bd10d5..523d8496c 100644 --- a/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go +++ b/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go @@ -6,6 +6,7 @@ package vsphere_test import ( goctx "context" "fmt" + "os" "sync/atomic" . "github.com/onsi/ginkgo" @@ -57,7 +58,6 @@ func vmUtilTests() { }) Context("GetVirtualMachineClass", func() { - oldNamespacedVMClassFSSEnabledFunc := lib.IsNamespacedVMClassFSSEnabled When("WCP_Namespaced_VM_Class FSS is disabled", func() { var ( @@ -69,13 +69,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() { @@ -166,13 +164,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() { @@ -234,9 +230,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() { @@ -372,13 +370,19 @@ 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()) @@ -482,7 +486,7 @@ func vmUtilTests() { }) }) - When("Namespace scoped VirtualMachineImage exists and ready", func() { + When("Namespace scoped VirtualMachineImage is ready and VM uses the image CR name to deploy", func() { BeforeEach(func() { conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) @@ -491,15 +495,38 @@ 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("Namespace scoped VirtualMachineImage is ready and VM uses the image Status name to deploy", func() { + BeforeEach(func() { + conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) + conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) + conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageSyncedCondition) + initObjects = append(initObjects, cl, nsVMImage) + vmCtx.VM.Spec.ImageName = nsVMImage.Status.ImageName + }) + + 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 uses the image CR name to deploy", func() { BeforeEach(func() { conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) @@ -508,10 +535,33 @@ 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))) + }) + }) + + When("ClusterVirtualMachineImage is ready and VM uses the image Status name to deploy", func() { + BeforeEach(func() { + conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) + conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) + conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageSyncedCondition) + initObjects = append(initObjects, clusterCL, clusterVMImage) + vmCtx.VM.Spec.ImageName = clusterVMImage.Status.ImageName + }) + + It("returns expected cluster image spec, status, and 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))) }) }) @@ -668,8 +718,7 @@ func vmUtilTests() { Context("AddInstanceStorageVolumes", func() { var ( - vmClass *vmopv1.VirtualMachineClass - instanceStorageFSS uint32 + vmClass *vmopv1.VirtualMachineClass ) expectInstanceStorageVolumes := func( @@ -697,28 +746,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() { diff --git a/test/builder/util.go b/test/builder/util.go index b1ac9ece5..73ee0e179 100644 --- a/test/builder/util.go +++ b/test/builder/util.go @@ -367,7 +367,7 @@ func DummyVirtualMachineImage(imageName string) *vmopv1.VirtualMachineImage { }, }, Status: vmopv1.VirtualMachineImageStatus{ - ImageName: imageName, + ImageName: imageName + "-status-name", }, } } @@ -386,7 +386,7 @@ func DummyClusterVirtualMachineImage(imageName string) *vmopv1.ClusterVirtualMac }, }, Status: vmopv1.VirtualMachineImageStatus{ - ImageName: imageName, + ImageName: imageName + "-status-name", }, } } diff --git a/test/builder/utila2.go b/test/builder/utila2.go index a36c8bc11..1a93e4b1e 100644 --- a/test/builder/utila2.go +++ b/test/builder/utila2.go @@ -179,7 +179,7 @@ func DummyVirtualMachineImageA2(imageName string) *vmopv1.VirtualMachineImage { Name: imageName, }, Status: vmopv1.VirtualMachineImageStatus{ - Name: imageName, + Name: imageName + "-status-name", ProductInfo: vmopv1.VirtualMachineImageProductInfo{ FullVersion: DummyDistroVersion, },