diff --git a/compose/config/config.py b/compose/config/config.py index db2f67ea166..918946b3965 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -305,7 +305,8 @@ def merge_services(base, override): return { name: merge_service_dicts_from_files( base.get(name, {}), - override.get(name, {})) + override.get(name, {}), + version) for name in all_service_names } @@ -397,7 +398,10 @@ def resolve_extends(self, extended_config_path, service_dict, service_name): service_name, ) - return merge_service_dicts(other_service_dict, self.service_config.config) + return merge_service_dicts( + other_service_dict, + self.service_config.config, + self.version) def get_extended_config_path(self, extends_options): """Service we are extending either has a value for 'file' set, which we @@ -521,12 +525,12 @@ def normalize_v1_service_format(service_dict): return service_dict -def merge_service_dicts_from_files(base, override): +def merge_service_dicts_from_files(base, override, version): """When merging services from multiple files we need to merge the `extends` field. This is not handled by `merge_service_dicts()` which is used to perform the `extends`. """ - new_service = merge_service_dicts(base, override) + new_service = merge_service_dicts(base, override, version) if 'extends' in override: new_service['extends'] = override['extends'] elif 'extends' in base: @@ -534,7 +538,7 @@ def merge_service_dicts_from_files(base, override): return new_service -def merge_service_dicts(base, override): +def merge_service_dicts(base, override, version): d = {} def merge_field(field, merge_func, default=None): @@ -545,7 +549,6 @@ def merge_field(field, merge_func, default=None): merge_field('environment', merge_environment) merge_field('labels', merge_labels) - merge_image_or_build(base, override, d) for field in ['volumes', 'devices']: merge_field(field, merge_path_mappings) @@ -556,15 +559,19 @@ def merge_field(field, merge_func, default=None): for field in ['dns', 'dns_search', 'env_file']: merge_field(field, merge_list_or_string) - already_merged_keys = set(d) | {'image', 'build'} - for field in set(ALLOWED_KEYS) - already_merged_keys: + for field in set(ALLOWED_KEYS) - set(d): if field in base or field in override: d[field] = override.get(field, base.get(field)) + if version == 1: + legacy_v1_merge_image_or_build(d, base, override) + return d -def merge_image_or_build(base, override, output): +def legacy_v1_merge_image_or_build(output, base, override): + output.pop('image', None) + output.pop('build', None) if 'image' in override: output['image'] = override['image'] elif 'build' in override: diff --git a/compose/config/service_schema_v2.json b/compose/config/service_schema_v2.json index a64b3bdc0c1..d1d0854f3ee 100644 --- a/compose/config/service_schema_v2.json +++ b/compose/config/service_schema_v2.json @@ -167,17 +167,8 @@ "constraints": { "id": "#/definitions/constraints", "anyOf": [ - { - "required": ["build"], - "not": {"required": ["image"]} - }, - { - "required": ["image"], - "not": {"anyOf": [ - {"required": ["build"]}, - {"required": ["dockerfile"]} - ]} - } + {"required": ["build"]}, + {"required": ["image"]} ] } } diff --git a/compose/config/validation.py b/compose/config/validation.py index fea9a22b88a..e7006d5a4be 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -151,6 +151,7 @@ def handle_error_for_schema_with_id(error, service_name): VALID_NAME_CHARS) if schema_id == '#/definitions/constraints': + # TODO: only applies to v1 if 'image' in error.instance and 'build' in error.instance: return ( "Service '{}' has both an image and build path specified. " @@ -159,7 +160,8 @@ def handle_error_for_schema_with_id(error, service_name): if 'image' not in error.instance and 'build' not in error.instance: return ( "Service '{}' has neither an image nor a build path " - "specified. Exactly one must be provided.".format(service_name)) + "specified. At least one must be provided.".format(service_name)) + # TODO: only applies to v1 if 'image' in error.instance and 'dockerfile' in error.instance: return ( "Service '{}' has both an image and alternate Dockerfile. " diff --git a/compose/project.py b/compose/project.py index 50f991be7fa..d41b1f52a84 100644 --- a/compose/project.py +++ b/compose/project.py @@ -344,8 +344,8 @@ def _get_convergence_plans(self, services, strategy): updated_dependencies = [ name for name in service.get_dependency_names() - if name in plans - and plans[name].action in ('recreate', 'create') + if name in plans and + plans[name].action in ('recreate', 'create') ] if updated_dependencies and strategy.allows_recreate: diff --git a/compose/service.py b/compose/service.py index 693e1980b8b..d5c36f1ad68 100644 --- a/compose/service.py +++ b/compose/service.py @@ -275,10 +275,7 @@ def image(self): @property def image_name(self): - if self.can_be_built(): - return self.full_name - else: - return self.options['image'] + return self.options.get('image', '{s.project}_{s.name}'.format(s=self)) def convergence_plan(self, strategy=ConvergenceStrategy.changed): containers = self.containers(stopped=True) @@ -535,9 +532,9 @@ def _get_container_create_options( # unqualified hostname and a domainname unless domainname # was also given explicitly. This matches the behavior of # the official Docker CLI in that scenario. - if ('hostname' in container_options - and 'domainname' not in container_options - and '.' in container_options['hostname']): + if ('hostname' in container_options and + 'domainname' not in container_options and + '.' in container_options['hostname']): parts = container_options['hostname'].partition('.') container_options['hostname'] = parts[0] container_options['domainname'] = parts[2] @@ -665,13 +662,6 @@ def build(self, no_cache=False, pull=False, force_rm=False): def can_be_built(self): return 'build' in self.options - @property - def full_name(self): - """ - The tag to give to images built for this service. - """ - return '%s_%s' % (self.project, self.name) - def labels(self, one_off=False): return [ '{0}={1}'.format(LABEL_PROJECT, self.project), diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 86bc4d9dbb1..539be5a9b3a 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -491,7 +491,7 @@ def test_build(self): f.write("FROM busybox\n") self.create_service('web', build=base_dir).build() - self.assertEqual(len(self.client.images(name='composetest_web')), 1) + assert self.client.inspect_image('composetest_web') def test_build_non_ascii_filename(self): base_dir = tempfile.mkdtemp() @@ -504,7 +504,19 @@ def test_build_non_ascii_filename(self): f.write("hello world\n") self.create_service('web', build=text_type(base_dir)).build() - self.assertEqual(len(self.client.images(name='composetest_web')), 1) + assert self.client.inspect_image('composetest_web') + + def test_build_with_image_name(self): + base_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, base_dir) + + with open(os.path.join(base_dir, 'Dockerfile'), 'w') as f: + f.write("FROM busybox\n") + + image_name = 'examples/composetest:latest' + self.addCleanup(self.client.remove_image, image_name) + self.create_service('web', build=base_dir, image=image_name).build() + assert self.client.inspect_image(image_name) def test_build_with_git_url(self): build_url = "https://github.com/dnephin/docker-build-from-url.git" diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 1c2876f5a14..a59d1d34ac6 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -19,6 +19,9 @@ from tests import mock from tests import unittest +DEFAULT_VERSION = 2 +V1 = 1 + def make_service_dict(name, service_dict, working_dir, filename=None): """ @@ -238,8 +241,7 @@ def test_config_integer_service_name_raise_validation_error_v2(self): ) ) - @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash') - def test_load_with_multiple_files(self): + def test_load_with_multiple_files_v1(self): base_file = config.ConfigFile( 'base.yaml', { @@ -265,7 +267,7 @@ def test_load_with_multiple_files(self): expected = [ { 'name': 'web', - 'build': '/', + 'build': os.path.abspath('/'), 'links': ['db'], 'volumes': [VolumeSpec.parse('/home/user/project:/code')], }, @@ -274,7 +276,7 @@ def test_load_with_multiple_files(self): 'image': 'example/db', }, ] - self.assertEqual(service_sort(service_dicts), service_sort(expected)) + assert service_sort(service_dicts) == service_sort(expected) def test_load_with_multiple_files_and_empty_override(self): base_file = config.ConfigFile( @@ -428,6 +430,7 @@ def test_load_with_multiple_files_v2(self): { 'name': 'web', 'build': os.path.abspath('/'), + 'image': 'example/web', 'links': ['db'], 'volumes': [VolumeSpec.parse('/home/user/project:/code')], }, @@ -436,7 +439,7 @@ def test_load_with_multiple_files_v2(self): 'image': 'example/db', }, ] - self.assertEqual(service_sort(service_dicts), service_sort(expected)) + assert service_sort(service_dicts) == service_sort(expected) def test_config_valid_service_names(self): for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']: @@ -525,16 +528,15 @@ def test_invalid_list_of_strings_format(self): ) ) - def test_config_image_and_dockerfile_raise_validation_error(self): - expected_error_msg = "Service 'web' has both an image and alternate Dockerfile." - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): - config.load( - build_config_details( - {'web': {'image': 'busybox', 'dockerfile': 'Dockerfile.alt'}}, - 'working_dir', - 'filename.yml' - ) - ) + def test_load_config_dockerfile_without_build_raises_error(self): + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details({ + 'web': { + 'image': 'busybox', + 'dockerfile': 'Dockerfile.alt' + } + })) + assert "Service 'web' has both an image and alternate Dockerfile." in exc.exconly() def test_config_extra_hosts_string_raises_validation_error(self): expected_error_msg = "Service 'web' configuration key 'extra_hosts' contains an invalid type" @@ -746,7 +748,10 @@ def test_merge_service_dicts_from_files_with_extends_in_base(self): override = { 'image': 'alpine:edge', } - actual = config.merge_service_dicts_from_files(base, override) + actual = config.merge_service_dicts_from_files( + base, + override, + DEFAULT_VERSION) assert actual == { 'image': 'alpine:edge', 'volumes': ['.:/app'], @@ -762,7 +767,10 @@ def test_merge_service_dicts_from_files_with_extends_in_override(self): 'image': 'alpine:edge', 'extends': {'service': 'foo'} } - actual = config.merge_service_dicts_from_files(base, override) + actual = config.merge_service_dicts_from_files( + base, + override, + DEFAULT_VERSION) assert actual == { 'image': 'alpine:edge', 'volumes': ['.:/app'], @@ -1023,43 +1031,43 @@ def config_name(self): return "" def test_empty(self): - service_dict = config.merge_service_dicts({}, {}) - self.assertNotIn(self.config_name(), service_dict) + service_dict = config.merge_service_dicts({}, {}, DEFAULT_VERSION) + assert self.config_name() not in service_dict def test_no_override(self): service_dict = config.merge_service_dicts( {self.config_name(): ['/foo:/code', '/data']}, {}, - ) - self.assertEqual(set(service_dict[self.config_name()]), set(['/foo:/code', '/data'])) + DEFAULT_VERSION) + assert set(service_dict[self.config_name()]) == set(['/foo:/code', '/data']) def test_no_base(self): service_dict = config.merge_service_dicts( {}, {self.config_name(): ['/bar:/code']}, - ) - self.assertEqual(set(service_dict[self.config_name()]), set(['/bar:/code'])) + DEFAULT_VERSION) + assert set(service_dict[self.config_name()]) == set(['/bar:/code']) def test_override_explicit_path(self): service_dict = config.merge_service_dicts( {self.config_name(): ['/foo:/code', '/data']}, {self.config_name(): ['/bar:/code']}, - ) - self.assertEqual(set(service_dict[self.config_name()]), set(['/bar:/code', '/data'])) + DEFAULT_VERSION) + assert set(service_dict[self.config_name()]) == set(['/bar:/code', '/data']) def test_add_explicit_path(self): service_dict = config.merge_service_dicts( {self.config_name(): ['/foo:/code', '/data']}, {self.config_name(): ['/bar:/code', '/quux:/data']}, - ) - self.assertEqual(set(service_dict[self.config_name()]), set(['/bar:/code', '/quux:/data'])) + DEFAULT_VERSION) + assert set(service_dict[self.config_name()]) == set(['/bar:/code', '/quux:/data']) def test_remove_explicit_path(self): service_dict = config.merge_service_dicts( {self.config_name(): ['/foo:/code', '/quux:/data']}, {self.config_name(): ['/bar:/code', '/data']}, - ) - self.assertEqual(set(service_dict[self.config_name()]), set(['/bar:/code', '/data'])) + DEFAULT_VERSION) + assert set(service_dict[self.config_name()]) == set(['/bar:/code', '/data']) class MergeVolumesTest(unittest.TestCase, MergePathMappingTest): @@ -1075,63 +1083,62 @@ def config_name(self): class BuildOrImageMergeTest(unittest.TestCase): def test_merge_build_or_image_no_override(self): self.assertEqual( - config.merge_service_dicts({'build': '.'}, {}), + config.merge_service_dicts({'build': '.'}, {}, V1), {'build': '.'}, ) self.assertEqual( - config.merge_service_dicts({'image': 'redis'}, {}), + config.merge_service_dicts({'image': 'redis'}, {}, V1), {'image': 'redis'}, ) def test_merge_build_or_image_override_with_same(self): self.assertEqual( - config.merge_service_dicts({'build': '.'}, {'build': './web'}), + config.merge_service_dicts({'build': '.'}, {'build': './web'}, V1), {'build': './web'}, ) self.assertEqual( - config.merge_service_dicts({'image': 'redis'}, {'image': 'postgres'}), + config.merge_service_dicts({'image': 'redis'}, {'image': 'postgres'}, V1), {'image': 'postgres'}, ) def test_merge_build_or_image_override_with_other(self): self.assertEqual( - config.merge_service_dicts({'build': '.'}, {'image': 'redis'}), - {'image': 'redis'} + config.merge_service_dicts({'build': '.'}, {'image': 'redis'}, V1), + {'image': 'redis'}, ) self.assertEqual( - config.merge_service_dicts({'image': 'redis'}, {'build': '.'}), - {'build': '.'} + config.merge_service_dicts({'image': 'redis'}, {'build': '.'}, V1), + {'build': '.'}, ) class MergeListsTest(unittest.TestCase): def test_empty(self): - service_dict = config.merge_service_dicts({}, {}) - self.assertNotIn('ports', service_dict) + assert 'ports' not in config.merge_service_dicts({}, {}, DEFAULT_VERSION) def test_no_override(self): service_dict = config.merge_service_dicts( {'ports': ['10:8000', '9000']}, {}, - ) - self.assertEqual(set(service_dict['ports']), set(['10:8000', '9000'])) + DEFAULT_VERSION) + assert set(service_dict['ports']) == set(['10:8000', '9000']) def test_no_base(self): service_dict = config.merge_service_dicts( {}, {'ports': ['10:8000', '9000']}, - ) - self.assertEqual(set(service_dict['ports']), set(['10:8000', '9000'])) + DEFAULT_VERSION) + assert set(service_dict['ports']) == set(['10:8000', '9000']) def test_add_item(self): service_dict = config.merge_service_dicts( {'ports': ['10:8000', '9000']}, {'ports': ['20:8000']}, - ) - self.assertEqual(set(service_dict['ports']), set(['10:8000', '9000', '20:8000'])) + DEFAULT_VERSION) + assert set(service_dict['ports']) == set(['10:8000', '9000', '20:8000']) class MergeStringsOrListsTest(unittest.TestCase): @@ -1139,70 +1146,69 @@ def test_no_override(self): service_dict = config.merge_service_dicts( {'dns': '8.8.8.8'}, {}, - ) - self.assertEqual(set(service_dict['dns']), set(['8.8.8.8'])) + DEFAULT_VERSION) + assert set(service_dict['dns']) == set(['8.8.8.8']) def test_no_base(self): service_dict = config.merge_service_dicts( {}, {'dns': '8.8.8.8'}, - ) - self.assertEqual(set(service_dict['dns']), set(['8.8.8.8'])) + DEFAULT_VERSION) + assert set(service_dict['dns']) == set(['8.8.8.8']) def test_add_string(self): service_dict = config.merge_service_dicts( {'dns': ['8.8.8.8']}, {'dns': '9.9.9.9'}, - ) - self.assertEqual(set(service_dict['dns']), set(['8.8.8.8', '9.9.9.9'])) + DEFAULT_VERSION) + assert set(service_dict['dns']) == set(['8.8.8.8', '9.9.9.9']) def test_add_list(self): service_dict = config.merge_service_dicts( {'dns': '8.8.8.8'}, {'dns': ['9.9.9.9']}, - ) - self.assertEqual(set(service_dict['dns']), set(['8.8.8.8', '9.9.9.9'])) + DEFAULT_VERSION) + assert set(service_dict['dns']) == set(['8.8.8.8', '9.9.9.9']) class MergeLabelsTest(unittest.TestCase): def test_empty(self): - service_dict = config.merge_service_dicts({}, {}) - self.assertNotIn('labels', service_dict) + assert 'labels' not in config.merge_service_dicts({}, {}, DEFAULT_VERSION) def test_no_override(self): service_dict = config.merge_service_dicts( make_service_dict('foo', {'build': '.', 'labels': ['foo=1', 'bar']}, 'tests/'), make_service_dict('foo', {'build': '.'}, 'tests/'), - ) - self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': ''}) + DEFAULT_VERSION) + assert service_dict['labels'] == {'foo': '1', 'bar': ''} def test_no_base(self): service_dict = config.merge_service_dicts( make_service_dict('foo', {'build': '.'}, 'tests/'), make_service_dict('foo', {'build': '.', 'labels': ['foo=2']}, 'tests/'), - ) - self.assertEqual(service_dict['labels'], {'foo': '2'}) + DEFAULT_VERSION) + assert service_dict['labels'] == {'foo': '2'} def test_override_explicit_value(self): service_dict = config.merge_service_dicts( make_service_dict('foo', {'build': '.', 'labels': ['foo=1', 'bar']}, 'tests/'), make_service_dict('foo', {'build': '.', 'labels': ['foo=2']}, 'tests/'), - ) - self.assertEqual(service_dict['labels'], {'foo': '2', 'bar': ''}) + DEFAULT_VERSION) + assert service_dict['labels'] == {'foo': '2', 'bar': ''} def test_add_explicit_value(self): service_dict = config.merge_service_dicts( make_service_dict('foo', {'build': '.', 'labels': ['foo=1', 'bar']}, 'tests/'), make_service_dict('foo', {'build': '.', 'labels': ['bar=2']}, 'tests/'), - ) - self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': '2'}) + DEFAULT_VERSION) + assert service_dict['labels'] == {'foo': '1', 'bar': '2'} def test_remove_explicit_value(self): service_dict = config.merge_service_dicts( make_service_dict('foo', {'build': '.', 'labels': ['foo=1', 'bar=2']}, 'tests/'), make_service_dict('foo', {'build': '.', 'labels': ['bar']}, 'tests/'), - ) - self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': ''}) + DEFAULT_VERSION) + assert service_dict['labels'] == {'foo': '1', 'bar': ''} class MemoryOptionsTest(unittest.TestCase): @@ -1541,10 +1547,12 @@ def test_extends_validation_valid_config(self): self.assertEquals(service[0]['command'], "/bin/true") def test_extended_service_with_invalid_config(self): - expected_error_msg = "Service 'myweb' has neither an image nor a build path specified" - - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): + with pytest.raises(ConfigurationError) as exc: load_from_filename('tests/fixtures/extends/service-with-invalid-schema.yml') + assert ( + "Service 'myweb' has neither an image nor a build path specified" in + exc.exconly() + ) def test_extended_service_with_valid_config(self): service = load_from_filename('tests/fixtures/extends/service-with-valid-composite-extends.yml') diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index d7080faef35..63cf658ed93 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -492,6 +492,15 @@ def test_get_links_with_networking(self): use_networking=True) self.assertEqual(service._get_links(link_to_self=True), []) + def test_image_name_from_config(self): + image_name = 'example/web:latest' + service = Service('foo', image=image_name) + assert service.image_name == image_name + + def test_image_name_default(self): + service = Service('foo', project='testing') + assert service.image_name == 'testing_foo' + def sort_by_name(dictionary_list): return sorted(dictionary_list, key=lambda k: k['name'])