(opt) Classifier with conflicts

controllers/classifier_controller.go

@@ -64,14 +64,17 @@ const (
 )
 
 const (
-	// deleteRequeueAfter is how long to wait before checking again to see if the cluster still has
-	// children during deletion.
+	// deleteRequeueAfter is how long to wait before checking again to see if the classifier
+	// can be deleted
 	deleteRequeueAfter = 10 * time.Second
 
-	// normalRequeueAfter is how long to wait before checking again to see if the cluster can be moved
-	// to ready after or workload features (for instance ingress or reporter) have failed
+	// normalRequeueAfter is how long to wait before reconciling the classifier instance
 	normalRequeueAfter = 10 * time.Second
 
+	// conflictRequeueAfter is how long to wait before retrying when a label
+	// management conflict is detected with another classifier instance.
+	conflictRequeueAfter = time.Minute
+
 	controlplaneendpoint = "controlplaneendpoint-key"
 	configurationHash    = "configurationHash"
 
@@ -272,19 +275,19 @@ func (r *ClassifierReconciler) reconcileNormal(
 	err := r.updateMatchingClustersAndRegistrations(ctx, classifierScope, logger)
 	if err != nil {
 		logger.V(logs.LogDebug).Info("failed to update matchingClusterRefs")
-		return reconcile.Result{}, err
+		return reconcile.Result{Requeue: true, RequeueAfter: normalRequeueAfter}, nil
 	}
 
 	err = r.updateLabelsOnMatchingClusters(ctx, classifierScope, logger)
 	if err != nil {
 		logger.V(logs.LogDebug).Info("failed to update cluster labels")
-		return reconcile.Result{}, err
+		return reconcile.Result{Requeue: true, RequeueAfter: normalRequeueAfter}, nil
 	}
 
 	err = r.updateClusterInfo(ctx, classifierScope)
 	if err != nil {
 		logger.V(logs.LogDebug).Info("failed to update clusterInfo")
-		return reconcile.Result{}, err
+		return reconcile.Result{Requeue: true, RequeueAfter: normalRequeueAfter}, nil
 	}
 
 	r.updateMaps(classifierScope)
@@ -296,6 +299,10 @@ func (r *ClassifierReconciler) reconcileNormal(
 		return reconcile.Result{Requeue: true, RequeueAfter: normalRequeueAfter}, nil
 	}
 
+	if hasUnManagedLabels(classifierScope.Classifier) {
+		return reconcile.Result{Requeue: true, RequeueAfter: conflictRequeueAfter}, nil
+	}
+
 	logger.V(logs.LogDebug).Info("Reconcile success")
 	return reconcile.Result{}, nil
 }
@@ -319,12 +326,6 @@ func (r *ClassifierReconciler) SetupWithManager(ctx context.Context,
 				ClassifierReportPredicate(mgr.GetLogger().WithValues("predicate", "classifierreportpredicate")),
 			),
 		).
