Unregister prom gauges when recycling cluster watcher (#11875)

Unregister prom gauges when recycling cluster watcher

Fixes #11839

When in `restartClusterWatcher` we fail to connect to the target cluster
for whatever reason, the function gets called again 10s later, and tries
to register the same prometheus metrics without unregistering them
first, which generates warnings.

The problem lies in `NewRemoteClusterServiceWatcher`, which instantiates
the remote kube-api client and registers the metrics, returning a nil
object if the client can't connect. `cleanupWorkers` at the beginning of
`restartClusterWatcher` won't unregister those metrics because of that
nil object.

To fix this, gauges are unregistered on error.

Author: alpeb

controller/k8s/api.go

@@ -67,7 +67,10 @@ type API struct {
 	l5dCrdSharedInformers l5dcrdinformer.SharedInformerFactory
 }
 
-// InitializeAPI creates Kubernetes clients and returns an initialized API wrapper.
+// InitializeAPI creates Kubernetes clients and returns an initialized API
+// wrapper. This creates informers on each one of resources passed, registering
+// metrics on each one; don't forget to call UnregisterGauges() on the returned
+// API reference to clean them up!
 func InitializeAPI(ctx context.Context, kubeConfig string, ensureClusterWideAccess bool, cluster string, resources ...APIResource) (*API, error) {
 	config, err := k8s.GetConfig(kubeConfig, "")
 	if err != nil {
@@ -87,7 +90,10 @@ func InitializeAPI(ctx context.Context, kubeConfig string, ensureClusterWideAcce
 	return initAPI(ctx, k8sClient, dynamicClient, config, ensureClusterWideAccess, cluster, resources...)
 }
 
-// InitializeAPIForConfig creates Kubernetes clients and returns an initialized API wrapper.
+// InitializeAPIForConfig creates Kubernetes clients and returns an initialized
+// API wrapper. This creates informers on each one of resources passed,
+// registering metrics on each one; don't forget to call UnregisterGauges() on
+// the returned API reference to clean them up!
 func InitializeAPIForConfig(ctx context.Context, kubeConfig *rest.Config, ensureClusterWideAccess bool, cluster string, resources ...APIResource) (*API, error) {
 	k8sClient, err := k8s.NewAPIForConfig(kubeConfig, "", []string{}, 0, 0, 0)
 	if err != nil {
@@ -141,7 +147,10 @@ func initAPI(ctx context.Context, k8sClient *k8s.KubernetesAPI, dynamicClient dy
 	return api, nil
 }
 
-// NewClusterScopedAPI takes a Kubernetes client and returns an initialized cluster-wide API.
+// NewClusterScopedAPI takes a Kubernetes client and returns an initialized
+// cluster-wide API. This creates informers on each one of resources passed,
+// registering metrics on each one; don't forget to call UnregisterGauges() on
+// the returned API reference to clean them up!
 func NewClusterScopedAPI(
 	k8sClient kubernetes.Interface,
 	dynamicClient dynamic.Interface,
@@ -153,7 +162,10 @@ func NewClusterScopedAPI(
 	return newAPI(k8sClient, dynamicClient, l5dCrdClient, sharedInformers, cluster, resources...)
 }
 
-// NewNamespacedAPI takes a Kubernetes client and returns an initialized API scoped to namespace.
+// NewNamespacedAPI takes a Kubernetes client and returns an initialized API
+// scoped to namespace. This creates informers on each one of resources passed,
+// registering metrics on each one; don't forget to call UnregisterGauges() on
+// the returned API reference to clean them up!
 func NewNamespacedAPI(
 	k8sClient kubernetes.Interface,
 	dynamicClient dynamic.Interface,

multicluster/cmd/service-mirror/main.go

@@ -303,7 +303,7 @@ func restartClusterWatcher(
 	if err != nil {
 		return fmt.Errorf("unable to parse kube config: %w", err)
 	}
-	clusterWatcher, err = servicemirror.NewRemoteClusterServiceWatcher(
+	cw, err := servicemirror.NewRemoteClusterServiceWatcher(
 		ctx,
 		namespace,
 		controllerK8sAPI,
@@ -317,6 +317,7 @@ func restartClusterWatcher(
 	if err != nil {
 		return fmt.Errorf("unable to create cluster watcher: %w", err)
 	}
+	clusterWatcher = cw
 	err = clusterWatcher.Start(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to start cluster watcher: %w", err)

multicluster/service-mirror/cluster_watcher.go

@@ -175,6 +175,7 @@ func NewRemoteClusterServiceWatcher(
 	}
 	_, err = remoteAPI.Client.Discovery().ServerVersion()
 	if err != nil {
+		remoteAPI.UnregisterGauges()
 		return nil, fmt.Errorf("cannot connect to api for target cluster %s: %w", clusterName, err)
 	}