Skip to content

Clean up managed labels when clusters no longer match #437

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

Merged
gianlucam76 merged 1 commit into from
Jan 20, 2026
Merged

Clean up managed labels when clusters no longer match #437

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
370 changes: 266 additions & 104 deletions controllers/classifier_controller.go
View file
Open in desktop

Large diffs are not rendered by default.

133 changes: 115 additions & 18 deletions controllers/classifier_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"sync"

"github.com/go-logr/logr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

Expand All @@ -45,9 +46,11 @@ import (

var _ = Describe("Classifier: Reconciler", func() {
var classifier *libsveltosv1beta1.Classifier
var logger logr.Logger

BeforeEach(func() {
classifier = getClassifierInstance(randomString())
logger = textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1)))
})

It("Adds finalizer", func() {
Expand Down Expand Up @@ -131,7 +134,7 @@ var _ = Describe("Classifier: Reconciler", func() {
Expect(c.Status().Update(context.TODO(), currentClassifier)).To(Succeed())

dep := fakedeployer.GetClient(context.TODO(),
textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))), testEnv.Client)
logger, testEnv.Client)
Expect(dep.RegisterFeatureID(libsveltosv1beta1.FeatureClassifier)).To(Succeed())

reconciler := &controllers.ClassifierReconciler{
Expand Down Expand Up @@ -180,6 +183,14 @@ var _ = Describe("Classifier: Reconciler", func() {
classifierReport1.Spec.Match = false
classifierReport2 := getClassifierReport(randomString(), randomString(), randomString())

matchingClusters := map[corev1.ObjectReference]bool{}
matchingClusters[corev1.ObjectReference{
Namespace: clusterNamespace,
Name: clusterName,
APIVersion: clusterv1.GroupVersion.String(),
Kind: clusterv1.ClusterKind,
}] = true

initObjects := []client.Object{
classifier,
classifierReport0,
Expand All @@ -200,14 +211,14 @@ var _ = Describe("Classifier: Reconciler", func() {

classifierScope, err := scope.NewClassifierScope(scope.ClassifierScopeParams{
Client: c,
Logger: textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))),
Logger: logger,
Classifier: classifier,
ControllerName: "classifier",
})
Expect(err).To(BeNil())

Expect(controllers.UpdateMatchingClustersAndRegistrations(reconciler, context.TODO(), classifierScope,
textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))))).To(Succeed())
Expect(controllers.UpdateMatchingClustersAndRegistrations(reconciler, context.TODO(), classifierScope, matchingClusters,
logger)).To(Succeed())

Expect(classifier.Status.MachingClusterStatuses).ToNot(BeNil())
Expect(len(classifier.Status.MachingClusterStatuses)).To(Equal(1))
Expand All @@ -221,6 +232,14 @@ var _ = Describe("Classifier: Reconciler", func() {
classifierReport0 := getClassifierReport(classifier.Name, clusterNamespace, clusterName)
classifierReport0.Spec.Match = true

matchingClusters := map[corev1.ObjectReference]bool{}
matchingClusters[corev1.ObjectReference{
Namespace: clusterNamespace,
Name: clusterName,
APIVersion: clusterv1.GroupVersion.String(),
Kind: clusterv1.ClusterKind,
}] = true

// Create a second classifier with same ClassifierLabels as first classifier
classifier1 := &libsveltosv1beta1.Classifier{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -260,14 +279,14 @@ var _ = Describe("Classifier: Reconciler", func() {

classifierScope, err := scope.NewClassifierScope(scope.ClassifierScopeParams{
Client: c,
Logger: textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))),
Logger: logger,
Classifier: classifier,
ControllerName: "classifier",
})
Expect(err).To(BeNil())

Expect(controllers.UpdateMatchingClustersAndRegistrations(reconciler, context.TODO(), classifierScope,
textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))))).To(Succeed())
Expect(controllers.UpdateMatchingClustersAndRegistrations(reconciler, context.TODO(), classifierScope, matchingClusters,
logger)).To(Succeed())

