feat(ascend): add Device injection policy and fix A5 driver/UB mounts - #25
feat(ascend): add Device injection policy and fix A5 driver/UB mounts#25yxf0314 wants to merge 1 commit into
Conversation
On Docker, the default Env injection policy sets the visible-devices env (ASCEND_VISIBLE_DEVICES), which makes the Ascend Docker Runtime apply device isolation. On A5 (950) that isolation hides the UB fabric, so HCCL rootInfo detection fails. The CDI mount profile, modeled on the A2/A3 operator profile, also omits A5's UB driver components (ube_mgmt, under the driver tree) and blanket-mounts a host /etc/hccl_rootinfo.json whose stale content fails rootInfo detection. - Add a "Device" resource injection policy: inject device nodes and mounts as plain Docker devices/binds (reusing the CDI generator), with no visible-devices env and without requiring CDI-capable Docker. - Drop mirrored visible-devices envs under non-Env policies so a mirrored ASCEND_VISIBLE_DEVICES cannot re-trigger the isolation. - For A5, mount the whole /usr/local/Ascend/driver (brings ube_mgmt) and stop mounting /etc/hccl_rootinfo.json. - Keep injected mounts when appending container mounts, and fix a device mirroring typo exposed by the new plain-device path.
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Device' resource injection policy for the Docker deployer, allowing direct injection of device nodes and mounts to bypass the visible-devices environment variable, which resolves issues with the Ascend A5 UB fabric. It also refactors resource parsing and updates Ascend CDI configuration generation to mount the entire driver directory for A5 devices. The review feedback correctly identifies a critical bug in _inject_devices_plain where host and container paths are inverted when constructing the Docker device mapping string, and provides a code suggestion to fix it.
| for dn in device_nodes: | ||
| if dn.path in seen: | ||
| continue | ||
| seen.add(dn.path) | ||
| devices.append( | ||
| f"{dn.host_path or dn.path}:{dn.path}:{dn.permissions or 'rwm'}", | ||
| ) |
There was a problem hiding this comment.
In device_to_cdi_device_node (defined in gpustack_runtime/deployer/cdi/__utils__.py), the parameters are passed to ConfigDeviceNode as ConfigDeviceNode(path=dev.path, host_path=container_path). This means that for any dn (a ConfigDeviceNode):
dn.pathactually holds the host path.dn.host_pathactually holds the container path.
As a result, constructing the Docker device mapping string as f"{dn.host_path or dn.path}:{dn.path}" incorrectly maps container_path:host_path instead of host_path:container_path. This inversion will cause Docker to fail to mount the device correctly or fail to start the container.
Additionally, the duplicate check if dn.path in seen: compares the host path against container paths in seen (since seen extracts the container path from existing devices).
We should fix this by correctly resolving the container path and host path, and checking/adding the container path in seen.
| for dn in device_nodes: | |
| if dn.path in seen: | |
| continue | |
| seen.add(dn.path) | |
| devices.append( | |
| f"{dn.host_path or dn.path}:{dn.path}:{dn.permissions or 'rwm'}", | |
| ) | |
| for dn in device_nodes: | |
| container_path = dn.host_path or dn.path | |
| if container_path in seen: | |
| continue | |
| seen.add(container_path) | |
| devices.append( | |
| f"{dn.path}:{container_path}:{dn.permissions or 'rwm'}", | |
| ) |
On Docker, the default Env injection policy sets the visible-devices env (ASCEND_VISIBLE_DEVICES), which makes the Ascend Docker Runtime apply device isolation. On A5 (950) that isolation hides the UB fabric, so HCCL rootInfo detection fails. The CDI mount profile, modeled on the A2/A3 operator profile, also omits A5's UB driver components (ube_mgmt, under the driver tree) and blanket-mounts a host /etc/hccl_rootinfo.json whose stale content fails rootInfo detection.