Skip to content

Commit 7b286ec

Browse files
BaarsgaardBaarsgaard
committed
feat(ContactPoint): Track previously matched instances for cleanup
1 parent 91b8da1 commit 7b286ec

12 files changed

+137
-46
lines changed

‎controllers/alertrulegroup_controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctr
9191

9292
removeSuspended(&group.Status.Conditions)
9393

94-
instances, err := GetScopedMatchingInstances(ctx, r.Client, group)
94+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, group)
9595
if err != nil {
9696
setNoMatchingInstancesCondition(&group.Status.Conditions, group.Generation, err)
9797
meta.RemoveStatusCondition(&group.Status.Conditions, conditionAlertGroupSynchronized)
@@ -299,7 +299,7 @@ func (r *GrafanaAlertRuleGroupReconciler) finalize(ctx context.Context, group *g
299299
isCleanupInGrafanaRequired = false
300300
}
301301

302-
instances, err := GetScopedMatchingInstances(ctx, r.Client, group)
302+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, group)
303303
if err != nil {
304304
return fmt.Errorf("fetching instances: %w", err)
305305
}

‎controllers/contactpoint_controller.go‎

Lines changed: 68 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (r *GrafanaContactPointReconciler) Reconcile(ctx context.Context, req ctrl.
9999

100100
removeSuspended(&cr.Status.Conditions)
101101

102-
instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
102+
instances, instDelta, err := GetScopedMatchingInstances(ctx, r.Client, cr)
103103
if err != nil {
104104
setNoMatchingInstancesCondition(&cr.Status.Conditions, cr.Generation, err)
105105
meta.RemoveStatusCondition(&cr.Status.Conditions, conditionContactPointSynchronized)
@@ -154,11 +154,50 @@ func (r *GrafanaContactPointReconciler) Reconcile(ctx context.Context, req ctrl.
154154
}
155155
}
156156

157-
condition := buildSynchronizedCondition("Contact point", conditionContactPointSynchronized, cr.Generation, applyErrors, len(instances))
157+
cleanupErrors := make(map[string]string)
158+
dirtyInstances := make([]grafanav1beta1.Grafana, 0, len(instDelta))
159+
160+
for _, namespacedName := range instDelta {
161+
g := grafanav1beta1.Grafana{}
162+
163+
err := r.Get(ctx, namespacedName, &g)
164+
if err != nil {
165+
if kuberr.IsNotFound(err) {
166+
continue
167+
}
168+
169+
dirtyInstances = append(dirtyInstances, g)
170+
cleanupErrors[namespacedName.String()] = err.Error()
171+
}
172+
173+
err = r.deleteFromInstance(ctx, &g, cr)
174+
if err != nil {
175+
dirtyInstances = append(dirtyInstances, g)
176+
cleanupErrors[namespacedName.String()] = err.Error()
177+
178+
continue
179+
}
180+
}
181+
182+
av := make([]string, 0, len(instances)+len(dirtyInstances))
183+
for _, g := range append(instances, dirtyInstances...) {
184+
av = append(av, fmt.Sprintf("%s/%s", g.Namespace, g.Name))
185+
}
186+
187+
slices.Sort(av)
188+
v := strings.Join(av, ",")
189+
190+
err = addAnnotation(ctx, r.Client, cr, annotationMatchedInstances, v)
191+
if err != nil {
192+
log.Error(err, "annotating contact point with matched instances", "annotation", annotationMatchedInstances)
193+
}
194+
195+
allErrors := mergeReconcileErrors(applyErrors, cleanupErrors)
196+
condition := buildSynchronizedCondition("Contact point", conditionContactPointSynchronized, cr.Generation, allErrors, len(instances))
158197
meta.SetStatusCondition(&cr.Status.Conditions, condition)
159198

160-
if len(applyErrors) > 0 {
161-
return ctrl.Result{}, fmt.Errorf("failed to apply to all instances: %v", applyErrors)
199+
if len(allErrors) > 0 {
200+
return ctrl.Result{}, fmt.Errorf("syncing all instances: %v", allErrors)
162201
}
163202

164203
return ctrl.Result{RequeueAfter: r.Cfg.requeueAfter(cr.Spec.ResyncPeriod)}, nil
@@ -321,36 +360,45 @@ func (r *GrafanaContactPointReconciler) finalize(ctx context.Context, cr *grafan
321360
log := logf.FromContext(ctx)
322361
log.Info("Finalizing GrafanaContactPoint")
323362

324-
instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
363+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
325364
if err != nil {
326365
return fmt.Errorf("fetching instances: %w", err)
327366
}
328367

329368
for _, instance := range instances {
330-
cl, err := client2.NewGeneratedGrafanaClient(ctx, r.Client, &instance)
331-
if err != nil {
332-
return fmt.Errorf("building grafana client: %w", err)
333-
}
334-
335-
remoteReceivers, err := r.getReceiversFromName(cl, cr)
369+
err := r.deleteFromInstance(ctx, &instance, cr)
336370
if err != nil {
337371
return err
338372
}
373+
}
339374

340-
for _, rec := range remoteReceivers {
341-
_, err = cl.Provisioning.DeleteContactpoints(rec.UID) //nolint:errcheck
342-
if err != nil {
343-
return fmt.Errorf("deleting contact point: %w", err)
344-
}
345-
}
375+
return nil
376+
}
346377

347-
// Update grafana instance Status
348-
err = instance.RemoveNamespacedResource(ctx, r.Client, cr)
378+
func (r *GrafanaContactPointReconciler) deleteFromInstance(ctx context.Context, g *grafanav1beta1.Grafana, cr *grafanav1beta1.GrafanaContactPoint) error {
379+
cl, err := client2.NewGeneratedGrafanaClient(ctx, r.Client, g)
380+
if err != nil {
381+
return fmt.Errorf("building grafana client: %w", err)
382+
}
383+
384+
remoteReceivers, err := r.getReceiversFromName(cl, cr)
385+
if err != nil {
386+
return err
387+
}
388+
389+
for _, rec := range remoteReceivers {
390+
_, err = cl.Provisioning.DeleteContactpoints(rec.UID) //nolint:errcheck
349391
if err != nil {
350-
return fmt.Errorf("removing contact point from Grafana cr: %w", err)
392+
return fmt.Errorf("deleting contact point: %w", err)
351393
}
352394
}
353395

396+
// Update grafana instance Status
397+
err = g.RemoveNamespacedResource(ctx, r.Client, cr)
398+
if err != nil {
399+
return fmt.Errorf("removing contact point from Grafana cr: %w", err)
400+
}
401+
354402
return nil
355403
}
356404

‎controllers/contactpoint_controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ var _ = Describe("ContactPoint Reconciler: Provoke Conditions", func() {
5959
Type: conditionContactPointSynchronized,
6060
Reason: conditionReasonApplyFailed,
6161
},
62-
wantErr: "failed to apply to all instances",
62+
wantErr: "syncing all instances:",
6363
},
6464
{
6565
name: "Referenced secret does not exist",

‎controllers/controller_shared.go‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ const (
5353

5454
// Finalizer
5555
grafanaFinalizer = "operator.grafana.com/finalizer"
56+
57+
// Annotations
58+
annotationMatchedInstances = "operator.grafana.com/matched-instances"
5659
)
5760

5861
var (
@@ -92,13 +95,13 @@ func defaultRateLimiter() workqueue.TypedRateLimiter[reconcile.Request] {
9295
// Only matching instances in the scope of the resource are returned
9396
// Resources with allowCrossNamespaceImport expands the scope to the entire cluster
9497
// Intended to be used in reconciler functions
95-
func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr v1beta1.CommonResource) ([]v1beta1.Grafana, error) {
98+
func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr v1beta1.CommonResource) ([]v1beta1.Grafana, []types.NamespacedName, error) {
9699
log := logf.FromContext(ctx)
97100
instanceSelector := cr.MatchLabels()
98101

99102
// Should never happen, sanity check
100103
if instanceSelector == nil {
101-
return []v1beta1.Grafana{}, nil
104+
return []v1beta1.Grafana{}, []types.NamespacedName{}, nil
102105
}
103106

104107
opts := []client.ListOption{
@@ -115,11 +118,33 @@ func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr
115118

116119
err := k8sClient.List(ctx, &list, opts...)
117120
if err != nil {
118-
return []v1beta1.Grafana{}, err
121+
return []v1beta1.Grafana{}, []types.NamespacedName{}, err
122+
}
123+
124+
matchedInstancesDelta := make([]types.NamespacedName, 0, len(list.Items))
125+
126+
// Find all previously matched instances
127+
rawInst := cr.Metadata().Annotations[annotationMatchedInstances]
128+
if rawInst != "" {
129+
for inst := range strings.SplitSeq(rawInst, ",") {
130+
meta := strings.Split(inst, "/")
131+
132+
// Remove duplicates
133+
found := slices.ContainsFunc(matchedInstancesDelta, func(n types.NamespacedName) bool {
134+
return n.Namespace == meta[0] && n.Name == meta[1]
135+
})
136+
if !found {
137+
matchedInstancesDelta = append(matchedInstancesDelta, types.NamespacedName{
138+
Namespace: meta[0],
139+
Name: meta[1],
140+
})
141+
}
142+
}
119143
}
120144

145+
// NOTE Triggers cleanup against all previously matched instances if any
121146
if len(list.Items) == 0 {
122-
return []v1beta1.Grafana{}, nil
147+
return []v1beta1.Grafana{}, matchedInstancesDelta, nil
123148
}
124149

125150
selectedList := make([]v1beta1.Grafana, 0, len(list.Items))
@@ -133,6 +158,10 @@ func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr
133158
continue
134159
}
135160

161+
// Remove instances that were matched again
162+
// This way a delta is built for later cleanup
163+
matchedInstancesDelta = removeNamespacedNameEntry(matchedInstancesDelta, instance.Namespace, instance.Name)
164+
136165
// admin url is required to interact with Grafana
137166
// the instance or route might not yet be ready
138167
if instance.Status.Stage != v1beta1.OperatorStageComplete || instance.Status.StageStatus != v1beta1.OperatorStageResultSuccess {
@@ -147,11 +176,22 @@ func GetScopedMatchingInstances(ctx context.Context, k8sClient client.Client, cr
147176
log.Info("Grafana instances not ready, excluded from matching", "instances", unreadyInstances)
148177
}
149178

179+
// TODO Decide whether to log recently excluded instances here or in each controller
180+
// if len(matchedInstancesDelta) > 0 {
181+
// log.Info("Grafana instances excluded by instanceSelector, to be removed from instances", "instances", matchedInstancesDelta)
182+
// }
183+
150184
if len(selectedList) == 0 {
151185
log.Info("None of the available Grafana instances matched the selector, skipping reconciliation", "AllowCrossNamespaceImport", cr.AllowCrossNamespace())
152186
}
153187

154-
return selectedList, nil
188+
return selectedList, matchedInstancesDelta, nil
189+
}
190+
191+
func removeNamespacedNameEntry(s []types.NamespacedName, namespace, name string) []types.NamespacedName {
192+
return slices.DeleteFunc(s, func(n types.NamespacedName) bool {
193+
return n.Namespace == namespace && n.Name == name
194+
})
155195
}
156196

157197
// getFolderUID returns the folderUID from an existing GrafanaFolder CR within the same namespace
@@ -483,6 +523,9 @@ func addAnnotation(ctx context.Context, cl client.Client, cr client.Object, key
483523
return nil
484524
}
485525

526+
if crAnnotations == nil {
527+
crAnnotations = make(map[string]string, 0)
528+
}
486529
// Add key to map and create patch
487530
crAnnotations[key] = value
488531

‎controllers/controller_shared_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -533,18 +533,18 @@ var _ = Describe("GetMatchingInstances functions", Ordered, func() {
533533

534534
Context("Ensure AllowCrossNamespaceImport is upheld by GetScopedMatchingInstances", func() {
535535
It("Finds all ready instances when instanceSelector is empty", func() {
536-
instances, err := GetScopedMatchingInstances(testCtx, k8sClient, matchAllFolder)
536+
instances, _, err := GetScopedMatchingInstances(testCtx, k8sClient, matchAllFolder)
537537
Expect(err).ToNot(HaveOccurred())
538538
Expect(instances).To(HaveLen(2 + 2)) // +2 To account for instances created in controllers/suite_test.go to provoke conditions
539539
})
540540
It("Finds all ready and Matching instances", func() {
541-
instances, err := GetScopedMatchingInstances(testCtx, k8sClient, &allowFolder)
541+
instances, _, err := GetScopedMatchingInstances(testCtx, k8sClient, &allowFolder)
542542
Expect(err).ToNot(HaveOccurred())
543543
Expect(instances).ToNot(BeEmpty())
544544
Expect(instances).To(HaveLen(2))
545545
})
546546
It("Finds matching and ready and matching instance in namespace", func() {
547-
instances, err := GetScopedMatchingInstances(testCtx, k8sClient, denyFolder)
547+
instances, _, err := GetScopedMatchingInstances(testCtx, k8sClient, denyFolder)
548548
Expect(err).ToNot(HaveOccurred())
549549
Expect(instances).ToNot(BeEmpty())
550550
Expect(instances).To(HaveLen(1))

‎controllers/dashboard_controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req
115115

116116
removeInvalidSpec(&cr.Status.Conditions)
117117

118-
instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
118+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
119119
if err != nil {
120120
setNoMatchingInstancesCondition(&cr.Status.Conditions, cr.Generation, err)
121121
meta.RemoveStatusCondition(&cr.Status.Conditions, conditionDashboardSynchronized)
@@ -221,7 +221,7 @@ func (r *GrafanaDashboardReconciler) finalize(ctx context.Context, cr *v1beta1.G
221221

222222
uid := content.CustomUIDOrUID(cr, cr.Status.UID)
223223

224-
instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
224+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
225225
if err != nil {
226226
return fmt.Errorf("fetching instances: %w", err)
227227
}

‎controllers/datasource_controller.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re
9797

9898
removeSuspended(&cr.Status.Conditions)
9999

100-
instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
100+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
101101
if err != nil {
102102
setNoMatchingInstancesCondition(&cr.Status.Conditions, cr.Generation, err)
103103
meta.RemoveStatusCondition(&cr.Status.Conditions, conditionDatasourceSynchronized)
@@ -190,7 +190,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re
190190
}
191191

192192
func (r *GrafanaDatasourceReconciler) deleteOldDatasource(ctx context.Context, cr *v1beta1.GrafanaDatasource) error {
193-
instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
193+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
194194
if err != nil {
195195
return fmt.Errorf("fetching instances: %w", err)
196196
}
@@ -223,7 +223,7 @@ func (r *GrafanaDatasourceReconciler) finalize(ctx context.Context, cr *v1beta1.
223223
log := logf.FromContext(ctx)
224224
log.Info("Finalizing GrafanaDatasource")
225225

226-
instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
226+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
227227
if err != nil {
228228
return fmt.Errorf("fetching instances: %w", err)
229229
}

‎controllers/folder_controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func (r *GrafanaFolderReconciler) Reconcile(ctx context.Context, req ctrl.Reques
100100

101101
removeInvalidSpec(&folder.Status.Conditions)
102102

103-
instances, err := GetScopedMatchingInstances(ctx, r.Client, folder)
103+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, folder)
104104
if err != nil {
105105
setNoMatchingInstancesCondition(&folder.Status.Conditions, folder.Generation, err)
106106
meta.RemoveStatusCondition(&folder.Status.Conditions, conditionFolderSynchronized)
@@ -154,7 +154,7 @@ func (r *GrafanaFolderReconciler) finalize(ctx context.Context, folder *grafanav
154154

155155
uid := folder.CustomUIDOrUID()
156156

157-
instances, err := GetScopedMatchingInstances(ctx, r.Client, folder)
157+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, folder)
158158
if err != nil {
159159
return fmt.Errorf("fetching instances: %w", err)
160160
}

‎controllers/librarypanel_controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func (r *GrafanaLibraryPanelReconciler) Reconcile(ctx context.Context, req ctrl.
130130

131131
// begin instance selection and reconciliation
132132

133-
instances, err := GetScopedMatchingInstances(ctx, r.Client, libraryPanel)
133+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, libraryPanel)
134134
if err != nil {
135135
setNoMatchingInstancesCondition(&libraryPanel.Status.Conditions, libraryPanel.Generation, err)
136136
meta.RemoveStatusCondition(&libraryPanel.Status.Conditions, conditionLibraryPanelSynchronized)
@@ -238,7 +238,7 @@ func (r *GrafanaLibraryPanelReconciler) finalize(ctx context.Context, cr *v1beta
238238

239239
uid := content.CustomUIDOrUID(cr, cr.Status.UID)
240240

241-
instances, err := GetScopedMatchingInstances(ctx, r.Client, cr)
241+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, cr)
242242
if err != nil {
243243
return fmt.Errorf("fetching instances: %w", err)
244244
}

‎controllers/mutetiming_controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (r *GrafanaMuteTimingReconciler) Reconcile(ctx context.Context, req ctrl.Re
8585

8686
removeSuspended(&muteTiming.Status.Conditions)
8787

88-
instances, err := GetScopedMatchingInstances(ctx, r.Client, muteTiming)
88+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, muteTiming)
8989
if err != nil {
9090
setNoMatchingInstancesCondition(&muteTiming.Status.Conditions, muteTiming.Generation, err)
9191
meta.RemoveStatusCondition(&muteTiming.Status.Conditions, conditionMuteTimingSynchronized)
@@ -207,7 +207,7 @@ func (r *GrafanaMuteTimingReconciler) finalize(ctx context.Context, muteTiming *
207207
log := logf.FromContext(ctx)
208208
log.Info("Finalizing GrafanaMuteTiming")
209209

210-
instances, err := GetScopedMatchingInstances(ctx, r.Client, muteTiming)
210+
instances, _, err := GetScopedMatchingInstances(ctx, r.Client, muteTiming)
211211
if err != nil {
212212
return fmt.Errorf("fetching instances: %w", err)
213213
}

0 commit comments

Comments
 (0)