refactor: Move checking Docker connectivity to ContainerManager (aws#828

)

samcli/commands/local/cli_common/invoke_context.py

@@ -6,9 +6,6 @@
 import json
 import os
 
-import docker
-import requests
-
 import samcli.lib.utils.osutils as osutils
 from samcli.commands.local.lib.local_lambda import LocalLambdaRunner
 from samcli.commands.local.lib.debug_context import DebugContext
@@ -118,6 +115,7 @@ def __init__(self,  # pylint: disable=R0914
         self._log_file_handle = None
         self._debug_context = None
         self._layers_downloader = None
+        self._container_manager = None
 
     def __enter__(self):
         """
@@ -137,7 +135,11 @@ def __enter__(self):
                                                       self._debug_args,
                                                       self._debugger_path)
 
-        self._check_docker_connectivity()
+        self._container_manager = self._get_container_manager(self._docker_network,
+                                                              self._skip_pull_image)
+
+        if not self._container_manager.is_docker_reachable:
+            raise InvokeContextException("Running AWS SAM projects locally requires Docker. Have you got it installed?")
 
         return self
 
@@ -185,15 +187,12 @@ def local_lambda_runner(self):
             locally
         """
 
-        container_manager = ContainerManager(docker_network_id=self._docker_network,
-                                             skip_pull_image=self._skip_pull_image)
-
         layer_downloader = LayerDownloader(self._layer_cache_basedir, self.get_cwd())
         image_builder = LambdaImage(layer_downloader,
                                     self._skip_pull_image,
                                     self._force_image_build)
 
-        lambda_runtime = LambdaRuntime(container_manager, image_builder)
+        lambda_runtime = LambdaRuntime(self._container_manager, image_builder)
         return LocalLambdaRunner(local_runtime=lambda_runtime,
                                  function_provider=self._function_provider,
                                  cwd=self.get_cwd(),
@@ -350,19 +349,21 @@ def _get_debug_context(debug_port, debug_args, debugger_path):
         return DebugContext(debug_port=debug_port, debug_args=debug_args, debugger_path=debugger_path)
 
     @staticmethod
-    def _check_docker_connectivity(docker_client=None):
+    def _get_container_manager(docker_network, skip_pull_image):
         """
-        Checks if Docker daemon is running. This is required for us to invoke the function locally
+        Creates a ContainerManager with specified options
 
-        :param docker_client: Instance of Docker client
-        :return bool: True, if Docker is available
-        :raises InvokeContextException: If Docker is not available
-        """
+        Parameters
+        ----------
+        docker_network str
+            Docker network identifier
+        skip_pull_image bool
+            Should the manager skip pulling the image
 
-        docker_client = docker_client or docker.from_env()
+        Returns
+        -------
+        samcli.local.docker.manager.ContainerManager
+            Object representing Docker container manager
+        """
 
-        try:
-            docker_client.ping()
-        # When Docker is not installed, a request.exceptions.ConnectionError is thrown.
-        except (docker.errors.APIError, requests.exceptions.ConnectionError):
-            raise InvokeContextException("Running AWS SAM projects locally requires Docker. Have you got it installed?")
+        return ContainerManager(docker_network_id=docker_network, skip_pull_image=skip_pull_image)

samcli/local/docker/manager.py

@@ -6,6 +6,7 @@
 import sys
 
 import docker
+import requests
 
 LOG = logging.getLogger(__name__)
 
@@ -33,6 +34,26 @@ def __init__(self,
         self.docker_network_id = docker_network_id
         self.docker_client = docker_client or docker.from_env()
 
+    @property
+    def is_docker_reachable(self):
+        """
+        Checks if Docker daemon is running. This is required for us to invoke the function locally
+
+        Returns
+        -------
+        bool
+            True, if Docker is available, False otherwise
+        """
+        try:
+            self.docker_client.ping()
+
+            return True
+
+        # When Docker is not installed, a request.exceptions.ConnectionError is thrown.
+        except (docker.errors.APIError, requests.exceptions.ConnectionError):
+            LOG.debug("Docker is not reachable", exc_info=True)
+            return False
+
     def run(self, container, input_data=None, warm=False):
         """
         Create and run a Docker container based on the given configuration.

tests/unit/commands/local/cli_common/test_invoke_context.py

@@ -5,15 +5,11 @@
 import os
 import sys
 
-import docker
-import requests
-
 from samcli.commands.local.cli_common.user_exceptions import InvokeContextException, DebugContextException
 from samcli.commands.local.cli_common.invoke_context import InvokeContext
 
 from unittest import TestCase
-from mock import Mock, patch, ANY, mock_open
-from parameterized import parameterized, param
+from mock import Mock, PropertyMock, patch, ANY, mock_open
 
 
 class TestInvokeContext__enter__(TestCase):
@@ -57,24 +53,70 @@ def test_must_read_from_necessary_files(self, SamFunctionProviderMock):
         invoke_context._get_debug_context = Mock()
         invoke_context._get_debug_context.return_value = debug_context_mock
 
-        invoke_context._check_docker_connectivity = Mock()
+        container_manager_mock = Mock()
+        container_manager_mock.is_docker_reachable = True
+        invoke_context._get_container_manager = Mock(return_value=container_manager_mock)
 
         # Call Enter method manually for testing purposes
         result = invoke_context.__enter__()
         self.assertTrue(result is invoke_context, "__enter__() must return self")
 
-        self.assertEquals(invoke_context._template_dict, template_dict)
-        self.assertEquals(invoke_context._function_provider, function_provider)
-        self.assertEquals(invoke_context._env_vars_value, env_vars_value)
-        self.assertEquals(invoke_context._log_file_handle, log_file_handle)
-        self.assertEquals(invoke_context._debug_context, debug_context_mock)
+        self.assertEqual(invoke_context._template_dict, template_dict)
+        self.assertEqual(invoke_context._function_provider, function_provider)
+        self.assertEqual(invoke_context._env_vars_value, env_vars_value)
+        self.assertEqual(invoke_context._log_file_handle, log_file_handle)
+        self.assertEqual(invoke_context._debug_context, debug_context_mock)
+        self.assertEqual(invoke_context._container_manager, container_manager_mock)
 
         invoke_context._get_template_data.assert_called_with(template_file)
         SamFunctionProviderMock.assert_called_with(template_dict, {"AWS::Region": "region"})
         invoke_context._get_env_vars_value.assert_called_with(env_vars_file)
         invoke_context._setup_log_file.assert_called_with(log_file)
         invoke_context._get_debug_context.assert_called_once_with(1111, "args", "path-to-debugger")
-        invoke_context._check_docker_connectivity.assert_called_with()
+        invoke_context._get_container_manager.assert_called_once_with("network", True)
+
+    @patch("samcli.commands.local.cli_common.invoke_context.SamFunctionProvider")
+    def test_must_use_container_manager_to_check_docker_connectivity(self, SamFunctionProviderMock):
+        invoke_context = InvokeContext("template-file")
+
+        invoke_context._get_template_data = Mock()
+        invoke_context._get_env_vars_value = Mock()
+        invoke_context._setup_log_file = Mock()
+        invoke_context._get_debug_context = Mock()
+
+        container_manager_mock = Mock()
+
+        with patch.object(type(container_manager_mock), "is_docker_reachable",
+                          create=True, new_callable=PropertyMock, return_value=True) as is_docker_reachable_mock:
+            invoke_context._get_container_manager = Mock()
+            invoke_context._get_container_manager.return_value = container_manager_mock
+
+            invoke_context.__enter__()
+
+            is_docker_reachable_mock.assert_called_once_with()
+
+    @patch("samcli.commands.local.cli_common.invoke_context.SamFunctionProvider")
+    def test_must_raise_if_docker_is_not_reachable(self, SamFunctionProviderMock):
+        invoke_context = InvokeContext("template-file")
+
+        invoke_context._get_template_data = Mock()
+        invoke_context._get_env_vars_value = Mock()
+        invoke_context._setup_log_file = Mock()
+        invoke_context._get_debug_context = Mock()
+
+        container_manager_mock = Mock()
+
+        with patch.object(type(container_manager_mock), "is_docker_reachable",
+                          create=True, new_callable=PropertyMock, return_value=False):
+
+            invoke_context._get_container_manager = Mock()
+            invoke_context._get_container_manager.return_value = container_manager_mock
+
+            with self.assertRaises(InvokeContextException) as ex_ctx:
+                invoke_context.__enter__()
+
+                self.assertEqual("Running AWS SAM projects locally requires Docker. Have you got it installed?",
+                                 str(ex_ctx.exception))
 
 
 class TestInvokeContext__exit__(TestCase):
@@ -170,19 +212,16 @@ def setUp(self):
 
     @patch("samcli.commands.local.cli_common.invoke_context.LambdaImage")
     @patch("samcli.commands.local.cli_common.invoke_context.LayerDownloader")
-    @patch("samcli.commands.local.cli_common.invoke_context.ContainerManager")
     @patch("samcli.commands.local.cli_common.invoke_context.LambdaRuntime")
     @patch("samcli.commands.local.cli_common.invoke_context.LocalLambdaRunner")
+    @patch("samcli.commands.local.cli_common.invoke_context.SamFunctionProvider")
     def test_must_create_runner(self,
+                                SamFunctionProviderMock,
                                 LocalLambdaMock,
                                 LambdaRuntimeMock,
-                                ContainerManagerMock,
                                 download_layers_mock,
                                 lambda_image_patch):
 
-        container_mock = Mock()
-        ContainerManagerMock.return_value = container_mock
-
         runtime_mock = Mock()
         LambdaRuntimeMock.return_value = runtime_mock
 
@@ -199,18 +238,26 @@ def test_must_create_runner(self,
         self.context.get_cwd = Mock()
         self.context.get_cwd.return_value = cwd
 
-        result = self.context.local_lambda_runner
-        self.assertEquals(result, runner_mock)
+        self.context._get_template_data = Mock()
+        self.context._get_env_vars_value = Mock()
+        self.context._setup_log_file = Mock()
+        self.context._get_debug_context = Mock(return_value=None)
+
+        container_manager_mock = Mock()
+        container_manager_mock.is_docker_reachable = PropertyMock(return_value=True)
+        self.context._get_container_manager = Mock(return_value=container_manager_mock)
+
+        with self.context:
+            result = self.context.local_lambda_runner
+            self.assertEquals(result, runner_mock)
 
-        ContainerManagerMock.assert_called_with(docker_network_id="network",
-                                                skip_pull_image=True)
-        LambdaRuntimeMock.assert_called_with(container_mock, image_mock)
-        lambda_image_patch.assert_called_once_with(download_mock, True, True)
-        LocalLambdaMock.assert_called_with(local_runtime=runtime_mock,
-                                           function_provider=ANY,
-                                           cwd=cwd,
-                                           debug_context=None,
-                                           env_vars_values=ANY)
+            LambdaRuntimeMock.assert_called_with(container_manager_mock, image_mock)
+            lambda_image_patch.assert_called_once_with(download_mock, True, True)
+            LocalLambdaMock.assert_called_with(local_runtime=runtime_mock,
+                                               function_provider=ANY,
+                                               cwd=cwd,
+                                               debug_context=None,
+                                               env_vars_values=ANY)
 
 
 class TestInvokeContext_stdout_property(TestCase):
@@ -398,34 +445,3 @@ def test_debugger_path_resolves(self, pathlib_mock, debug_context_mock):
         resolve_path_mock.is_dir.assert_called_once()
         pathlib_path_mock.resolve.assert_called_once_with(strict=True)
         pathlib_mock.assert_called_once_with("./path")
-
-
-class TestInvokeContext_check_docker_connectivity(TestCase):
-
-    def test_must_call_ping(self):
-        client = Mock()
-        InvokeContext._check_docker_connectivity(client)
-        client.ping.assert_called_with()
-
-    @patch("samcli.commands.local.cli_common.invoke_context.docker")
-    def test_must_call_ping_with_docker_client_from_env(self, docker_mock):
-        client = Mock()
-        docker_mock.from_env.return_value = client
-
-        InvokeContext._check_docker_connectivity()
-        client.ping.assert_called_with()
-
-    @parameterized.expand([
-        param("Docker APIError thrown", docker.errors.APIError("error")),
-        param("Requests ConnectionError thrown", requests.exceptions.ConnectionError("error"))
-    ])
-    def test_must_raise_if_docker_not_found(self, test_name, error_docker_throws):
-        client = Mock()
-
-        client.ping.side_effect = error_docker_throws
-
-        with self.assertRaises(InvokeContextException) as ex_ctx:
-            InvokeContext._check_docker_connectivity(client)
-
-        msg = str(ex_ctx.exception)
-        self.assertEquals(msg, "Running AWS SAM projects locally requires Docker. Have you got it installed?")

tests/unit/local/docker/test_manager.py

@@ -3,8 +3,10 @@
 """
 
 import io
-
 from unittest import TestCase
+
+import requests
+
 from mock import Mock
 from docker.errors import APIError, ImageNotFound
 from samcli.local.docker.manager import ContainerManager, DockerImagePullFailedException
@@ -206,6 +208,41 @@ def test_must_raise_if_image_not_found(self):
         self.assertEquals(str(ex), msg)
 
 
+class TestContainerManager_is_docker_reachable(TestCase):
+
+    def setUp(self):
+        self.ping_mock = Mock()
+
+        docker_client_mock = Mock()
+        docker_client_mock.ping = self.ping_mock
+
+        self.manager = ContainerManager(docker_client=docker_client_mock)
+
+    def test_must_use_docker_client_ping(self):
+        self.manager.is_docker_reachable
+
+        self.ping_mock.assert_called_once_with()
+
+    def test_must_return_true_if_ping_does_not_raise(self):
+        is_reachable = self.manager.is_docker_reachable
+
+        self.assertTrue(is_reachable)
+
+    def test_must_return_false_if_ping_raises_api_error(self):
+        self.ping_mock.side_effect = APIError("error")
+
+        is_reachable = self.manager.is_docker_reachable
+
+        self.assertFalse(is_reachable)
+
+    def test_must_return_false_if_ping_raises_connection_error(self):
+        self.ping_mock.side_effect = requests.exceptions.ConnectionError("error")
+
+        is_reachable = self.manager.is_docker_reachable
+
+        self.assertFalse(is_reachable)
+
+
 class TestContainerManager_has_image(TestCase):
 
     def setUp(self):