diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index ec8045c0368..6af9e0797c5 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2816,6 +2816,19 @@ func validatePodResourceClaimSource(claimSource core.ClaimSource, fldPath *field return allErrs } +func validateReadinessProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if probe == nil { + return allErrs + } + allErrs = append(allErrs, validateProbe(probe, fldPath)...) + if probe.TerminationGracePeriodSeconds != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("terminationGracePeriodSeconds"), probe.TerminationGracePeriodSeconds, "must not be set for readinessProbes")) + } + return allErrs +} + func validateStartupProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -3223,10 +3236,7 @@ func validateInitContainers(containers []core.Container, regularContainers []cor if ctr.LivenessProbe != nil { allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers")) } - // TODO: Allow restartable init containers to have a readiness probe. - if ctr.ReadinessProbe != nil { - allErrs = append(allErrs, field.Forbidden(idxPath.Child("readinessProbe"), "may not be set for init containers")) - } + allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, idxPath.Child("readinessProbe"))...) allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, idxPath.Child("startupProbe"))...) default: @@ -3238,7 +3248,7 @@ func validateInitContainers(containers []core.Container, regularContainers []cor allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers")) } if ctr.ReadinessProbe != nil { - allErrs = append(allErrs, field.Forbidden(idxPath.Child("readinessProbe"), "may not be set for init containers")) + allErrs = append(allErrs, field.Forbidden(idxPath.Child("readinessProbe"), "may not be set for init containers without restartPolicy=Always")) } if ctr.StartupProbe != nil { allErrs = append(allErrs, field.Forbidden(idxPath.Child("startupProbe"), "may not be set for init containers without restartPolicy=Always")) @@ -3359,10 +3369,7 @@ func validateContainers(containers []core.Container, volumes map[string]core.Vol if ctr.LivenessProbe != nil && ctr.LivenessProbe.SuccessThreshold != 1 { allErrs = append(allErrs, field.Invalid(path.Child("livenessProbe", "successThreshold"), ctr.LivenessProbe.SuccessThreshold, "must be 1")) } - allErrs = append(allErrs, validateProbe(ctr.ReadinessProbe, path.Child("readinessProbe"))...) - if ctr.ReadinessProbe != nil && ctr.ReadinessProbe.TerminationGracePeriodSeconds != nil { - allErrs = append(allErrs, field.Invalid(path.Child("readinessProbe", "terminationGracePeriodSeconds"), ctr.ReadinessProbe.TerminationGracePeriodSeconds, "must not be set for readinessProbes")) - } + allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, path.Child("readinessProbe"))...) allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, path.Child("startupProbe"))...) // These fields are disallowed for regular containers diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 951384547cd..17610b0045a 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -8164,11 +8164,18 @@ func TestValidateInitContainers(t *testing.T) { ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", }, { - Name: "container-3-restart-always-with-startup-probe", + Name: "container-3-restart-always-with-probes", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", RestartPolicy: &containerRestartPolicyAlways, + ReadinessProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{ + Port: intstr.FromInt32(80), + }, + }, + }, StartupProbe: &core.Probe{ ProbeHandler: core.ProbeHandler{ TCPSocket: &core.TCPSocketAction{Port: intstr.FromInt(80)}, @@ -8392,6 +8399,25 @@ func TestValidateInitContainers(t *testing.T) { }, }}, field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].startupProbe.successThreshold", BadValue: int32(2)}}, + }, { + "invalid readiness probe, terminationGracePeriodSeconds set.", + line(), + []core.Container{{ + Name: "life-123", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + RestartPolicy: &containerRestartPolicyAlways, + ReadinessProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{ + Port: intstr.FromInt32(80), + }, + }, + TerminationGracePeriodSeconds: utilpointer.Int64(10), + }, + }}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].readinessProbe.terminationGracePeriodSeconds", BadValue: utilpointer.Int64(10)}}, }, } for _, tc := range errorCases { diff --git a/pkg/kubelet/prober/prober_manager_test.go b/pkg/kubelet/prober/prober_manager_test.go index 29d5551c443..9c8badbab52 100644 --- a/pkg/kubelet/prober/prober_manager_test.go +++ b/pkg/kubelet/prober/prober_manager_test.go @@ -152,8 +152,11 @@ func TestAddRemovePodsWithRestartableInitContainer(t *testing.T) { enableSidecarContainers: false, }, { - desc: "pod with sidecar (sidecar containers feature enabled)", - probePaths: []probeKey{{"restartable_init_container_pod", "restartable-init", readiness}}, + desc: "pod with sidecar (sidecar containers feature enabled)", + probePaths: []probeKey{ + {"restartable_init_container_pod", "restartable-init", readiness}, + {"restartable_init_container_pod", "restartable-init", startup}, + }, enableSidecarContainers: true, }, } @@ -180,6 +183,7 @@ func TestAddRemovePodsWithRestartableInitContainer(t *testing.T) { }, { Name: "restartable-init", ReadinessProbe: defaultProbe, + StartupProbe: defaultProbe, RestartPolicy: containerRestartPolicy(tc.enableSidecarContainers), }}, Containers: []v1.Container{{