diff --git a/pkg/providers/vsphere/vmprovider_resourcepolicy.go b/pkg/providers/vsphere/vmprovider_resourcepolicy.go index 963a4fbdb..e98097d80 100644 --- a/pkg/providers/vsphere/vmprovider_resourcepolicy.go +++ b/pkg/providers/vsphere/vmprovider_resourcepolicy.go @@ -32,14 +32,20 @@ func (vs *vSphereVMProvider) IsVirtualMachineSetResourcePolicyReady( return false, err } - folderExists, err := vcenter.DoesChildFolderExist(ctx, client.VimClient(), folderMoID, resourcePolicy.Spec.Folder) - if err != nil { - return false, err + folderExists := true + if folderName := resourcePolicy.Spec.Folder; folderName != "" { + folderExists, err = vcenter.DoesChildFolderExist(ctx, client.VimClient(), folderMoID, folderName) + if err != nil { + return false, err + } } - rpExists, err := vcenter.DoesChildResourcePoolExist(ctx, client.VimClient(), rpMoID, resourcePolicy.Spec.ResourcePool.Name) - if err != nil { - return false, err + rpExists := true + if rpName := resourcePolicy.Spec.ResourcePool.Name; rpName != "" { + rpExists, err = vcenter.DoesChildResourcePoolExist(ctx, client.VimClient(), rpMoID, rpName) + if err != nil { + return false, err + } } clusterRef, err := vcenter.GetResourcePoolOwnerMoRef(ctx, client.VimClient(), rpMoID) @@ -67,7 +73,7 @@ func (vs *vSphereVMProvider) CreateOrUpdateVirtualMachineSetResourcePolicy( ctx context.Context, resourcePolicy *vmopv1.VirtualMachineSetResourcePolicy) error { - folderMoID, rpMoIDs, err := vs.getNamespaceFolderAndRPMoIDs(ctx, resourcePolicy.Namespace) + folderMoID, rpMoIDs, err := topology.GetNamespaceFolderAndRPMoIDs(ctx, vs.k8sClient, resourcePolicy.Namespace) if err != nil { return err } @@ -80,24 +86,34 @@ func (vs *vSphereVMProvider) CreateOrUpdateVirtualMachineSetResourcePolicy( vimClient := client.VimClient() var errs []error - _, err = vcenter.CreateFolder(ctx, vimClient, folderMoID, resourcePolicy.Spec.Folder) - if err != nil { - errs = append(errs, err) + if folderName := resourcePolicy.Spec.Folder; folderName != "" { + if folderMoID != "" { + _, err = vcenter.CreateFolder(ctx, vimClient, folderMoID, folderName) + if err != nil { + errs = append(errs, err) + } + } else { + errs = append(errs, fmt.Errorf("cannot create child folder because Namespace folder not found")) + } } for _, rpMoID := range rpMoIDs { - _, err := vcenter.CreateOrUpdateChildResourcePool(ctx, vimClient, rpMoID, &resourcePolicy.Spec.ResourcePool) - if err != nil { - errs = append(errs, err) + if rpSpec := &resourcePolicy.Spec.ResourcePool; rpSpec.Name != "" { + _, err := vcenter.CreateOrUpdateChildResourcePool(ctx, vimClient, rpMoID, rpSpec) + if err != nil { + errs = append(errs, err) + } } - clusterRef, err := vcenter.GetResourcePoolOwnerMoRef(ctx, vimClient, rpMoID) - if err == nil { - clusterModuleProvider := clustermodules.NewProvider(client.RestClient()) - err = vs.createClusterModules(ctx, clusterModuleProvider, clusterRef.Reference(), resourcePolicy) - } - if err != nil { - errs = append(errs, err) + if len(resourcePolicy.Spec.ClusterModuleGroups) > 0 { + clusterRef, err := vcenter.GetResourcePoolOwnerMoRef(ctx, vimClient, rpMoID) + if err == nil { + clusterModuleProvider := clustermodules.NewProvider(client.RestClient()) + err = vs.createClusterModules(ctx, clusterModuleProvider, clusterRef.Reference(), resourcePolicy) + } + if err != nil { + errs = append(errs, err) + } } } @@ -109,7 +125,7 @@ func (vs *vSphereVMProvider) DeleteVirtualMachineSetResourcePolicy( ctx context.Context, resourcePolicy *vmopv1.VirtualMachineSetResourcePolicy) error { - folderMoID, rpMoIDs, err := vs.getNamespaceFolderAndRPMoIDs(ctx, resourcePolicy.Namespace) + folderMoID, rpMoIDs, err := topology.GetNamespaceFolderAndRPMoIDs(ctx, vs.k8sClient, resourcePolicy.Namespace) if err != nil { return err } @@ -132,8 +148,10 @@ func (vs *vSphereVMProvider) DeleteVirtualMachineSetResourcePolicy( clusterModuleProvider := clustermodules.NewProvider(client.RestClient()) errs = append(errs, vs.deleteClusterModules(ctx, clusterModuleProvider, resourcePolicy)...) - if err := vcenter.DeleteChildFolder(ctx, vimClient, folderMoID, resourcePolicy.Spec.Folder); err != nil { - errs = append(errs, err) + if folderName := resourcePolicy.Spec.Folder; folderMoID != "" && folderName != "" { + if err := vcenter.DeleteChildFolder(ctx, vimClient, folderMoID, folderName); err != nil { + errs = append(errs, err) + } } return apierrorsutil.NewAggregate(errs) @@ -246,19 +264,3 @@ func (vs *vSphereVMProvider) deleteClusterModules( resourcePolicy.Status.ClusterModules = errModStatus return errs } - -func (vs *vSphereVMProvider) getNamespaceFolderAndRPMoIDs( - ctx context.Context, - namespace string) (string, []string, error) { - - folderMoID, rpMoIDs, err := topology.GetNamespaceFolderAndRPMoIDs(ctx, vs.k8sClient, namespace) - if err != nil { - return "", nil, err - } - - if folderMoID == "" { - return "", nil, fmt.Errorf("namespace %s not present in any AvailabilityZones", namespace) - } - - return folderMoID, rpMoIDs, nil -} diff --git a/pkg/providers/vsphere/vmprovider_resourcepolicy_test.go b/pkg/providers/vsphere/vmprovider_resourcepolicy_test.go index ec58c1c99..797f53bed 100644 --- a/pkg/providers/vsphere/vmprovider_resourcepolicy_test.go +++ b/pkg/providers/vsphere/vmprovider_resourcepolicy_test.go @@ -16,7 +16,7 @@ import ( vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" "github.com/vmware-tanzu/vm-operator/pkg/providers" - vsphere "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere" + "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere" "github.com/vmware-tanzu/vm-operator/test/builder" ) @@ -69,6 +69,31 @@ func resourcePolicyTests() { initObjects = nil }) + Context("Empty VirtualMachineSetResourcePolicy", func() { + It("Creates and Deletes successfully", func() { + resourcePolicy := &vmopv1.VirtualMachineSetResourcePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-policy", + Namespace: nsInfo.Namespace, + }, + } + + By("Create", func() { + Expect(vmProvider.CreateOrUpdateVirtualMachineSetResourcePolicy(ctx, resourcePolicy)).To(Succeed()) + }) + By("Implicitly IsReady", func() { + for _, zoneName := range ctx.ZoneNames { + exists, err := vmProvider.IsVirtualMachineSetResourcePolicyReady(ctx, zoneName, resourcePolicy) + Expect(err).NotTo(HaveOccurred()) + Expect(exists).To(BeTrue()) + } + }) + By("Delete", func() { + Expect(vmProvider.DeleteVirtualMachineSetResourcePolicy(ctx, resourcePolicy)).To(Succeed()) + }) + }) + }) + Context("VirtualMachineSetResourcePolicy", func() { var ( resourcePolicy *vmopv1.VirtualMachineSetResourcePolicy diff --git a/pkg/topology/availability_zones.go b/pkg/topology/availability_zones.go index e2583f04f..f4131e7a7 100644 --- a/pkg/topology/availability_zones.go +++ b/pkg/topology/availability_zones.go @@ -109,10 +109,14 @@ func GetNamespaceFolderAndRPMoIDs( if err != nil && !errors.Is(err, ErrNoZones) { return "", nil, err } + for _, zone := range zones { - folderMoID = zone.Spec.ManagedVMs.FolderMoID + if folderMoID == "" { + folderMoID = zone.Spec.ManagedVMs.FolderMoID + } rpMoIDs = append(rpMoIDs, zone.Spec.ManagedVMs.PoolMoIDs...) } + return folderMoID, rpMoIDs, nil } @@ -123,7 +127,9 @@ func GetNamespaceFolderAndRPMoIDs( for _, az := range availabilityZones { if nsInfo, ok := az.Spec.Namespaces[namespace]; ok { - folderMoID = nsInfo.FolderMoId + if folderMoID == "" { + folderMoID = nsInfo.FolderMoId + } if len(nsInfo.PoolMoIDs) != 0 { rpMoIDs = append(rpMoIDs, nsInfo.PoolMoIDs...) } else { diff --git a/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator.go b/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator.go index d9baacb4c..010e992e6 100644 --- a/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator.go +++ b/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator.go @@ -107,9 +107,9 @@ func (v validator) validateSpec(ctx *pkgctx.WebhookRequestContext, vmRP *vmopv1. var fieldErrs field.ErrorList specPath := field.NewPath("spec") - fieldErrs = append(fieldErrs, v.validateResourcePool(ctx, specPath.Child("resourcepool"), vmRP.Spec.ResourcePool)...) + fieldErrs = append(fieldErrs, v.validateResourcePool(ctx, specPath.Child("resourcePool"), vmRP.Spec.ResourcePool)...) fieldErrs = append(fieldErrs, v.validateFolder(ctx, specPath.Child("folder"), vmRP.Spec.Folder)...) - fieldErrs = append(fieldErrs, v.validateClusterModules(ctx, specPath.Child("clustermodules"), vmRP.Spec.ClusterModuleGroups)...) + fieldErrs = append(fieldErrs, v.validateClusterModules(ctx, specPath.Child("clusterModuleGroups"), vmRP.Spec.ClusterModuleGroups)...) return fieldErrs } @@ -152,9 +152,9 @@ func (v validator) validateAllowedChanges(ctx *pkgctx.WebhookRequestContext, vmR specPath := field.NewPath("spec") // Validate all fields under spec which are not allowed to change. - allErrs = append(allErrs, validation.ValidateImmutableField(vmRP.Spec.ResourcePool, oldVMRP.Spec.ResourcePool, specPath.Child("resourcepool"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vmRP.Spec.ResourcePool, oldVMRP.Spec.ResourcePool, specPath.Child("resourcePool"))...) allErrs = append(allErrs, validation.ValidateImmutableField(vmRP.Spec.Folder, oldVMRP.Spec.Folder, specPath.Child("folder"))...) - allErrs = append(allErrs, validation.ValidateImmutableField(vmRP.Spec.ClusterModuleGroups, oldVMRP.Spec.ClusterModuleGroups, specPath.Child("clustermodules"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vmRP.Spec.ClusterModuleGroups, oldVMRP.Spec.ClusterModuleGroups, specPath.Child("clusterModuleGroups"))...) return allErrs } diff --git a/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator_intg_test.go b/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator_intg_test.go index bc542bfa2..a5add4bf6 100644 --- a/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator_intg_test.go +++ b/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator_intg_test.go @@ -4,15 +4,14 @@ package validation_test import ( - "k8s.io/apimachinery/pkg/api/resource" - "k8s.io/apimachinery/pkg/util/validation/field" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/validation/field" + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" "github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels" - "github.com/vmware-tanzu/vm-operator/test/builder" ) @@ -99,7 +98,7 @@ func intgTestsValidateCreate() { ctx = nil }) - fieldPath := field.NewPath("spec", "resourcepool", "reservations", "memory") + fieldPath := field.NewPath("spec", "resourcePool", "reservations", "memory") DescribeTable("create table", validateCreate, Entry("should work", true, "", nil), Entry("should not work for invalid", false, diff --git a/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator_unit_test.go b/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator_unit_test.go index f20d2f9d9..10ce9e524 100644 --- a/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator_unit_test.go +++ b/webhooks/virtualmachinesetresourcepolicy/validation/virtualmachinesetresourcepolicy_validator_unit_test.go @@ -128,7 +128,7 @@ func unitTestsValidateCreate() { ctx = nil }) - reservationsPath := field.NewPath("spec", "resourcepool", "reservations") + reservationsPath := field.NewPath("spec", "resourcePool", "reservations") detailMsg := "reservation value cannot exceed the limit value" DescribeTable("create table", validateCreate, Entry("should allow valid", createArgs{}, true, nil, nil),