From 4a058cdfca6c815d15284a44da765b9da9f70ceb Mon Sep 17 00:00:00 2001 From: Shay Arbov Date: Sat, 10 Feb 2018 15:31:21 +0200 Subject: [PATCH] Use docker python client instead of subprocess when possible --- Makefile.internal | 1 + docker_test_tools/environment.py | 164 ++++++++++--------------- requirements.txt | 3 +- tests/ut/test_environment.py | 202 ++++++++++++------------------- 4 files changed, 147 insertions(+), 223 deletions(-) diff --git a/Makefile.internal b/Makefile.internal index 6920297..b68ad7c 100644 --- a/Makefile.internal +++ b/Makefile.internal @@ -26,6 +26,7 @@ coverage: test nose2: mkdir -p build/ + # Run the example nose2 tests - validate the package works DTT_COMPOSE_PATH=$(DTT_COMPOSE_PATH) \ nose2 --config=tests/integration/nose2.cfg --verbose --project-directory . diff --git a/docker_test_tools/environment.py b/docker_test_tools/environment.py index 27a8fc2..6fbbe6c 100644 --- a/docker_test_tools/environment.py +++ b/docker_test_tools/environment.py @@ -19,13 +19,19 @@ class EnvironmentController(object): """Utility for managing environment operations.""" - def __init__(self, project_name, compose_path, log_path, collect_stats=False, reuse_containers=False): + def __init__(self, + project_name, + compose_path, + log_path, + collect_stats=False, + reuse_containers=False): self.log_path = log_path self.compose_path = compose_path self.project_name = project_name self.reuse_containers = reuse_containers + self.docker_client = docker.client.APIClient() self.environment_variables = self._get_environment_variables() self.services = self.get_services() @@ -42,12 +48,16 @@ def __init__(self, project_name, compose_path, log_path, collect_stats=False, re self.plugins = [] self.plugins.append(self.logs_collector) - self.docker_client = docker.client.APIClient() + if collect_stats: - self.plugins.append(stats.StatsCollector(encoding=self.encoding, - project=self.project_name, - target_dir_path=self.work_dir, - environment_variables=self.environment_variables)) + self.plugins.append( + stats.StatsCollector( + encoding=self.encoding, + project=self.project_name, + target_dir_path=self.work_dir, + environment_variables=self.environment_variables + ) + ) @classmethod def from_file(cls, config_path): @@ -65,7 +75,7 @@ def from_file(cls, config_path): def get_services(self): """Get the services info based on the compose file. - :return dict: of format {'service-name': check_callback} + :return list: service names. """ log.debug("Getting environment services, using docker compose: %s", self.compose_path) try: @@ -158,117 +168,70 @@ def kill_containers(self): stderr=subprocess.STDOUT, env=self.environment_variables ) except subprocess.CalledProcessError as error: - raise RuntimeError("Failed running environment containers, reason: %s" % error.output) + raise RuntimeError("Failed killing environment containers, reason: %s" % error.output) def kill_container(self, name): """Kill the container. :param str name: container name as it appears in the docker compose file. """ - self.validate_service_name(name) log.debug("Killing %s container", name) - try: - subprocess.check_output( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'kill', name], - stderr=subprocess.STDOUT, env=self.environment_variables - ) - except subprocess.CalledProcessError as error: - raise RuntimeError("Failed killing container %s reason: %s" % (name, error.output)) + container_id = self.get_container_id(name=name) + self.docker_client.kill(container_id) def restart_container(self, name): """Restart the container. :param str name: container name as it appears in the docker compose file. """ - self.validate_service_name(name) - log.debug("Restarting container %s", name) - try: - subprocess.check_output( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'restart', name], - stderr=subprocess.STDOUT, env=self.environment_variables - ) - except subprocess.CalledProcessError as error: - raise RuntimeError("Failed restarting container %s reason: %s" % (name, error.output)) + log.debug("Restarting %s container", name) + container_id = self.get_container_id(name=name) + self.docker_client.restart(container_id) def pause_container(self, name): """Pause the container. :param str name: container name as it appears in the docker compose file. """ - self.validate_service_name(name) log.debug("Pausing %s container", name) - try: - subprocess.check_output( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'pause', name], - stderr=subprocess.STDOUT, env=self.environment_variables - ) - except subprocess.CalledProcessError as error: - raise RuntimeError("Failed pausing container %s reason: %s" % (name, error.output)) + container_id = self.get_container_id(name=name) + self.docker_client.pause(container_id) def unpause_container(self, name): """Unpause the container. :param str name: container name as it appears in the docker compose file. """ - self.validate_service_name(name) log.debug("Unpausing %s container", name) - try: - subprocess.check_output( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'unpause', name], - stderr=subprocess.STDOUT, env=self.environment_variables - ) - except subprocess.CalledProcessError as error: - raise RuntimeError("Failed unpausing container %s reason: %s" % (name, error.output)) + container_id = self.get_container_id(name=name) + self.docker_client.unpause(container_id) def stop_container(self, name): """Stop the container. :param str name: container name as it appears in the docker compose file. """ - self.validate_service_name(name) log.debug("Stopping %s container", name) - try: - subprocess.check_output( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'stop', name], - stderr=subprocess.STDOUT, env=self.environment_variables - ) - except subprocess.CalledProcessError as error: - raise RuntimeError("Failed stopping container %s reason: %s" % (name, error.output)) + container_id = self.get_container_id(name=name) + self.docker_client.stop(container_id) def start_container(self, name): """Start the container. :param str name: container name as it appears in the docker compose file. """ - self.validate_service_name(name) log.debug("Starting %s container", name) - try: - subprocess.check_output( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'start', name], - stderr=subprocess.STDOUT, env=self.environment_variables - ) - except subprocess.CalledProcessError as error: - raise RuntimeError("Failed starting container %s reason: %s" % (name, error.output)) + container_id = self.get_container_id(name=name) + self.docker_client.start(container_id) - def get_container_id(self, name): - """Get container id by name. + def inspect_container(self, name): + """Returns the inspect content of a container - :param str name: container name as it appears in the docker compose file. + :param name: name of container """ - self.validate_service_name(name) - - # Filter the required container by container name docker-compose project. - # Since the python docker client support filtering only by one label, the second filter is done manually - filters = { - "label": "com.docker.compose.service={service}".format(service=name) - } - containers = self.docker_client.containers(filters=filters) - containers = [container for container in containers - if 'com.docker.compose.project' in container['Labels'] and - container['Labels']['com.docker.compose.project'] == self.project_name] - if len(containers) != 1: - raise RuntimeError("Unexpected containers number (%d) were found for name %s and project %s" % (len(containers), name, self.project_name)) - return containers[0]['Id'] + log.debug("Inspecting %s container", name) + container_id = self.get_container_id(name) + return self.docker_client.inspect_container(container_id) def is_container_ready(self, name): """Return True if the container is in ready state. @@ -278,7 +241,7 @@ def is_container_ready(self, name): :param str name: container name as it appears in the docker compose file. """ - status_output = self._inspect(name)['State'] + status_output = self.inspect_container(name)['State'] if 'Health' in status_output: is_ready = status_output['Health']['Status'] == "healthy" @@ -293,7 +256,7 @@ def container_status(self, name): :param str name: container name as it appears in the docker compose file. """ - return self._inspect(name)['State']['Status'] + return self.inspect_container(name)['State']['Status'] def wait_for_services(self, services=None, interval=1, timeout=60): """Wait for the services checks to pass. @@ -325,12 +288,12 @@ def container_down(self, name, health_check=None, interval=1, timeout=60): >>> >>> # container will be back up after context end """ - self.validate_service_name(name) - self.kill_container(name=name) + container_id = self.get_container_id(name) + self.docker_client.kill(container_id) try: yield finally: - self.restart_container(name=name) + self.docker_client.restart(container_id) self.wait_for_health(name=name, health_check=health_check, interval=interval, timeout=timeout) @contextmanager @@ -352,12 +315,12 @@ def container_paused(self, name, health_check=None, interval=1, timeout=60): >>> >>> # container will be back up after context end """ - self.validate_service_name(name) - self.pause_container(name=name) + container_id = self.get_container_id(name) + self.docker_client.pause(container_id) try: yield finally: - self.unpause_container(name=name) + self.docker_client.unpause(container_id) self.wait_for_health(name=name, health_check=health_check, interval=interval, timeout=timeout) @contextmanager @@ -379,12 +342,12 @@ def container_stopped(self, name, health_check=None, interval=1, timeout=60): >>> >>> # container will be back up after context end """ - self.validate_service_name(name) - self.stop_container(name=name) + container_id = self.get_container_id(name) + self.docker_client.stop(container_id) try: yield finally: - self.start_container(name=name) + self.docker_client.start(container_id) self.wait_for_health(name=name, health_check=health_check, interval=interval, timeout=timeout) def wait_for_health(self, name, health_check=None, interval=1, timeout=60): @@ -399,10 +362,6 @@ def wait_for_health(self, name, health_check=None, interval=1, timeout=60): health_check = health_check if health_check else lambda: self.is_container_ready(name) waiting.wait(health_check, sleep_seconds=interval, timeout_seconds=timeout) - def validate_service_name(self, name): - if name not in self.services: - raise ValueError('Invalid service name: %r, must be one of %s' % (name, self.services)) - @staticmethod def _get_environment_variables(): """Set the compose api version according to the server's api version""" @@ -416,14 +375,25 @@ def update_plugins(self, message): for plugin in self.plugins: plugin.update(message=message) - def _inspect(self, name): - """ - Returns the inspect content of a container - :param name: name of container + def get_container_id(self, name): + """Get container id by name. + + :param str name: container name as it appears in the docker compose file. """ self.validate_service_name(name) - log.debug("Getting %s container state", name) - container_id = self.get_container_id(name) - inspect_output = self.docker_client.inspect_container(container_id) - return inspect_output + # Filter the required container by container name docker-compose project. + # Since the python docker client support filtering only by one label, the second filter is done manually + filters = {"label": "com.docker.compose.service={service}".format(service=name)} + containers = self.docker_client.containers(filters=filters) + containers = [container for container in containers + if 'com.docker.compose.project' in container['Labels'] and + container['Labels']['com.docker.compose.project'] == self.project_name] + if len(containers) != 1: + raise RuntimeError("Unexpected containers number (%d) were found for name %s and project %s" % ( + len(containers), name, self.project_name)) + return containers[0]['Id'] + + def validate_service_name(self, name): + if name not in self.services: + raise ValueError('Invalid service name: %r, must be one of %s' % (name, self.services)) diff --git a/requirements.txt b/requirements.txt index 25c6bb8..a11e042 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,7 @@ pbr==1.8 +docker==2.7.0 waiting==1.3.0 requests==2.13.0 humanfriendly==2.2.1 -docker-compose==1.13.0 +docker-compose==1.19.0 requests-unixsocket==0.1.5 diff --git a/tests/ut/test_environment.py b/tests/ut/test_environment.py index 9cf80d3..c426807 100644 --- a/tests/ut/test_environment.py +++ b/tests/ut/test_environment.py @@ -1,5 +1,6 @@ import os import mock +import docker import unittest import subprocess @@ -56,96 +57,62 @@ def test_environment_general_methods_happy_flow(self, mocked_check_output): stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES ) - @mock.patch('docker_test_tools.environment.EnvironmentController.validate_service_name', mock.MagicMock()) - @mock.patch("subprocess.check_output") - def test_container_methods_happy_flow(self, mocked_check_output): + def test_container_methods_happy_flow(self): """Validate environment controller specific methods behave as expected.""" - service_name = 'test' + test_id = '111111' + service_name = 'service1' - self.controller.kill_container(service_name) - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'kill', service_name], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch.object(docker.APIClient, 'containers') as mock_containers: + mock_containers.return_value = [{'Labels': {'com.docker.compose.project': self.project_name}, + 'Id': 'container-id'}] + self.controller.get_container_id(service_name) + mock_containers.assert_called_with(filters={'label': 'com.docker.compose.service=service1'}) - self.controller.restart_container(service_name) - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'restart', service_name], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch('docker_test_tools.environment.EnvironmentController.get_container_id', + mock.MagicMock(return_value=test_id)): + with mock.patch.object(docker.APIClient, 'kill') as mock_kill: + self.controller.kill_container(service_name) + mock_kill.assert_called_with(test_id) - mocked_check_output.return_value = "DOCKER_SERVICE" - self.controller.docker_client.containers = mock.MagicMock(return_value=[ - { - 'Labels': {'com.docker.compose.project': self.project_name}, - 'Id': 'container-id' - } - ]) - self.controller.get_container_id(service_name) - self.controller.docker_client.containers.assert_called_with(filters={'label': 'com.docker.compose.service=test'}) - - self.controller.pause_container(service_name) - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'pause', service_name], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch.object(docker.APIClient, 'restart') as mock_restart: + self.controller.restart_container(service_name) + mock_restart.assert_called_with(test_id) - self.controller.unpause_container(service_name) - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'unpause', service_name], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch.object(docker.APIClient, 'pause') as mock_pause: + self.controller.pause_container(service_name) + mock_pause.assert_called_with(test_id) - self.controller.start_container(service_name) - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'start', service_name], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch.object(docker.APIClient, 'unpause') as mock_unpause: + self.controller.unpause_container(service_name) + mock_unpause.assert_called_with(test_id) - self.controller.stop_container(service_name) - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'stop', service_name], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch.object(docker.APIClient, 'start') as mock_start: + self.controller.start_container(service_name) + mock_start.assert_called_with(test_id) - mock_get_id = mock.MagicMock(return_value='test-id') - with mock.patch("docker_test_tools.environment.EnvironmentController.get_container_id", mock_get_id): - self.controller.docker_client.inspect_container = mock.MagicMock(return_value={ - "State": { - "Health": { - "Status": "healthy" - } - } - }) - self.assertTrue(self.controller.is_container_ready('test')) - self.controller.docker_client.inspect_container.assert_called_with('test-id') + with mock.patch.object(docker.APIClient, 'stop') as mock_stop: + self.controller.stop_container(service_name) + mock_stop.assert_called_with(test_id) - self.controller.docker_client.inspect_container = mock.MagicMock(return_value={ - "State": { - "Health": { - "Status": "unhealthy" - } - } - }) - self.assertFalse(self.controller.is_container_ready('test')) - self.controller.docker_client.inspect_container.assert_called_with('test-id') + with mock.patch.object(docker.APIClient, 'inspect_container') as mock_inspect: + self.controller.inspect_container(service_name) + mock_inspect.assert_called_with(test_id) - self.controller.docker_client.inspect_container = mock.MagicMock(return_value={ - "State": { - "Status": "running" - } - }) - self.assertTrue(self.controller.is_container_ready('test')) - self.controller.docker_client.inspect_container.assert_called_with('test-id') + with mock.patch.object(docker.APIClient, 'inspect_container', + return_value={"State": {"Health": {"Status": "healthy"}}}): + self.assertTrue(self.controller.is_container_ready('test')) - self.controller.docker_client.inspect_container = mock.MagicMock(return_value={ - "State": { - "Status": "not-running" - } - }) + with mock.patch.object(docker.APIClient, 'inspect_container', + return_value={"State": {"Health": {"Status": "unhealthy"}}}): + self.assertFalse(self.controller.is_container_ready('test')) - self.assertFalse(self.controller.is_container_ready('test')) - self.controller.docker_client.inspect_container.assert_called_with('test-id') + with mock.patch.object(docker.APIClient, 'inspect_container', + return_value={"State": {"Status": "running"}}): + self.assertTrue(self.controller.is_container_ready('test')) + + with mock.patch.object(docker.APIClient, 'inspect_container', + return_value={"State": {"Status": "not-running"}}): + self.assertFalse(self.controller.is_container_ready('test')) @mock.patch('subprocess.check_output', mock.MagicMock(side_effect=subprocess.CalledProcessError(1, '', ''))) @mock.patch('docker_test_tools.environment.EnvironmentController.validate_service_name', mock.MagicMock()) @@ -209,65 +176,50 @@ def test_container_methods_bad_service_name(self): with self.assertRaises(ValueError): self.assertTrue(self.controller.is_container_ready(service_name)) - @mock.patch('docker_test_tools.environment.EnvironmentController.validate_service_name', mock.MagicMock()) @mock.patch("docker_test_tools.environment.EnvironmentController.is_container_ready") - @mock.patch("subprocess.check_output") - def test_container_down(self, mocked_check_output, mock_is_ready): + def test_container_down(self, mock_is_ready): """Validate container_down context manager - without health check.""" - controller = self.get_controller() - + test_id = '222222' mock_is_ready.return_value = True - with controller.container_down('service1'): - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'kill', 'service1'], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) - mock_is_ready.assert_called_with('service1') - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'restart', 'service1'], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch("docker_test_tools.environment.EnvironmentController.get_container_id", return_value=test_id): + with mock.patch.object(docker.APIClient, 'kill') as mock_kill: + with mock.patch.object(docker.APIClient, 'restart')as mock_restart: + with self.controller.container_down('service1'): + mock_kill.assert_called_with(test_id) - @mock.patch('docker_test_tools.environment.EnvironmentController.validate_service_name', mock.MagicMock()) - @mock.patch("docker_test_tools.environment.EnvironmentController.is_container_ready") - @mock.patch("subprocess.check_output") - def test_container_paused(self, mocked_check_output, mock_is_ready): - """Validate container_down context manager - without health check.""" - controller = self.get_controller() + mock_is_ready.assert_called_with('service1') + mock_restart.assert_called_with(test_id) + @mock.patch("docker_test_tools.environment.EnvironmentController.is_container_ready") + def test_container_paused(self, mock_is_ready): + """Validate container_paused context manager - without health check.""" + test_id = '333333' mock_is_ready.return_value = True - with controller.container_paused('service1'): - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'pause', 'service1'], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) - mock_is_ready.assert_called_with('service1') - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'unpause', 'service1'], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch("docker_test_tools.environment.EnvironmentController.get_container_id", return_value=test_id): + with mock.patch.object(docker.APIClient, 'pause') as mock_pause: + with mock.patch.object(docker.APIClient, 'unpause')as mock_unpause: + with self.controller.container_paused('service1'): + mock_pause.assert_called_with(test_id) - @mock.patch('docker_test_tools.environment.EnvironmentController.validate_service_name', mock.MagicMock()) - @mock.patch("docker_test_tools.environment.EnvironmentController.is_container_ready") - @mock.patch("subprocess.check_output") - def test_container_stopped(self, mocked_check_output, mock_is_ready): - """Validate container_down context manager - without health check.""" - controller = self.get_controller() + mock_is_ready.assert_called_with('service1') + mock_unpause.assert_called_with(test_id) + @mock.patch("docker_test_tools.environment.EnvironmentController.is_container_ready") + def test_container_stopped(self, mock_is_ready): + """Validate container_stopped context manager - without health check.""" + test_id = '333333' mock_is_ready.return_value = True - with controller.container_stopped('service1'): - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'stop', 'service1'], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) - mock_is_ready.assert_called_with('service1') - mocked_check_output.assert_called_with( - ['docker-compose', '-f', self.compose_path, '-p', self.project_name, 'start', 'service1'], - stderr=subprocess.STDOUT, env=self.ENVIRONMENT_VARIABLES - ) + with mock.patch("docker_test_tools.environment.EnvironmentController.get_container_id", return_value=test_id): + with mock.patch.object(docker.APIClient, 'stop') as mock_stop: + with mock.patch.object(docker.APIClient, 'start')as mock_start: + with self.controller.container_stopped('service1'): + mock_stop.assert_called_with(test_id) + + mock_is_ready.assert_called_with('service1') + mock_start.assert_called_with(test_id) @mock.patch('docker_test_tools.environment.EnvironmentController.kill_containers') @mock.patch('docker_test_tools.environment.EnvironmentController.remove_containers')