Skip to content

Commit d17f3ba

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#119168 from gjkim42/sidecar-allow-probes-and-lifecycle-hooks
Allow all probes and lifecycle for restartable init containers
2 parents a9b3ca3 + d029f0e commit d17f3ba

File tree

13 files changed

+1814
-99
lines changed

13 files changed

+1814
-99
lines changed

‎pkg/apis/core/validation/validation.go‎

Lines changed: 54 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2859,6 +2859,45 @@ func validatePodResourceClaimSource(claimSource core.ClaimSource, fldPath *field
28592859
return allErrs
28602860
}
28612861

2862+
func validateLivenessProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList {
2863+
allErrs := field.ErrorList{}
2864+
2865+
if probe == nil {
2866+
return allErrs
2867+
}
2868+
allErrs = append(allErrs, validateProbe(probe, fldPath)...)
2869+
if probe.SuccessThreshold != 1 {
2870+
allErrs = append(allErrs, field.Invalid(fldPath.Child("successThreshold"), probe.SuccessThreshold, "must be 1"))
2871+
}
2872+
return allErrs
2873+
}
2874+
2875+
func validateReadinessProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList {
2876+
allErrs := field.ErrorList{}
2877+
2878+
if probe == nil {
2879+
return allErrs
2880+
}
2881+
allErrs = append(allErrs, validateProbe(probe, fldPath)...)
2882+
if probe.TerminationGracePeriodSeconds != nil {
2883+
allErrs = append(allErrs, field.Invalid(fldPath.Child("terminationGracePeriodSeconds"), probe.TerminationGracePeriodSeconds, "must not be set for readinessProbes"))
2884+
}
2885+
return allErrs
2886+
}
2887+
2888+
func validateStartupProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList {
2889+
allErrs := field.ErrorList{}
2890+
2891+
if probe == nil {
2892+
return allErrs
2893+
}
2894+
allErrs = append(allErrs, validateProbe(probe, fldPath)...)
2895+
if probe.SuccessThreshold != 1 {
2896+
allErrs = append(allErrs, field.Invalid(fldPath.Child("successThreshold"), probe.SuccessThreshold, "must be 1"))
2897+
}
2898+
return allErrs
2899+
}
2900+
28622901
func validateProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList {
28632902
allErrs := field.ErrorList{}
28642903

@@ -3245,36 +3284,26 @@ func validateInitContainers(containers []core.Container, regularContainers []cor
32453284

32463285
switch {
32473286
case restartAlways:
3248-
// TODO: Allow restartable init containers to have a lifecycle hook.
32493287
if ctr.Lifecycle != nil {
3250-
allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers"))
3251-
}
3252-
// TODO: Allow restartable init containers to have a liveness probe.
3253-
if ctr.LivenessProbe != nil {
3254-
allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers"))
3255-
}
3256-
// TODO: Allow restartable init containers to have a readiness probe.
3257-
if ctr.ReadinessProbe != nil {
3258-
allErrs = append(allErrs, field.Forbidden(idxPath.Child("readinessProbe"), "may not be set for init containers"))
3259-
}
3260-
allErrs = append(allErrs, validateProbe(ctr.StartupProbe, idxPath.Child("startupProbe"))...)
3261-
if ctr.StartupProbe != nil && ctr.StartupProbe.SuccessThreshold != 1 {
3262-
allErrs = append(allErrs, field.Invalid(idxPath.Child("startupProbe", "successThreshold"), ctr.StartupProbe.SuccessThreshold, "must be 1"))
3288+
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, idxPath.Child("lifecycle"))...)
32633289
}
3290+
allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, idxPath.Child("livenessProbe"))...)
3291+
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, idxPath.Child("readinessProbe"))...)
3292+
allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, idxPath.Child("startupProbe"))...)
32643293

