From 47ebd27cad70e1b3e83519b029aa0b7455252a69 Mon Sep 17 00:00:00 2001 From: Sean Eagan Date: Fri, 11 Jan 2019 13:44:13 -0600 Subject: [PATCH] Add configurability of delete timeout Previously the timeout for deleting chart releases was 300s and not configurable, this patchset makes it so via a new `delete.timeout` property in the `armada/Chart/v1` schema. Helm releases deleted which do not correspond to documents in this schema still do not use a configurable timeout. Those will be considered separately. This also includes a minor logging fix. Change-Id: Ia588faaafd18a3ac00eed3cda2f0556ffcec82c9 --- armada/cli/delete.py | 20 +++---- armada/const.py | 1 + armada/handlers/chart_delete.py | 52 +++++++++++++++++++ armada/handlers/chart_deploy.py | 5 +- armada/handlers/tiller.py | 19 +++++-- armada/schemas/armada-chart-schema.yaml | 5 ++ armada/tests/unit/handlers/test_armada.py | 5 +- .../operations/guide-build-armada-yaml.rst | 10 ++++ 8 files changed, 101 insertions(+), 16 deletions(-) create mode 100644 armada/handlers/chart_delete.py diff --git a/armada/cli/delete.py b/armada/cli/delete.py index c54d3b39..bb92d91f 100644 --- a/armada/cli/delete.py +++ b/armada/cli/delete.py @@ -19,6 +19,7 @@ from oslo_config import cfg from armada.cli import CliAction from armada import const +from armada.handlers.chart_delete import ChartDelete from armada.handlers.manifest import Manifest from armada.handlers.tiller import Tiller from armada.utils.release import release_prefixer @@ -108,13 +109,13 @@ class DeleteChartManifest(CliAction): if not self.ctx.obj.get('api', False): for r in target_releases: self.logger.info("Deleting release %s", r) - tiller.uninstall_release(r, purge=self.purge) + tiller.delete_release(r, purge=self.purge) else: raise NotImplementedError() if self.manifest: - target_releases = [] + target_deletes = [] with open(self.manifest) as f: documents = list(yaml.safe_load_all(f.read())) @@ -126,11 +127,11 @@ class DeleteChartManifest(CliAction): for group in armada_obj.get(const.KEYWORD_ARMADA).get( const.KEYWORD_GROUPS): for ch in group.get(const.KEYWORD_CHARTS): + chart = ch.get('chart') release_name = release_prefixer( - prefix, - ch.get('chart').get('release')) + prefix, chart.get('release')) if release_name in known_release_names: - target_releases.append(release_name) + target_deletes.append((chart, release_name)) except yaml.YAMLError as e: mark = e.problem_mark self.logger.info( @@ -138,14 +139,15 @@ class DeleteChartManifest(CliAction): "Error position: (%s:%s)", e.problem, mark.line + 1, mark.column + 1) - if not target_releases: + if not target_deletes: self.logger.info("There's no release to delete.") return if not self.ctx.obj.get('api', False): - for r in target_releases: - self.logger.info("Deleting release %s", r) - tiller.uninstall_release(r, purge=self.purge) + for chart, release in target_deletes: + chart_delete = ChartDelete( + chart, release, tiller, purge=self.purge) + chart_delete.delete() else: raise NotImplementedError() diff --git a/armada/const.py b/armada/const.py index e2538812..03dd16b5 100644 --- a/armada/const.py +++ b/armada/const.py @@ -27,6 +27,7 @@ DEFAULT_CHART_TIMEOUT = 900 # Tiller DEFAULT_TILLER_TIMEOUT = 300 +DEFAULT_DELETE_TIMEOUT = DEFAULT_TILLER_TIMEOUT STATUS_UNKNOWN = 'UNKNOWN' STATUS_DEPLOYED = 'DEPLOYED' STATUS_DELETED = 'DELETED' diff --git a/armada/handlers/chart_delete.py b/armada/handlers/chart_delete.py new file mode 100644 index 00000000..efb8abcf --- /dev/null +++ b/armada/handlers/chart_delete.py @@ -0,0 +1,52 @@ +# Copyright 2019 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 import const + + +class ChartDelete(object): + + def __init__(self, chart, release_name, tiller, 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 purge: Whether to purge the release + + :type chart: object + :type release_name: str + :type tiller: Tiller object + :type purge: bool + """ + + self.chart = chart + self.release_name = release_name + self.tiller = tiller + self.purge = purge + self.delete_config = self.chart.get('delete', {}) + # TODO(seaneagan): Consider allowing this to be a percentage of the + # chart's `wait.timeout` so that the timeouts can scale together, and + # likely default to some reasonable value, e.g. "50%". + self.timeout = self.delete_config.get('timeout', + const.DEFAULT_DELETE_TIMEOUT) + + def get_timeout(self): + return self.timeout + + def delete(self): + """Delete the release associated with the chart" + """ + self.tiller.uninstall_release( + self.release_name, timeout=self.get_timeout(), purge=self.purge) diff --git a/armada/handlers/chart_deploy.py b/armada/handlers/chart_deploy.py index b160f167..ba31b604 100644 --- a/armada/handlers/chart_deploy.py +++ b/armada/handlers/chart_deploy.py @@ -20,6 +20,7 @@ from armada import const from armada.exceptions import armada_exceptions from armada.handlers.chartbuilder import ChartBuilder from armada.handlers.release_diff import ReleaseDiff +from armada.handlers.chart_delete import ChartDelete from armada.handlers.test import Test from armada.handlers.wait import ChartWait from armada.exceptions import tiller_exceptions @@ -204,7 +205,9 @@ class ChartDeploy(object): # Purge the release LOG.info('Purging release %s with status %s', release_name, status) - self.tiller.uninstall_release(release_name) + chart_delete = ChartDelete(chart, release_name, + self.tiller) + chart_delete.delete() result['purge'] = release_name timer = int(round(deadline - time.time())) diff --git a/armada/handlers/tiller.py b/armada/handlers/tiller.py index 29c2b6cf..dc0a9bfb 100644 --- a/armada/handlers/tiller.py +++ b/armada/handlers/tiller.py @@ -580,14 +580,22 @@ class Tiller(object): LOG.exception('Failed to get Tiller version.') raise ex.TillerVersionException() - def uninstall_release(self, release, disable_hooks=False, purge=True): + def uninstall_release(self, + release, + disable_hooks=False, + purge=True, + timeout=None): ''' :param: release - Helm chart release name :param: purge - deep delete of chart + :param: timeout - timeout for the tiller call Deletes a Helm chart from Tiller ''' + if timeout is None: + timeout = const.DEFAULT_DELETE_TIMEOUT + # Helm client calls ReleaseContent in Delete dry-run scenario if self.dry_run: content = self.get_release_content(release) @@ -601,16 +609,17 @@ class Tiller(object): try: stub = ReleaseServiceStub(self.channel) LOG.info( - "Uninstall %s release with disable_hooks=%s, " - "purge=%s flags", release, disable_hooks, purge) + "Delete %s release with disable_hooks=%s, " + "purge=%s, timeout=%s flags", release, disable_hooks, purge, + timeout) release_request = UninstallReleaseRequest( name=release, disable_hooks=disable_hooks, purge=purge) return stub.UninstallRelease( - release_request, self.timeout, metadata=self.metadata) + release_request, timeout, metadata=self.metadata) except Exception: - LOG.exception('Error while uninstalling release %s', release) + LOG.exception('Error while deleting release %s', release) status = self.get_release_status(release) raise ex.ReleaseException(release, status, 'Delete') diff --git a/armada/schemas/armada-chart-schema.yaml b/armada/schemas/armada-chart-schema.yaml index cde89215..98018f49 100644 --- a/armada/schemas/armada-chart-schema.yaml +++ b/armada/schemas/armada-chart-schema.yaml @@ -126,6 +126,11 @@ data: - location - subpath - type + delete: + type: object + properties: + timeout: + type: integer install: # NOTE(sh8121att) Not clear that this key is actually used # in the code. Will leave it here for backward compatabilities diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index 110e78c0..2be30682 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -412,7 +412,10 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): protected = chart.get('protected', {}) if not protected: expected_uninstall_release_calls.append( - mock.call(release_name)) + mock.call( + release_name, + purge=True, + timeout=const.DEFAULT_DELETE_TIMEOUT)) expected_install_release_calls.append( mock.call( mock_chartbuilder().get_helm_chart(), diff --git a/doc/source/operations/guide-build-armada-yaml.rst b/doc/source/operations/guide-build-armada-yaml.rst index 5898a97a..2743cb53 100644 --- a/doc/source/operations/guide-build-armada-yaml.rst +++ b/doc/source/operations/guide-build-armada-yaml.rst @@ -110,6 +110,8 @@ Chart +-----------------+----------+---------------------------------------------------------------------------------------+ | upgrade | object | upgrade the chart managed by the armada yaml | +-----------------+----------+---------------------------------------------------------------------------------------+ +| delete | object | See Delete_. | ++-----------------+----------+---------------------------------------------------------------------------------------+ | values | object | override any default values in the charts | +-----------------+----------+---------------------------------------------------------------------------------------+ | source | object | provide a path to a ``git repo``, ``local dir``, or ``tarball url`` chart | @@ -301,6 +303,14 @@ Chart Example reference: master dependencies: [] +Delete +^^^^^^ + ++-------------+----------+-----------------------------------------------------------------------------------+ +| keyword | type | action | ++=============+==========+===================================================================================+ +| timeout | integer | time (in seconds) to wait for chart to be deleted | ++-------------+----------+-----------------------------------------------------------------------------------+ Source ^^^^^^