From 84a1822e407959bc4c75d4ed3b65c0444e8db885 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 27 Nov 2015 13:19:27 -0500 Subject: [PATCH 1/3] Reduce complexity of _get_container_create_options Signed-off-by: Daniel Nephin --- compose/service.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/compose/service.py b/compose/service.py index 01f17a12..dd3e8828 100644 --- a/compose/service.py +++ b/compose/service.py @@ -566,8 +566,7 @@ class Service(object): elif not container_options.get('name'): container_options['name'] = self.get_container_name(number, one_off) - if 'detach' not in container_options: - container_options['detach'] = True + container_options.setdefault('detach', True) # If a qualified hostname was given, split it into an # unqualified hostname and a domainname unless domainname @@ -581,16 +580,9 @@ class Service(object): container_options['domainname'] = parts[2] if 'ports' in container_options or 'expose' in self.options: - ports = [] - all_ports = container_options.get('ports', []) + self.options.get('expose', []) - for port_range in all_ports: - internal_range, _ = split_port(port_range) - for port in internal_range: - port = str(port) - if '/' in port: - port = tuple(port.split('/')) - ports.append(port) - container_options['ports'] = ports + container_options['ports'] = build_container_ports( + container_options, + self.options) container_options['environment'] = merge_environment( self.options.get('environment'), @@ -1031,3 +1023,18 @@ def format_environment(environment): return key return '{key}={value}'.format(key=key, value=value) return [format_env(*item) for item in environment.items()] + +# Ports + + +def build_container_ports(container_options, options): + ports = [] + all_ports = container_options.get('ports', []) + options.get('expose', []) + for port_range in all_ports: + internal_range, _ = split_port(port_range) + for port in internal_range: + port = str(port) + if '/' in port: + port = tuple(port.split('/')) + ports.append(port) + return ports From cdda616d6b21cd99bb2283009bfb066ee0b68767 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 27 Nov 2015 13:26:15 -0500 Subject: [PATCH 2/3] Reduce complexity of sort_service_dicts. Signed-off-by: Daniel Nephin --- compose/config/sort_services.py | 35 ++++++++++++++++++--------------- tox.ini | 2 +- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/compose/config/sort_services.py b/compose/config/sort_services.py index 9d29f329..20ac4461 100644 --- a/compose/config/sort_services.py +++ b/compose/config/sort_services.py @@ -23,28 +23,31 @@ def get_source_name_from_network_mode(network_mode, source_type): return net_name +def get_service_names(links): + return [link.split(':')[0] for link in links] + + +def get_service_names_from_volumes_from(volumes_from): + return [volume_from.source for volume_from in volumes_from] + + +def get_service_dependents(service_dict, services): + name = service_dict['name'] + return [ + service for service in services + if (name in get_service_names(service.get('links', [])) or + name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or + name == get_service_name_from_network_mode(service.get('network_mode')) or + name in service.get('depends_on', [])) + ] + + def sort_service_dicts(services): # Topological sort (Cormen/Tarjan algorithm). unmarked = services[:] temporary_marked = set() sorted_services = [] - def get_service_names(links): - return [link.split(':')[0] for link in links] - - def get_service_names_from_volumes_from(volumes_from): - return [volume_from.source for volume_from in volumes_from] - - def get_service_dependents(service_dict, services): - name = service_dict['name'] - return [ - service for service in services - if (name in get_service_names(service.get('links', [])) or - name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or - name == get_service_name_from_network_mode(service.get('network_mode')) or - name in service.get('depends_on', [])) - ] - def visit(n): if n['name'] in temporary_marked: if n['name'] in get_service_names(n.get('links', [])): diff --git a/tox.ini b/tox.ini index a18bfda7..7984775d 100644 --- a/tox.ini +++ b/tox.ini @@ -45,7 +45,7 @@ directory = coverage-html # Allow really long lines for now max-line-length = 140 # Set this high for now -max-complexity = 12 +max-complexity = 11 exclude = compose/packages [pytest] From 43ecf8793af1b1979c400634b31207684aa0ce8d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 19 Jan 2016 16:06:32 -0500 Subject: [PATCH 3/3] Address old TODO, and small refactor of container name logic in service. Signed-off-by: Daniel Nephin --- compose/service.py | 19 ++++++++++--------- tests/integration/service_test.py | 4 ++-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/compose/service.py b/compose/service.py index dd3e8828..2fbea8d1 100644 --- a/compose/service.py +++ b/compose/service.py @@ -162,11 +162,11 @@ class Service(object): - starts containers until there are at least `desired_num` running - removes all stopped containers """ - if self.custom_container_name() and desired_num > 1: + if self.custom_container_name and desired_num > 1: log.warn('The "%s" service is using the custom container name "%s". ' 'Docker requires each container to have a unique name. ' 'Remove the custom name to scale the service.' - % (self.name, self.custom_container_name())) + % (self.name, self.custom_container_name)) if self.specifies_host_port(): log.warn('The "%s" service specifies a port on the host. If multiple containers ' @@ -496,10 +496,6 @@ class Service(object): def get_volumes_from_names(self): return [s.source.name for s in self.volumes_from if isinstance(s.source, Service)] - def get_container_name(self, number, one_off=False): - # TODO: Implement issue #652 here - return build_container_name(self.project, self.name, number, one_off) - # TODO: this would benefit from github.com/docker/docker/pull/14699 # to remove the need to inspect every container def _next_container_number(self, one_off=False): @@ -561,9 +557,7 @@ class Service(object): for k in DOCKER_CONFIG_KEYS if k in self.options) container_options.update(override_options) - if self.custom_container_name() and not one_off: - container_options['name'] = self.custom_container_name() - elif not container_options.get('name'): + if not container_options.get('name'): container_options['name'] = self.get_container_name(number, one_off) container_options.setdefault('detach', True) @@ -706,9 +700,16 @@ class Service(object): '{0}={1}'.format(LABEL_ONE_OFF, "True" if one_off else "False") ] + @property def custom_container_name(self): return self.options.get('container_name') + def get_container_name(self, number, one_off=False): + if self.custom_container_name and not one_off: + return self.custom_container_name + + return build_container_name(self.project, self.name, number, one_off) + def remove_image(self, image_type): if not image_type or image_type == ImageType.none: return False diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 968c0947..35696ea3 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -782,7 +782,7 @@ class ServiceTest(DockerClientTestCase): results in warning output. """ service = self.create_service('app', container_name='custom-container') - self.assertEqual(service.custom_container_name(), 'custom-container') + self.assertEqual(service.custom_container_name, 'custom-container') service.scale(3) @@ -963,7 +963,7 @@ class ServiceTest(DockerClientTestCase): def test_custom_container_name(self): service = self.create_service('web', container_name='my-web-container') - self.assertEqual(service.custom_container_name(), 'my-web-container') + self.assertEqual(service.custom_container_name, 'my-web-container') container = create_and_start_container(service) self.assertEqual(container.name, 'my-web-container')