feat(detector): align the detect path with the operator, split information from usage, and report workload exit status

deploy/manifests/kubernetes.yaml

@@ -25,6 +25,17 @@ rules:
       - "pods/exec"
     verbs:
       - "*"
+  # Allow reading the Events of a workload Pod, so an image-pull failure
+  # reports the registry error behind a bare "ImagePullBackOff" reason.
+  # Kept as its own rule with only "get"/"list": the rule above grants "*",
+  # which would also allow creating and deleting Events.
+  - apiGroups:
+      - ""
+    resources:
+      - "events"
+    verbs:
+      - "get"
+      - "list"
   # Allow resolving the vendor RuntimeClass (e.g. "ascend", "nvidia") for
   # accelerated workload Pods, see GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_RUNTIME_CLASS.
   - apiGroups:

gpustack_runtime/cmds/detector.py

@@ -26,6 +26,7 @@ class DetectDevicesSubCommand(SubCommand):
 
     format: str = "table"
     watch: int = 0
+    no_usage: bool = False
 
     @staticmethod
     def register(parser: _SubParsersAction):
@@ -49,19 +50,26 @@ def register(parser: _SubParsersAction):
             help="Continuously watch for GPU in intervals of N seconds",
         )
 
+        detect_parser.add_argument(
+            "--no-usage",
+            action="store_true",
+            help="Report inventory only, without querying utilization, temperature or power",
+        )
+
         detect_parser.set_defaults(func=DetectDevicesSubCommand)
 
     def __init__(self, args: Namespace):
         self.format = args.format
         self.watch = args.watch
+        self.no_usage = args.no_usage
 
     def run(self):
         while True:
-            devs: Devices = detect_devices(fast=False)
+            devs: Devices = detect_devices(fast=False, usage=not self.no_usage)
             print("\033[2J\033[H", end="")
             match self.format.lower():
                 case "json":
-                    print(format_devices_json(devs))
+                    print(format_devices_json(devs, usage=not self.no_usage))
                 case _:
                     # Group devices by manufacturer.
                     group_devs = group_devices_by_manufacturer(devs)
@@ -70,7 +78,9 @@ def run(self):
                     else:
                         # Print each group separately.
                         for devs in group_devs.values():
-                            print(format_devices_table(devs))
+                            print(
+                                format_devices_table(devs, usage=not self.no_usage),
+                            )
             if not self.watch:
                 break
             time.sleep(self.watch)
@@ -134,11 +144,37 @@ def run(self):
                 print(os.linesep.join(legend_lines))
 
 
-def format_devices_json(devs: Devices) -> str:
-    return json.dumps([dev.to_dict() for dev in devs], indent=2)
+_USAGE_ONLY_KEYS = (
+    "cores_utilization",
+    "memory_used",
+    "memory_utilization",
+    "temperature",
+    "power_used",
+)
+"""
+The keys only the usage query fills. `memory_status` is deliberately absent:
+the information query reports it too, which is why the table keeps Status.
+"""
+
+
+def format_devices_json(devs: Devices, usage: bool = True) -> str:
+    devs_dict = [dev.to_dict() for dev in devs]
+
+    if not usage:
+        # Without the usage query these fields hold their defaults, and a
+        # serialized 0 reads as a real idle measurement. The table drops them
+        # for that reason, and a machine-readable consumer is the more likely of
+        # the two to act on it -- so absent, meaning unmeasured.
+        for dev_dict in devs_dict:
+            mig_devs_dict = (dev_dict.get("appendix") or {}).get("mig_devices") or []
+            for d in [dev_dict, *mig_devs_dict]:
+                for key in _USAGE_ONLY_KEYS:
+                    d.pop(key, None)
+
+    return json.dumps(devs_dict, indent=2)
 
 
-def format_devices_table(devs: Devices) -> str:
+def format_devices_table(devs: Devices, usage: bool = True) -> str:
     if not devs:
         return "No GPUs detected."
 
@@ -150,9 +186,14 @@ def format_devices_table(devs: Devices) -> str:
         row = [
             str(dev.index),
             dev.name if dev.name else "N/A",
-            f"{dev.memory_used}MiB / {dev.memory}MiB",
-            f"{dev.cores_utilization}%",
-            f"{dev.temperature}C" if dev.temperature is not None else "N/A",
+            # Without the usage query these fields hold their defaults, and a
+            # rendered 0 would read as a real idle measurement. Total memory
+            # and the status stay, being reported by the information query.
+            f"{dev.memory_used}MiB / {dev.memory}MiB"
+            if usage
+            else f"N/A / {dev.memory}MiB",
+            f"{dev.cores_utilization}%" if usage else "N/A",
+            f"{dev.temperature}C" if usage and dev.temperature is not None else "N/A",
             dev.compute_capability if dev.compute_capability else "N/A",
             "OK" if dev.memory_status == DeviceMemoryStatusEnum.HEALTHY else "ERR",
         ]

