From b69235b5dbd0b5cec50a575bad21526b15b286f1 Mon Sep 17 00:00:00 2001 From: Jim Broadbent Date: Thu, 10 Sep 2026 17:13:01 +0100 Subject: [PATCH] fix: compare waiting container reason to PodInitializing `get_status_from_container_state` compared a bare list literal to a string, so the condition was always False: if ["reason"] == 'PodInitializing': The refactor in kubeflow/kubeflow#7585 extracted `container_state["waiting"]` into `waiting_state` but dropped the receiver on this one line, leaving a list literal instead of a subscript. The WAITING branch became unreachable and every waiting container fell through to the WARNING branch, so notebooks that were simply starting up rendered as "warning" with "PodInitializing: No available message for container state." instead of "waiting" with "PodInitializing". Compare `waiting_state.get("reason")` instead. `.get` rather than a subscript because `status.containerState` is a `corev1.ContainerState` and `ContainerStateWaiting.Reason` is `+optional` with `omitempty`, so the key can legitimately be absent. `test_no_message_container_state` asserted the buggy output, so it is retargeted at a non-initializing reason to keep covering the missing-message default. Two tests are added: one pinning the PodInitializing path that regressed, and one for the warning path with both reason and message, which had no coverage. Co-Authored-By: Claude Opus 5 Signed-off-by: Jim Broadbent --- .../jupyter/backend/apps/common/status.py | 2 +- .../backend/apps/common/status_test.py | 37 ++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/components/crud-web-apps/jupyter/backend/apps/common/status.py b/components/crud-web-apps/jupyter/backend/apps/common/status.py index c2a2a6fe1..6d0abe7d3 100644 --- a/components/crud-web-apps/jupyter/backend/apps/common/status.py +++ b/components/crud-web-apps/jupyter/backend/apps/common/status.py @@ -133,7 +133,7 @@ def get_status_from_container_state(notebook): # If the Notebook is initializing, the status will be waiting waiting_state = container_state["waiting"] - if ["reason"] == 'PodInitializing': + if waiting_state.get("reason") == 'PodInitializing': status_phase = status.STATUS_PHASE.WAITING status_message = waiting_state.get("reason", "Undetermined reason.") return status_phase, status_message diff --git a/components/crud-web-apps/jupyter/backend/apps/common/status_test.py b/components/crud-web-apps/jupyter/backend/apps/common/status_test.py index 157409195..1e789afea 100644 --- a/components/crud-web-apps/jupyter/backend/apps/common/status_test.py +++ b/components/crud-web-apps/jupyter/backend/apps/common/status_test.py @@ -34,7 +34,7 @@ def test_ready_container_state(self): (None, None) ) - def test_no_message_container_state(self): + def test_pod_initializing_container_state(self): container_state = { "status": { "containerState": { @@ -45,8 +45,41 @@ def test_no_message_container_state(self): } } + self.assertEqual( + status.get_status_from_container_state(container_state), + ("waiting", "PodInitializing") + ) + + def test_no_message_container_state(self): + container_state = { + "status": { + "containerState": { + "waiting": { + "reason": "ImagePullBackOff", + } + } + } + } + self.assertEqual( status.get_status_from_container_state(container_state), ("warning", - "PodInitializing: No available message for container state.") + "ImagePullBackOff: No available message for container state.") + ) + + def test_waiting_container_state_with_message(self): + container_state = { + "status": { + "containerState": { + "waiting": { + "reason": "ImagePullBackOff", + "message": "Back-off pulling image", + } + } + } + } + + self.assertEqual( + status.get_status_from_container_state(container_state), + ("warning", "ImagePullBackOff: Back-off pulling image") )