Skip to content

Commit b3a2732

Browse files
EngHabuclaude
andauthored
fix: surface devbox docker/kubeconfig errors as ClickException (#1041)
## Summary - Wrap the docker volume/ps subprocess calls in `_devbox.py` with a small `_run_docker` helper that converts non-zero exits into `click.ClickException` with stderr details, instead of bubbling raw `CalledProcessError` up the stack. - Use `unlink(missing_ok=True)` and translate `PermissionError` on the stale k3s kubeconfig into an actionable `ClickException`. ## Closes - [FLYTE-SDK-D](https://unionai.sentry.io/issues/FLYTE-SDK-D) — `docker volume ls` non-zero exit reported as raw subprocess crash - [FLYTE-SDK-1H](https://unionai.sentry.io/issues/FLYTE-SDK-1H) — `PermissionError` unlinking `/tmp/.kube/kubeconfig` ## Test plan - [x] New unit tests in `tests/cli/test_devbox.py` cover the failure paths for `_ensure_volume`, `_container_is_running`, `_container_is_paused`. - [x] Existing devbox tests still pass. - [ ] Manual: \`flyte start devbox\` with docker stopped → friendly message; with a root-owned `/tmp/.kube/kubeconfig` → friendly message. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d2b9006 commit b3a2732

2 files changed

Lines changed: 62 additions & 15 deletions

File tree

src/flyte/cli/_devbox.py

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,29 +36,37 @@ def _ensure_docker_available() -> None:
3636
)
3737

3838

39+
def _run_docker(cmd: list[str], failure_message: str) -> subprocess.CompletedProcess:
40+
"""Run a docker command and translate failure into a user-facing ClickException."""
41+
result = subprocess.run(cmd, capture_output=True, text=True, check=False)
42+
if result.returncode != 0:
43+
details = (result.stderr or result.stdout or "").strip()
44+
raise click.ClickException(f"{failure_message}\n{details}" if details else failure_message)
45+
return result
46+
47+
3948
def _ensure_volume(volume_name: str) -> None:
40-
result = subprocess.run(
49+
result = _run_docker(
4150
["docker", "volume", "ls", "--filter", f"name=^{volume_name}$", "--format", "{{.Name}}"],
42-
capture_output=True,
43-
text=True,
44-
check=True,
51+
f"Failed to list docker volumes while checking for '{volume_name}'.",
4552
)
4653
if volume_name not in result.stdout:
47-
subprocess.run(["docker", "volume", "create", volume_name], check=True)
54+
_run_docker(
55+
["docker", "volume", "create", volume_name],
56+
f"Failed to create docker volume '{volume_name}'.",
57+
)
4858

4959

5060
def _container_is_running(container_name: str) -> bool:
51-
result = subprocess.run(
61+
result = _run_docker(
5262
["docker", "ps", "--filter", f"name=^{container_name}$", "--format", "{{.Names}}"],
53-
capture_output=True,
54-
text=True,
55-
check=True,
63+
f"Failed to query docker for container '{container_name}'.",
5664
)
5765
return container_name in result.stdout
5866

5967

6068
def _container_is_paused(container_name: str) -> bool:
61-
result = subprocess.run(
69+
result = _run_docker(
6270
[
6371
"docker",
6472
"ps",
@@ -69,9 +77,7 @@ def _container_is_paused(container_name: str) -> bool:
6977
"--format",
7078
"{{.Names}}",
7179
],
72-
capture_output=True,
73-
text=True,
74-
check=True,
80+
f"Failed to query docker for paused container '{container_name}'.",
7581
)
7682
return container_name in result.stdout
7783

@@ -258,8 +264,13 @@ def launch_devbox(image_name: str, is_dev_mode: bool, gpu: bool = False, log_for
258264

259265
_KUBE_DIR.mkdir(parents=True, exist_ok=True)
260266
# This step makes sure that we always used the latest k3s kubeconfig file
261-
if _KUBECONFIG_PATH.exists():
262-
_KUBECONFIG_PATH.unlink()
267+
try:
268+
_KUBECONFIG_PATH.unlink(missing_ok=True)
269+
except PermissionError as e:
270+
raise click.ClickException(
271+
f"Permission denied removing stale kubeconfig at {_KUBECONFIG_PATH}. "
272+
f"Delete it manually (e.g. `sudo rm {_KUBECONFIG_PATH}`) and retry.\n{e}"
273+
)
263274

264275
steps = _STEPS_DEV if is_dev_mode else _STEPS
265276

tests/cli/test_devbox.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,3 +181,39 @@ def test_explicit_image_with_gpu_is_respected(self):
181181
result = runner.invoke(devbox, ["--gpu", "--image", "myorg/custom:latest"])
182182
assert result.exit_code == 0, result.output
183183
assert mock_launch.call_args.args[0] == "myorg/custom:latest"
184+
185+
186+
class TestDockerSubprocessFailures:
187+
"""Docker CLI failures should surface as click.ClickException, not raw CalledProcessError."""
188+
189+
def test_ensure_volume_failure_raises_click_exception(self):
190+
import click
191+
192+
from flyte.cli._devbox import _ensure_volume
193+
194+
with patch("flyte.cli._devbox.subprocess.run") as mock_run:
195+
mock_run.return_value = MagicMock(returncode=1, stdout="", stderr="docker daemon not reachable")
196+
with pytest.raises(click.ClickException) as excinfo:
197+
_ensure_volume("flyte-devbox")
198+
assert "Failed to list docker volumes" in str(excinfo.value.message)
199+
assert "docker daemon not reachable" in str(excinfo.value.message)
200+
201+
def test_container_is_running_failure_raises_click_exception(self):
202+
import click
203+
204+
from flyte.cli._devbox import _container_is_running
205+
206+
with patch("flyte.cli._devbox.subprocess.run") as mock_run:
207+
mock_run.return_value = MagicMock(returncode=1, stdout="", stderr="boom")
208+
with pytest.raises(click.ClickException):
209+
_container_is_running("flyte-devbox")
210+
211+
def test_container_is_paused_failure_raises_click_exception(self):
212+
import click
213+
214+
from flyte.cli._devbox import _container_is_paused
215+
216+
with patch("flyte.cli._devbox.subprocess.run") as mock_run:
217+
mock_run.return_value = MagicMock(returncode=1, stdout="", stderr="boom")
218+
with pytest.raises(click.ClickException):
219+
_container_is_paused("flyte-devbox")

0 commit comments

Comments
 (0)