Expect(classifier.Status.MachingClusterStatuses).ToNot(BeNil())
Expect(len(classifier.Status.MachingClusterStatuses)).To(Equal(1))
Expand Down Expand Up @@ -325,7 +344,7 @@ var _ = Describe("Classifier: Reconciler", func() {

classifierScope, err := scope.NewClassifierScope(scope.ClassifierScopeParams{
Client: c,
Logger: textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))),
Logger: logger,
Classifier: classifier,
ControllerName: "classifier",
})
Expand All @@ -337,12 +356,11 @@ var _ = Describe("Classifier: Reconciler", func() {
currentMatchingClusters := map[corev1.ObjectReference]bool{
{Namespace: cluster.Namespace, Name: cluster.Name, APIVersion: cluster.APIVersion, Kind: cluster.Kind}: true,
}
oldMatchingClusters := map[corev1.ObjectReference]bool{}
Expect(controllers.HandleLabelRegistrations(reconciler, context.TODO(), classifier, currentMatchingClusters,
oldMatchingClusters, textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))))).To(Succeed())
Expect(controllers.RegisterMatchingClusters(reconciler, context.TODO(), classifier, currentMatchingClusters,
logger)).To(Succeed())

Expect(controllers.UpdateLabelsOnMatchingClusters(reconciler, context.TODO(), classifierScope,
textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))))).To(Succeed())
logger)).To(Succeed())

currentCluster := &clusterv1.Cluster{}
Expect(c.Get(context.TODO(),
Expand Down Expand Up @@ -402,18 +420,97 @@ var _ = Describe("Classifier: Reconciler", func() {

classifierScope, err := scope.NewClassifierScope(scope.ClassifierScopeParams{
Client: c,
Logger: textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))),
Logger: logger,
Classifier: classifier,
ControllerName: "classifier",
})
Expect(err).To(BeNil())

Expect(controllers.RemoveAllRegistrations(reconciler, context.TODO(), classifierScope,
textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))))).To(Succeed())
logger)).To(Succeed())
Expect(manager.CanManageLabel(classifier, clusterNamespace, clusterName, label,
libsveltosv1beta1.ClusterTypeCapi)).To(BeFalse())
})

It("cleanUpNonMatchingClusters removes labels from cluster which are not a match", func() {
labelKey := randomString()
labelValue := randomString()
clusterNamespace := randomString()
clusterName := randomString()
classifier.Spec.ClassifierLabels = []libsveltosv1beta1.ClassifierLabel{
{Key: labelKey, Value: labelValue},
}

cluster := &libsveltosv1beta1.SveltosCluster{
ObjectMeta: metav1.ObjectMeta{
Namespace: clusterNamespace,
Name: clusterName,
Labels: map[string]string{
labelKey: labelValue,
},
},
}

clusterRef := corev1.ObjectReference{
Kind: libsveltosv1beta1.SveltosClusterKind,
APIVersion: libsveltosv1beta1.GroupVersion.String(),
Namespace: clusterNamespace,
Name: clusterName,
}

classifier.Status.MachingClusterStatuses = []libsveltosv1beta1.MachingClusterStatus{
{
ClusterRef: corev1.ObjectReference{
Namespace: clusterNamespace,
Name: clusterName,
Kind: clusterKind,
APIVersion: clusterv1.GroupVersion.String(),
},
ManagedLabels: []string{labelKey},
},
}

initObjects := []client.Object{
classifier, cluster,
}

c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(initObjects...).
WithObjects(initObjects...).Build()

reconciler := &controllers.ClassifierReconciler{
Client: c,
Scheme: scheme,
ClusterMap: make(map[corev1.ObjectReference]*libsveltosset.Set),
ClassifierMap: make(map[corev1.ObjectReference]*libsveltosset.Set),
Mux: sync.Mutex{},
}

oldMatches := map[corev1.ObjectReference]bool{clusterRef: true}
currentMatches := map[corev1.ObjectReference]bool{} // No longer matches

err := controllers.CleanUpNonMatchingClusters(reconciler, context.TODO(), classifier, currentMatches, oldMatches, logger)
Expect(err).ToNot(HaveOccurred())

// Since the Classifier is not managing this label, label remains on cluster
currentCluster := &libsveltosv1beta1.SveltosCluster{}
err = c.Get(context.TODO(), client.ObjectKey{Name: clusterRef.Name, Namespace: clusterRef.Namespace}, currentCluster)
Expect(err).ToNot(HaveOccurred())
Expect(currentCluster.Labels).To(HaveKey(labelKey))

manager, err := keymanager.GetKeyManagerInstance(ctx, c)
Expect(err).To(BeNil())
// Register classifier as managing labels on cluster
manager.RegisterClassifierForLabels(classifier, clusterNamespace, clusterName, libsveltosv1beta1.ClusterTypeSveltos)

err = controllers.CleanUpNonMatchingClusters(reconciler, context.TODO(), classifier, currentMatches, oldMatches, logger)
Expect(err).ToNot(HaveOccurred())

// Since the Classifier is managing this label, label is removed from cluster
err = c.Get(context.TODO(), client.ObjectKey{Name: clusterRef.Name, Namespace: clusterRef.Namespace}, currentCluster)
Expect(err).ToNot(HaveOccurred())
Expect(currentCluster.Labels).ToNot(HaveKey(labelKey))
})

