From 0bce467782c83b9065a9efaadc1405ba8862116a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 19 Jan 2016 15:41:45 -0500 Subject: [PATCH 1/3] Implement depends_on to define an order for services in the v2 format. Signed-off-by: Daniel Nephin --- compose/config/config.py | 15 ++++++++++--- compose/config/service_schema_v2.json | 1 + compose/config/sort_services.py | 9 +++++--- compose/config/validation.py | 8 +++++++ compose/service.py | 3 ++- tests/unit/config/config_test.py | 28 ++++++++++++++++++++++++- tests/unit/config/sort_services_test.py | 14 +++++++++++++ 7 files changed, 70 insertions(+), 8 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index ac5e8d174..6bba53b8c 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -27,6 +27,7 @@ from .types import VolumeFromSpec from .types import VolumeSpec from .validation import validate_against_fields_schema from .validation import validate_against_service_schema +from .validation import validate_depends_on from .validation import validate_extends_file_path from .validation import validate_top_level_object from .validation import validate_top_level_service_objects @@ -312,9 +313,10 @@ def load_services(working_dir, config_file, service_configs): resolver = ServiceExtendsResolver(service_config, config_file) service_dict = process_service(resolver.run()) - validate_service(service_dict, service_config.name, config_file.version) + service_config = service_config._replace(config=service_dict) + validate_service(service_config, service_names, config_file.version) service_dict = finalize_service( - service_config._replace(config=service_dict), + service_config, service_names, config_file.version) return service_dict @@ -481,6 +483,10 @@ def validate_extended_service_dict(service_dict, filename, service): raise ConfigurationError( "%s services with 'net: container' cannot be extended" % error_prefix) + if 'depends_on' in service_dict: + raise ConfigurationError( + "%s services with 'depends_on' cannot be extended" % error_prefix) + def validate_ulimits(ulimit_config): for limit_name, soft_hard_values in six.iteritems(ulimit_config): @@ -491,13 +497,16 @@ def validate_ulimits(ulimit_config): "than 'hard' value".format(ulimit_config)) -def validate_service(service_dict, service_name, version): +def validate_service(service_config, service_names, version): + service_dict, service_name = service_config.config, service_config.name validate_against_service_schema(service_dict, service_name, version) validate_paths(service_dict) if 'ulimits' in service_dict: validate_ulimits(service_dict['ulimits']) + validate_depends_on(service_config, service_names) + if not service_dict.get('image') and has_uppercase(service_name): raise ConfigurationError( "Service '{name}' contains uppercase characters which are not valid " diff --git a/compose/config/service_schema_v2.json b/compose/config/service_schema_v2.json index 8623507a3..d4ec575a6 100644 --- a/compose/config/service_schema_v2.json +++ b/compose/config/service_schema_v2.json @@ -42,6 +42,7 @@ "cpu_shares": {"type": ["number", "string"]}, "cpu_quota": {"type": ["number", "string"]}, "cpuset": {"type": "string"}, + "depends_on": {"$ref": "#/definitions/list_of_strings"}, "devices": {"type": "array", "items": {"type": "string"}, "uniqueItems": true}, "dns": {"$ref": "#/definitions/string_or_list"}, "dns_search": {"$ref": "#/definitions/string_or_list"}, diff --git a/compose/config/sort_services.py b/compose/config/sort_services.py index 055521227..ac0fa4585 100644 --- a/compose/config/sort_services.py +++ b/compose/config/sort_services.py @@ -33,7 +33,8 @@ def sort_service_dicts(services): 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'))) + name == get_service_name_from_net(service.get('net')) or + name in service.get('depends_on', [])) ] def visit(n): @@ -42,8 +43,10 @@ def sort_service_dicts(services): 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['name'] in n.get('depends_on', []): + raise DependencyError('A service can not depend on itself: %s' % n['name']) + raise DependencyError('Circular dependency between %s' % ' and '.join(temporary_marked)) + if n in unmarked: temporary_marked.add(n['name']) for m in get_service_dependents(n, services): diff --git a/compose/config/validation.py b/compose/config/validation.py index 639e8bed2..6cce11056 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -123,6 +123,14 @@ def validate_extends_file_path(service_name, extends_options, filename): ) +def validate_depends_on(service_config, service_names): + for dependency in service_config.config.get('depends_on', []): + if dependency not in service_names: + raise ConfigurationError( + "Service '{s.name}' depends on service '{dep}' which is " + "undefined.".format(s=service_config, dep=dependency)) + + def get_unsupported_config_msg(service_name, error_key): msg = "Unsupported config option for '{}' service: '{}'".format(service_name, error_key) if error_key in DOCKER_CONFIG_HINTS: diff --git a/compose/service.py b/compose/service.py index f72863c9a..1dfda06a6 100644 --- a/compose/service.py +++ b/compose/service.py @@ -471,7 +471,8 @@ class Service(object): net_name = self.net.service_name return (self.get_linked_service_names() + self.get_volumes_from_names() + - ([net_name] if net_name else [])) + ([net_name] if net_name else []) + + self.options.get('depends_on', [])) def get_linked_service_names(self): return [service.name for (service, _) in self.links] diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index cc2051363..0416d5b76 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -894,9 +894,35 @@ class ConfigTest(unittest.TestCase): 'ext': {'external': True, 'driver': 'foo'} } }) - with self.assertRaises(ConfigurationError): + with pytest.raises(ConfigurationError): config.load(config_details) + def test_depends_on_orders_services(self): + config_details = build_config_details({ + 'version': 2, + 'services': { + 'one': {'image': 'busybox', 'depends_on': ['three', 'two']}, + 'two': {'image': 'busybox', 'depends_on': ['three']}, + 'three': {'image': 'busybox'}, + }, + }) + actual = config.load(config_details) + assert ( + [service['name'] for service in actual.services] == + ['three', 'two', 'one'] + ) + + def test_depends_on_unknown_service_errors(self): + config_details = build_config_details({ + 'version': 2, + 'services': { + 'one': {'image': 'busybox', 'depends_on': ['three']}, + }, + }) + with pytest.raises(ConfigurationError) as exc: + config.load(config_details) + assert "Service 'one' depends on service 'three'" in exc.exconly() + class PortsTest(unittest.TestCase): INVALID_PORTS_TYPES = [ diff --git a/tests/unit/config/sort_services_test.py b/tests/unit/config/sort_services_test.py index ebe444fee..3279ece49 100644 --- a/tests/unit/config/sort_services_test.py +++ b/tests/unit/config/sort_services_test.py @@ -1,6 +1,8 @@ from __future__ import absolute_import from __future__ import unicode_literals +import pytest + from compose.config.errors import DependencyError from compose.config.sort_services import sort_service_dicts from compose.config.types import VolumeFromSpec @@ -240,3 +242,15 @@ class SortServiceTest(unittest.TestCase): self.assertIn('web', e.msg) else: self.fail('Should have thrown an DependencyError') + + def test_sort_service_dicts_depends_on_self(self): + services = [ + { + 'depends_on': ['web'], + 'name': 'web' + }, + ] + + with pytest.raises(DependencyError) as exc: + sort_service_dicts(services) + assert 'A service can not depend on itself: web' in exc.exconly() From 146587643c32c076b93b1fa67cd531b5b2003227 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 19 Jan 2016 15:47:57 -0500 Subject: [PATCH 2/3] Move ulimits validation to validation.py and improve the error message. Signed-off-by: Daniel Nephin --- compose/config/config.py | 14 ++------------ compose/config/validation.py | 12 ++++++++++++ tests/unit/config/config_test.py | 2 +- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 6bba53b8c..72ad50af5 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -31,6 +31,7 @@ from .validation import validate_depends_on from .validation import validate_extends_file_path from .validation import validate_top_level_object from .validation import validate_top_level_service_objects +from .validation import validate_ulimits DOCKER_CONFIG_KEYS = [ @@ -488,23 +489,12 @@ def validate_extended_service_dict(service_dict, filename, service): "%s services with 'depends_on' cannot be extended" % error_prefix) -def validate_ulimits(ulimit_config): - for limit_name, soft_hard_values in six.iteritems(ulimit_config): - if isinstance(soft_hard_values, dict): - if not soft_hard_values['soft'] <= soft_hard_values['hard']: - raise ConfigurationError( - "ulimit_config \"{}\" cannot contain a 'soft' value higher " - "than 'hard' value".format(ulimit_config)) - - def validate_service(service_config, service_names, version): service_dict, service_name = service_config.config, service_config.name validate_against_service_schema(service_dict, service_name, version) validate_paths(service_dict) - if 'ulimits' in service_dict: - validate_ulimits(service_dict['ulimits']) - + validate_ulimits(service_config) validate_depends_on(service_config, service_names) if not service_dict.get('image') and has_uppercase(service_name): diff --git a/compose/config/validation.py b/compose/config/validation.py index 6cce11056..ecf8d4f92 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -110,6 +110,18 @@ def validate_top_level_object(config_file): type(config_file.config))) +def validate_ulimits(service_config): + ulimit_config = service_config.config.get('ulimits', {}) + for limit_name, soft_hard_values in six.iteritems(ulimit_config): + if isinstance(soft_hard_values, dict): + if not soft_hard_values['soft'] <= soft_hard_values['hard']: + raise ConfigurationError( + "Service '{s.name}' has invalid ulimit '{ulimit}'. " + "'soft' value can not be greater than 'hard' value ".format( + s=service_config, + ulimit=ulimit_config)) + + def validate_extends_file_path(service_name, extends_options, filename): """ The service to be extended must either be defined in the config key 'file', diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 0416d5b76..3c3c6326b 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -700,7 +700,7 @@ class ConfigTest(unittest.TestCase): assert "'hard' is a required property" in exc.exconly() def test_config_ulimits_soft_greater_than_hard_error(self): - expected = "cannot contain a 'soft' value higher than 'hard' value" + expected = "'soft' value can not be greater than 'hard' value" with pytest.raises(ConfigurationError) as exc: config.load(build_config_details( From 5aadf5a187b436ddb81772058dbedeaeea804d95 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 19 Jan 2016 15:52:17 -0500 Subject: [PATCH 3/3] Update tests in sort_services_test.py to use pytest. Signed-off-by: Daniel Nephin --- tests/unit/config/sort_services_test.py | 95 +++++++++++-------------- 1 file changed, 41 insertions(+), 54 deletions(-) diff --git a/tests/unit/config/sort_services_test.py b/tests/unit/config/sort_services_test.py index 3279ece49..f59906644 100644 --- a/tests/unit/config/sort_services_test.py +++ b/tests/unit/config/sort_services_test.py @@ -6,10 +6,9 @@ import pytest from compose.config.errors import DependencyError from compose.config.sort_services import sort_service_dicts from compose.config.types import VolumeFromSpec -from tests import unittest -class SortServiceTest(unittest.TestCase): +class TestSortService(object): def test_sort_service_dicts_1(self): services = [ { @@ -25,10 +24,10 @@ class SortServiceTest(unittest.TestCase): ] sorted_services = sort_service_dicts(services) - self.assertEqual(len(sorted_services), 3) - self.assertEqual(sorted_services[0]['name'], 'grunt') - self.assertEqual(sorted_services[1]['name'], 'redis') - self.assertEqual(sorted_services[2]['name'], 'web') + assert len(sorted_services) == 3 + assert sorted_services[0]['name'] == 'grunt' + assert sorted_services[1]['name'] == 'redis' + assert sorted_services[2]['name'] == 'web' def test_sort_service_dicts_2(self): services = [ @@ -46,10 +45,10 @@ class SortServiceTest(unittest.TestCase): ] sorted_services = sort_service_dicts(services) - self.assertEqual(len(sorted_services), 3) - self.assertEqual(sorted_services[0]['name'], 'redis') - self.assertEqual(sorted_services[1]['name'], 'postgres') - self.assertEqual(sorted_services[2]['name'], 'web') + assert len(sorted_services) == 3 + assert sorted_services[0]['name'] == 'redis' + assert sorted_services[1]['name'] == 'postgres' + assert sorted_services[2]['name'] == 'web' def test_sort_service_dicts_3(self): services = [ @@ -67,10 +66,10 @@ class SortServiceTest(unittest.TestCase): ] 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') + assert len(sorted_services) == 3 + assert sorted_services[0]['name'] == 'child' + assert sorted_services[1]['name'] == 'parent' + assert sorted_services[2]['name'] == 'grandparent' def test_sort_service_dicts_4(self): services = [ @@ -88,10 +87,10 @@ class SortServiceTest(unittest.TestCase): ] 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') + assert len(sorted_services) == 3 + assert sorted_services[0]['name'] == 'child' + assert sorted_services[1]['name'] == 'parent' + assert sorted_services[2]['name'] == 'grandparent' def test_sort_service_dicts_5(self): services = [ @@ -109,10 +108,10 @@ class SortServiceTest(unittest.TestCase): ] 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') + assert len(sorted_services) == 3 + assert sorted_services[0]['name'] == 'child' + assert sorted_services[1]['name'] == 'parent' + assert sorted_services[2]['name'] == 'grandparent' def test_sort_service_dicts_6(self): services = [ @@ -130,10 +129,10 @@ class SortServiceTest(unittest.TestCase): ] 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') + assert len(sorted_services) == 3 + assert sorted_services[0]['name'] == 'child' + assert sorted_services[1]['name'] == 'parent' + assert sorted_services[2]['name'] == 'grandparent' def test_sort_service_dicts_7(self): services = [ @@ -155,11 +154,11 @@ class SortServiceTest(unittest.TestCase): ] 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') + assert len(sorted_services) == 4 + assert sorted_services[0]['name'] == 'one' + assert sorted_services[1]['name'] == 'two' + assert sorted_services[2]['name'] == 'three' + assert sorted_services[3]['name'] == 'four' def test_sort_service_dicts_circular_imports(self): services = [ @@ -173,13 +172,10 @@ class SortServiceTest(unittest.TestCase): }, ] - try: + with pytest.raises(DependencyError) as exc: sort_service_dicts(services) - except DependencyError as e: - self.assertIn('redis', e.msg) - self.assertIn('web', e.msg) - else: - self.fail('Should have thrown an DependencyError') + assert 'redis' in exc.exconly() + assert 'web' in exc.exconly() def test_sort_service_dicts_circular_imports_2(self): services = [ @@ -196,13 +192,10 @@ class SortServiceTest(unittest.TestCase): } ] - try: + with pytest.raises(DependencyError) as exc: sort_service_dicts(services) - except DependencyError as e: - self.assertIn('redis', e.msg) - self.assertIn('web', e.msg) - else: - self.fail('Should have thrown an DependencyError') + assert 'redis' in exc.exconly() + assert 'web' in exc.exconly() def test_sort_service_dicts_circular_imports_3(self): services = [ @@ -220,13 +213,10 @@ class SortServiceTest(unittest.TestCase): } ] - try: + with pytest.raises(DependencyError) as exc: sort_service_dicts(services) - except DependencyError as e: - self.assertIn('a', e.msg) - self.assertIn('b', e.msg) - else: - self.fail('Should have thrown an DependencyError') + assert 'a' in exc.exconly() + assert 'b' in exc.exconly() def test_sort_service_dicts_self_imports(self): services = [ @@ -236,12 +226,9 @@ class SortServiceTest(unittest.TestCase): }, ] - try: + with pytest.raises(DependencyError) as exc: sort_service_dicts(services) - except DependencyError as e: - self.assertIn('web', e.msg) - else: - self.fail('Should have thrown an DependencyError') + assert 'web' in exc.exconly() def test_sort_service_dicts_depends_on_self(self): services = [