gpustack_runtime/deployer/__types__.py

@@ -8,7 +8,7 @@
 from concurrent.futures import ThreadPoolExecutor
 from dataclasses import dataclass, field
 from enum import Enum
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 from dataclasses_json import dataclass_json
 
@@ -1130,6 +1130,165 @@ def name_rfc1123_guard(self) -> WorkloadName:
         return self._name_rfc1123_guard
 
 
+@dataclass_json
+@dataclass
+class WorkloadStatusExit:
+    """
+    An exit information of a workload's container.
+
+    Attributes:
+        name (str):
+            Name representing the exited target, e.g., human-readable container name.
+        token (WorkloadOperationToken):
+            Token of the exited target, e.g., container ID.
+        exit_code (int | None):
+            Exit code of the container.
+        reason (str):
+            Short reason of the termination or the blocking.
+        message (str):
+            Detailed message of the termination or the blocking.
+        started_at (str):
+            Start time of the container's last execution.
+        finished_at (str):
+            Finish time of the container's last execution.
+        restart_count (int):
+            Restart times of the container.
+
+    """
+
+    name: str
+    """
+    Name representing the exited target, e.g., human-readable container name.
+    """
+    token: WorkloadOperationToken
+    """
+    Token of the exited target, e.g. container ID,
+    which addresses the same container as the logs/exec operations.
+    """
+    exit_code: int | None = None
+    """
+    Exit code of the container,
+    None if the container has not terminated, e.g. blocked from starting.
+    """
+    reason: str = ""
+    """
+    Short reason of the termination or the blocking,
+    e.g. "OOMKilled", "Error", "ImagePullBackOff".
+    """
+    message: str = ""
+    """
+    Detailed message of the termination or the blocking.
+    """
+    started_at: str = ""
+    """
+    Start time of the container's last execution,
+    empty if the container has never started.
+    """
+    finished_at: str = ""
+    """
+    Finish time of the container's last execution,
+    empty if the container has not finished.
+    """
+    restart_count: int = 0
+    """
+    Restart times of the container.
+    """
+
+
+_CONTAINER_EXITED_STATUSES = ("exited", "dead", "restarting")
+"""
+Container statuses carrying an exit code. "restarting" is one of them: a
+crash-looping container sits there between attempts with the last attempt's
+ExitCode already filled in, and that code is the diagnosis being asked for.
+"""
+
+_CONTAINER_UNSET_TIMESTAMP_PREFIX = "0001-01-01"
+"""
+What Docker fills an unset timestamp with -- Go's zero time -- rather than
+omitting the key, which is why a plain `.get(..., "")` never defaults.
+"""
+
+
+def parse_container_exit(
+    c: Any,
+    component_name_label: str,
+) -> WorkloadStatusExit | None:
+    """
+    Build the exit entry of a container that has terminated.
+
+    Docker and Podman report the same State object -- podman-py is Docker-API
+    compatible and both call sites inspect a fully reloaded container -- so both
+    deployers share this. Keeping it in one place is the point: the reason
+    derivation and the timestamp normalization below are exactly the kind of
+    detail that silently drifts when it lives in two files.
+
+    Args:
+        c:
+            A Docker-API-compatible container to inspect.
+        component_name_label:
+            The label the deployer names its components with.
+
+    Returns:
+        A WorkloadStatusExit if the container has terminated, None if it is
+        still running or pending and so contributes nothing.
+
+    """
+    if c.status not in _CONTAINER_EXITED_STATUSES:
+        return None
+
+    # Every read is .get()-able: a container whose State is missing keys (or
+    # missing entirely) must degrade gracefully, never raise.
+    state = c.attrs.get("State", {}) or {}
+
+    exit_code = state.get("ExitCode")
+    if state.get("OOMKilled"):
+        reason = "OOMKilled"
+    elif state.get("Error"):
+        reason = "Error"
+    elif exit_code not in (0, None):
+        # Docker fills State.Error for a container that failed to *start*, not
+        # for one that ran and exited non-zero -- the commonest crash of all.
+        # Without this the caller has an exit code and no reason, so it sets no
+        # state_message, while the Kubernetes deployer reports "Error" for the
+        # same event. Same field, same answer, either backend.
+        reason = "Error"
+    else:
+        reason = ""
+
+    return WorkloadStatusExit(
+        name=c.labels.get(component_name_label, "") or c.name,
+        token=c.attrs.get("Id", "") or c.name,
+        exit_code=exit_code,
+        reason=reason,
+        message=state.get("Error", ""),
+        started_at=_parse_container_timestamp(state.get("StartedAt")),
+        finished_at=_parse_container_timestamp(state.get("FinishedAt")),
+        restart_count=c.attrs.get("RestartCount", 0),
+    )
+
+
+def _parse_container_timestamp(value: str | None) -> str:
+    """
+    Normalize a container's State timestamp to what WorkloadStatusExit promises.
+
+    Args:
+        value:
+            The raw timestamp as the container's State reports it.
+
+    Returns:
+        The timestamp, empty if it never happened.
+
+    """
+    if not value or value.startswith(_CONTAINER_UNSET_TIMESTAMP_PREFIX):
+        # WorkloadStatusExit documents these as empty when they never happened,
+        # and Go's zero time rendered by a UI reads as "January 1, year 1".
+        return ""
+    # Docker reports 9-digit nanoseconds where the Kubernetes deployer formats 6
+    # (_TIMESTAMP_FORMAT), and strptime's %f accepts at most 6, so a consumer
+    # parsing this field would succeed on one backend and raise on the other.
+    return re.sub(r"(\.\d{6})\d+", r"\1", value)
+
+
 @dataclass_json
 @dataclass
 class WorkloadStatus:
