Skip to content

Commit 1832705

Browse files
ingvagabundingvagabund
committed
Addressing reviev comments
1 parent d7421d7 commit 1832705

File tree

4 files changed

+44
-39
lines changed

4 files changed

+44
-39
lines changed

‎README.md

+4-4
Original file line numberDiff line numberDiff line change
@@ -353,10 +353,10 @@ profiles:
353353
"cpu" : 50
354354
"memory": 50
355355
"pods": 50
356-
metricsUtilization:
357-
source: KubernetesMetrics
358-
# prometheus:
359-
# query: instance:node_cpu:rate:sum
356+
# metricsUtilization:
357+
# source: Prometheus
358+
# prometheus:
359+
# query: instance:node_cpu:rate:sum
360360
evictionLimits:
361361
node: 5
362362
plugins:

‎pkg/api/types.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ type ReferencedResourceList = map[v1.ResourceName]*resource.Quantity
142142
type Prometheus struct {
143143
URL string
144144
// authToken used for authentication with the prometheus server.
145-
// If not set the in cluster authentication token from the container's file system is read.
145+
// If not set the in cluster authentication token for the descheduler service
146+
// account is read from the container's file system is read.
146147
AuthToken *AuthToken
147148
}
148149

‎pkg/descheduler/descheduler.go

+33-30
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,14 @@ func (ir *informerResources) CopyTo(fakeClient *fakeclientset.Clientset, newFact
145145
return nil
146146
}
147147

148+
func metricsProviderListToMap(providersList []api.MetricsProvider) map[api.MetricsSource]*api.MetricsProvider {
149+
providersMap := make(map[api.MetricsSource]*api.MetricsProvider)
150+
for _, provider := range providersList {
151+
providersMap[provider.Source] = &provider
152+
}
153+
return providersMap
154+
}
155+
148156
func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedulerPolicy *api.DeschedulerPolicy, evictionPolicyGroupVersion string, eventRecorder events.EventRecorder, sharedInformerFactory, namespacedSharedInformerFactory informers.SharedInformerFactory) (*descheduler, error) {
149157
podInformer := sharedInformerFactory.Core().V1().Pods().Informer()
150158

@@ -198,9 +206,7 @@ func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedu
198206
metricsProviders: make(map[api.MetricsSource]*api.MetricsProvider),
199207
}
200208

201-
for _, provider := range deschedulerPolicy.MetricsProviders {
202-
desch.metricsProviders[provider.Source] = &provider
203-
}
209+
desch.metricsProviders = metricsProviderListToMap(deschedulerPolicy.MetricsProviders)
204210

205211
if rs.MetricsClient != nil {
206212
nodeSelector := labels.Everything()
@@ -215,12 +221,16 @@ func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedu
215221
}
216222

