Add clientgo metrics to node cleanup controlelr

Author: justinblalock87

cmd/node-cleanup/main.go

@@ -30,6 +30,9 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 
+	"k8s.io/component-base/metrics/legacyregistry"
+	_ "k8s.io/component-base/metrics/prometheus/clientgo" // for client metric registration
+
 	"k8s.io/klog/v2"
 	metrics "sigs.k8s.io/sig-storage-local-static-provisioner/pkg/metrics/node-cleanup"
 	"sigs.k8s.io/sig-storage-local-static-provisioner/pkg/node-cleanup/controller"
@@ -86,13 +89,19 @@ func main() {
 
 	// Prepare http endpoint for metrics
 	if *listenAddress != "" {
-		prometheus.MustRegister([]prometheus.Collector{
-			metrics.APIServerRequestsTotal,
+		reg := prometheus.NewRegistry()
+		reg.MustRegister([]prometheus.Collector{
+			metrics.PersistentVolumeDeleteTotal,
 			metrics.PersistentVolumeDeleteFailedTotal,
 			metrics.PersistentVolumeClaimDeleteTotal,
 			metrics.PersistentVolumeClaimDeleteFailedTotal,
 		}...)
-		http.Handle(*metricsPath, promhttp.Handler())
+		gatherers := prometheus.Gatherers{
+			reg,
+			legacyregistry.DefaultGatherer,
+		}
+
+		http.Handle(*metricsPath, promhttp.HandlerFor(gatherers, promhttp.HandlerOpts{}))
 
 		go func() {
 			klog.Infof("Starting metrics server at %s\n", *listenAddress)

pkg/metrics/node-cleanup/metrics.go

@@ -26,23 +26,25 @@ const (
 )
 
 var (
-	// APIServerRequestsTotal is used to collect accumulated count of apiserver requests.
-	APIServerRequestsTotal = prometheus.NewCounterVec(
+	// PersistentVolumeDeleteTotal is used to collect accumulated count of persistent volume delete attempts.
+	// This metric will report as higher than the true amount of persistent volumes deleted if
+	// the node-cleanup deleter has a short sync period.
+	PersistentVolumeDeleteTotal = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Subsystem: LocalVolumeNodeCleanupSubsystem,
-			Name:      "apiserver_requests_total",
-			Help:      "Total number of apiserver requests. Broken down by method.",
+			Name:      "persistentvolume_delete_total",
+			Help:      "Total number of successful persistent volume delete *attempts*. Broken down by persistent volume phase.",
 		},
-		[]string{"method"},
+		[]string{"phase"},
 	)
 	// PersistentVolumeDeleteFailedTotal is used to collect accumulated count of persistent volume delete failed attempts.
 	PersistentVolumeDeleteFailedTotal = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Subsystem: LocalVolumeNodeCleanupSubsystem,
 			Name:      "persistentvolume_delete_failed_total",
-			Help:      "Total number of persistent volume delete failed attempts. Broken down by persistent volume status and reclaim policy.",
+			Help:      "Total number of persistent volume delete failed attempts. Broken down by persistent volume phase.",
 		},
-		[]string{"status", "reclaim"},
+		[]string{"phase"},
 	)
 	// PersistentVolumeClaimDeleteTotal is used to collect accumulated count of persistent volume claims deleted.
 	PersistentVolumeClaimDeleteTotal = prometheus.NewCounter(

pkg/node-cleanup/controller/controller.go

@@ -38,7 +38,6 @@ import (
 	"k8s.io/klog/v2"
 
 	"sigs.k8s.io/sig-storage-local-static-provisioner/pkg/common"
-	"sigs.k8s.io/sig-storage-local-static-provisioner/pkg/metrics"
 	cleanupmetrics "sigs.k8s.io/sig-storage-local-static-provisioner/pkg/metrics/node-cleanup"
 )
 
@@ -300,7 +299,6 @@ func (c *CleanupController) deletePVC(ctx context.Context, pvc *v1.PersistentVol
 	options := metav1.DeleteOptions{
 		Preconditions: &metav1.Preconditions{UID: &pvc.UID},
 	}
-	cleanupmetrics.APIServerRequestsTotal.WithLabelValues(metrics.APIServerRequestDelete).Inc()
 	err := c.client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Delete(ctx, pvc.Name, options)
 	if err != nil && errors.IsNotFound(err) {
 		// The PVC could already be deleted by some other process

pkg/node-cleanup/deleter/deleter.go

@@ -30,7 +30,6 @@ import (
 	"k8s.io/klog/v2"
 
 	"sigs.k8s.io/sig-storage-local-static-provisioner/pkg/common"
-	"sigs.k8s.io/sig-storage-local-static-provisioner/pkg/metrics"
 	cleanupmetrics "sigs.k8s.io/sig-storage-local-static-provisioner/pkg/metrics/node-cleanup"
 )
 
@@ -102,10 +101,13 @@ func (d *Deleter) DeletePVs(ctx context.Context) {
 		if isReleasedWithDeleteReclaim || isAvailable {
 			klog.Infof("Attempting to delete PV that has NodeAffinity to deleted Node, pv: %s", pv.Name)
 			if err = d.deletePV(ctx, pv.Name); err != nil {
-				cleanupmetrics.PersistentVolumeDeleteFailedTotal.WithLabelValues(string(phase), string(reclaimPolicy)).Inc()
+				cleanupmetrics.PersistentVolumeDeleteFailedTotal.WithLabelValues(string(phase)).Inc()
 				klog.Errorf("Error deleting PV: %s", pv.Name)
 				continue
 			}
+			// TODO: Cache successful deletion to avoid multiple delete calls
+			// when there is a short sync period
+			cleanupmetrics.PersistentVolumeDeleteTotal.WithLabelValues(string(phase)).Inc()
 		}
 	}
 }
@@ -133,7 +135,6 @@ func (d *Deleter) referencesNonExistentNode(localPV *v1.PersistentVolume) (bool,
 }
 
 func (d *Deleter) deletePV(ctx context.Context, pvName string) error {
-	cleanupmetrics.APIServerRequestsTotal.WithLabelValues(metrics.APIServerRequestDelete).Inc()
 	err := d.client.CoreV1().PersistentVolumes().Delete(ctx, pvName, metav1.DeleteOptions{})
 	if err != nil && errors.IsNotFound(err) {
 		klog.Warningf("PV %q no longer exists", pvName)