From bbffb830b9f62b708ad0d232eb1a22713820cc05 Mon Sep 17 00:00:00 2001 From: googs1025 Date: Sun, 27 Oct 2024 01:01:16 +0800 Subject: [PATCH] feature(eviction): add event when EvictPod failed --- pkg/api/types.go | 4 + pkg/api/v1alpha2/types.go | 4 + pkg/api/v1alpha2/zz_generated.conversion.go | 2 + pkg/api/v1alpha2/zz_generated.deepcopy.go | 5 + pkg/api/zz_generated.deepcopy.go | 5 + pkg/descheduler/descheduler.go | 1 + pkg/descheduler/evictions/evictions.go | 66 +++-- pkg/descheduler/evictions/evictions_test.go | 306 ++++++++++++++++---- pkg/descheduler/evictions/options.go | 20 +- 9 files changed, 319 insertions(+), 94 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index f282d0fe3d..041c13e12c 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -42,6 +42,10 @@ type DeschedulerPolicy struct { // MaxNoOfPodsToTotal restricts maximum of pods to be evicted total. MaxNoOfPodsToEvictTotal *uint + // EvictionFailureEventNotification should be set to true to enable eviction failure event notification. + // Default is false. + EvictionFailureEventNotification *bool + // MetricsCollector configures collection of metrics about actual resource utilization MetricsCollector MetricsCollector } diff --git a/pkg/api/v1alpha2/types.go b/pkg/api/v1alpha2/types.go index 702dc16fde..67d4845f5d 100644 --- a/pkg/api/v1alpha2/types.go +++ b/pkg/api/v1alpha2/types.go @@ -41,6 +41,10 @@ type DeschedulerPolicy struct { // MaxNoOfPodsToTotal restricts maximum of pods to be evicted total. MaxNoOfPodsToEvictTotal *uint `json:"maxNoOfPodsToEvictTotal,omitempty"` + // EvictionFailureEventNotification should be set to true to enable eviction failure event notification. + // Default is false. + EvictionFailureEventNotification *bool + // MetricsCollector configures collection of metrics for actual resource utilization MetricsCollector MetricsCollector `json:"metricsCollector,omitempty"` } diff --git a/pkg/api/v1alpha2/zz_generated.conversion.go b/pkg/api/v1alpha2/zz_generated.conversion.go index 1b33be25cf..340617c2ca 100644 --- a/pkg/api/v1alpha2/zz_generated.conversion.go +++ b/pkg/api/v1alpha2/zz_generated.conversion.go @@ -115,6 +115,7 @@ func autoConvert_v1alpha2_DeschedulerPolicy_To_api_DeschedulerPolicy(in *Desched out.MaxNoOfPodsToEvictPerNode = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictPerNode)) out.MaxNoOfPodsToEvictPerNamespace = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictPerNamespace)) out.MaxNoOfPodsToEvictTotal = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictTotal)) + out.EvictionFailureEventNotification = (*bool)(unsafe.Pointer(in.EvictionFailureEventNotification)) if err := Convert_v1alpha2_MetricsCollector_To_api_MetricsCollector(&in.MetricsCollector, &out.MetricsCollector, s); err != nil { return err } @@ -137,6 +138,7 @@ func autoConvert_api_DeschedulerPolicy_To_v1alpha2_DeschedulerPolicy(in *api.Des out.MaxNoOfPodsToEvictPerNode = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictPerNode)) out.MaxNoOfPodsToEvictPerNamespace = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictPerNamespace)) out.MaxNoOfPodsToEvictTotal = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictTotal)) + out.EvictionFailureEventNotification = (*bool)(unsafe.Pointer(in.EvictionFailureEventNotification)) if err := Convert_api_MetricsCollector_To_v1alpha2_MetricsCollector(&in.MetricsCollector, &out.MetricsCollector, s); err != nil { return err } diff --git a/pkg/api/v1alpha2/zz_generated.deepcopy.go b/pkg/api/v1alpha2/zz_generated.deepcopy.go index bfaf198785..599839c0b9 100644 --- a/pkg/api/v1alpha2/zz_generated.deepcopy.go +++ b/pkg/api/v1alpha2/zz_generated.deepcopy.go @@ -56,6 +56,11 @@ func (in *DeschedulerPolicy) DeepCopyInto(out *DeschedulerPolicy) { *out = new(uint) **out = **in } + if in.EvictionFailureEventNotification != nil { + in, out := &in.EvictionFailureEventNotification, &out.EvictionFailureEventNotification + *out = new(bool) + **out = **in + } out.MetricsCollector = in.MetricsCollector return } diff --git a/pkg/api/zz_generated.deepcopy.go b/pkg/api/zz_generated.deepcopy.go index 2ac45bb6a8..0f34df15b7 100644 --- a/pkg/api/zz_generated.deepcopy.go +++ b/pkg/api/zz_generated.deepcopy.go @@ -56,6 +56,11 @@ func (in *DeschedulerPolicy) DeepCopyInto(out *DeschedulerPolicy) { *out = new(uint) **out = **in } + if in.EvictionFailureEventNotification != nil { + in, out := &in.EvictionFailureEventNotification, &out.EvictionFailureEventNotification + *out = new(bool) + **out = **in + } out.MetricsCollector = in.MetricsCollector return } diff --git a/pkg/descheduler/descheduler.go b/pkg/descheduler/descheduler.go index 212f9cdbbf..75b0bd8009 100644 --- a/pkg/descheduler/descheduler.go +++ b/pkg/descheduler/descheduler.go @@ -156,6 +156,7 @@ func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedu WithMaxPodsToEvictPerNode(deschedulerPolicy.MaxNoOfPodsToEvictPerNode). WithMaxPodsToEvictPerNamespace(deschedulerPolicy.MaxNoOfPodsToEvictPerNamespace). WithMaxPodsToEvictTotal(deschedulerPolicy.MaxNoOfPodsToEvictTotal). + WithEvictionFailureEventNotification(deschedulerPolicy.EvictionFailureEventNotification). WithDryRun(rs.DryRun). WithMetricsEnabled(!rs.DisableMetrics), ) diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index b5857ba9d7..df1060fdba 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -206,20 +206,21 @@ type ( ) type PodEvictor struct { - mu sync.RWMutex - client clientset.Interface - policyGroupVersion string - dryRun bool - maxPodsToEvictPerNode *uint - maxPodsToEvictPerNamespace *uint - maxPodsToEvictTotal *uint - nodePodCount nodePodEvictedCount - namespacePodCount namespacePodEvictCount - totalPodCount uint - metricsEnabled bool - eventRecorder events.EventRecorder - erCache *evictionRequestsCache - featureGates featuregate.FeatureGate + mu sync.RWMutex + client clientset.Interface + policyGroupVersion string + dryRun bool + evictionFailureEventNotification bool + maxPodsToEvictPerNode *uint + maxPodsToEvictPerNamespace *uint + maxPodsToEvictTotal *uint + nodePodCount nodePodEvictedCount + namespacePodCount namespacePodEvictCount + totalPodCount uint + metricsEnabled bool + eventRecorder events.EventRecorder + erCache *evictionRequestsCache + featureGates featuregate.FeatureGate // registeredHandlers contains the registrations of all handlers. It's used to check if all handlers have finished syncing before the scheduling cycles start. registeredHandlers []cache.ResourceEventHandlerRegistration @@ -238,17 +239,18 @@ func NewPodEvictor( } podEvictor := &PodEvictor{ - client: client, - eventRecorder: eventRecorder, - policyGroupVersion: options.policyGroupVersion, - dryRun: options.dryRun, - maxPodsToEvictPerNode: options.maxPodsToEvictPerNode, - maxPodsToEvictPerNamespace: options.maxPodsToEvictPerNamespace, - maxPodsToEvictTotal: options.maxPodsToEvictTotal, - metricsEnabled: options.metricsEnabled, - nodePodCount: make(nodePodEvictedCount), - namespacePodCount: make(namespacePodEvictCount), - featureGates: featureGates, + client: client, + eventRecorder: eventRecorder, + policyGroupVersion: options.policyGroupVersion, + dryRun: options.dryRun, + evictionFailureEventNotification: options.evictionFailureEventNotification, + maxPodsToEvictPerNode: options.maxPodsToEvictPerNode, + maxPodsToEvictPerNamespace: options.maxPodsToEvictPerNamespace, + maxPodsToEvictTotal: options.maxPodsToEvictTotal, + metricsEnabled: options.metricsEnabled, + nodePodCount: make(nodePodEvictedCount), + namespacePodCount: make(namespacePodEvictCount), + featureGates: featureGates, } if featureGates.Enabled(features.EvictionsInBackground) { @@ -481,6 +483,9 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, opts EvictOptio } span.AddEvent("Eviction Failed", trace.WithAttributes(attribute.String("node", pod.Spec.NodeName), attribute.String("err", err.Error()))) klog.ErrorS(err, "Error evicting pod", "limit", *pe.maxPodsToEvictTotal) + if pe.evictionFailureEventNotification { + pe.eventRecorder.Eventf(pod, nil, v1.EventTypeWarning, "EvictionFailed", "Descheduled", "pod eviction from %v node by sigs.k8s.io/descheduler failed: total eviction limit exceeded (%v)", pod.Spec.NodeName, *pe.maxPodsToEvictTotal) + } return err } @@ -492,6 +497,9 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, opts EvictOptio } span.AddEvent("Eviction Failed", trace.WithAttributes(attribute.String("node", pod.Spec.NodeName), attribute.String("err", err.Error()))) klog.ErrorS(err, "Error evicting pod", "limit", *pe.maxPodsToEvictPerNode, "node", pod.Spec.NodeName) + if pe.evictionFailureEventNotification { + pe.eventRecorder.Eventf(pod, nil, v1.EventTypeWarning, "EvictionFailed", "Descheduled", "pod eviction from %v node by sigs.k8s.io/descheduler failed: node eviction limit exceeded (%v)", pod.Spec.NodeName, *pe.maxPodsToEvictPerNode) + } return err } } @@ -503,6 +511,9 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, opts EvictOptio } span.AddEvent("Eviction Failed", trace.WithAttributes(attribute.String("node", pod.Spec.NodeName), attribute.String("err", err.Error()))) klog.ErrorS(err, "Error evicting pod", "limit", *pe.maxPodsToEvictPerNamespace, "namespace", pod.Namespace, "pod", klog.KObj(pod)) + if pe.evictionFailureEventNotification { + pe.eventRecorder.Eventf(pod, nil, v1.EventTypeWarning, "EvictionFailed", "Descheduled", "pod eviction from %v node by sigs.k8s.io/descheduler failed: namespace eviction limit exceeded (%v)", pod.Spec.NodeName, *pe.maxPodsToEvictPerNamespace) + } return err } @@ -514,6 +525,9 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, opts EvictOptio if pe.metricsEnabled { metrics.PodsEvicted.With(map[string]string{"result": "error", "strategy": opts.StrategyName, "namespace": pod.Namespace, "node": pod.Spec.NodeName, "profile": opts.ProfileName}).Inc() } + if pe.evictionFailureEventNotification { + pe.eventRecorder.Eventf(pod, nil, v1.EventTypeWarning, "EvictionFailed", "Descheduled", "pod eviction from %v node by sigs.k8s.io/descheduler failed: %v", pod.Spec.NodeName, err.Error()) + } return err } @@ -542,7 +556,7 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, opts EvictOptio reason = "NotSet" } } - pe.eventRecorder.Eventf(pod, nil, v1.EventTypeNormal, reason, "Descheduled", "pod evicted from %v node by sigs.k8s.io/descheduler", pod.Spec.NodeName) + pe.eventRecorder.Eventf(pod, nil, v1.EventTypeNormal, reason, "Descheduled", "pod eviction from %v node by sigs.k8s.io/descheduler", pod.Spec.NodeName) } return nil } diff --git a/pkg/descheduler/evictions/evictions_test.go b/pkg/descheduler/evictions/evictions_test.go index 6b72005aca..b210ad0eb8 100644 --- a/pkg/descheduler/evictions/evictions_test.go +++ b/pkg/descheduler/evictions/evictions_test.go @@ -19,6 +19,7 @@ package evictions import ( "context" "fmt" + "reflect" "testing" "time" @@ -28,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" fakeclientset "k8s.io/client-go/kubernetes/fake" @@ -43,6 +45,11 @@ import ( "sigs.k8s.io/descheduler/test" ) +const ( + notFoundText = "pod not found when evicting \"%s\": pods \"%s\" not found" + tooManyRequests = "error when evicting pod (ignoring) \"%s\": Too many requests: too many requests" +) + func initFeatureGates() featuregate.FeatureGate { featureGates := featuregate.NewFeatureGate() featureGates.Add(map[featuregate.Feature]featuregate.FeatureSpec{ @@ -57,32 +64,38 @@ func TestEvictPod(t *testing.T) { tests := []struct { description string node *v1.Node - pod *v1.Pod - pods []v1.Pod - want error + evictedPod *v1.Pod + pods []runtime.Object + wantErr error }{ { description: "test pod eviction - pod present", node: node1, - pod: pod1, - pods: []v1.Pod{*pod1}, - want: nil, + evictedPod: pod1, + pods: []runtime.Object{pod1}, + }, + { + description: "test pod eviction - pod absent (not found error)", + node: node1, + evictedPod: pod1, + pods: []runtime.Object{test.BuildTestPod("p2", 400, 0, "node1", nil), test.BuildTestPod("p3", 450, 0, "node1", nil)}, + wantErr: fmt.Errorf(notFoundText, pod1.Name, pod1.Name), }, { - description: "test pod eviction - pod absent", + description: "test pod eviction - pod absent (too many requests error)", node: node1, - pod: pod1, - pods: []v1.Pod{*test.BuildTestPod("p2", 400, 0, "node1", nil), *test.BuildTestPod("p3", 450, 0, "node1", nil)}, - want: nil, + evictedPod: pod1, + pods: []runtime.Object{test.BuildTestPod("p2", 400, 0, "node1", nil), test.BuildTestPod("p3", 450, 0, "node1", nil)}, + wantErr: fmt.Errorf(tooManyRequests, pod1.Name), }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { ctx := context.Background() - fakeClient := &fake.Clientset{} - fakeClient.Fake.AddReactor("list", "pods", func(action core.Action) (bool, runtime.Object, error) { - return true, &v1.PodList{Items: test.pods}, nil + fakeClient := fake.NewClientset(test.pods...) + fakeClient.PrependReactor("create", "pods/eviction", func(action core.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, test.wantErr }) sharedInformerFactory := informers.NewSharedInformerFactory(fakeClient, 0) sharedInformerFactory.Start(ctx.Done()) @@ -101,9 +114,9 @@ func TestEvictPod(t *testing.T) { t.Fatalf("Unexpected error when creating a pod evictor: %v", err) } - _, got := podEvictor.evictPod(ctx, test.pod) - if got != test.want { - t.Errorf("Test error for Desc: %s. Expected %v pod eviction to be %v, got %v", test.description, test.pod.Name, test.want, got) + _, got := podEvictor.evictPod(ctx, test.evictedPod) + if got != test.wantErr { + t.Errorf("Test error for Desc: %s. Expected %v pod eviction to be %v, got %v", test.description, test.evictedPod.Name, test.wantErr, got) } }) } @@ -158,60 +171,205 @@ func TestNewPodEvictor(t *testing.T) { ctx := context.Background() pod1 := test.BuildTestPod("pod", 400, 0, "node", nil) + type podEvictorTest struct { + description string + pod *v1.Pod + dryRun bool + evictionFailureEventNotification *bool + maxPodsToEvictTotal *uint + maxPodsToEvictPerNode *uint + maxPodsToEvictPerNamespace *uint + expectedNodeEvictions uint + expectedTotalEvictions uint + expectedError error + // expectedEvent is a slice of strings representing expected events. + // Each string in the slice should follow the format: "EventType Reason Message". + // - "Warning Failed processing failed" + events []string + } + tests := []podEvictorTest{ + { + description: "one eviction expected with eviction failure event notification", + pod: pod1, + evictionFailureEventNotification: utilptr.To[bool](true), + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 1, + expectedTotalEvictions: 1, + expectedError: nil, + events: []string{"Normal NotSet pod eviction from node node by sigs.k8s.io/descheduler"}, + }, + { + description: "eviction limit exceeded on total with eviction failure event notification", + pod: pod1, + evictionFailureEventNotification: utilptr.To[bool](true), + maxPodsToEvictTotal: utilptr.To[uint](0), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 0, + expectedTotalEvictions: 0, + expectedError: NewEvictionTotalLimitError(), + events: []string{"Warning EvictionFailed pod eviction from node node by sigs.k8s.io/descheduler failed: total eviction limit exceeded (0)"}, + }, + { + description: "eviction limit exceeded on node with eviction failure event notification", + pod: pod1, + evictionFailureEventNotification: utilptr.To[bool](true), + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](0), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 0, + expectedTotalEvictions: 0, + expectedError: NewEvictionNodeLimitError("node"), + events: []string{"Warning EvictionFailed pod eviction from node node by sigs.k8s.io/descheduler failed: node eviction limit exceeded (0)"}, + }, + { + description: "eviction limit exceeded on node with eviction failure event notification", + pod: pod1, + evictionFailureEventNotification: utilptr.To[bool](true), + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](0), + expectedNodeEvictions: 0, + expectedTotalEvictions: 0, + expectedError: NewEvictionNamespaceLimitError("default"), + events: []string{"Warning EvictionFailed pod eviction from node node by sigs.k8s.io/descheduler failed: namespace eviction limit exceeded (0)"}, + }, + { + description: "eviction error with eviction failure event notification", + pod: pod1, + evictionFailureEventNotification: utilptr.To[bool](true), + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 0, + expectedTotalEvictions: 0, + expectedError: fmt.Errorf("eviction error"), + events: []string{"Warning EvictionFailed pod eviction from node node by sigs.k8s.io/descheduler failed: eviction error"}, + }, + { + description: "eviction with dryRun with eviction failure event notification", + pod: pod1, + dryRun: true, + evictionFailureEventNotification: utilptr.To[bool](true), + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 1, + expectedTotalEvictions: 1, + expectedError: nil, + }, + { + description: "one eviction expected without eviction failure event notification", + pod: pod1, + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 1, + expectedTotalEvictions: 1, + expectedError: nil, + events: []string{"Normal NotSet pod eviction from node node by sigs.k8s.io/descheduler"}, + }, + { + description: "eviction limit exceeded on total without eviction failure event notification", + pod: pod1, + maxPodsToEvictTotal: utilptr.To[uint](0), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 0, + expectedTotalEvictions: 0, + expectedError: NewEvictionTotalLimitError(), + }, + { + description: "eviction limit exceeded on node without eviction failure event notification", + pod: pod1, + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](0), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 0, + expectedTotalEvictions: 0, + expectedError: NewEvictionNodeLimitError("node"), + }, + { + description: "eviction limit exceeded on node without eviction failure event notification", + pod: pod1, + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](0), + expectedNodeEvictions: 0, + expectedTotalEvictions: 0, + expectedError: NewEvictionNamespaceLimitError("default"), + }, + { + description: "eviction error without eviction failure event notification", + pod: pod1, + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 0, + expectedTotalEvictions: 0, + expectedError: fmt.Errorf("eviction error"), + }, + { + description: "eviction without dryRun with eviction failure event notification", + pod: pod1, + dryRun: true, + maxPodsToEvictTotal: utilptr.To[uint](1), + maxPodsToEvictPerNode: utilptr.To[uint](1), + maxPodsToEvictPerNamespace: utilptr.To[uint](1), + expectedNodeEvictions: 1, + expectedTotalEvictions: 1, + expectedError: nil, + }, + } + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + fakeClient := fake.NewSimpleClientset(pod1) + fakeClient.PrependReactor("create", "pods/eviction", func(action core.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, test.expectedError + }) - fakeClient := fake.NewSimpleClientset(pod1) - - sharedInformerFactory := informers.NewSharedInformerFactory(fakeClient, 0) - sharedInformerFactory.Start(ctx.Done()) - sharedInformerFactory.WaitForCacheSync(ctx.Done()) + sharedInformerFactory := informers.NewSharedInformerFactory(fakeClient, 0) + sharedInformerFactory.Start(ctx.Done()) + sharedInformerFactory.WaitForCacheSync(ctx.Done()) - eventRecorder := &events.FakeRecorder{} + eventRecorder := events.NewFakeRecorder(100) - podEvictor, err := NewPodEvictor( - ctx, - fakeClient, - eventRecorder, - sharedInformerFactory.Core().V1().Pods().Informer(), - initFeatureGates(), - NewOptions().WithMaxPodsToEvictPerNode(utilptr.To[uint](1)), - ) - if err != nil { - t.Fatalf("Unexpected error when creating a pod evictor: %v", err) - } + podEvictor, err := NewPodEvictor( + ctx, + fakeClient, + eventRecorder, + sharedInformerFactory.Core().V1().Pods().Informer(), + initFeatureGates(), + NewOptions(). + WithDryRun(test.dryRun). + WithMaxPodsToEvictTotal(test.maxPodsToEvictTotal). + WithMaxPodsToEvictPerNode(test.maxPodsToEvictPerNode). + WithEvictionFailureEventNotification(test.evictionFailureEventNotification). + WithMaxPodsToEvictPerNamespace(test.maxPodsToEvictPerNamespace), + ) + if err != nil { + t.Fatalf("Unexpected error when creating a pod evictor: %v", err) + } - stubNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node"}} + stubNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node"}} - // 0 evictions expected - if evictions := podEvictor.NodeEvicted(stubNode); evictions != 0 { - t.Errorf("Expected 0 node evictions, got %q instead", evictions) - } - // 0 evictions expected - if evictions := podEvictor.TotalEvicted(); evictions != 0 { - t.Errorf("Expected 0 total evictions, got %q instead", evictions) - } + if actualErr := podEvictor.EvictPod(ctx, test.pod, EvictOptions{}); actualErr != nil && actualErr.Error() != test.expectedError.Error() { + t.Errorf("Expected error: %v, got: %v", test.expectedError, actualErr) + } - if err := podEvictor.EvictPod(context.TODO(), pod1, EvictOptions{}); err != nil { - t.Errorf("Expected a pod eviction, got an eviction error instead: %v", err) - } + if evictions := podEvictor.NodeEvicted(stubNode); evictions != test.expectedNodeEvictions { + t.Errorf("Expected %d node evictions, got %d instead", test.expectedNodeEvictions, evictions) + } - // 1 node eviction expected - if evictions := podEvictor.NodeEvicted(stubNode); evictions != 1 { - t.Errorf("Expected 1 node eviction, got %q instead", evictions) - } - // 1 total eviction expected - if evictions := podEvictor.TotalEvicted(); evictions != 1 { - t.Errorf("Expected 1 total evictions, got %q instead", evictions) - } + if evictions := podEvictor.TotalEvicted(); evictions != test.expectedTotalEvictions { + t.Errorf("Expected %d total evictions, got %d instead", test.expectedTotalEvictions, evictions) + } - err = podEvictor.EvictPod(context.TODO(), pod1, EvictOptions{}) - if err == nil { - t.Errorf("Expected a pod eviction error, got nil instead") - } - switch err.(type) { - case *EvictionNodeLimitError: - // all good - default: - t.Errorf("Expected a pod eviction EvictionNodeLimitError error, got a different error instead: %v", err) + // Assert that the events are correct. + assertEqualEvents(t, test.events, eventRecorder.Events) + }) } } @@ -300,3 +458,27 @@ func TestEvictionRequestsCacheCleanup(t *testing.T) { t.Fatalf("Expected 0 eviction requests, got %v instead", totalERs) } } + +func assertEqualEvents(t *testing.T, expected []string, actual <-chan string) { + t.Logf("Assert for events: %v", expected) + c := time.After(wait.ForeverTestTimeout) + for _, e := range expected { + select { + case a := <-actual: + if !reflect.DeepEqual(a, e) { + t.Errorf("Expected event %q, got %q instead", e, a) + } + case <-c: + t.Errorf("Expected event %q, got nothing", e) + // continue iterating to print all expected events + } + } + for { + select { + case a := <-actual: + t.Errorf("Unexpected event: %q", a) + default: + return // No more events, as expected. + } + } +} diff --git a/pkg/descheduler/evictions/options.go b/pkg/descheduler/evictions/options.go index 267854ee46..b7c52b5505 100644 --- a/pkg/descheduler/evictions/options.go +++ b/pkg/descheduler/evictions/options.go @@ -5,12 +5,13 @@ import ( ) type Options struct { - policyGroupVersion string - dryRun bool - maxPodsToEvictPerNode *uint - maxPodsToEvictPerNamespace *uint - maxPodsToEvictTotal *uint - metricsEnabled bool + policyGroupVersion string + dryRun bool + maxPodsToEvictPerNode *uint + maxPodsToEvictPerNamespace *uint + maxPodsToEvictTotal *uint + evictionFailureEventNotification bool + metricsEnabled bool } // NewOptions returns an Options with default values. @@ -49,3 +50,10 @@ func (o *Options) WithMetricsEnabled(metricsEnabled bool) *Options { o.metricsEnabled = metricsEnabled return o } + +func (o *Options) WithEvictionFailureEventNotification(evictionFailureEventNotification *bool) *Options { + if evictionFailureEventNotification != nil { + o.evictionFailureEventNotification = *evictionFailureEventNotification + } + return o +}