Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(controller)!: Remove default category creation and deletion #471

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(controller)!: Remove default category creation and deletion #471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions api/v1beta1/conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,6 @@ const (
FailureDomainsReconciliationFailed = "FailureDomainsReconciliationFailed"
)

const (
// ClusterCategoryCreatedCondition indicates the status of the category linked to the NutanixCluster
ClusterCategoryCreatedCondition capiv1.ConditionType = "ClusterCategoryCreated"

ClusterCategoryCreationFailed = "ClusterCategoryCreationFailed"
)

const (
// PrismCentralClientCondition indicates the status of the client used to connect to Prism Central
PrismCentralClientCondition capiv1.ConditionType = "PrismClientInit"
Expand Down
188 changes: 3 additions & 185 deletions controllers/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,54 +404,6 @@ func GetSubnetUUIDList(ctx context.Context, client *prismclientv3.Client, machin
return subnetUUIDs, nil
}

// GetDefaultCAPICategoryIdentifiers returns the default CAPI category identifiers
func GetDefaultCAPICategoryIdentifiers(clusterName string) []*infrav1.NutanixCategoryIdentifier {
return []*infrav1.NutanixCategoryIdentifier{
{
Key: infrav1.DefaultCAPICategoryKeyForName,
Value: clusterName,
},
}
}

// GetObsoleteDefaultCAPICategoryIdentifiers returns the default CAPI category identifiers
func GetObsoleteDefaultCAPICategoryIdentifiers(clusterName string) []*infrav1.NutanixCategoryIdentifier {
return []*infrav1.NutanixCategoryIdentifier{
{
Key: fmt.Sprintf("%s%s", infrav1.ObsoleteDefaultCAPICategoryPrefix, clusterName),
Value: infrav1.ObsoleteDefaultCAPICategoryOwnedValue,
},
}
}

// GetOrCreateCategories returns the list of category UUIDs for the given list of category names
func GetOrCreateCategories(ctx context.Context, client *prismclientv3.Client, categoryIdentifiers []*infrav1.NutanixCategoryIdentifier) ([]*prismclientv3.CategoryValueStatus, error) {
categories := make([]*prismclientv3.CategoryValueStatus, 0)
for _, ci := range categoryIdentifiers {
if ci == nil {
return categories, fmt.Errorf("cannot get or create nil category")
}
category, err := getOrCreateCategory(ctx, client, ci)
if err != nil {
return categories, err
}
categories = append(categories, category)
}
return categories, nil
}

func getCategoryKey(ctx context.Context, client *prismclientv3.Client, key string) (*prismclientv3.CategoryKeyStatus, error) {
categoryKey, err := client.V3.GetCategoryKey(ctx, key)
if err != nil {
if !strings.Contains(fmt.Sprint(err), "ENTITY_NOT_FOUND") {
return nil, fmt.Errorf("failed to retrieve category with key %s. error: %v", key, err)
} else {
return nil, nil
}
}
return categoryKey, nil
}

func getCategoryValue(ctx context.Context, client *prismclientv3.Client, key, value string) (*prismclientv3.CategoryValueStatus, error) {
categoryValue, err := client.V3.GetCategoryValue(ctx, key, value)
if err != nil {
Expand All @@ -464,143 +416,6 @@ func getCategoryValue(ctx context.Context, client *prismclientv3.Client, key, va
return categoryValue, nil
}

func deleteCategoryKeyValues(ctx context.Context, client *prismclientv3.Client, categoryIdentifiers []*infrav1.NutanixCategoryIdentifier, ignoreKeyDeletion bool) error {
log := ctrl.LoggerFrom(ctx)
groupCategoriesByKey := make(map[string][]string, 0)
for _, ci := range categoryIdentifiers {
ciKey := ci.Key
ciValue := ci.Value
if gck, ok := groupCategoriesByKey[ciKey]; ok {
groupCategoriesByKey[ciKey] = append(gck, ciValue)
} else {
groupCategoriesByKey[ciKey] = []string{ciValue}
}
}

for key, values := range groupCategoriesByKey {
log.V(1).Info(fmt.Sprintf("Retrieving category with key %s", key))
categoryKey, err := getCategoryKey(ctx, client, key)
if err != nil {
errorMsg := fmt.Errorf("failed to retrieve category with key %s. error: %v", key, err)
log.Error(errorMsg, "failed to retrieve category")
return errorMsg
}
log.V(1).Info(fmt.Sprintf("Category with key %s found. Starting deletion of values", key))
if categoryKey == nil {
log.V(1).Info(fmt.Sprintf("Category with key %s not found. Already deleted?", key))
continue
}
for _, value := range values {
categoryValue, err := getCategoryValue(ctx, client, key, value)
if err != nil {
errorMsg := fmt.Errorf("failed to retrieve category value %s in category %s. error: %v", value, key, err)
log.Error(errorMsg, "failed to retrieve category value")
return errorMsg
}
if categoryValue == nil {
log.V(1).Info(fmt.Sprintf("Category with value %s in category %s not found. Already deleted?", value, key))
continue
}

err = client.V3.DeleteCategoryValue(ctx, key, value)
if err != nil {
errorMsg := fmt.Errorf("failed to delete category with key %s. error: %v", key, err)
log.Error(errorMsg, "failed to delete category")
return errorMsg
}
}

if !ignoreKeyDeletion {
// check if there are remaining category values
categoryKeyValues, err := client.V3.ListCategoryValues(ctx, key, &prismclientv3.CategoryListMetadata{})
if err != nil {
errorMsg := fmt.Errorf("failed to get values of category with key %s: %v", key, err)
log.Error(errorMsg, "failed to get values of category")
return errorMsg
}
if len(categoryKeyValues.Entities) > 0 {
errorMsg := fmt.Errorf("cannot remove category with key %s because it still has category values assigned", key)
log.Error(errorMsg, "cannot remove category")
return errorMsg
}
log.V(1).Info(fmt.Sprintf("No values assigned to category. Removing category with key %s", key))
err = client.V3.DeleteCategoryKey(ctx, key)
if err != nil {
errorMsg := fmt.Errorf("failed to delete category with key %s: %v", key, err)
log.Error(errorMsg, "failed to delete category")
return errorMsg
}
}
}
return nil
}

// DeleteCategories deletes the given list of categories
func DeleteCategories(ctx context.Context, client *prismclientv3.Client, categoryIdentifiers, obsoleteCategoryIdentifiers []*infrav1.NutanixCategoryIdentifier) error {
// Dont delete keys with newer format as key is constant string
err := deleteCategoryKeyValues(ctx, client, categoryIdentifiers, true)
if err != nil {
return err
}
// Delete obsolete keys with older format to cleanup brownfield setups
err = deleteCategoryKeyValues(ctx, client, obsoleteCategoryIdentifiers, false)
if err != nil {
return err
}

return nil
}

func getOrCreateCategory(ctx context.Context, client *prismclientv3.Client, categoryIdentifier *infrav1.NutanixCategoryIdentifier) (*prismclientv3.CategoryValueStatus, error) {
log := ctrl.LoggerFrom(ctx)
if categoryIdentifier == nil {
return nil, fmt.Errorf("category identifier cannot be nil when getting or creating categories")
}
if categoryIdentifier.Key == "" {
return nil, fmt.Errorf("category identifier key must be set when when getting or creating categories")
}
if categoryIdentifier.Value == "" {
return nil, fmt.Errorf("category identifier key must be set when when getting or creating categories")
}
log.V(1).Info(fmt.Sprintf("Checking existence of category with key %s", categoryIdentifier.Key))
categoryKey, err := getCategoryKey(ctx, client, categoryIdentifier.Key)
if err != nil {
errorMsg := fmt.Errorf("failed to retrieve category with key %s. error: %v", categoryIdentifier.Key, err)
log.Error(errorMsg, "failed to retrieve category")
return nil, errorMsg
}
if categoryKey == nil {
log.V(1).Info(fmt.Sprintf("Category with key %s did not exist.", categoryIdentifier.Key))
categoryKey, err = client.V3.CreateOrUpdateCategoryKey(ctx, &prismclientv3.CategoryKey{
Description: utils.StringPtr(infrav1.DefaultCAPICategoryDescription),
Name: utils.StringPtr(categoryIdentifier.Key),
})
if err != nil {
errorMsg := fmt.Errorf("failed to create category with key %s. error: %v", categoryIdentifier.Key, err)
log.Error(errorMsg, "failed to create category")
return nil, errorMsg
}
}
categoryValue, err := getCategoryValue(ctx, client, *categoryKey.Name, categoryIdentifier.Value)
if err != nil {
errorMsg := fmt.Errorf("failed to retrieve category value %s in category %s. error: %v", categoryIdentifier.Value, categoryIdentifier.Key, err)
log.Error(errorMsg, "failed to retrieve category")
return nil, errorMsg
}
if categoryValue == nil {
categoryValue, err = client.V3.CreateOrUpdateCategoryValue(ctx, *categoryKey.Name, &prismclientv3.CategoryValue{
Description: utils.StringPtr(infrav1.DefaultCAPICategoryDescription),
Value: utils.StringPtr(categoryIdentifier.Value),
})
if err != nil {
errorMsg := fmt.Errorf("failed to create category value %s in category key %s: %v", categoryIdentifier.Value, categoryIdentifier.Key, err)
log.Error(errorMsg, "failed to create category value")
return nil, errorMsg
}
}
return categoryValue, nil
}

// GetCategoryVMSpec returns a flatmap of categories and their values
func GetCategoryVMSpec(ctx context.Context, client *prismclientv3.Client, categoryIdentifiers []*infrav1.NutanixCategoryIdentifier) (map[string]string, error) {
log := ctrl.LoggerFrom(ctx)
Expand All @@ -612,13 +427,16 @@ func GetCategoryVMSpec(ctx context.Context, client *prismclientv3.Client, catego
log.Error(errorMsg, "failed to retrieve category")
return nil, errorMsg
}

if categoryValue == nil {
errorMsg := fmt.Errorf("category value %s not found in category %s. error", ci.Value, ci.Key)
log.Error(errorMsg, "category value not found")
return nil, errorMsg
}

categorySpec[ci.Key] = ci.Value
}

return categorySpec, nil
}

Expand Down
45 changes: 0 additions & 45 deletions controllers/nutanixcluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,12 +227,6 @@ func (r *NutanixClusterReconciler) reconcileDelete(rctx *nctx.ClusterContext) (r

log.V(1).Info("no existing nutanixMachine resources found. Continuing with deleting cluster")

err = r.reconcileCategoriesDelete(rctx)
if err != nil {
log.Error(err, "error occurred while running deletion of categories")
return reconcile.Result{}, err
}

// delete the client from the cache
log.Info(fmt.Sprintf("deleting nutanix prism client for cluster %s from cache", rctx.NutanixCluster.GetNamespacedName()))
nutanixclient.NutanixClientCache.Delete(&nutanixclient.CacheParams{NutanixCluster: rctx.NutanixCluster})
Expand Down Expand Up @@ -285,13 +279,6 @@ func (r *NutanixClusterReconciler) reconcileNormal(rctx *nctx.ClusterContext) (r
return reconcile.Result{}, nil
}

err := r.reconcileCategories(rctx)
if err != nil {
log.Error(err, "error occurred while reconciling categories")
// Don't return fatal error but keep retrying until categories are created.
return reconcile.Result{}, err
}

rctx.NutanixCluster.Status.Ready = true
return reconcile.Result{}, nil
}
Expand All @@ -315,38 +302,6 @@ func (r *NutanixClusterReconciler) reconcileFailureDomains(rctx *nctx.ClusterCon
return nil
}

func (r *NutanixClusterReconciler) reconcileCategories(rctx *nctx.ClusterContext) error {
log := ctrl.LoggerFrom(rctx.Context)
log.Info("Reconciling categories for cluster")
defaultCategories := GetDefaultCAPICategoryIdentifiers(rctx.Cluster.Name)
_, err := GetOrCreateCategories(rctx.Context, rctx.NutanixClient, defaultCategories)
if err != nil {
conditions.MarkFalse(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition, infrav1.ClusterCategoryCreationFailed, capiv1.ConditionSeverityError, err.Error())
return err
}
conditions.MarkTrue(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition)
return nil
}

func (r *NutanixClusterReconciler) reconcileCategoriesDelete(rctx *nctx.ClusterContext) error {
log := ctrl.LoggerFrom(rctx.Context)
log.Info(fmt.Sprintf("Reconciling deletion of categories for cluster %s", rctx.Cluster.Name))
if conditions.IsTrue(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition) ||
conditions.GetReason(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition) == infrav1.DeletionFailed {
defaultCategories := GetDefaultCAPICategoryIdentifiers(rctx.Cluster.Name)
obsoleteCategories := GetObsoleteDefaultCAPICategoryIdentifiers(rctx.Cluster.Name)
err := DeleteCategories(rctx.Context, rctx.NutanixClient, defaultCategories, obsoleteCategories)
if err != nil {
conditions.MarkFalse(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition, infrav1.DeletionFailed, capiv1.ConditionSeverityWarning, err.Error())
return err
}
} else {
log.V(1).Info(fmt.Sprintf("skipping category deletion since they were not created for cluster %s", rctx.Cluster.Name))
}
conditions.MarkFalse(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition, capiv1.DeletingReason, capiv1.ConditionSeverityInfo, "")
return nil
}

func (r *NutanixClusterReconciler) reconcileCredentialRefDelete(ctx context.Context, nutanixCluster *infrav1.NutanixCluster) error {
log := ctrl.LoggerFrom(ctx)
credentialRef, err := getPrismCentralCredentialRefForCluster(nutanixCluster)
Expand Down
16 changes: 3 additions & 13 deletions controllers/nutanixmachine_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,21 +887,11 @@
}

func (r *NutanixMachineReconciler) getMachineCategoryIdentifiers(rctx *nctx.MachineContext) []*infrav1.NutanixCategoryIdentifier {
log := ctrl.LoggerFrom(rctx.Context)
categoryIdentifiers := GetDefaultCAPICategoryIdentifiers(rctx.Cluster.Name)
categoryIdentifiers := make([]*infrav1.NutanixCategoryIdentifier, 0, len(rctx.NutanixMachine.Spec.AdditionalCategories))

Check warning on line 890 in controllers/nutanixmachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/nutanixmachine_controller.go#L890 

Added line #L890 was not covered by tests
// Only try to create default categories. ignoring error so that we can return all including
// additionalCategories as well
_, err := GetOrCreateCategories(rctx.Context, rctx.NutanixClient, categoryIdentifiers)
if err != nil {
log.Error(err, "Failed to getOrCreateCategories")
}

additionalCategories := rctx.NutanixMachine.Spec.AdditionalCategories
if len(additionalCategories) > 0 {
for _, at := range additionalCategories {
additionalCat := at
categoryIdentifiers = append(categoryIdentifiers, &additionalCat)
}
for _, category := range rctx.NutanixMachine.Spec.AdditionalCategories {
categoryIdentifiers = append(categoryIdentifiers, &category)

Check warning on line 894 in controllers/nutanixmachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/nutanixmachine_controller.go#L893-L894 

Added lines #L893 - L894 were not covered by tests
}

return categoryIdentifiers
Expand Down
49 changes: 2 additions & 47 deletions test/e2e/categories_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/test/framework/clusterctl"

infrav1 "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1"
Expand Down Expand Up @@ -60,48 +59,6 @@ var _ = Describe("Nutanix categories", Label("capx-feature-test", "categories"),
dumpSpecResourcesAndCleanup(ctx, specName, bootstrapClusterProxy, artifactFolder, namespace, cancelWatches, clusterResources.Cluster, e2eConfig.GetIntervals, skipCleanup)
})

It("Create a cluster with default cluster categories (no additional categories)", func() {
Expect(namespace).NotTo(BeNil())
flavor := clusterctl.DefaultFlavor
expectedClusterNameCategoryKey := infrav1.DefaultCAPICategoryKeyForName
By("Creating a workload cluster (no additional categories)", func() {
testHelper.deployClusterAndWait(
deployClusterParams{
clusterName: clusterName,
namespace: namespace,
flavor: flavor,
clusterctlConfigPath: clusterctlConfigPath,
artifactFolder: artifactFolder,
bootstrapClusterProxy: bootstrapClusterProxy,
}, clusterResources)
})

By("Checking cluster category condition is true", func() {
testHelper.verifyConditionOnNutanixCluster(verifyConditionParams{
clusterName: clusterName,
namespace: namespace,
bootstrapClusterProxy: bootstrapClusterProxy,
expectedCondition: clusterv1.Condition{
Type: infrav1.ClusterCategoryCreatedCondition,
Status: corev1.ConditionTrue,
},
})
})

By("Checking if a category was created", func() {
testHelper.verifyCategoryExists(ctx, expectedClusterNameCategoryKey, clusterName)
})

By("Checking if there are VMs assigned to this category", func() {
expectedCategories := map[string]string{
expectedClusterNameCategoryKey: clusterName,
}
testHelper.verifyCategoriesNutanixMachines(ctx, clusterName, namespace.Name, expectedCategories)
})

By("PASSED!")
})

It("Create a cluster with additional categories", func() {
Expect(namespace).NotTo(BeNil())
flavor := "additional-categories"
Expand All @@ -119,11 +76,9 @@ var _ = Describe("Nutanix categories", Label("capx-feature-test", "categories"),
})

By("Verify if additional categories are assigned to the vms", func() {
expectedClusterNameCategoryKey := infrav1.DefaultCAPICategoryKeyForName
expectedCategories := map[string]string{
expectedClusterNameCategoryKey: clusterName,
"AppType": "Kubernetes",
"Environment": "Dev",
"AppType": "Kubernetes",
"Environment": "Dev",
}

testHelper.verifyCategoriesNutanixMachines(ctx, clusterName, namespace.Name, expectedCategories)
Expand Down
Loading