32653294
default:
32663295
// These fields are disallowed for init containers.
32673296
if ctr.Lifecycle != nil {
3268-
allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers"))
3297+
allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers without restartPolicy=Always"))
32693298
}
32703299
if ctr.LivenessProbe != nil {
3271-
allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers"))
3300+
allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers without restartPolicy=Always"))
32723301
}
32733302
if ctr.ReadinessProbe != nil {
3274-
allErrs = append(allErrs, field.Forbidden(idxPath.Child("readinessProbe"), "may not be set for init containers"))
3303+
allErrs = append(allErrs, field.Forbidden(idxPath.Child("readinessProbe"), "may not be set for init containers without restartPolicy=Always"))
32753304
}
32763305
if ctr.StartupProbe != nil {
3277-
allErrs = append(allErrs, field.Forbidden(idxPath.Child("startupProbe"), "may not be set for init containers"))
3306+
allErrs = append(allErrs, field.Forbidden(idxPath.Child("startupProbe"), "may not be set for init containers without restartPolicy=Always"))
32783307
}
32793308
}
32803309

@@ -3383,23 +3412,16 @@ func validateContainers(containers []core.Container, volumes map[string]core.Vol
33833412
allNames.Insert(ctr.Name)
33843413
}
33853414

3386-
// These fields are only allowed for regular containers, so only check supported values here.
3387-
// Init and ephemeral container validation will return field.Forbidden() for these paths.
3415+
// These fields are allowed for regular containers and restartable init
3416+
// containers.
3417+
// Regular init container and ephemeral container validation will return
3418+
// field.Forbidden() for these paths.
33883419
if ctr.Lifecycle != nil {
33893420
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, path.Child("lifecycle"))...)
33903421
}
3391-
allErrs = append(allErrs, validateProbe(ctr.LivenessProbe, path.Child("livenessProbe"))...)
3392-
if ctr.LivenessProbe != nil && ctr.LivenessProbe.SuccessThreshold != 1 {
3393-
allErrs = append(allErrs, field.Invalid(path.Child("livenessProbe", "successThreshold"), ctr.LivenessProbe.SuccessThreshold, "must be 1"))
3394-
}
3395-
allErrs = append(allErrs, validateProbe(ctr.ReadinessProbe, path.Child("readinessProbe"))...)
3396-
if ctr.ReadinessProbe != nil && ctr.ReadinessProbe.TerminationGracePeriodSeconds != nil {
3397-
allErrs = append(allErrs, field.Invalid(path.Child("readinessProbe", "terminationGracePeriodSeconds"), ctr.ReadinessProbe.TerminationGracePeriodSeconds, "must not be set for readinessProbes"))
3398-
}
3399-
allErrs = append(allErrs, validateProbe(ctr.StartupProbe, path.Child("startupProbe"))...)
3400-
if ctr.StartupProbe != nil && ctr.StartupProbe.SuccessThreshold != 1 {
3401-
allErrs = append(allErrs, field.Invalid(path.Child("startupProbe", "successThreshold"), ctr.StartupProbe.SuccessThreshold, "must be 1"))
3402-
}
3422+
allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, path.Child("livenessProbe"))...)
3423+
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, path.Child("readinessProbe"))...)
3424+
allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, path.Child("startupProbe"))...)
34033425

