Skip to content

Commit 282d3fc

Browse files
committed
feat(pkg/descheduler): create profiles outside the descheduling cycle
Each plugin is expected to be built only once within a profile. This way informers and indexers can be registered before the shared informer factory is started. Including registration of event handlers as well. Allowing plugins to take part in the descheduler initialization.
1 parent a500ff9 commit 282d3fc

File tree

2 files changed

+162
-25
lines changed

2 files changed

+162
-25
lines changed

pkg/descheduler/descheduler.go

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ type descheduler struct {
9494
queue workqueue.RateLimitingInterface
9595
currentPrometheusAuthToken string
9696
metricsProviders map[api.MetricsSource]*api.MetricsProvider
97+
profileRunners []profileRunner
9798
}
9899

99100
func nodeSelectorFromPolicy(deschedulerPolicy *api.DeschedulerPolicy) (labels.Selector, error) {
@@ -195,6 +196,29 @@ func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedu
195196
desch.metricsCollector = metricscollector.NewMetricsCollector(sharedInformerFactory.Core().V1().Nodes().Lister(), rs.MetricsClient, nodeSelector)
196197
}
197198

199+
var profileRunners []profileRunner
200+
for idx, profile := range deschedulerPolicy.Profiles {
201+
currProfile, err := frameworkprofile.NewProfile(
202+
ctx,
203+
profile,
204+
pluginregistry.PluginRegistry,
205+
frameworkprofile.WithClientSet(desch.client),
206+
frameworkprofile.WithSharedInformerFactory(desch.sharedInformerFactory),
207+
frameworkprofile.WithPodEvictor(desch.podEvictor),
208+
frameworkprofile.WithGetPodsAssignedToNodeFnc(desch.getPodsAssignedToNode),
209+
frameworkprofile.WithMetricsCollector(desch.metricsCollector),
210+
frameworkprofile.WithPrometheusClient(desch.prometheusClient),
211+
// Generate a unique instance ID using just the index to avoid long IDs
212+
// when profile names are very long
213+
frameworkprofile.WithProfileInstanceID(fmt.Sprintf("%d", idx)),
214+
)
215+
if err != nil {
216+
return nil, fmt.Errorf("unable to create a profile %q: %w", profile.Name, err)
217+
}
218+
profileRunners = append(profileRunners, profileRunner{profile.Name, currProfile.RunDeschedulePlugins, currProfile.RunBalancePlugins})
219+
}
220+
desch.profileRunners = profileRunners
221+
198222
return desch, nil
199223
}
200224

@@ -368,30 +392,7 @@ func (d *descheduler) runProfiles(ctx context.Context) {
368392
return // gracefully skip this cycle instead of aborting
369393
}
370394

