Skip to content

Commit

Permalink
Allow restartable init containers to have livenessProbe
Browse files Browse the repository at this point in the history
  • Loading branch information
@gjkim42
gjkim42 committed Jul 17, 2023
1 parent 2c8b374 commit 7ef2d67
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 11 deletions.
25 changes: 16 additions & 9 deletions pkg/apis/core/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2816,6 +2816,19 @@ func validatePodResourceClaimSource(claimSource core.ClaimSource, fldPath *field
return allErrs
}

func validateLivenessProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if probe == nil {
return allErrs
}
allErrs = append(allErrs, validateProbe(probe, fldPath)...)
if probe.SuccessThreshold != 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("successThreshold"), probe.SuccessThreshold, "must be 1"))
}
return allErrs
}

func validateReadinessProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

Expand Down Expand Up @@ -3232,10 +3245,7 @@ func validateInitContainers(containers []core.Container, regularContainers []cor
if ctr.Lifecycle != nil {
allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers"))
}
// TODO: Allow restartable init containers to have a liveness probe.
if ctr.LivenessProbe != nil {
allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers"))
}
allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, idxPath.Child("livenessProbe"))...)
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, idxPath.Child("readinessProbe"))...)
allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, idxPath.Child("startupProbe"))...)

Expand All @@ -3245,7 +3255,7 @@ func validateInitContainers(containers []core.Container, regularContainers []cor
allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers"))
}
if ctr.LivenessProbe != nil {
allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers"))
allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers without restartPolicy=Always"))
}
if ctr.ReadinessProbe != nil {
allErrs = append(allErrs, field.Forbidden(idxPath.Child("readinessProbe"), "may not be set for init containers without restartPolicy=Always"))
Expand Down Expand Up @@ -3365,10 +3375,7 @@ func validateContainers(containers []core.Container, volumes map[string]core.Vol
if ctr.Lifecycle != nil {
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, path.Child("lifecycle"))...)
}
allErrs = append(allErrs, validateProbe(ctr.LivenessProbe, path.Child("livenessProbe"))...)
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, validateLivenessProbe(ctr.LivenessProbe, path.Child("livenessProbe"))...)
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, path.Child("readinessProbe"))...)
allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, path.Child("startupProbe"))...)

