Skip to content

Commit 66d9597

Browse files
link_names are now checked to be unique
Both across docker_links and needed_services. needed_services multiple instances are allowed if they define the same NeededServiceSerializer (same service_name, same env, etc..). Goal: guarantee that each different docker container has a unique link name, while allowing multiple services linking to the same docker container (thus without ambiguity). Rules: - docker_links: no duplicate link_name - needed_services: can duplicate link_name iff NeededServices are equivalent (same service_name, same env_exports, but env_exports and other attributes can be different) - same link_name cannot be reused between docker_links and needed_services How: compare kind and object: - NeededServices has a specialized __eq__() testing the previously defined equivalence - DockerLink has the standard __eq__() testing object instance equality Unit tests: test various combinations of DockerLink and NeededService creations. Fix tests: - prefix all link_names with `test-`, like we did for service name - update test-resources accordingly TODO later: isolate by app name, but it's not the first time I broke that.
1 parent 1acbb0d commit 66d9597

15 files changed

+472
-249
lines changed

dmake/deepobuild.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,13 @@ class DockerLinkSerializer(YAML2PipelineSerializer):
431431
env = FieldSerializer("dict", child = "string", default = {}, example = {'REDIS_URL': '${REDIS_URL}'}, help_text = "Additional environment variables defined when running this image.")
432432
env_exports = FieldSerializer("dict", child = "string", default = {}, help_text = "A set of environment variables that will be exported in services that use this link when testing.")
433433

434+
def _validate_(self, file, needed_migrations, data, field_name=''):
435+
result = super(DockerLinkSerializer, self)._validate_(file, needed_migrations=needed_migrations, data=data, field_name=field_name)
436+
if not allowed_link_name_pattern.match(self.link_name):
437+
raise ValidationError("Invalid link name '%s': only '[a-z0-9-]{1,63}' is allowed. " % (self.link_name))
438+
LinkNames.check_duplicate_link_name('docker_link', self, file)
439+
return result
440+
434441
def get_options(self, path, env):
435442
options = common.eval_str_in_env(self.testing_options, env)
436443

@@ -1092,6 +1099,36 @@ def kind(self, kind):
10921099
return getattr(self, kind)
10931100

10941101

1102+
class LinkNames(object):
1103+
# link_name to (kind[needed_link|needed_service], object, file)
1104+
link_names = dict()
1105+
1106+
@staticmethod
1107+
def reset():
1108+
LinkNames.link_names = dict()
1109+
1110+
@staticmethod
1111+
def check_duplicate_link_name(kind, link, file):
1112+
"""
1113+
Goal: guarantee that each different docker container has a unique link name, while allowing multiple services linking to the same docker container (thus without ambiguity)
1114+
Rules:
1115+
- docker_links: no duplicate link_name
1116+
- needed_services: can duplicate link_name iff NeededServices are equivalent (same service_name, same env_exports, but env_exports and other attributes can be different)
1117+
- same link_name cannot be reused between docker_links and needed_services
1118+
How: compare kind and object:
1119+
- NeededServices has a specialized __eq__() testing the previously defined equivalence
1120+
- DockerLink has the standard __eq__() testing object instance equality
1121+
"""
1122+
# TODO isolate by app name
1123+
link_name = link.link_name
1124+
if not link_name:
1125+
return
1126+
if link_name in LinkNames.link_names:
1127+
other_kind, other_link, other_file = LinkNames.link_names[link_name]
1128+
if not (kind == other_kind and link == other_link):
1129+
raise ValidationError("Duplicate link name '{link_name}' with different definitions: '{kind}' in '{file}', was previously defined as '{other_kind}' in '{other_file}'".format(link_name=link_name, kind=kind, file=file, other_kind=other_kind, other_file=other_file))
1130+
LinkNames.link_names[link_name] = (kind, link, file)
1131+
10951132
@functools.total_ordering
10961133
class NeededServiceSerializer(YAML2PipelineSerializer):
10971134
service_name = FieldSerializer("string", help_text = "The name of the needed application part.", example = "worker-nn", no_slash_no_space = True)
@@ -1149,6 +1186,7 @@ def _validate_(self, file, needed_migrations, data, field_name=''):
11491186
if self.link_name and \
11501187
not allowed_link_name_pattern.match(self.link_name):
11511188
raise ValidationError("Invalid link name '%s': only '[a-z0-9-]{1,63}' is allowed. " % (self.link_name))
1189+
LinkNames.check_duplicate_link_name('needed_link', self, file)
11521190
# a unique identifier that is the same for all equivalent NeededServices
11531191
self._id = hash(self)
11541192
common.logger.debug("NeededService _id: %s for %r" % (self._id, self))
@@ -1545,3 +1583,4 @@ def generate_deploy(self, commands, service_name):
15451583

15461584
def reset():
15471585
SharedVolumes.reset()
1586+
LinkNames.reset()

test/e2e/dmake.yml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,19 @@ services:
2121
- service_name: test-e2e
2222
needed_services:
2323
- service_name: test-web
24-
link_name: web
24+
link_name: test-web
2525
env_exports:
26-
WEB_URL: http://web:8000
26+
WEB_URL: http://test-web:8000
2727
- service_name: test-web2
28-
link_name: web2
28+
link_name: test-web2
2929
env_exports:
30-
WEB2_URL: http://web2:8000
30+
WEB2_URL: http://test-web2:8000
3131
- service_name: test-external-dependency-nginx
32-
link_name: nginx
32+
link_name: test-nginx
3333
env_exports:
34-
NGINX_URL: http://nginx/
34+
NGINX_URL: http://test-nginx/
3535
- service_name: test-external-dependency-nginx
36-
link_name: nginx2
36+
link_name: test-nginx2
3737
config:
3838
docker_image:
3939
build:

0 commit comments

Comments
 (0)