Merge pull request #1813 from ingvagabund/refactorings

refactor(pkg/descheduler): more handlers and dropping unused code

pkg/descheduler/descheduler.go

@@ -160,6 +160,17 @@ type kubeClientSandbox struct {
 	podEvictionReactionFnc func(*fakeclientset.Clientset, *evictedPodsCache) func(action core.Action) (bool, runtime.Object, error)
 }
 
+func newDefaultKubeClientSandbox(client clientset.Interface, sharedInformerFactory informers.SharedInformerFactory) (*kubeClientSandbox, error) {
+	return newKubeClientSandbox(client, sharedInformerFactory,
+		v1.SchemeGroupVersion.WithResource("pods"),
+		v1.SchemeGroupVersion.WithResource("nodes"),
+		v1.SchemeGroupVersion.WithResource("namespaces"),
+		schedulingv1.SchemeGroupVersion.WithResource("priorityclasses"),
+		policyv1.SchemeGroupVersion.WithResource("poddisruptionbudgets"),
+		v1.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
+	)
+}
+
 func newKubeClientSandbox(client clientset.Interface, sharedInformerFactory informers.SharedInformerFactory, resources ...schema.GroupVersionResource) (*kubeClientSandbox, error) {
 	sandbox := &kubeClientSandbox{
 		client:                 client,
@@ -267,19 +278,29 @@ func metricsProviderListToMap(providersList []api.MetricsProvider) map[api.Metri
 	return providersMap
 }
 
+// setupPrometheusProvider sets up the prometheus provider on the descheduler if configured
+func setupPrometheusProvider(d *descheduler, namespacedSharedInformerFactory informers.SharedInformerFactory) error {
+	prometheusProvider := d.metricsProviders[api.PrometheusMetrics]
+	if prometheusProvider != nil && prometheusProvider.Prometheus != nil && prometheusProvider.Prometheus.AuthToken != nil {
+		authTokenSecret := prometheusProvider.Prometheus.AuthToken.SecretReference
+		if authTokenSecret == nil || authTokenSecret.Namespace == "" {
+			return fmt.Errorf("prometheus metrics source configuration is missing authentication token secret")
+		}
+		if namespacedSharedInformerFactory == nil {
+			return fmt.Errorf("namespacedSharedInformerFactory not configured")
+		}
+		namespacedSharedInformerFactory.Core().V1().Secrets().Informer().AddEventHandler(d.eventHandler())
+		d.namespacedSecretsLister = namespacedSharedInformerFactory.Core().V1().Secrets().Lister().Secrets(authTokenSecret.Namespace)
+	}
+	return nil
+}
+
 func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedulerPolicy *api.DeschedulerPolicy, evictionPolicyGroupVersion string, eventRecorder events.EventRecorder, sharedInformerFactory, namespacedSharedInformerFactory informers.SharedInformerFactory) (*descheduler, error) {
 	podInformer := sharedInformerFactory.Core().V1().Pods().Informer()
 
 	// Future work could be to let each plugin declare what type of resources it needs; that way dry runs would stay
 	// consistent with the real runs without having to keep the list here in sync.
-	kubeClientSandbox, err := newKubeClientSandbox(rs.Client, sharedInformerFactory,
+	kubeClientSandbox, err := newDefaultKubeClientSandbox(rs.Client, sharedInformerFactory)
-		v1.SchemeGroupVersion.WithResource("pods"),
-		v1.SchemeGroupVersion.WithResource("nodes"),
-		v1.SchemeGroupVersion.WithResource("namespaces"),
-		schedulingv1.SchemeGroupVersion.WithResource("priorityclasses"),
-		policyv1.SchemeGroupVersion.WithResource("poddisruptionbudgets"),
-		v1.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
-	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create kube client sandbox: %v", err)
 	}
@@ -335,19 +356,6 @@ func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedu
 		desch.metricsCollector = metricscollector.NewMetricsCollector(sharedInformerFactory.Core().V1().Nodes().Lister(), rs.MetricsClient, nodeSelector)
 	}
 
-	prometheusProvider := desch.metricsProviders[api.PrometheusMetrics]
-	if prometheusProvider != nil && prometheusProvider.Prometheus != nil && prometheusProvider.Prometheus.AuthToken != nil {
-		authTokenSecret := prometheusProvider.Prometheus.AuthToken.SecretReference
-		if authTokenSecret == nil || authTokenSecret.Namespace == "" {
-			return nil, fmt.Errorf("prometheus metrics source configuration is missing authentication token secret")
-		}
-		if namespacedSharedInformerFactory == nil {
-			return nil, fmt.Errorf("namespacedSharedInformerFactory not configured")
-		}
-		namespacedSharedInformerFactory.Core().V1().Secrets().Informer().AddEventHandler(desch.eventHandler())
-		desch.namespacedSecretsLister = namespacedSharedInformerFactory.Core().V1().Secrets().Lister().Secrets(authTokenSecret.Namespace)
-	}
-
 	return desch, nil
 }
 
