feat(Controllers): Track previously matched instances for cleanup

controllers/alertrulegroup_controller.go

@@ -91,7 +91,7 @@ func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctr
 
 	removeSuspended(&group.Status.Conditions)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, group)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, group)
 	if err != nil {
 		setNoMatchingInstancesCondition(&group.Status.Conditions, group.Generation, err)
 		meta.RemoveStatusCondition(&group.Status.Conditions, conditionAlertGroupSynchronized)
@@ -299,7 +299,7 @@ func (r *GrafanaAlertRuleGroupReconciler) finalize(ctx context.Context, group *g
 		isCleanupInGrafanaRequired = false
 	}
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, group)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, group)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}

controllers/contactpoint_controller.go

@@ -99,7 +99,7 @@ func (r *GrafanaContactPointReconciler) Reconcile(ctx context.Context, req ctrl.
 
 	removeSuspended(&cr.Status.Conditions)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
+	instances, instDelta, err := GetScopedMatchingInstances(ctx, r.Client, cr)
 	if err != nil {
 		setNoMatchingInstancesCondition(&cr.Status.Conditions, cr.Generation, err)
 		meta.RemoveStatusCondition(&cr.Status.Conditions, conditionContactPointSynchronized)
@@ -154,11 +154,50 @@ func (r *GrafanaContactPointReconciler) Reconcile(ctx context.Context, req ctrl.
 		}
 	}
 
-	condition := buildSynchronizedCondition("Contact point", conditionContactPointSynchronized, cr.Generation, applyErrors, len(instances))
+	cleanupErrors := make(map[string]string)
+	dirtyInstances := make([]grafanav1beta1.Grafana, 0, len(instDelta))
+
+	for _, namespacedName := range instDelta {
+		g := grafanav1beta1.Grafana{}
+
+		err := r.Get(ctx, namespacedName, &g)
+		if err != nil {
+			if kuberr.IsNotFound(err) {
+				continue
+			}
+
+			dirtyInstances = append(dirtyInstances, g)
+			cleanupErrors[namespacedName.String()] = err.Error()
+		}
+
+		err = r.deleteFromInstance(ctx, &g, cr)
+		if err != nil {
+			dirtyInstances = append(dirtyInstances, g)
+			cleanupErrors[namespacedName.String()] = err.Error()
+
+			continue
+		}
+	}
+
+	av := make([]string, 0, len(instances)+len(dirtyInstances))
+	for _, g := range append(instances, dirtyInstances...) {
+		av = append(av, fmt.Sprintf("%s/%s", g.Namespace, g.Name))
+	}
+
+	slices.Sort(av)
+	v := strings.Join(av, ",")
+
+	err = addAnnotation(ctx, r.Client, cr, annotationMatchedInstances, v)
+	if err != nil {
+		log.Error(err, "annotating contact point with matched instances", "annotation", annotationMatchedInstances)
+	}
+
+	allErrors := mergeReconcileErrors(applyErrors, cleanupErrors)
+	condition := buildSynchronizedCondition("Contact point", conditionContactPointSynchronized, cr.Generation, allErrors, len(instances))
 	meta.SetStatusCondition(&cr.Status.Conditions, condition)
 
-	if len(applyErrors) > 0 {
-		return ctrl.Result{}, fmt.Errorf("failed to apply to all instances: %v", applyErrors)
+	if len(allErrors) > 0 {
+		return ctrl.Result{}, fmt.Errorf("syncing all instances: %v", allErrors)
 	}
 
 	return ctrl.Result{RequeueAfter: r.Cfg.requeueAfter(cr.Spec.ResyncPeriod)}, nil
@@ -321,36 +360,45 @@ func (r *GrafanaContactPointReconciler) finalize(ctx context.Context, cr *grafan
 	log := logf.FromContext(ctx)
 	log.Info("Finalizing GrafanaContactPoint")
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}
 
 	for _, instance := range instances {
-		cl, err := client2.NewGeneratedGrafanaClient(ctx, r.Client, &instance)
-		if err != nil {
-			return fmt.Errorf("building grafana client: %w", err)
-		}
-
-		remoteReceivers, err := r.getReceiversFromName(cl, cr)
+		err := r.deleteFromInstance(ctx, &instance, cr)
 		if err != nil {
 			return err
 		}
+	}
 
-		for _, rec := range remoteReceivers {
-			_, err = cl.Provisioning.DeleteContactpoints(rec.UID) //nolint:errcheck
-			if err != nil {
-				return fmt.Errorf("deleting contact point: %w", err)
-			}
-		}
+	return nil
+}
 
-		// Update grafana instance Status
-		err = instance.RemoveNamespacedResource(ctx, r.Client, cr)
+func (r *GrafanaContactPointReconciler) deleteFromInstance(ctx context.Context, g *grafanav1beta1.Grafana, cr *grafanav1beta1.GrafanaContactPoint) error {
+	cl, err := client2.NewGeneratedGrafanaClient(ctx, r.Client, g)
+	if err != nil {
+		return fmt.Errorf("building grafana client: %w", err)
+	}
+
+	remoteReceivers, err := r.getReceiversFromName(cl, cr)
+	if err != nil {
+		return err
+	}
+
+	for _, rec := range remoteReceivers {
+		_, err = cl.Provisioning.DeleteContactpoints(rec.UID) //nolint:errcheck
 		if err != nil {
-			return fmt.Errorf("removing contact point from Grafana cr: %w", err)
+			return fmt.Errorf("deleting contact point: %w", err)
 		}
 	}
 
+	// Update grafana instance Status
+	err = g.RemoveNamespacedResource(ctx, r.Client, cr)
+	if err != nil {
+		return fmt.Errorf("removing contact point from Grafana cr: %w", err)
+	}
+
 	return nil
 }
 

controllers/contactpoint_controller_test.go

@@ -59,7 +59,7 @@ var _ = Describe("ContactPoint Reconciler: Provoke Conditions", func() {
 				Type:   conditionContactPointSynchronized,
 				Reason: conditionReasonApplyFailed,
 			},
-			wantErr: "failed to apply to all instances",
+			wantErr: "syncing all instances:",
 		},
 		{
 			name: "Referenced secret does not exist",

controllers/controller_shared.go

@@ -53,6 +53,9 @@ const (
 
 	// Finalizer
 	grafanaFinalizer = "operator.grafana.com/finalizer"
+
+	// Annotations
+	annotationMatchedInstances = "operator.grafana.com/matched-instances"
 )
 
 var (
@@ -92,13 +95,13 @@ func defaultRateLimiter() workqueue.TypedRateLimiter[reconcile.Request] {
 // Only matching instances in the scope of the resource are returned
 // Resources with allowCrossNamespaceImport expands the scope to the entire cluster
 // Intended to be used in reconciler functions
-func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr v1beta1.CommonResource) ([]v1beta1.Grafana, error) {
+func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr v1beta1.CommonResource) ([]v1beta1.Grafana, []types.NamespacedName, error) {
 	log := logf.FromContext(ctx)
 	instanceSelector := cr.MatchLabels()
 
 	// Should never happen, sanity check
 	if instanceSelector == nil {
-		return []v1beta1.Grafana{}, nil
+		return []v1beta1.Grafana{}, []types.NamespacedName{}, nil
 	}
 
 	opts := []client.ListOption{
@@ -115,11 +118,33 @@ func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr
 
 	err := k8sClient.List(ctx, &list, opts...)
 	if err != nil {
-		return []v1beta1.Grafana{}, err
+		return []v1beta1.Grafana{}, []types.NamespacedName{}, err
+	}
+
+	matchedInstancesDelta := make([]types.NamespacedName, 0, len(list.Items))
+
+	// Find all previously matched instances
+	rawInst := cr.Metadata().Annotations[annotationMatchedInstances]
+	if rawInst != "" {
+		for inst := range strings.SplitSeq(rawInst, ",") {
+			meta := strings.Split(inst, "/")
+
+			// Remove duplicates
+			found := slices.ContainsFunc(matchedInstancesDelta, func(n types.NamespacedName) bool {
+				return n.Namespace == meta[0] && n.Name == meta[1]
+			})
+			if !found {
+				matchedInstancesDelta = append(matchedInstancesDelta, types.NamespacedName{
+					Namespace: meta[0],
+					Name:      meta[1],
+				})
+			}
+		}
 	}
 
+	// NOTE Triggers cleanup against all previously matched instances if any
 	if len(list.Items) == 0 {
-		return []v1beta1.Grafana{}, nil
+		return []v1beta1.Grafana{}, matchedInstancesDelta, nil
 	}
 
 	selectedList := make([]v1beta1.Grafana, 0, len(list.Items))
@@ -133,6 +158,10 @@ func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr
 			continue
 		}
 
+		// Remove instances that were matched again
+		// This way a delta is built for later cleanup
+		matchedInstancesDelta = removeNamespacedNameEntry(matchedInstancesDelta, instance.Namespace, instance.Name)
+
 		// admin url is required to interact with Grafana
 		// the instance or route might not yet be ready
 		if instance.Status.Stage != v1beta1.OperatorStageComplete || instance.Status.StageStatus != v1beta1.OperatorStageResultSuccess {
@@ -147,11 +176,22 @@ func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr
 		log.Info("Grafana instances not ready, excluded from matching", "instances", unreadyInstances)
 	}
 
+	// TODO Decide whether to log recently excluded instances here or in each controller
+	// if len(matchedInstancesDelta) > 0 {
+	// 	log.Info("Grafana instances excluded by instanceSelector, to be removed from instances", "instances", matchedInstancesDelta)
+	// }
+
 	if len(selectedList) == 0 {
 		log.Info("None of the available Grafana instances matched the selector, skipping reconciliation", "AllowCrossNamespaceImport", cr.AllowCrossNamespace())
 	}
 
-	return selectedList, nil
+	return selectedList, matchedInstancesDelta, nil
+}
+
+func removeNamespacedNameEntry(s []types.NamespacedName, namespace, name string) []types.NamespacedName {
+	return slices.DeleteFunc(s, func(n types.NamespacedName) bool {
+		return n.Namespace == namespace && n.Name == name
+	})
 }
 
 // getFolderUID returns the folderUID from an existing GrafanaFolder CR within the same namespace

controllers/controller_shared_test.go

@@ -533,18 +533,18 @@ var _ = Describe("GetMatchingInstances functions", Ordered, func() {
 
 	Context("Ensure AllowCrossNamespaceImport is upheld by GetScopedMatchingInstances", func() {
 		It("Finds all ready instances when instanceSelector is empty", func() {
-			instances, err := GetScopedMatchingInstances(testCtx, k8sClient, matchAllFolder)
+			instances, _, err := GetScopedMatchingInstances(testCtx, k8sClient, matchAllFolder)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(instances).To(HaveLen(2 + 2)) // +2 To account for instances created in controllers/suite_test.go to provoke conditions
 		})
 		It("Finds all ready and Matching instances", func() {
-			instances, err := GetScopedMatchingInstances(testCtx, k8sClient, &allowFolder)
+			instances, _, err := GetScopedMatchingInstances(testCtx, k8sClient, &allowFolder)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(instances).ToNot(BeEmpty())
 			Expect(instances).To(HaveLen(2))
 		})
 		It("Finds matching and ready and matching instance in namespace", func() {
-			instances, err := GetScopedMatchingInstances(testCtx, k8sClient, denyFolder)
+			instances, _, err := GetScopedMatchingInstances(testCtx, k8sClient, denyFolder)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(instances).ToNot(BeEmpty())
 			Expect(instances).To(HaveLen(1))

controllers/dashboard_controller.go

@@ -115,7 +115,7 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	removeInvalidSpec(&cr.Status.Conditions)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
 	if err != nil {
 		setNoMatchingInstancesCondition(&cr.Status.Conditions, cr.Generation, err)
 		meta.RemoveStatusCondition(&cr.Status.Conditions, conditionDashboardSynchronized)
@@ -221,7 +221,7 @@ func (r *GrafanaDashboardReconciler) finalize(ctx context.Context, cr *v1beta1.G
 
 	uid := content.CustomUIDOrUID(cr, cr.Status.UID)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}

controllers/datasource_controller.go

@@ -97,7 +97,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re
 
 	removeSuspended(&cr.Status.Conditions)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
 	if err != nil {
 		setNoMatchingInstancesCondition(&cr.Status.Conditions, cr.Generation, err)
 		meta.RemoveStatusCondition(&cr.Status.Conditions, conditionDatasourceSynchronized)
@@ -190,7 +190,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re
 }
 
 func (r *GrafanaDatasourceReconciler) deleteOldDatasource(ctx context.Context, cr *v1beta1.GrafanaDatasource) error {
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}
@@ -223,7 +223,7 @@ func (r *GrafanaDatasourceReconciler) finalize(ctx context.Context, cr *v1beta1.
 	log := logf.FromContext(ctx)
 	log.Info("Finalizing GrafanaDatasource")
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}

controllers/folder_controller.go

@@ -100,7 +100,7 @@ func (r *GrafanaFolderReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	removeInvalidSpec(&folder.Status.Conditions)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, folder)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, folder)
 	if err != nil {
 		setNoMatchingInstancesCondition(&folder.Status.Conditions, folder.Generation, err)
 		meta.RemoveStatusCondition(&folder.Status.Conditions, conditionFolderSynchronized)
@@ -154,7 +154,7 @@ func (r *GrafanaFolderReconciler) finalize(ctx context.Context, folder *grafanav
 
 	uid := folder.CustomUIDOrUID()
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, folder)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, folder)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}

controllers/librarypanel_controller.go

@@ -130,7 +130,7 @@ func (r *GrafanaLibraryPanelReconciler) Reconcile(ctx context.Context, req ctrl.
 
 	// begin instance selection and reconciliation
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, libraryPanel)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, libraryPanel)
 	if err != nil {
 		setNoMatchingInstancesCondition(&libraryPanel.Status.Conditions, libraryPanel.Generation, err)
 		meta.RemoveStatusCondition(&libraryPanel.Status.Conditions, conditionLibraryPanelSynchronized)
@@ -238,7 +238,7 @@ func (r *GrafanaLibraryPanelReconciler) finalize(ctx context.Context, cr *v1beta
 
 	uid := content.CustomUIDOrUID(cr, cr.Status.UID)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}

controllers/mutetiming_controller.go

@@ -85,7 +85,7 @@ func (r *GrafanaMuteTimingReconciler) Reconcile(ctx context.Context, req ctrl.Re
 
 	removeSuspended(&muteTiming.Status.Conditions)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, muteTiming)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, muteTiming)
 	if err != nil {
 		setNoMatchingInstancesCondition(&muteTiming.Status.Conditions, muteTiming.Generation, err)
 		meta.RemoveStatusCondition(&muteTiming.Status.Conditions, conditionMuteTimingSynchronized)
@@ -207,7 +207,7 @@ func (r *GrafanaMuteTimingReconciler) finalize(ctx context.Context, muteTiming *
 	log := logf.FromContext(ctx)
 	log.Info("Finalizing GrafanaMuteTiming")
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, muteTiming)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, muteTiming)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}

controllers/notificationpolicy_controller.go

@@ -134,7 +134,7 @@ func (r *GrafanaNotificationPolicyReconciler) Reconcile(ctx context.Context, req
 
 	meta.RemoveStatusCondition(&notificationPolicy.Status.Conditions, conditionNotificationPolicyLoopDetected)
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, notificationPolicy)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, notificationPolicy)
 	if err != nil {
 		setNoMatchingInstancesCondition(&notificationPolicy.Status.Conditions, notificationPolicy.Generation, err)
 		meta.RemoveStatusCondition(&notificationPolicy.Status.Conditions, conditionNotificationPolicySynchronized)
@@ -304,7 +304,7 @@ func (r *GrafanaNotificationPolicyReconciler) finalize(ctx context.Context, noti
 	log := logf.FromContext(ctx)
 	log.Info("Finalizing GrafanaNotificationPolicy")
 
-	instances, err := GetScopedMatchingInstances(ctx, r.Client, notificationPolicy)
+	instances, _, err := GetScopedMatchingInstances(ctx, r.Client, notificationPolicy)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}