From 5d2447560b84633243bb3b3c776a7fd9d58f0968 Mon Sep 17 00:00:00 2001 From: Sean Eagan Date: Thu, 15 Aug 2019 16:53:43 -0500 Subject: [PATCH] Support builtin chart dependencies This adds support for using the same builtin chart dependencies [0] as `helm install|upgrade ...` would use. [0]: https://helm.sh/docs/developing_charts/#chart-dependencies Change-Id: Ifc541dc273fa2a5c5b4e43125f468ea3fdb0f379 --- armada/handlers/armada.py | 2 +- armada/handlers/chart_deploy.py | 2 +- armada/handlers/chartbuilder.py | 113 ++++++++++++------ armada/schemas/armada-chart-schema-v1.yaml | 1 - armada/tests/unit/handlers/test_armada.py | 3 +- .../tests/unit/handlers/test_chartbuilder.py | 36 +++--- .../operations/documents/migration-v1-v2.rst | 6 +- .../documents/v1/document-authoring.rst | 71 +++++++++-- .../documents/v2/document-authoring.rst | 61 +++++++++- examples/keystone-manifest.yaml | 1 - examples/simple.yaml | 2 - examples/tar_example.yaml | 1 - 12 files changed, 223 insertions(+), 76 deletions(-) diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index a5ba5e51..44bbeb37 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -169,7 +169,7 @@ class Armada(object): self.chart_cache[source_key] = repo_dir chart['source_dir'] = (self.chart_cache.get(source_key), subpath) else: - name = chart['metadata']['name'] + name = ch['metadata']['name'] raise source_exceptions.ChartSourceException(ct_type, name) for dep in ch.get(const.KEYWORD_DATA, {}).get('dependencies', []): diff --git a/armada/handlers/chart_deploy.py b/armada/handlers/chart_deploy.py index 5317d129..30a91e6b 100644 --- a/armada/handlers/chart_deploy.py +++ b/armada/handlers/chart_deploy.py @@ -96,7 +96,7 @@ class ChartDeploy(object): native_wait_enabled = chart_wait.is_native_enabled() - chartbuilder = ChartBuilder(ch) + chartbuilder = ChartBuilder.from_chart_doc(ch) new_chart = chartbuilder.get_helm_chart() if status == const.STATUS_DEPLOYED: diff --git a/armada/handlers/chartbuilder.py b/armada/handlers/chartbuilder.py index fce581d3..cb1f42fe 100644 --- a/armada/handlers/chartbuilder.py +++ b/armada/handlers/chartbuilder.py @@ -13,6 +13,7 @@ # limitations under the License. import os +import re from google.protobuf.any_pb2 import Any from hapi.chart.chart_pb2 import Chart @@ -37,38 +38,74 @@ class ChartBuilder(object): into proper ``protoc`` Helm charts that can be pushed to Tiller. ''' - def __init__(self, chart): - '''Initialize the :class:`ChartBuilder` class. - - :param dict chart: The document containing all intentions to pass to - Tiller. + @classmethod + def from_chart_doc(cls, chart): ''' + Returns a ChartBuilder defined by an Armada Chart doc. + + :param chart: Armada Chart doc for which to build the Helm chart. + ''' + + name = chart['metadata']['name'] + chart_data = chart[const.KEYWORD_DATA] + source_dir = chart_data.get('source_dir') + source_directory = os.path.join(*source_dir) + dependencies = chart_data.get('dependencies') + 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.from_source(name, source_directory) + + @classmethod + def from_source(cls, name, source_directory): + ''' + Returns a ChartBuilder, which gets it dependencies from within the Helm + chart itself. + + :param name: A name to use for the chart. + :param source_directory: The source directory of the Helm chart. + ''' + dependency_builders = [] + charts_dir = os.path.join(source_directory, 'charts') + if os.path.isdir(charts_dir): + for f in os.scandir(charts_dir): + if not f.is_dir(): + # TODO: Support ".tgz" dependency charts. + + # Ignore regular files. + continue + + # Ignore directories that start with "." or "_". + if re.match(r'^[._]', f.name): + continue + + builder = ChartBuilder.from_source(f.name, f.path) + dependency_builders.append(builder) + + return cls(name, source_directory, dependency_builders) + + def __init__(self, name, source_directory, dependency_builders): + ''' + :param name: A name to use for the chart. + :param source_directory: The source directory of the Helm chart. + :param dependency_builders: ChartBuilders to use to build the Helm + chart's dependency charts. + ''' + self.name = name + self.source_directory = source_directory + self.dependency_builders = dependency_builders # cache for generated protoc chart object self._helm_chart = None - # store chart schema - self.chart = chart - self.chart_data = chart[const.KEYWORD_DATA] - - # extract, pull, whatever the chart from its source - self.source_directory = self.get_source_path() - # load ignored files from .helmignore if present self.ignored_files = self.get_ignored_files() - def get_source_path(self): - '''Return the joined path of the source directory and subpath. - - Returns "/" taken from the "source_dir" - property from the chart, or else "" if the property isn't a 2-tuple. - ''' - source_dir = self.chart_data.get('source_dir') - return ( - os.path.join(*source_dir) if ( - source_dir and isinstance(source_dir, (list, tuple)) - and len(source_dir) == 2) else "") - def get_ignored_files(self): '''Load files to ignore from .helmignore if present.''' try: @@ -209,7 +246,7 @@ class ChartBuilder(object): Process all files in templates/ as a template to attach to the chart, building a :class:`hapi.chart.template_pb2.Template` object. ''' - chart_name = self.chart['metadata']['name'] + chart_name = self.name templates = [] if not os.path.exists(os.path.join(self.source_directory, 'templates')): @@ -238,22 +275,23 @@ class ChartBuilder(object): Constructs a :class:`hapi.chart.chart_pb2.Chart` object from the ``chart`` intentions, including all dependencies. ''' - if self._helm_chart: - return self._helm_chart + if not self._helm_chart: + self._helm_chart = self._get_helm_chart() + return self._helm_chart + + def _get_helm_chart(self): + LOG.info( + "Building chart %s from path %s", self.name, self.source_directory) dependencies = [] - chart_dependencies = self.chart_data.get('dependencies', []) - chart_name = self.chart['metadata']['name'] - chart_release = self.chart_data.get('release', None) - for dep_chart in chart_dependencies: - dep_chart_name = dep_chart['metadata']['name'] + for dep_builder in self.dependency_builders: LOG.info( - "Building dependency chart %s for release %s.", dep_chart_name, - chart_release) + "Building dependency chart %s for chart %s.", dep_builder.name, + self.name) try: - dependencies.append(ChartBuilder(dep_chart).get_helm_chart()) + dependencies.append(dep_builder.get_helm_chart()) except Exception: - raise chartbuilder_exceptions.DependencyException(chart_name) + raise chartbuilder_exceptions.DependencyException(self.name) try: helm_chart = Chart( @@ -264,9 +302,8 @@ class ChartBuilder(object): files=self.get_files()) except Exception as e: raise chartbuilder_exceptions.HelmChartBuildException( - chart_name, details=e) + self.name, details=e) - self._helm_chart = helm_chart return helm_chart def dump(self): diff --git a/armada/schemas/armada-chart-schema-v1.yaml b/armada/schemas/armada-chart-schema-v1.yaml index a8814fa7..148850fc 100644 --- a/armada/schemas/armada-chart-schema-v1.yaml +++ b/armada/schemas/armada-chart-schema-v1.yaml @@ -169,7 +169,6 @@ data: - no_hooks additionalProperties: false required: - - dependencies - namespace - chart_name - release diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index 7f7ed7be..c1aa7813 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -366,7 +366,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): @mock.patch.object(armada.Armada, 'post_flight_ops') @mock.patch.object(armada.Armada, 'pre_flight_ops') - @mock.patch('armada.handlers.chart_deploy.ChartBuilder') + @mock.patch('armada.handlers.chart_deploy.ChartBuilder.from_chart_doc') @mock.patch('armada.handlers.chart_deploy.Test') def _do_test( mock_test, mock_chartbuilder, mock_pre_flight, @@ -400,7 +400,6 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): mock_test.return_value.timeout = const.DEFAULT_TEST_TIMEOUT # Stub out irrelevant methods called by `armada.sync()`. - mock_chartbuilder.get_source_path.return_value = None mock_chartbuilder.get_helm_chart.return_value = None # Simulate chart diff, upgrade should only happen if non-empty. diff --git a/armada/tests/unit/handlers/test_chartbuilder.py b/armada/tests/unit/handlers/test_chartbuilder.py index c1202533..70b0d832 100644 --- a/armada/tests/unit/handlers/test_chartbuilder.py +++ b/armada/tests/unit/handlers/test_chartbuilder.py @@ -145,7 +145,8 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): self._write_temporary_file_contents( chart_dir.path, 'Chart.yaml', self.chart_yaml) - chartbuilder = ChartBuilder(self._get_test_chart(chart_dir)) + chartbuilder = ChartBuilder.from_chart_doc( + self._get_test_chart(chart_dir)) # Validate response type is :class:`hapi.chart.metadata_pb2.Metadata` resp = chartbuilder.get_metadata() @@ -155,7 +156,8 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): chart_dir = self.useFixture(fixtures.TempDir()) self.addCleanup(shutil.rmtree, chart_dir.path) - chartbuilder = ChartBuilder(self._get_test_chart(chart_dir)) + chartbuilder = ChartBuilder.from_chart_doc( + self._get_test_chart(chart_dir)) self.assertRaises( chartbuilder_exceptions.MetadataLoadException, @@ -181,7 +183,8 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): for filename in ['template%d' % x for x in range(3)]: self._write_temporary_file_contents(templates_subdir, filename, "") - chartbuilder = ChartBuilder(self._get_test_chart(chart_dir)) + chartbuilder = ChartBuilder.from_chart_doc( + self._get_test_chart(chart_dir)) expected_files = ( '[type_url: "%s"\n, type_url: "%s"\n]' % ('./bar', './foo')) @@ -197,7 +200,8 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): self._write_temporary_file_contents( chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") - chartbuilder = ChartBuilder(self._get_test_chart(chart_dir)) + chartbuilder = ChartBuilder.from_chart_doc( + self._get_test_chart(chart_dir)) chartbuilder.get_files() def test_get_basic_helm_chart(self): @@ -212,7 +216,7 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): ch['data']['source_dir'] = (chart_dir.path, '') test_chart = ch - chartbuilder = ChartBuilder(test_chart) + chartbuilder = ChartBuilder.from_chart_doc(test_chart) helm_chart = chartbuilder.get_helm_chart() expected = inspect.cleandoc( @@ -244,7 +248,7 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): ch['data']['source_dir'] = (chart_dir.path, '') test_chart = ch - chartbuilder = ChartBuilder(test_chart) + chartbuilder = ChartBuilder.from_chart_doc(test_chart) helm_chart = chartbuilder.get_helm_chart() self.assertIsInstance(helm_chart, Chart) @@ -273,7 +277,7 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): ch['data']['source_dir'] = (chart_dir.path, '') test_chart = ch - chartbuilder = ChartBuilder(test_chart) + chartbuilder = ChartBuilder.from_chart_doc(test_chart) helm_chart = chartbuilder.get_helm_chart() expected_files = ( @@ -315,14 +319,14 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): # Files to ignore within templates/ subdirectory. self._write_temporary_file_contents( templates_subdir, file_to_ignore, "") - # Files to ignore within charts/ subdirectory. - self._write_temporary_file_contents(charts_subdir, file_to_ignore, "") # Files to ignore within templates/bin subdirectory. self._write_temporary_file_contents( templates_nested_subdir, file_to_ignore, "") # Files to ignore within charts/extra subdirectory. self._write_temporary_file_contents( charts_nested_subdir, file_to_ignore, "") + self._write_temporary_file_contents( + charts_nested_subdir, 'Chart.yaml', self.chart_yaml) # Files to **include** within charts/ subdirectory. self._write_temporary_file_contents(charts_subdir, '.prov', "xyzzy") @@ -330,7 +334,7 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): ch['data']['source_dir'] = (chart_dir.path, '') test_chart = ch - chartbuilder = ChartBuilder(test_chart) + chartbuilder = ChartBuilder.from_chart_doc(test_chart) helm_chart = chartbuilder.get_helm_chart() expected_files = ( @@ -369,7 +373,7 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): dependency_chart = dep_ch main_chart['data']['dependencies'] = [dependency_chart] - chartbuilder = ChartBuilder(main_chart) + chartbuilder = ChartBuilder.from_chart_doc(main_chart) helm_chart = chartbuilder.get_helm_chart() expected_dependency = inspect.cleandoc( @@ -429,7 +433,7 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): ch['data']['source_dir'] = (chart_dir.path, '') test_chart = ch - chartbuilder = ChartBuilder(test_chart) + chartbuilder = ChartBuilder.from_chart_doc(test_chart) self.assertRegex( repr(chartbuilder.dump()), 'hello-world-chart.*A sample Helm chart for Kubernetes.*') @@ -444,7 +448,7 @@ class ChartBuilderTestCase(BaseChartBuilderTestCase): dependency_chart = dep_ch test_chart['data']['dependencies'] = [dependency_chart] - chartbuilder = ChartBuilder(test_chart) + chartbuilder = ChartBuilder.from_chart_doc(test_chart) re = inspect.cleandoc( """ @@ -473,7 +477,8 @@ class ChartBuilderNegativeTestCase(BaseChartBuilderTestCase): self._write_temporary_file_contents( chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") - chartbuilder = ChartBuilder(self._get_test_chart(chart_dir)) + chartbuilder = ChartBuilder.from_chart_doc( + self._get_test_chart(chart_dir)) # Confirm it failed for both encodings. error_re = ( @@ -494,7 +499,8 @@ class ChartBuilderNegativeTestCase(BaseChartBuilderTestCase): self._write_temporary_file_contents( chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") - chartbuilder = ChartBuilder(self._get_test_chart(chart_dir)) + chartbuilder = ChartBuilder.from_chart_doc( + self._get_test_chart(chart_dir)) side_effects = [self.exc_to_raise, "", ""] with mock.patch("builtins.open", mock.mock_open(read_data="")) \ diff --git a/doc/source/operations/documents/migration-v1-v2.rst b/doc/source/operations/documents/migration-v1-v2.rst index 40ddfa97..3b796552 100644 --- a/doc/source/operations/documents/migration-v1-v2.rst +++ b/doc/source/operations/documents/migration-v1-v2.rst @@ -52,9 +52,9 @@ Chart | ``upgrade.options.no_hooks``, | | | and now optional | | +--------------------------------+------------------------------------------------------------+ -| ``dependencies``, | Remove as desired. | -| ``source.subpath`` | | -| now optional | | +| ``source.subpath`` | Remove as desired. | +| now optional, deafults to no | | +| subpath. | | +--------------------------------+------------------------------------------------------------+ | ``wait`` improvements | See `Wait Improvements`_. | +--------------------------------+------------------------------------------------------------+ diff --git a/doc/source/operations/documents/v1/document-authoring.rst b/doc/source/operations/documents/v1/document-authoring.rst index e5f25aa2..c441510c 100644 --- a/doc/source/operations/documents/v1/document-authoring.rst +++ b/doc/source/operations/documents/v1/document-authoring.rst @@ -134,7 +134,9 @@ Chart +-----------------+----------+---------------------------------------------------------------------------------------+ | source | object | provide a path to a ``git repo``, ``local dir``, or ``tarball url`` chart | +-----------------+----------+---------------------------------------------------------------------------------------+ -| dependencies | object | reference any chart dependencies before install | +| dependencies | object | (optional) Override the `builtin chart dependencies`_ with a list of Chart documents | +| | | to use as dependencies instead. | +| | | NOTE: Builtin ".tgz" dependencies are not yet supported. | +-----------------+----------+---------------------------------------------------------------------------------------+ | timeout | int | time (in seconds) allotted for chart to deploy when 'wait' flag is set (DEPRECATED) | +-----------------+----------+---------------------------------------------------------------------------------------+ @@ -318,7 +320,6 @@ Chart Example location: https://github.com/namespace/repo subpath: . reference: master - dependencies: [] Delete ^^^^^^ @@ -373,7 +374,6 @@ Source Example location: https://github.com/namespace/repo subpath: . reference: master - dependencies: [] # type local --- @@ -397,7 +397,6 @@ Source Example location: /path/to/charts subpath: chart reference: master - dependencies: [] # type tar --- @@ -421,7 +420,6 @@ Source Example location: https://localhost:8879/charts/chart-0.1.0.tgz subpath: mariadb reference: null - dependencies: [] @@ -481,7 +479,6 @@ Simple Example location: https://github.com/namespace/repo subpath: blog-1 reference: new-feat - dependencies: [] --- schema: armada/ChartGroup/v1 metadata: @@ -522,7 +519,6 @@ Multichart Example location: https://github.com/namespace/repo subpath: blog1 reference: master - dependencies: [] --- schema: armada/Chart/v1 metadata: @@ -537,7 +533,6 @@ Multichart Example type: tar location: https://github.com/namespace/repo/blog2.tgz subpath: blog2 - dependencies: [] --- schema: armada/Chart/v1 metadata: @@ -551,7 +546,6 @@ Multichart Example source: type: local location: /home/user/namespace/repo/blog3 - dependencies: [] --- schema: armada/ChartGroup/v1 metadata: @@ -584,8 +578,67 @@ Multichart Example - blog-group-1 - blog-group-2 +Dependency Override Example +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +:: + + --- + schema: armada/Chart/v1 + metadata: + schema: metadata/Document/v1 + name: blog-1 + data: + chart_name: blog-1 + release: blog-1 + namespace: default + values: {} + source: + type: git + location: https://github.com/namespace/repo + subpath: blog-1 + reference: new-feat + dependencies: + - blog-1-dep + --- + schema: armada/Chart/v1 + metadata: + schema: metadata/Document/v1 + name: blog-1-dep + data: + chart_name: blog-1-dep + release: blog-1-dep + namespace: default + values: {} + source: + type: git + location: https://github.com/namespace/dep-repo + subpath: blog-1-dep + reference: new-feat + --- + schema: armada/ChartGroup/v1 + metadata: + schema: metadata/Document/v1 + name: blog-group + data: + description: Deploys Simple Service + sequenced: False + chart_group: + - blog-1 + --- + schema: armada/Manifest/v1 + metadata: + schema: metadata/Document/v1 + name: simple-armada + data: + release_prefix: armada + chart_groups: + - blog-group + References ~~~~~~~~~~ For working examples please check the examples in our repo `here `__. + +.. _builtin chart dependencies: https://helm.sh/docs/developing_charts/#chart-dependencies diff --git a/doc/source/operations/documents/v2/document-authoring.rst b/doc/source/operations/documents/v2/document-authoring.rst index a9552350..f110ec34 100644 --- a/doc/source/operations/documents/v2/document-authoring.rst +++ b/doc/source/operations/documents/v2/document-authoring.rst @@ -121,7 +121,9 @@ Chart +-----------------+----------+---------------------------------------------------------------------------------------+ | source | object | provide a path to a ``git repo``, ``local dir``, or ``tarball url`` chart | +-----------------+----------+---------------------------------------------------------------------------------------+ -| dependencies | object | (optional) reference any chart dependencies before install | +| dependencies | object | (optional) Override the `builtin chart dependencies`_ with a list of Chart documents | +| | | to use as dependencies instead. | +| | | NOTE: Builtin ".tgz" dependencies are not yet supported. | +-----------------+----------+---------------------------------------------------------------------------------------+ .. _wait_v2: @@ -570,8 +572,63 @@ Multichart Example - blog-group-1 - blog-group-2 +Dependency Override Example +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +:: + + --- + schema: armada/Chart/v2 + metadata: + schema: metadata/Document/v1 + name: blog-1 + data: + release: blog-1 + namespace: default + source: + type: git + location: https://github.com/namespace/repo + subpath: blog-1 + reference: new-feat + dependencies: + - blog-dep-1 + --- + schema: armada/Chart/v2 + metadata: + schema: metadata/Document/v1 + name: blog-1-dep + data: + release: blog-1-dep + namespace: default + source: + type: git + location: https://github.com/namespace/dep-repo + subpath: blog-1-dep + reference: new-feat + --- + schema: armada/ChartGroup/v2 + metadata: + schema: metadata/Document/v1 + name: blog-group + data: + description: Deploys Simple Service + chart_group: + - blog-1 + --- + schema: armada/Manifest/v2 + metadata: + schema: metadata/Document/v1 + name: simple-armada + data: + release_prefix: armada + chart_groups: + - blog-group + References ~~~~~~~~~~ For working examples please check the examples in our repo -`here `__. +`here `__ + + +.. _builtin chart dependencies: https://helm.sh/docs/developing_charts/#chart-dependencies diff --git a/examples/keystone-manifest.yaml b/examples/keystone-manifest.yaml index 3112be3a..16716395 100644 --- a/examples/keystone-manifest.yaml +++ b/examples/keystone-manifest.yaml @@ -17,7 +17,6 @@ data: location: https://git.openstack.org/openstack/openstack-helm-infra subpath: helm-toolkit reference: master - dependencies: [] --- schema: armada/Chart/v1 metadata: diff --git a/examples/simple.yaml b/examples/simple.yaml index fcf87075..33e8032f 100644 --- a/examples/simple.yaml +++ b/examples/simple.yaml @@ -17,7 +17,6 @@ data: location: https://github.com/gardlt/hello-world-chart subpath: . reference: 87aad18f7d8c6a1a08f3adc8866efd33bee6aa52 - dependencies: [] --- schema: armada/Chart/v1 metadata: @@ -38,7 +37,6 @@ data: location: https://github.com/gardlt/hello-world-chart subpath: . reference: master - dependencies: [] --- schema: armada/ChartGroup/v1 metadata: diff --git a/examples/tar_example.yaml b/examples/tar_example.yaml index 0aaefe84..07445ef1 100644 --- a/examples/tar_example.yaml +++ b/examples/tar_example.yaml @@ -13,7 +13,6 @@ data: location: git://opendev.org/openstack/openstack-helm-infra.git subpath: helm-toolkit reference: master - dependencies: [] --- schema: armada/Chart/v1 metadata: