From 68747d08150142ca465278ffc587c53ea293d385 Mon Sep 17 00:00:00 2001 From: Sean Eagan Date: Thu, 15 Jul 2021 17:08:24 -0500 Subject: [PATCH] Use helm 3 CLI as backend Helm 3 breaking changes (likely non-exhaustive): - crd-install hook removed and replaced with crds directory in chart where all CRDs defined in it will be installed before any rendering of the chart - test-failure hook annotation value removed, and test-success deprecated. Use test instead - `--force` no longer handles recreating resources which cannot be updated due to e.g. immutability [0] - `--recreate-pods` removed, use declarative approach instead [1] [0]: https://github.com/helm/helm/issues/7082 [1]: https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments Signed-off-by: Sean Eagan Change-Id: I20ff40ba55197de3d37e5fd647e7d2524a53248f --- armada/api/__init__.py | 4 + armada/api/controller/armada.py | 8 +- armada/api/controller/releases.py | 54 +++ armada/api/controller/test.py | 40 +- armada/api/controller/tiller.py | 32 -- armada/api/server.py | 6 +- armada/cli/apply.py | 43 +- armada/cli/test.py | 79 +--- armada/common/policies/service.py | 8 + armada/common/policies/tiller.py | 8 - armada/exceptions/armada_exceptions.py | 10 +- armada/exceptions/chartbuilder_exceptions.py | 70 --- armada/exceptions/helm_exceptions.py | 27 ++ armada/exceptions/tiller_exceptions.py | 11 - armada/handlers/armada.py | 53 +-- armada/handlers/chart_delete.py | 16 +- armada/handlers/chart_deploy.py | 149 +++--- armada/handlers/chartbuilder.py | 325 ++------------ armada/handlers/helm.py | 245 ++++++++++ armada/handlers/k8s.py | 10 + armada/handlers/lock.py | 14 +- armada/handlers/release_diff.py | 52 +-- armada/handlers/test.py | 72 ++- armada/handlers/tiller.py | 3 +- armada/handlers/wait.py | 27 +- armada/schemas/armada-chart-schema-v1.yaml | 2 - armada/schemas/armada-chart-schema-v2.yaml | 2 - .../tests/unit/api/test_armada_controller.py | 14 +- .../unit/api/test_releases_controller.py | 100 +++++ armada/tests/unit/api/test_test_controller.py | 108 ++--- .../tests/unit/api/test_tiller_controller.py | 73 --- armada/tests/unit/fake_policy.py | 2 +- armada/tests/unit/handlers/test_armada.py | 216 ++++----- .../tests/unit/handlers/test_chartbuilder.py | 424 ++---------------- armada/tests/unit/handlers/test_lock.py | 6 +- .../tests/unit/handlers/test_release_diff.py | 184 +------- armada/tests/unit/handlers/test_test.py | 191 +++----- armada/tests/unit/handlers/test_wait.py | 9 +- armada/utils/helm.py | 5 +- armada/utils/release.py | 23 +- charts/armada/values.yaml | 2 +- .../documents/v2/document-authoring.rst | 2 - .../exceptions/chartbuilder-exceptions.inc | 20 - etc/armada/policy.yaml | 8 +- examples/podinfo.yaml | 59 +++ 45 files changed, 1038 insertions(+), 1778 deletions(-) create mode 100644 armada/api/controller/releases.py create mode 100644 armada/exceptions/helm_exceptions.py create mode 100644 armada/handlers/helm.py create mode 100644 armada/tests/unit/api/test_releases_controller.py create mode 100644 examples/podinfo.yaml diff --git a/armada/api/__init__.py b/armada/api/__init__.py index 8f1f3f47..de12a114 100644 --- a/armada/api/__init__.py +++ b/armada/api/__init__.py @@ -22,6 +22,7 @@ from oslo_log import log as logging from oslo_utils import excutils import yaml +from armada.handlers.helm import Helm from armada.handlers.tiller import Tiller CONF = cfg.CONF @@ -120,6 +121,9 @@ class BaseResource(object): def get_tiller(self, req, resp): return Tiller() + def get_helm(self, req, resp): + return Helm() + class ArmadaRequestContext(object): def __init__(self): diff --git a/armada/api/controller/armada.py b/armada/api/controller/armada.py index 577afd7c..053c8eb4 100644 --- a/armada/api/controller/armada.py +++ b/armada/api/controller/armada.py @@ -69,8 +69,8 @@ class Apply(api.BaseResource): message="Request must be in application/x-yaml" "or application/json") try: - with self.get_tiller(req, resp) as tiller: - msg = self.handle(req, documents, tiller) + with self.get_helm(req, resp) as helm: + msg = self.handle(req, documents, helm) resp.text = json.dumps({ 'message': msg, }) @@ -88,7 +88,7 @@ class Apply(api.BaseResource): self.return_error(resp, falcon.HTTP_500, message=err_message) @lock_and_thread() - def handle(self, req, documents, tiller): + def handle(self, req, documents, helm): armada = Armada( documents, disable_update_pre=req.get_param_as_bool('disable_update_pre'), @@ -96,7 +96,7 @@ class Apply(api.BaseResource): enable_chart_cleanup=req.get_param_as_bool('enable_chart_cleanup'), force_wait=req.get_param_as_bool('wait'), timeout=req.get_param_as_int('timeout'), - tiller=tiller, + helm=helm, target_manifest=req.get_param('target_manifest')) return armada.sync() diff --git a/armada/api/controller/releases.py b/armada/api/controller/releases.py new file mode 100644 index 00000000..8b8a1249 --- /dev/null +++ b/armada/api/controller/releases.py @@ -0,0 +1,54 @@ +# Copyright 2017 The Armada Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import json + +import falcon +from oslo_config import cfg +from oslo_log import log as logging + +from armada import api +from armada.common import policy + +CONF = cfg.CONF +LOG = logging.getLogger(__name__) + + +class Releases(api.BaseResource): + @policy.enforce('armada:get_release') + def on_get(self, req, resp): + '''Controller for listing Helm releases. + ''' + try: + with self.get_helm(req, resp) as helm: + releases = self.handle(helm) + resp.text = json.dumps({ + 'releases': releases, + }) + resp.content_type = 'application/json' + resp.status = falcon.HTTP_200 + + except Exception as e: + err_message = 'Unable to find Helm Releases: {}'.format(e) + self.error(req.context, err_message) + self.return_error(resp, falcon.HTTP_500, message=err_message) + + def handle(self, helm): + LOG.debug('Getting helm releases') + + releases = {} + for release in helm.list_release_ids(): + releases.setdefault(release.namespace, []) + releases[release.namespace].append(release.name) + return releases diff --git a/armada/api/controller/test.py b/armada/api/controller/test.py index 229fac47..a032ae84 100644 --- a/armada/api/controller/test.py +++ b/armada/api/controller/test.py @@ -21,6 +21,7 @@ import yaml from armada import api from armada.common import policy from armada import const +from armada.handlers.helm import HelmReleaseId from armada.handlers.lock import lock_and_thread, LockException from armada.handlers.manifest import Manifest from armada.handlers.test import Test @@ -36,11 +37,12 @@ class TestReleasesReleaseNameController(api.BaseResource): ''' @policy.enforce('armada:test_release') - def on_get(self, req, resp, release): + def on_get(self, req, resp, namespace, release): try: + release_id = HelmReleaseId(namespace, release) + with self.get_helm(req, resp) as helm: - with self.get_tiller(req, resp) as tiller: - success = self.handle(req, release, tiller) + success = self.handle(req, release_id, helm) if success: msg = { @@ -60,9 +62,8 @@ class TestReleasesReleaseNameController(api.BaseResource): self.return_error(resp, falcon.HTTP_409, message=str(e)) @lock_and_thread() - def handle(self, req, release, tiller): - cleanup = req.get_param_as_bool('cleanup') - test_handler = Test({}, release, tiller, cleanup=cleanup) + def handle(self, req, release_id, helm): + test_handler = Test({}, release_id, helm) return test_handler.test_release_for_success() @@ -111,13 +112,13 @@ class TestReleasesManifestController(api.BaseResource): def on_post(self, req, resp): # TODO(fmontei): Validation Content-Type is application/x-yaml. try: - with self.get_tiller(req, resp) as tiller: - return self.handle(req, resp, tiller) + with self.get_helm(req, resp) as helm: + return self.handle(req, resp, helm) except LockException as e: self.return_error(resp, falcon.HTTP_409, message=str(e)) @lock_and_thread() - def handle(self, req, resp, tiller): + def handle(self, req, resp, helm): try: documents = self.req_yaml(req, default=[]) except yaml.YAMLError: @@ -134,7 +135,7 @@ class TestReleasesManifestController(api.BaseResource): documents, target_manifest=target_manifest).get_manifest() prefix = armada_obj[const.KEYWORD_DATA][const.KEYWORD_PREFIX] - known_releases = [release[0] for release in tiller.list_charts()] + release_ids = helm.list_release_ids() message = {'tests': {'passed': [], 'skipped': [], 'failed': []}} @@ -143,31 +144,30 @@ class TestReleasesManifestController(api.BaseResource): for ch in group.get(const.KEYWORD_CHARTS): chart = ch['chart'] - release_name = release_prefixer(prefix, chart.get('release')) - if release_name in known_releases: - cleanup = req.get_param_as_bool('cleanup') + release_id = helm.HelmReleaseId( + ch['namespace'], release_prefixer(prefix, ch['release'])) + if release_id in release_ids: enable_all = req.get_param_as_bool('enable_all') cg_test_charts = group.get('test_charts') test_handler = Test( chart, - release_name, - tiller, + release_id, + helm, cg_test_charts=cg_test_charts, - cleanup=cleanup, enable_all=enable_all) if test_handler.test_enabled: success = test_handler.test_release_for_success() if success: - message['test']['passed'].append(release_name) + message['test']['passed'].append(release_id) else: - message['test']['failed'].append(release_name) + message['test']['failed'].append(release_id) else: self.logger.info( - 'Release %s not found - SKIPPING', release_name) - message['test']['skipped'].append(release_name) + 'Release %s not found - SKIPPING', release_id) + message['test']['skipped'].append(release_id) resp.status = falcon.HTTP_200 resp.text = json.dumps(message) diff --git a/armada/api/controller/tiller.py b/armada/api/controller/tiller.py index 6baf78d2..21c59e22 100644 --- a/armada/api/controller/tiller.py +++ b/armada/api/controller/tiller.py @@ -56,35 +56,3 @@ class Status(api.BaseResource): } } return message - - -class Release(api.BaseResource): - @policy.enforce('tiller:get_release') - def on_get(self, req, resp): - '''Controller for listing Tiller releases. - ''' - try: - with self.get_tiller(req, resp) as tiller: - releases = self.handle(tiller) - resp.text = json.dumps({ - 'releases': releases, - }) - resp.content_type = 'application/json' - resp.status = falcon.HTTP_200 - - except Exception as e: - err_message = 'Unable to find Tiller Releases: {}'.format(e) - self.error(req.context, err_message) - self.return_error(resp, falcon.HTTP_500, message=err_message) - - def handle(self, tiller): - LOG.debug( - 'Tiller (Release) at: %s:%s, namespace=%s, ' - 'timeout=%s', tiller.tiller_host, tiller.tiller_port, - tiller.tiller_namespace, tiller.timeout) - - releases = {} - for release in tiller.list_releases(): - releases.setdefault(release.namespace, []) - releases[release.namespace].append(release.name) - return releases diff --git a/armada/api/server.py b/armada/api/server.py index 55f0a56e..8ce47073 100644 --- a/armada/api/server.py +++ b/armada/api/server.py @@ -27,7 +27,7 @@ from armada.api.controller.test import TestReleasesReleaseNameController from armada.api.controller.test import TestReleasesManifestController from armada.api.controller.health import Health from armada.api.controller.metrics import Metrics -from armada.api.controller.tiller import Release +from armada.api.controller.releases import Releases from armada.api.controller.tiller import Status from armada.api.controller.validation import Validate from armada.api.controller.versions import Versions @@ -62,10 +62,10 @@ def create(enable_middleware=CONF.middleware): url_routes_v1 = [ (HEALTH_PATH, Health()), ('apply', Apply()), - ('releases', Release()), + ('releases', Releases()), ('status', Status()), ('tests', TestReleasesManifestController()), - ('test/{release}', TestReleasesReleaseNameController()), + ('test/{namespace}/{release}', TestReleasesReleaseNameController()), ('validatedesign', Validate()), (METRICS_PATH, Metrics()), ] diff --git a/armada/cli/apply.py b/armada/cli/apply.py index cbc8c236..deb6c55a 100644 --- a/armada/cli/apply.py +++ b/armada/cli/apply.py @@ -23,7 +23,7 @@ from armada.handlers import metrics from armada.handlers.armada import Armada from armada.handlers.document import ReferenceResolver from armada.handlers.lock import lock_and_thread -from armada.handlers.tiller import Tiller +from armada.handlers.helm import Helm CONF = cfg.CONF @@ -100,15 +100,6 @@ SHORT_DESC = "Command installs manifest charts." multiple=True, type=str, default=[]) -@click.option('--tiller-host', help="Tiller host IP.", default=None) -@click.option( - '--tiller-port', help="Tiller host port.", type=int, default=None) -@click.option( - '--tiller-namespace', - '-tn', - help="Tiller namespace.", - type=str, - default=None) @click.option( '--timeout', help="Specifies time to wait for each chart to fully " @@ -141,23 +132,20 @@ SHORT_DESC = "Command installs manifest charts." @click.pass_context def apply_create( ctx, locations, api, disable_update_post, disable_update_pre, - enable_chart_cleanup, metrics_output, use_doc_ref, set, tiller_host, - tiller_port, tiller_namespace, timeout, values, wait, target_manifest, - bearer_token, debug): + enable_chart_cleanup, metrics_output, use_doc_ref, set, timeout, + values, wait, target_manifest, bearer_token, debug): CONF.debug = debug ApplyManifest( ctx, locations, api, disable_update_post, disable_update_pre, - enable_chart_cleanup, metrics_output, use_doc_ref, set, tiller_host, - tiller_port, tiller_namespace, timeout, values, wait, target_manifest, - bearer_token).safe_invoke() + enable_chart_cleanup, metrics_output, use_doc_ref, set, timeout, + values, wait, target_manifest, bearer_token).safe_invoke() class ApplyManifest(CliAction): def __init__( self, ctx, locations, api, disable_update_post, disable_update_pre, - enable_chart_cleanup, metrics_output, use_doc_ref, set, - tiller_host, tiller_port, tiller_namespace, timeout, values, wait, - target_manifest, bearer_token): + enable_chart_cleanup, metrics_output, use_doc_ref, set, timeout, + values, wait, target_manifest, bearer_token): super(ApplyManifest, self).__init__() self.ctx = ctx # Filename can also be a URL reference @@ -169,9 +157,6 @@ class ApplyManifest(CliAction): self.metrics_output = metrics_output self.use_doc_ref = use_doc_ref self.set = set - self.tiller_host = tiller_host - self.tiller_port = tiller_port - self.tiller_namespace = tiller_namespace self.timeout = timeout self.values = values self.wait = wait @@ -210,13 +195,10 @@ class ApplyManifest(CliAction): return if not self.ctx.obj.get('api', False): - with Tiller(tiller_host=self.tiller_host, - tiller_port=self.tiller_port, - tiller_namespace=self.tiller_namespace, - bearer_token=self.bearer_token) as tiller: + with Helm(bearer_token=self.bearer_token) as helm: try: - resp = self.handle(documents, tiller) + resp = self.handle(documents, helm) self.output(resp) finally: if self.metrics_output: @@ -235,9 +217,6 @@ class ApplyManifest(CliAction): 'disable_update_post': self.disable_update_post, 'disable_update_pre': self.disable_update_pre, 'enable_chart_cleanup': self.enable_chart_cleanup, - 'tiller_host': self.tiller_host, - 'tiller_port': self.tiller_port, - 'tiller_namespace': self.tiller_namespace, 'timeout': self.timeout, 'wait': self.wait } @@ -252,7 +231,7 @@ class ApplyManifest(CliAction): self.output(resp.get('message')) @lock_and_thread() - def handle(self, documents, tiller): + def handle(self, documents, helm): armada = Armada( documents, disable_update_pre=self.disable_update_pre, @@ -261,7 +240,7 @@ class ApplyManifest(CliAction): set_ovr=self.set, force_wait=self.wait, timeout=self.timeout, - tiller=tiller, + helm=helm, values=self.values, target_manifest=self.target_manifest) return armada.sync() diff --git a/armada/cli/test.py b/armada/cli/test.py index b8f4cea1..bdae8adc 100644 --- a/armada/cli/test.py +++ b/armada/cli/test.py @@ -21,7 +21,7 @@ from armada import const from armada.handlers.lock import lock_and_thread from armada.handlers.manifest import Manifest from armada.handlers.test import Test -from armada.handlers.tiller import Tiller +from armada.handlers.helm import Helm, HelmReleaseId from armada.utils.release import release_prefixer CONF = cfg.CONF @@ -37,7 +37,6 @@ def test(): DESC = """ This command tests deployed charts. -The tiller command uses flags to obtain information from Tiller services. The test command will run the release chart tests either via a the manifest or by targeting a release. @@ -47,7 +46,7 @@ To test Armada deployed releases: To test release: - $ armada test --release blog-1 + $ armada test --namespace blog --release blog-1 """ @@ -56,27 +55,14 @@ SHORT_DESC = "Command tests releases." @test.command(name='test', help=DESC, short_help=SHORT_DESC) @click.option('--file', help="Armada manifest.", type=str) +@click.option('--namespace', help="Helm release namespace.", type=str) @click.option('--release', help="Helm release.", type=str) -@click.option('--tiller-host', help="Tiller host IP.", default=None) -@click.option( - '--tiller-port', help="Tiller host port.", type=int, default=None) -@click.option( - '--tiller-namespace', - '-tn', - help="Tiller Namespace.", - type=str, - default=None) @click.option( '--target-manifest', help=( "The target manifest to run. Required for specifying " "which manifest to run when multiple are available."), default=None) -@click.option( - '--cleanup', - help=("Delete test pods upon completion."), - is_flag=True, - default=None) @click.option( '--enable-all', help=( @@ -87,54 +73,42 @@ SHORT_DESC = "Command tests releases." @click.option('--debug', help="Enable debug logging.", is_flag=True) @click.pass_context def test_charts( - ctx, file, release, tiller_host, tiller_port, tiller_namespace, - target_manifest, cleanup, enable_all, debug): + ctx, file, namespace, release, target_manifest, enable_all, debug): CONF.debug = debug TestChartManifest( - ctx, file, release, tiller_host, tiller_port, tiller_namespace, - target_manifest, cleanup, enable_all).safe_invoke() + ctx, file, namespace, release, target_manifest, + enable_all).safe_invoke() class TestChartManifest(CliAction): def __init__( - self, ctx, file, release, tiller_host, tiller_port, - tiller_namespace, target_manifest, cleanup, enable_all): + self, ctx, file, namespace, release, target_manifest, enable_all): super(TestChartManifest, self).__init__() self.ctx = ctx self.file = file + self.namespace = namespace self.release = release - self.tiller_host = tiller_host - self.tiller_port = tiller_port - self.tiller_namespace = tiller_namespace self.target_manifest = target_manifest - self.cleanup = cleanup self.enable_all = enable_all def invoke(self): - with Tiller(tiller_host=self.tiller_host, tiller_port=self.tiller_port, - tiller_namespace=self.tiller_namespace) as tiller: + with Helm() as helm: - self.handle(tiller) + self.handle(helm) @lock_and_thread() - def handle(self, tiller): - known_release_names = [release[0] for release in tiller.list_charts()] + def handle(self, helm): + release_ids = helm.list_release_ids() if self.release: if not self.ctx.obj.get('api', False): - test_handler = Test( - {}, self.release, tiller, cleanup=self.cleanup) + release_id = HelmReleaseId(self.namespace, self.release) + test_handler = Test({}, release_id, helm) test_handler.test_release_for_success() else: client = self.ctx.obj.get('CLIENT') - query = { - 'tiller_host': self.tiller_host, - 'tiller_port': self.tiller_port, - 'tiller_namespace': self.tiller_namespace - } - resp = client.get_test_release( - release=self.release, query=query) + resp = client.get_test_release(release=self.release) self.logger.info(resp.get('result')) self.logger.info(resp.get('message')) @@ -153,33 +127,26 @@ class TestChartManifest(CliAction): for ch in group.get(const.KEYWORD_CHARTS): chart = ch['chart'] - release_name = release_prefixer( - prefix, chart.get('release')) - if release_name in known_release_names: + release_id = HelmReleaseId( + chart['namespace'], + release_prefixer(prefix, chart['release'])) + if release_id in release_ids: test_handler = Test( chart, - release_name, - tiller, - cleanup=self.cleanup, + release_id, + helm, enable_all=self.enable_all) if test_handler.test_enabled: test_handler.test_release_for_success() else: self.logger.info( - 'Release %s not found - SKIPPING', - release_name) + 'Release %s not found - SKIPPING', release_id) else: client = self.ctx.obj.get('CLIENT') - query = { - 'tiller_host': self.tiller_host, - 'tiller_port': self.tiller_port, - 'tiller_namespace': self.tiller_namespace - } with open(self.filename, 'r') as f: - resp = client.get_test_manifest( - manifest=f.read(), query=query) + resp = client.get_test_manifest(manifest=f.read()) for test in resp.get('tests'): self.logger.info('Test State: %s', test) for item in test.get('tests').get(test): diff --git a/armada/common/policies/service.py b/armada/common/policies/service.py index 48163788..18b5f359 100644 --- a/armada/common/policies/service.py +++ b/armada/common/policies/service.py @@ -47,6 +47,14 @@ armada_policies = [ 'path': '/api/v1.0/tests/', 'method': 'POST' }]), + policy.DocumentedRuleDefault( + name=base.ARMADA % 'get_release', + check_str=base.RULE_ADMIN_VIEWER, + description='Get helm releases', + operations=[{ + 'path': '/api/v1.0/releases/', + 'method': 'GET' + }]), ] diff --git a/armada/common/policies/tiller.py b/armada/common/policies/tiller.py index fe9b2e6e..0d3a6465 100644 --- a/armada/common/policies/tiller.py +++ b/armada/common/policies/tiller.py @@ -23,14 +23,6 @@ tiller_policies = [ 'path': '/api/v1.0/status/', 'method': 'GET' }]), - policy.DocumentedRuleDefault( - name=base.TILLER % 'get_release', - check_str=base.RULE_ADMIN_VIEWER, - description='Get Tiller release', - operations=[{ - 'path': '/api/v1.0/releases/', - 'method': 'GET' - }]), ] diff --git a/armada/exceptions/armada_exceptions.py b/armada/exceptions/armada_exceptions.py index 05c6c57c..8ded780d 100644 --- a/armada/exceptions/armada_exceptions.py +++ b/armada/exceptions/armada_exceptions.py @@ -36,10 +36,10 @@ class ProtectedReleaseException(ArmadaException): `continue_processing` is False. ''' - def __init__(self, release, status): + def __init__(self, release_id, status): self._message = ( 'Armada encountered protected release {} in {} status'.format( - release, status)) + release_id, status)) super(ProtectedReleaseException, self).__init__(self._message) @@ -92,15 +92,15 @@ class WaitException(ArmadaException): class DeploymentLikelyPendingException(ArmadaException): ''' Exception that occurs when it is detected that an existing release - operation (e.g. install, update, rollback, delete) is likely still pending. + operation (e.g. install, update, delete) is likely still pending. ''' - def __init__(self, release, status, last_deployment_age, timeout): + def __init__(self, release_id, status, last_deployment_age, timeout): self._message = ( 'Existing deployment likely pending ' 'release={}, status={}, ' '(last deployment age={}s) < (chart wait timeout={}s)'.format( - release, status, last_deployment_age, timeout)) + release_id, status, last_deployment_age, timeout)) super(DeploymentLikelyPendingException, self).__init__(self._message) diff --git a/armada/exceptions/chartbuilder_exceptions.py b/armada/exceptions/chartbuilder_exceptions.py index 59282553..d183d42c 100644 --- a/armada/exceptions/chartbuilder_exceptions.py +++ b/armada/exceptions/chartbuilder_exceptions.py @@ -21,28 +21,9 @@ class ChartBuilderException(base_exception.ArmadaBaseException): message = 'An unknown Armada handler error occurred.' -class DependencyException(ChartBuilderException): - ''' - Exception that occurs when dependencies cannot be resolved. - - **Troubleshoot:** - *Coming Soon* - ''' - - def __init__(self, chart_name): - self._chart_name = chart_name - self._message = 'Failed to resolve dependencies for ' + \ - self._chart_name + '.' - - super(DependencyException, self).__init__(self._message) - - class HelmChartBuildException(ChartBuilderException): ''' Exception that occurs when Helm Chart fails to build. - - **Troubleshoot:** - *Coming Soon* ''' def __init__(self, chart_name, details): @@ -56,54 +37,3 @@ class HelmChartBuildException(ChartBuilderException): })) super(HelmChartBuildException, self).__init__(self._message) - - -class FilesLoadException(ChartBuilderException): - ''' - Exception that occurs while trying to read a file in the chart directory. - - **Troubleshoot:** - - * Ensure that the file can be encoded to utf-8 or else it cannot be parsed. - ''' - - message = ( - 'A %(clazz)s exception occurred while trying to read ' - 'file: %(file)s. Details:\n%(details)s') - - -class IgnoredFilesLoadException(ChartBuilderException): - ''' - Exception that occurs when there is an error loading files contained in - .helmignore. - - **Troubleshoot:** - *Coming Soon* - ''' - - message = 'An error occurred while loading the ignored files in \ - .helmignore' - - -class MetadataLoadException(ChartBuilderException): - ''' - Exception that occurs when metadata loading fails. - - **Troubleshoot:** - *Coming Soon* - ''' - - message = 'Failed to load metadata from chart yaml file' - - -class UnknownChartSourceException(ChartBuilderException): - '''Exception for unknown chart source type.''' - - def __init__(self, chart_name, source_type): - self._chart_name = chart_name - self._source_type = source_type - - self._message = 'Unknown source type \"' + self._source_type + '\" for \ - chart \"' + self._chart_name + '\"' - - super(UnknownChartSourceException, self).__init__(self._message) diff --git a/armada/exceptions/helm_exceptions.py b/armada/exceptions/helm_exceptions.py new file mode 100644 index 00000000..29d181f3 --- /dev/null +++ b/armada/exceptions/helm_exceptions.py @@ -0,0 +1,27 @@ +# Copyright 2021 The Armada Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from armada.exceptions.base_exception import ArmadaBaseException as ex + + +class HelmCommandException(ex): + ''' + Exception that occurs when a helm command fails. + ''' + + def __init__(self, called_process_error): + self.called_process_error = called_process_error + message = 'helm command failed: {}'.format( + self.called_process_error.stderr) + super(HelmCommandException, self).__init__(message) diff --git a/armada/exceptions/tiller_exceptions.py b/armada/exceptions/tiller_exceptions.py index bfff94e3..15d0394f 100644 --- a/armada/exceptions/tiller_exceptions.py +++ b/armada/exceptions/tiller_exceptions.py @@ -21,17 +21,6 @@ class TillerException(ex): message = 'An unknown Tiller error occurred.' -class TillerServicesUnavailableException(TillerException): - ''' - Exception for tiller service being unavailable. - - **Troubleshoot:** - *Coming Soon* - ''' - - message = 'Tiller services unavailable.' - - class ChartCleanupException(TillerException): '''Exception that occurs during chart cleanup.''' diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index 6506a49f..4dfa6c5e 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -21,11 +21,11 @@ from armada import const from armada.conf import set_current_chart from armada.exceptions import armada_exceptions from armada.exceptions import override_exceptions -from armada.exceptions import tiller_exceptions from armada.exceptions import validate_exceptions from armada.handlers import metrics from armada.handlers.chart_deploy import ChartDeploy from armada.handlers.chart_download import ChartDownload +from armada.handlers.helm import HelmReleaseId from armada.handlers.manifest import Manifest from armada.handlers.override import Override from armada.utils.release import release_prefixer @@ -43,7 +43,7 @@ class Armada(object): def __init__( self, documents, - tiller, + helm, disable_update_pre=False, disable_update_post=False, enable_chart_cleanup=False, @@ -55,18 +55,16 @@ class Armada(object): k8s_wait_attempts=1, k8s_wait_attempt_sleep=1): ''' - Initialize the Armada engine and establish a connection to Tiller. + Initialize the Armada engine. :param List[dict] documents: Armada documents. - :param tiller: Tiller instance to use. - :param bool disable_update_pre: Disable pre-update Tiller operations. - :param bool disable_update_post: Disable post-update Tiller - operations. + :param bool disable_update_pre: Disable pre-update operations. + :param bool disable_update_post: Disable post-update operations. :param bool enable_chart_cleanup: Clean up unmanaged charts. - :param bool force_wait: Force Tiller to wait until all charts are + :param bool force_wait: Force to wait until all charts are deployed, rather than using each chart's specified wait policy. - :param int timeout: Specifies overall time in seconds that Tiller - should wait for charts until timing out. + :param int timeout: Specifies overall time in seconds t to wait for + charts until timing out. :param str target_manifest: The target manifest to run. Useful for specifying which manifest to run when multiple are available. :param int k8s_wait_attempts: The number of times to attempt waiting @@ -77,7 +75,7 @@ class Armada(object): self.enable_chart_cleanup = enable_chart_cleanup self.force_wait = force_wait - self.tiller = tiller + self.helm = helm try: self.documents = Override( documents, overrides=set_ovr, @@ -90,7 +88,7 @@ class Armada(object): self.chart_download = ChartDownload() self.chart_deploy = ChartDeploy( self.manifest, disable_update_pre, disable_update_post, - k8s_wait_attempts, k8s_wait_attempt_sleep, timeout, self.tiller) + k8s_wait_attempts, k8s_wait_attempt_sleep, timeout, self.helm) def pre_flight_ops(self): """Perform a series of checks and operations to ensure proper @@ -98,10 +96,6 @@ class Armada(object): """ LOG.info("Performing pre-flight operations.") - # Ensure Tiller is available and manifest is valid - if not self.tiller.tiller_status(): - raise tiller_exceptions.TillerServicesUnavailableException() - # Clone the chart sources manifest_data = self.manifest.get(const.KEYWORD_DATA, {}) for group in manifest_data.get(const.KEYWORD_GROUPS, []): @@ -130,8 +124,6 @@ class Armada(object): # a more cleaner format self.pre_flight_ops() - known_releases = self.tiller.list_releases() - manifest_data = self.manifest.get(const.KEYWORD_DATA, {}) prefix = manifest_data.get(const.KEYWORD_PREFIX) @@ -155,8 +147,7 @@ class Armada(object): set_current_chart(chart) try: return self.chart_deploy.execute( - chart, cg_test_all_charts, prefix, known_releases, - concurrency) + chart, cg_test_all_charts, prefix, concurrency) finally: set_current_chart(None) @@ -225,20 +216,22 @@ class Armada(object): def _chart_cleanup(self, prefix, chart_groups, msg): LOG.info('Processing chart cleanup to remove unspecified releases.') - valid_releases = [] + valid_release_ids = [] for group in chart_groups: group_data = group.get(const.KEYWORD_DATA, {}) for chart in group_data.get(const.KEYWORD_CHARTS, []): chart_data = chart.get(const.KEYWORD_DATA, {}) - valid_releases.append( - release_prefixer(prefix, chart_data.get('release'))) + valid_release_ids.append( + HelmReleaseId( + chart_data['namespace'], + release_prefixer(prefix, chart_data['release']))) - actual_releases = [x.name for x in self.tiller.list_releases()] - release_diff = list(set(actual_releases) - set(valid_releases)) + actual_release_ids = self.helm.list_release_ids() + release_diff = list(set(actual_release_ids) - set(valid_release_ids)) - for release in release_diff: - if release.startswith(prefix): + for release_id in release_diff: + if release_id.name.startswith(prefix): LOG.info( - 'Purging release %s as part of chart cleanup.', release) - self.tiller.uninstall_release(release) - msg['purge'].append(release) + 'Purging release %s as part of chart cleanup.', release_id) + self.helm.uninstall_release(release_id) + msg['purge'].append('{}'.format(release_id)) diff --git a/armada/handlers/chart_delete.py b/armada/handlers/chart_delete.py index 58f2ec91..f3eb1be0 100644 --- a/armada/handlers/chart_delete.py +++ b/armada/handlers/chart_delete.py @@ -16,23 +16,23 @@ from armada import const class ChartDelete(object): - def __init__(self, chart, release_name, tiller, purge=True): + def __init__(self, chart, release_id, helm, purge=True): """Initialize a chart delete handler. :param chart: The armada chart document - :param release_name: Name of a Helm release - :param tiller: Tiller object + :param release_id: HelmReleaseId + :param helm: Helm object :param purge: Whether to purge the release :type chart: object :type release_name: str - :type tiller: Tiller object + :type helm: Helm object :type purge: bool """ self.chart = chart - self.release_name = release_name - self.tiller = tiller + self.release_id = release_id + self.helm = helm self.purge = purge self.delete_config = self.chart.get('delete', {}) # TODO(seaneagan): Consider allowing this to be a percentage of the @@ -47,5 +47,5 @@ class ChartDelete(object): def delete(self): """Delete the release associated with the chart" """ - self.tiller.uninstall_release( - self.release_name, timeout=self.get_timeout(), purge=self.purge) + self.helm.uninstall_release( + self.release_id, timeout=self.get_timeout(), purge=self.purge) diff --git a/armada/handlers/chart_deploy.py b/armada/handlers/chart_deploy.py index 644f5dcd..5bf040e7 100644 --- a/armada/handlers/chart_deploy.py +++ b/armada/handlers/chart_deploy.py @@ -12,22 +12,22 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os import time from oslo_log import log as logging -import yaml from armada import const from armada.exceptions import armada_exceptions from armada.handlers import metrics from armada.handlers.chartbuilder import ChartBuilder +from armada.handlers import helm from armada.handlers.release_diff import ReleaseDiff from armada.handlers.chart_delete import ChartDelete from armada.handlers.pre_update_actions import PreUpdateActions from armada.handlers.schema import get_schema_info from armada.handlers.test import Test from armada.handlers.wait import ChartWait -from armada.exceptions import tiller_exceptions as tiller_ex import armada.utils.release as r LOG = logging.getLogger(__name__) @@ -36,40 +36,40 @@ LOG = logging.getLogger(__name__) class ChartDeploy(object): def __init__( self, manifest, disable_update_pre, disable_update_post, - k8s_wait_attempts, k8s_wait_attempt_sleep, timeout, tiller): + k8s_wait_attempts, k8s_wait_attempt_sleep, timeout, helm): self.manifest = manifest self.disable_update_pre = disable_update_pre self.disable_update_post = disable_update_post self.k8s_wait_attempts = k8s_wait_attempts self.k8s_wait_attempt_sleep = k8s_wait_attempt_sleep self.timeout = timeout - self.tiller = tiller + self.helm = helm - def execute( - self, ch, cg_test_all_charts, prefix, known_releases, concurrency): + def execute(self, ch, cg_test_all_charts, prefix, concurrency): chart_name = ch['metadata']['name'] manifest_name = self.manifest['metadata']['name'] with metrics.CHART_HANDLE.get_context(concurrency, manifest_name, chart_name): - return self._execute( - ch, cg_test_all_charts, prefix, known_releases) + return self._execute(ch, cg_test_all_charts, prefix) - def _execute(self, ch, cg_test_all_charts, prefix, known_releases): + def _execute(self, ch, cg_test_all_charts, prefix): manifest_name = self.manifest['metadata']['name'] chart = ch[const.KEYWORD_DATA] chart_name = ch['metadata']['name'] namespace = chart.get('namespace') release = chart.get('release') release_name = r.release_prefixer(prefix, release) - LOG.info('Processing Chart, release=%s', release_name) + release_id = helm.HelmReleaseId(namespace, release_name) + source_dir = chart['source_dir'] + source_directory = os.path.join(*source_dir) + LOG.info('Processing Chart, release=%s', release_id) result = {} chart_wait = ChartWait( - self.tiller.k8s, - release_name, + self.helm.k8s, + release_id, ch, - namespace, k8s_wait_attempts=self.k8s_wait_attempts, k8s_wait_attempt_sleep=self.k8s_wait_attempt_sleep, timeout=self.timeout) @@ -77,7 +77,7 @@ class ChartDeploy(object): # Begin Chart timeout deadline deadline = time.time() + wait_timeout - old_release = self.find_chart_release(known_releases, release_name) + old_release = self.helm.release_metadata(release_id) action = metrics.ChartDeployAction.NOOP def noop(): @@ -95,20 +95,17 @@ class ChartDeploy(object): native_wait_enabled = chart_wait.is_native_enabled() - chartbuilder = ChartBuilder.from_chart_doc(ch) - new_chart = chartbuilder.get_helm_chart() + chartbuilder = ChartBuilder.from_chart_doc(ch, self.helm) - if status == const.STATUS_DEPLOYED: + if status == helm.STATUS_DEPLOYED: # indicate to the end user what path we are taking - LOG.info( - "Existing release %s found in namespace %s", release_name, - namespace) + LOG.info("Existing release %s found", release_id) # extract the installed chart and installed values from the # latest release so we can compare to the intended state - old_chart = old_release.chart - old_values_string = old_release.config.raw + old_chart = old_release['chart'] + old_values = old_release.get('config', {}) upgrade = chart.get('upgrade', {}) options = upgrade.get('options', {}) @@ -122,7 +119,6 @@ class ChartDeploy(object): disable_hooks = no_hooks_location.get('no_hooks', False) force = options.get('force', False) - recreate_pods = options.get('recreate_pods', False) if upgrade: upgrade_pre = upgrade.get('pre', {}) @@ -136,15 +132,8 @@ class ChartDeploy(object): 'Post upgrade actions are ignored by Armada' 'and will not affect deployment.') - try: - old_values = yaml.safe_load(old_values_string) - except yaml.YAMLError: - chart_desc = '{} (previously deployed)'.format( - old_chart.metadata.name) - raise armada_exceptions.\ - InvalidOverrideValuesYamlException(chart_desc) - LOG.info('Checking for updates to chart release inputs.') + new_chart = chartbuilder.get_helm_chart(release_id, values) diff = self.get_diff(old_chart, old_values, new_chart, values) if not diff: @@ -152,34 +141,29 @@ class ChartDeploy(object): else: action = metrics.ChartDeployAction.UPGRADE LOG.info("Found updates to chart release inputs") - LOG.debug("Release=%s, diff=%s", release_name, diff) + LOG.debug("Release=%s, diff=%s", release_id, diff) result['diff'] = {chart['release']: str(diff)} def upgrade(): # do actual update timer = int(round(deadline - time.time())) - PreUpdateActions(self.tiller.k8s).execute( + PreUpdateActions(self.helm.k8s).execute( pre_actions, release, namespace, chart, disable_hooks, values, timer) LOG.info( - "Upgrading release %s in namespace %s, wait=%s, " - "timeout=%ss", release_name, namespace, - native_wait_enabled, timer) - tiller_result = self.tiller.update_release( - new_chart, - release_name, - namespace, + "Upgrading release=%s, wait=%s, " + "timeout=%ss", release_id, native_wait_enabled, timer) + self.helm.upgrade_release( + source_directory, + release_id, disable_hooks=disable_hooks, - values=yaml.safe_dump(values), + values=values, wait=native_wait_enabled, timeout=timer, - force=force, - recreate_pods=recreate_pods) + force=force) - LOG.info( - 'Upgrade completed with results from Tiller: %s', - tiller_result.__dict__) - result['upgrade'] = release_name + LOG.info('Upgrade completed') + result['upgrade'] = release_id deploy = upgrade else: @@ -187,28 +171,24 @@ class ChartDeploy(object): def install(): timer = int(round(deadline - time.time())) LOG.info( - "Installing release %s in namespace %s, wait=%s, " - "timeout=%ss", release_name, namespace, - native_wait_enabled, timer) - tiller_result = self.tiller.install_release( - new_chart, - release_name, - namespace, - values=yaml.safe_dump(values), + "Installing release=%s, wait=%s, " + "timeout=%ss", release_id, native_wait_enabled, timer) + self.helm.install_release( + source_directory, + release_id, + values=values, wait=native_wait_enabled, timeout=timer) - LOG.info( - 'Install completed with results from Tiller: %s', - tiller_result.__dict__) - result['install'] = release_name + LOG.info('Install completed') + result['install'] = release_id # Check for release with status other than DEPLOYED if status: - if status != const.STATUS_FAILED: + if status != helm.STATUS_FAILED: LOG.warn( 'Unexpected release status encountered ' - 'release=%s, status=%s', release_name, status) + 'release=%s, status=%s', release_id, status) # Make best effort to determine whether a deployment is # likely pending, by checking if the last deployment @@ -222,7 +202,7 @@ class ChartDeploy(object): deploy = noop deadline = deadline - last_deployment_age else: - # Release is likely stuck in an unintended (by tiller) + # Release is likely stuck in an unintended # state. Log and continue on with remediation steps # below. LOG.info( @@ -231,7 +211,7 @@ class ChartDeploy(object): '(chart wait timeout=%ss)', release, status, last_deployment_age, wait_timeout) res = self.purge_release( - chart, release_name, status, manifest_name, + chart, release_id, status, manifest_name, chart_name, result) if isinstance(res, dict): if 'protected' in res: @@ -242,7 +222,7 @@ class ChartDeploy(object): # The chart is in Failed state, hence we purge # the chart and attempt to install it again. res = self.purge_release( - chart, release_name, status, manifest_name, chart_name, + chart, release_id, status, manifest_name, chart_name, result) if isinstance(res, dict): if 'protected' in res: @@ -269,22 +249,19 @@ class ChartDeploy(object): last_test_passed = old_release and r.get_last_test_result(old_release) test_handler = Test( - chart, - release_name, - self.tiller, - cg_test_charts=cg_test_all_charts) + chart, release_id, self.helm, cg_test_charts=cg_test_all_charts) run_test = test_handler.test_enabled and ( just_deployed or not last_test_passed) if run_test: with metrics.CHART_TEST.get_context(test_handler.timeout, manifest_name, chart_name): - self._test_chart(release_name, test_handler) + self._test_chart(test_handler) return result def purge_release( - self, chart, release_name, status, manifest_name, chart_name, + self, chart, release_id, status, manifest_name, chart_name, result): protected = chart.get('protected', {}) if protected: @@ -293,41 +270,27 @@ class ChartDeploy(object): LOG.warn( 'Release %s is `protected`, ' 'continue_processing=True. Operator must ' - 'handle %s release manually.', release_name, status) - result['protected'] = release_name + 'handle %s release manually.', release_id, status) + result['protected'] = release_id return result else: LOG.error( 'Release %s is `protected`, ' - 'continue_processing=False.', release_name) + 'continue_processing=False.', release_id) raise armada_exceptions.ProtectedReleaseException( - release_name, status) + release_id, status) else: # Purge the release with metrics.CHART_DELETE.get_context(manifest_name, chart_name): LOG.info( - 'Purging release %s with status %s', release_name, status) - chart_delete = ChartDelete(chart, release_name, self.tiller) + 'Purging release %s with status %s', release_id, status) + chart_delete = ChartDelete(chart, release_id, self.helm) chart_delete.delete() - result['purge'] = release_name + result['purge'] = release_id - def _test_chart(self, release_name, test_handler): - success = test_handler.test_release_for_success() - if not success: - raise tiller_ex.TestFailedException(release_name) + def _test_chart(self, test_handler): + test_handler.test_release() def get_diff(self, old_chart, old_values, new_chart, values): return ReleaseDiff(old_chart, old_values, new_chart, values).get_diff() - - def find_chart_release(self, known_releases, release_name): - ''' - Find a release given a list of known_releases and a release name - ''' - for release in known_releases: - if release.name == release_name: - return release - LOG.info( - "known: %s, release_name: %s", - list(map(lambda r: r.name, known_releases)), release_name) - return None diff --git a/armada/handlers/chartbuilder.py b/armada/handlers/chartbuilder.py index 35a9a9ee..7952b554 100644 --- a/armada/handlers/chartbuilder.py +++ b/armada/handlers/chartbuilder.py @@ -13,20 +13,14 @@ # limitations under the License. import os -import re +from pathlib import Path +from shutil import rmtree -from google.protobuf.any_pb2 import Any -from hapi.chart.chart_pb2 import Chart -from hapi.chart.config_pb2 import Config -from hapi.chart.metadata_pb2 import Metadata -from hapi.chart.template_pb2 import Template from oslo_config import cfg from oslo_log import log as logging -import yaml from armada import const from armada.exceptions import chartbuilder_exceptions -from armada.handlers.schema import get_schema_info LOG = logging.getLogger(__name__) @@ -36,11 +30,11 @@ CONF = cfg.CONF class ChartBuilder(object): ''' This class handles taking chart intentions as a parameter and turning those - into proper ``protoc`` Helm charts that can be pushed to Tiller. + into proper Helm chart metadata. ''' @classmethod - def from_chart_doc(cls, chart): + def from_chart_doc(cls, chart, helm): ''' Returns a ChartBuilder defined by an Armada Chart doc. @@ -49,294 +43,71 @@ class ChartBuilder(object): name = chart['metadata']['name'] chart_data = chart[const.KEYWORD_DATA] - source_dir = chart_data.get('source_dir') + source_dir = chart_data['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 = [] + # Ensure `charts` dir exists and is empty. + charts_dir = os.path.join(source_directory, 'charts') + charts_path = Path(charts_dir) + if charts_path.is_dir(): + # NOTE: Ideally we would only delete the subcharts being + # overridden, and leave the others in place, but we delete all + # for backward compatibility with the Helm 2 based Armada. + (rmtree(d) for d in charts_path.iterdir() if d.is_dir()) + else: + if charts_path.exists(): + # NOTE: Ideally we would throw an error if `charts` is a + # non-directory, but we don't for backward compatibility + # with the Helm 2 based Armada. + charts_path.unlink() + + charts_path.mkdir() + + # Add symlinks to dependencies into `charts` dir. for chart_dep in dependencies: - builder = ChartBuilder.from_chart_doc(chart_dep) - dependency_builders.append(builder) + # Handle any recursive dependencies. + ChartBuilder.from_chart_doc(chart_dep, helm) - return cls( - name, - source_directory, - dependency_builders, - fix_tpl_name=fix_tpl_name) + dep_data = chart_dep[const.KEYWORD_DATA] + dep_source_dir = dep_data['source_dir'] + dep_source_directory = os.path.join(*dep_source_dir) + dep_charts_yaml = helm.show_chart(dep_source_directory) + dep_name = dep_charts_yaml['name'] + dep_target_directory = os.path.join(charts_dir, dep_name) + Path(dep_target_directory).symlink_to(dep_source_directory) - return cls.from_source( - name, source_directory, fix_tpl_name=fix_tpl_name) + return cls(name, source_directory, helm) - @classmethod - def from_source(cls, name, source_directory, fix_tpl_name=False): - ''' - 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, fix_tpl_name=fix_tpl_name) - dependency_builders.append(builder) - - return cls( - name, - source_directory, - dependency_builders, - fix_tpl_name=fix_tpl_name) - - def __init__( - self, name, source_directory, dependency_builders, - fix_tpl_name=False): + def __init__(self, name, source_directory, helm): ''' :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. + :param helm: Helm client to calculate the helm chart object. ''' self.name = name self.source_directory = source_directory - self.dependency_builders = dependency_builders - self.fix_tpl_name = fix_tpl_name + self.helm = helm - # cache for generated protoc chart object + # cache for generated chart object self._helm_chart = None - # load ignored files from .helmignore if present - self.ignored_files = self.get_ignored_files() - - def get_ignored_files(self): - '''Load files to ignore from .helmignore if present.''' - try: - ignored_files = [] - if os.path.exists(os.path.join(self.source_directory, - '.helmignore')): - with open(os.path.join(self.source_directory, - '.helmignore')) as f: - ignored_files = f.readlines() - return [filename.strip() for filename in ignored_files] - except Exception: - raise chartbuilder_exceptions.IgnoredFilesLoadException() - - def ignore_file(self, filename): - '''Returns whether a given ``filename`` should be ignored. - - :param filename: Filename to compare against list of ignored files. - :returns: True if file matches an ignored file wildcard or exact name, - False otherwise. - ''' - for ignored_file in self.ignored_files: - if (ignored_file.startswith('*') - and filename.endswith(ignored_file.strip('*'))): - return True - elif ignored_file == filename: - return True - return False - - def get_metadata(self): - '''Extract metadata from Chart.yaml to construct an instance of - :class:`hapi.chart.metadata_pb2.Metadata`. - ''' - try: - with open(os.path.join(self.source_directory, 'Chart.yaml')) as f: - chart_yaml = yaml.safe_load(f.read().encode('utf-8')) - - except Exception: - raise chartbuilder_exceptions.MetadataLoadException() - - # Construct Metadata object. - return Metadata( - description=chart_yaml.get('description'), - name=chart_yaml.get('name'), - version=chart_yaml.get('version')) - - def get_files(self): - ''' - Return (non-template) files in this chart. - - Non-template files include all files *except* Chart.yaml, values.yaml, - values.toml, and any file nested under charts/ or templates/. The only - exception to this rule is charts/.prov - - The class :class:`google.protobuf.any_pb2.Any` is wrapped around - each file as that is what Helm uses. - - For more information, see: - https://github.com/kubernetes/helm/blob/fa06dd176dbbc247b40950e38c09f978efecaecc/pkg/chartutil/load.go - - :returns: List of non-template files. - :rtype: List[:class:`google.protobuf.any_pb2.Any`] - ''' - - files_to_ignore = ['Chart.yaml', 'values.yaml', 'values.toml'] - non_template_files = [] - - def _append_file_to_result(root, rel_folder_path, file): - abspath = os.path.abspath(os.path.join(root, file)) - relpath = os.path.join(rel_folder_path, file) - - encodings = ('utf-8', 'latin1') - unicode_errors = [] - - for encoding in encodings: - try: - with open(abspath, 'r') as f: - file_contents = f.read().encode(encoding) - except OSError as e: - LOG.debug( - 'Failed to open and read file %s in the helm ' - 'chart directory.', abspath) - raise chartbuilder_exceptions.FilesLoadException( - file=abspath, details=e) - except UnicodeError as e: - LOG.debug( - 'Attempting to read %s using encoding %s.', abspath, - encoding) - msg = "(encoding=%s) %s" % (encoding, str(e)) - unicode_errors.append(msg) - else: - break - - if len(unicode_errors) == 2: - LOG.debug( - 'Failed to read file %s in the helm chart directory.' - ' Ensure that it is encoded using utf-8.', abspath) - raise chartbuilder_exceptions.FilesLoadException( - file=abspath, - clazz=unicode_errors[0].__class__.__name__, - details='\n'.join(e for e in unicode_errors)) - - non_template_files.append( - Any(type_url=relpath, value=file_contents)) - - for root, dirs, files in os.walk(self.source_directory): - relfolder = os.path.split(root)[-1] - rel_folder_path = os.path.relpath(root, self.source_directory) - - if not any(root.startswith(os.path.join(self.source_directory, x)) - for x in ['templates', 'charts']): - for file in files: - if (file not in files_to_ignore - and file not in non_template_files): - _append_file_to_result(root, rel_folder_path, file) - elif relfolder == 'charts' and '.prov' in files: - _append_file_to_result(root, rel_folder_path, '.prov') - - return non_template_files - - def get_values(self): - '''Return the chart's (default) values.''' - - # create config object representing unmarshaled values.yaml - if os.path.exists(os.path.join(self.source_directory, 'values.yaml')): - with open(os.path.join(self.source_directory, 'values.yaml')) as f: - raw_values = f.read() - else: - LOG.warn( - "No values.yaml in %s, using empty values", - self.source_directory) - raw_values = '' - - return Config(raw=raw_values) - - def get_templates(self): - '''Return all the chart templates. - - 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.name - 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(tpl_dir, topdown=True): - for tpl_file in files: - tname = os.path.relpath( - os.path.join(root, tpl_file), - # 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 - - with open(os.path.join(root, tpl_file)) as f: - templates.append( - Template(name=tname, data=f.read().encode())) - - return templates - - def get_helm_chart(self): + # We do a dry-run upgrade here to get the helm chart metadata. + # Ideally helm would support an explicit machine readable way to + # get that data so we don't need the dry run upgrade which could + # fail for other reasons than not being able to get the chart + # metadata, see: + # https://github.com/helm/helm/issues/9968 + def get_helm_chart(self, release_id, values): '''Return a Helm chart object. - - Constructs a :class:`hapi.chart.chart_pb2.Chart` object from the - ``chart`` intentions, including all dependencies. ''' - if not self._helm_chart: - self._helm_chart = self._get_helm_chart() - - return self._helm_chart - - def _get_helm_chart(self): - LOG.info( + LOG.debug( "Building chart %s from path %s", self.name, self.source_directory) - dependencies = [] - for dep_builder in self.dependency_builders: - LOG.info( - "Building dependency chart %s for chart %s.", dep_builder.name, - self.name) - try: - dependencies.append(dep_builder.get_helm_chart()) - except Exception: - raise chartbuilder_exceptions.DependencyException(self.name) - try: - helm_chart = Chart( - metadata=self.get_metadata(), - templates=self.get_templates(), - dependencies=dependencies, - values=self.get_values(), - files=self.get_files()) + result = self.helm.upgrade_release( + self.source_directory, release_id, values=values, dry_run=True) + return result['chart'] except Exception as e: raise chartbuilder_exceptions.HelmChartBuildException( self.name, details=e) - - return helm_chart - - def dump(self): - '''Dumps a chart object as a serialized string so that we can perform a - diff. - - It recurses into dependencies. - ''' - return self.get_helm_chart().SerializeToString() diff --git a/armada/handlers/helm.py b/armada/handlers/helm.py new file mode 100644 index 00000000..69132a51 --- /dev/null +++ b/armada/handlers/helm.py @@ -0,0 +1,245 @@ +# Copyright 2021 The Armada Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from base64 import b64decode +import gzip +import io +import json as JSON +import subprocess # nosec +import tempfile +from typing import NamedTuple + +from oslo_config import cfg +from oslo_log import log as logging +import yaml + +from armada.exceptions.helm_exceptions import HelmCommandException +from armada.handlers.k8s import K8s + +CONF = cfg.CONF +LOG = logging.getLogger(__name__) + +DEFAULT_HELM_TIMEOUT = 300 + +STATUS_DEPLOYED = 'deployed' +STATUS_FAILED = 'failed' + + +class Helm(object): + ''' + Helm CLI handler + ''' + + def __init__(self, bearer_token=None): + self.bearer_token = bearer_token + + # init k8s connectivity + self.k8s = K8s(bearer_token=self.bearer_token) + + def _run(self, sub_command, args, json=True, timeout=None): + if isinstance(sub_command, str): + sub_command = [sub_command] + command = ['helm'] + sub_command + if json: + command = command + ['--output', 'json'] + command = command + args + LOG.info('Running command=%s', command) + try: + result = subprocess.run( # nosec + command, check=True, universal_newlines=True, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + timeout=timeout) + except subprocess.CalledProcessError as e: + raise HelmCommandException(e) + + if json: + return JSON.loads(result.stdout) + return result.stdout + + def list_releases(self): + return self._run( + 'ls', ['--all-namespaces', '--all'], timeout=DEFAULT_HELM_TIMEOUT) + + def list_release_ids(self): + return [ + HelmReleaseId(r.namespace, r.name) for r in self.list_releases() + ] + + def install_release( + self, + chart, + release_id, + values=None, + wait=False, + dry_run=False, + timeout=None): + timeout = self._check_timeout(wait, timeout) + + args = [ + release_id.name, + chart, + '--namespace', + release_id.namespace, + '--create-namespace', + '--timeout', + '{}s'.format(timeout), + # TODO: (also for upgrade_release) This is for backward + # compatibility with helm 2 based Armada. We may want to consider + # making this configurable and/or defaulting to enabling. + '--disable-openapi-validation' + ] + if wait: + args = args + ['--wait'] + if dry_run: + args = args + ['--dry-run'] + + with _TempValuesFile(values) as values_file: + args = args + ['--values', values_file.file.name] + return self._run('install', args, timeout=timeout) + + def upgrade_release( + self, + chart, + release_id, + disable_hooks=False, + values=None, + wait=False, + dry_run=False, + timeout=None, + force=False): + timeout = self._check_timeout(wait, timeout) + + args = [ + release_id.name, chart, '--namespace', release_id.namespace, + '--timeout', '{}s'.format(timeout), '--disable-openapi-validation' + ] + if disable_hooks: + args = args + ['--no-hooks'] + if force: + args = args + ['--force'] + if wait: + args = args + ['--wait'] + if dry_run: + args = args + ['--dry-run'] + + with _TempValuesFile(values) as values_file: + args = args + ['--values', values_file.file.name] + return self._run('upgrade', args, timeout=timeout) + + def test_release(self, release_id, timeout=DEFAULT_HELM_TIMEOUT): + return self._run( + 'test', [ + release_id.name, '--namespace', release_id.namespace, + '--timeout', '{}s'.format(timeout) + ], + json=False, + timeout=timeout) + + def release_status(self, release_id, version=None): + args = [release_id.name, '--namespace', release_id.namespace] + if version is not None: + args = args + ['--version', version] + + try: + return self._run('status', args) + except HelmCommandException as e: + stderr = e.called_process_error.stderr.strip() + if 'Error: release: not found' == stderr: + return None + raise + + # Ideally we could just use `helm status`, but this is missing the + # chart metadata which we use for release diffing: + # https://github.com/helm/helm/issues/9968 + # + # So instead we access the helm release metadata secret directly + # as describe here: + # https://gist.github.com/DzeryCZ/c4adf39d4a1a99ae6e594a183628eaee + def release_metadata(self, release_id, version=None): + if version is None: + # determine latest version + release = self.release_status(release_id) + if release is None: + return None + version = release['version'] + secret_name = 'sh.helm.release.v1.{}.v{}'.format( + release_id.name, version) + secret_namespace = release_id.namespace + secret = self.k8s.read_namespaced_secret(secret_name, secret_namespace) + raw_data = secret.data['release'] + k8s_data = b64decode(raw_data) + helm_data = b64decode(k8s_data) + helm_data_file_handle = io.BytesIO(helm_data) + helm_json = gzip.GzipFile(fileobj=helm_data_file_handle).read() + return JSON.loads(helm_json) + + def uninstall_release( + self, + release_id, + disable_hooks=False, + purge=True, + timeout=DEFAULT_HELM_TIMEOUT): + + args = [release_id.name, '--namespace', release_id.namespace] + if not purge: + args = args + ['--keep-history'] + if disable_hooks: + args = args + ['--no-hooks'] + return self._run('uninstall', args, json=False, timeout=timeout) + + def show_chart(self, chart_dir): + + output = self._run(['show', 'chart'], [chart_dir], json=False) + return yaml.safe_load(output) + + def _check_timeout(self, wait, timeout): + if timeout is None or timeout <= 0: + timeout = DEFAULT_HELM_TIMEOUT + if wait: + LOG.warn( + 'Helm timeout is invalid or unspecified, ' + 'using default %ss.', timeout) + return timeout + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + pass + + +class _TempValuesFile(): + def __init__(self, values): + self.values = values + self.file = tempfile.NamedTemporaryFile( + mode='w', prefix='armada_values', suffix='.yaml') + + def __enter__(self): + self.file.__enter__() + values_content = yaml.safe_dump(self.values) + self.file.write(values_content) + self.file.flush() + return self + + def __exit__(self, exc_type, exc_value, traceback): + return self.file.__exit__(exc_type, exc_value, traceback) + + +class HelmReleaseId(NamedTuple('HelmReleaseId', [('namespace', str), + ('name', str)])): + """Represents a helm release id.""" + __slots__ = () + + def __str__(self): + return '{}/{}'.format(self.namespace, self.name) diff --git a/armada/handlers/k8s.py b/armada/handlers/k8s.py index 31b00c70..439b047d 100644 --- a/armada/handlers/k8s.py +++ b/armada/handlers/k8s.py @@ -287,6 +287,16 @@ class K8s(object): return self.apps_v1_api.delete_namespaced_daemon_set( name, namespace, body) + def read_namespaced_secret(self, name, namespace="default", **kwargs): + ''' + :param namespace: namespace of the Pod + :param label_selector: filters Pods by label + + This will return a list of objects req namespace + ''' + + return self.client.read_namespaced_secret(name, namespace, **kwargs) + def wait_for_pod_redeployment(self, old_pod_name, namespace): ''' :param old_pod_name: name of pods diff --git a/armada/handlers/lock.py b/armada/handlers/lock.py index bd99d384..23221f48 100644 --- a/armada/handlers/lock.py +++ b/armada/handlers/lock.py @@ -23,7 +23,7 @@ from oslo_config import cfg from oslo_log import log as logging from armada.handlers.k8s import K8s -from armada.handlers.tiller import Tiller +from armada.handlers.helm import Helm CONF = cfg.CONF @@ -54,17 +54,17 @@ def lock_and_thread(lock_name="lock"): @functools.wraps(func) def func_wrapper(*args, **kwargs): bearer_token = None - found_tiller = False + found_helm = False for arg in args: - if type(arg) == Tiller: + if type(arg) == Helm: bearer_token = arg.bearer_token - found_tiller = True + found_helm = True - # we did not find a Tiller object to extract a bearer token from + # we did not find a Helm object to extract a bearer token from # log this to assist with potential debugging in the future - if not found_tiller: + if not found_helm: LOG.info( - "no Tiller object found in parameters of function " + "no Helm object found in parameters of function " "decorated by lock_and_thread, this might create " "authentication issues in Kubernetes clusters with " "external auth backend") diff --git a/armada/handlers/release_diff.py b/armada/handlers/release_diff.py index 7c5321dd..e376c6b2 100644 --- a/armada/handlers/release_diff.py +++ b/armada/handlers/release_diff.py @@ -13,9 +13,6 @@ # limitations under the License. from deepdiff import DeepDiff -import yaml - -from armada.exceptions import armada_exceptions class ReleaseDiff(object): @@ -60,51 +57,10 @@ class ReleaseDiff(object): :rtype: dict ''' - old_input = self.make_release_input( - self.old_chart, self.old_values, 'previously deployed') - new_input = self.make_release_input( - self.new_chart, self.new_values, 'currently being deployed') + old_input = self.make_release_input(self.old_chart, self.old_values) + new_input = self.make_release_input(self.new_chart, self.new_values) return DeepDiff(old_input, new_input, view='tree') - def make_release_input(self, chart, values, desc): - return {'chart': self.make_chart_dict(chart, desc), 'values': values} - - def make_chart_dict(self, chart, desc): - try: - default_values = yaml.safe_load(chart.values.raw) - except yaml.YAMLError: - chart_desc = '{} ({})'.format(chart.metadata.name, desc) - raise armada_exceptions.InvalidValuesYamlException(chart_desc) - files = {f.type_url: f.value for f in chart.files} - - # 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)) - for d in chart.dependencies - } - - return { - # TODO(seaneagan): Are there use cases to include other - # `chart.metadata` (Chart.yaml) fields? If so, could include option - # under `upgrade` key in armada chart schema for this. Or perhaps - # can even add `upgrade.always` there to handle dynamic things - # used in charts like dates, environment variables, etc. - 'name': chart.metadata.name, - 'values': default_values, - 'files': files, - 'templates': templates, - 'dependencies': dependencies - } + def make_release_input(self, chart, values): + return {'chart': chart, 'values': values} diff --git a/armada/handlers/test.py b/armada/handlers/test.py index e426cb27..4972213f 100644 --- a/armada/handlers/test.py +++ b/armada/handlers/test.py @@ -16,43 +16,36 @@ from oslo_log import log as logging from armada import const from armada.handlers.wait import get_wait_labels +from armada.exceptions.helm_exceptions import HelmCommandException from armada.utils.release import label_selectors -from armada.utils.helm import get_test_suite_run_success, is_test_pod +from armada.utils.helm import is_test_pod LOG = logging.getLogger(__name__) class Test(object): def __init__( - self, - chart, - release_name, - tiller, - cg_test_charts=None, - cleanup=None, + self, chart, release_id, helm, cg_test_charts=None, enable_all=False): """Initialize a test handler to run Helm tests corresponding to a release. :param chart: The armada chart document - :param release_name: Name of a Helm release - :param tiller: Tiller object + :param release_id: Id of a Helm release + :param helm: helm object :param cg_test_charts: Chart group `test_charts` key - :param cleanup: Triggers cleanup; overrides `test.options.cleanup` :param enable_all: Run tests regardless of the value of `test.enabled` :type chart: dict - :type release_name: str - :type tiller: Tiller object + :type release_id: HelmReleaseId + :type helm: helm object :type cg_test_charts: bool - :type cleanup: bool :type enable_all: bool """ self.chart = chart - self.release_name = release_name - self.tiller = tiller - self.cleanup = cleanup + self.release_id = release_id + self.helm = helm self.k8s_timeout = const.DEFAULT_K8S_TIMEOUT test_values = self.chart.get('test', None) @@ -76,28 +69,12 @@ class Test(object): self.test_enabled = test_values - # NOTE: Use old, default cleanup value (i.e. True) if none is - # provided. - if self.cleanup is None: - self.cleanup = True - elif test_values: test_enabled_opt = test_values.get('enabled') if test_enabled_opt is not None: self.test_enabled = test_enabled_opt - # NOTE(drewwalters96): `self.cleanup`, the cleanup value provided - # by the API/CLI, takes precedence over the chart value - # `test.cleanup`. - if self.cleanup is None: - test_options = test_values.get('options', {}) - self.cleanup = test_options.get('cleanup', False) - self.timeout = test_values.get('timeout', self.timeout) - else: - # Default cleanup value - if self.cleanup is None: - self.cleanup = False if enable_all: self.test_enabled = True @@ -106,10 +83,22 @@ class Test(object): """Run the Helm tests corresponding to a release for success (i.e. exit code 0). + :return: Helm test suite run result + """ + try: + self.test_release() + except HelmCommandException: + return False + return True + + def test_release(self): + """Run the Helm tests corresponding to a release for success (i.e. exit + code 0). + :return: Helm test suite run result """ LOG.info( - 'RUNNING: %s tests with timeout=%ds', self.release_name, + 'RUNNING: %s tests with timeout=%ds', self.release_id, self.timeout) try: @@ -117,18 +106,9 @@ class Test(object): except Exception: LOG.exception( "Exception when deleting test pods for release: %s", - self.release_name) + self.release_id) - test_suite_run = self.tiller.test_release( - self.release_name, timeout=self.timeout, cleanup=self.cleanup) - - success = get_test_suite_run_success(test_suite_run) - if success: - LOG.info('PASSED: %s', self.release_name) - else: - LOG.info('FAILED: %s', self.release_name) - - return success + self.helm.test_release(self.release_id, timeout=self.timeout) def delete_test_pods(self): """Deletes any existing test pods for the release, as identified by the @@ -151,7 +131,7 @@ class Test(object): 'timeout_seconds': self.k8s_timeout } - pod_list = self.tiller.k8s.client.list_namespaced_pod(**list_args) + pod_list = self.helm.k8s.client.list_namespaced_pod(**list_args) test_pods = [pod for pod in pod_list.items if is_test_pod(pod)] if test_pods: @@ -162,5 +142,5 @@ class Test(object): for test_pod in test_pods: pod_name = test_pod.metadata.name LOG.info('Deleting existing test pod: %s', pod_name) - self.tiller.k8s.delete_pod_action( + self.helm.k8s.delete_pod_action( pod_name, namespace, timeout=self.k8s_timeout) diff --git a/armada/handlers/tiller.py b/armada/handlers/tiller.py index fdae465f..9226de77 100644 --- a/armada/handlers/tiller.py +++ b/armada/handlers/tiller.py @@ -31,7 +31,6 @@ from armada import const from armada.exceptions import tiller_exceptions as ex from armada.handlers.k8s import K8s from armada.utils import helm -from armada.utils.release import get_release_status TILLER_VERSION = b'2.16.9' GRPC_EPSILON = 60 @@ -268,7 +267,7 @@ class Tiller(object): if latest_versions[r.name] == r.version: LOG.debug( 'Found release %s, version %s, status: %s', r.name, - r.version, get_release_status(r)) + r.version, r.info.status) latest_releases.append(r) return latest_releases diff --git a/armada/handlers/wait.py b/armada/handlers/wait.py index 54b417ed..5fcfa14f 100644 --- a/armada/handlers/wait.py +++ b/armada/handlers/wait.py @@ -46,15 +46,14 @@ def get_wait_labels(chart): # TODO: Validate this object up front in armada validate flow. class ChartWait(): def __init__( - self, k8s, release_name, chart, namespace, k8s_wait_attempts, + self, k8s, release_id, chart, k8s_wait_attempts, k8s_wait_attempt_sleep, timeout): self.k8s = k8s - self.release_name = release_name + self.release_id = release_id self.chart = chart chart_data = self.chart[const.KEYWORD_DATA] self.chart_data = chart_data self.wait_config = self.chart_data.get('wait', {}) - self.namespace = namespace self.k8s_wait_attempts = max(k8s_wait_attempts, 1) self.k8s_wait_attempt_sleep = max(k8s_wait_attempt_sleep, 1) @@ -276,8 +275,8 @@ class ResourceWait(ABC): LOG.info( "Waiting for resource type=%s, namespace=%s labels=%s " "required=%s%s for %ss", self.resource_type, - self.chart_wait.namespace, self.label_selector, self.required, - min_ready_msg, timeout) + self.chart_wait.release_id.namespace, self.label_selector, + self.required, min_ready_msg, timeout) if not self.label_selector: LOG.warn( '"label_selector" not specified, waiting with no labels ' @@ -336,7 +335,7 @@ class ResourceWait(ABC): error = ( "Timed out waiting for resource type={}, namespace={}, " "labels={}".format( - self.resource_type, self.chart_wait.namespace, + self.resource_type, self.chart_wait.release_id.namespace, self.label_selector)) LOG.error(error) raise k8s_exceptions.KubernetesWatchTimeoutException(error) @@ -360,7 +359,7 @@ class ResourceWait(ABC): error = ( 'Timed out waiting for {}s (namespace={}, labels=({})). {}'. format( - self.resource_type, self.chart_wait.namespace, + self.resource_type, self.chart_wait.release_id.namespace, self.label_selector, details)) LOG.error(error) raise k8s_exceptions.KubernetesWatchTimeoutException(error) @@ -375,14 +374,15 @@ class ResourceWait(ABC): ''' LOG.debug( 'Starting to wait on: namespace=%s, resource type=%s, ' - 'label_selector=(%s), timeout=%s', self.chart_wait.namespace, - self.resource_type, self.label_selector, timeout) + 'label_selector=(%s), timeout=%s', + self.chart_wait.release_id.namespace, self.resource_type, + self.label_selector, timeout) ready = {} modified = set() found_resources = False kwargs = { - 'namespace': self.chart_wait.namespace, + 'namespace': self.chart_wait.release_id.namespace, 'label_selector': self.label_selector, 'timeout_seconds': timeout } @@ -420,8 +420,8 @@ class ResourceWait(ABC): 'Watch event: type=%s, name=%s, namespace=%s, ' 'resource_version=%s') LOG.debug( - msg, event_type, resource_name, self.chart_wait.namespace, - resource_version) + msg, event_type, resource_name, + self.chart_wait.release_id.namespace, resource_version) if event_type in {'ADDED', 'MODIFIED'}: found_resources = True @@ -491,9 +491,6 @@ class PodWait(ResourceWait): return 'owned by job' else: # Exclude all pods with an owner (only include raw pods) - # TODO: In helm 3, all resources will likely have the release CR as - # an owner, so this will need to be updated to not exclude pods - # directly owned by the release. if has_owner(pod): return 'owned by another resource' diff --git a/armada/schemas/armada-chart-schema-v1.yaml b/armada/schemas/armada-chart-schema-v1.yaml index 148850fc..9de5e327 100644 --- a/armada/schemas/armada-chart-schema-v1.yaml +++ b/armada/schemas/armada-chart-schema-v1.yaml @@ -162,8 +162,6 @@ data: properties: force: type: boolean - recreate_pods: - type: boolean additionalProperties: false required: - no_hooks diff --git a/armada/schemas/armada-chart-schema-v2.yaml b/armada/schemas/armada-chart-schema-v2.yaml index 2fa36995..48ea3a33 100644 --- a/armada/schemas/armada-chart-schema-v2.yaml +++ b/armada/schemas/armada-chart-schema-v2.yaml @@ -149,8 +149,6 @@ data: properties: force: type: boolean - recreate_pods: - type: boolean no_hooks: type: boolean additionalProperties: false diff --git a/armada/tests/unit/api/test_armada_controller.py b/armada/tests/unit/api/test_armada_controller.py index 74691943..74c39986 100644 --- a/armada/tests/unit/api/test_armada_controller.py +++ b/armada/tests/unit/api/test_armada_controller.py @@ -29,11 +29,11 @@ CONF = cfg.CONF @mock.patch.object( armada_api.Apply, 'handle', armada_api.Apply.handle.__wrapped__) class ArmadaControllerTest(base.BaseControllerTest): - @mock.patch.object(api, 'Tiller') + @mock.patch.object(api, 'Helm') @mock.patch.object(armada_api, 'Armada') @mock.patch.object(armada_api, 'ReferenceResolver') def test_armada_apply_resource( - self, mock_resolver, mock_armada, mock_tiller): + self, mock_resolver, mock_armada, mock_helm): """Tests the POST /api/v1.0/apply endpoint.""" rules = {'armada:create_endpoints': '@'} self.policy.set_rules(rules) @@ -48,8 +48,8 @@ class ArmadaControllerTest(base.BaseControllerTest): 'timeout': '100' } - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller + m_helm = mock_helm.return_value + m_helm.__enter__.return_value = m_helm expected_armada_options = { 'disable_update_pre': False, @@ -57,7 +57,7 @@ class ArmadaControllerTest(base.BaseControllerTest): 'enable_chart_cleanup': False, 'force_wait': False, 'timeout': 100, - 'tiller': m_tiller, + 'helm': m_helm, 'target_manifest': None } @@ -87,8 +87,8 @@ class ArmadaControllerTest(base.BaseControllerTest): }], **expected_armada_options) mock_armada.return_value.sync.assert_called() - mock_tiller.assert_called() - m_tiller.__exit__.assert_called() + mock_helm.assert_called() + m_helm.__exit__.assert_called() def test_armada_apply_no_href(self): """Tests /api/v1.0/apply returns 400 when hrefs list is empty.""" diff --git a/armada/tests/unit/api/test_releases_controller.py b/armada/tests/unit/api/test_releases_controller.py new file mode 100644 index 00000000..4b933ab5 --- /dev/null +++ b/armada/tests/unit/api/test_releases_controller.py @@ -0,0 +1,100 @@ +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the 'License'); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an 'AS IS' BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import mock +from oslo_config import cfg + +from armada import api +from armada.common.policies import base as policy_base +from armada.tests import test_utils +from armada.tests.unit.api import base + +CONF = cfg.CONF + + +class ReleasesControllerTest(base.BaseControllerTest): + @mock.patch.object(api, 'Helm') + def test_helm_releases(self, mock_helm): + """Tests GET /api/v1.0/releases endpoint.""" + rules = {'armada:get_release': '@'} + self.policy.set_rules(rules) + + def _get_fake_release(name, namespace): + fake_release = mock.Mock(namespace='%s_namespace' % namespace) + fake_release.configure_mock(name=name) + return fake_release + + m_helm = mock_helm.return_value + m_helm.__enter__.return_value = m_helm + m_helm.list_release_ids.return_value = [ + _get_fake_release('foo', 'bar'), + _get_fake_release('baz', 'qux') + ] + + result = self.app.simulate_get('/api/v1.0/releases') + expected = { + 'releases': { + 'bar_namespace': ['foo'], + 'qux_namespace': ['baz'] + } + } + + self.assertEqual(expected, result.json) + mock_helm.assert_called_once() + m_helm.list_release_ids.assert_called_once_with() + m_helm.__exit__.assert_called() + + @mock.patch.object(api, 'Helm') + def test_helm_releases_with_params(self, mock_helm): + """Tests GET /api/v1.0/releases endpoint with query parameters.""" + rules = {'armada:get_release': '@'} + self.policy.set_rules(rules) + + def _get_fake_release(name, namespace): + fake_release = mock.Mock(namespace='%s_namespace' % namespace) + fake_release.configure_mock(name=name) + return fake_release + + m_helm = mock_helm.return_value + m_helm.__enter__.return_value = m_helm + m_helm.list_release_ids.return_value = [ + _get_fake_release('foo', 'bar'), + _get_fake_release('baz', 'qux') + ] + + result = self.app.simulate_get( + '/api/v1.0/releases', params_csv=False, params={}) + expected = { + 'releases': { + 'bar_namespace': ['foo'], + 'qux_namespace': ['baz'] + } + } + + self.assertEqual(expected, result.json) + mock_helm.assert_called_once() + m_helm.list_release_ids.assert_called_once_with() + m_helm.__exit__.assert_called() + + +class ReleasesControllerNegativeRbacTest(base.BaseControllerTest): + @test_utils.attr(type=['negative']) + def test_list_helm_releases_insufficient_permissions(self): + """Tests the GET /api/v1.0/releases endpoint returns 403 following + failed authorization. + """ + rules = {'armada:get_release': policy_base.RULE_ADMIN_REQUIRED} + self.policy.set_rules(rules) + resp = self.app.simulate_get('/api/v1.0/releases') + self.assertEqual(403, resp.status_code) diff --git a/armada/tests/unit/api/test_test_controller.py b/armada/tests/unit/api/test_test_controller.py index 6b7e9717..296175bc 100644 --- a/armada/tests/unit/api/test_test_controller.py +++ b/armada/tests/unit/api/test_test_controller.py @@ -31,21 +31,20 @@ from armada.tests.unit.api import base test.TestReleasesManifestController.handle.__wrapped__) class TestReleasesManifestControllerTest(base.BaseControllerTest): @mock.patch.object(test, 'Manifest') - @mock.patch.object(api, 'Tiller') - def test_test_controller_with_manifest(self, mock_tiller, mock_manifest): + @mock.patch.object(api, 'Helm') + def test_test_controller_with_manifest(self, mock_Helm, mock_manifest): rules = {'armada:test_manifest': '@'} self.policy.set_rules(rules) # TODO: Don't use example charts in tests. - # TODO: Test cleanup arg is taken from url, then manifest. manifest_path = os.path.join( os.getcwd(), 'examples', 'keystone-manifest.yaml') with open(manifest_path, 'r') as f: payload = f.read() documents = list(yaml.safe_load_all(payload)) - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller + m_helm = mock_Helm.return_value + m_helm.__enter__.return_value = m_helm resp = self.app.simulate_post('/api/v1.0/tests', body=payload) self.assertEqual(200, resp.status_code) @@ -55,8 +54,8 @@ class TestReleasesManifestControllerTest(base.BaseControllerTest): self.assertEqual(expected, result) mock_manifest.assert_called_once_with(documents, target_manifest=None) - mock_tiller.assert_called() - m_tiller.__exit__.assert_called() + mock_Helm.assert_called() + m_helm.__exit__.assert_called() @mock.patch.object( @@ -64,65 +63,48 @@ class TestReleasesManifestControllerTest(base.BaseControllerTest): test.TestReleasesReleaseNameController.handle.__wrapped__) class TestReleasesReleaseNameControllerTest(base.BaseControllerTest): @mock.patch.object(test.Test, 'test_release_for_success') - @mock.patch.object(api, 'Tiller') + @mock.patch.object(api, 'Helm') def test_test_controller_test_pass( - self, mock_tiller, mock_test_release_for_success): + self, mock_Helm, mock_test_release_for_success): rules = {'armada:test_release': '@'} self.policy.set_rules(rules) mock_test_release_for_success.return_value = True - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller + m_helm = mock_Helm.return_value + m_helm.__enter__.return_value = m_helm + namespace = 'fake-namespace' release = 'fake-release' - resp = self.app.simulate_get('/api/v1.0/test/{}'.format(release)) + resp = self.app.simulate_get( + '/api/v1.0/test/{}/{}'.format(namespace, release)) mock_test_release_for_success.assert_called_once() self.assertEqual(200, resp.status_code) self.assertEqual( 'MESSAGE: Test Pass', json.loads(resp.text)['message']) - m_tiller.__exit__.assert_called() + m_helm.__exit__.assert_called() @mock.patch.object(test.Test, 'test_release_for_success') - @mock.patch.object(api, 'Tiller') + @mock.patch.object(api, 'Helm') def test_test_controller_test_fail( - self, mock_tiller, mock_test_release_for_success): + self, mock_Helm, mock_test_release_for_success): rules = {'armada:test_release': '@'} self.policy.set_rules(rules) - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller + m_helm = mock_Helm.return_value + m_helm.__enter__.return_value = m_helm mock_test_release_for_success.return_value = False + namespace = 'fake-namespace' release = 'fake-release' - resp = self.app.simulate_get('/api/v1.0/test/{}'.format(release)) + resp = self.app.simulate_get( + '/api/v1.0/test/{}/{}'.format(namespace, release)) self.assertEqual(200, resp.status_code) self.assertEqual( 'MESSAGE: Test Fail', json.loads(resp.text)['message']) - m_tiller.__exit__.assert_called() - - @mock.patch.object(test.Test, 'test_release_for_success') - @mock.patch.object(api, 'Tiller') - def test_test_controller_cleanup( - self, mock_tiller, mock_test_release_for_success): - rules = {'armada:test_release': '@'} - self.policy.set_rules(rules) - - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller - - mock_test_release_for_success.return_value = True - release = 'fake-release' - resp = self.app.simulate_get( - '/api/v1.0/test/{}'.format(release), query_string='cleanup=true') - mock_test_release_for_success.assert_called_once() - self.assertEqual(200, resp.status_code) - self.assertEqual( - 'MESSAGE: Test Pass', - json.loads(resp.text)['message']) - m_tiller.__exit__.assert_called() + m_helm.__exit__.assert_called() @test_utils.attr(type=['negative']) @@ -131,23 +113,23 @@ class TestReleasesReleaseNameControllerTest(base.BaseControllerTest): test.TestReleasesManifestController.handle.__wrapped__) class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): @mock.patch.object(test, 'Manifest') - @mock.patch.object(api, 'Tiller') + @mock.patch.object(api, 'Helm') @mock.patch.object(test.Test, 'test_release_for_success') - def test_test_controller_tiller_exc_returns_500( - self, mock_test_release_for_success, mock_tiller, _): + def test_test_controller_Helm_exc_returns_500( + self, mock_test_release_for_success, mock_Helm, _): rules = {'armada:test_manifest': '@'} self.policy.set_rules(rules) - mock_tiller.side_effect = Exception + mock_Helm.side_effect = Exception mock_test_release_for_success.side_effect = Exception resp = self.app.simulate_post('/api/v1.0/tests') self.assertEqual(500, resp.status_code) @mock.patch.object(test, 'Manifest') - @mock.patch.object(api, 'Tiller') + @mock.patch.object(api, 'Helm') def test_test_controller_validation_failure_returns_400( - self, mock_tiller, mock_manifest): + self, mock_Helm, mock_manifest): rules = {'armada:test_manifest': '@'} self.policy.set_rules(rules) @@ -163,8 +145,8 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): resp = self.app.simulate_post('/api/v1.0/tests', body=invalid_payload) self.assertEqual(400, resp.status_code) - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller + m_helm = mock_Helm.return_value + m_helm.__enter__.return_value = m_helm resp_body = json.loads(resp.text) self.assertEqual(400, resp_body['code']) @@ -185,12 +167,12 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): ( 'Failed to validate documents or generate Armada ' 'Manifest from documents.'), resp_body['message']) - m_tiller.__exit__.assert_called() + m_helm.__exit__.assert_called() @mock.patch('armada.utils.validate.Manifest') - @mock.patch.object(api, 'Tiller') + @mock.patch.object(api, 'Helm') def test_test_controller_manifest_failure_returns_400( - self, mock_tiller, mock_manifest): + self, mock_Helm, mock_manifest): rules = {'armada:test_manifest': '@'} self.policy.set_rules(rules) @@ -205,8 +187,8 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): resp = self.app.simulate_post('/api/v1.0/tests', body=payload) self.assertEqual(400, resp.status_code) - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller + m_helm = mock_Helm.return_value + m_helm.__enter__.return_value = m_helm resp_body = json.loads(resp.text) self.assertEqual(400, resp_body['code']) @@ -228,22 +210,25 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): ( 'Failed to validate documents or generate Armada ' 'Manifest from documents.'), resp_body['message']) - m_tiller.__exit__.assert_called() + m_helm.__exit__.assert_called() @test_utils.attr(type=['negative']) class TestReleasesReleaseNameControllerNegativeTest(base.BaseControllerTest): - @mock.patch.object(api, 'Tiller') + @mock.patch.object(api, 'Helm') @mock.patch.object(test.Test, 'test_release_for_success') - def test_test_controller_tiller_exc_returns_500( - self, mock_test_release_for_success, mock_tiller): + def test_test_controller_Helm_exc_returns_500( + self, mock_test_release_for_success, mock_Helm): rules = {'armada:test_release': '@'} self.policy.set_rules(rules) - mock_tiller.side_effect = Exception + mock_Helm.side_effect = Exception mock_test_release_for_success.side_effect = Exception - resp = self.app.simulate_get('/api/v1.0/test/fake-release') + namespace = 'fake-namespace' + release = 'fake-release' + resp = self.app.simulate_get( + '/api/v1.0/test/{}/{}'.format(namespace, release)) self.assertEqual(500, resp.status_code) @@ -251,12 +236,13 @@ class TestReleasesReleaseNameControllerNegativeRbacTest(base.BaseControllerTest ): @test_utils.attr(type=['negative']) def test_test_release_insufficient_permissions(self): - """Tests the GET /api/v1.0/test/{release} endpoint returns 403 + """Tests the GET /api/v1.0/test/{namespace}/{release} endpoint returns 403 following failed authorization. """ rules = {'armada:test_release': policy_base.RULE_ADMIN_REQUIRED} self.policy.set_rules(rules) - resp = self.app.simulate_get('/api/v1.0/test/test-release') + resp = self.app.simulate_get( + '/api/v1.0/test/test-namespace/test-release') self.assertEqual(403, resp.status_code) diff --git a/armada/tests/unit/api/test_tiller_controller.py b/armada/tests/unit/api/test_tiller_controller.py index 933c767e..a8cc5431 100644 --- a/armada/tests/unit/api/test_tiller_controller.py +++ b/armada/tests/unit/api/test_tiller_controller.py @@ -73,81 +73,8 @@ class TillerControllerTest(base.BaseControllerTest): mock_tiller.assert_called_once() m_tiller.__exit__.assert_called() - @mock.patch.object(api, 'Tiller') - def test_tiller_releases(self, mock_tiller): - """Tests GET /api/v1.0/releases endpoint.""" - rules = {'tiller:get_release': '@'} - self.policy.set_rules(rules) - - def _get_fake_release(name, namespace): - fake_release = mock.Mock(namespace='%s_namespace' % namespace) - fake_release.configure_mock(name=name) - return fake_release - - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller - m_tiller.list_releases.return_value = [ - _get_fake_release('foo', 'bar'), - _get_fake_release('baz', 'qux') - ] - - result = self.app.simulate_get('/api/v1.0/releases') - expected = { - 'releases': { - 'bar_namespace': ['foo'], - 'qux_namespace': ['baz'] - } - } - - self.assertEqual(expected, result.json) - mock_tiller.assert_called_once() - m_tiller.list_releases.assert_called_once_with() - m_tiller.__exit__.assert_called() - - @mock.patch.object(api, 'Tiller') - def test_tiller_releases_with_params(self, mock_tiller): - """Tests GET /api/v1.0/releases endpoint with query parameters.""" - rules = {'tiller:get_release': '@'} - self.policy.set_rules(rules) - - def _get_fake_release(name, namespace): - fake_release = mock.Mock(namespace='%s_namespace' % namespace) - fake_release.configure_mock(name=name) - return fake_release - - m_tiller = mock_tiller.return_value - m_tiller.__enter__.return_value = m_tiller - m_tiller.list_releases.return_value = [ - _get_fake_release('foo', 'bar'), - _get_fake_release('baz', 'qux') - ] - - result = self.app.simulate_get( - '/api/v1.0/releases', params_csv=False, params={}) - expected = { - 'releases': { - 'bar_namespace': ['foo'], - 'qux_namespace': ['baz'] - } - } - - self.assertEqual(expected, result.json) - mock_tiller.assert_called_once() - m_tiller.list_releases.assert_called_once_with() - m_tiller.__exit__.assert_called() - class TillerControllerNegativeRbacTest(base.BaseControllerTest): - @test_utils.attr(type=['negative']) - def test_list_tiller_releases_insufficient_permissions(self): - """Tests the GET /api/v1.0/releases endpoint returns 403 following - failed authorization. - """ - rules = {'tiller:get_release': policy_base.RULE_ADMIN_REQUIRED} - self.policy.set_rules(rules) - resp = self.app.simulate_get('/api/v1.0/releases') - self.assertEqual(403, resp.status_code) - @test_utils.attr(type=['negative']) def test_get_tiller_status_insufficient_permissions(self): """Tests the GET /api/v1.0/status endpoint returns 403 following diff --git a/armada/tests/unit/fake_policy.py b/armada/tests/unit/fake_policy.py index 35721234..f0e040ef 100644 --- a/armada/tests/unit/fake_policy.py +++ b/armada/tests/unit/fake_policy.py @@ -18,6 +18,6 @@ policy_data = """ "armada:validate_manifest": "rule:admin_required" "armada:test_release": "rule:admin_required" "armada:test_manifest": "rule:admin_required" +"armada:get_release": "rule:admin_required" "tiller:get_status": "rule:admin_required" -"tiller:get_release": "rule:admin_required" """ diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index 51369747..8ebac9fb 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -12,19 +12,19 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os + import mock import yaml from armada import const from armada.handlers import armada -from armada.utils.helm import TESTRUN_STATUS_SUCCESS, TESTRUN_STATUS_FAILURE from armada.tests.unit import base -from armada.tests.test_utils import AttrDict +from armada.handlers import helm from armada.utils.release import release_prefixer, get_release_status from armada.exceptions import ManifestException from armada.exceptions.override_exceptions import InvalidOverrideValueException from armada.exceptions.validate_exceptions import InvalidManifestException -from armada.exceptions import tiller_exceptions from armada.exceptions.armada_exceptions import ChartDeployException TEST_YAML = """ @@ -114,11 +114,8 @@ data: no_hooks: false options: force: true - recreate_pods: true test: enabled: true - options: - cleanup: true --- schema: armada/Chart/v1 metadata: @@ -150,14 +147,14 @@ CHART_SOURCES = ( # TODO(seaneagan): Add unit tests with dependencies, including transitive. +def set_source_dir(ch, manifest=None): + d = ch['data'] + d['source_dir'] = (d['source']['location'], d['source']['subpath']) + + class ArmadaHandlerTestCase(base.ArmadaTestCase): def _test_pre_flight_ops(self, armada_obj, MockChartDownload): - def set_source_dir(ch, manifest=None): - d = ch['data'] - d['source_dir'] = (d['source']['location'], d['source']['subpath']) - MockChartDownload.return_value.get_chart.side_effect = set_source_dir - armada_obj.pre_flight_ops() expected_config = { @@ -232,15 +229,11 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): 'upgrade': { 'no_hooks': False, 'options': { - 'force': True, - 'recreate_pods': True + 'force': True } }, 'test': { - 'enabled': True, - 'options': { - 'cleanup': True - } + 'enabled': True } } }, { @@ -314,9 +307,8 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): def test_pre_flight_ops(self, MockChartDownload): """Test pre-flight checks and operations.""" yaml_documents = list(yaml.safe_load_all(TEST_YAML)) - m_tiller = mock.Mock() - m_tiller.tiller_status.return_value = True - armada_obj = armada.Armada(yaml_documents, m_tiller) + m_helm = mock.Mock() + armada_obj = armada.Armada(yaml_documents, m_helm) self._test_pre_flight_ops(armada_obj, MockChartDownload) @@ -328,10 +320,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): yaml_documents = list(yaml.safe_load_all(TEST_YAML)) # Mock methods called by `pre_flight_ops()`. - m_tiller = mock.Mock() - m_tiller.tiller_status.return_value = True + m_helm = mock.Mock() - armada_obj = armada.Armada(yaml_documents, m_tiller) + armada_obj = armada.Armada(yaml_documents, m_helm) self._test_pre_flight_ops(armada_obj, MockChartDownload) @@ -359,19 +350,30 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): """Test install functionality from the sync() method.""" @mock.patch.object(armada.Armada, 'post_flight_ops') - @mock.patch.object(armada.Armada, 'pre_flight_ops') + @mock.patch.object(armada, 'ChartDownload') @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, + mock_test, mock_chartbuilder, MockChartDownload, mock_post_flight): + MockChartDownload.return_value.get_chart.side_effect = \ + set_source_dir # Instantiate Armada object. yaml_documents = list(yaml.safe_load_all(TEST_YAML)) + m_helm = mock.MagicMock() + armada_obj = armada.Armada(yaml_documents, m_helm) + prefix = armada_obj.manifest['data']['release_prefix'] - m_tiller = mock.MagicMock() - m_tiller.list_releases.return_value = known_releases + def release_metadata(release_id, **kwargs): + try: + return next( + r for r in known_releases + if release_id.name == r['name'] + and release_id.namespace == r['namespace']) + except StopIteration: + return None - armada_obj = armada.Armada(yaml_documents, m_tiller) + m_helm.release_metadata.side_effect = release_metadata armada_obj.chart_deploy.get_diff = mock.Mock() cg = armada_obj.manifest['data']['chart_groups'][0] @@ -379,18 +381,12 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): charts = chart_group['chart_group'] cg_test_all_charts = chart_group.get('test_charts') - mock_test_release = mock_test.return_value.test_release_for_success + mock_test_release = mock_test.return_value.test_release if test_failure_to_run: - - def fail(tiller, release, timeout=None, cleanup=False): - status = AttrDict( - **{'info': AttrDict(**{'Description': 'Failed'})}) - raise tiller_exceptions.ReleaseException( - release, status, 'Test') - - mock_test_release.side_effect = fail + mock_test_release.side_effect = Exception('test failed to run') else: - mock_test_release.return_value = test_success + if not test_success: + mock_test_release.side_effect = Exception('test failed') mock_test.return_value.timeout = const.DEFAULT_TEST_TIMEOUT # Stub out irrelevant methods called by `armada.sync()`. @@ -402,56 +398,53 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): armada_obj.sync() expected_install_release_calls = [] - expected_update_release_calls = [] + expected_upgrade_release_calls = [] expected_uninstall_release_calls = [] expected_test_constructor_calls = [] for c in charts: chart = c['data'] release = chart['release'] - prefix = armada_obj.manifest['data']['release_prefix'] release_name = release_prefixer(prefix, release) + release_id = helm.HelmReleaseId( + chart['namespace'], release_name) + source_dir = chart['source_dir'] + source_directory = os.path.join(*source_dir) + # Simplified check because the actual code uses logical-or's # multiple conditions, so this is enough. native_wait_enabled = ( chart['wait'].get('native', {}).get('enabled', True)) - if release_name not in [x.name for x in known_releases]: + if release_name not in [x['name'] for x in known_releases]: expected_install_release_calls.append( mock.call( - mock_chartbuilder().get_helm_chart(), - "{}-{}".format( - armada_obj.manifest['data']['release_prefix'], - chart['release']), - chart['namespace'], - values=yaml.safe_dump(chart['values']), + source_directory, + release_id, + values=chart['values'], wait=native_wait_enabled, timeout=mock.ANY)) else: target_release = None for known_release in known_releases: - if known_release.name == release_name: + if known_release['name'] == release_name: target_release = known_release break if target_release: status = get_release_status(target_release) - if status == const.STATUS_FAILED: + if status == helm.STATUS_FAILED: protected = chart.get('protected', {}) if not protected: expected_uninstall_release_calls.append( mock.call( - release_name, + release_id, purge=True, timeout=const.DEFAULT_DELETE_TIMEOUT)) expected_install_release_calls.append( mock.call( - mock_chartbuilder().get_helm_chart(), - "{}-{}".format( - armada_obj.manifest['data'] - ['release_prefix'], - chart['release']), - chart['namespace'], - values=yaml.safe_dump(chart['values']), + source_directory, + release_id, + values=chart['values'], wait=native_wait_enabled, timeout=mock.ANY)) else: @@ -463,62 +456,55 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): if chart_group['sequenced']: break - if status == const.STATUS_DEPLOYED: + if status == helm.STATUS_DEPLOYED: if diff: upgrade = chart.get('upgrade', {}) disable_hooks = upgrade.get('no_hooks', False) options = upgrade.get('options', {}) force = options.get('force', False) - recreate_pods = options.get( - 'recreate_pods', False) - expected_update_release_calls.append( + expected_upgrade_release_calls.append( mock.call( - mock_chartbuilder().get_helm_chart(), - "{}-{}".format( - armada_obj.manifest['data'] - ['release_prefix'], - chart['release']), - chart['namespace'], + source_directory, + release_id, disable_hooks=disable_hooks, force=force, - recreate_pods=recreate_pods, - values=yaml.safe_dump(chart['values']), + values=chart['values'], wait=native_wait_enabled, timeout=mock.ANY)) expected_test_constructor_calls.append( mock.call( chart, - release_name, - m_tiller, + release_id, + m_helm, cg_test_charts=cg_test_all_charts)) any_order = not chart_group['sequenced'] # Verify that at least 1 release is either installed or updated. self.assertTrue( len(expected_install_release_calls) >= 1 - or len(expected_update_release_calls) >= 1) + or len(expected_upgrade_release_calls) >= 1) # Verify that the expected number of non-deployed releases are # installed with expected arguments. self.assertEqual( len(expected_install_release_calls), - m_tiller.install_release.call_count) - m_tiller.install_release.assert_has_calls( + m_helm.install_release.call_count) + m_helm.install_release.assert_has_calls( expected_install_release_calls, any_order=any_order) # Verify that the expected number of deployed releases are # updated with expected arguments. self.assertEqual( - len(expected_update_release_calls), - m_tiller.update_release.call_count) - m_tiller.update_release.assert_has_calls( - expected_update_release_calls, any_order=any_order) + len(expected_upgrade_release_calls), + m_helm.upgrade_release.call_count) + m_helm.upgrade_release.assert_has_calls( + expected_upgrade_release_calls, any_order=any_order) # Verify that the expected number of deployed releases are # uninstalled with expected arguments. self.assertEqual( len(expected_uninstall_release_calls), - m_tiller.uninstall_release.call_count) - m_tiller.uninstall_release.assert_has_calls( + m_helm.uninstall_release.call_count) + m_helm.uninstall_release.assert_has_calls( expected_uninstall_release_calls, any_order=any_order) # Verify that the expected number of deployed releases are # tested with expected arguments. @@ -537,37 +523,31 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): c for c in yaml_documents if c['data'].get('chart_name') == name ][0] - def get_mock_release(self, name, status, last_test_results=None): - status_mock = mock.Mock() - status_mock.return_value = status + def get_mock_release(self, name, status, last_test_results=[]): chart = self._get_chart_by_name(name) - def get_test_result(success): - status = ( - TESTRUN_STATUS_SUCCESS if success else TESTRUN_STATUS_FAILURE) - return mock.Mock(status=status) + def get_test_hook(index, success): + return { + "kind": "Pod", + "events": ["test"], + "last_run": { + "phase": "" + } + } - last_test_suite_run = None - if last_test_results is not None: - results = [get_test_result(r) for r in last_test_results] - last_test_suite_run = mock.Mock(results=results) + hooks = [get_test_hook(i, r) for i, r in enumerate(last_test_results)] - def has_last_test(name): - if name == 'last_test_suite_run': - return last_test_results is not None - self.fail('Called HasField() with unexpected field') - - mock_release = mock.Mock( - version=1, - chart=chart, - config=mock.Mock(raw="{}"), - info=mock.Mock( - status=mock.Mock( - Code=mock.MagicMock(Name=status_mock), - HasField=mock.MagicMock(side_effect=has_last_test), - last_test_suite_run=last_test_suite_run))) - mock_release.name = name - return mock_release + return { + "name": name, + "namespace": "test", + "version": 1, + "chart": chart, + "config": {}, + "info": { + "status": status + }, + "hooks": hooks + } def test_armada_sync_with_no_deployed_releases(self): known_releases = [] @@ -576,13 +556,13 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): def test_armada_sync_with_one_deployed_release(self): c1 = 'armada-test_chart_1' - known_releases = [self.get_mock_release(c1, const.STATUS_DEPLOYED)] + known_releases = [self.get_mock_release(c1, helm.STATUS_DEPLOYED)] self._test_sync(known_releases) def test_armada_sync_with_one_deployed_release_no_diff(self): c1 = 'armada-test_chart_1' - known_releases = [self.get_mock_release(c1, const.STATUS_DEPLOYED)] + known_releases = [self.get_mock_release(c1, helm.STATUS_DEPLOYED)] self._test_sync(known_releases, diff=set()) def test_armada_sync_with_failed_test_result(self): @@ -590,7 +570,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): known_releases = [ self.get_mock_release( - c1, const.STATUS_DEPLOYED, last_test_results=[False]) + c1, helm.STATUS_DEPLOYED, last_test_results=[False]) ] self._test_sync( known_releases, diff=set(), expected_last_test_result=False) @@ -600,7 +580,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): known_releases = [ self.get_mock_release( - c1, const.STATUS_DEPLOYED, last_test_results=[True]) + c1, helm.STATUS_DEPLOYED, last_test_results=[True]) ] self._test_sync( known_releases, diff=set(), expected_last_test_result=True) @@ -610,7 +590,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): known_releases = [ self.get_mock_release( - c1, const.STATUS_DEPLOYED, last_test_results=[]) + c1, helm.STATUS_DEPLOYED, last_test_results=[]) ] self._test_sync( known_releases, diff=set(), expected_last_test_result=True) @@ -620,15 +600,15 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): c2 = 'armada-test_chart_2' known_releases = [ - self.get_mock_release(c1, const.STATUS_DEPLOYED), - self.get_mock_release(c2, const.STATUS_DEPLOYED) + self.get_mock_release(c1, helm.STATUS_DEPLOYED), + self.get_mock_release(c2, helm.STATUS_DEPLOYED) ] self._test_sync(known_releases) def test_armada_sync_with_unprotected_releases(self): c1 = 'armada-test_chart_1' - known_releases = [self.get_mock_release(c1, const.STATUS_FAILED)] + known_releases = [self.get_mock_release(c1, helm.STATUS_FAILED)] self._test_sync(known_releases) def test_armada_sync_with_protected_releases_continue(self): @@ -636,15 +616,15 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): c2 = 'armada-test_chart_2' known_releases = [ - self.get_mock_release(c2, const.STATUS_FAILED), - self.get_mock_release(c1, const.STATUS_FAILED) + self.get_mock_release(c2, helm.STATUS_FAILED), + self.get_mock_release(c1, helm.STATUS_FAILED) ] self._test_sync(known_releases) def test_armada_sync_with_protected_releases_halt(self): c3 = 'armada-test_chart_3' - known_releases = [self.get_mock_release(c3, const.STATUS_FAILED)] + known_releases = [self.get_mock_release(c3, helm.STATUS_FAILED)] def _test_method(): self._test_sync(known_releases) diff --git a/armada/tests/unit/handlers/test_chartbuilder.py b/armada/tests/unit/handlers/test_chartbuilder.py index de202c44..f7f2a74a 100644 --- a/armada/tests/unit/handlers/test_chartbuilder.py +++ b/armada/tests/unit/handlers/test_chartbuilder.py @@ -12,13 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. -import inspect import os +from pathlib import Path import shutil import fixtures -from hapi.chart.chart_pb2 import Chart -from hapi.chart.metadata_pb2 import Metadata import mock import testtools import yaml @@ -36,30 +34,7 @@ class BaseChartBuilderTestCase(testtools.TestCase): version: 0.1.0 """ - chart_value = """ - # Default values for hello-world-chart. - # This is a YAML-formatted file. - # Declare variables to be passed into your templates. - replicaCount: 1 - image: - repository: nginx - tag: stable - pullPolicy: IfNotPresent - service: - name: nginx - type: ClusterIP - externalPort: 38443 - internalPort: 80 - resources: - limits: - cpu: 100m - memory: 128Mi - requests: - cpu: 100m - memory: 128Mi - """ - - chart_stream = """ + chart_doc_yaml = """ schema: armada/Chart/v1 metadata: name: test @@ -83,14 +58,14 @@ class BaseChartBuilderTestCase(testtools.TestCase): dependencies: [] """ - dependency_chart_yaml = """ + dep_chart_yaml = """ apiVersion: v1 description: Another sample Helm chart for Kubernetes name: dependency-chart version: 0.1.0 """ - dependency_chart_stream = """ + dep_chart_doc_yaml = """ schema: armada/Chart/v1 metadata: name: dep @@ -120,13 +95,6 @@ class BaseChartBuilderTestCase(testtools.TestCase): finally: os.close(fd) - def _make_temporary_subdirectory(self, parent_path, sub_path): - subdir = os.path.join(parent_path, sub_path) - if not os.path.exists(subdir): - os.makedirs(subdir) - self.addCleanup(shutil.rmtree, subdir) - return subdir - def _get_test_chart(self, chart_dir): return { 'schema': 'armada/Chart/v1', @@ -134,379 +102,67 @@ class BaseChartBuilderTestCase(testtools.TestCase): 'name': 'test' }, const.KEYWORD_DATA: { - 'source_dir': (chart_dir.path, '') + 'source_dir': (chart_dir, '') } } class ChartBuilderTestCase(BaseChartBuilderTestCase): - def test_source_clone(self): - # Create a temporary directory with Chart.yaml that contains data - # from ``self.chart_yaml``. + def test_get_helm_chart_success(self): chart_dir = self.useFixture(fixtures.TempDir()) self.addCleanup(shutil.rmtree, chart_dir.path) - self._write_temporary_file_contents( - chart_dir.path, 'Chart.yaml', self.chart_yaml) - + helm_mock = mock.Mock() + helm_mock.upgrade_release.return_value = {"chart": mock.sentinel.chart} chartbuilder = ChartBuilder.from_chart_doc( - self._get_test_chart(chart_dir)) + self._get_test_chart(chart_dir.path), helm_mock) + release_id = mock.Mock() + values = mock.Mock() + actual_chart = chartbuilder.get_helm_chart(release_id, values) + self.assertIs(mock.sentinel.chart, actual_chart) - # Validate response type is :class:`hapi.chart.metadata_pb2.Metadata` - resp = chartbuilder.get_metadata() - self.assertIsInstance(resp, Metadata) - - def test_get_metadata_with_incorrect_file_invalid(self): + def test_get_helm_chart_fail(self): chart_dir = self.useFixture(fixtures.TempDir()) self.addCleanup(shutil.rmtree, chart_dir.path) - + helm_mock = mock.Mock() + helm_mock.upgrade_release.side_effect = Exception() chartbuilder = ChartBuilder.from_chart_doc( - self._get_test_chart(chart_dir)) + self._get_test_chart(chart_dir.path), helm_mock) + + def test(): + release_id = mock.Mock() + values = mock.Mock() + chartbuilder.get_helm_chart(release_id, values) self.assertRaises( - chartbuilder_exceptions.MetadataLoadException, - chartbuilder.get_metadata) + chartbuilder_exceptions.HelmChartBuildException, test) - def test_get_files(self): - """Validates that ``get_files()`` ignores 'Chart.yaml', 'values.yaml' - and 'templates' subfolder and all the files contained therein. - """ - - # Create a temporary directory that represents a chart source directory - # with various files, including 'Chart.yaml' and 'values.yaml' which - # should be ignored by `get_files()`. - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - for filename in ['foo', 'bar', 'Chart.yaml', 'values.yaml']: - self._write_temporary_file_contents(chart_dir.path, filename, "") - - # Create a template directory -- 'templates' -- nested inside the chart - # directory which should also be ignored. - templates_subdir = self._make_temporary_subdirectory( - chart_dir.path, 'templates') - for filename in ['template%d' % x for x in range(3)]: - self._write_temporary_file_contents(templates_subdir, filename, "") - - chartbuilder = ChartBuilder.from_chart_doc( - self._get_test_chart(chart_dir)) - - expected_files = ( - '[type_url: "%s"\n, type_url: "%s"\n]' % ('./bar', './foo')) - # Validate that only 'foo' and 'bar' are returned. - actual_files = sorted( - chartbuilder.get_files(), key=lambda x: x.type_url) - self.assertEqual(expected_files, repr(actual_files).strip()) - - def test_get_files_with_unicode_characters(self): - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - for filename in ['foo', 'bar', 'Chart.yaml', 'values.yaml']: - self._write_temporary_file_contents( - chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") - - chartbuilder = ChartBuilder.from_chart_doc( - self._get_test_chart(chart_dir)) - chartbuilder.get_files() - - def test_get_basic_helm_chart(self): - # Before ChartBuilder is executed the `source_dir` points to a - # directory that was either clone or unpacked from a tarball... pretend - # that that logic has already been performed. - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - self._write_temporary_file_contents( - chart_dir.path, 'Chart.yaml', self.chart_yaml) - ch = yaml.safe_load(self.chart_stream) - ch['data']['source_dir'] = (chart_dir.path, '') - - test_chart = ch - chartbuilder = ChartBuilder.from_chart_doc(test_chart) - helm_chart = chartbuilder.get_helm_chart() - - expected = inspect.cleandoc( - """ - metadata { - name: "hello-world-chart" - version: "0.1.0" - description: "A sample Helm chart for Kubernetes" - } - values { - } - """).strip() - - self.assertIsInstance(helm_chart, Chart) - self.assertTrue(hasattr(helm_chart, 'metadata')) - self.assertTrue(hasattr(helm_chart, 'values')) - self.assertEqual(expected, repr(helm_chart).strip()) - - def test_get_helm_chart_with_values(self): - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - - self._write_temporary_file_contents( - chart_dir.path, 'Chart.yaml', self.chart_yaml) - self._write_temporary_file_contents( - chart_dir.path, 'values.yaml', self.chart_value) - - ch = yaml.safe_load(self.chart_stream) - ch['data']['source_dir'] = (chart_dir.path, '') - - test_chart = ch - chartbuilder = ChartBuilder.from_chart_doc(test_chart) - helm_chart = chartbuilder.get_helm_chart() - - self.assertIsInstance(helm_chart, Chart) - self.assertTrue(hasattr(helm_chart, 'metadata')) - self.assertTrue(hasattr(helm_chart, 'values')) - self.assertTrue(hasattr(helm_chart.values, 'raw')) - self.assertEqual(self.chart_value, helm_chart.values.raw) - - def test_get_helm_chart_with_files(self): - # Create a chart directory with some test files. - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - # Chart.yaml is mandatory for `ChartBuilder.get_metadata`. - self._write_temporary_file_contents( - chart_dir.path, 'Chart.yaml', self.chart_yaml) - self._write_temporary_file_contents(chart_dir.path, 'foo', "foobar") - self._write_temporary_file_contents(chart_dir.path, 'bar', "bazqux") - - # Also create a nested directory and verify that files from it are also - # added. - nested_dir = self._make_temporary_subdirectory( - chart_dir.path, 'nested') - self._write_temporary_file_contents(nested_dir, 'nested0', "random") - - ch = yaml.safe_load(self.chart_stream) - ch['data']['source_dir'] = (chart_dir.path, '') - - test_chart = ch - chartbuilder = ChartBuilder.from_chart_doc(test_chart) - helm_chart = chartbuilder.get_helm_chart() - - expected_files = ( - '[type_url: "%s"\nvalue: "bazqux"\n, ' - 'type_url: "%s"\nvalue: "foobar"\n, ' - 'type_url: "%s"\nvalue: "random"\n]' % - ('./bar', './foo', 'nested/nested0')) - - self.assertIsInstance(helm_chart, Chart) - self.assertTrue(hasattr(helm_chart, 'metadata')) - self.assertTrue(hasattr(helm_chart, 'values')) - self.assertTrue(hasattr(helm_chart, 'files')) - actual_files = sorted(helm_chart.files, key=lambda x: x.value) - self.assertEqual(expected_files, repr(actual_files).strip()) - - def test_get_helm_chart_includes_only_relevant_files(self): - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - - templates_subdir = self._make_temporary_subdirectory( - chart_dir.path, 'templates') - charts_subdir = self._make_temporary_subdirectory( - chart_dir.path, 'charts') - templates_nested_subdir = self._make_temporary_subdirectory( - templates_subdir, 'bin') - charts_nested_subdir = self._make_temporary_subdirectory( - charts_subdir, 'extra') - - self._write_temporary_file_contents( - chart_dir.path, 'Chart.yaml', self.chart_yaml) - self._write_temporary_file_contents(chart_dir.path, 'foo', "foobar") - self._write_temporary_file_contents(chart_dir.path, 'bar', "bazqux") - - # Files to ignore within top-level directory. - files_to_ignore = ['Chart.yaml', 'values.yaml', 'values.toml'] - for file in files_to_ignore: - self._write_temporary_file_contents(chart_dir.path, file, "") - file_to_ignore = 'file_to_ignore' - # Files to ignore within templates/ subdirectory. - self._write_temporary_file_contents( - templates_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") - - ch = yaml.safe_load(self.chart_stream) - ch['data']['source_dir'] = (chart_dir.path, '') - - test_chart = ch - chartbuilder = ChartBuilder.from_chart_doc(test_chart) - helm_chart = chartbuilder.get_helm_chart() - - expected_files = ( - '[type_url: "%s"\nvalue: "bazqux"\n, ' - 'type_url: "%s"\nvalue: "foobar"\n, ' - 'type_url: "%s"\nvalue: "xyzzy"\n]' % - ('./bar', './foo', 'charts/.prov')) - - # Validate that only relevant files are included, that the ignored - # files are present. - self.assertIsInstance(helm_chart, Chart) - self.assertTrue(hasattr(helm_chart, 'metadata')) - self.assertTrue(hasattr(helm_chart, 'values')) - self.assertTrue(hasattr(helm_chart, 'files')) - actual_files = sorted(helm_chart.files, key=lambda x: x.value) - self.assertEqual(expected_files, repr(actual_files).strip()) - - def test_get_helm_chart_with_dependencies(self): + def test_dependency_resolution(self): # Main chart directory and files. chart_dir = self.useFixture(fixtures.TempDir()) self.addCleanup(shutil.rmtree, chart_dir.path) self._write_temporary_file_contents( chart_dir.path, 'Chart.yaml', self.chart_yaml) - ch = yaml.safe_load(self.chart_stream) - ch['data']['source_dir'] = (chart_dir.path, '') + chart_doc = yaml.safe_load(self.chart_doc_yaml) + chart_doc['data']['source_dir'] = (chart_dir.path, '') # Dependency chart directory and files. dep_chart_dir = self.useFixture(fixtures.TempDir()) self.addCleanup(shutil.rmtree, dep_chart_dir.path) self._write_temporary_file_contents( - dep_chart_dir.path, 'Chart.yaml', self.dependency_chart_yaml) - dep_ch = yaml.safe_load(self.dependency_chart_stream) - dep_ch['data']['source_dir'] = (dep_chart_dir.path, '') + dep_chart_dir.path, 'Chart.yaml', self.dep_chart_yaml) + dep_chart_doc = yaml.safe_load(self.dep_chart_doc_yaml) + dep_chart_doc['data']['source_dir'] = (dep_chart_dir.path, '') - main_chart = ch - dependency_chart = dep_ch - main_chart['data']['dependencies'] = [dependency_chart] + # Add dependency + chart_doc['data']['dependencies'] = [dep_chart_doc] - chartbuilder = ChartBuilder.from_chart_doc(main_chart) - helm_chart = chartbuilder.get_helm_chart() + # Mock helm cli call + helm_mock = mock.Mock() + helm_mock.show_chart.return_value = yaml.safe_load(self.dep_chart_yaml) + ChartBuilder.from_chart_doc(chart_doc, helm_mock) - expected_dependency = inspect.cleandoc( - """ - metadata { - name: "dependency-chart" - version: "0.1.0" - description: "Another sample Helm chart for Kubernetes" - } - values { - } - """).strip() - - expected = inspect.cleandoc( - """ - metadata { - name: "hello-world-chart" - version: "0.1.0" - description: "A sample Helm chart for Kubernetes" - } - dependencies { - metadata { - name: "dependency-chart" - version: "0.1.0" - description: "Another sample Helm chart for Kubernetes" - } - values { - } - } - values { - } - """).strip() - - # Validate the main chart. - self.assertIsInstance(helm_chart, Chart) - self.assertTrue(hasattr(helm_chart, 'metadata')) - self.assertTrue(hasattr(helm_chart, 'values')) - self.assertEqual(expected, repr(helm_chart).strip()) - - # Validate the dependency chart. - self.assertTrue(hasattr(helm_chart, 'dependencies')) - self.assertEqual(1, len(helm_chart.dependencies)) - - dep_helm_chart = helm_chart.dependencies[0] - self.assertIsInstance(dep_helm_chart, Chart) - self.assertTrue(hasattr(dep_helm_chart, 'metadata')) - self.assertTrue(hasattr(dep_helm_chart, 'values')) - self.assertEqual(expected_dependency, repr(dep_helm_chart).strip()) - - def test_dump(self): - # Validate base case. - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - self._write_temporary_file_contents( - chart_dir.path, 'Chart.yaml', self.chart_yaml) - ch = yaml.safe_load(self.chart_stream) - ch['data']['source_dir'] = (chart_dir.path, '') - - test_chart = ch - chartbuilder = ChartBuilder.from_chart_doc(test_chart) - self.assertRegex( - repr(chartbuilder.dump()), - 'hello-world-chart.*A sample Helm chart for Kubernetes.*') - - # Validate recursive case (with dependencies). - dep_chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, dep_chart_dir.path) - self._write_temporary_file_contents( - dep_chart_dir.path, 'Chart.yaml', self.dependency_chart_yaml) - dep_ch = yaml.safe_load(self.dependency_chart_stream) - dep_ch['data']['source_dir'] = (dep_chart_dir.path, '') - - dependency_chart = dep_ch - test_chart['data']['dependencies'] = [dependency_chart] - chartbuilder = ChartBuilder.from_chart_doc(test_chart) - - re = inspect.cleandoc( - """ - hello-world-chart.*A sample Helm chart for Kubernetes.* - dependency-chart.*Another sample Helm chart for Kubernetes.* - """).replace('\n', '').strip() - self.assertRegex(repr(chartbuilder.dump()), re) - - -class ChartBuilderNegativeTestCase(BaseChartBuilderTestCase): - def setUp(self): - super(ChartBuilderNegativeTestCase, self).setUp() - # Create an exception for testing since instantiating one manually - # is tedious. - try: - str(b'\xff', 'utf8') - except UnicodeDecodeError as e: - self.exc_to_raise = e - else: - self.fail('Failed to create an exception needed for testing.') - - def test_get_files_always_fails_to_read_binary_file_raises_exc(self): - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - for filename in ['foo', 'bar', 'Chart.yaml', 'values.yaml']: - self._write_temporary_file_contents( - chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") - - chartbuilder = ChartBuilder.from_chart_doc( - self._get_test_chart(chart_dir)) - - # Confirm it failed for both encodings. - error_re = ( - r'.*A str exception occurred while trying to read file:' - r'.*Details:\n.*\(encoding=utf-8\).*\n\(encoding=latin1\)') - with mock.patch("builtins.open", mock.mock_open(read_data="")) \ - as mock_file: - mock_file.return_value.read.side_effect = self.exc_to_raise - self.assertRaisesRegexp( - chartbuilder_exceptions.FilesLoadException, error_re, - chartbuilder.get_files) - - def test_get_files_fails_once_to_read_binary_file_passes(self): - chart_dir = self.useFixture(fixtures.TempDir()) - self.addCleanup(shutil.rmtree, chart_dir.path) - files = ['foo', 'bar'] - for filename in files: - self._write_temporary_file_contents( - chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") - - 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="")) \ - as mock_file: - mock_file.return_value.read.side_effect = side_effects - chartbuilder.get_files() + expected_symlink_path = Path( + chart_dir.path).joinpath('charts').joinpath('dependency-chart') + self.assertTrue(expected_symlink_path.is_symlink()) + self.assertEqual( + dep_chart_dir.path, str(expected_symlink_path.resolve())) diff --git a/armada/tests/unit/handlers/test_lock.py b/armada/tests/unit/handlers/test_lock.py index 0aba7fb7..7b36ddd5 100644 --- a/armada/tests/unit/handlers/test_lock.py +++ b/armada/tests/unit/handlers/test_lock.py @@ -37,10 +37,10 @@ class LockTestCase(testtools.TestCase): def setUp(self): super(LockTestCase, self).setUp() - self_link = "/apis/armada.tiller/v1/namespaces/default/locks/"\ - "locks.armada.tiller.test" + self_link = "/apis/armada.helm/v1/namespaces/default/locks/"\ + "locks.armada.helm.test" self.resp = { - 'apiVersion': "armada.tiller/v1", + 'apiVersion': "armada.helm/v1", 'data': { 'lastUpdated': "2019-01-22T16:20:14Z" }, diff --git a/armada/tests/unit/handlers/test_release_diff.py b/armada/tests/unit/handlers/test_release_diff.py index 5c66123f..844a3998 100644 --- a/armada/tests/unit/handlers/test_release_diff.py +++ b/armada/tests/unit/handlers/test_release_diff.py @@ -12,190 +12,22 @@ # See the License for the specific language governing permissions and # limitations under the License. -from google.protobuf.any_pb2 import Any -from hapi.chart.chart_pb2 import Chart -from hapi.chart.config_pb2 import Config -from hapi.chart.metadata_pb2 import Metadata -from hapi.chart.template_pb2 import Template +import mock from armada.handlers.release_diff import ReleaseDiff from armada.tests.unit import base -# Tests for diffs which can occur in both top-level or dependency charts, -# and thus are inherited by both of those test classes. -class _BaseReleaseDiffTestCase(): - def setUp(self): - super(base.ArmadaTestCase, self).setUp() - self.old_chart = self.make_chart() - self.old_values = self.make_values() - - def make_chart(self): - chart = self._make_chart() - dep = self._make_chart() - dep.metadata.name = 'dep1' - sub_dep = self._make_chart() - sub_dep.metadata.name = 'dep1' - sub_sub_dep = self._make_chart() - sub_sub_dep.metadata.name = 'dep1' - sub_dep.dependencies.extend([sub_sub_dep]) - dep.dependencies.extend([sub_dep]) - chart.dependencies.extend([dep]) - return chart - - def _make_chart(self): - return Chart( - metadata=Metadata( - description='chart description', - name='chart_name', - version='0.1.2'), - templates=[ - Template( - name='template_name', data='template content'.encode()) - ], - files=[ - Any(type_url='./file_name.ext', value='file content'.encode()) - ], - dependencies=[], - values=Config(raw='{param: d1}')) - - def make_values(self): - return {'param': 'o1'} - - def _test_chart_diff(self, update_chart): - new_chart = self.make_chart() - chart_to_update = self.get_chart_to_update(new_chart) - update_chart(chart_to_update) - diff = ReleaseDiff( - self.old_chart, self.old_values, new_chart, - self.old_values).get_diff() - self.assertTrue(diff) - - def get_chart_to_update(self, chart): - raise NotImplementedError('Implement in subclass') - - def test_metadata_non_name_diff_ignored(self): - new_chart = self.make_chart() - chart_to_update = self.get_chart_to_update(new_chart) - chart_to_update.metadata.description = 'new chart description' - diff = ReleaseDiff( - self.old_chart, self.old_values, new_chart, - self.old_values).get_diff() - self.assertFalse(diff) - - def test_metadata_name_diff(self): - def update_chart(chart): - chart.metadata.name = 'new_chart_name' - - self._test_chart_diff(update_chart) - - def test_default_values_diff(self): - def update_chart(chart): - chart.values.raw = '{param: d2}' - - self._test_chart_diff(update_chart) - - def test_template_name_diff(self): - def update_chart(chart): - chart.templates[0].name = 'new_template_name' - - self._test_chart_diff(update_chart) - - def test_template_data_diff(self): - def update_chart(chart): - chart.templates[0].data = 'new template content'.encode() - - self._test_chart_diff(update_chart) - - def test_add_template_diff(self): - def update_chart(chart): - chart.templates.extend( - [ - Template( - name='new_template_name', - data='new template content'.encode()) - ]) - - self._test_chart_diff(update_chart) - - def test_remove_template_diff(self): - def update_chart(chart): - del chart.templates[0] - - self._test_chart_diff(update_chart) - - def test_file_type_url_diff(self): - def update_chart(chart): - chart.files[0].type_url = './new_file_name.ext' - - self._test_chart_diff(update_chart) - - def test_file_value_diff(self): - def update_chart(chart): - chart.files[0].value = 'new file content'.encode() - - self._test_chart_diff(update_chart) - - def test_add_file_diff(self): - def update_chart(chart): - chart.files.extend( - [ - Any( - type_url='./new_file_name.ext', - value='new file content'.encode()) - ]) - - self._test_chart_diff(update_chart) - - def test_remove_file_diff(self): - def update_chart(chart): - del chart.files[0] - - self._test_chart_diff(update_chart) - - def test_add_dependency_diff(self): - def update_chart(chart): - dep = self._make_chart() - dep.metadata.name = 'dep2' - chart.dependencies.extend([dep]) - - self._test_chart_diff(update_chart) - - def test_remove_dependency_diff(self): - def update_chart(chart): - del chart.dependencies[0] - - self._test_chart_diff(update_chart) - - # Test diffs (or absence of) in top-level chart / values. -class ReleaseDiffTestCase(_BaseReleaseDiffTestCase, base.ArmadaTestCase): - def get_chart_to_update(self, chart): - return chart - - def test_same_input_no_diff(self): +class ReleaseDiffTestCase(base.ArmadaTestCase): + def test_same_input(self): diff = ReleaseDiff( - self.old_chart, self.old_values, self.make_chart(), - self.make_values()).get_diff() + mock.sentinel.chart, mock.sentinel.values, mock.sentinel.chart, + mock.sentinel.values).get_diff() self.assertFalse(diff) - def test_override_values_diff(self): - new_values = {'param': 'o2'} + def test_diff_input(self): diff = ReleaseDiff( - self.old_chart, self.old_values, self.old_chart, - new_values).get_diff() + mock.sentinel.old_chart, mock.sentinel.old_values, + mock.sentinel.new_chart, mock.sentinel.new_values).get_diff() self.assertTrue(diff) - - -# Test diffs in dependencies. -class DependencyReleaseDiffTestCase(_BaseReleaseDiffTestCase, - base.ArmadaTestCase): - def get_chart_to_update(self, chart): - return chart.dependencies[0] - - -# Test diffs in transitive dependencies. -class TransitiveDependencyReleaseDiffTestCase(_BaseReleaseDiffTestCase, - base.ArmadaTestCase): - def get_chart_to_update(self, chart): - return chart.dependencies[0].dependencies[0] diff --git a/armada/tests/unit/handlers/test_test.py b/armada/tests/unit/handlers/test_test.py index f22607d3..2ecfa035 100644 --- a/armada/tests/unit/handlers/test_test.py +++ b/armada/tests/unit/handlers/test_test.py @@ -15,58 +15,42 @@ import mock from armada import const +from armada.exceptions.helm_exceptions import HelmCommandException +from armada.handlers import helm from armada.handlers import test -from armada.handlers import tiller from armada.tests.unit import base -from armada.tests.test_utils import AttrDict -from armada.utils import helm class TestHandlerTestCase(base.ArmadaTestCase): - def _test_test_release_for_success(self, expected_success, results): - @mock.patch('armada.handlers.tiller.K8s') + def _test_test_release_for_success(self, expected_success, exception): + @mock.patch('armada.handlers.helm.K8s') def do_test(_): - tiller_obj = tiller.Tiller('host', '8080', None) + helm_obj = helm.Helm() release = 'release' - tiller_obj.test_release = mock.Mock() - tiller_obj.test_release.return_value = AttrDict( - **{'results': results}) + helm_obj.test_release = mock.Mock() + if exception: + helm_obj.test_release.side_effect = exception - test_handler = test.Test({}, release, tiller_obj) + test_handler = test.Test({}, release, helm_obj) success = test_handler.test_release_for_success() self.assertEqual(expected_success, success) do_test() - def test_no_results(self): - self._test_test_release_for_success(True, []) - - def test_unknown(self): - self._test_test_release_for_success( - False, [ - AttrDict(**{'status': helm.TESTRUN_STATUS_SUCCESS}), - AttrDict(**{'status': helm.TESTRUN_STATUS_UNKNOWN}) - ]) - def test_success(self): - self._test_test_release_for_success( - True, [AttrDict(**{'status': helm.TESTRUN_STATUS_SUCCESS})]) + self._test_test_release_for_success(True, None) def test_failure(self): self._test_test_release_for_success( - False, [ - AttrDict(**{'status': helm.TESTRUN_STATUS_SUCCESS}), - AttrDict(**{'status': helm.TESTRUN_STATUS_FAILURE}) - ]) + False, HelmCommandException(mock.Mock())) - def test_running(self): - self._test_test_release_for_success( - False, [ - AttrDict(**{'status': helm.TESTRUN_STATUS_SUCCESS}), - AttrDict(**{'status': helm.TESTRUN_STATUS_RUNNING}) - ]) + def test_exception(self): + def test(): + self._test_test_release_for_success(False, Exception()) + + self.assertRaises(Exception, test) def test_cg_disabled(self): """Test that tests are disabled when a chart group disables all @@ -74,8 +58,8 @@ class TestHandlerTestCase(base.ArmadaTestCase): """ test_handler = test.Test( chart={}, - release_name='release', - tiller=mock.Mock(), + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock(), cg_test_charts=False) assert test_handler.test_enabled is False @@ -86,8 +70,8 @@ class TestHandlerTestCase(base.ArmadaTestCase): """ test_handler = test.Test( chart={'test': True}, - release_name='release', - tiller=mock.Mock(), + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock(), cg_test_charts=False) assert test_handler.test_enabled is True @@ -100,8 +84,8 @@ class TestHandlerTestCase(base.ArmadaTestCase): chart={'test': { 'enabled': True }}, - release_name='release', - tiller=mock.Mock(), + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock(), cg_test_charts=False) assert test_handler.test_enabled is True @@ -112,8 +96,8 @@ class TestHandlerTestCase(base.ArmadaTestCase): """ test_handler = test.Test( chart={'test': False}, - release_name='release', - tiller=mock.Mock(), + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock(), cg_test_charts=True) assert test_handler.test_enabled is False @@ -126,8 +110,8 @@ class TestHandlerTestCase(base.ArmadaTestCase): chart={'test': { 'enabled': False }}, - release_name='release', - tiller=mock.Mock(), + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock(), cg_test_charts=True) assert test_handler.test_enabled is False @@ -138,8 +122,8 @@ class TestHandlerTestCase(base.ArmadaTestCase): """ test_handler = test.Test( chart={}, - release_name='release', - tiller=mock.Mock(), + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock(), cg_test_charts=False, enable_all=True) @@ -151,8 +135,8 @@ class TestHandlerTestCase(base.ArmadaTestCase): """ test_handler = test.Test( chart={'test': True}, - release_name='release', - tiller=mock.Mock(), + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock(), enable_all=True) assert test_handler.test_enabled is True @@ -165,8 +149,8 @@ class TestHandlerTestCase(base.ArmadaTestCase): chart={'test': { 'enabled': False }}, - release_name='release', - tiller=mock.Mock(), + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock(), enable_all=True) assert test_handler.test_enabled is True @@ -176,27 +160,21 @@ class TestHandlerTestCase(base.ArmadaTestCase): for a chart's test key. """ test_handler = test.Test( - chart={'test': False}, release_name='release', tiller=mock.Mock()) + chart={'test': False}, + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock()) assert not test_handler.test_enabled - def test_deprecated_test_key_true(self): - """Test that cleanup is enabled by default when tests are enabled using - the deprecated, boolean value for a chart's `test` key. - """ - test_handler = test.Test( - chart={'test': True}, release_name='release', tiller=mock.Mock()) - - assert test_handler.test_enabled is True - assert test_handler.cleanup is True - def test_deprecated_test_key_timeout(self): - """Test that the default Tiller timeout is used when tests are enabled + """Test that the default helm timeout is used when tests are enabled using the deprecated, boolean value for a chart's `test` key. """ - mock_tiller = mock.Mock() + mock_helm = mock.Mock() test_handler = test.Test( - chart={'test': True}, release_name='release', tiller=mock_tiller) + chart={'test': True}, + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock_helm) assert test_handler.timeout == const.DEFAULT_TEST_TIMEOUT @@ -208,86 +186,21 @@ class TestHandlerTestCase(base.ArmadaTestCase): chart={'test': { 'enabled': False }}, - release_name='release', - tiller=mock.Mock()) + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock()) assert test_handler.test_enabled is False - def test_tests_enabled(self): - """Test that cleanup is disabled (by default) when tests are enabled by - a chart's values using the `test.enabled` path. - """ - test_handler = test.Test( - chart={'test': { - 'enabled': True - }}, - release_name='release', - tiller=mock.Mock()) - - assert test_handler.test_enabled is True - assert test_handler.cleanup is False - - def test_tests_enabled_cleanup_enabled(self): - """Test that the test handler uses the values provided by a chart's - `test` key. - """ - test_handler = test.Test( - chart={'test': { - 'enabled': True, - 'options': { - 'cleanup': True - } - }}, - release_name='release', - tiller=mock.Mock()) - - assert test_handler.test_enabled is True - assert test_handler.cleanup is True - - def test_tests_enabled_cleanup_disabled(self): - """Test that the test handler uses the values provided by a chart's - `test` key. - """ - test_handler = test.Test( - chart={'test': { - 'enabled': True, - 'options': { - 'cleanup': False - } - }}, - release_name='release', - tiller=mock.Mock()) - - assert test_handler.test_enabled is True - assert test_handler.cleanup is False - def test_no_test_values(self): """Test that the default values are enforced when no chart `test` - values are provided (i.e. tests are enabled and cleanup is disabled). + values are provided (i.e. tests are enabled). """ test_handler = test.Test( - chart={}, release_name='release', tiller=mock.Mock()) + chart={}, + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock()) assert test_handler.test_enabled is True - assert test_handler.cleanup is False - - def test_override_cleanup(self): - """Test that a cleanup value passed to the Test handler (i.e. from the - API/CLI) takes precedence over a chart's `test.cleanup` value. - """ - test_handler = test.Test( - chart={'test': { - 'enabled': True, - 'options': { - 'cleanup': False - } - }}, - release_name='release', - tiller=mock.Mock(), - cleanup=True) - - assert test_handler.test_enabled is True - assert test_handler.cleanup is True def test_default_timeout_value(self): """Test that the default timeout value is used if a test timeout value, @@ -297,11 +210,10 @@ class TestHandlerTestCase(base.ArmadaTestCase): chart={'test': { 'enabled': True }}, - release_name='release', - tiller=mock.Mock(), - cleanup=True) + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock()) - assert test_handler.timeout == const.DEFAULT_TILLER_TIMEOUT + assert test_handler.timeout == helm.DEFAULT_HELM_TIMEOUT def test_timeout_value(self): """Test that a chart's test timeout value, `test.timeout` overrides the @@ -311,8 +223,7 @@ class TestHandlerTestCase(base.ArmadaTestCase): test_handler = test.Test( chart=chart, - release_name='release', - tiller=mock.Mock(), - cleanup=True) + release_id=helm.HelmReleaseId('release_ns', 'release'), + helm=mock.Mock()) assert test_handler.timeout is chart['test']['timeout'] diff --git a/armada/tests/unit/handlers/test_wait.py b/armada/tests/unit/handlers/test_wait.py index 81b38b0d..dd5be78c 100644 --- a/armada/tests/unit/handlers/test_wait.py +++ b/armada/tests/unit/handlers/test_wait.py @@ -16,6 +16,7 @@ import mock from armada import const from armada.exceptions import manifest_exceptions +from armada.handlers import helm from armada.handlers import wait from armada.tests.unit import base @@ -33,9 +34,8 @@ class ChartWaitTestCase(base.ArmadaTestCase): } return wait.ChartWait( k8s=mock.MagicMock(), - release_name='test-test', + release_id=helm.HelmReleaseId('test', 'test-test'), chart=chart, - namespace='test', k8s_wait_attempts=1, k8s_wait_attempt_sleep=1, timeout=timeout) @@ -215,10 +215,11 @@ class PodWaitTestCase(base.ArmadaTestCase): test_pods = [ mock_resource({ 'key': 'value', - 'helm.sh/hook': 'test-success' + 'helm.sh/hook': 'test' }), + mock_resource({'helm.sh/hook': 'test-success'}), mock_resource({'helm.sh/hook': 'test-failure'}), - mock_resource({'helm.sh/hook': 'test-success,pre-install'}), + mock_resource({'helm.sh/hook': 'test,pre-install'}), ] job_pods = [ mock_resource(owner_references=[mock.Mock(kind='Job')]), diff --git a/armada/utils/helm.py b/armada/utils/helm.py index d3f5cb8c..6b2eb6e0 100644 --- a/armada/utils/helm.py +++ b/armada/utils/helm.py @@ -18,7 +18,10 @@ TESTRUN_STATUS_FAILURE = 2 TESTRUN_STATUS_RUNNING = 3 HELM_HOOK_ANNOTATION = 'helm.sh/hook' -HELM_TEST_HOOKS = ['test-success', 'test-failure'] +# TODO: Eventually remove 'test-failure' as it is removed in Helm 3, +# leaving for now to ensure test runs leftover from Helm 2 get deleted +# and don't cause name conflicts. +HELM_TEST_HOOKS = ['test', 'test-success', 'test-failure'] def is_test_pod(pod): diff --git a/armada/utils/release.py b/armada/utils/release.py index bb6572b1..fd6ba0cc 100644 --- a/armada/utils/release.py +++ b/armada/utils/release.py @@ -14,7 +14,7 @@ import time -from armada.utils.helm import get_test_suite_run_success +import dateutil def release_prefixer(prefix, release): @@ -35,26 +35,24 @@ def label_selectors(labels): def get_release_status(release): """ - :param release: protobuf release object + :param release: helm release metadata :return: status name of release """ - status = release.info.status - return status.Code.Name(status.code) + return release['info']['status'] def get_last_test_result(release): """ - :param release: protobuf release object + :param release: helm release metadata - :return: status name of release + :return: whether tests are successful (no tests defined implies success) """ - - status = release.info.status - if not status.HasField('last_test_suite_run'): - return None - return get_test_suite_run_success(status.last_test_suite_run) + test_hooks = ( + hook for hook in release.get('hooks', []) if any( + e in ['test', 'test-success'] for e in hook['events'])) + return all(test['last_run']['phase'] == 'Succeeded' for test in test_hooks) def get_last_deployment_age(release): @@ -64,7 +62,8 @@ def get_last_deployment_age(release): :return: age in seconds of last deployment of release """ - last_deployed = release.info.last_deployed.seconds + last_deployed_str = release['info']['last_deployed'] + last_deployed = dateutil.parser.isoparse(last_deployed_str).timestamp() now = int(time.time()) last_deployment_age = now - last_deployed diff --git a/charts/armada/values.yaml b/charts/armada/values.yaml index d5f015b2..1b2e42d1 100644 --- a/charts/armada/values.yaml +++ b/charts/armada/values.yaml @@ -200,7 +200,7 @@ conf: 'armada:test_manifest': 'rule:admin_required' 'armada:test_release': 'rule:admin_required' 'armada:validate_manifest': 'rule:admin_viewer' - 'tiller:get_release': 'rule:admin_viewer' + 'armada:get_release': 'rule:admin_viewer' 'tiller:get_status': 'rule:admin_viewer' tiller: # If set to false then some form of Tiller needs to be provided diff --git a/doc/source/operations/documents/v2/document-authoring.rst b/doc/source/operations/documents/v2/document-authoring.rst index 52b19b40..4a71299c 100644 --- a/doc/source/operations/documents/v2/document-authoring.rst +++ b/doc/source/operations/documents/v2/document-authoring.rst @@ -328,8 +328,6 @@ Upgrade options to pass through directly to helm. +---------------+----------+---------------------------------------------------------------+ | force | boolean | Same as Helm CLI. | +---------------+----------+---------------------------------------------------------------+ -| recreate_pods | boolean | Same as Helm CLI. | -+---------------+----------+---------------------------------------------------------------+ Upgrade - Pre ~~~~~~~~~~~~~ diff --git a/doc/source/operations/exceptions/chartbuilder-exceptions.inc b/doc/source/operations/exceptions/chartbuilder-exceptions.inc index 44c5ea10..cdefef7d 100644 --- a/doc/source/operations/exceptions/chartbuilder-exceptions.inc +++ b/doc/source/operations/exceptions/chartbuilder-exceptions.inc @@ -19,27 +19,7 @@ Chartbuilder Exceptions .. currentmodule:: armada.exceptions.chartbuilder_exceptions -.. autoexception:: DependencyException - :members: - :show-inheritance: - :undoc-members: - -.. autoexception:: FilesLoadException - :members: - :show-inheritance: - :undoc-members: - .. autoexception:: HelmChartBuildException :members: :show-inheritance: :undoc-members: - -.. autoexception:: IgnoredFilesLoadException - :members: - :show-inheritance: - :undoc-members: - -.. autoexception:: MetadataLoadException - :members: - :show-inheritance: - :undoc-members: diff --git a/etc/armada/policy.yaml b/etc/armada/policy.yaml index 3d420473..f81b4da5 100644 --- a/etc/armada/policy.yaml +++ b/etc/armada/policy.yaml @@ -26,10 +26,10 @@ # POST /api/v1.0/tests/ #"armada:test_manifest": "rule:admin_required" +# Get helm releases +# GET /api/v1.0/releases/ +#"armada:get_release": "rule:admin_viewer" + # Get Tiller status # GET /api/v1.0/status/ #"tiller:get_status": "rule:admin_viewer" - -# Get Tiller release -# GET /api/v1.0/releases/ -#"tiller:get_release": "rule:admin_viewer" diff --git a/examples/podinfo.yaml b/examples/podinfo.yaml new file mode 100644 index 00000000..8b3e3101 --- /dev/null +++ b/examples/podinfo.yaml @@ -0,0 +1,59 @@ +--- +schema: armada/Chart/v1 +metadata: + schema: metadata/Document/v1 + name: helm-toolkit +data: + chart_name: helm-toolkit + release: helm-toolkit + namespace: helm-tookit + install: + no_hooks: false + upgrade: + no_hooks: false + values: {} + source: + type: tar + location: https://tarballs.opendev.org/openstack/openstack-helm-infra/helm-toolkit-0.2.23.tgz + subpath: helm-toolkit +--- +schema: armada/Chart/v1 +metadata: + schema: metadata/Document/v1 + name: podinfo +data: + chart_name: podinfo + release: podinfo + namespace: podinfo + install: + no_hooks: false + upgrade: + no_hooks: false + values: + foo: bar + dependencies: + - helm-toolkit + source: + type: local + location: /podinfo/charts/podinfo + subpath: . + reference: master +--- +schema: armada/ChartGroup/v1 +metadata: + schema: metadata/Document/v1 + name: podinfo +data: + description: Deploys Simple Service + sequenced: False + chart_group: + - podinfo +--- +schema: armada/Manifest/v1 +metadata: + schema: metadata/Document/v1 + name: podinfo +data: + release_prefix: podinfo + chart_groups: + - podinfo