From caa2fce0a1a2d1e21a556325f181513c7a209414 Mon Sep 17 00:00:00 2001 From: "Paul S. Schweigert" Date: Wed, 22 Jun 2022 16:19:41 -0400 Subject: [PATCH] add unit tests for grace period in killContainer func Signed-off-by: Paul S. Schweigert --- .../kuberuntime/kuberuntime_container.go | 51 ++++-- .../kuberuntime/kuberuntime_container_test.go | 164 ++++++++++++++++++ 2 files changed, 197 insertions(+), 18 deletions(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index 1ea7723f813..70ae9d742c4 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -675,24 +675,7 @@ func (m *kubeGenericRuntimeManager) killContainer(pod *v1.Pod, containerID kubec } // From this point, pod and container must be non-nil. - gracePeriod := int64(minimumGracePeriodInSeconds) - switch { - case pod.DeletionGracePeriodSeconds != nil: - gracePeriod = *pod.DeletionGracePeriodSeconds - case pod.Spec.TerminationGracePeriodSeconds != nil: - gracePeriod = *pod.Spec.TerminationGracePeriodSeconds - - switch reason { - case reasonStartupProbe: - if containerSpec.StartupProbe != nil && containerSpec.StartupProbe.TerminationGracePeriodSeconds != nil { - gracePeriod = *containerSpec.StartupProbe.TerminationGracePeriodSeconds - } - case reasonLivenessProbe: - if containerSpec.LivenessProbe != nil && containerSpec.LivenessProbe.TerminationGracePeriodSeconds != nil { - gracePeriod = *containerSpec.LivenessProbe.TerminationGracePeriodSeconds - } - } - } + gracePeriod := setTerminationGracePeriod(pod, containerSpec, containerName, containerID, reason) if len(message) == 0 { message = fmt.Sprintf("Stopping container %s", containerSpec.Name) @@ -993,3 +976,35 @@ func (m *kubeGenericRuntimeManager) removeContainerLog(containerID string) error func (m *kubeGenericRuntimeManager) DeleteContainer(containerID kubecontainer.ContainerID) error { return m.removeContainer(containerID.ID) } + +// setTerminationGracePeriod determines the grace period to use when killing a container +func setTerminationGracePeriod(pod *v1.Pod, containerSpec *v1.Container, containerName string, containerID kubecontainer.ContainerID, reason containerKillReason) int64 { + gracePeriod := int64(minimumGracePeriodInSeconds) + switch { + case pod.DeletionGracePeriodSeconds != nil: + return *pod.DeletionGracePeriodSeconds + case pod.Spec.TerminationGracePeriodSeconds != nil: + switch reason { + case reasonStartupProbe: + if isProbeTerminationGracePeriodSecondsSet(pod, containerSpec, containerSpec.StartupProbe, containerName, containerID, "StartupProbe") { + return *containerSpec.StartupProbe.TerminationGracePeriodSeconds + } + case reasonLivenessProbe: + if isProbeTerminationGracePeriodSecondsSet(pod, containerSpec, containerSpec.LivenessProbe, containerName, containerID, "LivenessProbe") { + return *containerSpec.LivenessProbe.TerminationGracePeriodSeconds + } + } + return *pod.Spec.TerminationGracePeriodSeconds + } + return gracePeriod +} + +func isProbeTerminationGracePeriodSecondsSet(pod *v1.Pod, containerSpec *v1.Container, probe *v1.Probe, containerName string, containerID kubecontainer.ContainerID, probeType string) bool { + if probe != nil && probe.TerminationGracePeriodSeconds != nil { + if *probe.TerminationGracePeriodSeconds > *pod.Spec.TerminationGracePeriodSeconds { + klog.V(4).InfoS("Using probe-level grace period that is greater than the pod-level grace period", "pod", klog.KObj(pod), "pod-uid", pod.UID, "containerName", containerName, "containerID", containerID.String(), "probe-type", probeType, "probe-grace-period", *probe.TerminationGracePeriodSeconds, "pod-grace-period", *pod.Spec.TerminationGracePeriodSeconds) + } + return true + } + return false +} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go index 91d73097611..e0ca6b7eca9 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go @@ -466,3 +466,167 @@ func TestRestartCountByLogDir(t *testing.T) { assert.Equal(t, count, tc.restartCount, "count %v should equal restartCount %v", count, tc.restartCount) } } + +func TestKillContainerGracePeriod(t *testing.T) { + + shortGracePeriod := int64(10) + mediumGracePeriod := int64(30) + longGracePeriod := int64(60) + + tests := []struct { + name string + pod *v1.Pod + reason containerKillReason + expectedGracePeriod int64 + }{ + { + name: "default termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{Containers: []v1.Container{{Name: "foo"}}}, + }, + reason: reasonUnknown, + expectedGracePeriod: int64(2), + }, + { + name: "use pod termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{Name: "foo"}}, + TerminationGracePeriodSeconds: &longGracePeriod, + }, + }, + reason: reasonUnknown, + expectedGracePeriod: longGracePeriod, + }, + { + name: "liveness probe overrides pod termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", LivenessProbe: &v1.Probe{TerminationGracePeriodSeconds: &shortGracePeriod}, + }}, + TerminationGracePeriodSeconds: &longGracePeriod, + }, + }, + reason: reasonLivenessProbe, + expectedGracePeriod: shortGracePeriod, + }, + { + name: "startup probe overrides pod termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", StartupProbe: &v1.Probe{TerminationGracePeriodSeconds: &shortGracePeriod}, + }}, + TerminationGracePeriodSeconds: &longGracePeriod, + }, + }, + reason: reasonStartupProbe, + expectedGracePeriod: shortGracePeriod, + }, + { + name: "startup probe overrides pod termination grace period, probe period > pod period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", StartupProbe: &v1.Probe{TerminationGracePeriodSeconds: &longGracePeriod}, + }}, + TerminationGracePeriodSeconds: &shortGracePeriod, + }, + }, + reason: reasonStartupProbe, + expectedGracePeriod: longGracePeriod, + }, + { + name: "liveness probe overrides pod termination grace period, probe period > pod period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", LivenessProbe: &v1.Probe{TerminationGracePeriodSeconds: &longGracePeriod}, + }}, + TerminationGracePeriodSeconds: &shortGracePeriod, + }, + }, + reason: reasonLivenessProbe, + expectedGracePeriod: longGracePeriod, + }, + { + name: "non-liveness probe failure, use pod termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", LivenessProbe: &v1.Probe{TerminationGracePeriodSeconds: &shortGracePeriod}, + }}, + TerminationGracePeriodSeconds: &longGracePeriod, + }, + }, + reason: reasonUnknown, + expectedGracePeriod: longGracePeriod, + }, + { + name: "non-startup probe failure, use pod termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", StartupProbe: &v1.Probe{TerminationGracePeriodSeconds: &shortGracePeriod}, + }}, + TerminationGracePeriodSeconds: &longGracePeriod, + }, + }, + reason: reasonUnknown, + expectedGracePeriod: longGracePeriod, + }, + { + name: "all three grace periods set, use pod termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", + StartupProbe: &v1.Probe{TerminationGracePeriodSeconds: &shortGracePeriod}, + LivenessProbe: &v1.Probe{TerminationGracePeriodSeconds: &mediumGracePeriod}, + }}, + TerminationGracePeriodSeconds: &longGracePeriod, + }, + }, + reason: reasonUnknown, + expectedGracePeriod: longGracePeriod, + }, + { + name: "all three grace periods set, use startup termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", + StartupProbe: &v1.Probe{TerminationGracePeriodSeconds: &shortGracePeriod}, + LivenessProbe: &v1.Probe{TerminationGracePeriodSeconds: &mediumGracePeriod}, + }}, + TerminationGracePeriodSeconds: &longGracePeriod, + }, + }, + reason: reasonStartupProbe, + expectedGracePeriod: shortGracePeriod, + }, + { + name: "all three grace periods set, use liveness termination grace period", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "foo", + StartupProbe: &v1.Probe{TerminationGracePeriodSeconds: &shortGracePeriod}, + LivenessProbe: &v1.Probe{TerminationGracePeriodSeconds: &mediumGracePeriod}, + }}, + TerminationGracePeriodSeconds: &longGracePeriod, + }, + }, + reason: reasonLivenessProbe, + expectedGracePeriod: mediumGracePeriod, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actualGracePeriod := setTerminationGracePeriod(test.pod, &test.pod.Spec.Containers[0], "", kubecontainer.ContainerID{}, test.reason) + require.Equal(t, test.expectedGracePeriod, actualGracePeriod) + }) + } +}