From ca6fda05ce5483770c449dddae8559534ee1227e Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Thu, 31 Aug 2023 01:17:37 +0900 Subject: [PATCH] Restart containers in right order after the podSandboxChanged 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. --- .../kuberuntime/kuberuntime_container.go | 26 +++- test/e2e_node/container_lifecycle_test.go | 141 ++++++++++++++++++ 2 files changed, 166 insertions(+), 1 deletion(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index 85203f46318..b40432374d7 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/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. diff --git a/test/e2e_node/container_lifecycle_test.go b/test/e2e_node/container_lifecycle_test.go index 55536e1f43d..fd93751afb5 100644 --- a/test/e2e_node/container_lifecycle_test.go +++ b/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)) + }) +})