Avoid SidecarContainers code path for non-sidecar pods

This fixes a regression in the SidecarContainers feature by minimizing
the impact of the new code path. Use the old code path for pods without
restartable init containers, and apply the new code path only to pods
with restartable init containers.

Author: gjkim42

pkg/kubelet/kuberuntime/kuberuntime_container.go

@@ -832,7 +832,7 @@ func (m *kubeGenericRuntimeManager) killContainersWithSyncResult(ctx context.Con
 	wg.Add(len(runningPod.Containers))
 	var termOrdering *terminationOrdering
 	// we only care about container termination ordering if the sidecars feature is enabled
-	if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) && types.HasRestartableInitContainer(pod) {
 		var runningContainerNames []string
 		for _, container := range runningPod.Containers {
 			runningContainerNames = append(runningContainerNames, container.Name)

pkg/kubelet/kuberuntime/kuberuntime_manager.go

@@ -833,6 +833,8 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
 		ContainersToKill:  make(map[kubecontainer.ContainerID]containerToKillInfo),
 	}
 
+	handleRestartableInitContainers := utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) && types.HasRestartableInitContainer(pod)
+
 	// 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 {
@@ -862,7 +864,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
 		// is done and there is no container to start.
 		if len(containersToStart) == 0 {
 			hasInitialized := false
-			if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
+			if !handleRestartableInitContainers {
 				_, _, hasInitialized = findNextInitContainerToRun(pod, podStatus)
 			} else {
 				// If there is any regular container, it means all init containers have
@@ -880,7 +882,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
 		// state.
 		if len(pod.Spec.InitContainers) != 0 {
 			// Pod has init containers, return the first one.
-			if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
+			if !handleRestartableInitContainers {
 				changes.NextInitContainerToStart = &pod.Spec.InitContainers[0]
 			} else {
 				changes.InitContainersToStart = []int{0}
@@ -903,7 +905,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
 	}
 
 	// Check initialization progress.
-	if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
+	if !handleRestartableInitContainers {
 		initLastStatus, next, done := findNextInitContainerToRun(pod, podStatus)
 		if !done {
 			if next != nil {
@@ -1027,7 +1029,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
 
 	if keepCount == 0 && len(changes.ContainersToStart) == 0 {
 		changes.KillPod = true
-		if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
+		if handleRestartableInitContainers {
 			// To prevent the restartable init containers to keep pod alive, we should
 			// not restart them.
 			changes.InitContainersToStart = nil
@@ -1285,7 +1287,7 @@ func (m *kubeGenericRuntimeManager) SyncPod(ctx context.Context, pod *v1.Pod, po
 		start(ctx, "ephemeral container", metrics.EphemeralContainer, ephemeralContainerStartSpec(&pod.Spec.EphemeralContainers[idx]))
 	}
 
-	if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
+	if !types.HasRestartableInitContainer(pod) {
 		// Step 6: start the init container.
 		if container := podContainerChanges.NextInitContainerToStart; container != nil {
 			// Start the next init container.

pkg/kubelet/kuberuntime/kuberuntime_manager_test.go

@@ -51,6 +51,7 @@ import (
 	containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
 	imagetypes "k8s.io/kubernetes/pkg/kubelet/images"
 	proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
+	kubelettypes "k8s.io/kubernetes/pkg/kubelet/types"
 )
 
 var (
@@ -1428,6 +1429,20 @@ func testComputePodActionsWithInitContainers(t *testing.T, sidecarContainersEnab
 				ContainersToKill:  getKillMapWithInitContainers(basePod, baseStatus, []int{}),
 			},
 		},
+		"an init container is in the created state due to an unknown error when starting container; restart it": {
+			mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways },
+			mutateStatusFn: func(status *kubecontainer.PodStatus) {
+				status.ContainerStatuses[2].State = kubecontainer.ContainerStateCreated
+			},
+			actions: podActions{
+				KillPod:                  false,
+				SandboxID:                baseStatus.SandboxStatuses[0].Id,
+				NextInitContainerToStart: &basePod.Spec.InitContainers[2],
+				InitContainersToStart:    []int{2},
+				ContainersToStart:        []int{},
+				ContainersToKill:         getKillMapWithInitContainers(basePod, baseStatus, []int{}),
+			},
+		},
 	} {
 		pod, status := makeBasePodAndStatusWithInitContainers()
 		if test.mutatePodFn != nil {
@@ -1438,12 +1453,15 @@ func testComputePodActionsWithInitContainers(t *testing.T, sidecarContainersEnab
 		}
 		ctx := context.Background()
 		actions := m.computePodActions(ctx, pod, status)
-		if !sidecarContainersEnabled {
-			// If sidecar containers are disabled, we should not see any
+		handleRestartableInitContainers := sidecarContainersEnabled && kubelettypes.HasRestartableInitContainer(pod)
+		if !handleRestartableInitContainers {
+			// If sidecar containers are disabled or the pod does not have any
+			// restartable init container, we should not see any
 			// InitContainersToStart in the actions.
 			test.actions.InitContainersToStart = nil
 		} else {
-			// If sidecar containers are enabled, we should not see any
+			// If sidecar containers are enabled and the pod has any
+			// restartable init container, we should not see any
 			// NextInitContainerToStart in the actions.
 			test.actions.NextInitContainerToStart = nil
 		}
@@ -2041,12 +2059,15 @@ func testComputePodActionsWithInitAndEphemeralContainers(t *testing.T, sidecarCo
 		}
 		ctx := context.Background()
 		actions := m.computePodActions(ctx, pod, status)
-		if !sidecarContainersEnabled {
-			// If sidecar containers are disabled, we should not see any
+		handleRestartableInitContainers := sidecarContainersEnabled && kubelettypes.HasRestartableInitContainer(pod)
+		if !handleRestartableInitContainers {
+			// If sidecar containers are disabled or the pod does not have any
+			// restartable init container, we should not see any
 			// InitContainersToStart in the actions.
 			test.actions.InitContainersToStart = nil
 		} else {
-			// If sidecar containers are enabled, we should not see any
+			// If sidecar containers are enabled and the pod has any
+			// restartable init container, we should not see any
 			// NextInitContainerToStart in the actions.
 			test.actions.NextInitContainerToStart = nil
 		}

pkg/kubelet/types/pod_update.go

@@ -202,3 +202,14 @@ func IsRestartableInitContainer(initContainer *v1.Container) bool {
 
 	return *initContainer.RestartPolicy == v1.ContainerRestartPolicyAlways
 }
+
+// HasRestartableInitContainer returns true if the pod has any restartable init
+// container
+func HasRestartableInitContainer(pod *v1.Pod) bool {
+	for _, container := range pod.Spec.InitContainers {
+		if IsRestartableInitContainer(&container) {
+			return true
+		}
+	}
+	return false
+}