fix: fix missing updaterun metrics and refactor (#292)

Author: jwtty

cmd/hubagent/main.go

@@ -35,7 +35,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
-	"sigs.k8s.io/controller-runtime/pkg/metrics"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	ctrlwebhook "sigs.k8s.io/controller-runtime/pkg/webhook"
 	workv1alpha1 "sigs.k8s.io/work-api/pkg/apis/v1alpha1"
@@ -50,7 +49,6 @@ import (
 	"github.com/kubefleet-dev/kubefleet/cmd/hubagent/workload"
 	mcv1alpha1 "github.com/kubefleet-dev/kubefleet/pkg/controllers/membercluster/v1alpha1"
 	mcv1beta1 "github.com/kubefleet-dev/kubefleet/pkg/controllers/membercluster/v1beta1"
-	fleetmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics"
 	"github.com/kubefleet-dev/kubefleet/pkg/webhook"
 	// +kubebuilder:scaffold:imports
 )
@@ -84,17 +82,6 @@ func init() {
 	utilruntime.Must(clusterinventory.AddToScheme(scheme))
 	// +kubebuilder:scaffold:scheme
 	klog.InitFlags(nil)
-
-	metrics.Registry.MustRegister(
-		fleetmetrics.JoinResultMetrics,
-		fleetmetrics.LeaveResultMetrics,
-		fleetmetrics.PlacementApplyFailedCount,
-		fleetmetrics.PlacementApplySucceedCount,
-		fleetmetrics.SchedulingCycleDurationMilliseconds,
-		fleetmetrics.SchedulerActiveWorkers,
-		fleetmetrics.FleetPlacementStatusLastTimeStampSeconds,
-		fleetmetrics.FleetEvictionStatus,
-	)
 }
 
 func main() {

cmd/memberagent/main.go

@@ -45,7 +45,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
-	"sigs.k8s.io/controller-runtime/pkg/metrics"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	workv1alpha1 "sigs.k8s.io/work-api/pkg/apis/v1alpha1"
@@ -57,7 +56,6 @@ import (
 	imcv1beta1 "github.com/kubefleet-dev/kubefleet/pkg/controllers/internalmembercluster/v1beta1"
 	"github.com/kubefleet-dev/kubefleet/pkg/controllers/workapplier"
 	workv1alpha1controller "github.com/kubefleet-dev/kubefleet/pkg/controllers/workv1alpha1"
-	fleetmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics"
 	"github.com/kubefleet-dev/kubefleet/pkg/propertyprovider"
 	"github.com/kubefleet-dev/kubefleet/pkg/propertyprovider/azure"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils"
@@ -121,13 +119,6 @@ func init() {
 	utilruntime.Must(clusterv1beta1.AddToScheme(scheme))
 	utilruntime.Must(placementv1beta1.AddToScheme(scheme))
 	//+kubebuilder:scaffold:scheme
-
-	metrics.Registry.MustRegister(
-		fleetmetrics.JoinResultMetrics,
-		fleetmetrics.LeaveResultMetrics,
-		fleetmetrics.FleetWorkProcessingRequestsTotal,
-		fleetmetrics.FleetManifestProcessingRequestsTotal,
-	)
 }
 
 func main() {

pkg/controllers/clusterresourceplacementeviction/controller.go

@@ -35,7 +35,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
-	"github.com/kubefleet-dev/kubefleet/pkg/metrics"
+	hubmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics/hub"
 	bindingutils "github.com/kubefleet-dev/kubefleet/pkg/utils/binding"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/condition"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/controller"
@@ -58,7 +58,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 	var internalError bool
 	defer func() {
 		if internalError {
-			metrics.FleetEvictionStatus.WithLabelValues(evictionName, "false", "unknown").SetToCurrentTime()
+			hubmetrics.FleetEvictionStatus.WithLabelValues(evictionName, "false", "unknown").SetToCurrentTime()
 		}
 		latency := time.Since(startTime).Milliseconds()
 		klog.V(2).InfoS("ClusterResourcePlacementEviction reconciliation ends", "clusterResourcePlacementEviction", evictionName, "latency", latency)
@@ -346,12 +346,12 @@ func markEvictionNotExecuted(eviction *placementv1beta1.ClusterResourcePlacement
 }
 
 func emitEvictionCompleteMetric(eviction *placementv1beta1.ClusterResourcePlacementEviction) {
-	metrics.FleetEvictionStatus.DeletePartialMatch(prometheus.Labels{"name": eviction.GetName(), "isCompleted": "false"})
+	hubmetrics.FleetEvictionStatus.DeletePartialMatch(prometheus.Labels{"name": eviction.GetName(), "isCompleted": "false"})
 	// check to see if eviction is valid.
 	if condition.IsConditionStatusTrue(eviction.GetCondition(string(placementv1beta1.PlacementEvictionConditionTypeValid)), eviction.GetGeneration()) {
-		metrics.FleetEvictionStatus.WithLabelValues(eviction.Name, "true", "true").SetToCurrentTime()
+		hubmetrics.FleetEvictionStatus.WithLabelValues(eviction.Name, "true", "true").SetToCurrentTime()
 	} else {
-		metrics.FleetEvictionStatus.WithLabelValues(eviction.Name, "true", "false").SetToCurrentTime()
+		hubmetrics.FleetEvictionStatus.WithLabelValues(eviction.Name, "true", "false").SetToCurrentTime()
 	}
 }
 
@@ -363,7 +363,7 @@ func (r *Reconciler) SetupWithManager(mgr runtime.Manager) error {
 		WithEventFilter(predicate.Funcs{
 			DeleteFunc: func(e event.DeleteEvent) bool {
 				// delete complete status metric for eviction and skip reconciliation.
-				count := metrics.FleetEvictionStatus.DeletePartialMatch(prometheus.Labels{"name": e.Object.GetName()})
+				count := hubmetrics.FleetEvictionStatus.DeletePartialMatch(prometheus.Labels{"name": e.Object.GetName()})
 				klog.V(2).InfoS("ClusterResourcePlacementEviction is being deleted", "clusterResourcePlacementEviction", e.Object.GetName(), "metricCount", count)
 				return false
 			},

pkg/controllers/clusterresourceplacementeviction/controller_intergration_test.go

@@ -33,7 +33,7 @@ import (
 	ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
 
 	placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
-	"github.com/kubefleet-dev/kubefleet/pkg/metrics"
+	hubmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics/hub"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/condition"
 	testutilseviction "github.com/kubefleet-dev/kubefleet/test/utils/eviction"
 )
@@ -58,17 +58,17 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", func() {
 
 	BeforeEach(func() {
 		// Reset metrics before each test
-		metrics.FleetEvictionStatus.Reset()
+		hubmetrics.FleetEvictionStatus.Reset()
 		// emit incomplete eviction metric to simulate eviction failed once.
-		metrics.FleetEvictionStatus.WithLabelValues(evictionName, "false", "unknown").SetToCurrentTime()
+		hubmetrics.FleetEvictionStatus.WithLabelValues(evictionName, "false", "unknown").SetToCurrentTime()
 	})
 
 	AfterEach(func() {
 		ensureCRPDBRemoved(crpName)
 		ensureAllBindingsAreRemoved(crpName)
 		ensureEvictionRemoved(evictionName)
 		ensureCRPRemoved(crpName)
-		metrics.FleetEvictionStatus.Reset()
+		hubmetrics.FleetEvictionStatus.Reset()
 	})
 
 	It("Invalid Eviction Blocked - emit complete metric with isValid=false, isComplete=true", func() {

pkg/controllers/clusterresourceplacementeviction/controller_test.go

@@ -38,7 +38,7 @@ import (
 	ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
 
 	placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
-	"github.com/kubefleet-dev/kubefleet/pkg/metrics"
+	hubmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics/hub"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/condition"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/defaulter"
 )
@@ -1493,7 +1493,7 @@ func TestReconcileForIncompleteEvictionMetric(t *testing.T) {
 	isComplete := "false"
 
 	// Reset metrics before each test
-	metrics.FleetEvictionStatus.Reset()
+	hubmetrics.FleetEvictionStatus.Reset()
 
 	scheme := serviceScheme(t)
 	fakeClient := fake.NewClientBuilder().

pkg/controllers/clusterresourceplacementeviction/suite_test.go

@@ -19,7 +19,6 @@ package clusterresourceplacementeviction
 import (
 	"context"
 	"flag"
-	"os"
 	"path/filepath"
 	"testing"
 
@@ -34,11 +33,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
-	ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
 	placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
-	"github.com/kubefleet-dev/kubefleet/pkg/metrics"
 )
 
 var (
@@ -50,13 +47,6 @@ var (
 	cancel    context.CancelFunc
 )
 
-func TestMain(m *testing.M) {
-	// Register here as the metric is both tested in ginkgo tests and go unit tests.
-	ctrlmetrics.Registry.MustRegister(metrics.FleetEvictionStatus)
-
-	os.Exit(m.Run())
-}
-
 func TestAPIs(t *testing.T) {
 	RegisterFailHandler(Fail)
 

pkg/controllers/internalmembercluster/v1alpha1/member_controller.go

@@ -36,7 +36,7 @@ import (
 
 	fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1"
 	"github.com/kubefleet-dev/kubefleet/pkg/controllers/workv1alpha1"
-	"github.com/kubefleet-dev/kubefleet/pkg/metrics"
+	sharedmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics/shared"
 )
 
 // Reconciler reconciles a InternalMemberCluster object in the member cluster.
@@ -273,7 +273,7 @@ func (r *Reconciler) markInternalMemberClusterJoined(imc fleetv1alpha1.Condition
 	if existingCondition == nil || existingCondition.ObservedGeneration != imc.GetGeneration() || existingCondition.Status != newCondition.Status {
 		r.recorder.Event(imc, corev1.EventTypeNormal, eventReasonInternalMemberClusterJoined, "internal member cluster joined")
 		klog.V(2).InfoS("joined", "InternalMemberCluster", klog.KObj(imc))
-		metrics.ReportJoinResultMetric()
+		sharedmetrics.ReportJoinResultMetric()
 	}
 
 	imc.SetConditionsWithType(fleetv1alpha1.MemberAgent, newCondition)
@@ -313,7 +313,7 @@ func (r *Reconciler) markInternalMemberClusterLeft(imc fleetv1alpha1.Conditioned
 	if existingCondition == nil || existingCondition.ObservedGeneration != imc.GetGeneration() || existingCondition.Status != newCondition.Status {
 		r.recorder.Event(imc, corev1.EventTypeNormal, eventReasonInternalMemberClusterLeft, "internal member cluster left")
 		klog.V(2).InfoS("left", "InternalMemberCluster", klog.KObj(imc))
-		metrics.ReportLeaveResultMetric()
+		sharedmetrics.ReportLeaveResultMetric()
 	}
 
 	imc.SetConditionsWithType(fleetv1alpha1.MemberAgent, newCondition)

pkg/controllers/internalmembercluster/v1beta1/member_controller.go

@@ -40,7 +40,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1"
-	"github.com/kubefleet-dev/kubefleet/pkg/metrics"
+	sharedmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics/shared"
 	"github.com/kubefleet-dev/kubefleet/pkg/propertyprovider"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/condition"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/controller"
@@ -627,7 +627,7 @@ func (r *Reconciler) markInternalMemberClusterJoined(imc clusterv1beta1.Conditio
 	if existingCondition == nil || existingCondition.ObservedGeneration != imc.GetGeneration() || existingCondition.Status != newCondition.Status {
 		r.recorder.Event(imc, corev1.EventTypeNormal, EventReasonInternalMemberClusterJoined, "internal member cluster joined")
 		klog.V(2).InfoS("InternalMemberCluster has joined", "internalMemberCluster", klog.KObj(imc))
-		metrics.ReportJoinResultMetric()
+		sharedmetrics.ReportJoinResultMetric()
 	}
 
 	imc.SetConditionsWithType(clusterv1beta1.MemberAgent, newCondition)
@@ -667,7 +667,7 @@ func (r *Reconciler) markInternalMemberClusterLeft(imc clusterv1beta1.Conditione
 	if existingCondition == nil || existingCondition.ObservedGeneration != imc.GetGeneration() || existingCondition.Status != newCondition.Status {
 		r.recorder.Event(imc, corev1.EventTypeNormal, EventReasonInternalMemberClusterLeft, "internal member cluster left")
 		klog.V(2).InfoS("InternalMemberCluster has left", "internalMemberCluster", klog.KObj(imc))
-		metrics.ReportLeaveResultMetric()
+		sharedmetrics.ReportLeaveResultMetric()
 	}
 
 	imc.SetConditionsWithType(clusterv1beta1.MemberAgent, newCondition)

pkg/controllers/membercluster/v1alpha1/membercluster_controller.go

@@ -42,7 +42,7 @@ import (
 	"github.com/kubefleet-dev/kubefleet/apis"
 	fleetv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
 	fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1"
-	"github.com/kubefleet-dev/kubefleet/pkg/metrics"
+	sharedmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics/shared"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils"
 )
 
@@ -523,7 +523,7 @@ func markMemberClusterJoined(recorder record.EventRecorder, mc apis.ConditionedO
 	if existingCondition == nil || existingCondition.Status != newCondition.Status {
 		recorder.Event(mc, corev1.EventTypeNormal, reasonMemberClusterJoined, "member cluster joined")
 		klog.V(2).InfoS("memberCluster joined", "memberCluster", klog.KObj(mc))
-		metrics.ReportJoinResultMetric()
+		sharedmetrics.ReportJoinResultMetric()
 	}
 
 	mc.SetConditions(newCondition)
@@ -550,7 +550,7 @@ func markMemberClusterLeft(recorder record.EventRecorder, mc apis.ConditionedObj
 	if existingCondition == nil || existingCondition.Status != newCondition.Status {
 		recorder.Event(mc, corev1.EventTypeNormal, reasonMemberClusterJoined, "member cluster left")
 		klog.V(2).InfoS("memberCluster left", "memberCluster", klog.KObj(mc))
-		metrics.ReportLeaveResultMetric()
+		sharedmetrics.ReportLeaveResultMetric()
 	}
 
 	mc.SetConditions(newCondition, notReadyCondition)

pkg/controllers/membercluster/v1beta1/membercluster_controller.go

@@ -43,7 +43,7 @@ import (
 	"github.com/kubefleet-dev/kubefleet/apis"
 	clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1"
 	placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
-	"github.com/kubefleet-dev/kubefleet/pkg/metrics"
+	sharedmetrics "github.com/kubefleet-dev/kubefleet/pkg/metrics/shared"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/condition"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/controller"
@@ -635,7 +635,7 @@ func markMemberClusterJoined(recorder record.EventRecorder, mc apis.ConditionedO
 	if existingCondition == nil || existingCondition.Status != newCondition.Status {
 		recorder.Event(mc, corev1.EventTypeNormal, reasonMemberClusterJoined, "member cluster joined")
 		klog.V(2).InfoS("memberCluster joined", "memberCluster", klog.KObj(mc))
-		metrics.ReportJoinResultMetric()
+		sharedmetrics.ReportJoinResultMetric()
 	}
 
 	mc.SetConditions(newCondition)
@@ -664,7 +664,7 @@ func markMemberClusterLeft(recorder record.EventRecorder, mc apis.ConditionedObj
 	if existingCondition == nil || existingCondition.Status != newCondition.Status {
 		recorder.Event(mc, corev1.EventTypeNormal, reasonMemberClusterJoined, "member cluster left")
 		klog.V(2).InfoS("memberCluster left", "memberCluster", klog.KObj(mc))
-		metrics.ReportLeaveResultMetric()
+		sharedmetrics.ReportLeaveResultMetric()
 	}
 
 	mc.SetConditions(newCondition, notReadyCondition)