diff --git a/pkg/framework/plugins/defaultevictor/defaultevictor.go b/pkg/framework/plugins/defaultevictor/defaultevictor.go index f7e635562..ff25ff5c0 100644 --- a/pkg/framework/plugins/defaultevictor/defaultevictor.go +++ b/pkg/framework/plugins/defaultevictor/defaultevictor.go @@ -244,6 +244,15 @@ func (d *DefaultEvictor) Filter(pod *v1.Pod) bool { func getPodIndexerByOwnerRefs(indexName string, handle frameworktypes.Handle) (cache.Indexer, error) { podInformer := handle.SharedInformerFactory().Core().V1().Pods().Informer() + indexer := podInformer.GetIndexer() + + // do not reinitialize the indexer, if it's been defined already + for name := range indexer.GetIndexers() { + if name == indexName { + return indexer, nil + } + } + if err := podInformer.AddIndexers(cache.Indexers{ indexName: func(obj interface{}) ([]string, error) { pod, ok := obj.(*v1.Pod) @@ -257,6 +266,5 @@ func getPodIndexerByOwnerRefs(indexName string, handle frameworktypes.Handle) (c return nil, err } - indexer := podInformer.GetIndexer() return indexer, nil } diff --git a/pkg/framework/plugins/defaultevictor/defaultevictor_test.go b/pkg/framework/plugins/defaultevictor/defaultevictor_test.go index 0cd3d804d..16dcad49d 100644 --- a/pkg/framework/plugins/defaultevictor/defaultevictor_test.go +++ b/pkg/framework/plugins/defaultevictor/defaultevictor_test.go @@ -15,6 +15,7 @@ package defaultevictor import ( "context" + "fmt" "testing" v1 "k8s.io/api/core/v1" @@ -31,6 +32,19 @@ import ( "sigs.k8s.io/descheduler/test" ) +type testCase struct { + description string + pods []*v1.Pod + nodes []*v1.Node + evictFailedBarePods bool + evictLocalStoragePods bool + evictSystemCriticalPods bool + priorityThreshold *int32 + nodeFit bool + minReplicas uint + result bool +} + func TestDefaultEvictorPreEvictionFilter(t *testing.T) { n1 := test.BuildTestNode("node1", 1000, 2000, 13, nil) @@ -39,17 +53,6 @@ func TestDefaultEvictorPreEvictionFilter(t *testing.T) { nodeLabelKey := "datacenter" nodeLabelValue := "east" - type testCase struct { - description string - pods []*v1.Pod - nodes []*v1.Node - evictFailedBarePods bool - evictLocalStoragePods bool - evictSystemCriticalPods bool - priorityThreshold *int32 - nodeFit bool - result bool - } testCases := []testCase{ { @@ -305,45 +308,7 @@ func TestDefaultEvictorPreEvictionFilter(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - var objs []runtime.Object - for _, node := range test.nodes { - objs = append(objs, node) - } - for _, pod := range test.pods { - objs = append(objs, pod) - } - - fakeClient := fake.NewSimpleClientset(objs...) - - sharedInformerFactory := informers.NewSharedInformerFactory(fakeClient, 0) - podInformer := sharedInformerFactory.Core().V1().Pods().Informer() - - getPodsAssignedToNode, err := podutil.BuildGetPodsAssignedToNodeFunc(podInformer) - if err != nil { - t.Errorf("Build get pods assigned to node function error: %v", err) - } - - sharedInformerFactory.Start(ctx.Done()) - sharedInformerFactory.WaitForCacheSync(ctx.Done()) - - defaultEvictorArgs := &DefaultEvictorArgs{ - EvictLocalStoragePods: test.evictLocalStoragePods, - EvictSystemCriticalPods: test.evictSystemCriticalPods, - IgnorePvcPods: false, - EvictFailedBarePods: test.evictFailedBarePods, - PriorityThreshold: &api.PriorityThreshold{ - Value: test.priorityThreshold, - }, - NodeFit: test.nodeFit, - } - - evictorPlugin, err := New( - defaultEvictorArgs, - &frameworkfake.HandleImpl{ - ClientsetImpl: fakeClient, - GetPodsAssignedToNodeFuncImpl: getPodsAssignedToNode, - SharedInformerFactoryImpl: sharedInformerFactory, - }) + evictorPlugin, err := initializePlugin(ctx, test) if err != nil { t.Fatalf("Unable to initialize the plugin: %v", err) } @@ -366,19 +331,6 @@ func TestDefaultEvictorFilter(t *testing.T) { ownerRefUUID := uuid.NewUUID() - type testCase struct { - description string - pods []*v1.Pod - nodes []*v1.Node - evictFailedBarePods bool - evictLocalStoragePods bool - evictSystemCriticalPods bool - priorityThreshold *int32 - nodeFit bool - minReplicas uint - result bool - } - testCases := []testCase{ { description: "Failed pod eviction with no ownerRefs", @@ -757,46 +709,7 @@ func TestDefaultEvictorFilter(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - var objs []runtime.Object - for _, node := range test.nodes { - objs = append(objs, node) - } - for _, pod := range test.pods { - objs = append(objs, pod) - } - - fakeClient := fake.NewSimpleClientset(objs...) - - sharedInformerFactory := informers.NewSharedInformerFactory(fakeClient, 0) - podInformer := sharedInformerFactory.Core().V1().Pods().Informer() - - getPodsAssignedToNode, err := podutil.BuildGetPodsAssignedToNodeFunc(podInformer) - if err != nil { - t.Errorf("Build get pods assigned to node function error: %v", err) - } - - sharedInformerFactory.Start(ctx.Done()) - sharedInformerFactory.WaitForCacheSync(ctx.Done()) - - defaultEvictorArgs := &DefaultEvictorArgs{ - EvictLocalStoragePods: test.evictLocalStoragePods, - EvictSystemCriticalPods: test.evictSystemCriticalPods, - IgnorePvcPods: false, - EvictFailedBarePods: test.evictFailedBarePods, - PriorityThreshold: &api.PriorityThreshold{ - Value: test.priorityThreshold, - }, - NodeFit: test.nodeFit, - MinReplicas: test.minReplicas, - } - - evictorPlugin, err := New( - defaultEvictorArgs, - &frameworkfake.HandleImpl{ - ClientsetImpl: fakeClient, - GetPodsAssignedToNodeFuncImpl: getPodsAssignedToNode, - SharedInformerFactoryImpl: sharedInformerFactory, - }) + evictorPlugin, err := initializePlugin(ctx, test) if err != nil { t.Fatalf("Unable to initialize the plugin: %v", err) } @@ -808,3 +721,94 @@ func TestDefaultEvictorFilter(t *testing.T) { }) } } + +func TestReinitialization(t *testing.T) { + n1 := test.BuildTestNode("node1", 1000, 2000, 13, nil) + ownerRefUUID := uuid.NewUUID() + + testCases := []testCase{ + { + description: "minReplicas of 2, multiple owners, eviction", + pods: []*v1.Pod{ + test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) { + pod.ObjectMeta.OwnerReferences = append(test.GetNormalPodOwnerRefList(), test.GetNormalPodOwnerRefList()...) + pod.ObjectMeta.OwnerReferences[0].UID = ownerRefUUID + }), + test.BuildTestPod("p2", 1, 1, n1.Name, func(pod *v1.Pod) { + pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList() + }), + }, + minReplicas: 2, + result: true, + }, + } + + for _, test := range testCases { + t.Run(test.description, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + evictorPlugin, err := initializePlugin(ctx, test) + if err != nil { + t.Fatalf("Unable to initialize the plugin: %v", err) + } + + defaultEvictor, ok := evictorPlugin.(*DefaultEvictor) + if !ok { + t.Fatalf("Unable to initialize as a DefaultEvictor plugin") + } + _, err = New(defaultEvictor.args, defaultEvictor.handle) + if err != nil { + t.Fatalf("Unable to reinitialize the plugin: %v", err) + } + }) + } +} + +func initializePlugin(ctx context.Context, test testCase) (frameworktypes.Plugin, error) { + var objs []runtime.Object + for _, node := range test.nodes { + objs = append(objs, node) + } + for _, pod := range test.pods { + objs = append(objs, pod) + } + + fakeClient := fake.NewSimpleClientset(objs...) + + sharedInformerFactory := informers.NewSharedInformerFactory(fakeClient, 0) + podInformer := sharedInformerFactory.Core().V1().Pods().Informer() + + getPodsAssignedToNode, err := podutil.BuildGetPodsAssignedToNodeFunc(podInformer) + if err != nil { + return nil, fmt.Errorf("build get pods assigned to node function error: %v", err) + } + + sharedInformerFactory.Start(ctx.Done()) + sharedInformerFactory.WaitForCacheSync(ctx.Done()) + + defaultEvictorArgs := &DefaultEvictorArgs{ + EvictLocalStoragePods: test.evictLocalStoragePods, + EvictSystemCriticalPods: test.evictSystemCriticalPods, + IgnorePvcPods: false, + EvictFailedBarePods: test.evictFailedBarePods, + PriorityThreshold: &api.PriorityThreshold{ + Value: test.priorityThreshold, + }, + NodeFit: test.nodeFit, + MinReplicas: test.minReplicas, + } + + evictorPlugin, err := New( + defaultEvictorArgs, + &frameworkfake.HandleImpl{ + ClientsetImpl: fakeClient, + GetPodsAssignedToNodeFuncImpl: getPodsAssignedToNode, + SharedInformerFactoryImpl: sharedInformerFactory, + }) + if err != nil { + return nil, fmt.Errorf("unable to initialize the plugin: %v", err) + } + + return evictorPlugin, nil +}