From 675ea536a3259a1a42fe627a11dabbfac4ea0bca Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Wed, 8 Mar 2017 13:39:47 +0000 Subject: [PATCH 1/2] Made the async docker client instance private --- remoteappmanager/docker/container_manager.py | 31 ++++++++++--------- .../docker/tests/test_container_manager.py | 12 +++---- remoteappmanager/tests/mocking/dummy.py | 2 +- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/remoteappmanager/docker/container_manager.py b/remoteappmanager/docker/container_manager.py index 1a63fd9f9..b2c96f441 100644 --- a/remoteappmanager/docker/container_manager.py +++ b/remoteappmanager/docker/container_manager.py @@ -37,22 +37,23 @@ class OperationInProgress(Exception): class ContainerManager(LoggingMixin): - #: The asynchronous docker client. - docker_client = Instance(AsyncDockerClient) #: The container (not host) port. We decided it's 8888 by default. It will #: be mapped to a random port on the host, so that our reverse proxy can #: refer to it. container_port = Int(8888) + #: The docker client configuration + docker_config = Dict() + #: Tracks if a given mapping id is starting up. _start_pending = Set() #: Tracks if a given container id is stopping down. _stop_pending = Set() - #: The docker client configuration - docker_config = Dict() + #: The asynchronous docker client. + _docker_client = Instance(AsyncDockerClient) def __init__(self, docker_config, *args, **kwargs): """Initializes the Container manager. @@ -210,7 +211,7 @@ def containers_from_filters(self, filters): A list of Container objects, or an empty list if nothing is found. """ containers = [] - infos = yield self.docker_client.containers(filters=filters) + infos = yield self._docker_client.containers(filters=filters) for info in infos: try: container = Container.from_docker_dict(info) @@ -241,7 +242,7 @@ def image(self, image_id_or_name): """Returns the Image object associated to a given id """ try: - image_dict = yield self.docker_client.inspect_image( + image_dict = yield self._docker_client.inspect_image( image_id_or_name) except NotFound: return None @@ -266,7 +267,7 @@ def _start_container(self, """ try: - image_info = yield self.docker_client.inspect_image(image_name) + image_info = yield self._docker_client.inspect_image(image_name) image_id = image_info["Id"] except NotFound as e: self.log.error('Could not find requested image {}'.format( @@ -349,7 +350,7 @@ def _start_container(self, ) self.log.debug("Starting host with config: %s", host_config) - host_config = yield self.docker_client.create_host_config( + host_config = yield self._docker_client.create_host_config( **host_config) # Get the host_config configuration in create_kwargs. @@ -357,7 +358,7 @@ def _start_container(self, # Then update it with the current configuration. create_kwargs.setdefault('host_config', {}).update(host_config) - resp = yield self.docker_client.create_container(**create_kwargs) + resp = yield self._docker_client.create_container(**create_kwargs) container_id = resp['Id'] @@ -366,7 +367,7 @@ def _start_container(self, # start the container try: - yield self.docker_client.start(container_id) + yield self._docker_client.start(container_id) except Exception as e: self.log.exception("Could not start container {}".format( container_id)) @@ -432,8 +433,8 @@ def _get_ip_and_port(self, container_id): # retrieve the actual port binding try: - resp = yield self.docker_client.port(container_id, - self.container_port) + resp = yield self._docker_client.port(container_id, + self.container_port) except Exception as e: raise RuntimeError("Failed to get port info for {}. " "Exception: {}.".format(container_id, @@ -483,7 +484,7 @@ def _get_container_info(self, container_id): self.log.debug("Getting container '%s'", container_id) try: - container_info = yield self.docker_client.inspect_container( + container_info = yield self._docker_client.inspect_container( container_id ) except APIError as e: @@ -508,7 +509,7 @@ def _stop_and_remove_container(self, container_id): # Stop the container try: - yield self.docker_client.stop(container_id) + yield self._docker_client.stop(container_id) except APIError: self.log.exception( "Container '{}' could not be stopped.".format( @@ -520,7 +521,7 @@ def _stop_and_remove_container(self, container_id): # Remove the container from docker try: - yield self.docker_client.remove_container(container_id) + yield self._docker_client.remove_container(container_id) except NotFound: self.log.error('Could not find requested container {} ' 'during removal'.format(container_id)) diff --git a/remoteappmanager/docker/tests/test_container_manager.py b/remoteappmanager/docker/tests/test_container_manager.py index 902da481b..b4b9ed96f 100644 --- a/remoteappmanager/docker/tests/test_container_manager.py +++ b/remoteappmanager/docker/tests/test_container_manager.py @@ -18,10 +18,10 @@ def setUp(self): super().setUp() self.manager = ContainerManager({}) self.mock_docker_client = create_docker_client() - self.manager.docker_client._sync_client = self.mock_docker_client + self.manager._docker_client._sync_client = self.mock_docker_client def test_instantiation(self): - self.assertIsNotNone(self.manager.docker_client) + self.assertIsNotNone(self.manager._docker_client) @gen_test def test_start_stop(self): @@ -102,7 +102,7 @@ def test_containers_from_url_id_exceptions(self): # Making it so that no valid dictionary is returned. docker_client.port = mock.Mock(return_value=1234) self.mock_docker_client = docker_client - self.manager.docker_client._sync_client = docker_client + self.manager._docker_client._sync_client = docker_client result = yield self.manager.container_from_url_id("url_id") self.assertEqual(result, None) @@ -205,7 +205,7 @@ def test_start_container_with_nonexisting_volume_source(self): ) # Call args and keyword args that create_container receives - docker_client = self.manager.docker_client + docker_client = self.manager._docker_client args = docker_client._sync_client.create_container.call_args actual_volume_targets = args[1]['volumes'] @@ -224,7 +224,7 @@ def test_start_container_exception_cleanup(self): def raiser(*args, **kwargs): raise Exception("Boom!") - self.manager.docker_client.start = mock.Mock(side_effect=raiser) + self.manager._docker_client.start = mock.Mock(side_effect=raiser) with self.assertRaisesRegex(Exception, 'Boom!'): yield self.manager.start_container("username", @@ -246,7 +246,7 @@ def test_start_container_exception_cleanup_2(self): def raiser(*args, **kwargs): raise Exception("Boom!") - self.manager.docker_client.port = mock.Mock(side_effect=raiser) + self.manager._docker_client.port = mock.Mock(side_effect=raiser) with self.assertRaisesRegex(Exception, 'Boom!'): yield self.manager.start_container("username", diff --git a/remoteappmanager/tests/mocking/dummy.py b/remoteappmanager/tests/mocking/dummy.py index bf2922742..1c9c6b68f 100644 --- a/remoteappmanager/tests/mocking/dummy.py +++ b/remoteappmanager/tests/mocking/dummy.py @@ -224,7 +224,7 @@ def create_container_manager(params=None): params = {'docker_config': {}} manager = ContainerManager(**params) - manager.docker_client._sync_client = create_docker_client() + manager._docker_client._sync_client = create_docker_client() return manager From c15d5d13cc1fab7596d70880e67d2651f98ac1de Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Wed, 8 Mar 2017 13:41:11 +0000 Subject: [PATCH 2/2] Forgot the default --- remoteappmanager/docker/container_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/docker/container_manager.py b/remoteappmanager/docker/container_manager.py index b2c96f441..04183bf81 100644 --- a/remoteappmanager/docker/container_manager.py +++ b/remoteappmanager/docker/container_manager.py @@ -532,7 +532,7 @@ def _stop_and_remove_container(self, container_id): else: self.log.info("Container '{}' is removed.".format(container_id)) - @default("docker_client") + @default("_docker_client") def _docker_client_default(self): return AsyncDockerClient(**self.docker_config)