diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index 0289cedeb3e..f40d5320520 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -250,5 +250,9 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta return true }) + // warn if the terminationGracePeriodSeconds is negative. + if podSpec.TerminationGracePeriodSeconds != nil && *podSpec.TerminationGracePeriodSeconds < 0 { + warnings = append(warnings, fmt.Sprintf("%s: must be >= 0; negative values are invalid and will be treated as 1", fieldPath.Child("spec", "terminationGracePeriodSeconds"))) + } return warnings } diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index 6b28eac14dd..b1b814a4f12 100644 --- a/pkg/api/pod/warnings_test.go +++ b/pkg/api/pod/warnings_test.go @@ -24,6 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" api "k8s.io/kubernetes/pkg/apis/core" + utilpointer "k8s.io/utils/pointer" ) func BenchmarkNoWarnings(b *testing.B) { @@ -489,6 +490,18 @@ func TestWarnings(t *testing.T) { `spec.volumes[0].ephemeral.volumeClaimTemplate.spec.resources.requests[storage]: fractional byte value "200m" is invalid, must be an integer`, }, }, + { + name: "terminationGracePeriodSeconds is negative", + template: &api.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: api.PodSpec{ + TerminationGracePeriodSeconds: utilpointer.Int64Ptr(-1), + }, + }, + expected: []string{ + `spec.terminationGracePeriodSeconds: must be >= 0; negative values are invalid and will be treated as 1`, + }, + }, } for _, tc := range testcases { diff --git a/pkg/apis/core/v1/conversion.go b/pkg/apis/core/v1/conversion.go index 7869f0389b9..dd92428cdf3 100644 --- a/pkg/apis/core/v1/conversion.go +++ b/pkg/apis/core/v1/conversion.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/apis/apps" "k8s.io/kubernetes/pkg/apis/core" + utilpointer "k8s.io/utils/pointer" ) func addConversionFuncs(scheme *runtime.Scheme) error { @@ -372,6 +373,11 @@ func Convert_v1_Pod_To_core_Pod(in *v1.Pod, out *core.Pod, s conversion.Scope) e // drop init container annotations so they don't show up as differences when receiving requests from old clients out.Annotations = dropInitContainerAnnotations(out.Annotations) + // Forcing the value of TerminationGracePeriodSeconds to 1 if it is negative. + // Just for Pod, not for PodSpec, because we don't want to change the behavior of the PodTemplate. + if in.Spec.TerminationGracePeriodSeconds != nil && *in.Spec.TerminationGracePeriodSeconds < 0 { + out.Spec.TerminationGracePeriodSeconds = utilpointer.Int64(1) + } return nil } @@ -384,6 +390,11 @@ func Convert_core_Pod_To_v1_Pod(in *core.Pod, out *v1.Pod, s conversion.Scope) e // remove this once the oldest supported kubelet no longer honors the annotations over the field. out.Annotations = dropInitContainerAnnotations(out.Annotations) + // Forcing the value of TerminationGracePeriodSeconds to 1 if it is negative. + // Just for Pod, not for PodSpec, because we don't want to change the behavior of the PodTemplate. + if in.Spec.TerminationGracePeriodSeconds != nil && *in.Spec.TerminationGracePeriodSeconds < 0 { + out.Spec.TerminationGracePeriodSeconds = utilpointer.Int64(1) + } return nil } diff --git a/pkg/apis/core/v1/conversion_test.go b/pkg/apis/core/v1/conversion_test.go index 68161256293..77e14f2711f 100644 --- a/pkg/apis/core/v1/conversion_test.go +++ b/pkg/apis/core/v1/conversion_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/apitesting/fuzzer" @@ -704,3 +705,82 @@ func Test_v1_NodeSpec_to_core_NodeSpec(t *testing.T) { } } } + +func TestConvert_v1_Pod_To_core_Pod(t *testing.T) { + type args struct { + in *v1.Pod + out *core.Pod + } + tests := []struct { + name string + args args + wantErr bool + wantOut *core.Pod + }{ + { + args: args{ + in: &v1.Pod{ + Spec: v1.PodSpec{ + TerminationGracePeriodSeconds: utilpointer.Int64(-1), + }, + }, + out: &core.Pod{}, + }, + wantOut: &core.Pod{ + Spec: core.PodSpec{ + TerminationGracePeriodSeconds: utilpointer.Int64(1), + SecurityContext: &core.PodSecurityContext{}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := corev1.Convert_v1_Pod_To_core_Pod(tt.args.in, tt.args.out, nil); (err != nil) != tt.wantErr { + t.Errorf("Convert_v1_Pod_To_core_Pod() error = %v, wantErr %v", err, tt.wantErr) + } + if diff := cmp.Diff(tt.args.out, tt.wantOut); diff != "" { + t.Errorf("Convert_v1_Pod_To_core_Pod() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestConvert_core_Pod_To_v1_Pod(t *testing.T) { + type args struct { + in *core.Pod + out *v1.Pod + } + tests := []struct { + name string + args args + wantErr bool + wantOut *v1.Pod + }{ + { + args: args{ + in: &core.Pod{ + Spec: core.PodSpec{ + TerminationGracePeriodSeconds: utilpointer.Int64(-1), + }, + }, + out: &v1.Pod{}, + }, + wantOut: &v1.Pod{ + Spec: v1.PodSpec{ + TerminationGracePeriodSeconds: utilpointer.Int64(1), + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := corev1.Convert_core_Pod_To_v1_Pod(tt.args.in, tt.args.out, nil); (err != nil) != tt.wantErr { + t.Errorf("Convert_core_Pod_To_v1_Pod() error = %v, wantErr %v", err, tt.wantErr) + } + if diff := cmp.Diff(tt.args.out, tt.wantOut); diff != "" { + t.Errorf("Convert_core_Pod_To_v1_Pod() mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/test/e2e/node/pods.go b/test/e2e/node/pods.go index dc1fc02029f..b38f8d18e3c 100644 --- a/test/e2e/node/pods.go +++ b/test/e2e/node/pods.go @@ -44,6 +44,7 @@ import ( e2epod "k8s.io/kubernetes/test/e2e/framework/pod" imageutils "k8s.io/kubernetes/test/utils/image" admissionapi "k8s.io/pod-security-admission/api" + utilpointer "k8s.io/utils/pointer" "github.com/onsi/ginkgo/v2" "github.com/prometheus/client_golang/prometheus" @@ -341,6 +342,76 @@ var _ = SIGDescribe("Pods Extended", func() { }) }) + ginkgo.Describe("Pod TerminationGracePeriodSeconds is negative", func() { + var podClient *e2epod.PodClient + ginkgo.BeforeEach(func() { + podClient = e2epod.NewPodClient(f) + }) + + ginkgo.It("pod with negative grace period", func(ctx context.Context) { + name := "pod-negative-grace-period" + string(uuid.NewUUID()) + image := imageutils.GetE2EImage(imageutils.BusyBox) + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: v1.PodSpec{ + RestartPolicy: v1.RestartPolicyOnFailure, + Containers: []v1.Container{ + { + Name: "foo", + Image: image, + Command: []string{ + "/bin/sh", "-c", "sleep 10000", + }, + }, + }, + TerminationGracePeriodSeconds: utilpointer.Int64(-1), + }, + } + + ginkgo.By("submitting the pod to kubernetes") + podClient.Create(ctx, pod) + + pod, err := podClient.Get(ctx, pod.Name, metav1.GetOptions{}) + framework.ExpectNoError(err, "failed to query for pod") + + if pod.Spec.TerminationGracePeriodSeconds == nil { + framework.Failf("pod spec TerminationGracePeriodSeconds is nil") + } + + if *pod.Spec.TerminationGracePeriodSeconds != 1 { + framework.Failf("pod spec TerminationGracePeriodSeconds is not 1: %d", *pod.Spec.TerminationGracePeriodSeconds) + } + + time.Sleep(5 * time.Second) + + pod, err = podClient.Get(ctx, pod.Name, metav1.GetOptions{}) + framework.ExpectNoError(err, "failed to query for pod") + + ginkgo.By("updating the pod to have a negative TerminationGracePeriodSeconds") + pod.Spec.TerminationGracePeriodSeconds = utilpointer.Int64(-1) + _, err = podClient.PodInterface.Update(ctx, pod, metav1.UpdateOptions{}) + framework.ExpectNoError(err, "failed to update pod") + + pod, err = podClient.Get(ctx, pod.Name, metav1.GetOptions{}) + framework.ExpectNoError(err, "failed to query for pod") + + if pod.Spec.TerminationGracePeriodSeconds == nil { + framework.Failf("pod spec TerminationGracePeriodSeconds is nil") + } + + if *pod.Spec.TerminationGracePeriodSeconds != 1 { + framework.Failf("pod spec TerminationGracePeriodSeconds is not 1: %d", *pod.Spec.TerminationGracePeriodSeconds) + } + + ginkgo.DeferCleanup(func(ctx context.Context) error { + ginkgo.By("deleting the pod") + return podClient.Delete(ctx, pod.Name, metav1.DeleteOptions{}) + }) + }) + }) + }) func createAndTestPodRepeatedly(ctx context.Context, workers, iterations int, scenario podScenario, podClient v1core.PodInterface) {