Follow-up: improve Docker daemon image-build errors

[aniketmaurya]

Motivation

• Users hit a generic Docker is required to build images error even when Docker is installed but the daemon is not reachable, which provides poor guidance for common failures.
• Make image-build failures actionable by surfacing whether Docker is missing, the daemon is unreachable, or the caller lacks socket permissions.

Description

• Update ImageBuilder.check_docker to use docker info so the check verifies the daemon is reachable as well as the client binary being present (src/smolvm/build.py).
• Add ImageBuilder.docker_requirement_error() to produce richer, actionable ImageError messages for three cases: Docker not installed, Docker daemon unreachable (include original Docker error text), and permission/socket-access issues (src/smolvm/build.py).
• Replace direct ImageError raises in the image build entry points to call the new diagnostic helper so all image-build paths (build_alpine_ssh, build_alpine_ssh_key, build_debian_ssh_key, and similar) surface the improved message (src/smolvm/build.py).
• Add unit tests covering the new diagnostics: missing Docker and daemon-unreachable error paths (tests/test_build.py).

Testing

• Ran ruff check src/smolvm/build.py tests/test_build.py and it passed.
• Ran PYTHONPATH=src pytest tests/test_build.py and all tests passed (7 passed).
• Running pytest tests/test_build.py without PYTHONPATH=src fails in this environment due to the package not being installed into the interpreter path (import error), which is unrelated to the code changes.

---

Codex Task

Summary by CodeRabbit

• New Features

  • Enhanced Docker availability diagnostics with detailed error messages that specify when Docker is unavailable, unreachable, or inaccessible, including installation guidance and daemon status information.

• Tests

  • Added comprehensive tests validating Docker diagnostic messages across various availability scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change enhances Docker error reporting by introducing a new diagnostic helper method that provides context-aware error messages for various Docker failure modes (missing, daemon unreachable, permissions). The method replaces hard-coded error messages across multiple build methods with richer error objects.

Changes

Cohort / File(s)	Summary
Docker Diagnostic Error Handling src/smolvm/build.py	Adds docker_requirement_error() method to diagnose and report detailed Docker availability issues. Updates check_docker() to use docker info for daemon reachability checks. Modifies four build methods (build_alpine_ssh, build_alpine_ssh_key, build_debian_ssh_key, build_openclaw_rootfs) to raise diagnostic errors instead of static messages.
Docker Diagnostics Test Coverage tests/test_build.py	Introduces TestDockerDiagnostics class with two tests: one verifying error messages when Docker is missing, another verifying error messages when the Docker daemon is unreachable with appropriate remediation advice.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Docker woes, now clearer told,

With diagnostic paths of gold,

No more cryptic error's call,

We diagnose it, one and all! 🔍✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: improving Docker daemon error messages for image-build operations with better diagnostics.
Description check	✅ Passed	The description includes all required template sections: Summary, Related Issues, Testing, and Checklist with detailed motivation, implementation details, and testing results.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/github-mention-bug]-docker-is-required-to-build-images

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can use your project's `ruff` configuration to improve the quality of Python code reviews.

Add a Ruff configuration file to your project to customize how CodeRabbit runs ruff.

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_build.py (1)

36-70: Good test coverage for the primary Docker diagnostic scenarios.

The tests correctly validate:

1. Missing Docker binary (shutil.which returns None)
2. Daemon unreachable (connection error in stderr)

Consider adding a test for the permission-denied path (lines 148-151 in docker_requirement_error) to ensure complete coverage of the diagnostic branches:

def test_docker_requirement_error_when_permission_denied(
    self, tmp_path: Path
) -> None:
    builder = ImageBuilder(cache_dir=tmp_path / "images")
    
    with (
        patch("smolvm.build.shutil.which", return_value="/usr/bin/docker"),
        patch("smolvm.build.subprocess.run") as mock_run,
    ):
        mock_run.side_effect = subprocess.CalledProcessError(
            1,
            ["docker", "info"],
            stderr="permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock",
        )
        error = builder.docker_requirement_error()
    
    assert "cannot access the Docker daemon socket" in str(error)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_build.py` around lines 36 - 70, Add a new unit test in
tests/test_build.py that exercises the permission-denied branch of
ImageBuilder.docker_requirement_error: create an
ImageBuilder(cache_dir=tmp_path/"images"), patch "smolvm.build.shutil.which" to
return a docker path and patch "smolvm.build.subprocess.run" to raise
subprocess.CalledProcessError(1, ["docker","info"], stderr="permission denied
... unix:///var/run/docker.sock"), call builder.docker_requirement_error(), and
assert the returned error message contains the expected phrase (e.g. "cannot
access the Docker daemon socket") to cover the permission-denied diagnostic
branch.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/test_build.py`:
- Around line 36-70: Add a new unit test in tests/test_build.py that exercises
the permission-denied branch of ImageBuilder.docker_requirement_error: create an
ImageBuilder(cache_dir=tmp_path/"images"), patch "smolvm.build.shutil.which" to
return a docker path and patch "smolvm.build.subprocess.run" to raise
subprocess.CalledProcessError(1, ["docker","info"], stderr="permission denied
... unix:///var/run/docker.sock"), call builder.docker_requirement_error(), and
assert the returned error message contains the expected phrase (e.g. "cannot
access the Docker daemon socket") to cover the permission-denied diagnostic
branch.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9ad8c547-318a-4d57-8aec-96bb1dd6a52b

📥 Commits

Reviewing files that changed from the base of the PR and between 30e0ca9 and 4d885a8.

📒 Files selected for processing (2)

• src/smolvm/build.py
• tests/test_build.py