-		Watches(&libsveltosv1beta1.Classifier{},
-			handler.EnqueueRequestsFromMapFunc(r.requeueClassifierForClassifier),
-			builder.WithPredicates(
-				OtherClassifierPredicate(mgr.GetLogger().WithValues("predicate", "otherClassifiepredicate")),
-			),
-		).
 		Watches(&libsveltosv1beta1.SveltosCluster{},
 			handler.EnqueueRequestsFromMapFunc(r.requeueClassifierForSveltosCluster),
 			builder.WithPredicates(
@@ -754,3 +755,15 @@ func startSveltosAgentInMgmtCluster(o deployer.Options) bool {
 
 	return runInMgtmCluster
 }
+
+// hasUnManagedLabels returns true if there is a conflict where this classifier
+// wants to manage labels that are already being managed by another instance.
+func hasUnManagedLabels(classifier *libsveltosv1beta1.Classifier) bool {
+	for i := range classifier.Status.MachingClusterStatuses {
+		if len(classifier.Status.MachingClusterStatuses[i].UnManagedLabels) > 0 {
+			return true
+		}
+	}
+
+	return false
+}

controllers/classifier_predicates.go

@@ -56,7 +56,7 @@ func (p ClassifierPredicate) Update(e event.UpdateEvent) bool {
 
 	if !reflect.DeepEqual(oldClassifier.Spec, newClassifier.Spec) {
 		log.V(logs.LogVerbose).Info(
-			"ClusterSummary Spec changed. Will attempt to reconcile ClusterSummary.",
+			"Classifier Spec changed. Will attempt to reconcile ClusterSummary.",
 		)
 		return true
 	}
@@ -201,7 +201,7 @@ func ConfigMapPredicates(logger logr.Logger) predicate.Funcs {
 			}
 
 			log.V(logs.LogVerbose).Info(
-				"ConfigMap did match expected conditions.  Will attempt to reconcile associated Classifiers.")
+				"ConfigMap did not match expected conditions.  Will not attempt to reconcile associated Classifiers.")
 			return false
 		},
 		DeleteFunc: func(e event.DeleteEvent) bool {
@@ -217,7 +217,7 @@ func ConfigMapPredicates(logger logr.Logger) predicate.Funcs {
 			}
 
 			log.V(logs.LogVerbose).Info(
-				"ConfigMap did match expected conditions.  Will attempt to reconcile associated Classifiers.")
+				"ConfigMap did not match expected conditions.  Will not attempt to reconcile associated Classifiers.")
 			return false
 		},
 		GenericFunc: func(e event.GenericEvent) bool {
@@ -296,67 +296,3 @@ func ClassifierReportPredicate(logger logr.Logger) predicate.Funcs {
 		},
 	}
 }
-
-// OtherClassifierPredicate predicates for Classifier. ClassifierReconciler watches Classifier events
-// and react to those by reconciling itself based on following predicates
-func OtherClassifierPredicate(logger logr.Logger) predicate.Funcs {
-	return predicate.Funcs{
-		UpdateFunc: func(e event.UpdateEvent) bool {
-			newClassifer := e.ObjectNew.(*libsveltosv1beta1.Classifier)
-			oldClassifier := e.ObjectOld.(*libsveltosv1beta1.Classifier)
-			log := logger.WithValues("predicate", "updateEvent",
-				"name", newClassifer.Name,
-			)
-
-			if oldClassifier == nil {
-				log.V(logs.LogVerbose).Info("Old Classifier is nil. Reconcile Classifier")
-				return true
-			}
-
-			// return true if Classifier.Status has changed
-			if !reflect.DeepEqual(oldClassifier.Status.MachingClusterStatuses, newClassifer.Status.MachingClusterStatuses) {
-				log.V(logs.LogVerbose).Info(
-					"Classifier Status.MachingClusterStatuses changed. Will attempt to reconcile associated Classifiers.")
-				return true
-			}
-
-			// otherwise, return false
-			log.V(logs.LogVerbose).Info(
-				"ClassifierReport did not match expected conditions.  Will not attempt to reconcile associated Classifiers.")
-			return false
-		},
-		CreateFunc: func(e event.CreateEvent) bool {
-			classifier := e.Object.(*libsveltosv1beta1.Classifier)
-			log := logger.WithValues("predicate", "createEvent",
-				"namespace", classifier.Namespace,
-				"name", classifier.Name,
-			)
-
-			log.V(logs.LogVerbose).Info(
-				"Classifier did not match expected conditions.  Will attempt to reconcile associated Classifiers.")
-			return false
-		},
-		DeleteFunc: func(e event.DeleteEvent) bool {
-			classifier := e.Object.(*libsveltosv1beta1.Classifier)
-			log := logger.WithValues("predicate", "createEvent",
-				"namespace", classifier.Namespace,
-				"name", classifier.Name,
-			)
-
-			log.V(logs.LogVerbose).Info(
-				"Classifier did match expected conditions.  Will attempt to reconcile associated Classifiers.")
-			return true
-		},
-		GenericFunc: func(e event.GenericEvent) bool {
-			classifier := e.Object.(*libsveltosv1beta1.Classifier)
-			log := logger.WithValues("predicate", "createEvent",
-				"namespace", classifier.Namespace,
-				"name", classifier.Name,
-			)
-
-			log.V(logs.LogVerbose).Info(
-				"Classifier did not match expected conditions.  Will not attempt to reconcile associated Classifiers.")
-			return false
-		},
-	}
-}

