Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubelet: retry pod sandbox creation when containers were never created #79451

Merged
merged 1 commit into from
Jun 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion pkg/kubelet/kuberuntime/kuberuntime_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,11 +472,15 @@ func (m *kubeGenericRuntimeManager) computePodActions(pod *v1.Pod, podStatus *ku
// If we need to (re-)create the pod sandbox, everything will need to be
// killed and recreated, and init containers should be purged.
if createPodSandbox {
if !shouldRestartOnFailure(pod) && attempt != 0 {
if !shouldRestartOnFailure(pod) && attempt != 0 && len(podStatus.ContainerStatuses) != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add check len(podStatus.InitContainerStatuses) != 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a kubelet's internal type; there's no initcontianerstatuses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unit tests added show that this would work for init containers too.

// Should not restart the pod, just return.
// we should not create a sandbox for a pod if it is already done.
// if all containers are done and should not be started, there is no need to create a new sandbox.
// this stops confusing logs on pods whose containers all have exit codes, but we recreate a sandbox before terminating it.
//
// If ContainerStatuses is empty, we assume that we've never
// successfully created any containers. In this case, we should
// retry creating the sandbox.
changes.CreateSandbox = false
return changes
}
Expand Down
50 changes: 50 additions & 0 deletions pkg/kubelet/kuberuntime/kuberuntime_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,26 @@ func TestComputePodActions(t *testing.T) {
ContainersToKill: map[kubecontainer.ContainerID]containerToKillInfo{},
},
},
"Verify we create a pod sandbox if no ready sandbox for pod with RestartPolicy=Never and no containers have ever been created": {
mutatePodFn: func(pod *v1.Pod) {
pod.Spec.RestartPolicy = v1.RestartPolicyNever
},
mutateStatusFn: func(status *kubecontainer.PodStatus) {
// no ready sandbox
status.SandboxStatuses[0].State = runtimeapi.PodSandboxState_SANDBOX_NOTREADY
status.SandboxStatuses[0].Metadata.Attempt = uint32(2)
// no visible containers
status.ContainerStatuses = []*kubecontainer.ContainerStatus{}
},
actions: podActions{
SandboxID: baseStatus.SandboxStatuses[0].Id,
Attempt: uint32(3),
CreateSandbox: true,
KillPod: true,
ContainersToStart: []int{0, 1, 2},
ContainersToKill: map[kubecontainer.ContainerID]containerToKillInfo{},
},
},
"Kill and recreate the container if the container is in unknown state": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyNever },
mutateStatusFn: func(status *kubecontainer.PodStatus) {
Expand Down Expand Up @@ -1028,6 +1048,36 @@ func TestComputePodActionsWithInitContainers(t *testing.T) {
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
},
"Pod sandbox not ready, init container failed, but RestartPolicy == Never; kill pod only": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyNever },
mutateStatusFn: func(status *kubecontainer.PodStatus) {
status.SandboxStatuses[0].State = runtimeapi.PodSandboxState_SANDBOX_NOTREADY
},
actions: podActions{
KillPod: true,
CreateSandbox: false,
SandboxID: baseStatus.SandboxStatuses[0].Id,
Attempt: uint32(1),
ContainersToStart: []int{},
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
},
"Pod sandbox not ready, and RestartPolicy == Never, but no visible init containers; create a new pod sandbox": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyNever },
mutateStatusFn: func(status *kubecontainer.PodStatus) {
status.SandboxStatuses[0].State = runtimeapi.PodSandboxState_SANDBOX_NOTREADY
status.ContainerStatuses = []*kubecontainer.ContainerStatus{}
},
actions: podActions{
KillPod: true,
CreateSandbox: true,
SandboxID: baseStatus.SandboxStatuses[0].Id,
Attempt: uint32(1),
NextInitContainerToStart: &basePod.Spec.InitContainers[0],
ContainersToStart: []int{},
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
},
} {
pod, status := makeBasePodAndStatusWithInitContainers()
if test.mutatePodFn != nil {
Expand Down