Skip to content

Commit

Permalink
Add check for Marathon/Consul app/service namespace clash
Browse files Browse the repository at this point in the history
* Throw an error if there is a clash
  • Loading branch information
JayH5 committed Sep 14, 2015
1 parent 9d5b5ee commit 1d472f6
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 2 deletions.
34 changes: 32 additions & 2 deletions consular/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,43 @@ def sync_apps(self, purge=False):
"""
d = self.marathon_request('GET', '/v2/apps')
d.addCallback(lambda response: response.json())
d.addCallback(lambda response_json: response_json['apps'])
d.addCallback(self.check_apps_namespace_clash)
d.addCallback(
lambda data: gatherResults(
[self.sync_app(app) for app in data['apps']]))
lambda apps: gatherResults([self.sync_app(app) for app in apps]))
if purge:
d.addCallback(lambda _: self.purge_dead_services())
return d

def check_apps_namespace_clash(self, apps):
"""
Checks if app names in Marathon will cause a namespace clash in Consul.
Throws an exception if there is a collision, else returns the apps.
:param: apps:
The JSON list of apps from Marathon's API.
"""
# Collect the app name to app id(s) mapping.
name_ids = {}
for app in apps:
app_id = app['id']
app_name = get_app_name(app_id)
name_ids.setdefault(app_name, []).append(app_id)

# Check if any app names map to more than one app id.
collisions = {name: ids
for name, ids in name_ids.items() if len(ids) > 1}

if collisions:
collisions_string = '\n'.join(sorted(
['%s => %s' % (name, ', '.join(ids),)
for name, ids in collisions.items()]))
raise RuntimeError(
'The following Consul service name will resolve to multiple '
'Marathon app names: \n%s' % (collisions_string,))

return apps

def get_app(self, app_id):
d = self.marathon_request('GET', '/v2/apps%s' % (app_id,))
d.addCallback(lambda response: response.json())
Expand Down
33 changes: 33 additions & 0 deletions consular/tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,39 @@ def test_sync_apps(self):
FakeResponse(200, [], json.dumps({'apps': []})))
yield d

def test_check_apps_namespace_clash_no_clash(self):
"""
When checking for app namespace clashes and there are no clashes, the
list of apps is returned.
"""
apps = [
{'id': '/my-group/my-app'},
{'id': '/my-app'},
{'id': '/my-group/my-app2'},
]
apps_returned = self.consular.check_apps_namespace_clash(apps)
self.assertEqual(apps, apps_returned)

def test_check_apps_namespace_clash_clashing(self):
"""
When checking for app namespace clashes and there are clashes, an
error is raised with an error message describing the clashes.
"""
apps = [
{'id': '/my-group/my-subgroup/my-app'},
{'id': '/my-group/my-subgroup-my-app'},
{'id': '/my-group-my-subgroup-my-app'},
{'id': '/my-app'},
]
exception = self.assertRaises(
RuntimeError, self.consular.check_apps_namespace_clash, apps)

self.assertEqual('The following Consul service name will resolve to '
'multiple Marathon app names: \nmy-group-my-subgroup-'
'my-app => /my-group/my-subgroup/my-app, /my-group/my'
'-subgroup-my-app, /my-group-my-subgroup-my-app',
str(exception))

@inlineCallbacks
def test_purge_dead_services(self):
d = self.consular.purge_dead_services()
Expand Down

0 comments on commit 1d472f6

Please sign in to comment.