[workbenches] improve the failure information shown in logs

tests/workbenches/conftest.py

@@ -1,4 +1,5 @@
 from collections.abc import Generator
+from typing import Any
 
 import pytest
 from kubernetes.dynamic import DynamicClient
@@ -8,6 +9,7 @@
 from ocp_resources.pod import Pod
 from pytest_testconfig import config as py_config
 from simple_logger.logger import get_logger
+from timeout_sampler import TimeoutExpiredError
 
 from tests.workbenches.utils import get_username
 from utilities import constants
@@ -18,6 +20,73 @@
 LOGGER = get_logger(name=__name__)
 
 
+def _read_obj_field(obj: Any, field_name: str, default: Any = None) -> Any:
+    """Safely read attribute/key from k8s dynamic objects."""
+    if isinstance(obj, dict):
+        return obj.get(field_name, default)
+    return getattr(obj, field_name, default)
+
+
+def _format_container_status(container_status: Any, status_prefix: str) -> str:
+    """Build one-line diagnostic summary for a container status."""
+    name = _read_obj_field(obj=container_status, field_name="name", default="<unknown>")
+    ready = _read_obj_field(obj=container_status, field_name="ready", default=False)
+    restart_count = _read_obj_field(obj=container_status, field_name="restartCount", default=0)
+    state = _read_obj_field(obj=container_status, field_name="state", default={})
+
+    state_description = "unknown"
+    details: list[str] = []
+    for state_name in ("waiting", "terminated", "running"):
+        state_value = _read_obj_field(obj=state, field_name=state_name, default=None)
+        if not state_value:
+            continue
+
+        reason = _read_obj_field(obj=state_value, field_name="reason", default=None)
+        message = _read_obj_field(obj=state_value, field_name="message", default=None)
+        state_description = state_name if not reason else f"{state_name}({reason})"
+        if message:
+            details.append(f"message={message}")
+        break
+
+    details_str = f", {', '.join(details)}" if details else ""
+    return f"{status_prefix} '{name}': ready={ready}, restarts={restart_count}, state={state_description}{details_str}"
+
+
+def _collect_notebook_pod_diagnostics(notebook_pod: Pod) -> str:
+    """Collect concise pod status details for pytest assertion messages."""
+    pod_instance = notebook_pod.instance
+    pod_status = _read_obj_field(obj=pod_instance, field_name="status", default=None)
+    pod_phase = _read_obj_field(obj=pod_status, field_name="phase", default="Unknown")
+    pod_reason = _read_obj_field(obj=pod_status, field_name="reason", default=None)
+    pod_message = _read_obj_field(obj=pod_status, field_name="message", default=None)
+
+    lines = [f"Pod phase={pod_phase}, reason={pod_reason}, message={pod_message}"]
+
+    pod_conditions = _read_obj_field(obj=pod_status, field_name="conditions", default=[]) or []
+    lines.extend(
+        "Condition "
+        f"{_read_obj_field(obj=condition, field_name='type', default='<unknown>')}: "
+        f"status={_read_obj_field(obj=condition, field_name='status', default='Unknown')}, "
+        f"reason={_read_obj_field(obj=condition, field_name='reason', default='')}, "
+        f"message={_read_obj_field(obj=condition, field_name='message', default='')}"
+        for condition in pod_conditions
+    )
+
+    init_container_statuses = _read_obj_field(obj=pod_status, field_name="initContainerStatuses", default=[]) or []
+    lines.extend(
+        _format_container_status(container_status=container_status, status_prefix="Init container")
+        for container_status in init_container_statuses
+    )
+
+    container_statuses = _read_obj_field(obj=pod_status, field_name="containerStatuses", default=[]) or []
+    lines.extend(
+        _format_container_status(container_status=container_status, status_prefix="Container")
+        for container_status in container_statuses
+    )
+
+    return "\n".join(lines)
+
+
 @pytest.fixture(scope="function")
 def users_persistent_volume_claim(
     request: pytest.FixtureRequest, unprivileged_model_namespace: Namespace, unprivileged_client: DynamicClient
@@ -107,8 +176,13 @@ def default_notebook(
     admin_client: DynamicClient,
     unprivileged_client: DynamicClient,
     notebook_image: str,
+    users_persistent_volume_claim: PersistentVolumeClaim,
 ) -> Generator[Notebook]:
-    """Returns a new Notebook CR for a given namespace, name, and image"""
+    """Returns a new Notebook CR for a given namespace, name, and image.
+
+    The PVC fixture dependency guarantees the Notebook is created only after
+    the user PVC exists, avoiding pod scheduling races on claim lookup.
+    """
     namespace = request.param["namespace"]
     name = request.param["name"]
 
@@ -223,6 +297,7 @@ def default_notebook(
 
 @pytest.fixture(scope="function")
 def notebook_pod(
+    request: pytest.FixtureRequest,
     unprivileged_client: DynamicClient,
     default_notebook: Notebook,
 ) -> Pod:
@@ -232,10 +307,11 @@ def notebook_pod(
     This fixture:
     - Creates a Pod object for the notebook
     - Waits for pod to exist
-    - Waits for pod to reach Ready state (10-minute timeout)
+    - Waits for pod to reach Ready state (configurable timeout)
     - Provides detailed diagnostics on failure
 
     Args:
+        request: Optional fixture params. Supports {"timeout": <seconds>} via indirect parametrization.
         unprivileged_client: Client for interacting with the cluster
         default_notebook: The notebook CR to get the pod for
 
@@ -245,10 +321,13 @@ def notebook_pod(
     Raises:
         AssertionError: If pod fails to reach Ready state or is not created
     """
+    params = getattr(request, "param", {})
+    pod_ready_timeout = params.get("timeout", Timeout.TIMEOUT_10MIN)
+
     # Error messages
     _ERR_POD_NOT_READY = (
-        "Pod '{pod_name}-0' failed to reach Ready state within 10 minutes.\n"
-        "Pod Phase: {pod_phase}\n"
+        "Pod '{pod_name}-0' failed to reach Ready state within {timeout_seconds} seconds.\n"
+        "Pod diagnostics:\n{pod_diagnostics}\n"
         "Original Error: {original_error}\n"
         "Pod information collected to must-gather directory for debugging."
     )
@@ -266,18 +345,32 @@ def notebook_pod(
         notebook_pod.wait_for_condition(
             condition=Pod.Condition.READY,
             status=Pod.Condition.Status.TRUE,
-            timeout=Timeout.TIMEOUT_10MIN,
+            timeout=pod_ready_timeout,
         )
-    except (TimeoutError, RuntimeError) as e:
-        if notebook_pod.exists:
+    except (TimeoutError, TimeoutExpiredError, RuntimeError) as e:
+        try:
+            pod_exists = notebook_pod.exists
+        except Exception as exists_error:  # noqa: BLE001
+            LOGGER.warning(f"Failed to verify pod existence after timeout: {exists_error}")
+            pod_exists = False
+
+        if pod_exists:
             # Collect pod information for debugging purposes (YAML + logs saved to must-gather dir)
-            collect_pod_information(notebook_pod)
-            pod_status = notebook_pod.instance.status
-            pod_phase = pod_status.phase
+            try:
+                collect_pod_information(notebook_pod)
+            except Exception as collect_error:  # noqa: BLE001
+                LOGGER.warning(f"Failed to collect pod artifacts: {collect_error}")
+
+            try:
+                pod_diagnostics = _collect_notebook_pod_diagnostics(notebook_pod=notebook_pod)
+            except Exception as diagnostics_error:  # noqa: BLE001
+                pod_diagnostics = f"<failed to collect pod diagnostics: {diagnostics_error}>"
+
             raise AssertionError(
                 _ERR_POD_NOT_READY.format(
                     pod_name=default_notebook.name,
-                    pod_phase=pod_phase,
+                    timeout_seconds=pod_ready_timeout,
+                    pod_diagnostics=pod_diagnostics,
                     original_error=e,
                 )
             ) from e

tests/workbenches/notebook-controller/test_spawning.py

@@ -1,15 +1,16 @@
 import pytest
-from kubernetes.dynamic.client import DynamicClient
 from ocp_resources.namespace import Namespace
 from ocp_resources.notebook import Notebook
 from ocp_resources.persistent_volume_claim import PersistentVolumeClaim
 from ocp_resources.pod import Pod
 
+from utilities.constants import Timeout
+
 
 class TestNotebook:
     @pytest.mark.smoke
     @pytest.mark.parametrize(
-        "unprivileged_model_namespace,users_persistent_volume_claim,default_notebook",
+        "unprivileged_model_namespace,users_persistent_volume_claim,default_notebook,notebook_pod",
         [
             pytest.param(
                 {
@@ -21,31 +22,26 @@ class TestNotebook:
                     "namespace": "test-odh-notebook",
                     "name": "test-odh-notebook",
                 },
+                {"timeout": Timeout.TIMEOUT_5MIN},
             )
         ],
         indirect=True,
     )
     def test_create_simple_notebook(
         self,
-        unprivileged_client: DynamicClient,
+        notebook_pod: Pod,
         unprivileged_model_namespace: Namespace,
         users_persistent_volume_claim: PersistentVolumeClaim,
         default_notebook: Notebook,
     ):
         """
         Create a simple Notebook CR with all necessary resources and see if the Notebook Operator creates it properly
         """
-        notebook_pod = Pod(
-            client=unprivileged_client,
-            namespace=default_notebook.namespace,
-            name=f"{default_notebook.name}-0",
-        )
-        notebook_pod.wait()
-        notebook_pod.wait_for_condition(condition=Pod.Condition.READY, status=Pod.Condition.Status.TRUE)
+        assert notebook_pod.exists
 
     @pytest.mark.smoke
     @pytest.mark.parametrize(
-        "unprivileged_model_namespace,users_persistent_volume_claim,default_notebook",
+        "unprivileged_model_namespace,users_persistent_volume_claim,default_notebook,notebook_pod",
         [
             pytest.param(
                 {
@@ -63,13 +59,14 @@ def test_create_simple_notebook(
                         "notebooks.opendatahub.io/auth-sidecar-memory-limit": "256Mi",
                     },
                 },
+                {"timeout": Timeout.TIMEOUT_5MIN},
             )
         ],
         indirect=True,
     )
     def test_auth_container_resource_customization(
         self,
-        unprivileged_client: DynamicClient,
+        notebook_pod: Pod,
         unprivileged_model_namespace: Namespace,
         users_persistent_volume_claim: PersistentVolumeClaim,
         default_notebook: Notebook,
@@ -80,14 +77,6 @@ def test_auth_container_resource_customization(
         This test verifies that when a Notebook CR is created with custom Auth container resource
         annotations, the spawned pod has the Auth container with the specified resource values.
         """
-        notebook_pod = Pod(
-            client=unprivileged_client,
-            namespace=default_notebook.namespace,
-            name=f"{default_notebook.name}-0",
-        )
-        notebook_pod.wait()
-        notebook_pod.wait_for_condition(condition=Pod.Condition.READY, status=Pod.Condition.Status.TRUE)
-
         # Verify Auth container has the expected resource values
         auth_container = self._get_auth_container(pod=notebook_pod)
         assert auth_container, "Auth proxy container not found in the pod"