From 42c890796bca50bd8317b094dbfab1be0c2043b3 Mon Sep 17 00:00:00 2001 From: Mazz Mosley Date: Tue, 18 Aug 2015 16:43:19 +0100 Subject: [PATCH 1/3] Use docker.client.create_host_config create_host_config from docker.utils will be deprecated so that the new create_host_config has access to the _version so we can ensure that network_mode only gets set to 'default' by default if the version is high enough and won't explode. Signed-off-by: Mazz Mosley --- compose/service.py | 3 +-- tests/integration/service_test.py | 7 +++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/compose/service.py b/compose/service.py index 7e035e29..6d3df1f7 100644 --- a/compose/service.py +++ b/compose/service.py @@ -11,7 +11,6 @@ from operator import attrgetter import enum import six from docker.errors import APIError -from docker.utils import create_host_config from docker.utils import LogConfig from docker.utils.ports import build_port_bindings from docker.utils.ports import split_port @@ -678,7 +677,7 @@ class Service(object): devices = options.get('devices', None) - return create_host_config( + return self.client.create_host_config( links=self._get_links(link_to_self=one_off), port_bindings=port_bindings, binds=options.get('binds'), diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 17fd0aaf..040098c9 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -813,6 +813,13 @@ class ServiceTest(DockerClientTestCase): for k, v in {'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', 'NO_DEF': ''}.items(): self.assertEqual(env[k], v) + def test_with_high_enough_api_version_we_get_default_network_mode(self): + # TODO: remove this test once minimum docker version is 1.8.x + with mock.patch.object(self.client, '_version', '1.20'): + service = self.create_service('web') + service_config = service._get_container_host_config({}) + self.assertEquals(service_config['NetworkMode'], 'default') + def test_labels(self): labels_dict = { 'com.example.description': "Accounting webapp", From 6f6c04b5c938a7ee510d63bb36912a2e7513cb71 Mon Sep 17 00:00:00 2001 From: Mazz Mosley Date: Wed, 16 Sep 2015 12:02:58 +0100 Subject: [PATCH 2/3] Test what we are sending, not what we get This is a unit test and we are mocking the client. The method to get the create_config_host now lives on the client, so we mock that too. So we can test to the boundary that the method is called with the arguments we expect. Signed-off-by: Mazz Mosley --- tests/unit/service_test.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index de973339..7ba630fb 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -2,7 +2,6 @@ from __future__ import absolute_import from __future__ import unicode_literals import docker -from docker.utils import LogConfig from .. import mock from .. import unittest @@ -108,19 +107,33 @@ class ServiceTest(unittest.TestCase): self.assertFalse('domainname' in opts, 'domainname') def test_memory_swap_limit(self): + self.mock_client.create_host_config.return_value = {} + service = Service(name='foo', image='foo', hostname='name', client=self.mock_client, mem_limit=1000000000, memswap_limit=2000000000) - opts = service._get_container_create_options({'some': 'overrides'}, 1) - self.assertEqual(opts['host_config']['MemorySwap'], 2000000000) - self.assertEqual(opts['host_config']['Memory'], 1000000000) + service._get_container_create_options({'some': 'overrides'}, 1) + + self.assertTrue(self.mock_client.create_host_config.called) + self.assertEqual( + self.mock_client.create_host_config.call_args[1]['mem_limit'], + 1000000000 + ) + self.assertEqual( + self.mock_client.create_host_config.call_args[1]['memswap_limit'], + 2000000000 + ) def test_log_opt(self): + self.mock_client.create_host_config.return_value = {} + log_opt = {'syslog-address': 'tcp://192.168.0.42:123'} service = Service(name='foo', image='foo', hostname='name', client=self.mock_client, log_driver='syslog', log_opt=log_opt) - opts = service._get_container_create_options({'some': 'overrides'}, 1) + service._get_container_create_options({'some': 'overrides'}, 1) - self.assertIsInstance(opts['host_config']['LogConfig'], LogConfig) - self.assertEqual(opts['host_config']['LogConfig'].type, 'syslog') - self.assertEqual(opts['host_config']['LogConfig'].config, log_opt) + self.assertTrue(self.mock_client.create_host_config.called) + self.assertEqual( + self.mock_client.create_host_config.call_args[1]['log_config'], + {'Type': 'syslog', 'Config': {'syslog-address': 'tcp://192.168.0.42:123'}} + ) def test_split_domainname_fqdn(self): service = Service( From 39ba2c5a7cb5a4f7cec1e5a28bd43dc95492b22d Mon Sep 17 00:00:00 2001 From: Mazz Mosley Date: Thu, 17 Sep 2015 15:33:58 +0100 Subject: [PATCH 3/3] Fix leaky tests It was mocking self.client but relying on the call to utils.create_host_config which was not mocked. So now that function has moved to also be on self.client we need to redefine the test boundary, up to where we would call docker-py, not the result of docker-py. Signed-off-by: Mazz Mosley --- tests/unit/cli_test.py | 13 +++++++++---- tests/unit/service_test.py | 10 +++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/unit/cli_test.py b/tests/unit/cli_test.py index 1fd9f529..d12f4195 100644 --- a/tests/unit/cli_test.py +++ b/tests/unit/cli_test.py @@ -144,8 +144,11 @@ class CLITestCase(unittest.TestCase): '--rm': None, '--name': None, }) - _, _, call_kwargs = mock_client.create_container.mock_calls[0] - self.assertEquals(call_kwargs['host_config']['RestartPolicy']['Name'], 'always') + + self.assertEquals( + mock_client.create_host_config.call_args[1]['restart_policy']['Name'], + 'always' + ) command = TopLevelCommand() mock_client = mock.create_autospec(docker.Client) @@ -170,8 +173,10 @@ class CLITestCase(unittest.TestCase): '--rm': True, '--name': None, }) - _, _, call_kwargs = mock_client.create_container.mock_calls[0] - self.assertFalse('RestartPolicy' in call_kwargs['host_config']) + + self.assertFalse( + mock_client.create_host_config.call_args[1].get('restart_policy') + ) def test_command_manula_and_service_ports_together(self): command = TopLevelCommand() diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 7ba630fb..5f7ae948 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -543,13 +543,13 @@ class ServiceVolumesTest(unittest.TestCase): } } - create_options = service._get_container_create_options( + service._get_container_create_options( override_options={}, number=1, ) self.assertEqual( - set(create_options['host_config']['Binds']), + set(self.mock_client.create_host_config.call_args[1]['binds']), set([ '/host/path:/data1:rw', '/host/path:/data2:rw', @@ -581,14 +581,14 @@ class ServiceVolumesTest(unittest.TestCase): }, } - create_options = service._get_container_create_options( + service._get_container_create_options( override_options={}, number=1, previous_container=Container(self.mock_client, {'Id': '123123123'}), ) self.assertEqual( - create_options['host_config']['Binds'], + self.mock_client.create_host_config.call_args[1]['binds'], ['/mnt/sda1/host/path:/data:rw'], ) @@ -613,4 +613,4 @@ class ServiceVolumesTest(unittest.TestCase): ).create_container() self.assertEqual(len(create_calls), 1) - self.assertEqual(create_calls[0][1]['host_config']['Binds'], volumes) + self.assertEqual(self.mock_client.create_host_config.call_args[1]['binds'], volumes)