Repository: beeware/briefcase
Pull request #443 (2020-07-07 03:39:53)

Enhanced verification that Briefcase will be able to run docker commands
Code now uses a preflight `docker info` command to check the availabilty of docker commands.  Tested only on linux
Addresses issue #442

Unfortunately I don't have access to a Mac environment or a Windows install that is capable of running docker (you need to have the professional edition or sign up for advanced telemetry to get WSL2 to run docker for windows :cry: ), so I am not sure if these changes are safe enough to be run cross-platform.  Appreciate any advice in this regard

Limitations: On linux (ubuntu 20.04 specifically) the docker commands will just fail with error code 1 in a variety of error cases.  The 2 I check for here I differentiate by examining the text of the error message - I don't know if docker considers these errors part of their API, so these checks may be brittle.  I think the worst that happens is it just hits the fallback to the generic error message, so this should be at worst extra code that can never get run, if docker changes the text of the errors.


Patch:
diff --git a/changes/442.doc.rst b/changes/442.doc.rst
new file mode 100644
index 00000000..42f441ca
--- /dev/null
+++ b/changes/442.doc.rst
@@ -0,0 +1,1 @@
+Docker-using commands now check whether the Docker daemon is running and if the user has permission to access it.
diff --git a/src/briefcase/integrations/docker.py b/src/briefcase/integrations/docker.py
index c35015c7..fa6bdbbb 100644
--- a/src/briefcase/integrations/docker.py
+++ b/src/briefcase/integrations/docker.py
@@ -36,79 +36,60 @@ def verify_docker(command):
 
     :param command: The command that needs to perform the verification check.
     """
-    try:
-        # Try to get the version of docker that is installed.
-        output = command.subprocess.check_output(
-            ['docker', '--version'],
-            universal_newlines=True,
-            stderr=subprocess.STDOUT,
-        ).strip('\n')
 
-        # Do a simple check that the docker that was invoked
-        # actually looks like the real deal, and is a version that
-        # meets our requirements.
-        if output.startswith('Docker version '):
-            docker_version = output[15:]
-            version = docker_version.split('.')
-            if int(version[0]) < 19:
-                # Docker version isn't compatible.
-                raise BriefcaseCommandError("""
+    WRONG_DOCKER_VERSION_ERROR = """
 Briefcase requires Docker 19 or higher, but you are currently running
 version {docker_version}. Visit:
 
     {install_url}
 
 to download and install an updated version of Docker.
-{extra_content}""".format(
-                    docker_version=docker_version,
-                    **docker_install_details(command.host_os)
-                ))
-
-        else:
-            print("""
+{extra_content}
+"""
+    UNKNOWN_DOCKER_VERSION_WARNING = """
 *************************************************************************
 ** WARNING: Unable to determine the version of Docker                  **
 *************************************************************************
 
-   Briefcase will proceed, assuming everything is OK. If you experience
-   problems, this is almost certainly the cause of those problems.
+    Briefcase will proceed, assuming everything is OK. If you
+    experience problems, this is almost certainly the cause of those
+    problems.
 
-   Please report this as a bug at:
+    Please report this as a bug at:
 
-     https://github.com/beeware/briefcase/issues/new
+      https://github.com/beeware/briefcase/issues/new
 
-   In your report, please including the output from running:
+    In your report, please including the output from running:
 
-     docker --version
+      docker --version
 
-   from the command prompt.
+    from the command prompt.
 
 *************************************************************************
-""")
-    except subprocess.CalledProcessError:
-        print("""
+"""
+
+    DOCKER_INSTALLATION_STATUS_UNKNOWN_WARNING = """
 *************************************************************************
 ** WARNING: Unable to determine if Docker is installed                 **
 *************************************************************************
 
-   Briefcase will proceed, assuming everything is OK. If you experience
-   problems, this is almost certainly the cause of those problems.
+    Briefcase will proceed, assuming everything is OK. If you
+    experience problems, this is almost certainly the cause of those
+    problems.
 
-   Please report this as a bug at:
+    Please report this as a bug at:
 
-     https://github.com/beeware/briefcase/issues/new
+      https://github.com/beeware/briefcase/issues/new
 
-   In your report, please including the output from running:
+    In your report, please including the output from running:
 
-     docker --version
+      docker --version
 
-   from the command prompt.
+    from the command prompt.
 
 *************************************************************************
-""")
-    except FileNotFoundError:
-        # Docker executable doesn't exist.
-        raise BriefcaseCommandError("""
+"""
+    DOCKER_NOT_INSTALLED_ERROR = """
 Briefcase requires Docker, but it is not installed (or is not on your PATH).
 Visit:
 
@@ -118,12 +99,106 @@ to download and install git manually.
 {extra_content}
 If you have installed Docker recently and are still getting this error, you may
 need to restart your terminal session.
-""".format(**docker_install_details(command.host_os)))
+"""
+    try:
+        # Try to get the version of docker that is installed.
+        output = command.subprocess.check_output(
+            ['docker', '--version'],
+            universal_newlines=True,
+            stderr=subprocess.STDOUT,
+        ).strip('\n')
+
+        # Do a simple check that the docker that was invoked
+        # actually looks like the real deal, and is a version that
+        # meets our requirements.
+        if output.startswith('Docker version '):
+            docker_version = output[15:]
+            version = docker_version.split('.')
+            if int(version[0]) < 19:
+                # Docker version isn't compatible.
+                raise BriefcaseCommandError(
+                    WRONG_DOCKER_VERSION_ERROR.format(
+                        docker_version=docker_version,
+                        **docker_install_details(command.host_os),
+                    )
+                )
+
+        else:
+            print(UNKNOWN_DOCKER_VERSION_WARNING)
+    except subprocess.CalledProcessError:
+        print(DOCKER_INSTALLATION_STATUS_UNKNOWN_WARNING)
+    except FileNotFoundError:
+        # Docker executable doesn't exist.
+        raise BriefcaseCommandError(
+            DOCKER_NOT_INSTALLED_ERROR.format(
+                **docker_install_details(command.host_os)
+            )
+        )
+
+    # Check that docker commands can actually run.
+    _verify_docker_can_run(command)
 
     # Return the Docker wrapper
     return Docker
 
 
+def _verify_docker_can_run(command):
+    """
+    Verify that the user has sufficient permissions, and that the Docker
+    daemon is running.
+
+    This is done by attempting to run a low-impact command (docker info)
+    that requires both permissions and a working daemon.
+
+    :param command: The command that needs to perform the verification check.
+    """
+
+    LACKS_PERMISSION_ERROR = """
+Docker has been installed, but Briefcase is unable to invoke
+Docker commands. It is possible that your user does not have
+permissions to invoke Docker.
+
+See https://docs.docker.com/engine/install/linux-postinstall/
+for details on configuring access to your Docker installation.
+"""
+
+    DAEMON_NOT_RUNNING_ERROR = """
+Briefcase is unable to use Docker commands. It appears the Docker
+daemon is not running.
+
+See https://docs.docker.com/config/daemon/ for details on how to
+configure your Docker daemon.
+"""
+
+    GENERIC_DOCKER_ERROR = """
+Briefcase was unable to use Docker commands. Check your Docker
+installation, and try again.
+"""
+
+    try:
+        # Invoke a docker command to check if the daemon is running,
+        # and the user has sufficient permissions.
+        # We don't care about the output, just that it succeeds.
+        command.subprocess.check_output(
+            ['docker', 'info'],
+            universal_newlines=True,
+            stderr=subprocess.STDOUT,
+        ).strip('\n')
+    except subprocess.CalledProcessError as e:
+        failure_output = e.output
+        if 'permission denied while trying to connect' in failure_output:
+            raise BriefcaseCommandError(LACKS_PERMISSION_ERROR)
+        elif (
+            # error message on Ubuntu
+            'Is the docker daemon running?' in failure_output
+            # error message on macOS
+            or 'connect: connection refused' in failure_output
+        ):
+            raise BriefcaseCommandError(DAEMON_NOT_RUNNING_ERROR)
+        else:
+            raise BriefcaseCommandError(GENERIC_DOCKER_ERROR)
+
+
 class Docker:
     def __init__(self, command, app):
         self.command = command


Test patch:
diff --git a/tests/integrations/docker/test_verify_docker.py b/tests/integrations/docker/test_verify_docker.py
index 80fa5cc6..d2a40035 100644
--- a/tests/integrations/docker/test_verify_docker.py
+++ b/tests/integrations/docker/test_verify_docker.py
@@ -4,7 +4,7 @@ from unittest import mock
 import pytest
 
 from briefcase.exceptions import BriefcaseCommandError
-from briefcase.integrations.docker import verify_docker, Docker
+from briefcase.integrations.docker import verify_docker, Docker, _verify_docker_can_run
 
 
 @pytest.fixture
@@ -25,11 +25,16 @@ def test_docker_exists(test_command, capsys):
     # The verify call should return the Docker wrapper
     assert result == Docker
 
-    test_command.subprocess.check_output.assert_called_with(
-        ['docker', '--version'],
-        universal_newlines=True,
-        stderr=subprocess.STDOUT,
-    )
+    (
+        docker_version_called_with,
+        docker_info_called_with,
+    ) = test_command.subprocess.check_output.call_args_list
+
+    assert docker_version_called_with[0] == (['docker', '--version'],)
+    assert docker_version_called_with[1] == {'universal_newlines': True, 'stderr': subprocess.STDOUT}
+
+    assert docker_info_called_with[0] == (['docker', 'info'],)
+    assert docker_info_called_with[1] == {'universal_newlines': True, 'stderr': subprocess.STDOUT}
 
     # No console output
     output = capsys.readouterr()
@@ -57,9 +62,12 @@ def test_docker_doesnt_exist(test_command):
 def test_docker_failure(test_command, capsys):
     "If docker failed during execution, the Docker wrapper is returned with a warning"
     # Mock the return value of Docker Version
-    test_command.subprocess.check_output.side_effect = subprocess.CalledProcessError(
-        returncode=1, cmd="docker --version"
-    )
+    test_command.subprocess.check_output.side_effect = [
+        subprocess.CalledProcessError(
+            returncode=1, cmd="docker --version",
+        ),
+        'Success!',
+    ]
 
     # Invoke verify_docker
     result = verify_docker(command=test_command)
@@ -67,11 +75,16 @@ def test_docker_failure(test_command, capsys):
     # The verify call should return the Docker wrapper
     assert result == Docker
 
-    test_command.subprocess.check_output.assert_called_with(
-        ['docker', '--version'],
-        universal_newlines=True,
-        stderr=subprocess.STDOUT,
-    )
+    (
+        docker_version_called_with,
+        docker_info_called_with,
+    ) = test_command.subprocess.check_output.call_args_list
+
+    assert docker_version_called_with[0] == (['docker', '--version'],)
+    assert docker_version_called_with[1] == {'universal_newlines': True, 'stderr': subprocess.STDOUT}
+
+    assert docker_info_called_with[0] == (['docker', 'info'],)
+    assert docker_info_called_with[1] == {'universal_newlines': True, 'stderr': subprocess.STDOUT}
 
     # console output
     output = capsys.readouterr()
@@ -103,13 +116,82 @@ def test_docker_unknown_version(test_command, capsys):
     # The verify call should return the Docker wrapper
     assert result == Docker
 
-    test_command.subprocess.check_output.assert_called_with(
-        ['docker', '--version'],
-        universal_newlines=True,
-        stderr=subprocess.STDOUT,
-    )
+    docker_version_called_with, docker_info_called_with = test_command.subprocess.check_output.call_args_list
+
+    assert docker_version_called_with[0] == (['docker', '--version'],)
+    assert docker_version_called_with[1] == {'universal_newlines': True, 'stderr': subprocess.STDOUT}
+
+    assert docker_info_called_with[0] == (['docker', 'info'],)
+    assert docker_info_called_with[1] == {'universal_newlines': True, 'stderr': subprocess.STDOUT}
 
     # console output
     output = capsys.readouterr()
     assert '** WARNING: Unable to determine the version of Docker' in output.out
     assert output.err == ''
+
+
+def test_docker_exists_but_process_lacks_permission_to_use_it(test_command):
+    "If the docker daemon isn't running, the check fails."
+    message1 = '''
+Client:
+ Debug Mode: false
+
+Server:
+ERROR: Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: '''
+
+    message2 = '''Get http://%2Fvar%2Frun%2Fdocker.sock/v1.40/info: dial unix /var/run/docker.sock: connect: permission denied
+errors pretty printing info'''
+
+    error_message = message1 + message2
+    # splitting it up is to appease flake8 line length - not sure how to add noqa to a triple quoted string line
+
+    test_command.subprocess.check_output.side_effect = subprocess.CalledProcessError(
+        returncode=1, cmd="docker info", output=error_message
+    )
+    with pytest.raises(BriefcaseCommandError) as exc_info:
+        _verify_docker_can_run(command=test_command)
+
+    assert 'does not have\npermissions to invoke Docker.' in exc_info.value.msg
+
+
+docker_not_running_error_messages = [
+    '''
+    Client:
+     Debug Mode: false
+
+    Server:
+    ERROR: Error response from daemon: dial unix docker.raw.sock: connect: connection refused
+    errors pretty printing info
+    ''',  # this is the error shown on mac
+    '''
+    Client:
+     Debug Mode: false
+
+    Server:
+    ERROR: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?
+    errors pretty printing info''',  # this is the error show on linux
+]
+
+
+@pytest.mark.parametrize('error_message', docker_not_running_error_messages)
+def test_docker_exists_but_is_not_running(error_message, test_command):
+    "If the docker daemon isn't running, the check fails."
+    test_command.subprocess.check_output.side_effect = subprocess.CalledProcessError(
+        returncode=1, cmd='docker info', output=error_message,
+    )
+    with pytest.raises(BriefcaseCommandError) as exc_info:
+        _verify_docker_can_run(command=test_command)
+
+    assert 'the Docker\ndaemon is not running' in exc_info.value.msg
+
+
+def test_docker_exists_but_unknown_error_when_running_command(test_command):
+    "If docker info fails in unknown ways, the check fails."
+    test_command.subprocess.check_output.side_effect = subprocess.CalledProcessError(
+        returncode=1, cmd='docker info', output='This command failed!',
+    )
+
+    with pytest.raises(BriefcaseCommandError) as exc_info:
+        _verify_docker_can_run(command=test_command)
+
+    assert 'Check your Docker\ninstallation, and try again' in exc_info.value.msg