From f0a9598db636e2349482c39e4fae0d8c30cac2e7 Mon Sep 17 00:00:00 2001 From: Zihan Jiang Date: Wed, 17 Apr 2024 09:28:28 -0700 Subject: [PATCH 1/3] filter out pods that have deletion timestamp set in currentlyDrainedPods --- .../core/podlistprocessor/currently_drained_nodes.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes.go b/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes.go index 495aae1c08f8..be44a8708481 100644 --- a/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes.go +++ b/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes.go @@ -51,6 +51,11 @@ func currentlyDrainedPods(context *context.AutoscalingContext) []*apiv1.Pod { continue } for _, podInfo := range nodeInfo.Pods { + // Filter out pods that has deletion timestamp set + if podInfo.Pod.DeletionTimestamp != nil { + klog.Infof("Pod %v has deletion timestamp set, skipping", podInfo.Pod.Name) + continue + } pods = append(pods, podInfo.Pod) } } From 15ca53f4a6082a1f487750aeb84de91b8444e431 Mon Sep 17 00:00:00 2001 From: Zihan Jiang Date: Thu, 25 Apr 2024 14:26:59 -0700 Subject: [PATCH 2/3] add unit test --- .../currently_drained_nodes_test.go | 14 ++++++++++++++ cluster-autoscaler/utils/test/test_utils.go | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes_test.go b/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes_test.go index 678afbd05709..ac94966d5d33 100644 --- a/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes_test.go +++ b/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes_test.go @@ -69,6 +69,20 @@ func TestCurrentlyDrainedNodesPodListProcessor(t *testing.T) { BuildTestPod("p2", 200, 1), }, }, + { + name: "single node undergoing deletion, pods with deletion timestamp set", + drainedNodes: []string{"n"}, + nodes: []*apiv1.Node{ + BuildTestNode("n", 1000, 10), + }, + pods: []*apiv1.Pod{ + BuildScheduledTestPod("p1", 100, 1, "n"), + BuildTestPodWithDeletionTimestamp("p2", 200, 1, "n", time.Now()), + }, + wantPods: []*apiv1.Pod{ + BuildTestPod("p1", 100, 1), + }, + }, { name: "single empty node undergoing deletion", drainedNodes: []string{"n"}, diff --git a/cluster-autoscaler/utils/test/test_utils.go b/cluster-autoscaler/utils/test/test_utils.go index 81c939cbfc43..2a71befe4823 100644 --- a/cluster-autoscaler/utils/test/test_utils.go +++ b/cluster-autoscaler/utils/test/test_utils.go @@ -195,6 +195,13 @@ func BuildScheduledTestPod(name string, cpu, memory int64, nodeName string) *api return p } +// BuildTestPodWithDeletionTimestamp builds a test pod with deletion timestamp set +func BuildTestPodWithDeletionTimestamp(name string, cpu, memory int64, nodeName string, deletionTimestamp time.Time) *apiv1.Pod { + p := BuildScheduledTestPod(name, cpu, memory, nodeName) + p.DeletionTimestamp = &metav1.Time{Time: deletionTimestamp} + return p +} + // SetStaticPodSpec sets pod spec to make it a static pod func SetStaticPodSpec(pod *apiv1.Pod) *apiv1.Pod { pod.Annotations[kube_types.ConfigSourceAnnotationKey] = kube_types.FileSource From e6ab3438fa223586dfc22750a2f069aa709eaf83 Mon Sep 17 00:00:00 2001 From: Zihan Jiang Date: Fri, 26 Apr 2024 20:06:37 -0700 Subject: [PATCH 3/3] refine log and test case --- .../podlistprocessor/currently_drained_nodes.go | 2 +- .../currently_drained_nodes_test.go | 2 +- cluster-autoscaler/utils/test/test_utils.go | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes.go b/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes.go index be44a8708481..58459da50f17 100644 --- a/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes.go +++ b/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes.go @@ -53,7 +53,7 @@ func currentlyDrainedPods(context *context.AutoscalingContext) []*apiv1.Pod { for _, podInfo := range nodeInfo.Pods { // Filter out pods that has deletion timestamp set if podInfo.Pod.DeletionTimestamp != nil { - klog.Infof("Pod %v has deletion timestamp set, skipping", podInfo.Pod.Name) + klog.Infof("Pod %v has deletion timestamp set, skipping injection to unschedulable pods list", podInfo.Pod.Name) continue } pods = append(pods, podInfo.Pod) diff --git a/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes_test.go b/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes_test.go index ac94966d5d33..0aebcbd4d671 100644 --- a/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes_test.go +++ b/cluster-autoscaler/core/podlistprocessor/currently_drained_nodes_test.go @@ -77,7 +77,7 @@ func TestCurrentlyDrainedNodesPodListProcessor(t *testing.T) { }, pods: []*apiv1.Pod{ BuildScheduledTestPod("p1", 100, 1, "n"), - BuildTestPodWithDeletionTimestamp("p2", 200, 1, "n", time.Now()), + BuildTestPod("p2", 200, 1, WithNodeName("n"), WithDeletionTimestamp(time.Now())), }, wantPods: []*apiv1.Pod{ BuildTestPod("p1", 100, 1), diff --git a/cluster-autoscaler/utils/test/test_utils.go b/cluster-autoscaler/utils/test/test_utils.go index 2a71befe4823..de868d1ef9c9 100644 --- a/cluster-autoscaler/utils/test/test_utils.go +++ b/cluster-autoscaler/utils/test/test_utils.go @@ -150,6 +150,13 @@ func WithMaxSkew(maxSkew int32, topologySpreadingKey string) func(*apiv1.Pod) { } } +// WithDeletionTimestamp sets deletion timestamp to the pod. +func WithDeletionTimestamp(deletionTimestamp time.Time) func(*apiv1.Pod) { + return func(pod *apiv1.Pod) { + pod.DeletionTimestamp = &metav1.Time{Time: deletionTimestamp} + } +} + // BuildTestPodWithEphemeralStorage creates a pod with cpu, memory and ephemeral storage resources. func BuildTestPodWithEphemeralStorage(name string, cpu, mem, ephemeralStorage int64) *apiv1.Pod { startTime := metav1.Unix(0, 0) @@ -195,13 +202,6 @@ func BuildScheduledTestPod(name string, cpu, memory int64, nodeName string) *api return p } -// BuildTestPodWithDeletionTimestamp builds a test pod with deletion timestamp set -func BuildTestPodWithDeletionTimestamp(name string, cpu, memory int64, nodeName string, deletionTimestamp time.Time) *apiv1.Pod { - p := BuildScheduledTestPod(name, cpu, memory, nodeName) - p.DeletionTimestamp = &metav1.Time{Time: deletionTimestamp} - return p -} - // SetStaticPodSpec sets pod spec to make it a static pod func SetStaticPodSpec(pod *apiv1.Pod) *apiv1.Pod { pod.Annotations[kube_types.ConfigSourceAnnotationKey] = kube_types.FileSource