Merge pull request #120269 from gjkim42/fix-restart-containers-in-right-order-after-podsandbox-changed

Restart containers in right order with SidecarContainers enabled

pkg/kubelet/kuberuntime/kuberuntime_container.go

@@ -949,7 +949,31 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
 	// have been executed at some point in the past.  However, they could have been removed
 	// from the container runtime now, and if we proceed, it would appear as if they
 	// never ran and will re-execute improperly except for the restartable init containers.
-	podHasInitialized := hasAnyRegularContainerCreated(pod, podStatus)
+	podHasInitialized := false
+	for _, container := range pod.Spec.Containers {
+		status := podStatus.FindContainerStatusByName(container.Name)
+		if status == nil {
+			continue
+		}
+		switch status.State {
+		case kubecontainer.ContainerStateCreated,
+			kubecontainer.ContainerStateRunning:
+			podHasInitialized = true
+		case kubecontainer.ContainerStateExited:
+			// This is a workaround for the issue that the kubelet cannot
+			// differentiate the container statuses of the previous podSandbox
+			// from the current one.
+			// If the node is rebooted, all containers will be in the exited
+			// state and the kubelet will try to recreate a new podSandbox.
+			// In this case, the kubelet should not mistakenly think that
+			// the newly created podSandbox has been initialized.
+		default:
+			// Ignore other states
+		}
+		if podHasInitialized {
+			break
+		}
+	}
 
 	// isPreviouslyInitialized indicates if the current init container is
 	// previously initialized.

test/e2e_node/container_lifecycle_test.go

@@ -2506,3 +2506,144 @@ var _ = SIGDescribe("[NodeAlphaFeature:SidecarContainers] Containers Lifecycle",
 		framework.ExpectNoError(results.Starts(regular1))
 	})
 })
