Change order kubelet starts containers

This starts ephemeral containers prior to init containers so that
ephemeral containers will still be started when init containers fail to
start.

Also improves tests and comments with review suggestions.

pkg/kubelet/container/helpers_test.go

@@ -383,10 +383,12 @@ func TestGetContainerSpec(t *testing.T) {
 			wantContainer: &v1.Container{Name: "debug-container"},
 		},
 	} {
+		t.Run(tc.name, func(t *testing.T) {
 			gotContainer := GetContainerSpec(tc.havePod, tc.haveName)
 			if diff := cmp.Diff(tc.wantContainer, gotContainer); diff != "" {
-			t.Errorf("GetContainerSpec for %q returned diff (-want +got):%v", tc.name, diff)
+				t.Fatalf("GetContainerSpec for %q returned diff (-want +got):%v", tc.name, diff)
 			}
+		})
 	}
 }
 

pkg/kubelet/kubelet_pods.go

@@ -1420,7 +1420,7 @@ func (kl *Kubelet) convertStatusToAPIStatus(pod *v1.Pod, podStatus *kubecontaine
 			ecSpecs = append(ecSpecs, v1.Container(pod.Spec.EphemeralContainers[i].EphemeralContainerCommon))
 		}
 
-		// TODO(verb): By now we've iterated podStatus 3 times. We could refactor this to make a single
+		// #80875: By now we've iterated podStatus 3 times. We could refactor this to make a single
 		// pass through podStatus.ContainerStatuses
 		apiPodStatus.EphemeralContainerStatuses = kl.convertToAPIContainerStatuses(
 			pod, podStatus,

pkg/kubelet/kuberuntime/kuberuntime_manager.go

@@ -625,9 +625,9 @@ func (m *kubeGenericRuntimeManager) computePodActions(pod *v1.Pod, podStatus *ku
 //  2. Kill pod sandbox if necessary.
 //  3. Kill any containers that should not be running.
 //  4. Create sandbox if necessary.
-//  5. Create init containers.
+//  5. Create ephemeral containers.
-//  6. Create normal containers.
+//  6. Create init containers.
-//  7. Create ephemeral containers.
+//  7. Create normal containers.
 func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontainer.PodStatus, pullSecrets []v1.Secret, backOff *flowcontrol.Backoff) (result kubecontainer.PodSyncResult) {
 	// Step 1: Compute sandbox and container changes.
 	podContainerChanges := m.computePodActions(pod, podStatus)
@@ -758,7 +758,8 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine
 	}
 
 	// Helper containing boilerplate common to starting all types of containers.
-	// typeName is a label used to describe this type of container in log messages.
+	// typeName is a label used to describe this type of container in log messages,
+	// currently: "container", "init container" or "ephemeral container"
 	start := func(typeName string, container *v1.Container) error {
 		startContainerResult := kubecontainer.NewSyncResult(kubecontainer.StartContainer, container.Name)
 		result.AddSyncResult(startContainerResult)
@@ -787,7 +788,18 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine
 		return nil
 	}
 
-	// Step 5: start the init container.
+	// Step 5: start ephemeral containers
+	// These are started "prior" to init containers to allow running ephemeral containers even when there
+	// are errors starting an init container. In practice init containers will start first since ephemeral
+	// containers cannot be specified on pod creation.
+	if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) {
+		for _, idx := range podContainerChanges.EphemeralContainersToStart {
+			c := (*v1.Container)(&pod.Spec.EphemeralContainers[idx].EphemeralContainerCommon)
+			start("ephemeral container", c)
+		}
+	}
+
+	// Step 6: start the init container.
 	if container := podContainerChanges.NextInitContainerToStart; container != nil {
 		// Start the next init container.
 		if err := start("init container", container); err != nil {
@@ -798,19 +810,11 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *v1.Pod, podStatus *kubecontaine
 		klog.V(4).Infof("Completed init container %q for pod %q", container.Name, format.Pod(pod))
 	}
 
-	// Step 6: start containers in podContainerChanges.ContainersToStart.
+	// Step 7: start containers in podContainerChanges.ContainersToStart.
 	for _, idx := range podContainerChanges.ContainersToStart {
 		start("container", &pod.Spec.Containers[idx])
 	}
 
-	// Step 7: start ephemeral containers
-	if utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) {
-		for _, idx := range podContainerChanges.EphemeralContainersToStart {
-			c := (*v1.Container)(&pod.Spec.EphemeralContainers[idx].EphemeralContainerCommon)
-			start("ephemeral container", c)
-		}
-	}
-
 	return
 }