From 7ff8c2b224768c89f4a45e359338ae48084a9145 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 27 Aug 2015 17:42:26 -0400 Subject: [PATCH] Resolves #1804 Fix mutation of service.options when a label or environment variable is specified in the config. Signed-off-by: Daniel Nephin --- compose/config.py | 2 +- compose/service.py | 20 +++++++++----------- tests/unit/service_test.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/compose/config.py b/compose/config.py index 75391e9b8..6bb0fea6a 100644 --- a/compose/config.py +++ b/compose/config.py @@ -382,7 +382,7 @@ def parse_environment(environment): return dict(split_env(e) for e in environment) if isinstance(environment, dict): - return environment + return dict(environment) raise ConfigurationError( "environment \"%s\" must be a list or mapping," % diff --git a/compose/service.py b/compose/service.py index 1433260be..fb859ab7a 100644 --- a/compose/service.py +++ b/compose/service.py @@ -554,7 +554,6 @@ class Service(object): number, one_off=False, previous_container=None): - add_config_hash = (not one_off and not override_options) container_options = dict( @@ -567,13 +566,6 @@ class Service(object): else: container_options['name'] = self.get_container_name(number, one_off) - if add_config_hash: - config_hash = self.config_hash() - if 'labels' not in container_options: - container_options['labels'] = {} - container_options['labels'][LABEL_CONFIG_HASH] = config_hash - log.debug("Added config hash: %s" % config_hash) - if 'detach' not in container_options: container_options['detach'] = True @@ -621,7 +613,8 @@ class Service(object): container_options['labels'] = build_container_labels( container_options.get('labels', {}), self.labels(one_off=one_off), - number) + number, + self.config_hash if add_config_hash else None) # Delete options which are only used when starting for key in DOCKER_START_KEYS: @@ -943,11 +936,16 @@ def split_port(port): # Labels -def build_container_labels(label_options, service_labels, number, one_off=False): - labels = label_options or {} +def build_container_labels(label_options, service_labels, number, config_hash): + labels = dict(label_options or {}) labels.update(label.split('=', 1) for label in service_labels) labels[LABEL_CONTAINER_NUMBER] = str(number) labels[LABEL_VERSION] = __version__ + + if config_hash: + log.debug("Added config hash: %s" % config_hash) + labels[LABEL_CONFIG_HASH] = config_hash + return labels diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 6ed3d981a..f407d73e9 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -274,6 +274,40 @@ class ServiceTest(unittest.TestCase): } ) + def test_get_container_create_options_does_not_mutate_options(self): + labels = {'thing': 'real'} + environment = {'also': 'real'} + service = Service( + 'foo', + image='foo', + labels=dict(labels), + client=self.mock_client, + environment=dict(environment), + ) + self.mock_client.inspect_image.return_value = {'Id': 'abcd'} + prev_container = mock.Mock( + id='ababab', + image_config={'ContainerConfig': {}}) + + opts = service._get_container_create_options( + {}, + 1, + previous_container=prev_container) + + self.assertEqual(service.options['labels'], labels) + self.assertEqual(service.options['environment'], environment) + + self.assertEqual( + opts['labels'][LABEL_CONFIG_HASH], + 'b30306d0a73b67f67a45b99b88d36c359e470e6fa0c04dda1cf62d2087205b81') + self.assertEqual( + opts['environment'], + { + 'affinity:container': '=ababab', + 'also': 'real', + } + ) + def test_get_container_not_found(self): self.mock_client.containers.return_value = [] service = Service('foo', client=self.mock_client, image='foo')