Skip to content

Commit 1a54b30

Browse files
ingvagabundingvagabund
committed
refactor(pkg/descheduler): move the main descheduler run entry under a dedicated function
1 parent 7e80912 commit 1a54b30

File tree

2 files changed

+43
-38
lines changed

2 files changed

+43
-38
lines changed

‎pkg/descheduler/descheduler.go‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -581,24 +581,24 @@ func bootstrapDescheduler(
581581
aTConfig *authtokenConfig,
582582
sharedInformerFactory, namespacedSharedInformerFactory informers.SharedInformerFactory,
583583
eventRecorder events.EventRecorder,
584-
) (*descheduler, error) {
584+
) (*descheduler, func(context.Context) error, error) {
585585
// Always create descheduler with real client/factory first to register all informers
586586
descheduler, err := newDescheduler(ctx, rs, deschedulerPolicy, evictionPolicyGroupVersion, eventRecorder, rs.Client, sharedInformerFactory, nil)
587587
if err != nil {
588-
return nil, fmt.Errorf("failed to create new descheduler: %v", err)
588+
return nil, nil, fmt.Errorf("failed to create new descheduler: %v", err)
589589
}
590590

591591
// Setup Prometheus provider (only for real client case, not for dry run)
592592
if err := setupPrometheusProvider(descheduler, namespacedSharedInformerFactory); err != nil {
593-
return nil, fmt.Errorf("failed to setup Prometheus provider: %v", err)
593+
return nil, nil, fmt.Errorf("failed to setup Prometheus provider: %v", err)
594594
}
595595

596596
// If in dry run mode, replace the descheduler with one using fake client/factory
597597
if rs.DryRun {
598598
// Create sandbox with resources to mirror from real client
599599
kubeClientSandbox, err := newDefaultKubeClientSandbox(rs.Client, sharedInformerFactory)
600600
if err != nil {
601-
return nil, fmt.Errorf("failed to create kube client sandbox: %v", err)
601+
return nil, nil, fmt.Errorf("failed to create kube client sandbox: %v", err)
602602
}
603603

604604
klog.V(3).Infof("Building a cached client from the cluster for the dry run")
@@ -608,7 +608,7 @@ func bootstrapDescheduler(
608608
// Replace descheduler with one using fake client/factory
609609
descheduler, err = newDescheduler(ctx, rs, deschedulerPolicy, evictionPolicyGroupVersion, eventRecorder, kubeClientSandbox.fakeClient(), kubeClientSandbox.fakeSharedInformerFactory(), kubeClientSandbox)
610610
if err != nil {
611-
return nil, fmt.Errorf("failed to create dry run descheduler: %v", err)
611+
return nil, nil, fmt.Errorf("failed to create dry run descheduler: %v", err)
612612
}
613613
}
614614

@@ -656,10 +656,26 @@ func bootstrapDescheduler(
656656
}
657657

658658
if err := deschedulerInitFnc(ctx); err != nil {
659-
return nil, err
659+
return nil, nil, err
660660
}
661661

662-
return descheduler, nil
662+
run := func(ctx context.Context) error {
663+
if aTConfig.source == inClusterReconciliation {
664+
// Read the sa token and assume it has the sufficient permissions to authenticate
665+
if err := descheduler.promClientCtrl.reconcileInClusterSAToken(); err != nil {
666+
return fmt.Errorf("unable to reconcile an in cluster SA token: %v", err)
667+
}
668+
}
669+
670+
err = descheduler.runDeschedulerLoop(ctx)
671+
if err != nil {
672+
return fmt.Errorf("failed to run descheduler loop: %v", err)
673+
}
674+
675+
return nil
676+
}
677+
678+
return descheduler, run, nil
663679
}
664680

665681
func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer, deschedulerPolicy *api.DeschedulerPolicy, evictionPolicyGroupVersion string) error {
@@ -688,28 +704,19 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer
688704
namespacedSharedInformerFactory = informers.NewSharedInformerFactoryWithOptions(rs.Client, 0, informers.WithTransform(trimManagedFields), informers.WithNamespace(aTConfig.config.AuthToken.SecretReference.Namespace))
689705
}
690706

691-
descheduler, err := bootstrapDescheduler(ctx, rs, deschedulerPolicy, evictionPolicyGroupVersion, aTConfig, sharedInformerFactory, namespacedSharedInformerFactory, eventRecorder)
707+
_, runLoop, err := bootstrapDescheduler(ctx, rs, deschedulerPolicy, evictionPolicyGroupVersion, aTConfig, sharedInformerFactory, namespacedSharedInformerFactory, eventRecorder)
692708
if err != nil {
693709
span.AddEvent("Failed to bootstrap a descheduler", trace.WithAttributes(attribute.String("err", err.Error())))
694710
return err
695711
}
696712

