fix: Do not remove the Classifier when undeploying from the management cluster

controllers/classifier_deployer.go

@@ -42,6 +42,7 @@ import (
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/yaml"
 
 	"github.com/projectsveltos/classifier/controllers/keymanager"
@@ -781,8 +782,40 @@ func undeployClassifierFromCluster(ctx context.Context, c client.Client,
 		return err
 	}
 
+	return removeClassifierInstance(ctx, c, remoteClient, currentClassifier, logger)
+}
+
+// removeClassifierInstance deletes the Classifier instance found in a managed cluster.
+//
+// When the cluster is the management cluster itself (registered as a managed cluster), the instance
+// found is the Classifier being reconciled, not a copy. It must be left alone: removing its
+// finalizer or deleting it from here would bypass the cleanup of all the other clusters.
+func removeClassifierInstance(ctx context.Context, managementClient, remoteClient client.Client,
+	instance *libsveltosv1beta1.Classifier, logger logr.Logger) error {
+
+	classifier := &libsveltosv1beta1.Classifier{}
+	err := managementClient.Get(ctx, types.NamespacedName{Name: instance.Name}, classifier)
+	if err != nil && !apierrors.IsNotFound(err) {
+		return err
+	}
+	if err == nil && classifier.UID == instance.UID {
+		logger.V(logs.LogDebug).Info("classifier instance is the classifier itself. Nothing to remove")
+		return nil
+	}
+
+	// Instances deployed by older versions of sveltos-agent carry the finalizer owned by classifier-manager.
+	// sveltos-agent now uses its own finalizer and ignores this one, which would block the removal of the instance.
+	if controllerutil.ContainsFinalizer(instance, libsveltosv1beta1.ClassifierFinalizer) {
+		logger.V(logs.LogDebug).Info("remove legacy finalizer from classifier instance")
+		patch := client.MergeFromWithOptions(instance.DeepCopy(), client.MergeFromWithOptimisticLock{})
+		controllerutil.RemoveFinalizer(instance, libsveltosv1beta1.ClassifierFinalizer)
+		if err := remoteClient.Patch(ctx, instance, patch); err != nil {
+			return err
+		}
+	}
+
 	logger.V(logs.LogDebug).Info("remove classifier instance")
-	return remoteClient.Delete(ctx, currentClassifier)
+	return remoteClient.Delete(ctx, instance)
 }
 
 func undeployClassifierInPullMode(ctx context.Context, c client.Client,

controllers/classifier_deployer_test.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"reflect"
 	"strings"
+	"time"
 
 	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo/v2"
@@ -38,6 +39,7 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/projectsveltos/classifier/controllers"
 	"github.com/projectsveltos/classifier/controllers/keymanager"
@@ -405,12 +407,14 @@ var _ = Describe("Classifier Deployer", func() {
 		Expect(dep.IsKeyInProgress(key)).To(BeTrue())
 	})
 
-	It("undeployClassifierFromCluster removes Classifier from Cluster", func() {
+	It("undeployClassifierFromCluster does not remove the Classifier when the cluster is the management cluster", func() {
 		dep := fakedeployer.GetClient(context.TODO(), logger, testEnv.Client)
 		Expect(dep.RegisterFeatureID(libsveltosv1beta1.FeatureClassifier)).To(Succeed())
 
 		classifier := getClassifierInstance(randomString())
+		classifier.Finalizers = []string{libsveltosv1beta1.ClassifierFinalizer}
 
+		// prepareCluster registers testEnv, the cluster the Classifier is created in, as the managed cluster
 		cluster := prepareCluster()
 		Expect(testEnv.Create(context.TODO(), classifier)).To(Succeed())
 
@@ -421,13 +425,51 @@ var _ = Describe("Classifier Deployer", func() {
 			deployer.Options{}, logger)
 		Expect(err).To(BeNil())
 
-		// Eventual loop so testEnv Cache is synced
+		verifyClassifierIsUntouched(classifier.Name)
+		removeFinalizersAndDeleteClassifier(classifier.Name)
+	})
+
+	It("removeClassifierInstance leaves the Classifier alone when the instance is the Classifier itself", func() {
+		classifier := getClassifierInstance(randomString())
+		classifier.Finalizers = []string{libsveltosv1beta1.ClassifierFinalizer}
+		Expect(testEnv.Create(context.TODO(), classifier)).To(Succeed())
+		Expect(waitForObject(context.TODO(), testEnv.Client, classifier)).To(Succeed())
+
+		// Management cluster and managed cluster are the same cluster, so the instance is the Classifier
+		Expect(controllers.RemoveClassifierInstance(context.TODO(), testEnv, testEnv, classifier, logger)).To(Succeed())
+
+		verifyClassifierIsUntouched(classifier.Name)
+		removeFinalizersAndDeleteClassifier(classifier.Name)
+	})
+
+	It("removeClassifierInstance removes the legacy finalizer and deletes the instance of a managed cluster", func() {
+		const agentFinalizer = "sveltos-agent.projectsveltos.io/classifier"
+
+		instance := getClassifierInstance(randomString())
+		instance.Finalizers = []string{libsveltosv1beta1.ClassifierFinalizer, agentFinalizer}
+		Expect(testEnv.Create(context.TODO(), instance)).To(Succeed())
+		Expect(waitForObject(context.TODO(), testEnv.Client, instance)).To(Succeed())
+
+		// The Classifier in the management cluster is a different object from the instance in the managed cluster
+		classifier := getClassifierInstance(instance.Name)
+		classifier.UID = types.UID(randomString())
+		managementClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(classifier).Build()
+
+		Expect(controllers.RemoveClassifierInstance(context.TODO(), managementClient, testEnv, instance, logger)).To(Succeed())
+
+		// The finalizer of sveltos-agent is still there, so the instance is terminating
 		Eventually(func() bool {
-			currentClassifier := &libsveltosv1beta1.Classifier{}
-			err = testEnv.Get(context.TODO(), types.NamespacedName{Name: classifier.Name},
-				currentClassifier)
-			return err != nil && apierrors.IsNotFound(err)
+			currentInstance := &libsveltosv1beta1.Classifier{}
+			err := testEnv.Get(context.TODO(), types.NamespacedName{Name: instance.Name}, currentInstance)
+			if err != nil {
+				return false
+			}
+			return !currentInstance.DeletionTimestamp.IsZero() &&
+				!controllerutil.ContainsFinalizer(currentInstance, libsveltosv1beta1.ClassifierFinalizer) &&
+				controllerutil.ContainsFinalizer(currentInstance, agentFinalizer)
 		}, timeout, pollingInterval).Should(BeTrue())
+
+		removeFinalizersAndDeleteClassifier(instance.Name)
 	})
 
 	It("undeployClassifier queues a job to remove classifier from cluster", func() {
@@ -1478,3 +1520,25 @@ func verifyPatches(patches []libsveltosv1beta1.Patch) {
 	}
 	Expect(found).To(BeTrue())
 }
+
+// verifyClassifierIsUntouched verifies the Classifier is not marked for deletion and still has its finalizer
+func verifyClassifierIsUntouched(name string) {
+	Consistently(func() bool {
+		classifier := &libsveltosv1beta1.Classifier{}
+		err := testEnv.Get(context.TODO(), types.NamespacedName{Name: name}, classifier)
+		if err != nil {
+			return false
+		}
+		return classifier.DeletionTimestamp.IsZero() &&
+			controllerutil.ContainsFinalizer(classifier, libsveltosv1beta1.ClassifierFinalizer)
+	}, 3*time.Second, time.Second).Should(BeTrue())
+}
+
+// removeFinalizersAndDeleteClassifier removes all finalizers and deletes the Classifier, so it does not outlive the test
+func removeFinalizersAndDeleteClassifier(name string) {
+	classifier := &libsveltosv1beta1.Classifier{}
+	Expect(testEnv.Get(context.TODO(), types.NamespacedName{Name: name}, classifier)).To(Succeed())
+	classifier.Finalizers = nil
+	Expect(testEnv.Update(context.TODO(), classifier)).To(Succeed())
+	Expect(client.IgnoreNotFound(testEnv.Delete(context.TODO(), classifier))).To(Succeed())
+}

controllers/export_test.go

@@ -54,6 +54,7 @@ var (
 	ClassifierHash                                    = classifierHash
 	DeployClassifierInCluster                         = deployClassifierInCluster
 	UndeployClassifierFromCluster                     = undeployClassifierFromCluster
+	RemoveClassifierInstance                          = removeClassifierInstance
 	RemoveClassifierReports                           = removeClassifierReports
 	RemoveClusterClassifierReports                    = removeClusterClassifierReports
 	PruneClassifierReportsForDeletedClusters          = pruneClassifierReportsForDeletedClusters

pkg/agent/sveltos-agent-in-mgmt-cluster.go

@@ -63,7 +63,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: docker.io/projectsveltos/sveltos-agent@sha256:6bcf5d20ad39145e617109509c452daa420e3e5b86636adf6a1462908789ff70
+        image: docker.io/projectsveltos/sveltos-agent@sha256:59272b4f68af3cdce0d59ba221530ea7783b7e30901750474a1e805f17eab673
         livenessProbe:
           failureThreshold: 3
           httpGet:

pkg/agent/sveltos-agent-in-mgmt-cluster.yaml

@@ -45,7 +45,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: docker.io/projectsveltos/sveltos-agent@sha256:6bcf5d20ad39145e617109509c452daa420e3e5b86636adf6a1462908789ff70
+        image: docker.io/projectsveltos/sveltos-agent@sha256:59272b4f68af3cdce0d59ba221530ea7783b7e30901750474a1e805f17eab673
         livenessProbe:
           failureThreshold: 3
           httpGet:

pkg/agent/sveltos-agent.go

@@ -212,7 +212,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: docker.io/projectsveltos/sveltos-agent@sha256:6bcf5d20ad39145e617109509c452daa420e3e5b86636adf6a1462908789ff70
+        image: docker.io/projectsveltos/sveltos-agent@sha256:59272b4f68af3cdce0d59ba221530ea7783b7e30901750474a1e805f17eab673
         livenessProbe:
           failureThreshold: 3
           httpGet:

pkg/agent/sveltos-agent.yaml

@@ -194,7 +194,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: docker.io/projectsveltos/sveltos-agent@sha256:6bcf5d20ad39145e617109509c452daa420e3e5b86636adf6a1462908789ff70
+        image: docker.io/projectsveltos/sveltos-agent@sha256:59272b4f68af3cdce0d59ba221530ea7783b7e30901750474a1e805f17eab673
         livenessProbe:
           failureThreshold: 3
           httpGet: