From 4b15a3935d2210662f6bfc34d342352e78d619c2 Mon Sep 17 00:00:00 2001 From: T-Rajeev30 Date: Sat, 24 Jan 2026 13:56:40 +0530 Subject: [PATCH 1/2] Improve Docker availability and permission detection (Fixes #34) - Detect missing Docker binary via PATH check - Detect daemon unavailability via direct client connection failure - Detect permission issues via socket access checks - Provide clear, actionable error messages - Avoid fragile error-string parsing entirely - Use argparse parser.error() for clean CLI exits Tested scenarios: - Normal operation - Docker socket permission denied - Docker daemon stopped - Recovery after restoring permissions --- src/rocker/cli.py | 2 +- src/rocker/core.py | 68 +++++++++++++++++++++++++++++++++++++--------- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/rocker/cli.py b/src/rocker/cli.py index ca1cc715..ea07551d 100644 --- a/src/rocker/cli.py +++ b/src/rocker/cli.py @@ -50,7 +50,7 @@ def main(): extension_manager.extend_cli_parser(parser, default_args) except DependencyMissing as ex: # Catch errors if docker is missing or inaccessible. - parser.error("DependencyMissing encountered: %s" % ex) + parser.error(str(ex)) args = parser.parse_args() args_dict = vars(args) diff --git a/src/rocker/core.py b/src/rocker/core.py index 26f7a8a5..6572a2e8 100644 --- a/src/rocker/core.py +++ b/src/rocker/core.py @@ -208,24 +208,66 @@ def topological_sort(source: typing.Dict[str, typing.Set[str]]) -> typing.List[s find_reqs = find_reqs.union(missing_reqs) return sort_extensions(active_extensions, cli_args) +def _docker_binary_exists(): + import shutil + return shutil.which("docker") is not None + + +def _docker_socket_exists(): + return os.path.exists("/var/run/docker.sock") + + +def _docker_socket_accessible(): + return os.access("/var/run/docker.sock", os.R_OK | os.W_OK) + + def get_docker_client(): - """Simple helper function for pre 2.0 imports""" + """Create and validate a Docker client with actionable diagnostics.""" + + if not _docker_binary_exists(): + raise DependencyMissing( + "Docker is not installed.\n" + "Install instructions:\n" + " https://docs.docker.com/engine/install/" + ) + + if not _docker_socket_exists(): + raise DependencyMissing( + "Docker daemon is not running.\n\n" + "Fix:\n" + " sudo systemctl start docker" + ) + + if not _docker_socket_accessible(): + user = os.getenv("USER", "") + raise DependencyMissing( + "Docker permission error.\n" + f"User '{user}' does not have access to the Docker socket.\n\n" + "Fix:\n" + f" sudo usermod -aG docker {user}\n" + " log out and log back in" + ) + try: try: - docker_client = docker.from_env().api + client = docker.from_env().api except AttributeError: - # docker-py pre 2.0 - docker_client = docker.Client() - # Validate that the server is available - docker_client.ping() - return docker_client - except (docker.errors.DockerException, docker.errors.APIError, ConnectionError) as ex: - raise DependencyMissing('Docker Client failed to connect to docker daemon.' - ' Please verify that docker is installed and running.' - ' As well as that you have permission to access the docker daemon.' - ' This is usually by being a member of the docker group.' - ' The underlying error was:\n"""\n%s\n"""\n' % ex) + client = docker.Client() + + client.ping() + return client + + except (docker.errors.DockerException, + docker.errors.APIError, + ConnectionError) as ex: + raise DependencyMissing( + "Docker daemon is not responding.\n\n" + "Verify:\n" + " - Docker is running\n" + " - `docker ps` works\n\n" + f"Original error: {ex}" + ) def get_user_name(): userinfo = pwd.getpwuid(os.getuid()) From d947562de49985ac8a7a0221a503ee688c78079a Mon Sep 17 00:00:00 2001 From: T-Rajeev30 Date: Tue, 27 Jan 2026 12:21:52 +0530 Subject: [PATCH 2/2] Address maintainer feedback: move diagnostics to exception handler - Move Docker checks into exception block (only run on failure) - Restore original variable names (docker_client not client) - Restore original comments (docker-py pre 2.0, etc) - Remove separate helper functions to reduce diff noise This addresses both review comments on PR #343: 1. Minimal diff by preserving unchanged code structure 2. Better performance by only running checks when needed All error messages and user experience remain identical. --- src/rocker/core.py | 89 +++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 49 deletions(-) diff --git a/src/rocker/core.py b/src/rocker/core.py index 6572a2e8..3db27b6f 100644 --- a/src/rocker/core.py +++ b/src/rocker/core.py @@ -208,59 +208,51 @@ def topological_sort(source: typing.Dict[str, typing.Set[str]]) -> typing.List[s find_reqs = find_reqs.union(missing_reqs) return sort_extensions(active_extensions, cli_args) -def _docker_binary_exists(): - import shutil - return shutil.which("docker") is not None - - -def _docker_socket_exists(): - return os.path.exists("/var/run/docker.sock") - - -def _docker_socket_accessible(): - return os.access("/var/run/docker.sock", os.R_OK | os.W_OK) - - def get_docker_client(): - """Create and validate a Docker client with actionable diagnostics.""" - - if not _docker_binary_exists(): - raise DependencyMissing( - "Docker is not installed.\n" - "Install instructions:\n" - " https://docs.docker.com/engine/install/" - ) - - if not _docker_socket_exists(): - raise DependencyMissing( - "Docker daemon is not running.\n\n" - "Fix:\n" - " sudo systemctl start docker" - ) - - if not _docker_socket_accessible(): - user = os.getenv("USER", "") - raise DependencyMissing( - "Docker permission error.\n" - f"User '{user}' does not have access to the Docker socket.\n\n" - "Fix:\n" - f" sudo usermod -aG docker {user}\n" - " log out and log back in" - ) - + """Simple helper function for pre 2.0 imports""" try: try: - client = docker.from_env().api + docker_client = docker.from_env().api except AttributeError: - client = docker.Client() - - client.ping() - return client - - except (docker.errors.DockerException, - docker.errors.APIError, - ConnectionError) as ex: + # docker-py pre 2.0 + docker_client = docker.Client() + # Validate that the server is available + docker_client.ping() + return docker_client + except (docker.errors.DockerException, docker.errors.APIError, ConnectionError) as ex: + # Connection failed - run diagnostics to provide helpful error + + # Check 1: Is Docker installed? + import shutil + if not shutil.which("docker"): + raise DependencyMissing( + "Docker is not installed.\n" + "Install instructions:\n" + " https://docs.docker.com/engine/install/" + ) + + # Check 2: Does Docker socket exist? + socket_path = "/var/run/docker.sock" + if not os.path.exists(socket_path): + raise DependencyMissing( + "Docker daemon is not running.\n\n" + "Fix:\n" + " sudo systemctl start docker" + ) + + # Check 3: Can user access the socket? + if not os.access(socket_path, os.R_OK | os.W_OK): + user = os.getenv("USER", "your-user") + raise DependencyMissing( + "Docker permission error.\n" + f"User '{user}' does not have access to the Docker socket.\n\n" + "Fix:\n" + f" sudo usermod -aG docker {user}\n" + " log out and log back in" + ) + + # All checks passed but connection still failed raise DependencyMissing( "Docker daemon is not responding.\n\n" "Verify:\n" @@ -268,7 +260,6 @@ def get_docker_client(): " - `docker ps` works\n\n" f"Original error: {ex}" ) - def get_user_name(): userinfo = pwd.getpwuid(os.getuid()) return getattr(userinfo, 'pw_' + 'name')