diff --git a/gpustack_runtime/deployer/__types__.py b/gpustack_runtime/deployer/__types__.py index 6f6af93..f02bf5e 100644 --- a/gpustack_runtime/deployer/__types__.py +++ b/gpustack_runtime/deployer/__types__.py @@ -1787,6 +1787,35 @@ def map_backend_visible_devices( ) return ret + def count_requested_devices( + self, + runtime_envs: list[str], + resource_values: list[str], + ) -> int: + """ + Count the devices a resource request resolves to. + + "all" is a stand-in for every device the host has, so it is measured + rather than counted as the single literal token it is written as. + + Args: + runtime_envs: + The runtime visible devices environment variable names. + resource_values: + The resource values requested, as split from the resource + value, e.g. ``["0", "1"]`` or ``["all"]``. + + Returns: + The number of devices the request resolves to. + + """ + if resource_values == ["all"]: + return sum( + len(self.get_runtime_visible_devices(runtime_env, "plain")) + for runtime_env in runtime_envs + ) + return len(resource_values) + def map_visible_devices_ordering( self, runtime_envs: list[str], @@ -1795,7 +1824,7 @@ def map_visible_devices_ordering( 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: + Only meaningful for a container seeing more than one device: 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. diff --git a/gpustack_runtime/deployer/docker.py b/gpustack_runtime/deployer/docker.py index 96ed7db..7c85369 100644 --- a/gpustack_runtime/deployer/docker.py +++ b/gpustack_runtime/deployer/docker.py @@ -1118,11 +1118,17 @@ def _create_containers( ) create_options["environment"].update(b_vs) - # If requesting all devices or privileged, - # the container sees every device of the host, - # so pin the device ordering to keep its numbering - # aligned with the detection. - if r_v == "all" or privileged: + # Pin the device ordering whenever the container ends up + # seeing more than one device, so its numbering stays + # aligned with the detection. Requesting all devices is + # measured rather than special-cased: a single-device host + # has nothing to reorder. A privileged container sees every + # device of the host whatever it requested. + if ( + privileged + or self.count_requested_devices(runtime_envs, resource_values) + > 1 + ): o_vs = self.map_visible_devices_ordering(runtime_envs) # Take the ordering as default, # never overwrite the one declared by the container. diff --git a/gpustack_runtime/deployer/k8s/devicemanager/__init__.py b/gpustack_runtime/deployer/k8s/devicemanager/__init__.py index 4da266b..9265080 100644 --- a/gpustack_runtime/deployer/k8s/devicemanager/__init__.py +++ b/gpustack_runtime/deployer/k8s/devicemanager/__init__.py @@ -3,10 +3,13 @@ import stat from functools import lru_cache from pathlib import Path -from typing import Literal +from typing import TYPE_CHECKING, Any, Literal from gpustack_runtime import envs +if TYPE_CHECKING: + from collections.abc import Callable, Mapping + def is_kubelet_socket_accessible( kubelet_endpoint: Path | None = None, @@ -32,11 +35,71 @@ def is_kubelet_socket_accessible( return False -@lru_cache -def get_resource_injection_policy() -> Literal["env", "kdp"]: +_DEVICE_PLUGIN_RESOURCE_FAMILIES = ( + "shared", + "sliced", + "partitioned", +) +""" +Resource-name families a device plugin advertises on top of a plain CDI kind, +mirroring the GPUStack Operator's own families +(`gpustack-operator pkg/nodefeature`): "nvidia.com/gpu.shared", +"nvidia.com/gpu.sliced.units", "nvidia.com/gpu.partitioned.mig-1g.20gb", ... +A stock vendor plugin advertises only the bare kind ("nvidia.com/gpu"), so the +family segment is what tells the two apart. +""" + + +def node_has_device_plugin_resources( + node_allocatable: Mapping[str, Any], +) -> bool: + """ + Report whether a node advertises accelerators through a device plugin that + allocates them the way the GPUStack Operator does. + + Only the suffixed families count (see + :data:`_DEVICE_PLUGIN_RESOURCE_FAMILIES`): a bare CDI kind on its own is + what a stock vendor plugin advertises, and requesting a device from it + yields none of the operator's accounting. + + Args: + node_allocatable: + The allocatable resources of a node, keyed by resource name. + + Returns: + True if any allocatable resource name carries a family segment. + + """ + return any( + family in name.split(".") + for name in node_allocatable + for family in _DEVICE_PLUGIN_RESOURCE_FAMILIES + ) + + +def get_resource_injection_policy( + probe_node_allocatable: Callable[[], Mapping[str, Any] | None] | None = None, +) -> Literal["env", "kdp"]: """ Get the resource injection policy (in lowercase) for the deployer. + An explicit policy always wins. Under "auto" the decision belongs to the + cluster, not to the process doing the deploying: the Kubernetes deployer + orchestrates remotely, so whether *it* can reach a kubelet socket says + nothing about whether the *target* node runs a device plugin. So the probe + reads that node's allocatable resources and looks for a device-plugin + resource family there. + + A probe that cannot answer -- absent, failing, or unauthorized -- falls + back to KDP rather than to env: env injection hands the container every + device of the host and leaves the allocation off the plugin's ledger, so + guessing it wrong is the more damaging of the two. + + Args: + probe_node_allocatable: + Called only under the "auto" policy, to read the target node's + allocatable resources. Returns None when the node cannot be read. + Returns: The resource injection policy. @@ -45,7 +108,14 @@ def get_resource_injection_policy() -> Literal["env", "kdp"]: if policy != "auto": return policy - return "kdp" if is_kubelet_socket_accessible() else "env" + if probe_node_allocatable is None: + return "kdp" + + node_allocatable = probe_node_allocatable() + if node_allocatable is None: + return "kdp" + + return "kdp" if node_has_device_plugin_resources(node_allocatable) else "env" @lru_cache @@ -75,4 +145,5 @@ def cdi_kind_to_kdp_resource( "cdi_kind_to_kdp_resource", "get_resource_injection_policy", "is_kubelet_socket_accessible", + "node_has_device_plugin_resources", ] diff --git a/gpustack_runtime/deployer/kuberentes.py b/gpustack_runtime/deployer/kuberentes.py index 6437a48..72bbd59 100644 --- a/gpustack_runtime/deployer/kuberentes.py +++ b/gpustack_runtime/deployer/kuberentes.py @@ -656,7 +656,7 @@ def _resolve_runtime_class_name( pod.spec.runtime_class_name = runtime_class_name -def _resolve_privileged(container: Container) -> bool: +def _resolve_privileged(container: Container, kdp: bool) -> bool: """ Resolve whether a container runs privileged. @@ -670,13 +670,20 @@ def _resolve_privileged(container: Container) -> bool: slicing: a workload holding a single MIG device or a single memory slice still sees the untouched cards next to it, and a soft-slicing limit lands on whichever device comes first instead of the allocated one. + + Args: + container: + The container to resolve. + kdp: + Whether the KDP injection policy is in effect, resolved once per + Pod so a workload's containers cannot disagree on it. + """ if not container.execution or not container.execution.privileged: return False if not container.resources: return True - kdp = get_resource_injection_policy() == "kdp" for r_k in container.resources: if r_k in ("cpu", "memory"): continue @@ -1074,6 +1081,58 @@ def _parameterize_probe( return probe + def _probe_node_allocatable(self) -> dict[str, str] | None: + """ + Read the allocatable resources of the node this deployer targets, + so the injection policy can tell whether a device plugin runs there. + + Reads through ``list_node`` rather than ``read_node`` so it needs no + permission beyond the list the deployer already requires to resolve a + default node name. With no node configured it reads the same first node + ``_get_default_node_name`` would, and remembers it, so resolving the + default costs one call rather than two. + + Returns: + The node's allocatable resources, or None when the node cannot be + read -- no permission, API error, or no node at all -- so the + caller can tell "advertises nothing" from "could not look". + + """ + core_api = kubernetes.client.CoreV1Api(self._client) + try: + nodes = core_api.list_node( + field_selector=( + f"metadata.name={self._node_name}" if self._node_name else None + ), + limit=1, + ) + except kubernetes.client.exceptions.ApiException as e: + clogger.warning( + "Failed to read node allocatable resources" + "%s, assuming a device plugin is present", + _detail_api_call_error(e), + ) + return None + + if not nodes.items: + return None + + node = nodes.items[0] + if not self._node_name: + self._node_name = node.metadata.name + return node.status.allocatable or {} + + def _resolve_resource_injection_policy(self) -> str: + """ + Resolve the resource injection policy for this deployer, probing the + target node when the configured policy is "auto". + + Returns: + The resource injection policy. + + """ + return get_resource_injection_policy(self._probe_node_allocatable) + def _get_default_node_name(self) -> str: """ Get the default node name of the cluster. @@ -1386,6 +1445,10 @@ def _create_pod( ephemeral_filename_mapping, ) + # Resolve the injection policy once per Pod: under "auto" it probes the + # target node, and every container of the Pod lands on that same node. + kdp = self._resolve_resource_injection_policy() == "kdp" + cnt_init, cnt_run = -1, -1 for ci, c in enumerate(workload.containers): # Annotate container info. @@ -1421,7 +1484,7 @@ def _create_pod( run_as_user=c.execution.run_as_user, run_as_group=c.execution.run_as_group, read_only_root_filesystem=c.execution.readonly_rootfs, - privileged=_resolve_privileged(c), + privileged=_resolve_privileged(c, kdp), capabilities=( kubernetes.client.V1Capabilities( add=c.execution.capabilities.add, @@ -1441,7 +1504,6 @@ def _create_pod( # Parameterize resources if c.resources: - kdp = get_resource_injection_policy() == "kdp" fmt = "kdp" if kdp else "plain" resources: dict[str, str] = {} @@ -1546,15 +1608,26 @@ def _create_pod( ], ) - # If requesting all devices or privileged, - # the container sees every device of the host, - # so pin the device ordering to keep its numbering + # Pin the device ordering whenever the container ends up + # seeing more than one device, so its numbering stays # aligned with the detection. - # This includes requesting all devices under KDP: - # the device plugin allocates every device of the node, - # hence the container still enumerates all of them. + # That covers every multi-device request, not only "all": + # any container holding several devices numbers them + # itself, and a performance-sorted default reshuffles those + # ordinals on a heterogeneous host. + # Requesting all devices is measured rather than + # special-cased -- including under KDP, where the device + # plugin allocates every device of the node and the + # container still enumerates all of them -- because a + # single-device host has nothing to reorder. + # A privileged container sees every device of the host + # whatever it requested, so it pins regardless. # Never overwrite the ordering declared by the container. - if r_v == "all" or privileged: + if ( + privileged + or self.count_requested_devices(runtime_envs, resource_values) + > 1 + ): declared_envs = {e.name for e in container.env} container.env.extend( [ @@ -1700,10 +1773,20 @@ def __init__(self): super().__init__(_NAME) self._client = self._get_client() self._node_name = envs.GPUSTACK_RUNTIME_KUBERNETES_NODE_NAME + self._runtime_uuid_values_allowed: bool | None = None @property def allowed_runtime_uuid_values(self) -> bool: - return get_resource_injection_policy() != "kdp" + # Resolved once per deployer, unlike the per-Pod resolution the + # creation path wants: this gates how `_prepare` builds the device + # materials, which are themselves built once, and it is read there once + # per manufacturer -- so probing on every read would spend one API call + # per manufacturer to answer a question already settled. + if self._runtime_uuid_values_allowed is None: + self._runtime_uuid_values_allowed = ( + self._resolve_resource_injection_policy() != "kdp" + ) + return self._runtime_uuid_values_allowed @property def allowed_mig_devices(self) -> bool: diff --git a/gpustack_runtime/deployer/podman.py b/gpustack_runtime/deployer/podman.py index eefe869..2279362 100644 --- a/gpustack_runtime/deployer/podman.py +++ b/gpustack_runtime/deployer/podman.py @@ -1097,11 +1097,17 @@ def _create_containers( ) create_options["environment"].update(b_vs) - # If requesting all devices or privileged, - # the container sees every device of the host, - # so pin the device ordering to keep its numbering - # aligned with the detection. - if r_v == "all" or privileged: + # Pin the device ordering whenever the container ends up + # seeing more than one device, so its numbering stays + # aligned with the detection. Requesting all devices is + # measured rather than special-cased: a single-device host + # has nothing to reorder. A privileged container sees every + # device of the host whatever it requested. + if ( + privileged + or self.count_requested_devices(runtime_envs, resource_values) + > 1 + ): o_vs = self.map_visible_devices_ordering(runtime_envs) # Take the ordering as default, # never overwrite the one declared by the container. diff --git a/tests/gpustack_runtime/deployer/test_privileged.py b/tests/gpustack_runtime/deployer/test_privileged.py index 785fbf5..f8660b1 100644 --- a/tests/gpustack_runtime/deployer/test_privileged.py +++ b/tests/gpustack_runtime/deployer/test_privileged.py @@ -128,10 +128,6 @@ def _container(privileged: bool | None, resources: dict | None = None) -> Contai ), ], ) -def test_resolve_privileged(name, privileged, resources, policy, expected, monkeypatch): - monkeypatch.setattr( - "gpustack_runtime.deployer.kuberentes.get_resource_injection_policy", - lambda: policy, - ) - actual = _resolve_privileged(_container(privileged, resources)) +def test_resolve_privileged(name, privileged, resources, policy, expected): + actual = _resolve_privileged(_container(privileged, resources), policy == "kdp") assert actual == expected, f"case {name} expected {expected}, but got {actual}" diff --git a/tests/gpustack_runtime/deployer/test_resource_injection_policy.py b/tests/gpustack_runtime/deployer/test_resource_injection_policy.py new file mode 100644 index 0000000..7fcbd4f --- /dev/null +++ b/tests/gpustack_runtime/deployer/test_resource_injection_policy.py @@ -0,0 +1,208 @@ +# The deployer cases below drive its own probe and policy resolution, which are +# internal by design -- the API traffic they guard is not observable from the +# public surface. +# ruff: noqa: SLF001 + +from types import SimpleNamespace + +import kubernetes.client +import pytest + +from gpustack_runtime.deployer.k8s.devicemanager import ( + get_resource_injection_policy, + node_has_device_plugin_resources, +) +from gpustack_runtime.deployer.kuberentes import KubernetesDeployer + +# Allocatable of a node whose accelerators are advertised by a device plugin +# allocating them the way the GPUStack Operator does. +_OPERATOR_ALLOCATABLE = { + "cpu": "32", + "memory": "128Gi", + "nvidia.com/gpu": "1", + "nvidia.com/gpu.shared": "10", + "nvidia.com/gpu.sliced": "256", + "nvidia.com/gpu.sliced.units": "3200k", + "device.gpustack.ai/nvidia.visibility": "1024", +} + +# Allocatable of a node running a stock vendor device plugin: the bare CDI kind +# and nothing else. +_STOCK_ALLOCATABLE = { + "cpu": "32", + "memory": "128Gi", + "nvidia.com/gpu": "2", +} + + +@pytest.mark.parametrize( + "name, allocatable, expected", + [ + ("operator families", _OPERATOR_ALLOCATABLE, True), + ("bare CDI kind only", _STOCK_ALLOCATABLE, False), + ("no accelerator at all", {"cpu": "32", "memory": "128Gi"}, False), + ("nothing allocatable", {}, False), + # The visibility resource is deliberately outside the families, so on + # its own it must not pass for one. + ( + "visibility resource only", + {"device.gpustack.ai/nvidia.visibility": "1024"}, + False, + ), + # A partitioned family carries the profile after the family segment. + ( + "partitioned profile key", + {"nvidia.com/gpu.partitioned.mig-1g.20gb": "7"}, + True, + ), + ], +) +def test_node_has_device_plugin_resources(name, allocatable, expected): + actual = node_has_device_plugin_resources(allocatable) + assert actual == expected, f"case {name} expected {expected}, but got {actual}" + + +@pytest.mark.parametrize( + "name, configured, probe, expected", + [ + # An explicit policy is never second-guessed, and never probes. + ("explicit env", "Env", None, "env"), + ("explicit kdp", "KDP", None, "kdp"), + ( + "explicit env wins over an operator node", + "Env", + lambda: _OPERATOR_ALLOCATABLE, + "env", + ), + # Auto decides from the target node. + ("auto on an operator node", "Auto", lambda: _OPERATOR_ALLOCATABLE, "kdp"), + ("auto on a stock node", "Auto", lambda: _STOCK_ALLOCATABLE, "env"), + # A probe that cannot answer falls back to KDP: env injection hands the + # container every device of the host and leaves the allocation off the + # plugin's ledger, so guessing it wrong that way is the costlier miss. + ("auto with a failing probe", "Auto", lambda: None, "kdp"), + ("auto with no probe", "Auto", None, "kdp"), + ], +) +def test_get_resource_injection_policy(name, configured, probe, expected, monkeypatch): + monkeypatch.setattr( + "gpustack_runtime.envs.GPUSTACK_RUNTIME_KUBERNETES_RESOURCE_INJECTION_POLICY", + configured, + ) + actual = get_resource_injection_policy(probe) + assert actual == expected, f"case {name} expected {expected}, but got {actual}" + + +def test_explicit_policy_does_not_probe(monkeypatch): + """An explicit policy must not spend an API call on the node.""" + monkeypatch.setattr( + "gpustack_runtime.envs.GPUSTACK_RUNTIME_KUBERNETES_RESOURCE_INJECTION_POLICY", + "KDP", + ) + + calls = [] + + def _probe(): + calls.append(1) + return _STOCK_ALLOCATABLE + + assert get_resource_injection_policy(_probe) == "kdp" + assert calls == [], "the probe must not run under an explicit policy" + + +class _FakeNode: + def __init__(self, name, allocatable): + self.metadata = SimpleNamespace(name=name) + self.status = SimpleNamespace(allocatable=allocatable) + + +def _kubernetes_deployer(monkeypatch, nodes, node_name=None): + """A KubernetesDeployer whose only live dependency is list_node, counted.""" + calls = [] + + class _FakeCoreV1Api: + def __init__(self, client=None): + pass + + def list_node(self, field_selector=None, limit=None): + calls.append(field_selector) + items = nodes + if field_selector: + wanted = field_selector.split("=", 1)[1] + items = [n for n in nodes if n.metadata.name == wanted] + return SimpleNamespace(items=items[:limit] if limit else items) + + monkeypatch.setattr(kubernetes.client, "CoreV1Api", _FakeCoreV1Api) + + deployer = object.__new__(KubernetesDeployer) + deployer._client = None + deployer._node_name = node_name + deployer._runtime_uuid_values_allowed = None + return deployer, calls + + +def test_probe_remembers_the_node_it_resolved(monkeypatch): + """ + With no node configured the probe reads the same first node + `_get_default_node_name` would, so it remembers it rather than leaving the + next caller to pay for the lookup again. + """ + monkeypatch.setattr( + "gpustack_runtime.envs.GPUSTACK_RUNTIME_KUBERNETES_RESOURCE_INJECTION_POLICY", + "Auto", + ) + deployer, calls = _kubernetes_deployer( + monkeypatch, + [_FakeNode("node-a", _OPERATOR_ALLOCATABLE)], + ) + + assert deployer._probe_node_allocatable() == _OPERATOR_ALLOCATABLE + assert deployer._node_name == "node-a" + # The next probe addresses that node by name instead of scanning again. + deployer._probe_node_allocatable() + assert calls == [None, "metadata.name=node-a"] + + +def test_probe_reports_none_when_the_node_cannot_be_read(monkeypatch): + deployer, _ = _kubernetes_deployer(monkeypatch, []) + assert deployer._probe_node_allocatable() is None + assert deployer._node_name is None + + +def test_allowed_runtime_uuid_values_probes_once(monkeypatch): + """ + `_prepare` reads this once per manufacturer while building the device + materials, so it must not spend an API call per read. + """ + monkeypatch.setattr( + "gpustack_runtime.envs.GPUSTACK_RUNTIME_KUBERNETES_RESOURCE_INJECTION_POLICY", + "Auto", + ) + deployer, calls = _kubernetes_deployer( + monkeypatch, + [_FakeNode("node-a", _OPERATOR_ALLOCATABLE)], + node_name="node-a", + ) + + assert deployer.allowed_runtime_uuid_values is False # operator node -> kdp + assert deployer.allowed_runtime_uuid_values is False + assert deployer.allowed_runtime_uuid_values is False + assert len(calls) == 1, f"probed {len(calls)} times, expected 1" + + +def test_creation_path_still_probes_per_pod(monkeypatch): + """ + The per-Pod resolution stays live: a node that gains (or loses) a device + plugin must be seen by the next deployment, not only by the next process. + """ + monkeypatch.setattr( + "gpustack_runtime.envs.GPUSTACK_RUNTIME_KUBERNETES_RESOURCE_INJECTION_POLICY", + "Auto", + ) + nodes = [_FakeNode("node-a", dict(_STOCK_ALLOCATABLE))] + deployer, calls = _kubernetes_deployer(monkeypatch, nodes, node_name="node-a") + + assert deployer._resolve_resource_injection_policy() == "env" + nodes[0].status.allocatable = _OPERATOR_ALLOCATABLE + assert deployer._resolve_resource_injection_policy() == "kdp" + assert len(calls) == 2 diff --git a/tests/gpustack_runtime/deployer/test_visible_devices_ordering.py b/tests/gpustack_runtime/deployer/test_visible_devices_ordering.py index 4001a2c..72fbe80 100644 --- a/tests/gpustack_runtime/deployer/test_visible_devices_ordering.py +++ b/tests/gpustack_runtime/deployer/test_visible_devices_ordering.py @@ -39,6 +39,16 @@ backend_values={"CUDA_VISIBLE_DEVICES": {"0": "0", "1": "1"}}, ), } +_SINGLE_NVIDIA_MATERIALS = { + "NVIDIA_VISIBLE_DEVICES": DevicesMaterial( + manufacturer=ManufacturerEnum.NVIDIA, + runtime_env="NVIDIA_VISIBLE_DEVICES", + backend_env=["CUDA_VISIBLE_DEVICES"], + cdi="nvidia.com/gpu", + runtime_values={"0": "0"}, + backend_values={"CUDA_VISIBLE_DEVICES": {"0": "0"}}, + ), +} _AMD_MATERIALS = { "AMD_VISIBLE_DEVICES": DevicesMaterial( manufacturer=ManufacturerEnum.AMD, @@ -155,7 +165,7 @@ def _kubernetes_container_envs( ) -> list[tuple[str, str]]: monkeypatch.setattr( "gpustack_runtime.deployer.kuberentes.get_resource_injection_policy", - lambda: policy, + lambda *_args: policy, ) # Resolving the RuntimeClass reads the cluster. monkeypatch.setattr( @@ -269,6 +279,27 @@ def test_map_visible_devices_ordering(name, materials, runtime_envs, expected): None, [], ), + ( + # Several devices number themselves inside the container just as + # "all" does, so the ordering is pinned without privilege and + # without asking for every device of the host. + "several specific devices, unprivileged", + _NVIDIA_MATERIALS, + {"nvidia.com/devices": "0,1"}, + False, + None, + ["PCI_BUS_ID"], + ), + ( + # "all" on a single-device host resolves to one device, which has + # no ordering to pin -- the request is measured, not special-cased. + "all devices on a single-device host", + _SINGLE_NVIDIA_MATERIALS, + {"nvidia.com/devices": "all"}, + False, + None, + [], + ), ( "all devices, non-NVIDIA", _AMD_MATERIALS,