From 2020642b6fe7b505732692783c1b366a001ff8c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Jankovi=C4=8D?= Date: Wed, 24 Nov 2021 15:39:12 +0100 Subject: [PATCH] chore: add pod.Status.Reason to the list of reasons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Erik Jankovič --- pkg/descheduler/strategies/failedpods.go | 4 + pkg/descheduler/strategies/failedpods_test.go | 110 +++++++++++------- 2 files changed, 72 insertions(+), 42 deletions(-) diff --git a/pkg/descheduler/strategies/failedpods.go b/pkg/descheduler/strategies/failedpods.go index 10eeae3d6..b2942509b 100644 --- a/pkg/descheduler/strategies/failedpods.go +++ b/pkg/descheduler/strategies/failedpods.go @@ -147,6 +147,10 @@ func validateFailedPodShouldEvict(pod *v1.Pod, strategyParams validatedFailedPod if len(strategyParams.reasons) > 0 { reasons := getFailedContainerStatusReasons(pod.Status.ContainerStatuses) + if pod.Status.Phase == v1.PodFailed && pod.Status.Reason != "" { + reasons = append(reasons, pod.Status.Reason) + } + if strategyParams.includingInitContainers { reasons = append(reasons, getFailedContainerStatusReasons(pod.Status.InitContainerStatuses)...) } diff --git a/pkg/descheduler/strategies/failedpods_test.go b/pkg/descheduler/strategies/failedpods_test.go index 8c398f0ae..3d73a49aa 100644 --- a/pkg/descheduler/strategies/failedpods_test.go +++ b/pkg/descheduler/strategies/failedpods_test.go @@ -64,9 +64,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 1, pods: []*v1.Pod{ - buildTestPod("p1", "node1", nil, &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", nil, &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "NodeAffinity"}, - }, nil), + }), nil), }, }, { @@ -75,9 +75,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 1, pods: []*v1.Pod{ - buildTestPod("p1", "node1", &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "NodeAffinity"}, - }, nil, nil), + }, nil), nil), }, }, { @@ -86,9 +86,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 1, pods: []*v1.Pod{ - buildTestPod("p1", "node1", &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", &v1.ContainerState{ Waiting: &v1.ContainerStateWaiting{Reason: "CreateContainerConfigError"}, - }, nil, nil), + }, nil), nil), }, }, { @@ -100,12 +100,12 @@ func TestRemoveFailedPods(t *testing.T) { }, expectedEvictedPodCount: 2, pods: []*v1.Pod{ - buildTestPod("p1", "node1", &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "CreateContainerConfigError"}, - }, nil, nil), - buildTestPod("p2", "node2", &v1.ContainerState{ + }, nil), nil), + buildTestPod("p2", "node2", newPodStatus("", "", &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "CreateContainerConfigError"}, - }, nil, nil), + }, nil), nil), }, }, { @@ -114,9 +114,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 1, pods: []*v1.Pod{ - buildTestPod("p1", "node1", nil, &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", nil, &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "CreateContainerConfigError"}, - }, nil), + }), nil), }, }, { @@ -125,9 +125,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 1, pods: []*v1.Pod{ - buildTestPod("p1", "node1", nil, &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", nil, &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "CreateContainerConfigError"}, - }, nil), + }), nil), }, }, { @@ -136,9 +136,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 0, pods: []*v1.Pod{ - buildTestPod("p1", "node1", nil, &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", nil, &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "NodeAffinity"}, - }, nil), + }), nil), }, }, { @@ -147,9 +147,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 0, pods: []*v1.Pod{ - buildTestPod("p1", "node1", &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", &v1.ContainerState{ Waiting: &v1.ContainerStateWaiting{Reason: "CreateContainerConfigError"}, - }, nil, nil), + }, nil), nil), }, }, { @@ -158,9 +158,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 0, pods: []*v1.Pod{ - buildTestPod("p1", "node1", nil, &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", nil, &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "NodeAffinity"}, - }, nil), + }), nil), }, }, { @@ -171,9 +171,9 @@ func TestRemoveFailedPods(t *testing.T) { })}, expectedEvictedPodCount: 0, pods: []*v1.Pod{ - buildTestPod("p1", "node1", nil, &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", nil, &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "NodeAffinity"}, - }, nil), + }), nil), }, }, { @@ -182,9 +182,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 0, pods: []*v1.Pod{ - buildTestPod("p1", "node1", &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "NodeAffinity"}, - }, nil, nil), + }, nil), nil), }, }, { @@ -193,9 +193,9 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 1, pods: []*v1.Pod{ - buildTestPod("p1", "node1", &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "NodeAffinity"}, - }, nil, nil), + }, nil), nil), }, }, { @@ -204,9 +204,28 @@ func TestRemoveFailedPods(t *testing.T) { nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, expectedEvictedPodCount: 0, pods: []*v1.Pod{ - buildTestPod("p1", "node1", &v1.ContainerState{ + buildTestPod("p1", "node1", newPodStatus("", "", &v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{Reason: "NodeAffinity"}, - }, nil, &metav1.Time{}), + }, nil), &metav1.Time{}), + }, + }, + { + description: "1 container terminated with reason ShutDown, 0 evictions", + strategy: createStrategy(true, false, nil, nil, nil, true), + nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, + expectedEvictedPodCount: 0, + pods: []*v1.Pod{ + buildTestPod("p1", "node1", newPodStatus("Shutdown", v1.PodFailed, nil, nil), nil), + }, + }, + { + description: "include reason=Shutdown, 2 containers terminated with reason ShutDown, 2 evictions", + strategy: createStrategy(true, false, []string{"Shutdown"}, nil, nil, false), + nodes: []*v1.Node{test.BuildTestNode("node1", 2000, 3000, 10, nil)}, + expectedEvictedPodCount: 2, + pods: []*v1.Pod{ + buildTestPod("p1", "node1", newPodStatus("Shutdown", v1.PodFailed, nil, nil), nil), + buildTestPod("p2", "node1", newPodStatus("Shutdown", v1.PodFailed, nil, nil), nil), }, }, } @@ -292,21 +311,28 @@ func TestValidRemoveFailedPodsParams(t *testing.T) { } } -func buildTestPod(podName, nodeName string, initContainerState, containerState *v1.ContainerState, deletionTimestamp *metav1.Time) *v1.Pod { +func newPodStatus(reason string, phase v1.PodPhase, initContainerState, containerState *v1.ContainerState) v1.PodStatus { + ps := v1.PodStatus{ + Reason: reason, + Phase: phase, + } + + if initContainerState != nil { + ps.InitContainerStatuses = []v1.ContainerStatus{{State: *initContainerState}} + ps.Phase = v1.PodFailed + } + + if containerState != nil { + ps.ContainerStatuses = []v1.ContainerStatus{{State: *containerState}} + ps.Phase = v1.PodFailed + } + + return ps +} + +func buildTestPod(podName, nodeName string, podStatus v1.PodStatus, deletionTimestamp *metav1.Time) *v1.Pod { pod := test.BuildTestPod(podName, 1, 1, nodeName, func(p *v1.Pod) { - ps := v1.PodStatus{} - - if initContainerState != nil { - ps.InitContainerStatuses = []v1.ContainerStatus{{State: *initContainerState}} - ps.Phase = v1.PodFailed - } - - if containerState != nil { - ps.ContainerStatuses = []v1.ContainerStatus{{State: *containerState}} - ps.Phase = v1.PodFailed - } - - p.Status = ps + p.Status = podStatus }) pod.ObjectMeta.OwnerReferences = test.GetReplicaSetOwnerRefList() pod.ObjectMeta.SetCreationTimestamp(metav1.Now())