From 4e1c3e52378e8cdd0e2a563f7bd0d48b650e6f2b Mon Sep 17 00:00:00 2001
From: Olga Shestopalova <oshestopalova1@gmail.com>
Date: Tue, 2 Apr 2024 09:13:25 -0400
Subject: [PATCH] fix grace period set for force pod deletion

Signed-off-by: Olga Shestopalova <oshestopalova1@gmail.com>
---
 pkg/kubelet/pod_workers.go      |  5 +++-
 pkg/kubelet/pod_workers_test.go | 49 +++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/pkg/kubelet/pod_workers.go b/pkg/kubelet/pod_workers.go
index 8be367f8f0e..30850d830f6 100644
--- a/pkg/kubelet/pod_workers.go
+++ b/pkg/kubelet/pod_workers.go
@@ -979,10 +979,12 @@ func calculateEffectiveGracePeriod(status *podSyncStatus, pod *v1.Pod, options *
 	// enforce the restriction that a grace period can only decrease and track whatever our value is,
 	// then ensure a calculated value is passed down to lower levels
 	gracePeriod := status.gracePeriod
+	overridden := false
 	// this value is bedrock truth - the apiserver owns telling us this value calculated by apiserver
 	if override := pod.DeletionGracePeriodSeconds; override != nil {
 		if gracePeriod == 0 || *override < gracePeriod {
 			gracePeriod = *override
+			overridden = true
 		}
 	}
 	// we allow other parts of the kubelet (namely eviction) to request this pod be terminated faster
@@ -990,12 +992,13 @@ func calculateEffectiveGracePeriod(status *podSyncStatus, pod *v1.Pod, options *
 		if override := options.PodTerminationGracePeriodSecondsOverride; override != nil {
 			if gracePeriod == 0 || *override < gracePeriod {
 				gracePeriod = *override
+				overridden = true
 			}
 		}
 	}
 	// make a best effort to default this value to the pod's desired intent, in the event
 	// the kubelet provided no requested value (graceful termination?)
-	if gracePeriod == 0 && pod.Spec.TerminationGracePeriodSeconds != nil {
+	if !overridden && gracePeriod == 0 && pod.Spec.TerminationGracePeriodSeconds != nil {
 		gracePeriod = *pod.Spec.TerminationGracePeriodSeconds
 	}
 	// no matter what, we always supply a grace period of 1
diff --git a/pkg/kubelet/pod_workers_test.go b/pkg/kubelet/pod_workers_test.go
index 8fca3c255a8..e09ba70cbea 100644
--- a/pkg/kubelet/pod_workers_test.go
+++ b/pkg/kubelet/pod_workers_test.go
@@ -2382,3 +2382,52 @@ func Test_allowPodStart(t *testing.T) {
 		})
 	}
 }
+
+func Test_calculateEffectiveGracePeriod(t *testing.T) {
+	zero := int64(0)
+	two := int64(2)
+	five := int64(5)
+	thirty := int64(30)
+	// no overrides, use what's on the spec
+	pod := newNamedPod("1", "ns", "running-pod", false)
+	pod.Spec.TerminationGracePeriodSeconds = &thirty
+	gracePeriod, _ := calculateEffectiveGracePeriod(&podSyncStatus{}, pod, &KillPodOptions{})
+	expectedGracePeriod := int64(30)
+	if gracePeriod != expectedGracePeriod {
+		t.Errorf("Expected a grace period of %v, but was %v", expectedGracePeriod, gracePeriod)
+	}
+
+	// pod DeletionGracePeriodSeconds is set
+	pod.DeletionGracePeriodSeconds = &five
+	gracePeriod, _ = calculateEffectiveGracePeriod(&podSyncStatus{}, pod, &KillPodOptions{})
+	expectedGracePeriod = five
+	if gracePeriod != expectedGracePeriod {
+		t.Errorf("Expected a grace period of %v, but was %v", expectedGracePeriod, gracePeriod)
+	}
+
+	// grace period override
+	gracePeriod, _ = calculateEffectiveGracePeriod(&podSyncStatus{}, pod, &KillPodOptions{
+		PodTerminationGracePeriodSecondsOverride: &two,
+	})
+	expectedGracePeriod = two
+	if gracePeriod != expectedGracePeriod {
+		t.Errorf("Expected a grace period of %v, but was %v", expectedGracePeriod, gracePeriod)
+	}
+
+	// pod DeletionGracePeriodSeconds is zero
+	pod.DeletionGracePeriodSeconds = &zero
+	gracePeriod, _ = calculateEffectiveGracePeriod(&podSyncStatus{}, pod, &KillPodOptions{})
+	expectedGracePeriod = int64(1)
+	if gracePeriod != expectedGracePeriod {
+		t.Errorf("Expected a grace period of %v, but was %v", expectedGracePeriod, gracePeriod)
+	}
+
+	// grace period override is zero
+	gracePeriod, _ = calculateEffectiveGracePeriod(&podSyncStatus{}, pod, &KillPodOptions{
+		PodTerminationGracePeriodSecondsOverride: &zero,
+	})
+	expectedGracePeriod = int64(1)
+	if gracePeriod != expectedGracePeriod {
+		t.Errorf("Expected a grace period of %v, but was %v", expectedGracePeriod, gracePeriod)
+	}
+}