697713
wait.NonSlidingUntil(func() {
698-
if aTConfig.source == inClusterReconciliation {
699-
// Read the sa token and assume it has the sufficient permissions to authenticate
700-
if err := descheduler.promClientCtrl.reconcileInClusterSAToken(); err != nil {
701-
klog.ErrorS(err, "unable to reconcile an in cluster SA token")
702-
return
703-
}
704-
}
705-
706714
// A next context is created here intentionally to avoid nesting the spans via context.
707715
sCtx, sSpan := tracing.Tracer().Start(ctx, "NonSlidingUntil")
708716
defer sSpan.End()
709717

710-
err = descheduler.runDeschedulerLoop(sCtx)
711-
if err != nil {
712-
sSpan.AddEvent("Failed to run descheduler loop", trace.WithAttributes(attribute.String("err", err.Error())))
718+
if err := runLoop(sCtx); err != nil {
719+
sSpan.AddEvent("Descheduling loop failed", trace.WithAttributes(attribute.String("err", err.Error())))
713720
klog.Error(err)
714721
cancel()
715722
return

‎pkg/descheduler/descheduler_test.go‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func lowNodeUtilizationPolicy(thresholds, targetThresholds api.ResourceThreshold
202202
}
203203
}
204204

205-
func initDescheduler(t *testing.T, ctx context.Context, featureGates featuregate.FeatureGate, internalDeschedulerPolicy *api.DeschedulerPolicy, source tokenReconciliation, metricsClient metricsclient.Interface, dryRun bool, objects ...runtime.Object) (*options.DeschedulerServer, *descheduler, *fakeclientset.Clientset) {
205+
func initDescheduler(t *testing.T, ctx context.Context, featureGates featuregate.FeatureGate, internalDeschedulerPolicy *api.DeschedulerPolicy, source tokenReconciliation, metricsClient metricsclient.Interface, dryRun bool, objects ...runtime.Object) (*options.DeschedulerServer, *descheduler, func(context.Context) error, *fakeclientset.Clientset) {
206206
client := fakeclientset.NewSimpleClientset(objects...)
207207
eventClient := fakeclientset.NewSimpleClientset(objects...)
208208

@@ -231,7 +231,7 @@ func initDescheduler(t *testing.T, ctx context.Context, featureGates featuregate
231231
namespacedSharedInformerFactory = informers.NewSharedInformerFactoryWithOptions(client, 0, informers.WithNamespace(namespace))
232232
}
233233

234-
descheduler, err := bootstrapDescheduler(ctx, rs, internalDeschedulerPolicy, "v1", aTConfig, sharedInformerFactory, namespacedSharedInformerFactory, eventRecorder)
234+
descheduler, runFnc, err := bootstrapDescheduler(ctx, rs, internalDeschedulerPolicy, "v1", aTConfig, sharedInformerFactory, namespacedSharedInformerFactory, eventRecorder)
235235
if err != nil {
236236
eventBroadcaster.Shutdown()
237237
t.Fatalf("Failed to bootstrap a descheduler: %v", err)
@@ -261,7 +261,7 @@ func initDescheduler(t *testing.T, ctx context.Context, featureGates featuregate
261261
}
262262
}
263263

264-
return rs, descheduler, client
264+
return rs, descheduler, runFnc, client
265265
}
266266

267267
func TestTaintsUpdated(t *testing.T) {
@@ -582,7 +582,7 @@ func TestPodEvictorReset(t *testing.T) {
582582

583583
internalDeschedulerPolicy := removePodsViolatingNodeTaintsPolicy()
584584
ctxCancel, cancel := context.WithCancel(ctx)
585-
_, descheduler, client := initDescheduler(t, ctxCancel, initFeatureGates(), internalDeschedulerPolicy, noReconciliation, nil, tc.dryRun, node1, node2, p1, p2)
585+
_, descheduler, _, client := initDescheduler(t, ctxCancel, initFeatureGates(), internalDeschedulerPolicy, noReconciliation, nil, tc.dryRun, node1, node2, p1, p2)
586586
defer cancel()
587587

588588
var evictedPods []string
@@ -675,7 +675,7 @@ func TestEvictionRequestsCache(t *testing.T) {
675675
featureGates.Add(map[featuregate.Feature]featuregate.FeatureSpec{
676676
features.EvictionsInBackground: {Default: true, PreRelease: featuregate.Alpha},
677677
})
678-
_, descheduler, client := initDescheduler(t, ctxCancel, featureGates, internalDeschedulerPolicy, noReconciliation, nil, false, node1, node2, p1, p2, p3, p4)
678+
_, descheduler, _, client := initDescheduler(t, ctxCancel, featureGates, internalDeschedulerPolicy, noReconciliation, nil, false, node1, node2, p1, p2, p3, p4)
679679
defer cancel()
680680

681681
var evictedPods []string
@@ -811,7 +811,7 @@ func TestDeschedulingLimits(t *testing.T) {
811811
featureGates.Add(map[featuregate.Feature]featuregate.FeatureSpec{
812812
features.EvictionsInBackground: {Default: true, PreRelease: featuregate.Alpha},
813813
})
814-
_, descheduler, client := initDescheduler(t, ctxCancel, featureGates, tc.policy, noReconciliation, nil, false, node1, node2)
814+
_, descheduler, _, client := initDescheduler(t, ctxCancel, featureGates, tc.policy, noReconciliation, nil, false, node1, node2)
815815
defer cancel()
816816

817817
var evictedPods []string
@@ -1009,7 +1009,7 @@ func TestNodeLabelSelectorBasedEviction(t *testing.T) {
10091009
}
10101010

10111011
ctxCancel, cancel := context.WithCancel(ctx)
1012-
_, deschedulerInstance, client := initDescheduler(t, ctxCancel, initFeatureGates(), policy, noReconciliation, nil, tc.dryRun, objects...)
1012+
_, deschedulerInstance, _, client := initDescheduler(t, ctxCancel, initFeatureGates(), policy, noReconciliation, nil, tc.dryRun, objects...)
10131013
defer cancel()
10141014

10151015
// Verify all pods are created initially
@@ -1123,7 +1123,7 @@ func TestLoadAwareDescheduling(t *testing.T) {
11231123
policy.MetricsProviders = []api.MetricsProvider{{Source: api.KubernetesMetrics}}
11241124

11251125
ctxCancel, cancel := context.WithCancel(ctx)
1126-
_, descheduler, _ := initDescheduler(
1126+
_, descheduler, _, _ := initDescheduler(
11271127
t,
11281128
ctxCancel,
11291129
initFeatureGates(),
@@ -1506,7 +1506,7 @@ func TestPluginPrometheusClientAccess(t *testing.T) {
15061506
node1 := test.BuildTestNode("node1", 1000, 2000, 9, nil)
15071507
node2 := test.BuildTestNode("node2", 1000, 2000, 9, nil)
15081508

1509-
_, descheduler, _ := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, inClusterReconciliation, nil, tc.dryRun, node1, node2)
1509+
_, descheduler, runFnc, _ := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, inClusterReconciliation, nil, tc.dryRun, node1, node2)
15101510

15111511
// Test cycles with different Prometheus client values
15121512
cycles := []struct {
@@ -1552,18 +1552,16 @@ func TestPluginPrometheusClientAccess(t *testing.T) {
15521552
return &rest.Config{BearerToken: cycle.token}, nil
15531553
}
15541554

1555-
// Use reconcileInClusterSAToken to set the client
1556-
t.Logf("Setting descheduler.promClientCtrl.promClient from %v to %v", descheduler.promClientCtrl.promClient, cycle.client)
1557-
if err := descheduler.promClientCtrl.reconcileInClusterSAToken(); err != nil {
1558-
t.Fatalf("Failed to reconcile in-cluster SA token: %v", err)
1559-
}
1560-
15611555
newInvoked = false
15621556
reactorInvoked = false
15631557
prometheusClientFromPluginNewHandle = nil
15641558
prometheusClientFromReactor = nil
15651559

1566-
descheduler.runProfiles(ctx)
1560+
// Use reconcileInClusterSAToken to set the client
1561+
t.Logf("Setting descheduler.promClientCtrl.promClient from %v to %v", descheduler.promClientCtrl.promClient, cycle.client)
1562+
if err := runFnc(ctx); err != nil {
1563+
t.Fatalf("Failed to reconcile in-cluster SA token: %v", err)
1564+
}
15671565

15681566
t.Logf("After cycle %d: prometheusClientFromReactor=%v, descheduler.promClientCtrl.promClient=%v", i+1, prometheusClientFromReactor, descheduler.promClientCtrl.promClient)
15691567

@@ -1813,7 +1811,7 @@ func TestPromClientControllerSync_ClientCreation(t *testing.T) {
18131811
},
18141812
}
18151813

1816-
_, descheduler, _ := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, secretReconciliation, nil, false, objects...)
1814+
_, descheduler, _, _ := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, secretReconciliation, nil, false, objects...)
18171815
return descheduler.promClientCtrl
18181816
},
18191817
},
@@ -1984,7 +1982,7 @@ func TestPromClientControllerSync_EventHandler(t *testing.T) {
19841982
},
19851983
}
19861984

1987-
_, descheduler, client := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, secretReconciliation, nil, false)
1985+
_, descheduler, _, client := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, secretReconciliation, nil, false)
19881986
// The reconciler is already started by initDescheduler via bootstrapDescheduler
19891987

19901988
return descheduler.promClientCtrl, client, prometheusConfig.AuthToken.SecretReference.Namespace
@@ -2233,7 +2231,7 @@ func TestReconcileInClusterSAToken(t *testing.T) {
22332231
},
22342232
}
22352233

2236-
_, descheduler, _ := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, inClusterReconciliation, nil, false)
2234+
_, descheduler, _, _ := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, inClusterReconciliation, nil, false)
22372235

22382236
// Override the fields needed for this test
22392237
descheduler.promClientCtrl.currentPrometheusAuthToken = tc.currentAuthToken

0 commit comments

Comments
 (0)