Delete stale metrics on object delete

Move record suspend metrics next to readiness and duration metrics so that it gets recorded along with others always at the end and the metrics delete, which requires the knowledge of deleted finalizers, applies to suspend too. HelmRepository cache event metrics for a given helmrepo also continues to be exported even after the object is deleted. This change deletes the cache event metrics when the object is deleted. Signed-off-by: Sunny <darkowlzz@protonmail.com>
fluxcd · Aug 10, 2023 · 0734b61 · 0734b61
1 parent d218731
commit 0734b61
Show file tree

Hide file tree

Showing 11 changed files with 29 additions and 28 deletions.
diff --git a/go.mod b/go.mod
@@ -14,6 +14,8 @@ replace github.com/opencontainers/go-digest => github.com/opencontainers/go-dige
 // Check again when oras.land/oras-go is updated, which is a dependency of Helm.
 replace github.com/docker/docker => github.com/docker/docker v23.0.6+incompatible
 
+replace github.com/fluxcd/pkg/runtime => github.com/fluxcd/pkg/runtime v0.41.1-0.20230810135612-f3f524bcdcff
+
 require (
 	cloud.google.com/go/storage v1.31.0
 	github.com/AdaLogics/go-fuzz-headers v0.0.0-20230106234847-43070de90fa1

diff --git a/go.sum b/go.sum
@@ -407,8 +407,8 @@ github.com/fluxcd/pkg/masktoken v0.2.0 h1:HoSPTk4l1fz5Fevs2vVRvZGru33blfMwWSZKsH
 github.com/fluxcd/pkg/masktoken v0.2.0/go.mod h1:EA7GleAHL33kN6kTW06m5R3/Q26IyuGO7Ef/0CtpDI0=
 github.com/fluxcd/pkg/oci v0.30.1 h1:XRCWzufSRtI6g6TvCH8pJHIqw9qXUf2+9fBH8pOpoU0=
 github.com/fluxcd/pkg/oci v0.30.1/go.mod h1:HAWYIdzEbCnAT7Me2YGVUlgA5y/CCBdJ0+tFdEOb2nI=
-github.com/fluxcd/pkg/runtime v0.41.0 h1:hjWUwVRCKDuGEUhovWrygt/6PRry4p278yKuJNgTfv8=
-github.com/fluxcd/pkg/runtime v0.41.0/go.mod h1:1GN+nxoQ7LmSsLJwjH8JW8pA27tBSO+KLH43HpywCDM=
+github.com/fluxcd/pkg/runtime v0.41.1-0.20230810135612-f3f524bcdcff h1:BCeZdDLd7t4psyu/C06pAgv1usA5pk50G5QoWuZ4NuA=
+github.com/fluxcd/pkg/runtime v0.41.1-0.20230810135612-f3f524bcdcff/go.mod h1:fnUIMXry6BvO5vK45rMCpTjK0QxHz/DI69937yE0zeA=
 github.com/fluxcd/pkg/sourceignore v0.3.5 h1:omcHTH5X5tlPr9w1b9T7WuJTOP+o/KdVdarYb4kgkCU=
 github.com/fluxcd/pkg/sourceignore v0.3.5/go.mod h1:6Xz3jErz8RsidsdrjUBBUGKes24rbdp/F38MnTGibEw=
 github.com/fluxcd/pkg/ssh v0.8.1 h1:v35y7Ks/+ABWce8RcnrC7psVIhf3EdCUNFJi5+tYOps=

diff --git a/internal/cache/metrics.go b/internal/cache/metrics.go
@@ -67,6 +67,11 @@ func (r *CacheRecorder) IncCacheEvents(event, name, namespace string) {
 	r.cacheEventsCounter.WithLabelValues(event, name, namespace).Inc()
 }
 
+// DeleteCacheEvent deletes the cache event metric.
+func (r *CacheRecorder) DeleteCacheEvent(event, name, namespace string) {
+	r.cacheEventsCounter.DeleteLabelValues(event, name, namespace)
+}
+
 // MustMakeMetrics creates a new CacheRecorder, and registers the metrics collectors in the controller-runtime metrics registry.
 func MustMakeMetrics() *CacheRecorder {
 	r := NewCacheRecorder()

diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go
@@ -184,9 +184,6 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// Record suspended status metric
-	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
-
 	// Initialize the patch helper with the current version of the object.
 	serialPatcher := patch.NewSerialPatcher(obj, r.Client)
 
@@ -213,7 +210,8 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 		}
 		result, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
 
-		// Always record readiness and duration metrics
+		// Always record suspend, readiness and duration metrics.
+		r.Metrics.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 		r.Metrics.RecordReadiness(ctx, obj)
 		r.Metrics.RecordDuration(ctx, obj, start)
 	}()

diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go
@@ -177,9 +177,6 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// Record suspended status metric
-	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
-
 	// Initialize the patch helper with the current version of the object.
 	serialPatcher := patch.NewSerialPatcher(obj, r.Client)
 
@@ -207,7 +204,8 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		}
 		result, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
 
-		// Always record readiness and duration metrics
+		// Always record suspend, readiness and duration metrics.
+		r.Metrics.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 		r.Metrics.RecordReadiness(ctx, obj)
 		r.Metrics.RecordDuration(ctx, obj, start)
 	}()

diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go
@@ -198,9 +198,6 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// Record suspended status metric
-	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
-
 	// Initialize the patch helper with the current version of the object.
 	serialPatcher := patch.NewSerialPatcher(obj, r.Client)
 
@@ -228,7 +225,8 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 		result, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
 
-		// Always record readiness and duration metrics
+		// Always record suspend, readiness and duration metrics.
+		r.Metrics.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 		r.Metrics.RecordReadiness(ctx, obj)
 		r.Metrics.RecordDuration(ctx, obj, start)
 	}()

diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go
@@ -161,9 +161,6 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// Record suspended status metric
-	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
-
 	// Initialize the patch helper with the current version of the object.
 	serialPatcher := patch.NewSerialPatcher(obj, r.Client)
 
@@ -190,7 +187,8 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		}
 		result, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
 
-		// Always record readiness and duration metrics
+		// Always record suspend, readiness and duration metrics.
+		r.Metrics.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 		r.Metrics.RecordReadiness(ctx, obj)
 		r.Metrics.RecordDuration(ctx, obj, start)
 	}()
@@ -622,6 +620,12 @@ func (r *HelmRepositoryReconciler) reconcileDelete(ctx context.Context, obj *hel
 		controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer)
 	}
 
+	// Delete cache metrics.
+	if r.CacheRecorder != nil && r.Metrics.IsDelete(obj) {
+		r.DeleteCacheEvent(cache.CacheEventTypeHit, obj.Name, obj.Namespace)
+		r.DeleteCacheEvent(cache.CacheEventTypeMiss, obj.Name, obj.Namespace)
+	}
+
 	// Stop reconciliation as the object is being deleted
 	return sreconcile.ResultEmpty, nil
 }

diff --git a/internal/controller/helmrepository_controller_oci.go b/internal/controller/helmrepository_controller_oci.go
@@ -136,9 +136,6 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return ctrl.Result{RequeueAfter: time.Second}, nil
 	}
 
-	// Record suspended status metric
-	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
-
 	// Initialize the patch helper with the current version of the object.
 	serialPatcher := patch.NewSerialPatcher(obj, r.Client)
 
@@ -167,7 +164,8 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re
 			retErr = kerrors.NewAggregate([]error{retErr, err})
 		}
 
-		// Always record readiness and duration metrics
+		// Always record suspend, readiness and duration metrics.
+		r.Metrics.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 		r.Metrics.RecordReadiness(ctx, obj)
 		r.Metrics.RecordDuration(ctx, obj, start)
 	}()

diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go
@@ -178,9 +178,6 @@ func (r *OCIRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// Record suspended status metric
-	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
-
 	// Initialize the patch helper with the current version of the object.
 	serialPatcher := patch.NewSerialPatcher(obj, r.Client)
 
@@ -208,7 +205,8 @@ func (r *OCIRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		}
 		result, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
 
-		// Always record readiness and duration metrics
+		// Always record suspend, readiness and duration metrics.
+		r.Metrics.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 		r.Metrics.RecordReadiness(ctx, obj)
 		r.Metrics.RecordDuration(ctx, obj, start)
 	}()

diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
@@ -310,7 +310,7 @@ func TestMain(m *testing.M) {
 		panic(fmt.Sprintf("Failed to create a test storage: %v", err))
 	}
 
-	testMetricsH = controller.MustMakeMetrics(testEnv)
+	testMetricsH = controller.MustMakeMetrics(testEnv, sourcev1.SourceFinalizer)
 
 	testWorkspaceDir, err := os.MkdirTemp("", "registry-test-")
 	if err != nil {

diff --git a/main.go b/main.go
@@ -178,7 +178,7 @@ func main() {
 	probes.SetupChecks(mgr, setupLog)
 	pprof.SetupHandlers(mgr, setupLog)
 
-	metrics := helper.MustMakeMetrics(mgr)
+	metrics := helper.MustMakeMetrics(mgr, v1.SourceFinalizer)
 	cacheRecorder := cache.MustMakeMetrics()
 	eventRecorder := mustSetupEventRecorder(mgr, eventsAddr, controllerName)
 	storage := mustInitStorage(storagePath, storageAdvAddr, artifactRetentionTTL, artifactRetentionRecords, artifactDigestAlgo)