From 95f4e2c7c3214c184aae2717417f273ddc6ad2ca Mon Sep 17 00:00:00 2001 From: Gil Clark Date: Fri, 6 Mar 2015 13:30:56 -0800 Subject: [PATCH] Make volumes_from and net containers first class dependencies and assure that starting order is correct. Added supporting unit and integration tests as well. Signed-off-by: Gil Clark --- compose/cli/main.py | 6 +- compose/project.py | 88 +++++++++++++----- compose/service.py | 35 +++++++- tests/integration/project_test.py | 145 +++++++++++++++++++++++++++--- tests/unit/project_test.py | 107 +++++++++++++++++++++- tests/unit/sort_service_test.py | 89 ++++++++++++++++++ 6 files changed, 429 insertions(+), 41 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index eee59bb74..15c5e05f4 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -292,7 +292,7 @@ class TopLevelCommand(Command): if len(deps) > 0: project.up( service_names=deps, - start_links=True, + start_deps=True, recreate=False, insecure_registry=insecure_registry, detach=options['-d'] @@ -430,13 +430,13 @@ class TopLevelCommand(Command): monochrome = options['--no-color'] - start_links = not options['--no-deps'] + start_deps = not options['--no-deps'] recreate = not options['--no-recreate'] service_names = options['SERVICE'] project.up( service_names=service_names, - start_links=start_links, + start_deps=start_deps, recreate=recreate, insecure_registry=insecure_registry, detach=options['-d'], diff --git a/compose/project.py b/compose/project.py index f2fa6a7ee..794ef2b65 100644 --- a/compose/project.py +++ b/compose/project.py @@ -10,6 +10,17 @@ from docker.errors import APIError log = logging.getLogger(__name__) +def get_service_name_from_net(net_config): + if not net_config: + return + + if not net_config.startswith('container:'): + return + + _, net_name = net_config.split(':', 1) + return net_name + + def sort_service_dicts(services): # Topological sort (Cormen/Tarjan algorithm). unmarked = services[:] @@ -19,6 +30,15 @@ def sort_service_dicts(services): def get_service_names(links): return [link.split(':')[0] for link in links] + 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 service.get('volumes_from', []) or + name == get_service_name_from_net(service.get('net'))) + ] + def visit(n): if n['name'] in temporary_marked: if n['name'] in get_service_names(n.get('links', [])): @@ -29,8 +49,7 @@ def sort_service_dicts(services): raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked)) if n in unmarked: temporary_marked.add(n['name']) - dependents = [m for m in services if (n['name'] in get_service_names(m.get('links', []))) or (n['name'] in m.get('volumes_from', []))] - for m in dependents: + for m in get_service_dependents(n, services): visit(m) temporary_marked.remove(n['name']) unmarked.remove(n) @@ -60,8 +79,10 @@ class Project(object): for service_dict in sort_service_dicts(service_dicts): links = project.get_links(service_dict) volumes_from = project.get_volumes_from(service_dict) + net = project.get_net(service_dict) - project.services.append(Service(client=client, project=name, links=links, volumes_from=volumes_from, **service_dict)) + project.services.append(Service(client=client, project=name, links=links, net=net, + volumes_from=volumes_from, **service_dict)) return project @classmethod @@ -85,31 +106,31 @@ class Project(object): raise NoSuchService(name) - def get_services(self, service_names=None, include_links=False): + def get_services(self, service_names=None, include_deps=False): """ Returns a list of this project's services filtered by the provided list of names, or all services if service_names is None or []. - If include_links is specified, returns a list including the links for + If include_deps is specified, returns a list including the dependencies for service_names, in order of dependency. Preserves the original order of self.services where possible, - reordering as needed to resolve links. + reordering as needed to resolve dependencies. Raises NoSuchService if any of the named services do not exist. """ if service_names is None or len(service_names) == 0: return self.get_services( service_names=[s.name for s in self.services], - include_links=include_links + include_deps=include_deps ) else: unsorted = [self.get_service(name) for name in service_names] services = [s for s in self.services if s in unsorted] - if include_links: - services = reduce(self._inject_links, services, []) + if include_deps: + services = reduce(self._inject_deps, services, []) uniques = [] [uniques.append(s) for s in services if s not in uniques] @@ -146,6 +167,28 @@ class Project(object): del service_dict['volumes_from'] return volumes_from + def get_net(self, service_dict): + if 'net' in service_dict: + net_name = get_service_name_from_net(service_dict.get('net')) + + if net_name: + try: + net = self.get_service(net_name) + except NoSuchService: + try: + net = Container.from_id(self.client, net_name) + except APIError: + raise ConfigurationError('Serivce "%s" is trying to use the network of "%s", which is not the name of a service or container.' % (service_dict['name'], net_name)) + else: + net = service_dict['net'] + + del service_dict['net'] + + else: + net = 'bridge' + + return net + def start(self, service_names=None, **options): for service in self.get_services(service_names): service.start(**options) @@ -171,13 +214,13 @@ class Project(object): def up(self, service_names=None, - start_links=True, + start_deps=True, recreate=True, insecure_registry=False, detach=False, do_build=True): running_containers = [] - for service in self.get_services(service_names, include_links=start_links): + for service in self.get_services(service_names, include_deps=start_deps): if recreate: for (_, container) in service.recreate_containers( insecure_registry=insecure_registry, @@ -194,7 +237,7 @@ class Project(object): return running_containers def pull(self, service_names=None, insecure_registry=False): - for service in self.get_services(service_names, include_links=True): + for service in self.get_services(service_names, include_deps=True): service.pull(insecure_registry=insecure_registry) def remove_stopped(self, service_names=None, **options): @@ -207,19 +250,22 @@ class Project(object): for service in self.get_services(service_names) if service.has_container(container, one_off=one_off)] - def _inject_links(self, acc, service): - linked_names = service.get_linked_names() + def _inject_deps(self, acc, service): + net_name = service.get_net_name() + dep_names = (service.get_linked_names() + + service.get_volumes_from_names() + + ([net_name] if net_name else [])) - if len(linked_names) > 0: - linked_services = self.get_services( - service_names=linked_names, - include_links=True + if len(dep_names) > 0: + dep_services = self.get_services( + service_names=list(set(dep_names)), + include_deps=True ) else: - linked_services = [] + dep_services = [] - linked_services.append(service) - return acc + linked_services + dep_services.append(service) + return acc + dep_services class NoSuchService(Exception): diff --git a/compose/service.py b/compose/service.py index df6dd6ab5..377198cf4 100644 --- a/compose/service.py +++ b/compose/service.py @@ -88,7 +88,7 @@ ServiceName = namedtuple('ServiceName', 'project service number') class Service(object): - def __init__(self, name, client=None, project='default', links=None, external_links=None, volumes_from=None, **options): + def __init__(self, name, client=None, project='default', links=None, external_links=None, volumes_from=None, net=None, **options): if not re.match('^%s+$' % VALID_NAME_CHARS, name): raise ConfigError('Invalid service name "%s" - only %s are allowed' % (name, VALID_NAME_CHARS)) if not re.match('^%s+$' % VALID_NAME_CHARS, project): @@ -116,6 +116,7 @@ class Service(object): self.links = links or [] self.external_links = external_links or [] self.volumes_from = volumes_from or [] + self.net = net or None self.options = options def containers(self, stopped=False, one_off=False): @@ -320,7 +321,6 @@ class Service(object): if ':' in volume) privileged = options.get('privileged', False) - net = options.get('net', 'bridge') dns = options.get('dns', None) dns_search = options.get('dns_search', None) cap_add = options.get('cap_add', None) @@ -334,7 +334,7 @@ class Service(object): binds=volume_bindings, volumes_from=self._get_volumes_from(intermediate_container), privileged=privileged, - network_mode=net, + network_mode=self._get_net(), dns=dns, dns_search=dns_search, restart_policy=restart, @@ -364,6 +364,15 @@ class Service(object): def get_linked_names(self): return [s.name for (s, _) in self.links] + def get_volumes_from_names(self): + return [s.name for s in self.volumes_from if isinstance(s, Service)] + + def get_net_name(self): + if isinstance(self.net, Service): + return self.net.name + else: + return + def _next_container_name(self, all_containers, one_off=False): bits = [self.project, self.name] if one_off: @@ -399,7 +408,6 @@ class Service(object): for volume_source in self.volumes_from: if isinstance(volume_source, Service): containers = volume_source.containers(stopped=True) - if not containers: volumes_from.append(volume_source.create_container().id) else: @@ -413,6 +421,25 @@ class Service(object): return volumes_from + def _get_net(self): + if not self.net: + return "bridge" + + if isinstance(self.net, Service): + containers = self.net.containers() + if len(containers) > 0: + net = 'container:' + containers[0].id + else: + log.warning("Warning: Service %s is trying to use reuse the network stack " + "of another service that is not running." % (self.net.name)) + net = None + elif isinstance(self.net, Container): + net = 'container:' + self.net.id + else: + net = self.net + + return net + def _get_container_create_options(self, override_options, one_off=False): container_options = dict( (k, self.options[k]) diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 2577fd616..17b54daee 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -44,6 +44,63 @@ class ProjectTest(DockerClientTestCase): db = project.get_service('db') self.assertEqual(db.volumes_from, [data_container]) + project.kill() + project.remove_stopped() + + def test_net_from_service(self): + project = Project.from_config( + name='composetest', + config={ + 'net': { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"] + }, + 'web': { + 'image': 'busybox:latest', + 'net': 'container:net', + 'command': ["/bin/sleep", "300"] + }, + }, + client=self.client, + ) + + project.up() + + web = project.get_service('web') + net = project.get_service('net') + self.assertEqual(web._get_net(), 'container:'+net.containers()[0].id) + + project.kill() + project.remove_stopped() + + def test_net_from_container(self): + net_container = Container.create( + self.client, + image='busybox:latest', + name='composetest_net_container', + command='/bin/sleep 300' + ) + net_container.start() + + project = Project.from_config( + name='composetest', + config={ + 'web': { + 'image': 'busybox:latest', + 'net': 'container:composetest_net_container' + }, + }, + client=self.client, + ) + + project.up() + + web = project.get_service('web') + self.assertEqual(web._get_net(), 'container:'+net_container.id) + + project.kill() + project.remove_stopped() + def test_start_stop_kill_remove(self): web = self.create_service('web') db = self.create_service('db') @@ -199,20 +256,86 @@ class ProjectTest(DockerClientTestCase): project.kill() project.remove_stopped() - def test_project_up_with_no_deps(self): - console = self.create_service('console') - db = self.create_service('db', volumes=['/var/db']) - web = self.create_service('web', links=[(db, 'db')]) - - project = Project('composetest', [web, db, console], self.client) + def test_project_up_starts_depends(self): + project = Project.from_config( + name='composetest', + config={ + 'console': { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"], + }, + 'net' : { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"] + }, + 'app': { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"], + 'net': 'container:net' + }, + 'web': { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"], + 'net': 'container:net', + 'links': ['app'] + }, + }, + client=self.client, + ) project.start() self.assertEqual(len(project.containers()), 0) - project.up(['web'], start_links=False) - self.assertEqual(len(project.containers()), 1) - self.assertEqual(len(web.containers()), 1) - self.assertEqual(len(db.containers()), 0) - self.assertEqual(len(console.containers()), 0) + project.up(['web']) + self.assertEqual(len(project.containers()), 3) + self.assertEqual(len(project.get_service('web').containers()), 1) + self.assertEqual(len(project.get_service('app').containers()), 1) + self.assertEqual(len(project.get_service('net').containers()), 1) + self.assertEqual(len(project.get_service('console').containers()), 0) + + project.kill() + project.remove_stopped() + + def test_project_up_with_no_deps(self): + project = Project.from_config( + name='composetest', + config={ + 'console': { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"], + }, + 'net' : { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"] + }, + 'vol': { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"], + 'volumes': ["/tmp"] + }, + 'app': { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"], + 'net': 'container:net' + }, + 'web': { + 'image': 'busybox:latest', + 'command': ["/bin/sleep", "300"], + 'net': 'container:net', + 'links': ['app'], + 'volumes_from': ['vol'] + }, + }, + client=self.client, + ) + project.start() + self.assertEqual(len(project.containers()), 0) + + project.up(['web'], start_deps=False) + self.assertEqual(len(project.containers(stopped=True)), 2) + self.assertEqual(len(project.get_service('web').containers()), 1) + self.assertEqual(len(project.get_service('vol').containers(stopped=True)), 1) + self.assertEqual(len(project.get_service('net').containers()), 0) + self.assertEqual(len(project.get_service('console').containers()), 0) project.kill() project.remove_stopped() diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index d7aca64cf..b06e14e58 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -2,6 +2,10 @@ from __future__ import unicode_literals from .. import unittest from compose.service import Service from compose.project import Project, ConfigurationError +from compose.container import Container + +import mock +import docker class ProjectTest(unittest.TestCase): def test_from_dict(self): @@ -120,7 +124,7 @@ class ProjectTest(unittest.TestCase): ) project = Project('test', [web, db, cache, console], None) self.assertEqual( - project.get_services(['console'], include_links=True), + project.get_services(['console'], include_deps=True), [db, web, console] ) @@ -136,6 +140,105 @@ class ProjectTest(unittest.TestCase): ) project = Project('test', [web, db], None) self.assertEqual( - project.get_services(['web', 'db'], include_links=True), + project.get_services(['web', 'db'], include_deps=True), [db, web] ) + + def test_use_volumes_from_container(self): + container_id = 'aabbccddee' + container_dict = dict(Name='aaa', Id=container_id) + mock_client = mock.create_autospec(docker.Client) + mock_client.inspect_container.return_value = container_dict + project = Project.from_dicts('test', [ + { + 'name': 'test', + 'image': 'busybox:latest', + 'volumes_from': ['aaa'] + } + ], mock_client) + self.assertEqual(project.get_service('test')._get_volumes_from(), [container_id]) + + def test_use_volumes_from_service_no_container(self): + container_name = 'test_vol_1' + mock_client = mock.create_autospec(docker.Client) + mock_client.containers.return_value = [ + { + "Name": container_name, + "Names": [container_name], + "Id": container_name, + "Image": 'busybox:latest' + } + ] + project = Project.from_dicts('test', [ + { + 'name': 'vol', + 'image': 'busybox:latest' + }, + { + 'name': 'test', + 'image': 'busybox:latest', + 'volumes_from': ['vol'] + } + ], mock_client) + self.assertEqual(project.get_service('test')._get_volumes_from(), [container_name]) + + @mock.patch.object(Service, 'containers') + def test_use_volumes_from_service_container(self, mock_return): + container_ids = ['aabbccddee', '12345'] + mock_return.return_value = [ + mock.Mock(id=container_id, spec=Container) + for container_id in container_ids] + + project = Project.from_dicts('test', [ + { + 'name': 'vol', + 'image': 'busybox:latest' + }, + { + 'name': 'test', + 'image': 'busybox:latest', + 'volumes_from': ['vol'] + } + ], None) + self.assertEqual(project.get_service('test')._get_volumes_from(), container_ids) + + def test_use_net_from_container(self): + container_id = 'aabbccddee' + container_dict = dict(Name='aaa', Id=container_id) + mock_client = mock.create_autospec(docker.Client) + mock_client.inspect_container.return_value = container_dict + project = Project.from_dicts('test', [ + { + 'name': 'test', + 'image': 'busybox:latest', + 'net': 'container:aaa' + } + ], mock_client) + service = project.get_service('test') + self.assertEqual(service._get_net(), 'container:'+container_id) + + def test_use_net_from_service(self): + container_name = 'test_aaa_1' + mock_client = mock.create_autospec(docker.Client) + mock_client.containers.return_value = [ + { + "Name": container_name, + "Names": [container_name], + "Id": container_name, + "Image": 'busybox:latest' + } + ] + project = Project.from_dicts('test', [ + { + 'name': 'aaa', + 'image': 'busybox:latest' + }, + { + 'name': 'test', + 'image': 'busybox:latest', + 'net': 'container:aaa' + } + ], mock_client) + + service = project.get_service('test') + self.assertEqual(service._get_net(), 'container:'+container_name) diff --git a/tests/unit/sort_service_test.py b/tests/unit/sort_service_test.py index 420353c8a..f42a94748 100644 --- a/tests/unit/sort_service_test.py +++ b/tests/unit/sort_service_test.py @@ -65,6 +65,95 @@ class SortServiceTest(unittest.TestCase): self.assertEqual(sorted_services[1]['name'], 'parent') self.assertEqual(sorted_services[2]['name'], 'grandparent') + def test_sort_service_dicts_4(self): + services = [ + { + 'name': 'child' + }, + { + 'name': 'parent', + 'volumes_from': ['child'] + }, + { + 'links': ['parent'], + 'name': 'grandparent' + }, + ] + + sorted_services = sort_service_dicts(services) + self.assertEqual(len(sorted_services), 3) + self.assertEqual(sorted_services[0]['name'], 'child') + self.assertEqual(sorted_services[1]['name'], 'parent') + self.assertEqual(sorted_services[2]['name'], 'grandparent') + + def test_sort_service_dicts_5(self): + services = [ + { + 'links': ['parent'], + 'name': 'grandparent' + }, + { + 'name': 'parent', + 'net': 'container:child' + }, + { + 'name': 'child' + } + ] + + sorted_services = sort_service_dicts(services) + self.assertEqual(len(sorted_services), 3) + self.assertEqual(sorted_services[0]['name'], 'child') + self.assertEqual(sorted_services[1]['name'], 'parent') + self.assertEqual(sorted_services[2]['name'], 'grandparent') + + def test_sort_service_dicts_6(self): + services = [ + { + 'links': ['parent'], + 'name': 'grandparent' + }, + { + 'name': 'parent', + 'volumes_from': ['child'] + }, + { + 'name': 'child' + } + ] + + sorted_services = sort_service_dicts(services) + self.assertEqual(len(sorted_services), 3) + self.assertEqual(sorted_services[0]['name'], 'child') + self.assertEqual(sorted_services[1]['name'], 'parent') + self.assertEqual(sorted_services[2]['name'], 'grandparent') + + def test_sort_service_dicts_7(self): + services = [ + { + 'net': 'container:three', + 'name': 'four' + }, + { + 'links': ['two'], + 'name': 'three' + }, + { + 'name': 'two', + 'volumes_from': ['one'] + }, + { + 'name': 'one' + } + ] + + sorted_services = sort_service_dicts(services) + self.assertEqual(len(sorted_services), 4) + self.assertEqual(sorted_services[0]['name'], 'one') + self.assertEqual(sorted_services[1]['name'], 'two') + self.assertEqual(sorted_services[2]['name'], 'three') + self.assertEqual(sorted_services[3]['name'], 'four') + def test_sort_service_dicts_circular_imports(self): services = [ {