controllers/classifier_predicates_test.go

@@ -356,88 +356,3 @@ var _ = Describe("Classifier Predicates: ClassifierReportPredicate", func() {
 		Expect(result).To(BeFalse())
 	})
 })
-
-var _ = Describe("Classifier Predicates: OtherClassifierPredicate", func() {
-	var logger logr.Logger
-	var classifier *libsveltosv1beta1.Classifier
-
-	BeforeEach(func() {
-		logger = textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1)))
-		classifier = &libsveltosv1beta1.Classifier{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: randomString(),
-			},
-		}
-	})
-
-	It("Create does not reprocesses", func() {
-		classifierPredicate := controllers.OtherClassifierPredicate(logger)
-
-		e := event.CreateEvent{
-			Object: classifier,
-		}
-
-		result := classifierPredicate.Create(e)
-		Expect(result).To(BeFalse())
-	})
-	It("Delete does reprocess ", func() {
-		classifierPredicate := controllers.OtherClassifierPredicate(logger)
-
-		e := event.DeleteEvent{
-			Object: classifier,
-		}
-
-		result := classifierPredicate.Delete(e)
-		Expect(result).To(BeTrue())
-	})
-	It("Update reprocesses when Status.MatchinClusterStatuses changes", func() {
-		classifierPredicate := controllers.OtherClassifierPredicate(logger)
-
-		classifier.Status.MachingClusterStatuses = []libsveltosv1beta1.MachingClusterStatus{
-			{
-				ClusterRef:    corev1.ObjectReference{Namespace: randomString(), Name: randomString()},
-				ManagedLabels: []string{randomString(), randomString()},
-			},
-		}
-
-		oldClassifier := &libsveltosv1beta1.Classifier{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: classifier.Name,
-			},
-		}
-		oldClassifier.Status.MachingClusterStatuses = nil
-
-		e := event.UpdateEvent{
-			ObjectNew: classifier,
-			ObjectOld: oldClassifier,
-		}
-
-		result := classifierPredicate.Update(e)
-		Expect(result).To(BeTrue())
-	})
-	It("Update does not reprocess when Status.MatchinClusterStatuses does not change", func() {
-		classifierPredicate := controllers.OtherClassifierPredicate(logger)
-
-		classifier.Status.MachingClusterStatuses = []libsveltosv1beta1.MachingClusterStatus{
-			{
-				ClusterRef:    corev1.ObjectReference{Namespace: randomString(), Name: randomString()},
-				ManagedLabels: []string{randomString(), randomString()},
-			},
-		}
-
-		oldClassifier := &libsveltosv1beta1.Classifier{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: classifier.Name,
-			},
-		}
-		oldClassifier.Status.MachingClusterStatuses = classifier.Status.MachingClusterStatuses
-
-		e := event.UpdateEvent{
-			ObjectNew: classifier,
-			ObjectOld: oldClassifier,
-		}
-
-		result := classifierPredicate.Update(e)
-		Expect(result).To(BeFalse())
-	})
-})

controllers/classifier_transformations.go

@@ -217,43 +217,3 @@ func (r *ClassifierReconciler) requeueClassifierForClassifierReport(
 
 	return requests
 }