It("classifyLabels divides labels in managed and unmanaged", func() {
clusterNamespace := randomString()
clusterName := randomString()
Expand Down Expand Up @@ -466,7 +563,7 @@ var _ = Describe("Classifier: Reconciler", func() {
}

managed, unManaged, err := controllers.ClassifyLabels(reconciler, context.TODO(), classifier,
clusterRef, textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))))
clusterRef, logger)
Expect(err).To(BeNil())
Expect(len(managed)).To(Equal(1))
Expect(len(unManaged)).To(Equal(1))
Expand All @@ -477,11 +574,12 @@ var _ = Describe("Classifier: Reconciler", func() {
var _ = Describe("ClassifierReconciler: requeue methods", func() {
var classifier *libsveltosv1beta1.Classifier
var cluster *clusterv1.Cluster
var logger logr.Logger

BeforeEach(func() {
cluster = prepareCluster()
controllers.SetVersion(version)

logger = textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1)))
classifier = getClassifierInstance(randomString())
})

Expand All @@ -502,8 +600,7 @@ var _ = Describe("ClassifierReconciler: requeue methods", func() {
Name: classifier.Name,
}

dep := fakedeployer.GetClient(context.TODO(), textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))),
testEnv.Client)
dep := fakedeployer.GetClient(context.TODO(), logger, testEnv.Client)
Expect(dep.RegisterFeatureID(libsveltosv1beta1.FeatureClassifier)).To(Succeed())

clusterProfileReconciler := getClassifierReconciler(testEnv.Client, dep)
Expand Down
2 changes: 1 addition & 1 deletion controllers/classifier_predicates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func ClassifierReportPredicate(logger logr.Logger) predicate.Funcs {
// return true if ClassifierReport.Spec.Match has changed
if oldReport.Spec.Match != newReport.Spec.Match {
log.V(logs.LogVerbose).Info(
"Cluster was unpaused. Will attempt to reconcile associated Classifiers.")
"ClassifierReport Spec.Match changed. Will attempt to reconcile associated Classifiers.")
return true
}

Expand Down
3 changes: 2 additions & 1 deletion controllers/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ var (
RequeueClassifierForClassifierReport = (*ClassifierReconciler).requeueClassifierForClassifierReport
UpdateMatchingClustersAndRegistrations = (*ClassifierReconciler).updateMatchingClustersAndRegistrations
UpdateLabelsOnMatchingClusters = (*ClassifierReconciler).updateLabelsOnMatchingClusters
HandleLabelRegistrations = (*ClassifierReconciler).handleLabelRegistrations
RegisterMatchingClusters = (*ClassifierReconciler).registerMatchingClusters
UndeployClassifier = (*ClassifierReconciler).undeployClassifier
RemoveAllRegistrations = (*ClassifierReconciler).removeAllRegistrations
ClassifyLabels = (*ClassifierReconciler).classifyLabels
CleanUpNonMatchingClusters = (*ClassifierReconciler).cleanUpNonMatchingClusters
)

