From 44b59f9b1d620633cf8d58eab92bd7192a1361fd Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Mon, 8 Jul 2024 16:55:27 +0200 Subject: [PATCH 1/4] initPluginRegistry as a single way to register all plugins in testing --- pkg/descheduler/descheduler_test.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/pkg/descheduler/descheduler_test.go b/pkg/descheduler/descheduler_test.go index a72621be90..5e83c1afc9 100644 --- a/pkg/descheduler/descheduler_test.go +++ b/pkg/descheduler/descheduler_test.go @@ -29,6 +29,13 @@ import ( "sigs.k8s.io/descheduler/test" ) +func initPluginRegistry() { + pluginregistry.PluginRegistry = pluginregistry.NewRegistry() + pluginregistry.Register(removeduplicates.PluginName, removeduplicates.New, &removeduplicates.RemoveDuplicates{}, &removeduplicates.RemoveDuplicatesArgs{}, removeduplicates.ValidateRemoveDuplicatesArgs, removeduplicates.SetDefaults_RemoveDuplicatesArgs, pluginregistry.PluginRegistry) + pluginregistry.Register(defaultevictor.PluginName, defaultevictor.New, &defaultevictor.DefaultEvictor{}, &defaultevictor.DefaultEvictorArgs{}, defaultevictor.ValidateDefaultEvictorArgs, defaultevictor.SetDefaults_DefaultEvictorArgs, pluginregistry.PluginRegistry) + pluginregistry.Register(removepodsviolatingnodetaints.PluginName, removepodsviolatingnodetaints.New, &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaints{}, &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaintsArgs{}, removepodsviolatingnodetaints.ValidateRemovePodsViolatingNodeTaintsArgs, removepodsviolatingnodetaints.SetDefaults_RemovePodsViolatingNodeTaintsArgs, pluginregistry.PluginRegistry) +} + // scope contains information about an ongoing conversion. type scope struct { converter *conversion.Converter @@ -46,9 +53,7 @@ func (s scope) Meta() *conversion.Meta { } func TestTaintsUpdated(t *testing.T) { - pluginregistry.PluginRegistry = pluginregistry.NewRegistry() - pluginregistry.Register(removepodsviolatingnodetaints.PluginName, removepodsviolatingnodetaints.New, &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaints{}, &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaintsArgs{}, removepodsviolatingnodetaints.ValidateRemovePodsViolatingNodeTaintsArgs, removepodsviolatingnodetaints.SetDefaults_RemovePodsViolatingNodeTaintsArgs, pluginregistry.PluginRegistry) - pluginregistry.Register(defaultevictor.PluginName, defaultevictor.New, &defaultevictor.DefaultEvictor{}, &defaultevictor.DefaultEvictorArgs{}, defaultevictor.ValidateDefaultEvictorArgs, defaultevictor.SetDefaults_DefaultEvictorArgs, pluginregistry.PluginRegistry) + initPluginRegistry() ctx := context.Background() n1 := test.BuildTestNode("n1", 2000, 3000, 10, nil) @@ -117,9 +122,7 @@ func TestTaintsUpdated(t *testing.T) { } func TestDuplicate(t *testing.T) { - pluginregistry.PluginRegistry = pluginregistry.NewRegistry() - pluginregistry.Register(removeduplicates.PluginName, removeduplicates.New, &removeduplicates.RemoveDuplicates{}, &removeduplicates.RemoveDuplicatesArgs{}, removeduplicates.ValidateRemoveDuplicatesArgs, removeduplicates.SetDefaults_RemoveDuplicatesArgs, pluginregistry.PluginRegistry) - pluginregistry.Register(defaultevictor.PluginName, defaultevictor.New, &defaultevictor.DefaultEvictor{}, &defaultevictor.DefaultEvictorArgs{}, defaultevictor.ValidateDefaultEvictorArgs, defaultevictor.SetDefaults_DefaultEvictorArgs, pluginregistry.PluginRegistry) + initPluginRegistry() ctx := context.Background() node1 := test.BuildTestNode("n1", 2000, 3000, 10, nil) @@ -327,12 +330,6 @@ func podEvictionReactionTestingFnc(evictedPods *[]string) func(action core.Actio } } -func initPluginRegistry() { - pluginregistry.PluginRegistry = pluginregistry.NewRegistry() - pluginregistry.Register(removeduplicates.PluginName, removeduplicates.New, &removeduplicates.RemoveDuplicates{}, &removeduplicates.RemoveDuplicatesArgs{}, removeduplicates.ValidateRemoveDuplicatesArgs, removeduplicates.SetDefaults_RemoveDuplicatesArgs, pluginregistry.PluginRegistry) - pluginregistry.Register(defaultevictor.PluginName, defaultevictor.New, &defaultevictor.DefaultEvictor{}, &defaultevictor.DefaultEvictorArgs{}, defaultevictor.ValidateDefaultEvictorArgs, defaultevictor.SetDefaults_DefaultEvictorArgs, pluginregistry.PluginRegistry) -} - func TestPodEvictorReset(t *testing.T) { initPluginRegistry() From a818c01832873c687de1c50de38c1f8d7e723704 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Mon, 8 Jul 2024 17:04:53 +0200 Subject: [PATCH 2/4] Use v1alpha2 descheduling policy --- pkg/descheduler/descheduler_test.go | 121 ++++++++++++++------------- pkg/descheduler/policyconfig_test.go | 17 ++++ 2 files changed, 80 insertions(+), 58 deletions(-) diff --git a/pkg/descheduler/descheduler_test.go b/pkg/descheduler/descheduler_test.go index 5e83c1afc9..17767064c1 100644 --- a/pkg/descheduler/descheduler_test.go +++ b/pkg/descheduler/descheduler_test.go @@ -9,7 +9,6 @@ import ( v1 "k8s.io/api/core/v1" policy "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/runtime" apiversion "k8s.io/apimachinery/pkg/version" fakediscovery "k8s.io/client-go/discovery/fake" @@ -18,7 +17,6 @@ import ( core "k8s.io/client-go/testing" "sigs.k8s.io/descheduler/cmd/descheduler/app/options" "sigs.k8s.io/descheduler/pkg/api" - "sigs.k8s.io/descheduler/pkg/api/v1alpha1" nodeutil "sigs.k8s.io/descheduler/pkg/descheduler/node" "sigs.k8s.io/descheduler/pkg/framework/pluginregistry" "sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor" @@ -36,20 +34,68 @@ func initPluginRegistry() { pluginregistry.Register(removepodsviolatingnodetaints.PluginName, removepodsviolatingnodetaints.New, &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaints{}, &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaintsArgs{}, removepodsviolatingnodetaints.ValidateRemovePodsViolatingNodeTaintsArgs, removepodsviolatingnodetaints.SetDefaults_RemovePodsViolatingNodeTaintsArgs, pluginregistry.PluginRegistry) } -// scope contains information about an ongoing conversion. -type scope struct { - converter *conversion.Converter - meta *conversion.Meta -} - -// Convert continues a conversion. -func (s scope) Convert(src, dest interface{}) error { - return s.converter.Convert(src, dest, s.meta) +func removePodsViolatingNodeTaintsPolicy() *api.DeschedulerPolicy { + return &api.DeschedulerPolicy{ + Profiles: []api.DeschedulerProfile{ + { + Name: "Profile", + PluginConfigs: []api.PluginConfig{ + { + Name: "RemovePodsViolatingNodeTaints", + Args: &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaintsArgs{}, + }, + { + Name: "DefaultEvictor", + Args: &defaultevictor.DefaultEvictorArgs{}, + }, + }, + Plugins: api.Plugins{ + Filter: api.PluginSet{ + Enabled: []string{ + "DefaultEvictor", + }, + }, + Deschedule: api.PluginSet{ + Enabled: []string{ + "RemovePodsViolatingNodeTaints", + }, + }, + }, + }, + }, + } } -// Meta returns the meta object that was originally passed to Convert. -func (s scope) Meta() *conversion.Meta { - return s.meta +func removeDuplicatesPolicy() *api.DeschedulerPolicy { + return &api.DeschedulerPolicy{ + Profiles: []api.DeschedulerProfile{ + { + Name: "Profile", + PluginConfigs: []api.PluginConfig{ + { + Name: "RemoveDuplicates", + Args: &removeduplicates.RemoveDuplicatesArgs{}, + }, + { + Name: "DefaultEvictor", + Args: &defaultevictor.DefaultEvictorArgs{}, + }, + }, + Plugins: api.Plugins{ + Filter: api.PluginSet{ + Enabled: []string{ + "DefaultEvictor", + }, + }, + Balance: api.PluginSet{ + Enabled: []string{ + "RemoveDuplicates", + }, + }, + }, + }, + }, + } } func TestTaintsUpdated(t *testing.T) { @@ -66,13 +112,6 @@ func TestTaintsUpdated(t *testing.T) { client := fakeclientset.NewSimpleClientset(n1, n2, p1) eventClient := fakeclientset.NewSimpleClientset(n1, n2, p1) - dp := &v1alpha1.DeschedulerPolicy{ - Strategies: v1alpha1.StrategyList{ - "RemovePodsViolatingNodeTaints": v1alpha1.DeschedulerStrategy{ - Enabled: true, - }, - }, - } rs, err := options.NewDeschedulerServer() if err != nil { @@ -105,14 +144,7 @@ func TestTaintsUpdated(t *testing.T) { var evictedPods []string client.PrependReactor("create", "pods", podEvictionReactionTestingFnc(&evictedPods)) - internalDeschedulerPolicy := &api.DeschedulerPolicy{} - scope := scope{} - err = v1alpha1.V1alpha1ToInternal(dp, pluginregistry.PluginRegistry, internalDeschedulerPolicy, scope) - if err != nil { - t.Fatalf("Unable to convert v1alpha1 to v1alpha2: %v", err) - } - - if err := RunDeschedulerStrategies(ctx, rs, internalDeschedulerPolicy, "v1"); err != nil { + if err := RunDeschedulerStrategies(ctx, rs, removePodsViolatingNodeTaintsPolicy(), "v1"); err != nil { t.Fatalf("Unable to run descheduler strategies: %v", err) } @@ -142,13 +174,6 @@ func TestDuplicate(t *testing.T) { client := fakeclientset.NewSimpleClientset(node1, node2, p1, p2, p3) eventClient := fakeclientset.NewSimpleClientset(node1, node2, p1, p2, p3) - dp := &v1alpha1.DeschedulerPolicy{ - Strategies: v1alpha1.StrategyList{ - "RemoveDuplicates": v1alpha1.DeschedulerStrategy{ - Enabled: true, - }, - }, - } rs, err := options.NewDeschedulerServer() if err != nil { @@ -169,13 +194,7 @@ func TestDuplicate(t *testing.T) { var evictedPods []string client.PrependReactor("create", "pods", podEvictionReactionTestingFnc(&evictedPods)) - internalDeschedulerPolicy := &api.DeschedulerPolicy{} - scope := scope{} - err = v1alpha1.V1alpha1ToInternal(dp, pluginregistry.PluginRegistry, internalDeschedulerPolicy, scope) - if err != nil { - t.Fatalf("Unable to convert v1alpha1 to v1alpha2: %v", err) - } - if err := RunDeschedulerStrategies(ctx, rs, internalDeschedulerPolicy, "v1"); err != nil { + if err := RunDeschedulerStrategies(ctx, rs, removeDuplicatesPolicy(), "v1"); err != nil { t.Fatalf("Unable to run descheduler strategies: %v", err) } @@ -354,20 +373,6 @@ func TestPodEvictorReset(t *testing.T) { client := fakeclientset.NewSimpleClientset(node1, node2, p1, p2, p3, p4) eventClient := fakeclientset.NewSimpleClientset(node1, node2, p1, p2, p3, p4) - dp := &v1alpha1.DeschedulerPolicy{ - Strategies: v1alpha1.StrategyList{ - "RemoveDuplicates": v1alpha1.DeschedulerStrategy{ - Enabled: true, - }, - }, - } - - internalDeschedulerPolicy := &api.DeschedulerPolicy{} - scope := scope{} - err := v1alpha1.V1alpha1ToInternal(dp, pluginregistry.PluginRegistry, internalDeschedulerPolicy, scope) - if err != nil { - t.Fatalf("Unable to convert v1alpha1 to v1alpha2: %v", err) - } rs, err := options.NewDeschedulerServer() if err != nil { @@ -383,7 +388,7 @@ func TestPodEvictorReset(t *testing.T) { eventBroadcaster, eventRecorder := utils.GetRecorderAndBroadcaster(ctx, client) defer eventBroadcaster.Shutdown() - descheduler, err := newDescheduler(rs, internalDeschedulerPolicy, "v1", eventRecorder, sharedInformerFactory) + descheduler, err := newDescheduler(rs, removeDuplicatesPolicy(), "v1", eventRecorder, sharedInformerFactory) if err != nil { t.Fatalf("Unable to create a descheduler instance: %v", err) } diff --git a/pkg/descheduler/policyconfig_test.go b/pkg/descheduler/policyconfig_test.go index 7f415150bd..023feb6f13 100644 --- a/pkg/descheduler/policyconfig_test.go +++ b/pkg/descheduler/policyconfig_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/conversion" fakeclientset "k8s.io/client-go/kubernetes/fake" utilpointer "k8s.io/utils/pointer" "sigs.k8s.io/descheduler/pkg/api" @@ -40,6 +41,22 @@ import ( "sigs.k8s.io/descheduler/pkg/utils" ) +// scope contains information about an ongoing conversion. +type scope struct { + converter *conversion.Converter + meta *conversion.Meta +} + +// Convert continues a conversion. +func (s scope) Convert(src, dest interface{}) error { + return s.converter.Convert(src, dest, s.meta) +} + +// Meta returns the meta object that was originally passed to Convert. +func (s scope) Meta() *conversion.Meta { + return s.meta +} + func TestV1alpha1ToV1alpha2(t *testing.T) { SetupPlugins() defaultEvictorPluginConfig := api.PluginConfig{ From f240648df28b00a3647354608998ec574ec6cb55 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Mon, 8 Jul 2024 17:10:07 +0200 Subject: [PATCH 3/4] Set OwnerReferences through GetReplicaSetOwnerRefList --- pkg/descheduler/descheduler_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/descheduler/descheduler_test.go b/pkg/descheduler/descheduler_test.go index 17767064c1..2a587c268a 100644 --- a/pkg/descheduler/descheduler_test.go +++ b/pkg/descheduler/descheduler_test.go @@ -106,9 +106,7 @@ func TestTaintsUpdated(t *testing.T) { n2 := test.BuildTestNode("n2", 2000, 3000, 10, nil) p1 := test.BuildTestPod(fmt.Sprintf("pod_1_%s", n1.Name), 200, 0, n1.Name, nil) - p1.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ - {}, - } + p1.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() client := fakeclientset.NewSimpleClientset(n1, n2, p1) eventClient := fakeclientset.NewSimpleClientset(n1, n2, p1) From 3362fec7b0caa4ea4f32e474d1add6df32336e6e Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Mon, 8 Jul 2024 17:28:49 +0200 Subject: [PATCH 4/4] Define initDescheduler for further use Can be used by other tests executing individual descheduling cycle explicitly. --- pkg/descheduler/descheduler_test.go | 56 +++++++++++++++++------------ 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/pkg/descheduler/descheduler_test.go b/pkg/descheduler/descheduler_test.go index 2a587c268a..bd8dae4781 100644 --- a/pkg/descheduler/descheduler_test.go +++ b/pkg/descheduler/descheduler_test.go @@ -98,6 +98,37 @@ func removeDuplicatesPolicy() *api.DeschedulerPolicy { } } +func initDescheduler(t *testing.T, ctx context.Context, internalDeschedulerPolicy *api.DeschedulerPolicy, objects ...runtime.Object) (*options.DeschedulerServer, *descheduler, *fakeclientset.Clientset, func()) { + client := fakeclientset.NewSimpleClientset(objects...) + eventClient := fakeclientset.NewSimpleClientset(objects...) + + rs, err := options.NewDeschedulerServer() + if err != nil { + t.Fatalf("Unable to initialize server: %v", err) + } + rs.Client = client + rs.EventClient = eventClient + + sharedInformerFactory := informers.NewSharedInformerFactoryWithOptions(rs.Client, 0, informers.WithTransform(trimManagedFields)) + eventBroadcaster, eventRecorder := utils.GetRecorderAndBroadcaster(ctx, client) + + descheduler, err := newDescheduler(rs, internalDeschedulerPolicy, "v1", eventRecorder, sharedInformerFactory) + if err != nil { + eventBroadcaster.Shutdown() + t.Fatalf("Unable to create a descheduler instance: %v", err) + } + + ctx, cancel := context.WithCancel(ctx) + + sharedInformerFactory.Start(ctx.Done()) + sharedInformerFactory.WaitForCacheSync(ctx.Done()) + + return rs, descheduler, client, func() { + cancel() + eventBroadcaster.Shutdown() + } +} + func TestTaintsUpdated(t *testing.T) { initPluginRegistry() @@ -369,38 +400,17 @@ func TestPodEvictorReset(t *testing.T) { p3.ObjectMeta.OwnerReferences = ownerRef1 p4.ObjectMeta.OwnerReferences = ownerRef1 - client := fakeclientset.NewSimpleClientset(node1, node2, p1, p2, p3, p4) - eventClient := fakeclientset.NewSimpleClientset(node1, node2, p1, p2, p3, p4) - - rs, err := options.NewDeschedulerServer() - if err != nil { - t.Fatalf("Unable to initialize server: %v", err) - } - rs.Client = client - rs.EventClient = eventClient + rs, descheduler, client, cancel := initDescheduler(t, ctx, removeDuplicatesPolicy(), node1, node2, p1, p2, p3, p4) + defer cancel() var evictedPods []string client.PrependReactor("create", "pods", podEvictionReactionTestingFnc(&evictedPods)) - sharedInformerFactory := informers.NewSharedInformerFactoryWithOptions(rs.Client, 0, informers.WithTransform(trimManagedFields)) - eventBroadcaster, eventRecorder := utils.GetRecorderAndBroadcaster(ctx, client) - defer eventBroadcaster.Shutdown() - - descheduler, err := newDescheduler(rs, removeDuplicatesPolicy(), "v1", eventRecorder, sharedInformerFactory) - if err != nil { - t.Fatalf("Unable to create a descheduler instance: %v", err) - } var fakeEvictedPods []string descheduler.podEvictionReactionFnc = func(*fakeclientset.Clientset) func(action core.Action) (bool, runtime.Object, error) { return podEvictionReactionTestingFnc(&fakeEvictedPods) } - ctx, cancel := context.WithCancel(ctx) - defer cancel() - - sharedInformerFactory.Start(ctx.Done()) - sharedInformerFactory.WaitForCacheSync(ctx.Done()) - nodes, err := nodeutil.ReadyNodes(ctx, rs.Client, descheduler.nodeLister, "") if err != nil { t.Fatalf("Unable to get ready nodes: %v", err)