diff --git a/pkg/reconciler/isbsvc/controller.go b/pkg/reconciler/isbsvc/controller.go index e1998f741..c482bc3af 100644 --- a/pkg/reconciler/isbsvc/controller.go +++ b/pkg/reconciler/isbsvc/controller.go @@ -116,9 +116,7 @@ func (r *interStepBufferServiceReconciler) reconcile(ctx context.Context, isbSvc if controllerutil.ContainsFinalizer(isbSvc, deprecatedFinalizerName) { // Remove deprecated finalizer if exists controllerutil.RemoveFinalizer(isbSvc, deprecatedFinalizerName) } - if needsFinalizer(isbSvc) { - controllerutil.AddFinalizer(isbSvc, finalizerName) - } + controllerutil.AddFinalizer(isbSvc, finalizerName) defer func() { if isbSvc.Status.IsHealthy() { @@ -139,13 +137,3 @@ func (r *interStepBufferServiceReconciler) reconcile(ctx context.Context, isbSvc } return installer.Install(ctx, isbSvc, r.client, r.kubeClient, r.config, log, r.recorder) } - -func needsFinalizer(isbSvc *dfv1.InterStepBufferService) bool { - if isbSvc.Spec.Redis != nil && isbSvc.Spec.Redis.Native != nil && isbSvc.Spec.Redis.Native.Persistence != nil { - return true - } - if isbSvc.Spec.JetStream != nil && isbSvc.Spec.JetStream.Persistence != nil { - return true - } - return false -} diff --git a/pkg/reconciler/isbsvc/controller_test.go b/pkg/reconciler/isbsvc/controller_test.go index 97189ff9e..5b77b5967 100644 --- a/pkg/reconciler/isbsvc/controller_test.go +++ b/pkg/reconciler/isbsvc/controller_test.go @@ -217,26 +217,6 @@ func TestNeedsUpdate(t *testing.T) { }) } -func TestNeedsFinalizer(t *testing.T) { - t.Run("needs finalizer redis", func(t *testing.T) { - testStorageClass := "test" - testIsbs := nativeRedisIsbs.DeepCopy() - testIsbs.Spec.Redis.Native.Persistence = &dfv1.PersistenceStrategy{ - StorageClassName: &testStorageClass, - } - assert.True(t, needsFinalizer(testIsbs)) - }) - - t.Run("needs finalizer jetstream", func(t *testing.T) { - testStorageClass := "test" - testIsbs := jetStreamIsbs.DeepCopy() - testIsbs.Spec.JetStream.Persistence = &dfv1.PersistenceStrategy{ - StorageClassName: &testStorageClass, - } - assert.True(t, needsFinalizer(testIsbs)) - }) -} - func contains(arr []string, str string) bool { for _, a := range arr { if a == str { diff --git a/pkg/reconciler/monovertex/controller.go b/pkg/reconciler/monovertex/controller.go index fb2cb85b9..4343b99cf 100644 --- a/pkg/reconciler/monovertex/controller.go +++ b/pkg/reconciler/monovertex/controller.go @@ -68,6 +68,8 @@ func (mr *monoVertexReconciler) Reconcile(ctx context.Context, req ctrl.Request) monoVtx := &dfv1.MonoVertex{} if err := mr.client.Get(ctx, req.NamespacedName, monoVtx); err != nil { if apierrors.IsNotFound(err) { + // Clean up metrics here, since there's no finalizer defined for MonoVertex objects, best effort + cleanupMetrics(req.NamespacedName.Namespace, req.NamespacedName.Name) return reconcile.Result{}, nil } mr.logger.Errorw("Unable to get MonoVertex", zap.Any("request", req), zap.Error(err)) @@ -100,12 +102,8 @@ func (mr *monoVertexReconciler) reconcile(ctx context.Context, monoVtx *dfv1.Mon if !monoVtx.DeletionTimestamp.IsZero() { log.Info("Deleting mono vertex") mr.scaler.StopWatching(mVtxKey) - // Clean up metrics - _ = reconciler.MonoVertexHealth.DeleteLabelValues(monoVtx.Namespace, monoVtx.Name) - _ = reconciler.MonoVertexDesiredReplicas.DeleteLabelValues(monoVtx.Namespace, monoVtx.Name) - _ = reconciler.MonoVertexCurrentReplicas.DeleteLabelValues(monoVtx.Namespace, monoVtx.Name) - _ = reconciler.MonoVertexMaxReplicas.DeleteLabelValues(monoVtx.Namespace, monoVtx.Name) - _ = reconciler.MonoVertexMinReplicas.DeleteLabelValues(monoVtx.Namespace, monoVtx.Name) + // Clean up metrics, best effort + cleanupMetrics(monoVtx.Namespace, monoVtx.Name) return ctrl.Result{}, nil } @@ -622,3 +620,12 @@ func (mr *monoVertexReconciler) checkChildrenResourceStatus(ctx context.Context, return nil } + +// Clean up metrics, should be called when corresponding mvtx is deleted +func cleanupMetrics(namespace, mvtx string) { + _ = reconciler.MonoVertexHealth.DeleteLabelValues(namespace, mvtx) + _ = reconciler.MonoVertexDesiredReplicas.DeleteLabelValues(namespace, mvtx) + _ = reconciler.MonoVertexCurrentReplicas.DeleteLabelValues(namespace, mvtx) + _ = reconciler.MonoVertexMaxReplicas.DeleteLabelValues(namespace, mvtx) + _ = reconciler.MonoVertexMinReplicas.DeleteLabelValues(namespace, mvtx) +}