var (
Expand Down
2 changes: 1 addition & 1 deletion test/fv/conflict_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ var _ = Describe("Classifier: update cluster labels", func() {
return err != nil && apierrors.IsNotFound(err)
}, timeout, pollingInterval).Should(BeTrue())

verifyClusterLabels(classifier2)
verifyClusterLabelsAreGone(classifier2)

removeLabels(classifier1)
removeLabels(classifier2)
Expand Down
24 changes: 23 additions & 1 deletion test/fv/labels_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,29 @@ func verifyFlow(namePrefix string) {
}

verifyClassifierReport(classifier.Name, true)
verifyClusterLabels(classifier)

Byf("Changing classifier so cluster is not a match anymore")
currentClassifer := &libsveltosv1beta1.Classifier{}
Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Name: classifier.Name},
currentClassifer)).To(Succeed())
Expect(currentClassifer.Spec.KubernetesVersionConstraints).ToNot(BeNil())
currentClassifer.Spec.KubernetesVersionConstraints[0].Comparison =
string(libsveltosv1beta1.ComparisonLessThan)
Expect(k8sClient.Update(context.TODO(), currentClassifer)).To(Succeed())

verifyClassifierReport(classifier.Name, false)
verifyClusterLabelsAreGone(classifier)

Byf("Changing classifier so cluster is a match again")
Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Name: classifier.Name},
currentClassifer)).To(Succeed())
Expect(currentClassifer.Spec.KubernetesVersionConstraints).ToNot(BeNil())
currentClassifer.Spec.KubernetesVersionConstraints[0].Comparison =
string(libsveltosv1beta1.ComparisonGreaterThanOrEqualTo)
Expect(k8sClient.Update(context.TODO(), currentClassifer)).To(Succeed())

verifyClassifierReport(classifier.Name, true)
verifyClusterLabels(classifier)

Byf("Deleting classifier instance %s in the management cluster", classifier.Name)
Expand Down Expand Up @@ -96,7 +118,7 @@ func verifyFlow(namePrefix string) {
}, timeout, pollingInterval).Should(BeTrue())

Byf("Verifying Cluster labels are not updated because of Classifier being deleted")
verifyClusterLabels(classifier)
verifyClusterLabelsAreGone(classifier)

removeLabels(classifier)
}
4 changes: 2 additions & 2 deletions test/fv/report_match_change_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ var _ = Describe("Classifier: update cluster labels", func() {
return err != nil && apierrors.IsNotFound(err)
}, timeout, pollingInterval).Should(BeTrue())

Byf("Verifying Cluster labels are not updated because of Classifier being deleted")
verifyClusterLabels(classifier)
Byf("Verifying Cluster labels are removed because of Classifier being deleted")
verifyClusterLabelsAreGone(classifier)

removeLabels(classifier)
})
Expand Down
23 changes: 22 additions & 1 deletion test/fv/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func getClassifier(namePrefix string, clusterLabels map[string]string) *libsvelt
ClassifierLabels: labels,
KubernetesVersionConstraints: []libsveltosv1beta1.KubernetesVersionConstraint{
{
Version: "1.25.0",
Version: "1.30.0",
Comparison: string(libsveltosv1beta1.ComparisonGreaterThanOrEqualTo),
},
},
Expand Down Expand Up @@ -130,6 +130,27 @@ func verifyClusterLabels(classifier *libsveltosv1beta1.Classifier) {
}, timeout, pollingInterval).Should(BeTrue())
}

func verifyClusterLabelsAreGone(classifier *libsveltosv1beta1.Classifier) {
Byf("Verifying Classifier labels are removed from cluster %s", classifier.Name)
Eventually(func() bool {
currentCuster, err := getCluster()
if err != nil {
return false
}
if currentCuster.GetLabels() == nil {
return false
}
for i := range classifier.Spec.ClassifierLabels {
cLabel := classifier.Spec.ClassifierLabels[i]
_, ok := currentCuster.GetLabels()[cLabel.Key]
if ok {
return false
}
}
return true
}, timeout, pollingInterval).Should(BeTrue())
}

func removeLabels(classifier *libsveltosv1beta1.Classifier) {
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
currentCluster, err := getCluster()
Expand Down