diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 59f1a198e75..1b507a6bb46 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -551,6 +551,20 @@ func dropDisabledFields( }) } + if !utilfeature.DefaultFeatureGate.Enabled(features.ProbeTerminationGracePeriod) && !probeGracePeriodInUse(oldPodSpec) { + // Set pod-level terminationGracePeriodSeconds to nil if the feature is disabled and it is not used + VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool { + if c.LivenessProbe != nil { + c.LivenessProbe.TerminationGracePeriodSeconds = nil + } + if c.StartupProbe != nil { + c.StartupProbe.TerminationGracePeriodSeconds = nil + } + // cannot be set for readiness probes + return true + }) + } + dropDisabledFSGroupFields(podSpec, oldPodSpec) if !utilfeature.DefaultFeatureGate.Enabled(features.PodOverhead) && !overheadInUse(oldPodSpec) { @@ -811,6 +825,27 @@ func subpathExprInUse(podSpec *api.PodSpec) bool { return inUse } +// probeGracePeriodInUse returns true if the pod spec is non-nil and has a probe that makes use +// of the probe-level terminationGracePeriodSeconds feature +func probeGracePeriodInUse(podSpec *api.PodSpec) bool { + if podSpec == nil { + return false + } + + var inUse bool + VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool { + // cannot be set for readiness probes + if (c.LivenessProbe != nil && c.LivenessProbe.TerminationGracePeriodSeconds != nil) || + (c.StartupProbe != nil && c.StartupProbe.TerminationGracePeriodSeconds != nil) { + inUse = true + return false + } + return true + }) + + return inUse +} + // csiInUse returns true if any pod's spec include inline CSI volumes. func csiInUse(podSpec *api.PodSpec) bool { if podSpec == nil { diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 5b83e801bf2..471e3a6c92c 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -1140,6 +1140,103 @@ func TestDropSubPathExpr(t *testing.T) { } } +func TestDropProbeGracePeriod(t *testing.T) { + gracePeriod := int64(10) + probe := api.Probe{TerminationGracePeriodSeconds: &gracePeriod} + podWithProbeGracePeriod := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + Containers: []api.Container{{Name: "container1", Image: "testimage", LivenessProbe: &probe, StartupProbe: &probe}}, + }, + } + } + podWithoutProbeGracePeriod := func() *api.Pod { + p := podWithProbeGracePeriod() + p.Spec.Containers[0].LivenessProbe.TerminationGracePeriodSeconds = nil + p.Spec.Containers[0].StartupProbe.TerminationGracePeriodSeconds = nil + return p + } + + podInfo := []struct { + description string + hasGracePeriod bool + pod func() *api.Pod + }{ + { + description: "has probe-level terminationGracePeriod", + hasGracePeriod: true, + pod: podWithProbeGracePeriod, + }, + { + description: "does not have probe-level terminationGracePeriod", + hasGracePeriod: false, + pod: podWithoutProbeGracePeriod, + }, + { + description: "only has liveness probe-level terminationGracePeriod", + hasGracePeriod: true, + pod: func() *api.Pod { + p := podWithProbeGracePeriod() + p.Spec.Containers[0].StartupProbe.TerminationGracePeriodSeconds = nil + return p + }, + }, + { + description: "is nil", + hasGracePeriod: false, + pod: func() *api.Pod { return nil }, + }, + } + + enabled := true + for _, oldPodInfo := range podInfo { + for _, newPodInfo := range podInfo { + oldPodHasGracePeriod, oldPod := oldPodInfo.hasGracePeriod, oldPodInfo.pod() + newPodHasGracePeriod, newPod := newPodInfo.hasGracePeriod, newPodInfo.pod() + if newPod == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) { + + var oldPodSpec *api.PodSpec + if oldPod != nil { + oldPodSpec = &oldPod.Spec + } + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) + + // old pod should never be changed + if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { + t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod())) + } + + switch { + case enabled || oldPodHasGracePeriod: + // new pod should not be changed if the feature is enabled, or if the old pod had subpaths + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) + } + case newPodHasGracePeriod: + // new pod should be changed + if reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod was not changed") + } + // new pod should not have subpaths + if !reflect.DeepEqual(newPod, podWithoutProbeGracePeriod()) { + t.Errorf("new pod had probe-level terminationGracePeriod: %v", diff.ObjectReflectDiff(newPod, podWithoutProbeGracePeriod())) + } + default: + // new pod should not need to be changed + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) + } + } + }) + } + } +} + // helper creates a podStatus with list of PodIPs func makePodStatus(podIPs []api.PodIP) *api.PodStatus { return &api.PodStatus{ diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index c3062d3ae22..8d4d5b0e6af 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -695,6 +695,12 @@ const ( // Enables topology aware hints for EndpointSlices TopologyAwareHints featuregate.Feature = "TopologyAwareHints" + // owner: @ehashman + // alpha: v1.21 + // + // Allows user to override pod-level terminationGracePeriod for probes + ProbeTerminationGracePeriod featuregate.Feature = "ProbeTerminationGracePeriod" + // owner: @ahg-g // alpha: v1.21 // @@ -849,6 +855,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS MixedProtocolLBService: {Default: false, PreRelease: featuregate.Alpha}, VolumeCapacityPriority: {Default: false, PreRelease: featuregate.Alpha}, PreferNominatedNode: {Default: false, PreRelease: featuregate.Alpha}, + ProbeTerminationGracePeriod: {Default: false, PreRelease: featuregate.Alpha}, RunAsGroup: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.22 PodDeletionCost: {Default: false, PreRelease: featuregate.Alpha}, TopologyAwareHints: {Default: false, PreRelease: featuregate.Alpha},