@@ -1153,6 +1312,9 @@ class WorkloadStatus:
             The operation for the executable containers of the workload.
         loggable (list[WorkloadStatusOperation]):
             The operation for the loggable containers of the workload.
+        exits (list[WorkloadStatusExit]):
+            The exit information for the terminated or start-blocked containers
+            of the workload.
         state (WorkloadStatusStateEnum):
             Current state of the workload.
 
@@ -1192,6 +1354,12 @@ class WorkloadStatus:
     """
     The operation for the loggable containers of the workload.
     """
+    exits: list[WorkloadStatusExit] | None = field(default_factory=list)
+    """
+    The exit information for the terminated or start-blocked containers
+    of the workload, one entry per container.
+    Containers in neither state are not listed.
+    """
     state: WorkloadStatusStateEnum = WorkloadStatusStateEnum.UNKNOWN
     """
     The current state of the workload.
@@ -1619,6 +1787,42 @@ def map_backend_visible_devices(
                 )
         return ret
 
+    def map_visible_devices_ordering(
+        self,
+        runtime_envs: list[str],
+    ) -> dict[str, str]:
+        """
+        Return the device ordering environment variables
+        for the given runtime visible devices env names.
+
+        Only meaningful for a container seeing every device of the host:
+        it must number the devices as the detector, the driver and the vendor
+        tooling do, otherwise an index computed from detection addresses
+        another device inside the container.
+        For example, CUDA's default ordering, CUDA_DEVICE_ORDER=FASTEST_FIRST,
+        sorts the visible devices by a performance heuristic, which reshuffles
+        the ordinals on a heterogeneous host,
+        while PCI_BUS_ID sorts by PCI bus id, as NVML and the detector enumerate,
+        see https://github.com/gpustack/gpustack/issues/6041.
+        No other manufacturer documents such an ordering switch,
+        so none of them contributes a variable.
+
+        Args:
+            runtime_envs:
+                The runtime visible devices environment variable names.
+
+        Returns:
+            A dictionary mapping device ordering environment variable names
+            to corresponding values.
+
+        """
+        if any(
+            self.get_manufacturer(runtime_env) == ManufacturerEnum.NVIDIA
+            for runtime_env in runtime_envs
+        ):
+            return {"CUDA_DEVICE_ORDER": "PCI_BUS_ID"}
+        return {}
+
     def map_visible_devices_affinities(
         self,
         runtime_envs: list[str],

gpustack_runtime/deployer/cdi/ascend.py

@@ -112,9 +112,17 @@ def generate(
 
             container_device_nodes = []
 
-            cdn_path = f"/dev/davinci{dev.index}"
-            if dev.appendix.get("vgpu", False):
-                cdn_path = f"/dev/vdavinci{dev.index}"
+            # The device node is numbered by the driver's physical id, which
+            # Device.index no longer carries: it is the detector's enumeration
+            # index, i.e. the DCMI logic id here. The two are different
+            # numbers, so a device without a physical id is skipped instead of
+            # addressed by the index, which would resolve to another NPU's
+            # node. The detector already drops such a device; this guards the
+            # devices a caller passes in.
+            cdn_number = dev.appendix.get("physical_id")
+            if cdn_number is None:
+                continue
+            cdn_path = f"/dev/davinci{cdn_number}"
             cdn = device_to_cdi_device_node(
                 path=cdn_path,
             )