From dceae3b38899dc6498724f0e8f4d547e5fa31e64 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Fri, 2 May 2025 10:31:59 +0200
Subject: [PATCH] DRA e2e: avoid terminationGracePeriodSeconds

`terminationGracePeriodSeconds: 0` was a mistake, it bypasses the normal
pod shutdown in the kubelet.

The right way to shut down a pod quickly is to have it react to SIGTERM.
The busybox implementation of "sleep" doesn't. `agnhost pause` does,
so let's use that instead.

For E2E tests, the InfiniteSleepCommand was already change about a year ago to
react to SIGTERM, so the `terminationGracePeriodSeconds: 1` workaround is no
longer needed.
---
 test/e2e/dra/dra.go                           | 21 ++++++-------------
 .../deploy/example/pod-external.yaml          |  9 ++++----
 .../deploy/example/pod-inline.yaml            |  8 +++----
 3 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/test/e2e/dra/dra.go b/test/e2e/dra/dra.go
index 3bdd32a2aa7..e75f214c3de 100644
--- a/test/e2e/dra/dra.go
+++ b/test/e2e/dra/dra.go
@@ -2230,24 +2230,15 @@ func (b *builder) parametersEnv() (string, []string) {
 // makePod returns a simple pod with no resource claims.
 // The pod prints its env and waits.
 func (b *builder) pod() *v1.Pod {
+	// The e2epod.InfiniteSleepCommand was changed so that it reacts to SIGTERM,
+	// causing the pod to shut down immediately. This is better than the previous approach
+	// with `terminationGraceperiodseconds: 1` because that still caused a one second delay.
+	//
+	// It is tempting to use `terminationGraceperiodSeconds: 0`, but that is a very bad
+	// idea because it removes the pod before the kubelet had a chance to react (https://github.com/kubernetes/kubernetes/issues/120671).
 	pod := e2epod.MakePod(b.f.Namespace.Name, nil, nil, b.f.NamespacePodSecurityLevel, "" /* no command = pause */)
 	pod.Labels = make(map[string]string)
 	pod.Spec.RestartPolicy = v1.RestartPolicyNever
-	// Let kubelet kill the pods quickly. Setting
-	// TerminationGracePeriodSeconds to zero would bypass kubelet
-	// completely because then the apiserver enables a force-delete even
-	// when DeleteOptions for the pod don't ask for it (see
-	// https://github.com/kubernetes/kubernetes/blob/0f582f7c3f504e807550310d00f130cb5c18c0c3/pkg/registry/core/pod/strategy.go#L151-L171).
-	//
-	// We don't do that because it breaks tracking of claim usage: the
-	// kube-controller-manager assumes that kubelet is done with the pod
-	// once it got removed or has a grace period of 0. Setting the grace
-	// period to zero directly in DeletionOptions or indirectly through
-	// TerminationGracePeriodSeconds causes the controller to remove
-	// the pod from ReservedFor before it actually has stopped on
-	// the node.
-	one := int64(1)
-	pod.Spec.TerminationGracePeriodSeconds = &one
 	pod.ObjectMeta.GenerateName = ""
 	b.podCounter++
 	pod.ObjectMeta.Name = fmt.Sprintf("tester%s-%d", b.driver.NameSuffix, b.podCounter)
diff --git a/test/e2e/dra/test-driver/deploy/example/pod-external.yaml b/test/e2e/dra/test-driver/deploy/example/pod-external.yaml
index 705312d058a..35ac1e2305b 100644
--- a/test/e2e/dra/test-driver/deploy/example/pod-external.yaml
+++ b/test/e2e/dra/test-driver/deploy/example/pod-external.yaml
@@ -23,15 +23,14 @@ spec:
   restartPolicy: Never
   containers:
   - name: with-resource
-    image: registry.k8s.io/e2e-test-images/busybox:1.36.1-1
-    command: ["sh", "-c", "set && mount && ls -la /dev/ && sleep 10000"]
+    image: registry.k8s.io/e2e-test-images/agnhost:2.53
+    command: ["sh", "-c", "set && mount && ls -la /dev/ && /agnhost pause"]
     resources:
       claims:
       - name: resource
   - name: without-resource
-    image: registry.k8s.io/e2e-test-images/busybox:1.36.1-1
-    command: ["sh", "-c", "set && mount && ls -la /dev/ && sleep 10000"]
-  terminationGracePeriodSeconds: 0 # Shut down immediately.
+    image: registry.k8s.io/e2e-test-images/agnhost:2.53
+    command: ["sh", "-c", "set && mount && ls -la /dev/ && /agnhost pause"]
   resourceClaims:
   - name: resource
     resourceClaimName: external-claim
diff --git a/test/e2e/dra/test-driver/deploy/example/pod-inline.yaml b/test/e2e/dra/test-driver/deploy/example/pod-inline.yaml
index 361290be946..44f6ba470bc 100644
--- a/test/e2e/dra/test-driver/deploy/example/pod-inline.yaml
+++ b/test/e2e/dra/test-driver/deploy/example/pod-inline.yaml
@@ -28,14 +28,14 @@ spec:
   restartPolicy: Never
   containers:
   - name: with-resource
-    image: registry.k8s.io/e2e-test-images/busybox:1.36.1-1
-    command: ["sh", "-c", "set && mount && ls -la /dev/ && sleep 10000"]
+    image: registry.k8s.io/e2e-test-images/agnhost:2.53
+    command: ["sh", "-c", "set && mount && ls -la /dev/ && /agnhost pause"]
     resources:
       claims:
       - name: resource
   - name: without-resource
-    image: registry.k8s.io/e2e-test-images/busybox:1.36.1-1
-    command: ["sh", "-c", "set && mount && ls -la /dev/ && sleep 10000"]
+    image: registry.k8s.io/e2e-test-images/agnhost:2.53
+    command: ["sh", "-c", "set && mount && ls -la /dev/ && /agnhost pause"]
   terminationGracePeriodSeconds: 0 # Shut down immediately.
   resourceClaims:
   - name: resource