diff --git a/controllers/contentlibrary/v1alpha1/utils/utils.go b/controllers/contentlibrary/v1alpha1/utils/utils.go index f446b6663..d2024935f 100644 --- a/controllers/contentlibrary/v1alpha1/utils/utils.go +++ b/controllers/contentlibrary/v1alpha1/utils/utils.go @@ -50,8 +50,9 @@ func IsItemReady(itemConditions imgregv1a1.Conditions) bool { // 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. +// and if not, checks if a cluster scope image exists by the resource name. +// If neither is found, it checks if *one and only one* namespace or cluster +// scope image exists by the image status (friendly) name. func GetVMImageSpecStatus( ctx context.Context, ctrlClient client.Client, @@ -70,45 +71,68 @@ func GetVMImageSpecStatus( return nil, nil, err } - // Check if a namespace scope image exists by the image status name. + // 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 single namespace scope image exists by the image status name. vmiList := vmopv1.VirtualMachineImageList{} err = ctrlClient.List(ctx, &vmiList, client.InNamespace(namespace)) if err == nil { + resImg := vmopv1.VirtualMachineImage{} + count := 0 for _, vmi := range vmiList.Items { if vmi.Status.ImageName == imageName { - return &vmi.Spec, &vmi.Status, nil + resImg = vmi + count++ + } + // Break early if there are more than one image with the same name. + if count > 1 { + break } } + if count == 1 { + return &resImg.Spec, &resImg.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. + // Check if a single cluster scope image exists by the image status name. cvmiList := vmopv1.ClusterVirtualMachineImageList{} err = ctrlClient.List(ctx, &cvmiList) if err == nil { + resImg := vmopv1.ClusterVirtualMachineImage{} + count := 0 for _, cvmi := range cvmiList.Items { if cvmi.Status.ImageName == imageName { - return &cvmi.Spec, &cvmi.Status, nil + resImg = cvmi + count++ + } + // Break early if there are more than one image with the same name. + if count > 1 { + break } } + if count == 1 { + return &resImg.Spec, &resImg.Status, nil + } } if err != nil && !apierrors.IsNotFound(err) { return nil, nil, err } - err = fmt.Errorf("no Virtual Machine image can be found for %q", imageName) + err = fmt.Errorf( + "no single VM image found with either resource or status name for %q", + imageName, + ) return nil, nil, err } diff --git a/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go b/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go index f1a3bf4e8..86114bb32 100644 --- a/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go +++ b/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go @@ -357,7 +357,7 @@ func resolveVMImage( vmopv1.ConditionSeverityError, imageNotFoundMsg, ) - 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. diff --git a/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go b/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go index 4d5669fea..529d88997 100644 --- a/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go +++ b/pkg/vmprovider/providers/vsphere/vmprovider_vm_utils_test.go @@ -385,7 +385,7 @@ func vmUtilTests() { 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("no single VM image found with either resource or status name for %q", vmCtx.VM.Spec.ImageName) Expect(err.Error()).To(ContainSubstring(expectedErrMsg)) expectedCondition := vmopv1.Conditions{ @@ -393,7 +393,7 @@ func vmUtilTests() { 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)) }) @@ -485,7 +485,7 @@ func vmUtilTests() { }) }) - When("Namespace scoped VirtualMachineImage is ready and VM uses the image CR name to deploy", 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) @@ -505,11 +505,12 @@ func vmUtilTests() { }) }) - When("Namespace scoped VirtualMachineImage is ready and VM uses the image Status name to deploy", func() { + When("Namespace scoped VirtualMachineImage is ready and VM specifies a unique image Status name", func() { BeforeEach(func() { conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageSyncedCondition) + nsVMImage.Status.ImageName = "ns-vm-image-status-name" initObjects = append(initObjects, cl, nsVMImage) vmCtx.VM.Spec.ImageName = nsVMImage.Status.ImageName }) @@ -525,7 +526,37 @@ func vmUtilTests() { }) }) - When("ClusterVirtualMachineImage is ready and VM uses the image CR name to deploy", func() { + When("Namespace scoped VirtualMachineImage is ready and VM specifies a duplicate image Status name", func() { + BeforeEach(func() { + conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) + conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) + conditions.MarkTrue(nsVMImage, vmopv1.VirtualMachineImageSyncedCondition) + nsVMImage.Status.ImageName = "ns-vm-image-status-name" + nsVMImageCopy := nsVMImage.DeepCopy() + nsVMImageCopy.Name = "ns-vm-image-copy" + nsVMImageCopy.Status.ImageName = "ns-vm-image-status-name" + initObjects = append(initObjects, cl, nsVMImage, nsVMImageCopy) + vmCtx.VM.Spec.ImageName = nsVMImage.Status.ImageName + }) + + It("returns error and sets VM condition", func() { + _, _, _, err := vsphere.GetVMImageAndContentLibraryUUID(vmCtx, k8sClient) + Expect(err).To(HaveOccurred()) + expectedErrMsg := fmt.Sprintf("no single VM image found with either resource or status name for %q", vmCtx.VM.Spec.ImageName) + Expect(err.Error()).To(ContainSubstring(expectedErrMsg)) + + expectedCondition := vmopv1.Conditions{ + *conditions.FalseCondition( + vmopv1.VirtualMachinePrereqReadyCondition, + vmopv1.VirtualMachineImageNotFoundReason, + vmopv1.ConditionSeverityError, + fmt.Sprintf("Failed to get the VM's image: %s", vmCtx.VM.Spec.ImageName)), + } + Expect(vmCtx.VM.Status.Conditions).To(conditions.MatchConditions(expectedCondition)) + }) + }) + + When("ClusterVirtualMachineImage is ready and VM specifies an image CR name", func() { BeforeEach(func() { conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) @@ -545,11 +576,12 @@ func vmUtilTests() { }) }) - When("ClusterVirtualMachineImage is ready and VM uses the image Status name to deploy", func() { + When("ClusterVirtualMachineImage is ready and VM specifies a unique image Status name", func() { BeforeEach(func() { conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageSyncedCondition) + clusterVMImage.Status.ImageName = "cluster-vm-image-status-name" initObjects = append(initObjects, clusterCL, clusterVMImage) vmCtx.VM.Spec.ImageName = clusterVMImage.Status.ImageName }) @@ -564,6 +596,36 @@ func vmUtilTests() { Expect(uuid).To(Equal(string(clusterCL.Spec.UUID))) }) }) + + When("ClusterVirtualMachineImage is ready and VM specifies a duplicate image Status name", func() { + BeforeEach(func() { + conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderReadyCondition) + conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageProviderSecurityComplianceCondition) + conditions.MarkTrue(clusterVMImage, vmopv1.VirtualMachineImageSyncedCondition) + clusterVMImage.Status.ImageName = "cluster-vm-image-status-name" + clusterVMImageCopy := clusterVMImage.DeepCopy() + clusterVMImageCopy.Name = "cluster-vm-image-copy" + clusterVMImageCopy.Status.ImageName = "cluster-vm-image-status-name" + initObjects = append(initObjects, clusterCL, clusterVMImage, clusterVMImageCopy) + vmCtx.VM.Spec.ImageName = clusterVMImage.Status.ImageName + }) + + It("returns error and sets VM condition", func() { + _, _, _, err := vsphere.GetVMImageAndContentLibraryUUID(vmCtx, k8sClient) + Expect(err).To(HaveOccurred()) + expectedErrMsg := fmt.Sprintf("no single VM image found with either resource or status name for %q", vmCtx.VM.Spec.ImageName) + Expect(err.Error()).To(ContainSubstring(expectedErrMsg)) + + expectedCondition := vmopv1.Conditions{ + *conditions.FalseCondition( + vmopv1.VirtualMachinePrereqReadyCondition, + vmopv1.VirtualMachineImageNotFoundReason, + vmopv1.ConditionSeverityError, + fmt.Sprintf("Failed to get the VM's image: %s", vmCtx.VM.Spec.ImageName)), + } + Expect(vmCtx.VM.Status.Conditions).To(conditions.MatchConditions(expectedCondition)) + }) + }) }) })