Repository: kubernetes/kubernetes (language: go)

Issue:
Pod status phase misses Pending transition after node reboot
### What happened?

When a kubernetes node is rebooted, all the containers running on the node are terminated and then created & started over again. When the pod has a list of init containers runing, and if the init container takes a little bit time (longer than PLEG period), the pod is supposed to have `status.phase: Pending`, but instead, during the whole operation, `status.phase` is always `Running`.

This new behavior (always has `status.phase: Running`) started when "restartable init container" was [introduced][1]. After node reboots, all the regular init containers are started one by one, so `pendingInitialization` is supposed by larger than 0 (unless the init container is completed pretty fast that PLEG doesn't catch any running init container), and should report `Pending` phase. But the behavior has changed to check further whether there are any other regular containers that have started. In the case of node reboot, all the other regular containers are already terminated, and then considered as "yes, they have started", and then no longer reported as `Pending`.

I think this behavior change is unexpected, as that violates the [definition][2] of "Running":

> // PodRunning means the pod has been bound to a node and all of the containers have been started.
	// At least one container is still running or is in the process of being restarted.

None of the containers are started or running, and they are not in the process of being started or restarted since init containers are still running.

[1]: https://github.com/kubernetes/kubernetes/blob/099a88370d017dacf16e67306ebcdec8394fae83/pkg/kubelet/kubelet_pods.go#L1675-L1685
[2]: https://github.com/kubernetes/kubernetes/blob/099a88370d017dacf16e67306ebcdec8394fae83/pkg/apis/core/types.go#L2871


### What did you expect to happen?

I expect that pods with init containers should transition into "Pending" when kubelet observed that its regular init container is still running and none of its main containers are started.

### How can we reproduce it (as minimally and precisely as possible)?

1. Launch a cluster with minikube:

```console
$ minikube start --nodes=2 --kubernetes-version=v1.30.3
```

2. Create the following pod:

```yaml
apiVersion: v1
kind: Pod
metadata:
  name: sleep
spec:
  restartPolicy: Always
  containers:
  - name: sleep
    image: busybox
    command:
    - sleep
    - infinity
  initContainers:
  - name: snap
    image: busybox
    command:
    - sleep
    - "10"
```

3. Log in the node and simulate "reboot"

```console
$ minikube ssh --node <node-which-runs-the-pod-above>
...
# systemctl stop kubelet
# docker stop $(docker ps -aq)
# systemctl start kubelet
```

4. Observe the pod status via:

```console
$ k get pods -w -o jsonpath='{$.status.phase}{"\n"}'
```

On 1.30.3, the output from 4 is `Running` constantly, while in previous kubernetes version (for e,g, v1.23.17), it will show "Pending" while the init container is still Running.

### Anything else we need to know?

_No response_

### Kubernetes version

<details>

```console
$ kubectl version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.0", GitCommit:"b46a3f887ca979b1a5d14fd39cb1af43e7e5d12d", GitTreeState:"clean", BuildDate:"2022-12-08T19:58:30Z", GoVersion:"go1.19.4", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"30", GitVersion:"v1.30.3", GitCommit:"6fc0a69044f1ac4c13841ec4391224a2df241460", GitTreeState:"clean", BuildDate:"2024-07-16T23:48:12Z", GoVersion:"go1.22.5", Compiler:"gc", Platform:"linux/amd64"}
WARNING: version difference between client (1.26) and server (1.30) exceeds the supported minor version skew of +/-1
```

</details>


### Cloud provider

<details>
None (minikube)
</details>


### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here
```

</details>


### Install tools

<details>

</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>



Discussion / hints:
/assign 
/unassign
/sig node
I think this is indeed a bug, and it should at least not affect the original logic for regular init containers. I will submit a PR to fix this problem.

/assign
/cc @SergeyKanzhelev @bobbypage

I believe this is indeed a bug and the issue makes sense to me. WDYT?
/cc @gjkim42 
I'll take a look.

Fix patch:
diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go
index e671c7af337b7..3e625a8006174 100644
--- a/pkg/kubelet/kubelet_pods.go
+++ b/pkg/kubelet/kubelet_pods.go
@@ -1579,7 +1579,8 @@ func (kl *Kubelet) GetKubeletContainerLogs(ctx context.Context, podFullName, con
 // getPhase returns the phase of a pod given its container info.
 func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.PodPhase {
 	spec := pod.Spec
-	pendingInitialization := 0
+	pendingRestartableInitContainers := 0
+	pendingRegularInitContainers := 0
 	failedInitialization := 0
 
 	// regular init containers
@@ -1593,13 +1594,13 @@ func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.Pod
 
 		containerStatus, ok := podutil.GetContainerStatus(info, container.Name)
 		if !ok {
-			pendingInitialization++
+			pendingRegularInitContainers++
 			continue
 		}
 
 		switch {
 		case containerStatus.State.Running != nil:
-			pendingInitialization++
+			pendingRegularInitContainers++
 		case containerStatus.State.Terminated != nil:
 			if containerStatus.State.Terminated.ExitCode != 0 {
 				failedInitialization++
@@ -1610,10 +1611,10 @@ func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.Pod
 					failedInitialization++
 				}
 			} else {
-				pendingInitialization++
+				pendingRegularInitContainers++
 			}
 		default:
-			pendingInitialization++
+			pendingRegularInitContainers++
 		}
 	}
 
@@ -1624,38 +1625,40 @@ func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.Pod
 	stopped := 0
 	succeeded := 0
 
-	// restartable init containers
-	for _, container := range spec.InitContainers {
-		if !kubetypes.IsRestartableInitContainer(&container) {
-			// Skip the regular init containers, as they have been handled above.
-			continue
-		}
-		containerStatus, ok := podutil.GetContainerStatus(info, container.Name)
-		if !ok {
-			unknown++
-			continue
-		}
-
-		switch {
-		case containerStatus.State.Running != nil:
-			if containerStatus.Started == nil || !*containerStatus.Started {
-				pendingInitialization++
+	if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
+		// restartable init containers
+		for _, container := range spec.InitContainers {
+			if !kubetypes.IsRestartableInitContainer(&container) {
+				// Skip the regular init containers, as they have been handled above.
+				continue
 			}
-			running++
-		case containerStatus.State.Terminated != nil:
-			// Do nothing here, as terminated restartable init containers are not
-			// taken into account for the pod phase.
-		case containerStatus.State.Waiting != nil:
-			if containerStatus.LastTerminationState.Terminated != nil {
+			containerStatus, ok := podutil.GetContainerStatus(info, container.Name)
+			if !ok {
+				unknown++
+				continue
+			}
+
+			switch {
+			case containerStatus.State.Running != nil:
+				if containerStatus.Started == nil || !*containerStatus.Started {
+					pendingRestartableInitContainers++
+				}
+				running++
+			case containerStatus.State.Terminated != nil:
 				// Do nothing here, as terminated restartable init containers are not
 				// taken into account for the pod phase.
-			} else {
-				pendingInitialization++
-				waiting++
+			case containerStatus.State.Waiting != nil:
+				if containerStatus.LastTerminationState.Terminated != nil {
+					// Do nothing here, as terminated restartable init containers are not
+					// taken into account for the pod phase.
+				} else {
+					pendingRestartableInitContainers++
+					waiting++
+				}
+			default:
+				pendingRestartableInitContainers++
+				unknown++
 			}
-		default:
-			pendingInitialization++
-			unknown++
 		}
 	}
 
@@ -1690,11 +1693,12 @@ func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.Pod
 	}
 
 	switch {
-	case pendingInitialization > 0 &&
-		// This is needed to handle the case where the pod has been initialized but
-		// the restartable init containers are restarting and the pod should not be
-		// placed back into v1.PodPending since the regular containers have run.
-		!kubecontainer.HasAnyRegularContainerStarted(&spec, info):
+	case pendingRegularInitContainers > 0 ||
+		(pendingRestartableInitContainers > 0 &&
+			// This is needed to handle the case where the pod has been initialized but
+			// the restartable init containers are restarting and the pod should not be
+			// placed back into v1.PodPending since the regular containers have run.
+			!kubecontainer.HasAnyRegularContainerStarted(&spec, info)):
 		fallthrough
 	case waiting > 0:
 		klog.V(5).InfoS("Pod waiting > 0, pending")


Test patch:
diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go
index d4aab600b183b..4bdb51dc56fd9 100644
--- a/pkg/kubelet/kubelet_pods_test.go
+++ b/pkg/kubelet/kubelet_pods_test.go
@@ -2408,7 +2408,8 @@ func TestPodPhaseWithRestartAlwaysInitContainers(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		statusInfo := append(test.pod.Status.InitContainerStatuses[:], test.pod.Status.ContainerStatuses[:]...)
+		statusInfo := test.pod.Status.InitContainerStatuses
+		statusInfo = append(statusInfo, test.pod.Status.ContainerStatuses...)
 		status := getPhase(test.pod, statusInfo, false)
 		assert.Equal(t, test.status, status, "[test %s]", test.test)
 	}
@@ -2620,12 +2621,105 @@ func TestPodPhaseWithRestartAlwaysRestartableInitContainers(t *testing.T) {
 	}
 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SidecarContainers, true)
 	for _, test := range tests {
-		statusInfo := append(test.pod.Status.InitContainerStatuses[:], test.pod.Status.ContainerStatuses[:]...)
+		statusInfo := test.pod.Status.InitContainerStatuses
+		statusInfo = append(statusInfo, test.pod.Status.ContainerStatuses...)
 		status := getPhase(test.pod, statusInfo, test.podIsTerminal)
 		assert.Equal(t, test.status, status, "[test %s]", test.test)
 	}
 }
 
+func TestPodPhaseWithRestartAlwaysAndPodHasRun(t *testing.T) {
+	desiredState := v1.PodSpec{
+		NodeName: "machine",
+		InitContainers: []v1.Container{
+			{Name: "containerX"},
+			{Name: "containerY", RestartPolicy: &containerRestartPolicyAlways},
+		},
+		Containers: []v1.Container{
+			{Name: "containerA"},
+		},
+		RestartPolicy: v1.RestartPolicyAlways,
+	}
+
+	tests := []struct {
+		pod    *v1.Pod
+		status v1.PodPhase
+		test   string
+	}{
+		{
+			&v1.Pod{
+				Spec: desiredState,
+				Status: v1.PodStatus{
+					InitContainerStatuses: []v1.ContainerStatus{
+						runningState("containerX"),
+						runningState("containerY"),
+					},
+					ContainerStatuses: []v1.ContainerStatus{
+						runningState("containerA"),
+					},
+				},
+			},
+			v1.PodPending,
+			"regular init containers, restartable init container and regular container are all running",
+		},
+		{
+			&v1.Pod{
+				Spec: desiredState,
+				Status: v1.PodStatus{
+					InitContainerStatuses: []v1.ContainerStatus{
+						runningState("containerX"),
+						runningState("containerY"),
+					},
+					ContainerStatuses: []v1.ContainerStatus{
+						stoppedState("containerA"),
+					},
+				},
+			},
+			v1.PodPending,
+			"regular containers is stopped, restartable init container and regular int container are both running",
+		},
+		{
+			&v1.Pod{
+				Spec: desiredState,
+				Status: v1.PodStatus{
+					InitContainerStatuses: []v1.ContainerStatus{
+						succeededState("containerX"),
+						runningState("containerY"),
+					},
+					ContainerStatuses: []v1.ContainerStatus{
+						stoppedState("containerA"),
+					},
+				},
+			},
+			v1.PodRunning,
+			"regular init container is succeeded, restartable init container is running, regular containers is stopped",
+		},
+		{
+			&v1.Pod{
+				Spec: desiredState,
+				Status: v1.PodStatus{
+					InitContainerStatuses: []v1.ContainerStatus{
+						succeededState("containerX"),
+						runningState("containerY"),
+					},
+					ContainerStatuses: []v1.ContainerStatus{
+						runningState("containerA"),
+					},
+				},
+			},
+			v1.PodRunning,
+			"regular init container is succeeded, restartable init container and regular containers are both running",
+		},
+	}
+	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SidecarContainers, true)
+	for _, test := range tests {
+		statusInfo := test.pod.Status.InitContainerStatuses
+		statusInfo = append(statusInfo, test.pod.Status.ContainerStatuses...)
+		status := getPhase(test.pod, statusInfo, false)
+		assert.Equal(t, test.status, status, "[test %s]", test.test)
+	}
+}
+
 func TestPodPhaseWithRestartNever(t *testing.T) {
 	desiredState := v1.PodSpec{
 		NodeName: "machine",
@@ -2823,7 +2917,8 @@ func TestPodPhaseWithRestartNeverInitContainers(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		statusInfo := append(test.pod.Status.InitContainerStatuses[:], test.pod.Status.ContainerStatuses[:]...)
+		statusInfo := test.pod.Status.InitContainerStatuses
+		statusInfo = append(statusInfo, test.pod.Status.ContainerStatuses...)
 		status := getPhase(test.pod, statusInfo, false)
 		assert.Equal(t, test.status, status, "[test %s]", test.test)
 	}
@@ -3022,7 +3117,8 @@ func TestPodPhaseWithRestartNeverRestartableInitContainers(t *testing.T) {
 	}
 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SidecarContainers, true)
 	for _, test := range tests {
-		statusInfo := append(test.pod.Status.InitContainerStatuses[:], test.pod.Status.ContainerStatuses[:]...)
+		statusInfo := test.pod.Status.InitContainerStatuses
+		statusInfo = append(statusInfo, test.pod.Status.ContainerStatuses...)
 		status := getPhase(test.pod, statusInfo, false)
 		assert.Equal(t, test.status, status, "[test %s]", test.test)
 	}
diff --git a/test/e2e/framework/pod/wait.go b/test/e2e/framework/pod/wait.go
index 96f46fb8f3687..537d59c32241a 100644
--- a/test/e2e/framework/pod/wait.go
+++ b/test/e2e/framework/pod/wait.go
@@ -809,6 +809,18 @@ func WaitForPodContainerStarted(ctx context.Context, c clientset.Interface, name
 	})
 }
 
+// WaitForPodInitContainerStarted waits for the given Pod init container to start.
+func WaitForPodInitContainerStarted(ctx context.Context, c clientset.Interface, namespace, podName string, initContainerIndex int, timeout time.Duration) error {
+	conditionDesc := fmt.Sprintf("init container %d started", initContainerIndex)
+	return WaitForPodCondition(ctx, c, namespace, podName, conditionDesc, timeout, func(pod *v1.Pod) (bool, error) {
+		if initContainerIndex > len(pod.Status.InitContainerStatuses)-1 {
+			return false, nil
+		}
+		initContainerStatus := pod.Status.InitContainerStatuses[initContainerIndex]
+		return *initContainerStatus.Started, nil
+	})
+}
+
 // WaitForPodFailedReason wait for pod failed reason in status, for example "SysctlForbidden".
 func WaitForPodFailedReason(ctx context.Context, c clientset.Interface, pod *v1.Pod, reason string, timeout time.Duration) error {
 	conditionDesc := fmt.Sprintf("failed with reason %s", reason)
diff --git a/test/e2e_node/pod_status_test.go b/test/e2e_node/pod_status_test.go
new file mode 100644
index 0000000000000..0bfff1e2534dc
--- /dev/null
+++ b/test/e2e_node/pod_status_test.go
@@ -0,0 +1,129 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2enode
+
+import (
+	"context"
+	"github.com/onsi/ginkgo/v2"
+	"github.com/onsi/gomega"
+
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
+	"k8s.io/kubernetes/test/e2e/framework"
+	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
+	admissionapi "k8s.io/pod-security-admission/api"
+)
+
+var _ = SIGDescribe(framework.WithSerial(), "Pods status phase", func() {
+	f := framework.NewDefaultFramework("pods-status-phase-test-serial")
+	addAfterEachForCleaningUpPods(f)
+	f.NamespacePodSecurityLevel = admissionapi.LevelPrivileged
+
+	ginkgo.It("should be pending during the execution of the init container after the node reboot", func(ctx context.Context) {
+		init := "init"
+		regular := "regular"
+
+		podLabels := map[string]string{
+			"test":      "pods-status-phase-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:  init,
+						Image: busyboxImage,
+						Command: ExecCommand(init, execCommand{
+							Delay:    30,
+							ExitCode: 0,
+						}),
+					},
+				},
+				Containers: []v1.Container{
+					{
+						Name:  regular,
+						Image: busyboxImage,
+						Command: ExecCommand(regular, execCommand{
+							Delay:    300,
+							ExitCode: 0,
+						}),
+					},
+				},
+			},
+		}
+		preparePod(pod)
+
+		client := e2epod.NewPodClient(f)
+		pod = client.Create(ctx, pod)
+
+		ginkgo.By("Waiting for the pod's status to become Running")
+		err := e2epod.WaitForPodNameRunningInNamespace(ctx, f.ClientSet, pod.Name, pod.Namespace)
+		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
+
+		// We need to wait for the pod to be Running before simulating the node reboot,
+		// to avoid any unintended effects from the previous init container state.
+		// Simulate node reboot by restarting the kubelet and the pod sandbox.
+		ginkgo.By("Stopping the kubelet")
+		startKubelet := mustStopKubelet(ctx, f)
+		gomega.Eventually(ctx, func() bool {
+			return kubeletHealthCheck(kubeletHealthCheckURL)
+		}, f.Timeouts.PodStart, f.Timeouts.Poll).Should(gomega.BeFalseBecause("kubelet should be stopped"))
+
+		ginkgo.By("Stopping the pod sandbox")
+		err = rs.StopPodSandbox(ctx, podSandboxID)
+		framework.ExpectNoError(err)
+
+		ginkgo.By("Starting the kubelet")
+		startKubelet(ctx)
+		gomega.Eventually(ctx, func() bool {
+			return kubeletHealthCheck(kubeletHealthCheckURL)
+		}, f.Timeouts.PodStart, f.Timeouts.Poll).Should(gomega.BeTrueBecause("kubelet should be started"))
+
+		ginkgo.By("Waiting for the regular init container to be started after the node reboot")
+		err = e2epod.WaitForPodInitContainerStarted(ctx, f.ClientSet, pod.Namespace, pod.Name, 0, f.Timeouts.PodStart)
+		framework.ExpectNoError(err)
+
+		pod, err = client.Get(ctx, pod.Name, metav1.GetOptions{})
+		framework.ExpectNoError(err)
+		gomega.Expect(pod.Status.Phase == v1.PodPending).To(gomega.BeTrueBecause("pod should be pending during the execution of the init container after the node reboot"))
+
+		ginkgo.By("Parse the logs of the pod after the kubelet restart")
+		results := parseOutput(ctx, f, pod)
+		_, err = results.TimeOfStart(init)
+		framework.ExpectNoError(err, "After the node restarts, the init container should restart.")
+
+		ginkgo.By("Verifying that the pod fully starts")
+		err = e2epod.WaitForPodNameRunningInNamespace(ctx, f.ClientSet, pod.Name, pod.Namespace)
+		framework.ExpectNoError(err)
+	})
+})