34043426
// These fields are disallowed for regular containers
34053427
if ctr.RestartPolicy != nil {

‎pkg/apis/core/validation/validation_test.go‎

Lines changed: 190 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8162,14 +8162,43 @@ func TestValidateInitContainers(t *testing.T) {
81628162
ImagePullPolicy: "IfNotPresent",
81638163
TerminationMessagePolicy: "File",
81648164
}, {
8165-
Name: "container-3-restart-always-with-startup-probe",
8165+
Name: "container-3-restart-always-with-lifecycle-hook-and-probes",
81668166
Image: "image",
81678167
ImagePullPolicy: "IfNotPresent",
81688168
TerminationMessagePolicy: "File",
81698169
RestartPolicy: &containerRestartPolicyAlways,
8170+
Lifecycle: &core.Lifecycle{
8171+
PostStart: &core.LifecycleHandler{
8172+
Exec: &core.ExecAction{
8173+
Command: []string{"echo", "post start"},
8174+
},
8175+
},
8176+
PreStop: &core.LifecycleHandler{
8177+
Exec: &core.ExecAction{
8178+
Command: []string{"echo", "pre stop"},
8179+
},
8180+
},
8181+
},
8182+
LivenessProbe: &core.Probe{
8183+
ProbeHandler: core.ProbeHandler{
8184+
TCPSocket: &core.TCPSocketAction{
8185+
Port: intstr.FromInt32(80),
8186+
},
8187+
},
8188+
SuccessThreshold: 1,
8189+
},
8190+
ReadinessProbe: &core.Probe{
8191+
ProbeHandler: core.ProbeHandler{
8192+
TCPSocket: &core.TCPSocketAction{
8193+
Port: intstr.FromInt32(80),
8194+
},
8195+
},
8196+
},
81708197
StartupProbe: &core.Probe{
81718198
ProbeHandler: core.ProbeHandler{
8172-
TCPSocket: &core.TCPSocketAction{Port: intstr.FromInt(80)},
8199+
TCPSocket: &core.TCPSocketAction{
8200+
Port: intstr.FromInt(80),
8201+
},
81738202
},
81748203
SuccessThreshold: 1,
81758204
},
@@ -8390,6 +8419,165 @@ func TestValidateInitContainers(t *testing.T) {
83908419
},
83918420
}},
83928421
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].startupProbe.successThreshold", BadValue: int32(2)}},
8422+
}, {
8423+
"invalid readiness probe, terminationGracePeriodSeconds set.",
8424+
line(),
8425+
[]core.Container{{
8426+
Name: "life-123",
8427+
Image: "image",
8428+
ImagePullPolicy: "IfNotPresent",
8429+
TerminationMessagePolicy: "File",
8430+
RestartPolicy: &containerRestartPolicyAlways,
8431+
ReadinessProbe: &core.Probe{
8432+
ProbeHandler: core.ProbeHandler{
8433+
TCPSocket: &core.TCPSocketAction{
8434+
Port: intstr.FromInt32(80),
8435+
},
8436+
},
8437+
TerminationGracePeriodSeconds: utilpointer.Int64(10),
8438+
},
8439+
}},
8440+
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].readinessProbe.terminationGracePeriodSeconds", BadValue: utilpointer.Int64(10)}},
8441+
}, {
8442+
"invalid liveness probe, successThreshold != 1",
8443+
line(),
8444+
[]core.Container{{
8445+
Name: "live-123",
8446+
Image: "image",
8447+
ImagePullPolicy: "IfNotPresent",
8448+
TerminationMessagePolicy: "File",
8449+
RestartPolicy: &containerRestartPolicyAlways,
8450+
LivenessProbe: &core.Probe{
8451+
ProbeHandler: core.ProbeHandler{
8452+
TCPSocket: &core.TCPSocketAction{
8453+
Port: intstr.FromInt32(80),
8454+
},
8455+
},
8456+
SuccessThreshold: 2,
8457+
},
8458+
}},
8459+
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].livenessProbe.successThreshold", BadValue: int32(2)}},
8460+
}, {
8461+
"invalid lifecycle, no exec command.",
8462+
line(),
8463+
[]core.Container{{
8464+
Name: "life-123",
8465+
Image: "image",
8466+
ImagePullPolicy: "IfNotPresent",
8467+
TerminationMessagePolicy: "File",
8468+
RestartPolicy: &containerRestartPolicyAlways,
8469+
Lifecycle: &core.Lifecycle{
8470+
PreStop: &core.LifecycleHandler{
8471+
Exec: &core.ExecAction{},
8472+
},
8473+
},
8474+
}},
8475+
field.ErrorList{{Type: field.ErrorTypeRequired, Field: "initContainers[0].lifecycle.preStop.exec.command", BadValue: ""}},
8476+
}, {
8477+
"invalid lifecycle, no http path.",
8478+
line(),
8479+
[]core.Container{{
8480+
Name: "life-123",
8481+
Image: "image",
8482+
ImagePullPolicy: "IfNotPresent",
8483+
TerminationMessagePolicy: "File",
8484+
RestartPolicy: &containerRestartPolicyAlways,
8485+
Lifecycle: &core.Lifecycle{
8486+
PreStop: &core.LifecycleHandler{
8487+
HTTPGet: &core.HTTPGetAction{
8488+
Port: intstr.FromInt32(80),
8489+
Scheme: "HTTP",
8490+
},
8491+
},
8492+
},
8493+
}},
8494+
field.ErrorList{{Type: field.ErrorTypeRequired, Field: "initContainers[0].lifecycle.preStop.httpGet.path", BadValue: ""}},
8495+
}, {
8496+
"invalid lifecycle, no http port.",
8497+
line(),
8498+
[]core.Container{{
8499+
Name: "life-123",
8500+
Image: "image",
8501+
ImagePullPolicy: "IfNotPresent",
8502+
TerminationMessagePolicy: "File",
8503+
RestartPolicy: &containerRestartPolicyAlways,
8504+
Lifecycle: &core.Lifecycle{
8505+
PreStop: &core.LifecycleHandler{
8506+
HTTPGet: &core.HTTPGetAction{
8507+
Path: "/",
8508+
Scheme: "HTTP",
8509+
},
8510+
},
8511+
},
8512+
}},
8513+
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].lifecycle.preStop.httpGet.port", BadValue: 0}},
8514+
}, {
8515+
"invalid lifecycle, no http scheme.",
8516+
line(),
8517+
[]core.Container{{
8518+
Name: "life-123",
8519+
Image: "image",
8520+
ImagePullPolicy: "IfNotPresent",
8521+
TerminationMessagePolicy: "File",
8522+
RestartPolicy: &containerRestartPolicyAlways,
8523+
Lifecycle: &core.Lifecycle{
8524+
PreStop: &core.LifecycleHandler{
8525+
HTTPGet: &core.HTTPGetAction{
8526+
Path: "/",
8527+
Port: intstr.FromInt32(80),
8528+
},
8529+
},
8530+
},
8531+
}},
8532+
field.ErrorList{{Type: field.ErrorTypeNotSupported, Field: "initContainers[0].lifecycle.preStop.httpGet.scheme", BadValue: core.URIScheme("")}},
8533+
}, {
8534+
"invalid lifecycle, no tcp socket port.",
8535+
line(),
8536+
[]core.Container{{
8537+
Name: "life-123",
8538+
Image: "image",
8539+
ImagePullPolicy: "IfNotPresent",
8540+
TerminationMessagePolicy: "File",
8541+
RestartPolicy: &containerRestartPolicyAlways,
8542+
Lifecycle: &core.Lifecycle{
8543+
PreStop: &core.LifecycleHandler{
8544+
TCPSocket: &core.TCPSocketAction{},
8545+
},
8546+
},
8547+
}},
8548+
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].lifecycle.preStop.tcpSocket.port", BadValue: 0}},
8549+
}, {
8550+
"invalid lifecycle, zero tcp socket port.",
8551+
line(),
8552+
[]core.Container{{
8553+
Name: "life-123",
8554+
Image: "image",
8555+
ImagePullPolicy: "IfNotPresent",
8556+
TerminationMessagePolicy: "File",
8557+
RestartPolicy: &containerRestartPolicyAlways,
8558+
Lifecycle: &core.Lifecycle{
8559+
PreStop: &core.LifecycleHandler{
8560+
TCPSocket: &core.TCPSocketAction{
8561+
Port: intstr.FromInt32(0),
8562+
},
8563+
},
8564+
},
8565+
}},
8566+
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].lifecycle.preStop.tcpSocket.port", BadValue: 0}},
8567+
}, {
8568+
"invalid lifecycle, no action.",
8569+
line(),
8570+
[]core.Container{{
8571+
Name: "life-123",
8572+
Image: "image",
8573+
ImagePullPolicy: "IfNotPresent",
8574+
TerminationMessagePolicy: "File",
8575+
RestartPolicy: &containerRestartPolicyAlways,
8576+
Lifecycle: &core.Lifecycle{
8577+
PreStop: &core.LifecycleHandler{},
8578+
},
8579+
}},
8580+
field.ErrorList{{Type: field.ErrorTypeRequired, Field: "initContainers[0].lifecycle.preStop", BadValue: ""}},
83938581
},
83948582
}
83958583
for _, tc := range errorCases {

‎pkg/kubelet/kubelet_pods.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,9 +1738,10 @@ func (kl *Kubelet) generateAPIPodStatus(pod *v1.Pod, podStatus *kubecontainer.Po
17381738
if utilfeature.DefaultFeatureGate.Enabled(features.PodReadyToStartContainersCondition) {
17391739
s.Conditions = append(s.Conditions, status.GeneratePodReadyToStartContainersCondition(pod, podStatus))
17401740
}
1741-
s.Conditions = append(s.Conditions, status.GeneratePodInitializedCondition(&pod.Spec, append(s.InitContainerStatuses, s.ContainerStatuses...), s.Phase))
1742-
s.Conditions = append(s.Conditions, status.GeneratePodReadyCondition(&pod.Spec, s.Conditions, s.ContainerStatuses, s.Phase))
1743-
s.Conditions = append(s.Conditions, status.GenerateContainersReadyCondition(&pod.Spec, s.ContainerStatuses, s.Phase))
1741+
allContainerStatuses := append(s.InitContainerStatuses, s.ContainerStatuses...)
1742+
s.Conditions = append(s.Conditions, status.GeneratePodInitializedCondition(&pod.Spec, allContainerStatuses, s.Phase))
1743+
s.Conditions = append(s.Conditions, status.GeneratePodReadyCondition(&pod.Spec, s.Conditions, allContainerStatuses, s.Phase))
1744+
s.Conditions = append(s.Conditions, status.GenerateContainersReadyCondition(&pod.Spec, allContainerStatuses, s.Phase))
17441745
s.Conditions = append(s.Conditions, v1.PodCondition{
17451746
Type: v1.PodScheduled,
17461747
Status: v1.ConditionTrue,

‎pkg/kubelet/kuberuntime/kuberuntime_container.go‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ func hasAnyRegularContainerCreated(pod *v1.Pod, podStatus *kubecontainer.PodStat
877877
// The actions include:
878878
// - Start the first init container that has not been started.
879879
// - Restart all restartable init containers that have started but are not running.
880-
// - Kill the restartable init containers that have failed the startup probe.
880+
// - Kill the restartable init containers that are not alive or started.
881881
func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, podStatus *kubecontainer.PodStatus, changes *podActions) bool {
882882
if len(pod.Spec.InitContainers) == 0 {
883883
return true
@@ -960,6 +960,27 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
960960
// this init container is initialized for the first time, start the next one
961961
changes.InitContainersToStart = append(changes.InitContainersToStart, i+1)
962962
}
963+
964+
// A restartable init container does not have to take into account its
965+
// liveness probe when it determines to start the next init container.
966+
if container.LivenessProbe != nil {
967+
liveness, found := m.livenessManager.Get(status.ID)
968+
if !found {
969+
// If the liveness probe has not been run, wait for it.
970+
break
971+
}
972+
if liveness == proberesults.Failure {
973+
// If the restartable init container failed the liveness probe,
974+
// restart it.
975+
changes.ContainersToKill[status.ID] = containerToKillInfo{
976+
name: container.Name,
977+
container: container,
978+
message: fmt.Sprintf("Init container %s failed liveness probe", container.Name),
979+
reason: reasonLivenessProbe,
980+
}
981+
changes.InitContainersToStart = append(changes.InitContainersToStart, i)
982+
}
983+
}
963984
} else { // init container
964985
// nothing do to but wait for it to finish
965986
break

0 commit comments

Comments
 (0)