Merge pull request #115835 from HirazawaUi/fix-terminationGracePeriod-bug

fix terminationGracePeriod blocked by preStop

pkg/kubelet/kuberuntime/kuberuntime_container.go

@@ -725,20 +725,22 @@ func (m *kubeGenericRuntimeManager) killContainer(ctx context.Context, pod *v1.P
 	}
 	m.recordContainerEvent(pod, containerSpec, containerID.ID, v1.EventTypeNormal, events.KillingContainer, message)
 
-	// Run the pre-stop lifecycle hooks if applicable and if there is enough time to run it
-	if containerSpec.Lifecycle != nil && containerSpec.Lifecycle.PreStop != nil && gracePeriod > 0 {
-		gracePeriod = gracePeriod - m.executePreStopHook(ctx, pod, containerID, containerSpec, gracePeriod)
-	}
-	// always give containers a minimal shutdown window to avoid unnecessary SIGKILLs
-	if gracePeriod < minimumGracePeriodInSeconds {
-		gracePeriod = minimumGracePeriodInSeconds
-	}
 	if gracePeriodOverride != nil {
 		gracePeriod = *gracePeriodOverride
 		klog.V(3).InfoS("Killing container with a grace period override", "pod", klog.KObj(pod), "podUID", pod.UID,
 			"containerName", containerName, "containerID", containerID.String(), "gracePeriod", gracePeriod)
 	}
 
+	// Run the pre-stop lifecycle hooks if applicable and if there is enough time to run it
+	if containerSpec.Lifecycle != nil && containerSpec.Lifecycle.PreStop != nil && gracePeriod > 0 {
+		gracePeriod = gracePeriod - m.executePreStopHook(ctx, pod, containerID, containerSpec, gracePeriod)
+	}
+
+	// always give containers a minimal shutdown window to avoid unnecessary SIGKILLs
+	if gracePeriod < minimumGracePeriodInSeconds {
+		gracePeriod = minimumGracePeriodInSeconds
+	}
+
 	klog.V(2).InfoS("Killing container with a grace period", "pod", klog.KObj(pod), "podUID", pod.UID,
 		"containerName", containerName, "containerID", containerID.String(), "gracePeriod", gracePeriod)
 

test/e2e_node/container_lifecycle_test.go

@@ -515,4 +515,84 @@ var _ = SIGDescribe("[NodeConformance] Containers Lifecycle ", func() {
 		// while the regular container did
 		framework.ExpectNoError(results.HasRestarted(regular1))
 	})
+
+	ginkgo.When("a pod cannot terminate gracefully", func() {
+		testPod := func(name string, gracePeriod int64) *v1.Pod {
+			return &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{
+						{
+							Name:  "busybox",
+							Image: imageutils.GetE2EImage(imageutils.BusyBox),
+							Command: []string{
+								"sleep",
+								"10000",
+							},
+						},
+					},
+					TerminationGracePeriodSeconds: &gracePeriod,
+				},
+			}
+		}
+
+		// To account for the time it takes to delete the pod, we add a 10 second
+		// buffer. The 10 second buffer is arbitrary, but it should be enough to
+		// account for the time it takes to delete the pod.
+		bufferSeconds := int64(10)
+
+		ginkgo.It("should respect termination grace period seconds [NodeConformance]", func() {
+			client := e2epod.NewPodClient(f)
+			gracePeriod := int64(30)
+
+			ginkgo.By("creating a pod with a termination grace period seconds")
+			pod := testPod("pod-termination-grace-period", gracePeriod)
+			pod = client.Create(context.TODO(), pod)
+
+			ginkgo.By("ensuring the pod is running")
+			err := e2epod.WaitForPodRunningInNamespace(context.TODO(), f.ClientSet, pod)
+			framework.ExpectNoError(err)
+
+			ginkgo.By("deleting the pod gracefully")
+			err = client.Delete(context.TODO(), pod.Name, metav1.DeleteOptions{})
+			framework.ExpectNoError(err)
+
+			ginkgo.By("ensuring the pod is terminated within the grace period seconds + buffer seconds")
+			err = e2epod.WaitForPodNotFoundInNamespace(context.TODO(), f.ClientSet, pod.Name, pod.Namespace, time.Duration(gracePeriod+bufferSeconds)*time.Second)
+			framework.ExpectNoError(err)
+		})
+
+		ginkgo.It("should respect termination grace period seconds with long-running preStop hook [NodeConformance]", func() {
+			client := e2epod.NewPodClient(f)
+			gracePeriod := int64(30)
+
+			ginkgo.By("creating a pod with a termination grace period seconds and long-running preStop hook")
+			pod := testPod("pod-termination-grace-period", gracePeriod)
+			pod.Spec.Containers[0].Lifecycle = &v1.Lifecycle{
+				PreStop: &v1.LifecycleHandler{
+					Exec: &v1.ExecAction{
+						Command: []string{
+							"sleep",
+							"10000",
+						},
+					},
+				},
+			}
+			pod = client.Create(context.TODO(), pod)
+
+			ginkgo.By("ensuring the pod is running")
+			err := e2epod.WaitForPodRunningInNamespace(context.TODO(), f.ClientSet, pod)
+			framework.ExpectNoError(err)
+
+			ginkgo.By("deleting the pod gracefully")
+			err = client.Delete(context.TODO(), pod.Name, metav1.DeleteOptions{})
+			framework.ExpectNoError(err)
+
+			ginkgo.By("ensuring the pod is terminated within the grace period seconds + buffer seconds")
+			err = e2epod.WaitForPodNotFoundInNamespace(context.TODO(), f.ClientSet, pod.Name, pod.Namespace, time.Duration(gracePeriod+bufferSeconds)*time.Second)
+			framework.ExpectNoError(err)
+		})
+	})
 })