+
+var _ = SIGDescribe("[NodeAlphaFeature:SidecarContainers][Serial] Containers Lifecycle", func() {
+	f := framework.NewDefaultFramework("containers-lifecycle-test-serial")
+	f.NamespacePodSecurityEnforceLevel = admissionapi.LevelPrivileged
+
+	ginkgo.It("should restart the containers in right order after the node reboot", func(ctx context.Context) {
+		init1 := "init-1"
+		restartableInit2 := "restartable-init-2"
+		init3 := "init-3"
+		regular1 := "regular-1"
+
+		podLabels := map[string]string{
+			"test":      "containers-lifecycle-test-serial",
+			"namespace": f.Namespace.Name,
+		}
+		pod := &v1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "initialized-pod",
+				Labels: podLabels,
+			},
+			Spec: v1.PodSpec{
+				RestartPolicy: v1.RestartPolicyAlways,
+				InitContainers: []v1.Container{
+					{
+						Name:  init1,
+						Image: busyboxImage,
+						Command: ExecCommand(init1, execCommand{
+							Delay:    5,
+							ExitCode: 0,
+						}),
+					},
+					{
+						Name:  restartableInit2,
+						Image: busyboxImage,
+						Command: ExecCommand(restartableInit2, execCommand{
+							Delay:    300,
+							ExitCode: 0,
+						}),
+						RestartPolicy: &containerRestartPolicyAlways,
+					},
+					{
+						Name:  init3,
+						Image: busyboxImage,
+						Command: ExecCommand(init3, execCommand{
+							Delay:    5,
+							ExitCode: 0,
+						}),
+					},
+				},
+				Containers: []v1.Container{
+					{
+						Name:  regular1,
+						Image: busyboxImage,
+						Command: ExecCommand(regular1, execCommand{
+							Delay:    300,
+							ExitCode: 0,
+						}),
+					},
+				},
+			},
+		}
+		preparePod(pod)
+
+		client := e2epod.NewPodClient(f)
+		pod = client.Create(ctx, pod)
+		ginkgo.By("Waiting for the pod to be initialized and run")
+		err := e2epod.WaitForPodRunningInNamespace(ctx, f.ClientSet, pod)
+		framework.ExpectNoError(err)
+
+		ginkgo.By("Getting the current pod sandbox ID")
+		rs, _, err := getCRIClient()
+		framework.ExpectNoError(err)
+
+		sandboxes, err := rs.ListPodSandbox(ctx, &runtimeapi.PodSandboxFilter{
+			LabelSelector: podLabels,
+		})
+		framework.ExpectNoError(err)
+		gomega.Expect(sandboxes).To(gomega.HaveLen(1))
+		podSandboxID := sandboxes[0].Id
+
+		ginkgo.By("Stopping the kubelet")
+		restartKubelet := stopKubelet()
+		gomega.Eventually(ctx, func() bool {
+			return kubeletHealthCheck(kubeletHealthCheckURL)
+		}, f.Timeouts.PodStart, f.Timeouts.Poll).Should(gomega.BeFalse())
+
+		ginkgo.By("Stopping the pod sandbox to simulate the node reboot")
+		err = rs.StopPodSandbox(ctx, podSandboxID)
+		framework.ExpectNoError(err)
+
+		ginkgo.By("Restarting the kubelet")
+		restartKubelet()
+		gomega.Eventually(ctx, func() bool {
+			return kubeletHealthCheck(kubeletHealthCheckURL)
+		}, f.Timeouts.PodStart, f.Timeouts.Poll).Should(gomega.BeTrue())
+
+		ginkgo.By("Waiting for the pod to be re-initialized and run")
+		err = e2epod.WaitForPodCondition(ctx, f.ClientSet, pod.Namespace, pod.Name, "re-initialized", f.Timeouts.PodStart, func(pod *v1.Pod) (bool, error) {
+			if pod.Status.ContainerStatuses[0].RestartCount < 1 {
+				return false, nil
+			}
+			if pod.Status.Phase != v1.PodRunning {
+				return false, nil
+			}
+			return true, nil
+		})
+		framework.ExpectNoError(err)
+
+		ginkgo.By("Parsing results")
+		pod, err = client.Get(ctx, pod.Name, metav1.GetOptions{})
+		framework.ExpectNoError(err)
+		results := parseOutput(context.TODO(), f, pod)
+
+		ginkgo.By("Analyzing results")
+		init1Started, err := results.FindIndex(init1, "Started", 0)
+		framework.ExpectNoError(err)
+		restartableInit2Started, err := results.FindIndex(restartableInit2, "Started", 0)
+		framework.ExpectNoError(err)
+		init3Started, err := results.FindIndex(init3, "Started", 0)
+		framework.ExpectNoError(err)
+		regular1Started, err := results.FindIndex(regular1, "Started", 0)
+		framework.ExpectNoError(err)
+
+		init1Restarted, err := results.FindIndex(init1, "Started", init1Started+1)
+		framework.ExpectNoError(err)
+		restartableInit2Restarted, err := results.FindIndex(restartableInit2, "Started", restartableInit2Started+1)
+		framework.ExpectNoError(err)
+		init3Restarted, err := results.FindIndex(init3, "Started", init3Started+1)
+		framework.ExpectNoError(err)
+		regular1Restarted, err := results.FindIndex(regular1, "Started", regular1Started+1)
+		framework.ExpectNoError(err)
+
+		framework.ExpectNoError(init1Started.IsBefore(restartableInit2Started))
+		framework.ExpectNoError(restartableInit2Started.IsBefore(init3Started))
+		framework.ExpectNoError(init3Started.IsBefore(regular1Started))
+
+		framework.ExpectNoError(init1Restarted.IsBefore(restartableInit2Restarted))
+		framework.ExpectNoError(restartableInit2Restarted.IsBefore(init3Restarted))
+		framework.ExpectNoError(init3Restarted.IsBefore(regular1Restarted))
+	})
+})