From 533f33271a9be73546673aa9aacd823ca8ea9c38 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 17 Nov 2015 13:35:28 -0500 Subject: [PATCH] Move service sorting to config package. Signed-off-by: Daniel Nephin --- compose/config/__init__.py | 1 - compose/config/config.py | 17 ++---- compose/config/errors.py | 4 ++ compose/config/sort_services.py | 55 +++++++++++++++++++ compose/project.py | 51 +---------------- tests/integration/testcases.py | 1 + tests/unit/config/config_test.py | 23 +++++++- .../sort_services_test.py} | 6 +- tests/unit/project_test.py | 23 -------- tests/unit/service_test.py | 2 - 10 files changed, 91 insertions(+), 92 deletions(-) create mode 100644 compose/config/sort_services.py rename tests/unit/{sort_service_test.py => config/sort_services_test.py} (98%) diff --git a/compose/config/__init__.py b/compose/config/__init__.py index ec607e087..6fe9ff9fb 100644 --- a/compose/config/__init__.py +++ b/compose/config/__init__.py @@ -2,7 +2,6 @@ from .config import ConfigurationError from .config import DOCKER_CONFIG_KEYS from .config import find -from .config import get_service_name_from_net from .config import load from .config import merge_environment from .config import parse_environment diff --git a/compose/config/config.py b/compose/config/config.py index 5b1de5efc..9d438ca12 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -14,6 +14,8 @@ from .errors import CircularReference from .errors import ComposeFileNotFound from .errors import ConfigurationError from .interpolation import interpolate_environment_variables +from .sort_services import get_service_name_from_net +from .sort_services import sort_service_dicts from .types import parse_extra_hosts from .types import parse_restart_spec from .types import VolumeFromSpec @@ -214,10 +216,10 @@ def load(config_details): return service_dict def build_services(config_file): - return [ + return sort_service_dicts([ build_service(config_file.filename, name, service_dict) for name, service_dict in config_file.config.items() - ] + ]) def merge_services(base, override): all_service_names = set(base) | set(override) @@ -638,17 +640,6 @@ def to_list(value): return value -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 load_yaml(filename): try: with open(filename, 'r') as fh: diff --git a/compose/config/errors.py b/compose/config/errors.py index 037b7ec84..6d6a69df9 100644 --- a/compose/config/errors.py +++ b/compose/config/errors.py @@ -6,6 +6,10 @@ class ConfigurationError(Exception): return self.msg +class DependencyError(ConfigurationError): + pass + + class CircularReference(ConfigurationError): def __init__(self, trail): self.trail = trail diff --git a/compose/config/sort_services.py b/compose/config/sort_services.py new file mode 100644 index 000000000..5d9adab11 --- /dev/null +++ b/compose/config/sort_services.py @@ -0,0 +1,55 @@ +from compose.config.errors import DependencyError + + +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[:] + 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_net(service.get('net'))) + ] + + def visit(n): + if n['name'] in temporary_marked: + if n['name'] in get_service_names(n.get('links', [])): + raise DependencyError('A service can not link to itself: %s' % n['name']) + if n['name'] in n.get('volumes_from', []): + raise DependencyError('A service can not mount itself as volume: %s' % n['name']) + else: + raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked)) + if n in unmarked: + temporary_marked.add(n['name']) + for m in get_service_dependents(n, services): + visit(m) + temporary_marked.remove(n['name']) + unmarked.remove(n) + sorted_services.insert(0, n) + + while unmarked: + visit(unmarked[-1]) + + return sorted_services diff --git a/compose/project.py b/compose/project.py index 5caa1ea37..30e81693c 100644 --- a/compose/project.py +++ b/compose/project.py @@ -9,7 +9,7 @@ from docker.errors import NotFound from . import parallel from .config import ConfigurationError -from .config import get_service_name_from_net +from .config.sort_services import get_service_name_from_net from .const import DEFAULT_TIMEOUT from .const import LABEL_ONE_OFF from .const import LABEL_PROJECT @@ -26,49 +26,6 @@ from .service import ServiceNet log = logging.getLogger(__name__) -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_net(service.get('net'))) - ] - - def visit(n): - if n['name'] in temporary_marked: - if n['name'] in get_service_names(n.get('links', [])): - raise DependencyError('A service can not link to itself: %s' % n['name']) - if n['name'] in n.get('volumes_from', []): - raise DependencyError('A service can not mount itself as volume: %s' % n['name']) - else: - raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked)) - if n in unmarked: - temporary_marked.add(n['name']) - for m in get_service_dependents(n, services): - visit(m) - temporary_marked.remove(n['name']) - unmarked.remove(n) - sorted_services.insert(0, n) - - while unmarked: - visit(unmarked[-1]) - - return sorted_services - - class Project(object): """ A collection of services. @@ -96,7 +53,7 @@ class Project(object): if use_networking: remove_links(service_dicts) - for service_dict in sort_service_dicts(service_dicts): + for service_dict in service_dicts: links = project.get_links(service_dict) volumes_from = project.get_volumes_from(service_dict) net = project.get_net(service_dict) @@ -404,7 +361,3 @@ class NoSuchService(Exception): def __str__(self): return self.msg - - -class DependencyError(ConfigurationError): - pass diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index f5de50ee1..a2218d6bc 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -8,6 +8,7 @@ from pytest import skip from .. import unittest from compose.cli.docker_client import docker_client from compose.config.config import resolve_environment +from compose.config.config import ServiceConfig from compose.const import LABEL_PROJECT from compose.progress_stream import stream_output from compose.service import Service diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index b2a4cd68f..a5eeb64f9 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -77,7 +77,7 @@ class ConfigTest(unittest.TestCase): ) ) - def test_config_invalid_service_names(self): + def test_load_config_invalid_service_names(self): for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']: with pytest.raises(ConfigurationError) as exc: config.load(build_config_details( @@ -232,6 +232,27 @@ class ConfigTest(unittest.TestCase): assert "service 'bogus' doesn't have any configuration" in exc.exconly() assert "In file 'override.yaml'" in exc.exconly() + def test_load_sorts_in_dependency_order(self): + config_details = build_config_details({ + 'web': { + 'image': 'busybox:latest', + 'links': ['db'], + }, + 'db': { + 'image': 'busybox:latest', + 'volumes_from': ['volume:ro'] + }, + 'volume': { + 'image': 'busybox:latest', + 'volumes': ['/tmp'], + } + }) + services = config.load(config_details) + + assert services[0]['name'] == 'volume' + assert services[1]['name'] == 'db' + assert services[2]['name'] == 'web' + def test_config_valid_service_names(self): for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']: services = config.load( diff --git a/tests/unit/sort_service_test.py b/tests/unit/config/sort_services_test.py similarity index 98% rename from tests/unit/sort_service_test.py rename to tests/unit/config/sort_services_test.py index ef0882877..8d0c3ae40 100644 --- a/tests/unit/sort_service_test.py +++ b/tests/unit/config/sort_services_test.py @@ -1,7 +1,7 @@ -from .. import unittest +from compose.config.errors import DependencyError +from compose.config.sort_services import sort_service_dicts from compose.config.types import VolumeFromSpec -from compose.project import DependencyError -from compose.project import sort_service_dicts +from tests import unittest class SortServiceTest(unittest.TestCase): diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index f8178ed8b..f4c6f8ca1 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -34,29 +34,6 @@ class ProjectTest(unittest.TestCase): self.assertEqual(project.get_service('db').name, 'db') self.assertEqual(project.get_service('db').options['image'], 'busybox:latest') - def test_from_dict_sorts_in_dependency_order(self): - project = Project.from_dicts('composetest', [ - { - 'name': 'web', - 'image': 'busybox:latest', - 'links': ['db'], - }, - { - 'name': 'db', - 'image': 'busybox:latest', - 'volumes_from': [VolumeFromSpec('volume', 'ro')] - }, - { - 'name': 'volume', - 'image': 'busybox:latest', - 'volumes': ['/tmp'], - } - ], None) - - self.assertEqual(project.services[0].name, 'volume') - self.assertEqual(project.services[1].name, 'db') - self.assertEqual(project.services[2].name, 'web') - def test_from_config(self): dicts = [ { diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index a439f0da9..e87ce5920 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -23,8 +23,6 @@ from compose.service import NoSuchImageError from compose.service import parse_repository_tag from compose.service import Service from compose.service import ServiceNet -from compose.service import VolumeFromSpec -from compose.service import VolumeSpec from compose.service import warn_on_masked_volume