371-
var profileRunners []profileRunner
372-
for idx, profile := range d.deschedulerPolicy.Profiles {
373-
currProfile, err := frameworkprofile.NewProfile(
374-
ctx,
375-
profile,
376-
pluginregistry.PluginRegistry,
377-
frameworkprofile.WithClientSet(d.client),
378-
frameworkprofile.WithSharedInformerFactory(d.sharedInformerFactory),
379-
frameworkprofile.WithPodEvictor(d.podEvictor),
380-
frameworkprofile.WithGetPodsAssignedToNodeFnc(d.getPodsAssignedToNode),
381-
frameworkprofile.WithMetricsCollector(d.metricsCollector),
382-
frameworkprofile.WithPrometheusClient(d.prometheusClient),
383-
// Generate a unique instance ID using just the index to avoid long IDs
384-
// when profile names are very long
385-
frameworkprofile.WithProfileInstanceID(fmt.Sprintf("%d", idx)),
386-
)
387-
if err != nil {
388-
klog.ErrorS(err, "unable to create a profile", "profile", profile.Name)
389-
continue
390-
}
391-
profileRunners = append(profileRunners, profileRunner{profile.Name, currProfile.RunDeschedulePlugins, currProfile.RunBalancePlugins})
392-
}
393-
394-
for _, profileR := range profileRunners {
395+
for _, profileR := range d.profileRunners {
395396
// First deschedule
396397
status := profileR.descheduleEPs(ctx, nodes)
397398
if status != nil && status.Err != nil {
@@ -401,7 +402,7 @@ func (d *descheduler) runProfiles(ctx context.Context) {
401402
}
402403
}
403404

404-
for _, profileR := range profileRunners {
405+
for _, profileR := range d.profileRunners {
405406
// Balance Later
406407
status := profileR.balanceEPs(ctx, nodes)
407408
if status != nil && status.Err != nil {

pkg/descheduler/descheduler_test.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"sigs.k8s.io/descheduler/pkg/api"
3737
"sigs.k8s.io/descheduler/pkg/descheduler/evictions"
3838
"sigs.k8s.io/descheduler/pkg/features"
39+
fakeplugin "sigs.k8s.io/descheduler/pkg/framework/fake/plugin"
3940
"sigs.k8s.io/descheduler/pkg/framework/pluginregistry"
4041
"sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor"
4142
"sigs.k8s.io/descheduler/pkg/framework/plugins/nodeutilization"
@@ -1406,3 +1407,138 @@ func TestEvictedPodRestorationInDryRun(t *testing.T) {
14061407
klog.Infof("Evicted pods cache was cleared after restoration in cycle %d", i)
14071408
}
14081409
}
1410+
1411+
// TestPluginInformerRegistration tests that plugin-specific informers are registered during newDescheduler
1412+
func TestPluginInformerRegistration(t *testing.T) {
1413+
testCases := []struct {
1414+
name string
1415+
dryRun bool
1416+
}{
1417+
{
1418+
name: "dry run disabled",
1419+
dryRun: false,
1420+
},
1421+
{
1422+
name: "dry run enabled",
1423+
dryRun: true,
1424+
},
1425+
}
1426+
1427+
for _, tc := range testCases {
1428+
t.Run(tc.name, func(t *testing.T) {
1429+
ctx := context.Background()
1430+
1431+
initPluginRegistry()
1432+
1433+
// Define the custom informers that should be registered by the plugin
1434+
customInformers := []schema.GroupVersionResource{
1435+
{Group: "apps", Version: "v1", Resource: "daemonsets"},
1436+
{Group: "apps", Version: "v1", Resource: "replicasets"},
1437+
{Group: "apps", Version: "v1", Resource: "statefulsets"},
1438+
}
1439+
1440+
callbackInvoked := false
1441+
fakePlugin := &fakeplugin.FakePlugin{
1442+
PluginName: "TestPluginWithInformers",
1443+
}
1444+
1445+
// Register our mock plugin using NewPluginFncFromFakeWithReactor
1446+
pluginregistry.Register(
1447+
fakePlugin.PluginName,
1448+
fakeplugin.NewPluginFncFromFakeWithReactor(fakePlugin, func(action fakeplugin.ActionImpl) {
1449+
callbackInvoked = true
1450+
// Register custom informers using the loop
1451+
for _, gvr := range customInformers {
1452+
_, err := action.Handle().SharedInformerFactory().ForResource(gvr)
1453+
if err != nil {
1454+
panic(fmt.Sprintf("Failed to register informer for %s: %v", gvr.Resource, err))
1455+
}
1456+
}
1457+
}),
1458+
&fakeplugin.FakePlugin{},
1459+
&fakeplugin.FakePluginArgs{},
1460+
fakeplugin.ValidateFakePluginArgs,
1461+
fakeplugin.SetDefaults_FakePluginArgs,
1462+
pluginregistry.PluginRegistry,
1463+
)
1464+
1465+
deschedulerPolicy := &api.DeschedulerPolicy{
1466+
Profiles: []api.DeschedulerProfile{
1467+
{
1468+
Name: "test-profile",
1469+
PluginConfigs: []api.PluginConfig{
1470+
{
1471+
Name: defaultevictor.PluginName,
1472+
Args: &defaultevictor.DefaultEvictorArgs{},
1473+
},
1474+
{
1475+
Name: fakePlugin.PluginName,
1476+
Args: &fakeplugin.FakePluginArgs{},
1477+
},
1478+
},
1479+
Plugins: api.Plugins{
1480+
Filter: api.PluginSet{
1481+
Enabled: []string{defaultevictor.PluginName},
1482+
},
1483+
PreEvictionFilter: api.PluginSet{
1484+
Enabled: []string{defaultevictor.PluginName},
1485+
},
1486+
Deschedule: api.PluginSet{
1487+
Enabled: []string{fakePlugin.PluginName},
1488+
},
1489+
},
1490+
},
1491+
},
1492+
}
1493+
1494+
node1 := test.BuildTestNode("node1", 1000, 2000, 9, nil)
1495+
node2 := test.BuildTestNode("node2", 1000, 2000, 9, nil)
1496+
1497+
_, descheduler, _ := initDescheduler(t, ctx, initFeatureGates(), deschedulerPolicy, nil, tc.dryRun, node1, node2)
1498+
1499+
if !callbackInvoked {
1500+
t.Fatal("Expected plugin initialization callback to be invoked")
1501+
}
1502+
1503+
// Verify that custom informers were registered in the SharedInformerFactory
1504+
for _, gvr := range customInformers {
1505+
informer, err := descheduler.sharedInformerFactory.ForResource(gvr)
1506+
if err != nil {
1507+
t.Errorf("Expected %s informer to be registered in SharedInformerFactory, got error: %v", gvr.Resource, err)
1508+
continue
1509+
}
1510+
1511+
if informer.Informer() == nil {
1512+
t.Errorf("Expected %s informer to be registered in SharedInformerFactory", gvr.Resource)
1513+
continue
1514+
}
1515+
1516+
// Verify the informer is cached (same instance when retrieved again)
1517+
var informer2 informers.GenericInformer
1518+
informer2, err = descheduler.sharedInformerFactory.ForResource(gvr)
1519+
if err != nil {
1520+
t.Errorf("Expected %s informer to be cached in factory, got error: %v", gvr.Resource, err)
1521+
continue
1522+
}
1523+
1524+
if informer.Informer() != informer2.Informer() {
1525+
t.Errorf("Expected %s informer to be cached in factory", gvr.Resource)
1526+
}
1527+
}
1528+
1529+
// Verify profileRunners were created
1530+
if len(descheduler.profileRunners) == 0 {
1531+
t.Fatal("Expected profileRunners to be created, got empty slice")
1532+
}
1533+
1534+
if len(descheduler.profileRunners) != 1 {
1535+
t.Fatalf("Expected 1 profileRunner, got %d", len(descheduler.profileRunners))
1536+
}
1537+
1538+
// Verify profile name
1539+
if descheduler.profileRunners[0].name != "test-profile" {
1540+
t.Errorf("Expected profile name to be 'test-profile', got '%s'", descheduler.profileRunners[0].name)
1541+
}
1542+
})
1543+
}
1544+
}

0 commit comments

Comments
 (0)