airship/armada
Code Issues Proposed changes

Fix improper indexing throughout armada handlers

Browse Source 
This patch set fixes indexing improperly through the manifest
handler. This is a straightforward fix by using .get() in conjunction
with sensible defaults (rather than None for everything). However,
the better longterm solution is to use a series of helper functions
to retrieve common properties from Armada objects.

Change-Id: Ic3edf954e2a7b9041a01a254cb5971c02adeca3f
This commit is contained in:
Felipe Monteiro 2018-04-15 11:07:14 -04:00
parent df91412ed6
commit b5d5c2dfd5
3 changed files with 51 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
armada/handlers/armada.py
View File

@ -140,15 +140,15 @@ class Armada(object):
details=','.join([m.get('message') for m in msg_list]))
# Purge known releases that have failed and are in the current yaml
prefix = self.manifest.get(const.KEYWORD_ARMADA).get(
const.KEYWORD_PREFIX)
armada_data = self.manifest.get(const.KEYWORD_ARMADA, {})
prefix = armada_data.get(const.KEYWORD_PREFIX, '')
failed_releases = self.get_releases_by_status(const.STATUS_FAILED)
for release in failed_releases:
for group in self.manifest.get(const.KEYWORD_ARMADA).get(
const.KEYWORD_GROUPS):
for ch in group.get(const.KEYWORD_CHARTS):
for group in armada_data.get(const.KEYWORD_GROUPS, []):
for ch in group.get(const.KEYWORD_CHARTS, []):
ch_release_name = release_prefix(
prefix, ch.get('chart').get('chart_name'))
prefix, ch.get('chart', {}).get('chart_name'))
if release[0] == ch_release_name:
LOG.info('Purging failed release %s '
'before deployment', release[0])
@ -159,12 +159,11 @@ class Armada(object):
# We only support a git source type right now, which can also
# handle git:// local paths as well
repos = {}
for group in self.manifest.get(const.KEYWORD_ARMADA).get(
const.KEYWORD_GROUPS):
for ch in group.get(const.KEYWORD_CHARTS):
for group in armada_data.get(const.KEYWORD_GROUPS, []):
for ch in group.get(const.KEYWORD_CHARTS, []):
self.tag_cloned_repo(ch, repos)
for dep in ch.get('chart').get('dependencies'):
for dep in ch.get('chart', {}).get('dependencies', []):
self.tag_cloned_repo(dep, repos)
def tag_cloned_repo(self, ch, repos):
@ -242,9 +241,13 @@ class Armada(object):
# extract known charts on tiller right now
known_releases = self.tiller.list_charts()
prefix = self.manifest.get(const.KEYWORD_ARMADA).get(
const.KEYWORD_PREFIX)
armada_data = self.manifest.get(const.KEYWORD_ARMADA, {})
prefix = armada_data.get(const.KEYWORD_PREFIX, '')
# TODO(fmontei): This is a useless exception that is probably never
# thrown as `known_releases` is a list and the proper behavior here
# should be to return early. Fix this once all side effects of
# correcting this are well understood.
if known_releases is None:
raise armada_exceptions.KnownReleasesException()
@ -252,21 +255,20 @@ class Armada(object):
LOG.debug("Release %s, Version %s found on Tiller", release[0],
release[1])
for entry in self.manifest[const.KEYWORD_ARMADA][const.KEYWORD_GROUPS]:
for group in armada_data.get(const.KEYWORD_GROUPS, []):
tiller_should_wait = self.tiller_should_wait
tiller_timeout = self.tiller_timeout
desc = entry.get('description', 'A Chart Group')
chart_groups = entry.get(const.KEYWORD_CHARTS, [])
test_charts = entry.get('test_charts', False)
desc = group.get('description', 'A Chart Group')
charts = group.get(const.KEYWORD_CHARTS, [])
test_charts = group.get('test_charts', False)
if entry.get('sequenced', False) or test_charts:
if group.get('sequenced', False) or test_charts:
tiller_should_wait = True
LOG.info('Deploying: %s', desc)
for chartgroup in chart_groups:
chart = chartgroup.get('chart', {})
for chart in charts:
chart = chart.get('chart', {})
values = chart.get('values', {})
test_chart = chart.get('test', False)
namespace = chart.get('namespace', None)
@ -288,7 +290,7 @@ class Armada(object):
# (caution: it always default to 3600,
# take care to differentiate user input)
if tiller_should_wait and tiller_timeout == DEFAULT_TIMEOUT:
tiller_timeout = chart.get('timeout', tiller_timeout)
tiller_timeout = chart.get('timeout', tiller_timeout)
wait_values = chart.get('wait', {})
wait_timeout = wait_values.get('timeout', tiller_timeout)
wait_values_labels = wait_values.get('labels', {})
@ -430,11 +432,14 @@ class Armada(object):
Operations to run after deployment process has terminated
'''
# Delete temp dirs used for deployment
for group in self.manifest.get(const.KEYWORD_ARMADA).get(
const.KEYWORD_GROUPS):
for ch in group.get(const.KEYWORD_CHARTS):
if ch.get('chart').get('source').get('type') == 'git':
source.source_cleanup(ch.get('chart').get('source_dir')[0])
for group in self.manifest.get(const.KEYWORD_ARMADA, {}).get(
const.KEYWORD_GROUPS, []):
for ch in group.get(const.KEYWORD_CHARTS, []):
chart = ch.get('chart', {})
if chart.get('source', {}).get('type') == 'git':
source_dir = chart.get('source_dir')
if isinstance(source_dir, tuple) and source_dir:
source.source_cleanup(source_dir[0])
def show_diff(self, chart, installed_chart, installed_values, target_chart,
target_values, msg):

17
armada/handlers/manifest.py
View File

@ -160,13 +160,14 @@ class Manifest(object):
"""
try:
dep = None
for iter, dep in enumerate(chart.get('data').get('dependencies')):
chart_dependencies = chart.get('data', {}).get('dependencies', [])
for iter, dep in enumerate(chart_dependencies):
if isinstance(dep, dict):
continue
chart_dep = self.find_chart_document(dep)
self.build_chart_deps(chart_dep)
chart['data']['dependencies'][iter] = {
'chart': chart_dep.get('data')
'chart': chart_dep.get('data', {})
}
except Exception:
raise exceptions.ManifestException(
@ -187,13 +188,13 @@ class Manifest(object):
"""
try:
chart = None
for iter, chart in enumerate(chart_group.get('data').get(
for iter, chart in enumerate(chart_group.get('data', {}).get(
'chart_group', [])):
if isinstance(chart, dict):
continue
chart_dep = self.find_chart_document(chart)
chart_group['data']['chart_group'][iter] = {
'chart': chart_dep.get('data')
'chart': chart_dep.get('data', {})
}
except Exception:
raise exceptions.ManifestException(
@ -213,15 +214,15 @@ class Manifest(object):
"""
try:
group = None
for iter, group in enumerate(self.manifest.get('data').get(
for iter, group in enumerate(self.manifest.get('data', {}).get(
'chart_groups', [])):
if isinstance(group, dict):
continue
chart_grp = self.find_chart_group_document(group)
# Add name to chart group
ch_grp_data = chart_grp.get('data')
ch_grp_data['name'] = chart_grp.get('metadata').get('name')
ch_grp_data = chart_grp.get('data', {})
ch_grp_data['name'] = chart_grp.get('metadata', {}).get('name')
self.manifest['data']['chart_groups'][iter] = ch_grp_data
except Exception:
@ -244,5 +245,5 @@ class Manifest(object):
self.build_armada_manifest()
return {
'armada': self.manifest.get('data')
'armada': self.manifest.get('data', {})
}

20
armada/handlers/override.py
View File

@ -61,7 +61,7 @@ class Override(object):
def find_manifest_document(self, doc_path):
for doc in self.documents:
if doc.get('schema') == self.find_document_type(
doc_path[0]) and doc.get('metadata').get(
doc_path[0]) and doc.get('metadata', {}).get(
'name') == doc_path[1]:
return doc
@ -104,10 +104,9 @@ class Override(object):
def override_manifest_value(self, doc_path, data_path, new_value):
document = self.find_manifest_document(doc_path)
new_data = self.array_to_dict(data_path, new_value)
self.update(document.get('data'), new_data)
self.update(document.get('data', {}), new_data)
def update_document(self, merging_values):
for doc in merging_values:
if doc.get('schema') == const.DOCUMENT_CHART:
self.update_chart_document(doc)
@ -119,22 +118,25 @@ class Override(object):
def update_chart_document(self, ovr):
for doc in self.documents:
if doc.get('schema') == const.DOCUMENT_CHART and doc.get(
'metadata').get('name') == ovr.get('metadata').get('name'):
self.update(doc.get('data'), ovr.get('data'))
'metadata', {}).get('name') == ovr.get('metadata', {}).get(
'name'):
self.update(doc.get('data', {}), ovr.get('data', {}))
return
def update_chart_group_document(self, ovr):
for doc in self.documents:
if doc.get('schema') == const.DOCUMENT_GROUP and doc.get(
'metadata').get('name') == ovr.get('metadata').get('name'):
self.update(doc.get('data'), ovr.get('data'))
'metadata', {}).get('name') == ovr.get('metadata', {}).get(
'name'):
self.update(doc.get('data', {}), ovr.get('data', {}))
return
def update_armada_manifest(self, ovr):
for doc in self.documents:
if doc.get('schema') == const.DOCUMENT_MANIFEST and doc.get(
'metadata').get('name') == ovr.get('metadata').get('name'):
self.update(doc.get('data'), ovr.get('data'))
'metadata', {}).get('name') == ovr.get('metadata', {}).get(
'name'):
self.update(doc.get('data', {}), ovr.get('data', {}))
return
def update_manifests(self):