feat: make the docker binary path a hook

Where docker lives is device-specific; what to do with it is not. QNAP's
Container Station installs docker under /share/<pool>/.qpkg/ and never
puts it on PATH, so a QTS driver inheriting this class found no docker at
all.

Rather than reimplementing the Docker surface in the vendor driver, the
path becomes a single overridable method and every call site goes through
it. Per docs/ARCHITECTURE.md 4.4, generic logic belongs to the shared
driver and only the device-specific mechanics belong to the vendor one.

A test asserts that *every* docker call site uses the hook — a half
converted set would let detection find the binary while the actual
queries still missed it, which only shows up against real hardware.
This commit is contained in:
2026-08-21 10:28:12 +07:00
parent 661d56074c
commit c8fc46c373
2 changed files with 74 additions and 10 deletions
+23 -10
View File
@@ -1480,6 +1480,16 @@ class LinuxDriver(OSDriver):
# Docker # Docker
# ------------------------------------------------------------------ # ------------------------------------------------------------------
def _docker_bin(self) -> str:
"""Path to the docker binary.
A hook rather than a literal because the Docker *logic* is the same
everywhere while the *location* is not: QTS ships Container Station's
docker under /share/<pool>/.qpkg/ and never puts it on PATH. Subclasses
override this one method instead of reimplementing the surface.
"""
return "docker"
def get_docker_info(self) -> DockerInfoDict: def get_docker_info(self) -> DockerInfoDict:
"""Return information about the local Docker environment. """Return information about the local Docker environment.
@@ -1498,26 +1508,28 @@ class LinuxDriver(OSDriver):
""" """
import json as _json import json as _json
docker = self._docker_bin()
# Check docker binary first (docker --version doesn't need socket access) # Check docker binary first (docker --version doesn't need socket access)
if not self._send("command -v docker 2>/dev/null").strip(): if not self._send(f"command -v {docker} 2>/dev/null").strip():
return {"available": False} return {"available": False}
# Verify socket access — docker ps is cheaper and fails immediately on permission errors # Verify socket access — docker ps is cheaper and fails immediately on permission errors
ps_check = self._send("docker ps 2>&1") ps_check = self._send(f"{docker} ps 2>&1")
if "permission denied" in ps_check.lower() or "cannot connect" in ps_check.lower(): if "permission denied" in ps_check.lower() or "cannot connect" in ps_check.lower():
return {"available": False, "permission_denied": True} return {"available": False, "permission_denied": True}
version = self._send("docker --version 2>/dev/null").strip() version = self._send(f"{docker} --version 2>/dev/null").strip()
combined = self._send( combined = self._send(
"echo '---CONTAINERS---'; " "echo '---CONTAINERS---'; "
"docker ps -a --format '{{json .}}' 2>/dev/null; " f"{docker} ps -a --format '{{{{json .}}}}' 2>/dev/null; "
"echo '---IMAGES---'; " "echo '---IMAGES---'; "
"docker images --format '{{json .}}' 2>/dev/null; " f"{docker} images --format '{{{{json .}}}}' 2>/dev/null; "
"echo '---VOLUMES---'; " "echo '---VOLUMES---'; "
"docker volume ls --format '{{json .}}' 2>/dev/null; " f"{docker} volume ls --format '{{{{json .}}}}' 2>/dev/null; "
"echo '---NETWORKS---'; " "echo '---NETWORKS---'; "
"docker network ls --format '{{json .}}' 2>/dev/null", f"{docker} network ls --format '{{{{json .}}}}' 2>/dev/null",
read_timeout=60, read_timeout=60,
) )
@@ -1662,7 +1674,8 @@ class LinuxDriver(OSDriver):
for img_name in candidate_images: for img_name in candidate_images:
try: try:
local_raw = self._send( local_raw = self._send(
f"docker inspect {img_name!r} --format '{{{{index .RepoDigests 0}}}}' 2>/dev/null", f"{self._docker_bin()} inspect {img_name!r} "
f"--format '{{{{index .RepoDigests 0}}}}' 2>/dev/null",
read_timeout=5, read_timeout=5,
).strip() ).strip()
if not local_raw or "@" not in local_raw: if not local_raw or "@" not in local_raw:
@@ -1670,7 +1683,7 @@ class LinuxDriver(OSDriver):
local_digest = local_raw.split("@", 1)[1] local_digest = local_raw.split("@", 1)[1]
remote_full = self._send( remote_full = self._send(
f"docker buildx imagetools inspect {img_name!r} 2>&1", f"{self._docker_bin()} buildx imagetools inspect {img_name!r} 2>&1",
read_timeout=30, read_timeout=30,
).strip() ).strip()
if ("429" in remote_full if ("429" in remote_full
@@ -1712,7 +1725,7 @@ class LinuxDriver(OSDriver):
import shlex as _shlex import shlex as _shlex
raw = self._send( raw = self._send(
f"docker inspect {_shlex.quote(container_id)} 2>/dev/null", f"{self._docker_bin()} inspect {_shlex.quote(container_id)} 2>/dev/null",
read_timeout=10, read_timeout=10,
).strip() ).strip()
if not raw: if not raw:
+51
View File
@@ -802,3 +802,54 @@ class TestRunDeviceActionDispatch:
) as mock_action: ) as mock_action:
driver.run_device_action("apt_update_upgrade") driver.run_device_action("apt_update_upgrade")
mock_action.assert_called_once() mock_action.assert_called_once()
class TestDockerBinHook:
"""Where the docker binary lives is device-specific; what to do with it is not.
QTS puts Container Station's docker under /share/<pool>/.qpkg/ and not on
PATH. Rather than duplicating the whole Docker surface in the QNAP driver,
the path is a one-method hook here and the logic stays generic. See
docs/ARCHITECTURE.md §4.4, "Generisch vs. treiberspezifisch".
"""
def test_defaults_to_docker_on_path(self, driver):
assert driver._docker_bin() == "docker"
def test_detection_uses_the_hook(self, driver):
"""A subclass pointing elsewhere must not be probed for a PATH docker."""
sent = []
def _record(cmd, **kwargs):
sent.append(cmd)
return ""
with patch.object(driver, "_docker_bin", return_value="/opt/cs/docker"):
with patch.object(driver, "_send", side_effect=_record):
result = driver.get_docker_info()
assert result == {"available": False}
assert any("/opt/cs/docker" in cmd for cmd in sent)
assert not any("command -v docker " in cmd for cmd in sent)
def test_all_docker_subcommands_use_the_hook(self, driver):
"""Half-converted call sites are the failure mode here: detection would
find the binary and the actual queries would still miss it."""
sent = []
def _record(cmd, **kwargs):
sent.append(cmd)
if "command -v" in cmd:
return "/opt/cs/docker"
if "---CONTAINERS---" in cmd:
return "---CONTAINERS---\n---IMAGES---\n---VOLUMES---\n---NETWORKS---\n"
return ""
with patch.object(driver, "_docker_bin", return_value="/opt/cs/docker"):
with patch.object(driver, "_send", side_effect=_record):
driver.get_docker_info()
docker_cmds = [c for c in sent if "docker" in c]
assert docker_cmds
for cmd in docker_cmds:
assert "/opt/cs/docker" in cmd, f"unconverted call site: {cmd}"