@@ -762,6 +770,12 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
+	// Setup Prometheus provider (only for real client case, not for dry run)
+	if err := setupPrometheusProvider(descheduler, namespacedSharedInformerFactory); err != nil {
+		span.AddEvent("Failed to setup Prometheus provider", trace.WithAttributes(attribute.String("err", err.Error())))
+		return err
+	}
+
 	sharedInformerFactory.Start(ctx.Done())
 	if metricProviderTokenReconciliation == secretReconciliation {
 		namespacedSharedInformerFactory.Start(ctx.Done())

pkg/descheduler/descheduler_test.go

@@ -202,6 +202,11 @@ func initDescheduler(t *testing.T, ctx context.Context, featureGates featuregate
 		t.Fatalf("Unable to create a descheduler instance: %v", err)
 	}
 
+	if err := setupPrometheusProvider(descheduler, nil); err != nil {
+		eventBroadcaster.Shutdown()
+		t.Fatalf("Failed to setup Prometheus provider: %v", err)
+	}
+
 	sharedInformerFactory.Start(ctx.Done())
 	sharedInformerFactory.WaitForCacheSync(ctx.Done())
 
@@ -642,11 +647,6 @@ func TestEvictionRequestsCache(t *testing.T) {
 	_, descheduler, client := initDescheduler(t, ctxCancel, featureGates, internalDeschedulerPolicy, nil, false, node1, node2, p1, p2, p3, p4)
 	defer cancel()
 
-	var fakeEvictedPods []string
-	descheduler.kubeClientSandbox.podEvictionReactionFnc = func(*fakeclientset.Clientset, *evictedPodsCache) func(action core.Action) (bool, runtime.Object, error) {
-		return podEvictionReactionTestingFnc(&fakeEvictedPods, nil, podEvictionError)
-	}
-
 	var evictedPods []string
 	client.PrependReactor("create", "pods", podEvictionReactionTestingFnc(&evictedPods, func(name string) bool { return name == "p1" || name == "p2" }, nil))
 
@@ -783,11 +783,6 @@ func TestDeschedulingLimits(t *testing.T) {
 			_, descheduler, client := initDescheduler(t, ctxCancel, featureGates, tc.policy, nil, false, node1, node2)
 			defer cancel()
 
-			var fakeEvictedPods []string
-			descheduler.kubeClientSandbox.podEvictionReactionFnc = func(*fakeclientset.Clientset, *evictedPodsCache) func(action core.Action) (bool, runtime.Object, error) {
-				return podEvictionReactionTestingFnc(&fakeEvictedPods, nil, podEvictionError)
-			}
-
 			var evictedPods []string
 			client.PrependReactor("create", "pods", podEvictionReactionTestingFnc(&evictedPods, func(name string) bool { return name == "p1" || name == "p2" }, nil))