diff --git a/controllers/vspherecluster_controller.go b/controllers/vspherecluster_controller.go index a7f45c283d..b9fcdfd50e 100644 --- a/controllers/vspherecluster_controller.go +++ b/controllers/vspherecluster_controller.go @@ -55,6 +55,10 @@ import ( infrautilv1 "sigs.k8s.io/cluster-api-provider-vsphere/pkg/util" ) +// legacyIdentityFinalizer is deprecated and should be used only while upgrading the cluster +// from v1alpha3(v.0.7). +const legacyIdentityFinalizer string = "identity/infrastructure.cluster.x-k8s.io" + var ( clusterControlledType = &infrav1.VSphereCluster{} clusterControlledTypeName = reflect.TypeOf(clusterControlledType).Elem().Name() @@ -268,8 +272,13 @@ func (r clusterReconciler) reconcileDelete(ctx *context.ClusterContext) (reconci } return reconcile.Result{}, err } - r.Logger.Info(fmt.Sprintf("Removing finalizer form Secret %s/%s", secret.Namespace, secret.Name)) + r.Logger.Info(fmt.Sprintf("Removing finalizer from Secret %s/%s", secret.Namespace, secret.Name)) ctrlutil.RemoveFinalizer(secret, infrav1.SecretIdentitySetFinalizer) + // Check if the old finalizer(from v0.7) is present, if yes, delete it + // For more context, please refer: https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/issues/1482 + if ctrlutil.ContainsFinalizer(secret, legacyIdentityFinalizer) { + ctrlutil.RemoveFinalizer(secret, legacyIdentityFinalizer) + } if err := ctx.Client.Update(ctx, secret); err != nil { return reconcile.Result{}, err } diff --git a/controllers/vspherecluster_controller_test.go b/controllers/vspherecluster_controller_test.go index 41ccd69c2a..5be5df79f9 100644 --- a/controllers/vspherecluster_controller_test.go +++ b/controllers/vspherecluster_controller_test.go @@ -25,7 +25,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/vmware/govmomi/simulator" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -238,6 +237,118 @@ var _ = Describe("ClusterReconciler", func() { }) }) + Context("Reconcile delete", func() { + var ( + ctx context.Context + secret *corev1.Secret + capiCluster *clusterv1.Cluster + instance *infrav1.VSphereCluster + legacyFinalizer = "identity/infrastructure.cluster.x-k8s.io" + key client.ObjectKey + ) + It("should remove legacy finalizer if present during the cluster deletion", func() { + ctx = context.Background() + capiCluster = &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test1-", + Namespace: "default", + }, + Spec: clusterv1.ClusterSpec{ + InfrastructureRef: &corev1.ObjectReference{ + APIVersion: infrav1.GroupVersion.String(), + Kind: "VsphereCluster", + Name: "vsphere-test1", + }, + }, + } + defer func() { + Expect(testEnv.Cleanup(ctx, capiCluster)).To(Succeed()) + }() + + // Create the CAPI cluster (owner) object + Expect(testEnv.Create(ctx, capiCluster)).To(Succeed()) + + fakeVCenter := startVcenter() + vcURL := fakeVCenter.ServerURL() + defer fakeVCenter.Destroy() + // Create the secret containing the credentials + password, _ := vcURL.User.Password() + + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "secret-", + Namespace: "default", + Finalizers: []string{legacyFinalizer}, + }, + Data: map[string][]byte{ + identity.UsernameKey: []byte(vcURL.User.Username()), + identity.PasswordKey: []byte(password), + }, + } + Expect(testEnv.Create(ctx, secret)).To(Succeed()) + Eventually(func() error { + secretKey := client.ObjectKey{Namespace: secret.Namespace, Name: secret.Name} + return testEnv.Get(ctx, secretKey, secret) + }).Should(BeNil()) + + // Create the VSphereCluster object + instance = &infrav1.VSphereCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vsphere-test1", + Namespace: "default", + }, + Spec: infrav1.VSphereClusterSpec{ + IdentityRef: &infrav1.VSphereIdentityReference{ + Kind: infrav1.SecretKind, + Name: secret.Name, + }, + Server: fmt.Sprintf("%s://%s", vcURL.Scheme, vcURL.Host), + }, + } + Expect(testEnv.Create(ctx, instance)).To(Succeed()) + + // Make sure the VSphereCluster exists. + key = client.ObjectKey{Namespace: instance.Namespace, Name: instance.Name} + Eventually(func() error { + return testEnv.Get(ctx, key, instance) + }, timeout).Should(BeNil()) + + By("setting the OwnerRef on the VSphereCluster") + Eventually(func() bool { + ph, err := patch.NewHelper(instance, testEnv) + Expect(err).ShouldNot(HaveOccurred()) + instance.OwnerReferences = append(instance.OwnerReferences, metav1.OwnerReference{Kind: "Cluster", APIVersion: clusterv1.GroupVersion.String(), Name: capiCluster.Name, UID: "blah"}) + Expect(ph.Patch(ctx, instance, patch.WithStatusObservedGeneration{})).ShouldNot(HaveOccurred()) + return true + }, timeout).Should(BeTrue()) + + By("setting the VSphereCluster's VCenterAvailableCondition to true") + Eventually(func() bool { + if err := testEnv.Get(ctx, key, instance); err != nil { + return false + } + return conditions.IsTrue(instance, infrav1.VCenterAvailableCondition) + }, timeout).Should(BeTrue()) + + By("deleting the vspherecluster which has the secret with legacy finalizer") + Eventually(func() error { + return testEnv.Delete(ctx, instance) + }, timeout).Should(BeNil()) + // confirm that the VSphereCluster is deleted + Eventually(func() bool { + err := testEnv.Get(ctx, key, instance) + return apierrors.IsNotFound(err) + }, timeout).Should(BeTrue()) + + By("checking that the secret is deleted") + secretKey := client.ObjectKey{Namespace: secret.Namespace, Name: secret.Name} + Eventually(func() bool { + err := testEnv.Get(ctx, secretKey, secret) + return apierrors.IsNotFound(err) + }, timeout).Should(BeTrue()) + }) + }) + It("should remove vspherecluster finalizer if the secret does not exist", func() { ctx := context.Background() diff --git a/controllers/vsphereclusteridentity_controller.go b/controllers/vsphereclusteridentity_controller.go index 25e580332f..cef35eb97f 100644 --- a/controllers/vsphereclusteridentity_controller.go +++ b/controllers/vsphereclusteridentity_controller.go @@ -26,9 +26,6 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/api/v1beta1" - "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context" - "sigs.k8s.io/cluster-api-provider-vsphere/pkg/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" clusterutilv1 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" @@ -39,6 +36,10 @@ import ( ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context" + "sigs.k8s.io/cluster-api-provider-vsphere/pkg/record" ) var ( @@ -169,7 +170,12 @@ func (r clusterIdentityReconciler) reconcileDelete(ctx _context.Context, identit } return reconcile.Result{}, err } - r.Logger.Info(fmt.Sprintf("Removing finalizer form Secret %s/%s", secret.Namespace, secret.Name)) + r.Logger.Info(fmt.Sprintf("Removing finalizer from Secret %s/%s", secret.Namespace, secret.Name)) + // Check if the old finalizer(from v0.7) is present, if yes, delete it + // For more context, please refer: https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/issues/1482 + if ctrlutil.ContainsFinalizer(secret, legacyIdentityFinalizer) { + ctrlutil.RemoveFinalizer(secret, legacyIdentityFinalizer) + } ctrlutil.RemoveFinalizer(secret, infrav1.SecretIdentitySetFinalizer) if err := r.Client.Update(ctx, secret); err != nil { return reconcile.Result{}, err