Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compose/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,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," %
Expand Down
20 changes: 9 additions & 11 deletions compose/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,6 @@ def _get_container_create_options(
number,
one_off=False,
previous_container=None):

add_config_hash = (not one_off and not override_options)

container_options = dict(
Expand All @@ -589,13 +588,6 @@ def _get_container_create_options(
elif not container_options.get('name'):
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

Expand Down Expand Up @@ -643,7 +635,8 @@ def _get_container_create_options(
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:
Expand Down Expand Up @@ -899,11 +892,16 @@ def parse_volume_spec(volume_config):
# 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


Expand Down
2 changes: 1 addition & 1 deletion tests/integration/service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ def test_scale_with_custom_container_name_outputs_warning(self, mock_log):
Test that calling scale on a service that has a custom container name
results in warning output.
"""
service = self.create_service('web', container_name='custom-container')
service = self.create_service('app', container_name='custom-container')
self.assertEqual(service.custom_container_name(), 'custom-container')

service.scale(3)
Expand Down
1 change: 0 additions & 1 deletion tests/integration/state_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ def test_trigger_start(self):

self.assertEqual([c.is_running for c in containers], [False, True])

web = self.create_service('web', **options)
self.assertEqual(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found it while debugging. It's a no-op since line 193 does the same thing.

('start', containers[0:1]),
web.convergence_plan(),
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/testcases.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ def create_service(self, name, **kwargs):

options = ServiceLoader(working_dir='.').make_service_dict(name, kwargs)

labels = options.setdefault('labels', {})
labels['com.docker.compose.test-name'] = self.id()

return Service(
project='composetest',
client=self.client,
Expand Down
35 changes: 35 additions & 0 deletions tests/unit/service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from .. import mock
from .. import unittest
from compose.const import LABEL_CONFIG_HASH
from compose.const import LABEL_ONE_OFF
from compose.const import LABEL_PROJECT
from compose.const import LABEL_SERVICE
Expand Down Expand Up @@ -163,6 +164,40 @@ def test_get_container_create_options_with_name_option(self):
one_off=True)
self.assertEqual(opts['name'], name)

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')
Expand Down