From 9fd1609a1ff5a4abe6aca083af2bc8ce9358eae7 Mon Sep 17 00:00:00 2001 From: Vignesh Goutham Ganesh Date: Wed, 31 Jan 2024 11:14:24 -0600 Subject: [PATCH] Remove finalizer if affinity group is already deleted on Cloudstack --- .../cloudstackaffinitygroup_controller.go | 9 ++++- ...cloudstackaffinitygroup_controller_test.go | 39 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/controllers/cloudstackaffinitygroup_controller.go b/controllers/cloudstackaffinitygroup_controller.go index d7617803..7e3ddc6e 100644 --- a/controllers/cloudstackaffinitygroup_controller.go +++ b/controllers/cloudstackaffinitygroup_controller.go @@ -76,9 +76,14 @@ func (r *CloudStackAGReconciliationRunner) Reconcile() (ctrl.Result, error) { } func (r *CloudStackAGReconciliationRunner) ReconcileDelete() (ctrl.Result, error) { - group := &cloud.AffinityGroup{Name: r.ReconciliationSubject.Name} + group := &cloud.AffinityGroup{Name: r.ReconciliationSubject.Spec.Name} _ = r.CSUser.FetchAffinityGroup(group) - if group.ID == "" { // Affinity group not found, must have been deleted. + // Affinity group not found, must have been deleted. + if group.ID == "" { + // Deleting affinity groups on Cloudstack can return error but succeed in + // deleting the affinity group. Removing finalizer if affinity group is not + // present on Cloudstack ensures affinity group object deletion is not blocked. + controllerutil.RemoveFinalizer(r.ReconciliationSubject, infrav1.AffinityGroupFinalizer) return ctrl.Result{}, nil } if err := r.CSUser.DeleteAffinityGroup(group); err != nil { diff --git a/controllers/cloudstackaffinitygroup_controller_test.go b/controllers/cloudstackaffinitygroup_controller_test.go index d04bdd3a..b59e2df4 100644 --- a/controllers/cloudstackaffinitygroup_controller_test.go +++ b/controllers/cloudstackaffinitygroup_controller_test.go @@ -20,7 +20,9 @@ import ( "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" + "sigs.k8s.io/cluster-api-provider-cloudstack/pkg/cloud" dummies "sigs.k8s.io/cluster-api-provider-cloudstack/test/dummies/v1beta3" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -53,4 +55,41 @@ var _ = Describe("CloudStackAffinityGroupReconciler", func() { return false }, timeout).WithPolling(pollInterval).Should(BeTrue()) }) + + It("Should remove affinity group finalizer if corresponding affinity group is not present on Cloudstack.", func() { + // Modify failure domain name the same way the cluster controller would. + dummies.CSAffinityGroup.Spec.FailureDomainName = dummies.CSFailureDomain1.Spec.Name + + Ω(k8sClient.Create(ctx, dummies.CSFailureDomain1)) + Ω(k8sClient.Create(ctx, dummies.CSAffinityGroup)).Should(Succeed()) + + mockCloudClient.EXPECT().GetOrCreateAffinityGroup(gomock.Any()).AnyTimes() + + // Test that the AffinityGroup controller sets Status.Ready to true. + Eventually(func() bool { + nameSpaceFilter := &client.ListOptions{Namespace: dummies.ClusterNameSpace} + affinityGroups := &infrav1.CloudStackAffinityGroupList{} + if err := k8sClient.List(ctx, affinityGroups, nameSpaceFilter); err == nil { + if len(affinityGroups.Items) == 1 { + return affinityGroups.Items[0].Status.Ready + } + } + return false + }, timeout).WithPolling(pollInterval).Should(BeTrue()) + + Ω(k8sClient.Delete(ctx, dummies.CSAffinityGroup)) + mockCloudClient.EXPECT().FetchAffinityGroup(gomock.Any()).Do(func(arg1 interface{}) { + arg1.(*cloud.AffinityGroup).ID = "" + }).AnyTimes().Return(nil) + + // Once the affinity group id was set to "" the controller should remove the finalizer and unblock deleting affinity group resource + Eventually(func() bool { + retrievedAffinityGroup := &infrav1.CloudStackAffinityGroup{} + affinityGroupKey := client.ObjectKey{Namespace: dummies.ClusterNameSpace, Name: dummies.AffinityGroup.Name} + if err := k8sClient.Get(ctx, affinityGroupKey, retrievedAffinityGroup); err != nil { + return errors.IsNotFound(err) + } + return false + }, timeout).WithPolling(pollInterval).Should(BeTrue()) + }) })