-
-func (r *ClassifierReconciler) requeueClassifierForClassifier(
-	ctx context.Context, o client.Object,
-) []reconcile.Request {
-
-	classifier := o.(*libsveltosv1beta1.Classifier)
-	logger := textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1))).WithValues(
-		"objectMapper",
-		"requeueClassifierForClassifier",
-		"classifier",
-		classifier.Name,
-	)
-
-	logger.V(logs.LogDebug).Info("reacting to Classifier change")
-
-	r.Mux.Lock()
-	defer r.Mux.Unlock()
-
-	// Get all Classifier with at least one conflict
-	requests := make([]ctrl.Request, r.ClassifierSet.Len())
-
-	classifierWithConflicts := r.ClassifierSet.Items()
-
-	for i := range classifierWithConflicts {
-		cName := classifierWithConflicts[i].Name
-
-		if cName == classifier.Name {
-			continue
-		}
-
-		logger.V(logs.LogDebug).Info(fmt.Sprintf("queing %s for reconciliation", cName))
-		requests[i] = ctrl.Request{
-			NamespacedName: client.ObjectKey{
-				Name: cName,
-			},
-		}
-	}
-
-	return requests
-}

controllers/classifier_transformations_test.go

@@ -155,36 +155,3 @@ var _ = Describe("ClassifierTransformations map functions", func() {
 		Expect(requests).To(ContainElement(reconcile.Request{NamespacedName: types.NamespacedName{Name: classifierName}}))
 	})
 })
-
-var _ = Describe("ClassifierTransformations map functions", func() {
-	It("requeueClassifierForClassifier returns Classifiers with at least one conflict", func() {
-		c := fake.NewClientBuilder().WithScheme(scheme).Build()
-
-		reconciler := &controllers.ClassifierReconciler{
-			Client:     c,
-			Scheme:     scheme,
-			Mux:        sync.Mutex{},
-			ClusterMap: make(map[corev1.ObjectReference]*libsveltosset.Set),
-		}
-
-		classifierName1 := randomString()
-		classifierInfo1 := corev1.ObjectReference{
-			Kind: libsveltosv1beta1.ClassifierKind, Name: classifierName1, APIVersion: libsveltosv1beta1.GroupVersion.String()}
-		reconciler.ClassifierSet.Insert(&classifierInfo1)
-		classifierName2 := randomString()
-		classifierInfo2 := corev1.ObjectReference{
-			Kind: libsveltosv1beta1.ClassifierKind, Name: classifierName2, APIVersion: libsveltosv1beta1.GroupVersion.String()}
-		reconciler.ClassifierSet.Insert(&classifierInfo2)
-
-		classifier := &libsveltosv1beta1.Classifier{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: randomString(),
-			},
-		}
-
-		requests := controllers.RequeueClassifierForClassifier(reconciler, context.TODO(), classifier)
-		Expect(requests).To(HaveLen(2))
-		Expect(requests).To(ContainElement(reconcile.Request{NamespacedName: types.NamespacedName{Name: classifierName2}}))
-		Expect(requests).To(ContainElement(reconcile.Request{NamespacedName: types.NamespacedName{Name: classifierName1}}))
-	})
-})

controllers/export_test.go

@@ -53,7 +53,6 @@ var (
 	RequeueClassifierForCluster            = (*ClassifierReconciler).requeueClassifierForCluster
 	RequeueClassifierForMachine            = (*ClassifierReconciler).requeueClassifierForMachine
 	RequeueClassifierForClassifierReport   = (*ClassifierReconciler).requeueClassifierForClassifierReport
-	RequeueClassifierForClassifier         = (*ClassifierReconciler).requeueClassifierForClassifier
 	UpdateMatchingClustersAndRegistrations = (*ClassifierReconciler).updateMatchingClustersAndRegistrations
 	UpdateLabelsOnMatchingClusters         = (*ClassifierReconciler).updateLabelsOnMatchingClusters
 	HandleLabelRegistrations               = (*ClassifierReconciler).handleLabelRegistrations