From 20c610c65adcbd2e5535b2c5bda657a25a33b6b1 Mon Sep 17 00:00:00 2001 From: Sebastiaan Tammer Date: Tue, 11 Jun 2019 20:35:55 +0200 Subject: [PATCH] Improved logic for duplicates testing --- pkg/descheduler/strategies/duplicates_test.go | 48 ++++++++----------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/pkg/descheduler/strategies/duplicates_test.go b/pkg/descheduler/strategies/duplicates_test.go index 428a1b29f..5d285a004 100644 --- a/pkg/descheduler/strategies/duplicates_test.go +++ b/pkg/descheduler/strategies/duplicates_test.go @@ -86,64 +86,58 @@ func TestFindDuplicatePods(t *testing.T) { // A Critical Pod. p7.Annotations = test.GetCriticalPodAnnotation() - tests := []struct { + testCases := []struct { description string maxPodsToEvict int pods []v1.Pod expectedEvictedPodCount int }{ { - description: "Three pods in the default Namespace, bound to same ReplicaSet. 2 should get deleted", - maxPodsToEvict: 2, + description: "Three pods in the `dev` Namespace, bound to same ReplicaSet. 2 should be evicted.", + maxPodsToEvict: 5, pods: []v1.Pod{*p1, *p2, *p3}, expectedEvictedPodCount: 2, }, { - description: "Three Pods in the test Namespace, bound to same ReplicaSet. 2 should be evicted", - maxPodsToEvict: 2, + description: "Three Pods in the `test` Namespace, bound to same ReplicaSet. 2 should be evicted.", + maxPodsToEvict: 5, pods: []v1.Pod{*p8, *p9, *p10}, expectedEvictedPodCount: 2, }, { - description: "pods are part of daemonset and local storage - none should get deleted", - maxPodsToEvict: 2, - pods: []v1.Pod{*p4, *p5}, - expectedEvictedPodCount: 0, - }, - { - description: "pods are mirror pod and critical pod - none should get deleted", - maxPodsToEvict: 2, - pods: []v1.Pod{*p6, *p7}, - expectedEvictedPodCount: 0, - }, - { - description: "pods are part of replicaset, daemonset and local storage - all duplicate replicaset pods should get deleted", - maxPodsToEvict: 4, - pods: []v1.Pod{*p1, *p2, *p3, *p8, *p9, *p10, *p4, *p5}, + description: "Three Pods in the `dev` Namespace, three Pods in the `test` Namespace. Bound to ReplicaSet with same name. 4 should be evicted.", + maxPodsToEvict: 5, + pods: []v1.Pod{*p1, *p2, *p3, *p8, *p9, *p10}, expectedEvictedPodCount: 4, }, { - description: "pods are part of replicaset, daemonset, local storage, mirror pod and critical pod - all duplicate replicaset pods should get deleted", + description: "Pods are: part of DaemonSet, with local storage, mirror pod annotation, critical pod annotation - none should be evicted.", + maxPodsToEvict: 2, + pods: []v1.Pod{*p4, *p5, *p6, *p7}, + expectedEvictedPodCount: 0, + }, + { + description: "Test all Pods: 4 should be evicted.", maxPodsToEvict: 5, - pods: []v1.Pod{*p1, *p2, *p3, *p8, *p9, *p10, *p4, *p5, *p6, *p7}, + pods: []v1.Pod{*p1, *p2, *p3, *p4, *p5, *p6, *p7, *p8, *p9, *p10}, expectedEvictedPodCount: 4, }, } - for _, test := range tests { + for _, testCase := range testCases { npe := nodePodEvictedCount{} npe[node] = 0 fakeClient := &fake.Clientset{} fakeClient.Fake.AddReactor("list", "pods", func(action core.Action) (bool, runtime.Object, error) { - return true, &v1.PodList{Items: test.pods}, nil + return true, &v1.PodList{Items: testCase.pods}, nil }) fakeClient.Fake.AddReactor("get", "nodes", func(action core.Action) (bool, runtime.Object, error) { return true, node, nil }) - podsEvicted := deleteDuplicatePods(fakeClient, "v1", []*v1.Node{node}, false, npe, test.maxPodsToEvict, false) - if podsEvicted != test.expectedEvictedPodCount { - t.Errorf("Test error for Desc: %s. Expected deleted pods count %v , got %v", test.description, test.expectedEvictedPodCount, podsEvicted) + podsEvicted := deleteDuplicatePods(fakeClient, "v1", []*v1.Node{node}, false, npe, testCase.maxPodsToEvict, false) + if podsEvicted != testCase.expectedEvictedPodCount { + t.Errorf("Test error for description: %s. Expected evicted pods count %v, got %v", testCase.description, testCase.expectedEvictedPodCount, podsEvicted) } }