From ae690ef82853e1209b07e55ea2513f8f52c4d68a Mon Sep 17 00:00:00 2001 From: Sean Eagan Date: Fri, 8 Jun 2018 17:23:28 -0500 Subject: [PATCH] Expose helm's upgrade/rollback force and recreate pods flags This exposes helm's force and recreate pods flags for upgrade and rollback. It exposes in the chart manifest an options field underneath the upgrade field to hold options to pass through to helm, and initializes it with these two flags. Since rollback is currently a standalone operation which does not consume manifests, these flags are directly exposed as api and cli arguments there. Change-Id: If65c1e97d437d9cf9d5838111fd485c80c76aa1d --- armada/api/controller/rollback.py | 4 +- armada/cli/rollback.py | 23 +++- armada/handlers/armada.py | 6 +- armada/handlers/tiller.py | 53 ++++--- armada/schemas/armada-chart-schema.yaml | 8 ++ .../unit/api/test_rollback_controller.py | 40 +++++- armada/tests/unit/handlers/test_armada.py | 21 ++- armada/tests/unit/handlers/test_tiller.py | 130 +++++++++++++++++- swagger/swaggerV2-api.yaml | 18 +++ swagger/swaggerV3-api.yaml | 20 +++ 10 files changed, 296 insertions(+), 27 deletions(-) diff --git a/armada/api/controller/rollback.py b/armada/api/controller/rollback.py index c94f8a4b..56a797c6 100644 --- a/armada/api/controller/rollback.py +++ b/armada/api/controller/rollback.py @@ -45,7 +45,9 @@ class Rollback(api.BaseResource): release, req.get_param_as_int('version') or 0, wait=req.get_param_as_bool('wait'), - timeout=req.get_param_as_int('timeout') or 0) + timeout=req.get_param_as_int('timeout') or 0, + force=req.get_param_as_bool('force'), + recreate_pods=req.get_param_as_bool('recreate_pods')) resp.body = json.dumps( { diff --git a/armada/cli/rollback.py b/armada/cli/rollback.py index 76c3a080..02395b70 100644 --- a/armada/cli/rollback.py +++ b/armada/cli/rollback.py @@ -73,15 +73,24 @@ SHORT_DESC = "Command performs a release rollback." @click.option('--wait', help=("Wait until rollback is complete before returning."), is_flag=True) +@click.option('--force', + help=("Force resource update through delete/recreate if" + " needed."), + is_flag=True) +@click.option('--recreate-pods', + help=("Restarts pods for the resource if applicable."), + is_flag=True) @click.option('--debug', help="Enable debug logging.", is_flag=True) @click.pass_context def rollback_charts(ctx, release, version, dry_run, tiller_host, tiller_port, - tiller_namespace, timeout, wait, debug): + tiller_namespace, timeout, wait, force, recreate_pods, + debug): CONF.debug = debug Rollback(ctx, release, version, dry_run, tiller_host, tiller_port, - tiller_namespace, timeout, wait).safe_invoke() + tiller_namespace, timeout, wait, force, + recreate_pods).safe_invoke() class Rollback(CliAction): @@ -94,7 +103,9 @@ class Rollback(CliAction): tiller_port, tiller_namespace, timeout, - wait): + wait, + force, + recreate_pods): super(Rollback, self).__init__() self.ctx = ctx self.release = release @@ -105,6 +116,8 @@ class Rollback(CliAction): self.tiller_namespace = tiller_namespace self.timeout = timeout self.wait = wait + self.force = force + self.recreate_pods = recreate_pods def invoke(self): tiller = Tiller( @@ -115,7 +128,9 @@ class Rollback(CliAction): self.release, self.version, wait=self.wait, - timeout=self.timeout) + timeout=self.timeout, + force=self.force, + recreate_pods=self.recreate_pods) self.output(response) diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index 34774261..15a79a49 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -333,6 +333,8 @@ class Armada(object): upgrade = chart.get('upgrade', {}) disable_hooks = upgrade.get('no_hooks', False) + force = upgrade.get('force', False) + recreate_pods = upgrade.get('recreate_pods', False) LOG.info("Checking Pre/Post Actions") if upgrade: @@ -374,7 +376,9 @@ class Armada(object): disable_hooks=disable_hooks, values=yaml.safe_dump(values), wait=this_chart_should_wait, - timeout=timer) + timeout=timer, + force=force, + recreate_pods=recreate_pods) if this_chart_should_wait: self._wait_until_ready( diff --git a/armada/handlers/tiller.py b/armada/handlers/tiller.py index dc29b4d1..37fb9832 100644 --- a/armada/handlers/tiller.py +++ b/armada/handlers/tiller.py @@ -51,8 +51,18 @@ CONF = cfg.CONF LOG = logging.getLogger(__name__) -class TillerResult(object): +class CommonEqualityMixin(object): + def __eq__(self, other): + return (isinstance(other, self.__class__) and + self.__dict__ == other.__dict__) + + def __ne__(self, other): + return not self.__eq__(other) + + +class TillerResult(CommonEqualityMixin): '''Object to hold Tiller results for Armada.''' + def __init__(self, release, namespace, status, description, version): self.release = release self.namespace = namespace @@ -318,15 +328,18 @@ class Tiller(object): disable_hooks=False, values=None, wait=False, - timeout=None): + timeout=None, + force=False, + recreate_pods=False): ''' Update a Helm Release ''' timeout = self._check_timeout(wait, timeout) - LOG.info('Helm update release%s: wait=%s, timeout=%s', + LOG.info('Helm update release%s: wait=%s, timeout=%s, force=%s, ' + 'recreate_pods=%s', (' (dry run)' if self.dry_run else ''), - wait, timeout) + wait, timeout, force, recreate_pods) if values is None: values = Config(raw='') @@ -336,6 +349,7 @@ class Tiller(object): self._pre_update_actions(pre_actions, release, namespace, chart, disable_hooks, values, timeout) + update_msg = None # build release install request try: stub = ReleaseServiceStub(self.channel) @@ -346,21 +360,14 @@ class Tiller(object): values=values, name=release, wait=wait, - timeout=timeout) + timeout=timeout, + force=force, + recreate=recreate_pods) update_msg = stub.UpdateRelease( release_request, timeout + GRPC_EPSILON, metadata=self.metadata) - tiller_result = TillerResult( - update_msg.release.name, - update_msg.release.namespace, - update_msg.release.info.status.Code.Name( - update_msg.release.info.status.code), - update_msg.release.info.Description, - update_msg.release.version) - - return tiller_result except Exception: LOG.exception('Error while updating release %s', release) status = self.get_release_status(release) @@ -368,6 +375,16 @@ class Tiller(object): self._post_update_actions(post_actions, namespace) + tiller_result = TillerResult( + update_msg.release.name, + update_msg.release.namespace, + update_msg.release.info.status.Code.Name( + update_msg.release.info.status.code), + update_msg.release.info.Description, + update_msg.release.version) + + return tiller_result + def install_release(self, chart, release, namespace, values=None, wait=False, @@ -686,7 +703,9 @@ class Tiller(object): release_name, version, wait=False, - timeout=None): + timeout=None, + force=False, + recreate_pods=False): ''' Rollback a helm release. ''' @@ -704,7 +723,9 @@ class Tiller(object): version=version, dry_run=self.dry_run, wait=wait, - timeout=timeout) + timeout=timeout, + force=force, + recreate=recreate_pods) rollback_msg = stub.RollbackRelease( rollback_request, diff --git a/armada/schemas/armada-chart-schema.yaml b/armada/schemas/armada-chart-schema.yaml index 7e15e067..10efa428 100644 --- a/armada/schemas/armada-chart-schema.yaml +++ b/armada/schemas/armada-chart-schema.yaml @@ -109,6 +109,14 @@ data: properties: create: $ref: '#/definitions/hook_action' + options: + type: object + properties: + force: + type: boolean + recreate_pods: + type: boolean + additionalProperties: false required: - no_hooks additionalProperties: false diff --git a/armada/tests/unit/api/test_rollback_controller.py b/armada/tests/unit/api/test_rollback_controller.py index 793e22a9..59144746 100644 --- a/armada/tests/unit/api/test_rollback_controller.py +++ b/armada/tests/unit/api/test_rollback_controller.py @@ -32,7 +32,45 @@ class RollbackReleaseControllerTest(base.BaseControllerTest): rollback_release = mock_tiller.return_value.rollback_release rollback_release.return_value = None - resp = self.app.simulate_post('/api/v1.0/rollback/test-release') + tiller_host = 'host' + tiller_port = '8080' + tiller_namespace = 'tn' + release = 'test-release' + version = '2' + dry_run = 'false' + wait = 'true' + timeout = '123' + force = 'true' + recreate_pods = 'true' + + resp = self.app.simulate_post( + '/api/v1.0/rollback/{}'.format(release), + params={ + 'tiller_host': tiller_host, + 'tiller_port': tiller_port, + 'tiller_namespace': tiller_namespace, + 'dry_run': dry_run, + 'version': version, + 'wait': wait, + 'timeout': timeout, + 'force': force, + 'recreate_pods': recreate_pods + }) + + mock_tiller.assert_called_once_with( + tiller_host=tiller_host, + tiller_port=8080, + tiller_namespace=tiller_namespace, + dry_run=False) + + rollback_release.assert_called_once_with( + release, + 2, + wait=True, + timeout=123, + force=True, + recreate_pods=True) + self.assertEqual(200, resp.status_code) self.assertEqual('Rollback of test-release complete.', json.loads(resp.text)['message']) diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index 8b82797a..e2b9962b 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -59,6 +59,11 @@ data: dependencies: [] wait: timeout: 10 + upgrade: + no_hooks: false + options: + force: true + recreate_pods: true --- schema: armada/Chart/v1 metadata: @@ -129,6 +134,13 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): 'values': {}, 'wait': { 'timeout': 10 + }, + 'upgrade': { + 'no_hooks': False, + 'options': { + 'force': True, + 'recreate_pods': True + } } } } @@ -279,6 +291,11 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): ) ) else: + upgrade = chart.get('upgrade', {}) + disable_hooks = upgrade.get('no_hooks', False) + force = upgrade.get('force', False) + recreate_pods = upgrade.get('recreate_pods', False) + expected_update_release_calls.append( mock.call( mock_chartbuilder().get_helm_chart(), @@ -288,7 +305,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): chart['namespace'], pre_actions={}, post_actions={}, - disable_hooks=False, + disable_hooks=disable_hooks, + force=force, + recreate_pods=recreate_pods, values=yaml.safe_dump(chart['values']), wait=this_chart_should_wait, timeout=chart['wait']['timeout'] diff --git a/armada/tests/unit/handlers/test_tiller.py b/armada/tests/unit/handlers/test_tiller.py index bf9f014c..0d330331 100644 --- a/armada/tests/unit/handlers/test_tiller.py +++ b/armada/tests/unit/handlers/test_tiller.py @@ -13,6 +13,7 @@ # limitations under the License. import mock +from mock import MagicMock from armada.exceptions import tiller_exceptions as ex from armada.handlers import tiller @@ -289,9 +290,29 @@ class TillerTestCase(base.ArmadaTestCase): mock_release_service_stub.return_value.RollbackRelease\ .return_value = {} - tiller_obj = tiller.Tiller('host', '8080', None) + dry_run = True - self.assertIsNone(tiller_obj.rollback_release('release', 0)) + tiller_obj = tiller.Tiller('host', '8080', None, dry_run=dry_run) + + release = 'release' + version = 0 + wait = True + timeout = 123 + recreate_pods = True + force = True + + self.assertIsNone(tiller_obj.rollback_release( + release, version, wait=wait, timeout=timeout, force=force, + recreate_pods=recreate_pods)) + + mock_rollback_release_request.assert_called_once_with( + name=release, + version=version, + dry_run=dry_run, + wait=wait, + timeout=timeout, + force=force, + recreate=recreate_pods) mock_release_service_stub.assert_called_once_with( tiller_obj.channel) @@ -299,6 +320,109 @@ class TillerTestCase(base.ArmadaTestCase): RollbackRelease rollback_release_stub.assert_called_once_with( - mock_rollback_release_request.return_value, tiller_obj.timeout + + mock_rollback_release_request.return_value, timeout + tiller.GRPC_EPSILON, metadata=tiller_obj.metadata) + + @mock.patch('armada.handlers.tiller.K8s') + @mock.patch('armada.handlers.tiller.grpc') + @mock.patch('armada.handlers.tiller.Config') + @mock.patch.object(tiller, 'UpdateReleaseRequest') + @mock.patch.object(tiller, 'ReleaseServiceStub') + def test_update_release(self, mock_release_service_stub, + mock_update_release_request, mock_config, + _, __): + release = 'release' + chart = {} + namespace = 'namespace' + code = 0 + status = 'DEPLOYED' + description = 'desc' + version = 2 + values = mock_config(raw=None) + mock_release_service_stub.return_value.UpdateRelease.return_value =\ + AttrDict(**{ + 'release': AttrDict(**{ + 'name': release, + 'namespace': namespace, + 'info': AttrDict(**{ + 'status': AttrDict(**{ + 'Code': AttrDict(**{ + 'Name': lambda c: + status if c == code else None + }), + 'code': code + }), + 'Description': description + }), + 'version': version + }) + }) + + tiller_obj = tiller.Tiller('host', '8080', None, dry_run=False) + + # TODO: Test these methods as well, either by unmocking, or adding + # separate tests for them. + tiller_obj._pre_update_actions = MagicMock() + tiller_obj._post_update_actions = MagicMock() + + pre_actions = {} + post_actions = {} + disable_hooks = False + wait = True + timeout = 123 + force = True + recreate_pods = True + + result = tiller_obj.update_release( + chart, release, namespace, + pre_actions=pre_actions, + post_actions=post_actions, + disable_hooks=disable_hooks, + values=values, + wait=wait, + timeout=timeout, + force=force, + recreate_pods=recreate_pods) + + tiller_obj._pre_update_actions.assert_called_once_with( + pre_actions, release, namespace, chart, disable_hooks, values, + timeout) + tiller_obj._post_update_actions.assert_called_once_with( + post_actions, namespace) + + mock_update_release_request.assert_called_once_with( + chart=chart, + name=release, + dry_run=tiller_obj.dry_run, + disable_hooks=False, + values=values, + wait=wait, + timeout=timeout, + force=force, + recreate=recreate_pods) + + mock_release_service_stub.assert_called_once_with( + tiller_obj.channel) + update_release_stub = mock_release_service_stub.return_value. \ + UpdateRelease + + update_release_stub.assert_called_once_with( + mock_update_release_request.return_value, timeout + + tiller.GRPC_EPSILON, + metadata=tiller_obj.metadata) + + expected_result = tiller.TillerResult( + release, + namespace, + status, + description, + version) + + self.assertEqual(expected_result, result) + + +class AttrDict(dict): + def __init__(self, *args, **kwargs): + super(AttrDict, self).__init__(*args, **kwargs) + self.__dict__ = self diff --git a/swagger/swaggerV2-api.yaml b/swagger/swaggerV2-api.yaml index f2307366..b611d769 100644 --- a/swagger/swaggerV2-api.yaml +++ b/swagger/swaggerV2-api.yaml @@ -197,6 +197,8 @@ paths: - $ref: "#/parameters/dry-run" - $ref: "#/parameters/wait" - $ref: "#/parameters/timeout" + - $ref: "#/parameters/force" + - $ref: "#/parameters/recreate-pods" responses: '200': $ref: "#/responses/response-post-rollback-release" @@ -307,6 +309,22 @@ parameters: description: Specifies time in seconds Tiller should wait for the action to complete before timing out. default: 3600 + force: + in: query + name: force + required: false + type: boolean + description: Specifies whether to force resource update through + delete/recreate if needed. + default: False + recreate-pods: + in: query + name: recreate_pods + required: false + type: boolean + description: Specifies whether to restart pods for the resource if + applicable. + default: False responses: # HTTP error responses err-bad-request: diff --git a/swagger/swaggerV3-api.yaml b/swagger/swaggerV3-api.yaml index bcfea6bc..8308b878 100644 --- a/swagger/swaggerV3-api.yaml +++ b/swagger/swaggerV3-api.yaml @@ -221,6 +221,8 @@ paths: - $ref: "#/components/parameters/dry-run" - $ref: "#/components/parameters/wait" - $ref: "#/components/parameters/timeout" + - $ref: "#/components/parameters/force" + - $ref: "#/components/parameters/recreate-pods" responses: '200': $ref: "#/components/responses/response-post-rollback-release" @@ -356,6 +358,24 @@ components: description: Specifies whether Tiller should wait until all charts are deployed schema: type: boolean + force: + in: query + name: force + required: false + description: Specifies whether to force resource update through + delete/recreate if needed. + schema: + type: boolean + default: false + recreate-pods: + in: query + name: recreate_pods + required: false + description: Specifies whether to restart pods for the resource if + applicable. + schema: + type: boolean + default: false requestBodies: apply-body: required: true