Fix a bug in ExtendsResolver where the service name of the extended service was wrong.

This bug can be seen by the change to the test case. When the extended service
uses a different name, the error was reported incorrectly.

By fixing this bug we can simplify self.signature and self.detect_cycles to
always use self.service_name.

Signed-off-by: Daniel Nephin <dnephin@docker.com>
This commit is contained in:
Daniel Nephin 2015-11-09 14:24:44 -05:00
parent a92d86308f
commit 5e97b806d5
4 changed files with 25 additions and 22 deletions

View File

@ -247,11 +247,17 @@ class ServiceExtendsResolver(object):
self.service_name = service_name self.service_name = service_name
self.service_dict['name'] = service_name self.service_dict['name'] = service_name
def detect_cycle(self, name): @property
if self.signature(name) in self.already_seen: def signature(self):
raise CircularReference(self.already_seen + [self.signature(name)]) return self.filename, self.service_name
def detect_cycle(self):
if self.signature in self.already_seen:
raise CircularReference(self.already_seen + [self.signature])
def run(self): def run(self):
self.detect_cycle()
service_dict = dict(self.service_dict) service_dict = dict(self.service_dict)
env = resolve_environment(self.working_dir, self.service_dict) env = resolve_environment(self.working_dir, self.service_dict)
if env: if env:
@ -284,12 +290,11 @@ class ServiceExtendsResolver(object):
resolver = ServiceExtendsResolver( resolver = ServiceExtendsResolver(
os.path.dirname(extended_config_path), os.path.dirname(extended_config_path),
extended_config_path, extended_config_path,
self.service_name, service_name,
service_config, service_config,
already_seen=self.already_seen + [self.signature(self.service_name)], already_seen=self.already_seen + [self.signature],
) )
resolver.detect_cycle(service_name)
other_service_dict = process_service(resolver.working_dir, resolver.run()) other_service_dict = process_service(resolver.working_dir, resolver.run())
validate_extended_service_dict( validate_extended_service_dict(
other_service_dict, other_service_dict,
@ -309,9 +314,6 @@ class ServiceExtendsResolver(object):
return expand_path(self.working_dir, extends_options['file']) return expand_path(self.working_dir, extends_options['file'])
return self.filename return self.filename
def signature(self, name):
return self.filename, name
def resolve_environment(working_dir, service_dict): def resolve_environment(working_dir, service_dict):
"""Unpack any environment variables from an env_file, if set. """Unpack any environment variables from an env_file, if set.

View File

@ -5,7 +5,7 @@ bar:
web: web:
extends: extends:
file: circle-2.yml file: circle-2.yml
service: web service: other
baz: baz:
image: busybox image: busybox
quux: quux:

View File

@ -2,7 +2,7 @@ foo:
image: busybox image: busybox
bar: bar:
image: busybox image: busybox
web: other:
extends: extends:
file: circle-1.yml file: circle-1.yml
service: web service: web

View File

@ -1080,18 +1080,19 @@ class ExtendsTest(unittest.TestCase):
])) ]))
def test_circular(self): def test_circular(self):
try: with pytest.raises(config.CircularReference) as exc:
load_from_filename('tests/fixtures/extends/circle-1.yml') load_from_filename('tests/fixtures/extends/circle-1.yml')
raise Exception("Expected config.CircularReference to be raised")
except config.CircularReference as e: path = [
self.assertEqual( (os.path.basename(filename), service_name)
[(os.path.basename(filename), service_name) for (filename, service_name) in e.trail], for (filename, service_name) in exc.value.trail
[ ]
('circle-1.yml', 'web'), expected = [
('circle-2.yml', 'web'), ('circle-1.yml', 'web'),
('circle-1.yml', 'web'), ('circle-2.yml', 'other'),
], ('circle-1.yml', 'web'),
) ]
self.assertEqual(path, expected)
def test_extends_validation_empty_dictionary(self): def test_extends_validation_empty_dictionary(self):
with self.assertRaisesRegexp(ConfigurationError, 'service'): with self.assertRaisesRegexp(ConfigurationError, 'service'):