217223
prometheusProvider := desch.metricsProviders[api.PrometheusMetrics]
218-
if namespacedSharedInformerFactory != nil && prometheusProvider != nil {
219-
if prometheusProvider.Prometheus == nil || prometheusProvider.Prometheus.AuthToken == nil || prometheusProvider.Prometheus.AuthToken.SecretReference == nil {
224+
if prometheusProvider != nil && prometheusProvider.Prometheus != nil && prometheusProvider.Prometheus.AuthToken != nil {
225+
authTokenSecret := prometheusProvider.Prometheus.AuthToken.SecretReference
226+
if authTokenSecret == nil || authTokenSecret.Namespace == "" {
220227
return nil, fmt.Errorf("prometheus metrics source configuration is missing authentication token secret")
221228
}
229+
if namespacedSharedInformerFactory == nil {
230+
return nil, fmt.Errorf("namespacedSharedInformerFactory not configured")
231+
}
222232
namespacedSharedInformerFactory.Core().V1().Secrets().Informer().AddEventHandler(desch.eventHandler())
223-
desch.namespacedSecretsLister = namespacedSharedInformerFactory.Core().V1().Secrets().Lister().Secrets(desch.metricsProviders[api.PrometheusMetrics].Prometheus.AuthToken.SecretReference.Namespace)
233+
desch.namespacedSecretsLister = namespacedSharedInformerFactory.Core().V1().Secrets().Lister().Secrets(authTokenSecret.Namespace)
224234
}
225235

226236
return desch, nil
@@ -299,6 +309,10 @@ func (d *descheduler) sync() error {
299309
// clear the token if the secret is not found
300310
if apierrors.IsNotFound(err) {
301311
d.currentPrometheusAuthToken = ""
312+
if d.previousPrometheusClientTransport != nil {
313+
d.previousPrometheusClientTransport.CloseIdleConnections()
314+
}
315+
d.previousPrometheusClientTransport = nil
302316
}
303317
return fmt.Errorf("unable to get %v/%v secret", ns, name)
304318
}
@@ -474,13 +488,8 @@ func Run(ctx context.Context, rs *options.DeschedulerServer) error {
474488
return err
475489
}
476490

477-
hasKubernetesMetricsProvider := false
478-
for _, provider := range deschedulerPolicy.MetricsProviders {
479-
if provider.Source == api.KubernetesMetrics {
480-
hasKubernetesMetricsProvider = true
481-
break
482-
}
483-
}
491+
providersMap := metricsProviderListToMap(deschedulerPolicy.MetricsProviders)
492+
_, hasKubernetesMetricsProvider := providersMap[api.KubernetesMetrics]
484493
if (deschedulerPolicy.MetricsCollector != nil && deschedulerPolicy.MetricsCollector.Enabled) || hasKubernetesMetricsProvider {
485494
metricsClient, err := client.CreateMetricsClient(clientConnection, "descheduler")
486495
if err != nil {
@@ -594,23 +603,17 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer
594603
defer eventBroadcaster.Shutdown()
595604

596605
var namespacedSharedInformerFactory informers.SharedInformerFactory
597-
saTokenReconciliation := noReconciliation
598-
599-
var prometheusConfiguration *api.Prometheus
600-
for _, provider := range deschedulerPolicy.MetricsProviders {
601-
if provider.Source == api.PrometheusMetrics {
602-
prometheusConfiguration = provider.Prometheus
603-
}
604-
}
606+
metricProviderTokenReconciliation := noReconciliation
605607

606-
if prometheusConfiguration != nil && prometheusConfiguration.URL != "" {
607-
if prometheusConfiguration.AuthToken != nil {
608+
prometheusProvider := metricsProviderListToMap(deschedulerPolicy.MetricsProviders)[api.PrometheusMetrics]
609+
if prometheusProvider != nil && prometheusProvider.Prometheus != nil && prometheusProvider.Prometheus.URL != "" {
610+
if prometheusProvider.Prometheus.AuthToken != nil {
608611
// Will get reconciled
609-
namespacedSharedInformerFactory = informers.NewSharedInformerFactoryWithOptions(rs.Client, 0, informers.WithTransform(trimManagedFields), informers.WithNamespace(prometheusConfiguration.AuthToken.SecretReference.Namespace))
610-
saTokenReconciliation = secretReconciliation
612+
namespacedSharedInformerFactory = informers.NewSharedInformerFactoryWithOptions(rs.Client, 0, informers.WithTransform(trimManagedFields), informers.WithNamespace(prometheusProvider.Prometheus.AuthToken.SecretReference.Namespace))
613+
metricProviderTokenReconciliation = secretReconciliation
611614
} else {
612615
// Use the sa token and assume it has the sufficient permissions to authenticate
613-
saTokenReconciliation = inClusterReconciliation
616+
metricProviderTokenReconciliation = inClusterReconciliation
614617
}
615618
}
616619

@@ -623,13 +626,13 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer
623626
defer cancel()
624627

625628
sharedInformerFactory.Start(ctx.Done())
626-
if saTokenReconciliation == secretReconciliation {
629+
if metricProviderTokenReconciliation == secretReconciliation {
627630
namespacedSharedInformerFactory.Start(ctx.Done())
628631
}
629632

630633
sharedInformerFactory.WaitForCacheSync(ctx.Done())
631634
descheduler.podEvictor.WaitForEventHandlersSync(ctx)
632-
if saTokenReconciliation == secretReconciliation {
635+
if metricProviderTokenReconciliation == secretReconciliation {
633636
namespacedSharedInformerFactory.WaitForCacheSync(ctx.Done())
634637
}
635638

@@ -647,12 +650,12 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer
647650
}
648651
}
649652

650-
if saTokenReconciliation == secretReconciliation {
653+
if metricProviderTokenReconciliation == secretReconciliation {
651654
go descheduler.runAuthenticationSecretReconciler(ctx)
652655
}
653656

654657
wait.NonSlidingUntil(func() {
655-
if saTokenReconciliation == inClusterReconciliation {
658+
if metricProviderTokenReconciliation == inClusterReconciliation {
656659
// Read the sa token and assume it has the sufficient permissions to authenticate
657660
if err := descheduler.reconcileInClusterSAToken(); err != nil {
658661
klog.ErrorS(err, "unable to reconcile an in cluster SA token")

‎pkg/framework/plugins/nodeutilization/lownodeutilization.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,20 @@ func NewLowNodeUtilization(args runtime.Object, handle frameworktypes.Handle) (f
107107
var usageClient usageClient
108108
// MetricsServer is deprecated, removed once dropped
109109
if metricsUtilization != nil {
110-
if metricsUtilization.MetricsServer || metricsUtilization.Source == api.KubernetesMetrics {
110+
switch {
111+
case metricsUtilization.MetricsServer, metricsUtilization.Source == api.KubernetesMetrics:
111112
if handle.MetricsCollector() == nil {
112113
return nil, fmt.Errorf("metrics client not initialized")
113114
}
114115
usageClient = newActualUsageClient(resourceNames, handle.GetPodsAssignedToNodeFunc(), handle.MetricsCollector())
115-
} else if metricsUtilization.Source == api.PrometheusMetrics {
116+
case metricsUtilization.Source == api.PrometheusMetrics:
116117
if handle.PrometheusClient() == nil {
117118
return nil, fmt.Errorf("prometheus client not initialized")
118119
}
119120
usageClient = newPrometheusUsageClient(handle.GetPodsAssignedToNodeFunc(), handle.PrometheusClient(), metricsUtilization.Prometheus.Query)
120-
} else if metricsUtilization.Source != "" {
121+
case metricsUtilization.Source != "":
121122
return nil, fmt.Errorf("unrecognized metrics source")
122-
} else {
123+
default:
123124
return nil, fmt.Errorf("metrics source is empty")
124125
}
125126
} else {

0 commit comments

Comments
 (0)