pkg/util/metrics: idem-potent registration of RateLimiterMetric and never cleanup

pkg/util/metrics/BUILD

@@ -12,7 +12,6 @@ go_library(
     importpath = "k8s.io/kubernetes/pkg/util/metrics",
     deps = [
         "//staging/src/k8s.io/client-go/util/flowcontrol:go_default_library",
-        "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
     ],
 )

pkg/util/metrics/util.go

@@ -19,21 +19,15 @@ package metrics
 import (
 	"fmt"
 	"sync"
-	"time"
 
 	"k8s.io/client-go/util/flowcontrol"
 
-	"github.com/golang/glog"
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-const (
-	updatePeriod = 5 * time.Second
-)
-
 var (
 	metricsLock        sync.Mutex
-	rateLimiterMetrics = make(map[string]rateLimiterMetric)
+	rateLimiterMetrics = make(map[string]*rateLimiterMetric)
 )
 
 type rateLimiterMetric struct {
@@ -46,7 +40,8 @@ func registerRateLimiterMetric(ownerName string) error {
 	defer metricsLock.Unlock()
 
 	if _, ok := rateLimiterMetrics[ownerName]; ok {
-		return fmt.Errorf("Rate Limiter Metric for %v already registered", ownerName)
+		// only register once in Prometheus. We happen to see an ownerName reused in parallel integration tests.
+		return nil
 	}
 	metric := prometheus.NewGauge(prometheus.GaugeOpts{
 		Name:      "rate_limiter_use",
@@ -57,7 +52,7 @@ func registerRateLimiterMetric(ownerName string) error {
 		return fmt.Errorf("error registering rate limiter usage metric: %v", err)
 	}
 	stopCh := make(chan struct{})
-	rateLimiterMetrics[ownerName] = rateLimiterMetric{
+	rateLimiterMetrics[ownerName] = &rateLimiterMetric{
 		metric: metric,
 		stopCh: stopCh,
 	}
@@ -79,22 +74,3 @@ func RegisterMetricAndTrackRateLimiterUsage(ownerName string, rateLimiter flowco
 	// }, updatePeriod, rateLimiterMetrics[ownerName].stopCh)
 	return nil
 }
-
-// UnregisterMetricAndUntrackRateLimiterUsage unregisters a metric ownerName_rate_limiter_use from prometheus and
-// stops the goroutine that updates this metric
-func UnregisterMetricAndUntrackRateLimiterUsage(ownerName string) bool {
-	metricsLock.Lock()
-	defer metricsLock.Unlock()
-
-	rlm, ok := rateLimiterMetrics[ownerName]
-	if !ok {
-		glog.Warningf("Rate Limiter Metric for %v not registered", ownerName)
-		return false
-	}
-
-	close(rlm.stopCh)
-	prometheus.Unregister(rlm.metric)
-	delete(rateLimiterMetrics, ownerName)
-
-	return true
-}

pkg/util/metrics/util_test.go

@@ -57,27 +57,3 @@ func TestRegisterMetricAndTrackRateLimiterUsage(t *testing.T) {
 		}
 	}
 }
-
-func TestUnregisterMetricAndUntrackRateLimiterUsage(t *testing.T) {
-	RegisterMetricAndTrackRateLimiterUsage("owner_name", flowcontrol.NewTokenBucketRateLimiter(1, 1))
-	testCases := []struct {
-		ownerName string
-		ok        bool
-	}{
-		{
-			ownerName: "owner_name",
-			ok:        true,
-		},
-		{
-			ownerName: "owner_name",
-			ok:        false,
-		},
-	}
-
-	for i, tc := range testCases {
-		ok := UnregisterMetricAndUntrackRateLimiterUsage(tc.ownerName)
-		if tc.ok != ok {
-			t.Errorf("Case[%d] Expected %v, got %v", i, tc.ok, ok)
-		}
-	}
-}

test/integration/daemonset/BUILD

@@ -24,7 +24,6 @@ go_test(
         "//pkg/scheduler/algorithmprovider:go_default_library",
         "//pkg/scheduler/factory:go_default_library",
         "//pkg/util/labels:go_default_library",
-        "//pkg/util/metrics:go_default_library",
         "//staging/src/k8s.io/api/apps/v1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",

test/integration/daemonset/daemonset_test.go

@@ -52,7 +52,6 @@ import (
 	_ "k8s.io/kubernetes/pkg/scheduler/algorithmprovider"
 	"k8s.io/kubernetes/pkg/scheduler/factory"
 	labelsutil "k8s.io/kubernetes/pkg/util/labels"
-	"k8s.io/kubernetes/pkg/util/metrics"
 	"k8s.io/kubernetes/test/integration/framework"
 )
 
@@ -69,7 +68,6 @@ func setup(t *testing.T) (*httptest.Server, framework.CloseFunc, *daemon.DaemonS
 	}
 	resyncPeriod := 12 * time.Hour
 	informers := informers.NewSharedInformerFactory(clientset.NewForConfigOrDie(restclient.AddUserAgent(&config, "daemonset-informers")), resyncPeriod)
-	metrics.UnregisterMetricAndUntrackRateLimiterUsage("daemon_controller")
 	dc, err := daemon.NewDaemonSetsController(
 		informers.Apps().V1().DaemonSets(),
 		informers.Apps().V1().ControllerRevisions(),

test/integration/deployment/BUILD

@@ -38,7 +38,6 @@ go_library(
         "//pkg/controller/deployment:go_default_library",
         "//pkg/controller/deployment/util:go_default_library",
         "//pkg/controller/replicaset:go_default_library",
-        "//pkg/util/metrics:go_default_library",
         "//staging/src/k8s.io/api/apps/v1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/extensions/v1beta1:go_default_library",

test/integration/deployment/util.go

@@ -36,7 +36,6 @@ import (
 	"k8s.io/kubernetes/pkg/controller/deployment"
 	deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
 	"k8s.io/kubernetes/pkg/controller/replicaset"
-	"k8s.io/kubernetes/pkg/util/metrics"
 	"k8s.io/kubernetes/test/integration/framework"
 	testutil "k8s.io/kubernetes/test/utils"
 )
@@ -156,7 +155,6 @@ func dcSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, *replicaset.R
 	resyncPeriod := 12 * time.Hour
 	informers := informers.NewSharedInformerFactory(clientset.NewForConfigOrDie(restclient.AddUserAgent(&config, "deployment-informers")), resyncPeriod)
 
-	metrics.UnregisterMetricAndUntrackRateLimiterUsage("deployment_controller")
 	dc, err := deployment.NewDeploymentController(
 		informers.Apps().V1().Deployments(),
 		informers.Apps().V1().ReplicaSets(),

test/integration/serviceaccount/BUILD

@@ -19,7 +19,6 @@ go_test(
         "//pkg/controller:go_default_library",
         "//pkg/controller/serviceaccount:go_default_library",
         "//pkg/serviceaccount:go_default_library",
-        "//pkg/util/metrics:go_default_library",
         "//plugin/pkg/admission/serviceaccount:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",

test/integration/serviceaccount/service_account_test.go

@@ -50,7 +50,6 @@ import (
 	"k8s.io/kubernetes/pkg/controller"
 	serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
 	"k8s.io/kubernetes/pkg/serviceaccount"
-	"k8s.io/kubernetes/pkg/util/metrics"
 	serviceaccountadmission "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
 	"k8s.io/kubernetes/test/integration/framework"
 )
@@ -437,7 +436,6 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie
 		apiServer.Close()
 	}
 
-	metrics.UnregisterMetricAndUntrackRateLimiterUsage("serviceaccount_tokens_controller")
 	tokenController, err := serviceaccountcontroller.NewTokensController(
 		informers.Core().V1().ServiceAccounts(),
 		informers.Core().V1().Secrets(),
@@ -449,7 +447,6 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie
 	}
 	go tokenController.Run(1, stopCh)
 
-	metrics.UnregisterMetricAndUntrackRateLimiterUsage("serviceaccount_controller")
 	serviceAccountController, err := serviceaccountcontroller.NewServiceAccountsController(
 		informers.Core().V1().ServiceAccounts(),
 		informers.Core().V1().Namespaces(),