airship/armada
Code Issues Proposed changes

fix: Align template file naming with Helm CLI

Browse Source 
Armada has previously named template files relative to the
`templates` dir, whereas the Helm CLI names them relative
to the chart root. This causes `include`s of these templates
to fail.

This change fixes this, for armada/Chart/v2 docs only, since it
is a breaking change, as some charts may have already aligned
with the existing Armada behavior. When updating a release
previously deployed with armada/Chart/v1, the fixed template
names alone will not cause the release to be updated, as the
diff logic accounts for this.

Change-Id: I243073ca4c2e1edbcb0d8f649475f568fc7c818f
This commit is contained in:
Sean Eagan 2019-09-09 11:48:05 -05:00 committed by Drew Walters
parent 19c2b1e299
commit aa75c3eb45
4 changed files with 64 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
armada/handlers/chartbuilder.py
View File

@ -24,8 +24,9 @@ from oslo_config import cfg
from oslo_log import log as logging
import yaml
from armada.exceptions import chartbuilder_exceptions
from armada import const
from armada.exceptions import chartbuilder_exceptions
from armada.handlers.schema import get_schema_info
LOG = logging.getLogger(__name__)
@ -51,18 +52,31 @@ class ChartBuilder(object):
source_dir = chart_data.get('source_dir')
source_directory = os.path.join(*source_dir)
dependencies = chart_data.get('dependencies')
# TODO: Remove when v1 doc support is removed.
schema_info = get_schema_info(chart['schema'])
if schema_info.version < 2:
fix_tpl_name = False
else:
fix_tpl_name = True
if dependencies is not None:
dependency_builders = []
for chart_dep in dependencies:
builder = ChartBuilder.from_chart_doc(chart_dep)
dependency_builders.append(builder)
return cls(name, source_directory, dependency_builders)
return cls(
name,
source_directory,
dependency_builders,
fix_tpl_name=fix_tpl_name)
return cls.from_source(name, source_directory)
return cls.from_source(
name, source_directory, fix_tpl_name=fix_tpl_name)
@classmethod
def from_source(cls, name, source_directory):
def from_source(cls, name, source_directory, fix_tpl_name=False):
'''
Returns a ChartBuilder, which gets it dependencies from within the Helm
chart itself.
@ -84,12 +98,19 @@ class ChartBuilder(object):
if re.match(r'^[._]', f.name):
continue
builder = ChartBuilder.from_source(f.name, f.path)
builder = ChartBuilder.from_source(
f.name, f.path, fix_tpl_name=fix_tpl_name)
dependency_builders.append(builder)
return cls(name, source_directory, dependency_builders)
return cls(
name,
source_directory,
dependency_builders,
fix_tpl_name=fix_tpl_name)
def __init__(self, name, source_directory, dependency_builders):
def __init__(
self, name, source_directory, dependency_builders,
fix_tpl_name=False):
'''
:param name: A name to use for the chart.
:param source_directory: The source directory of the Helm chart.
@ -99,6 +120,7 @@ class ChartBuilder(object):
self.name = name
self.source_directory = source_directory
self.dependency_builders = dependency_builders
self.fix_tpl_name = fix_tpl_name
# cache for generated protoc chart object
self._helm_chart = None
@ -248,17 +270,22 @@ class ChartBuilder(object):
'''
chart_name = self.name
templates = []
if not os.path.exists(os.path.join(self.source_directory,
'templates')):
tpl_dir = os.path.join(self.source_directory, 'templates')
if not os.path.exists(tpl_dir):
LOG.warn(
"Chart %s has no templates directory. "
"No templates will be deployed", chart_name)
for root, _, files in os.walk(os.path.join(self.source_directory,
'templates'), topdown=True):
for root, _, files in os.walk(tpl_dir, topdown=True):
for tpl_file in files:
tname = os.path.relpath(
os.path.join(root, tpl_file),
os.path.join(self.source_directory, 'templates'))
# For v1 compatibility, name template relative to template
# dir, for v2 fix the name to be relative to the chart root
# to match Helm CLI behavior.
self.source_directory if self.fix_tpl_name else tpl_dir)
# NOTE: If the template name is fixed (see above), then this
# also fixes the path passed here, which could theoretically
# affect which files get ignored, though unlikely.
if self.ignore_file(tname):
LOG.debug('Ignoring file %s', tname)
continue

14
armada/handlers/release_diff.py
View File

@ -77,7 +77,19 @@ class ReleaseDiff(object):
chart_desc = '{} ({})'.format(chart.metadata.name, desc)
raise armada_exceptions.InvalidValuesYamlException(chart_desc)
files = {f.type_url: f.value for f in chart.files}
templates = {t.name: t.data for t in chart.templates}
# With armada/Chart/v1, Armada deployed releases with incorrect
# template names, omitting the `templates/` prefix, which is fixed in
# v2. This aligns these template names, so that the prefixes match, to
# avoid unwanted updates to releases when consuming this fix.
def fix_tpl_name(tpl_name):
CORRECT_PREFIX = 'templates/'
if tpl_name.startswith(CORRECT_PREFIX):
return tpl_name
return CORRECT_PREFIX + tpl_name
templates = {fix_tpl_name(t.name): t.data for t in chart.templates}
dependencies = {
d.metadata.name: self.make_chart_dict(
d, '{}({} dependency)'.format(desc, d.metadata.name))

3
armada/tests/unit/handlers/test_chartbuilder.py
View File

@ -60,6 +60,7 @@ class BaseChartBuilderTestCase(testtools.TestCase):
"""
chart_stream = """
schema: armada/Chart/v1
metadata:
name: test
data:
@ -90,6 +91,7 @@ class BaseChartBuilderTestCase(testtools.TestCase):
"""
dependency_chart_stream = """
schema: armada/Chart/v1
metadata:
name: dep
data:
@ -127,6 +129,7 @@ class BaseChartBuilderTestCase(testtools.TestCase):
def _get_test_chart(self, chart_dir):
return {
'schema': 'armada/Chart/v1',
'metadata': {
'name': 'test'
},

9
doc/source/operations/documents/migration-v1-v2.rst
View File

@ -56,6 +56,15 @@ Chart
| now optional, deafults to no | |
| subpath. | |
+--------------------------------+------------------------------------------------------------+
| Template naming for template | If a chart was relying on Armada's previous misaligned |
| files aligned with Helm CLI. | template naming, where it was omitting the ``templates/`` |
| | prefix, such as via ``include`` argument, that argument |
| | will need to be updated. This could also theoretically |
| | affect whether the file is ignored, if the old or new |
| | name is in ``.helmignore`` (unlikely). The fixed template |
| | names alone will not cause a release to be updated, as the |
| | diff logic accounts for this. |
+--------------------------------+------------------------------------------------------------+
| ``wait`` improvements | See `Wait Improvements`_. |
+--------------------------------+------------------------------------------------------------+