Skip to content

fix(pod): show NoReady instead of Running when a completed pod is not ready - #4312

Open
kevin9327 wants to merge 2 commits into
derailed:masterfrom
kevin9327:fix/pod-phase-completed-not-ready
Open

kevin9327 wants to merge 2 commits into
derailed:masterfrom
kevin9327:fix/pod-phase-completed-not-ready

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

Problem

When some of a pod's containers have completed and another is still running and ready, Pod.Phase (the STATUS column) always turns Completed into Running, even if the pod's Ready condition is false. Example: a still-running container whose readiness depends on a container that has exited.

kubectl only reports Running in this case when the pod is ready, and NotReady otherwise:

if reason == "Completed" && hasRunning {
    if hasPodReadyCondition(pod.Status.Conditions) {
        reason = "Running"
    } else {
        reason = "NotReady"
    }
}

k9s's own PodStatus helper already does this, using PhaseNotReady. Pod.Phase didn't, so the two code paths disagreed.

Fix

Pod.Phase now checks the pod's Ready condition before turning Completed into Running, and returns PhaseNotReady otherwise, the same as PodStatus.

Tests

Two new TestCheckPhase cases: one container running and ready, one completed, with and without the Ready condition. The not-ready case returned Running before the fix and returns NoReady now. go test ./internal/render/... ./internal/view/... ./internal/dao/... passes.

AI-assisted: I directed the investigation and reviewed and verified the change myself.

@uozalp

uozalp commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

@kevin9327 Phase is also used by podIsRunning to gate shell and attach. With this change a pod with one completed and one running container shows NoReady, so s and a would be refused. Before it showed Running and worked.

We need to check this before merging. Can you test shell on such a pod?

@kevin9327

Copy link
Copy Markdown
Contributor Author

@uozalp You're right, thanks for catching that. With only the render change, podIsRunning would have refused s/a on a pod with one completed and one running container whenever the pod was not ready.

I don't have a live cluster on this machine to try shell end to end, so I fixed it in code and added a unit test instead (4c6521a):

  • podIsRunning now calls a small isPodRunning(*v1.Pod) helper that accepts Running and NoReady. NoReady only comes out of Phase on the completed+running path, so it still means "at least one container is running", and shell/attach behave as before.
  • TestIsPodRunning in internal/view/pod_int_test.go covers running, completed+running ready, completed+running not ready, pending and succeeded. The not-ready case fails without the helper change.

go test ./internal/view/ ./internal/render/ passes. If you'd rather keep the gate on the raw pod phase, or leave Phase alone and only change the rendered status, I can switch to that.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants