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 <seaneagan1@gmail.com>
Change-Id: I20ff40ba55197de3d37e5fd647e7d2524a53248f

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):

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()

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

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)

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

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()),
     ]

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()

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):

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'
+        }]),
 ]
 
 

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'
-        }]),
 ]
 
 

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)
 
 

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)

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)

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.'''
 

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))

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)

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

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()

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)

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

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")

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}

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)

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

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'
 

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

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

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."""

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)

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)
 
 

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

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"
 """

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)

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()))

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"
             },

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]

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']

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')]),

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):

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
 

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

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
 ~~~~~~~~~~~~~

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:

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"

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