From 0ced9d5773b62a673b8bf19380a789e232e4ba99 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Sun, 27 Oct 2024 13:54:58 +0200 Subject: [PATCH 1/5] VPA: Create event for VPA object when Pod is evicted --- .../eviction/pods_eviction_restriction.go | 7 +- .../pods_eviction_restriction_test.go | 117 +++++++++++++++--- .../pkg/updater/logic/updater.go | 14 ++- .../pkg/updater/logic/updater_test.go | 49 ++++++++ .../pkg/utils/test/test_utils.go | 31 ++++- 5 files changed, 196 insertions(+), 22 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go index 74ddbe2ccf04..b2bc51886dce 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go @@ -44,7 +44,7 @@ const ( type PodsEvictionRestriction interface { // Evict sends eviction instruction to the api client. // Returns error if pod cannot be evicted or if client returned error. - Evict(pod *apiv1.Pod, eventRecorder record.EventRecorder) error + Evict(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error // CanEvict checks if pod can be safely evicted CanEvict(pod *apiv1.Pod) bool } @@ -122,7 +122,7 @@ func (e *podsEvictionRestrictionImpl) CanEvict(pod *apiv1.Pod) bool { // Evict sends eviction instruction to api client. Returns error if pod cannot be evicted or if client returned error // Does not check if pod was actually evicted after eviction grace period. -func (e *podsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, eventRecorder record.EventRecorder) error { +func (e *podsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error { cr, present := e.podToReplicaCreatorMap[getPodID(podToEvict)] if !present { return fmt.Errorf("pod not suitable for eviction %s/%s: not in replicated pods map", podToEvict.Namespace, podToEvict.Name) @@ -146,6 +146,9 @@ func (e *podsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, eventRecorder eventRecorder.Event(podToEvict, apiv1.EventTypeNormal, "EvictedByVPA", "Pod was evicted by VPA Updater to apply resource recommendation.") + eventRecorder.Event(vpa, apiv1.EventTypeNormal, "EvictedPod", + "VPA Updater evicted Pod "+podToEvict.Name+" to apply resource recommendation.") + if podToEvict.Status.Phase != apiv1.PodPending { singleGroupStats, present := e.creatorToSingleGroupStatsMap[cr] if !present { diff --git a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go index 5ac12b0aecb3..fa9b4cca06f2 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go @@ -22,6 +22,8 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" apiv1 "k8s.io/api/core/v1" @@ -281,7 +283,7 @@ func TestEvictReplicatedByController(t *testing.T) { assert.Equalf(t, p.canEvict, eviction.CanEvict(p.pod), "TC %v - unexpected CanEvict result for pod-%v %#v", testCase.name, i, p.pod) } for i, p := range testCase.pods { - err := eviction.Evict(p.pod, test.FakeEventRecorder()) + err := eviction.Evict(p.pod, testCase.vpa, test.FakeEventRecorder()) if p.evictionSuccess { assert.NoErrorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod) } else { @@ -289,7 +291,6 @@ func TestEvictReplicatedByController(t *testing.T) { } } } - } func TestEvictReplicatedByReplicaSet(t *testing.T) { @@ -322,11 +323,11 @@ func TestEvictReplicatedByReplicaSet(t *testing.T) { } for _, pod := range pods[:2] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[2:] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -361,11 +362,11 @@ func TestEvictReplicatedByStatefulSet(t *testing.T) { } for _, pod := range pods[:2] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[2:] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -398,11 +399,11 @@ func TestEvictReplicatedByDaemonSet(t *testing.T) { } for _, pod := range pods[:2] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[2:] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -433,11 +434,11 @@ func TestEvictReplicatedByJob(t *testing.T) { } for _, pod := range pods[:2] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[2:] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -472,7 +473,7 @@ func TestEvictTooFewReplicas(t *testing.T) { } for _, pod := range pods { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -508,11 +509,11 @@ func TestEvictionTolerance(t *testing.T) { } for _, pod := range pods[:4] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[4:] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -548,15 +549,101 @@ func TestEvictAtLeastOne(t *testing.T) { } for _, pod := range pods[:1] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[1:] { - err := eviction.Evict(pod, test.FakeEventRecorder()) + err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } +func TestEvictEmitEvent(t *testing.T) { + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + } + + index := 0 + generatePod := func() test.PodBuilder { + index++ + return test.Pod().WithName(fmt.Sprintf("test-%v", index)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta) + } + + testCases := []struct { + name string + replicas int32 + evictionTolerance float64 + vpa *vpa_types.VerticalPodAutoscaler + pods []podWithExpectations + }{ + { + name: "Pods that can be evicted", + replicas: 4, + evictionTolerance: 0.5, + vpa: getBasicVpa(), + pods: []podWithExpectations{ + { + pod: generatePod().WithPhase(apiv1.PodPending).Get(), + canEvict: true, + evictionSuccess: true, + }, + { + pod: generatePod().WithPhase(apiv1.PodPending).Get(), + canEvict: true, + evictionSuccess: true, + }, + }, + }, + { + name: "Pod that can not be evicted", + replicas: 4, + evictionTolerance: 0.5, + vpa: getBasicVpa(), + pods: []podWithExpectations{ + + { + pod: generatePod().Get(), + canEvict: false, + evictionSuccess: false, + }, + }, + }, + } + + for _, testCase := range testCases { + rc.Spec = apiv1.ReplicationControllerSpec{ + Replicas: &testCase.replicas, + } + pods := make([]*apiv1.Pod, 0, len(testCase.pods)) + for _, p := range testCase.pods { + pods = append(pods, p.pod) + } + factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance) + eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa) + + for _, p := range testCase.pods { + mockRecorder := test.MockEventRecorder() + mockRecorder.On("Event", mock.Anything, apiv1.EventTypeNormal, "EvictedByVPA", mock.Anything).Return() + mockRecorder.On("Event", mock.Anything, apiv1.EventTypeNormal, "EvictedPod", mock.Anything).Return() + + eviction.Evict(p.pod, testCase.vpa, mockRecorder) + + if p.canEvict { + mockRecorder.AssertNumberOfCalls(t, "Event", 2) + + } else { + mockRecorder.AssertNumberOfCalls(t, "Event", 0) + } + } + } +} + func getEvictionRestrictionFactory(rc *apiv1.ReplicationController, rs *appsv1.ReplicaSet, ss *appsv1.StatefulSet, ds *appsv1.DaemonSet, minReplicas int, evictionToleranceFraction float64) (PodsEvictionRestrictionFactory, error) { diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater.go b/vertical-pod-autoscaler/pkg/updater/logic/updater.go index 5352b663e525..e4712decc37c 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater.go @@ -29,7 +29,8 @@ import ( "k8s.io/apimachinery/pkg/labels" kube_client "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" - "k8s.io/client-go/kubernetes/scheme" + + corescheme "k8s.io/client-go/kubernetes/scheme" clientv1 "k8s.io/client-go/kubernetes/typed/core/v1" v1lister "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" @@ -38,6 +39,7 @@ import ( vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned/scheme" vpa_lister "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/listers/autoscaling.k8s.io/v1" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target" controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" @@ -225,7 +227,7 @@ func (u *updater) RunOnce(ctx context.Context) { return } klog.V(2).InfoS("Evicting pod", "pod", klog.KObj(pod)) - evictErr := evictionLimiter.Evict(pod, u.eventRecorder) + evictErr := evictionLimiter.Evict(pod, vpa, u.eventRecorder) if evictErr != nil { klog.V(0).InfoS("Eviction failed", "error", evictErr, "pod", klog.KObj(pod)) } else { @@ -310,6 +312,12 @@ func newEventRecorder(kubeClient kube_client.Interface) record.EventRecorder { eventBroadcaster.StartLogging(klog.V(4).InfoS) if _, isFake := kubeClient.(*fake.Clientset); !isFake { eventBroadcaster.StartRecordingToSink(&clientv1.EventSinkImpl{Interface: clientv1.New(kubeClient.CoreV1().RESTClient()).Events("")}) + } else { + eventBroadcaster.StartRecordingToSink(&clientv1.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")}) } - return eventBroadcaster.NewRecorder(scheme.Scheme, apiv1.EventSource{Component: "vpa-updater"}) + + vpascheme := scheme.Scheme + corescheme.AddToScheme(vpascheme) + + return eventBroadcaster.NewRecorder(vpascheme, apiv1.EventSource{Component: "vpa-updater"}) } diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go index 2898b0b984cb..494c3c71f9ae 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go @@ -31,6 +31,8 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" @@ -357,3 +359,50 @@ func TestRunOnceIgnoreNamespaceMatching(t *testing.T) { updater.RunOnce(context.Background()) eviction.AssertNumberOfCalls(t, "Evict", 0) } + +func TestNewEventRecorder(t *testing.T) { + fakeClient := fake.NewSimpleClientset() + er := newEventRecorder(fakeClient) + + testCases := []struct { + reason string + object runtime.Object + message string + }{ + { + reason: "EvictedPod", + object: &apiv1.Pod{}, + message: "Evicted pod", + }, + { + reason: "EvictedPod", + object: &vpa_types.VerticalPodAutoscaler{}, + message: "Evicted pod", + }, + } + for _, tc := range testCases { + t.Run(tc.reason, func(t *testing.T) { + er.Event(tc.object, apiv1.EventTypeNormal, tc.reason, tc.message) + + var events *apiv1.EventList + var err error + // Add delay for fake client to catch up due to be being asynchronous + for i := 0; i < 5; i++ { + events, err = fakeClient.CoreV1().Events("default").List(context.TODO(), metav1.ListOptions{}) + if err == nil && len(events.Items) > 0 { + break + } + time.Sleep(100 * time.Millisecond) + } + + assert.NoError(t, err, "should be able to list events") + assert.Equal(t, 1, len(events.Items), "should have exactly 1 event") + + event := events.Items[0] + assert.Equal(t, tc.reason, event.Reason) + assert.Equal(t, tc.message, event.Message) + assert.Equal(t, apiv1.EventTypeNormal, event.Type) + assert.Equal(t, "vpa-updater", event.Source.Component) + }) + } +} diff --git a/vertical-pod-autoscaler/pkg/utils/test/test_utils.go b/vertical-pod-autoscaler/pkg/utils/test/test_utils.go index 4dd269a43d3e..e5d757973e7c 100644 --- a/vertical-pod-autoscaler/pkg/utils/test/test_utils.go +++ b/vertical-pod-autoscaler/pkg/utils/test/test_utils.go @@ -26,7 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/listers/core/v1" + v1 "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/record" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" @@ -110,7 +110,7 @@ type PodsEvictionRestrictionMock struct { } // Evict is a mock implementation of PodsEvictionRestriction.Evict -func (m *PodsEvictionRestrictionMock) Evict(pod *apiv1.Pod, eventRecorder record.EventRecorder) error { +func (m *PodsEvictionRestrictionMock) Evict(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error { args := m.Called(pod, eventRecorder) return args.Error(0) } @@ -268,3 +268,30 @@ func (f *fakeEventRecorder) AnnotatedEventf(object runtime.Object, annotations m func FakeEventRecorder() record.EventRecorder { return &fakeEventRecorder{} } + +// mockedEventRecorder is a dummy implementation of record.EventRecorder. +type mockedEventRecorder struct { + mock.Mock +} + +// Event is a mocked implementation of record.EventRecorder interface. +func (m *mockedEventRecorder) Event(object runtime.Object, eventtype, reason, message string) { + m.Called(object, eventtype, reason, message) +} + +// Eventf is a dummy implementation of record.EventRecorder interface. +func (m *mockedEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { +} + +// PastEventf is a dummy implementation of record.EventRecorder interface. +func (m *mockedEventRecorder) PastEventf(object runtime.Object, timestamp metav1.Time, eventtype, reason, messageFmt string, args ...interface{}) { +} + +// AnnotatedEventf is a dummy implementation of record.EventRecorder interface. +func (m *mockedEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { +} + +// MockEventRecorder returns a dummy implementation of record.EventRecorder. +func MockEventRecorder() *mockedEventRecorder { + return &mockedEventRecorder{} +} From 2896a69efb98369e44bb42a23e6432dfecd8d4a3 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Wed, 23 Oct 2024 12:23:08 +0200 Subject: [PATCH 2/5] Catch error when adding scheme --- vertical-pod-autoscaler/pkg/updater/logic/updater.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater.go b/vertical-pod-autoscaler/pkg/updater/logic/updater.go index e4712decc37c..d01c3cec29cc 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater.go @@ -317,7 +317,9 @@ func newEventRecorder(kubeClient kube_client.Interface) record.EventRecorder { } vpascheme := scheme.Scheme - corescheme.AddToScheme(vpascheme) + if err := corescheme.AddToScheme(vpascheme); err != nil { + klog.Fatalf("Error adding core scheme: %v", err) + } return eventBroadcaster.NewRecorder(vpascheme, apiv1.EventSource{Component: "vpa-updater"}) } From 953c8d06140e9731093a017c6a1b5e6e9f0f8140 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Wed, 23 Oct 2024 12:23:27 +0200 Subject: [PATCH 3/5] Tidy up test a bit and add a context --- .../pkg/updater/logic/updater_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go index 494c3c71f9ae..64357d4924e6 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go @@ -364,6 +364,10 @@ func TestNewEventRecorder(t *testing.T) { fakeClient := fake.NewSimpleClientset() er := newEventRecorder(fakeClient) + maxRetries := 5 + retryDelay := 100 * time.Millisecond + contextTimeout := 5 * time.Second + testCases := []struct { reason string object runtime.Object @@ -387,12 +391,14 @@ func TestNewEventRecorder(t *testing.T) { var events *apiv1.EventList var err error // Add delay for fake client to catch up due to be being asynchronous - for i := 0; i < 5; i++ { - events, err = fakeClient.CoreV1().Events("default").List(context.TODO(), metav1.ListOptions{}) + for i := 0; i < maxRetries; i++ { + ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) + defer cancel() + events, err = fakeClient.CoreV1().Events("default").List(ctx, metav1.ListOptions{}) if err == nil && len(events.Items) > 0 { break } - time.Sleep(100 * time.Millisecond) + time.Sleep(retryDelay) } assert.NoError(t, err, "should be able to list events") From 32f7183a03ed11611a42ce359d140769762753a3 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Wed, 23 Oct 2024 12:27:57 +0200 Subject: [PATCH 4/5] DRY up VPA a little --- .../pkg/updater/eviction/pods_eviction_restriction_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go index fa9b4cca06f2..d89354149c4e 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go @@ -322,12 +322,13 @@ func TestEvictReplicatedByReplicaSet(t *testing.T) { assert.True(t, eviction.CanEvict(pod)) } + basicVpa := getBasicVpa() for _, pod := range pods[:2] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[2:] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } From 81370199ad41b74f45c0495664f1819f66cd1475 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Wed, 23 Oct 2024 17:14:58 +0200 Subject: [PATCH 5/5] DRY up getBasicVpa() a bit more --- .../pods_eviction_restriction_test.go | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go index d89354149c4e..86918c7c5fed 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go @@ -315,14 +315,14 @@ func TestEvictReplicatedByReplicaSet(t *testing.T) { pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rs.ObjectMeta, &rs.TypeMeta).Get() } + basicVpa := getBasicVpa() factory, _ := getEvictionRestrictionFactory(nil, &rs, nil, nil, 2, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa()) + eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) } - basicVpa := getBasicVpa() for _, pod := range pods[:2] { err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") @@ -355,19 +355,20 @@ func TestEvictReplicatedByStatefulSet(t *testing.T) { pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&ss.ObjectMeta, &ss.TypeMeta).Get() } + basicVpa := getBasicVpa() factory, _ := getEvictionRestrictionFactory(nil, nil, &ss, nil, 2, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa()) + eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) } for _, pod := range pods[:2] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[2:] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -392,19 +393,21 @@ func TestEvictReplicatedByDaemonSet(t *testing.T) { for i := range pods { pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&ds.ObjectMeta, &ds.TypeMeta).Get() } + + basicVpa := getBasicVpa() factory, _ := getEvictionRestrictionFactory(nil, nil, nil, &ds, 2, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa()) + eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) } for _, pod := range pods[:2] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[2:] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -427,19 +430,20 @@ func TestEvictReplicatedByJob(t *testing.T) { pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&job.ObjectMeta, &job.TypeMeta).Get() } + basicVpa := getBasicVpa() factory, _ := getEvictionRestrictionFactory(nil, nil, nil, nil, 2, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa()) + eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) } for _, pod := range pods[:2] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[2:] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -466,15 +470,16 @@ func TestEvictTooFewReplicas(t *testing.T) { pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() } + basicVpa := getBasicVpa() factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 10, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa()) + eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) for _, pod := range pods { assert.False(t, eviction.CanEvict(pod)) } for _, pod := range pods { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -502,19 +507,20 @@ func TestEvictionTolerance(t *testing.T) { pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() } + basicVpa := getBasicVpa() factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance) - eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa()) + eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) } for _, pod := range pods[:4] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[4:] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -542,19 +548,20 @@ func TestEvictAtLeastOne(t *testing.T) { pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() } + basicVpa := getBasicVpa() factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, tolerance) - eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa()) + eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) } for _, pod := range pods[:1] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Nil(t, err, "Should evict with no error") } for _, pod := range pods[1:] { - err := eviction.Evict(pod, getBasicVpa(), test.FakeEventRecorder()) + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) assert.Error(t, err, "Error expected") } } @@ -576,6 +583,8 @@ func TestEvictEmitEvent(t *testing.T) { return test.Pod().WithName(fmt.Sprintf("test-%v", index)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta) } + basicVpa := getBasicVpa() + testCases := []struct { name string replicas int32 @@ -587,7 +596,7 @@ func TestEvictEmitEvent(t *testing.T) { name: "Pods that can be evicted", replicas: 4, evictionTolerance: 0.5, - vpa: getBasicVpa(), + vpa: basicVpa, pods: []podWithExpectations{ { pod: generatePod().WithPhase(apiv1.PodPending).Get(), @@ -605,7 +614,7 @@ func TestEvictEmitEvent(t *testing.T) { name: "Pod that can not be evicted", replicas: 4, evictionTolerance: 0.5, - vpa: getBasicVpa(), + vpa: basicVpa, pods: []podWithExpectations{ {