Expand Down
31 changes: 30 additions & 1 deletion pkg/apis/core/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8169,6 +8169,14 @@ func TestValidateInitContainers(t *testing.T) {
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
LivenessProbe: &core.Probe{
ProbeHandler: core.ProbeHandler{
TCPSocket: &core.TCPSocketAction{
Port: intstr.FromInt32(80),
},
},
SuccessThreshold: 1,
},
ReadinessProbe: &core.Probe{
ProbeHandler: core.ProbeHandler{
TCPSocket: &core.TCPSocketAction{
Expand All @@ -8178,7 +8186,9 @@ func TestValidateInitContainers(t *testing.T) {
},
StartupProbe: &core.Probe{
ProbeHandler: core.ProbeHandler{
TCPSocket: &core.TCPSocketAction{Port: intstr.FromInt(80)},
TCPSocket: &core.TCPSocketAction{
Port: intstr.FromInt(80),
},
},
SuccessThreshold: 1,
},
Expand Down Expand Up @@ -8418,6 +8428,25 @@ func TestValidateInitContainers(t *testing.T) {
},
}},
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].readinessProbe.terminationGracePeriodSeconds", BadValue: utilpointer.Int64(10)}},
}, {
"invalid liveness probe, successThreshold != 1",
line(),
[]core.Container{{
Name: "live-123",
Image: "image",
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
RestartPolicy: &containerRestartPolicyAlways,
LivenessProbe: &core.Probe{
ProbeHandler: core.ProbeHandler{
TCPSocket: &core.TCPSocketAction{
Port: intstr.FromInt32(80),
},
},
SuccessThreshold: 2,
},
}},
field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "initContainers[0].livenessProbe.successThreshold", BadValue: int32(2)}},
},
}
for _, tc := range errorCases {
Expand Down
23 changes: 22 additions & 1 deletion pkg/kubelet/kuberuntime/kuberuntime_container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ func hasAnyRegularContainerCreated(pod *v1.Pod, podStatus *kubecontainer.PodStat
// The actions include:
// - Start the first init container that has not been started.
// - Restart all restartable init containers that have started but are not running.
// - Kill the restartable init containers that have failed the startup probe.
// - Kill the restartable init containers that are not alive or started.
func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, podStatus *kubecontainer.PodStatus, changes *podActions) bool {
if len(pod.Spec.InitContainers) == 0 {
return true
Expand Down Expand Up @@ -960,6 +960,27 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
// this init container is initialized for the first time, start the next one
changes.InitContainersToStart = append(changes.InitContainersToStart, i+1)
}

// A restartable init container does not have to take into account its
// liveness probe when it determines to start the next init container.
if container.LivenessProbe != nil {
liveness, found := m.livenessManager.Get(status.ID)
if !found {
// If the liveness probe has not been run, wait for it.
break
}
if liveness == proberesults.Failure {
// If the restartable init container failed the liveness probe,
// restart it.
changes.ContainersToKill[status.ID] = containerToKillInfo{
name: container.Name,
container: container,
message: fmt.Sprintf("Init container %s failed liveness probe", container.Name),
reason: reasonLivenessProbe,
}
changes.InitContainersToStart = append(changes.InitContainersToStart, i)
}
}
} else { // init container
// nothing do to but wait for it to finish
break
Expand Down
60 changes: 60 additions & 0 deletions pkg/kubelet/kuberuntime/kuberuntime_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1518,6 +1518,62 @@ func TestComputePodActionsWithRestartableInitContainers(t *testing.T) {
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
},
"livenessProbe has not been run; start the nothing": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways },
mutateStatusFn: func(pod *v1.Pod, status *kubecontainer.PodStatus) {
m.livenessManager.Remove(status.ContainerStatuses[1].ID)
status.ContainerStatuses = status.ContainerStatuses[:2]
},
actions: podActions{
SandboxID: baseStatus.SandboxStatuses[0].Id,
InitContainersToStart: []int{2},
ContainersToStart: []int{},
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
},
"livenessProbe in progress; start the next": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways },
mutateStatusFn: func(pod *v1.Pod, status *kubecontainer.PodStatus) {
m.livenessManager.Set(status.ContainerStatuses[1].ID, proberesults.Unknown, basePod)
status.ContainerStatuses = status.ContainerStatuses[:2]
},
actions: podActions{
SandboxID: baseStatus.SandboxStatuses[0].Id,
InitContainersToStart: []int{2},
ContainersToStart: []int{},
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
resetStatusFn: func(status *kubecontainer.PodStatus) {
m.livenessManager.Remove(status.ContainerStatuses[1].ID)
},
},
"livenessProbe has completed; start the next": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways },
mutateStatusFn: func(pod *v1.Pod, status *kubecontainer.PodStatus) {
status.ContainerStatuses = status.ContainerStatuses[:2]
},
actions: podActions{
SandboxID: baseStatus.SandboxStatuses[0].Id,
InitContainersToStart: []int{2},
ContainersToStart: []int{},
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
},
"kill and recreate the restartable init container if the liveness check has failed": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways },
mutateStatusFn: func(pod *v1.Pod, status *kubecontainer.PodStatus) {
m.livenessManager.Set(status.ContainerStatuses[2].ID, proberesults.Failure, basePod)
},
actions: podActions{
SandboxID: baseStatus.SandboxStatuses[0].Id,
InitContainersToStart: []int{2},
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{2}),
ContainersToStart: []int{0, 1, 2},
},
resetStatusFn: func(status *kubecontainer.PodStatus) {
m.livenessManager.Remove(status.ContainerStatuses[2].ID)
},
},
"startupProbe has not been run; do nothing": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways },
mutateStatusFn: func(pod *v1.Pod, status *kubecontainer.PodStatus) {
Expand Down Expand Up @@ -1740,7 +1796,9 @@ func TestComputePodActionsWithRestartableInitContainers(t *testing.T) {
},
} {
pod, status := makeBasePodAndStatusWithRestartableInitContainers()
m.livenessManager.Set(status.ContainerStatuses[1].ID, proberesults.Success, basePod)
m.startupManager.Set(status.ContainerStatuses[1].ID, proberesults.Success, basePod)
m.livenessManager.Set(status.ContainerStatuses[2].ID, proberesults.Success, basePod)
m.startupManager.Set(status.ContainerStatuses[2].ID, proberesults.Success, basePod)
if test.mutatePodFn != nil {
test.mutatePodFn(pod)
Expand Down Expand Up @@ -1769,12 +1827,14 @@ func makeBasePodAndStatusWithRestartableInitContainers() (*v1.Pod, *kubecontaine
Name: "restartable-init-2",
Image: "bar-image",
RestartPolicy: &containerRestartPolicyAlways,
LivenessProbe: &v1.Probe{},
StartupProbe: &v1.Probe{},
},
{
Name: "restartable-init-3",
Image: "bar-image",
RestartPolicy: &containerRestartPolicyAlways,
LivenessProbe: &v1.Probe{},
StartupProbe: &v1.Probe{},
},
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/kubelet/prober/prober_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func TestAddRemovePodsWithRestartableInitContainer(t *testing.T) {
{
desc: "pod with sidecar (sidecar containers feature enabled)",
probePaths: []probeKey{
{"restartable_init_container_pod", "restartable-init", liveness},
{"restartable_init_container_pod", "restartable-init", readiness},
{"restartable_init_container_pod", "restartable-init", startup},
},
Expand Down Expand Up @@ -182,6 +183,7 @@ func TestAddRemovePodsWithRestartableInitContainer(t *testing.T) {
Name: "init",
}, {
Name: "restartable-init",
LivenessProbe: defaultProbe,
ReadinessProbe: defaultProbe,
StartupProbe: defaultProbe,
RestartPolicy: containerRestartPolicy(tc.enableSidecarContainers),
Expand Down

0 comments on commit 7ef2d67

Please sign in to comment.