From 44b5ae89f6788bbf7564b564a8c67aec806a9b08 Mon Sep 17 00:00:00 2001 From: Jude Cross Date: Tue, 11 Dec 2018 16:30:49 -0800 Subject: [PATCH] Add force to action_update This patch adds the force query parameter to action_update. When force is set to true on an action cancel request the action status will be updated in the database to cancelled rather than signaled to cancel. This will also update any child actions to update their status in the database to cancelled. Change-Id: I45ab4a6ad57e3bfe791da1d2e70d3b2ed3ad9a21 --- api-ref/source/actions.inc | 1 + api-ref/source/parameters.yaml | 10 +++- senlin/api/openstack/v1/actions.py | 9 ++++ senlin/common/consts.py | 6 +++ senlin/engine/actions/base.py | 32 ++++++++++++ senlin/engine/service.py | 7 ++- senlin/objects/requests/actions.py | 3 +- .../unit/api/openstack/v1/test_actions.py | 34 +++++++++++++ senlin/tests/unit/api/shared.py | 10 ++-- .../unit/engine/actions/test_action_base.py | 49 +++++++++++++++++++ .../tests/unit/engine/service/test_actions.py | 2 +- 11 files changed, 154 insertions(+), 9 deletions(-) diff --git a/api-ref/source/actions.inc b/api-ref/source/actions.inc index d0f2e56d4..072e262b5 100644 --- a/api-ref/source/actions.inc +++ b/api-ref/source/actions.inc @@ -163,6 +163,7 @@ Request Parameters - action_id: action_id_url - action: action - status: action_status_update + - force: action_update_force_query Request Example --------------- diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 344ddd092..b1f228415 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -131,6 +131,12 @@ action_status_query: description: | Filters the results by the ``status`` property of an action object. +action_update_force_query: + type: boolean + in: query + description: | + A boolean indicating if the action update request should be forced. + cluster_identity_query: type: string in: query @@ -358,11 +364,11 @@ action_status: A string representation of the current status of the action. action_status_update: - type: object + type: string in: body required: True description: | - A string representation of the action status to update CANCELLED is + A string representation of the action status to update. CANCELLED is the only valid status at this time. action_target: diff --git a/senlin/api/openstack/v1/actions.py b/senlin/api/openstack/v1/actions.py index ebbf5229d..8b8a6962f 100644 --- a/senlin/api/openstack/v1/actions.py +++ b/senlin/api/openstack/v1/actions.py @@ -107,6 +107,15 @@ class ActionController(wsgi.Controller): if data is None: raise exc.HTTPBadRequest(_("Malformed request data, missing " "'action' key in request body.")) + force_update = req.params.get('force') + + if force_update is not None: + force = util.parse_bool_param(consts.ACTION_UPDATE_FORCE, + force_update) + else: + force = False + + data['force'] = force data['identity'] = action_id obj = util.parse_request('ActionUpdateRequest', req, data) diff --git a/senlin/common/consts.py b/senlin/common/consts.py index ee26a9a37..1f961ed98 100755 --- a/senlin/common/consts.py +++ b/senlin/common/consts.py @@ -268,6 +268,12 @@ ACTION_STATUSES = ( 'SUSPENDED', ) +ACTION_PARAMS = ( + ACTION_UPDATE_FORCE, +) = ( + 'force', +) + EVENT_LEVELS = { 'CRITICAL': logging.CRITICAL, 'ERROR': logging.ERROR, diff --git a/senlin/engine/actions/base.py b/senlin/engine/actions/base.py index 86d113173..bad2e4c0b 100755 --- a/senlin/engine/actions/base.py +++ b/senlin/engine/actions/base.py @@ -368,6 +368,38 @@ class Action(object): action.set_status(action.RES_CANCEL, 'Action execution cancelled') + def force_cancel(self): + """Force the action and any depended actions to cancel. + + If the action or any depended actions are in status 'INIT', 'WAITING', + 'READY', 'RUNNING', or 'WAITING_LIFECYCLE_COMPLETION' immediately + update their status to cancelled. This should only be used if an action + is stuck/dead and has no expectation of ever completing. + + :raises: `ActionImmutable` if the action is in an unchangeable state + """ + expected = (self.INIT, self.WAITING, self.READY, self.RUNNING, + self.WAITING_LIFECYCLE_COMPLETION) + if self.status not in expected: + raise exception.ActionImmutable(id=self.id[:8], expected=expected, + actual=self.status) + LOG.debug('Forcing action %s to cancel.', self.id) + self.set_status(self.RES_CANCEL, 'Action execution force cancelled') + + depended = dobj.Dependency.get_depended(self.context, self.id) + if not depended: + return + + for child in depended: + # Force cancel all dependant actions + action = self.load(self.context, action_id=child) + if action.status in (action.INIT, action.WAITING, action.READY, + action.RUNNING, + action.WAITING_LIFECYCLE_COMPLETION): + LOG.debug('Forcing action %s to cancel.', action.id) + action.set_status(action.RES_CANCEL, + 'Action execution force cancelled') + def execute(self, **kwargs): """Execute the action. diff --git a/senlin/engine/service.py b/senlin/engine/service.py index 766d34be4..3c93f3d28 100755 --- a/senlin/engine/service.py +++ b/senlin/engine/service.py @@ -2315,8 +2315,11 @@ class EngineService(service.Service): if req.status == consts.ACTION_CANCELLED: action = action_mod.Action.load(ctx, req.identity, project_safe=False) - LOG.info("Signaling action '%s' to Cancel.", req.identity) - action.signal_cancel() + if req.force: + action.force_cancel() + else: + LOG.info("Signaling action '%s' to Cancel.", req.identity) + action.signal_cancel() else: msg = ("Unknown status %(status)s for action %(action)s" % {"status": req.status, "action": req.identity}) diff --git a/senlin/objects/requests/actions.py b/senlin/objects/requests/actions.py index 4e838750a..16b42917b 100644 --- a/senlin/objects/requests/actions.py +++ b/senlin/objects/requests/actions.py @@ -75,5 +75,6 @@ class ActionUpdateRequest(base.SenlinObject): fields = { 'identity': fields.StringField(), - 'status': fields.StringField() + 'status': fields.StringField(), + 'force': fields.BooleanField(default=False) } diff --git a/senlin/tests/unit/api/openstack/v1/test_actions.py b/senlin/tests/unit/api/openstack/v1/test_actions.py index 22360a922..6893ec0b7 100644 --- a/senlin/tests/unit/api/openstack/v1/test_actions.py +++ b/senlin/tests/unit/api/openstack/v1/test_actions.py @@ -336,7 +336,41 @@ class ActionControllerTest(shared.ControllerTest, base.SenlinTestCase): 'ActionUpdateRequest', req, { 'identity': aid, + 'status': 'CANCELLED', + 'force': False + }) + mock_call.assert_called_once_with(req.context, 'action_update', obj) + + @mock.patch.object(util, 'parse_bool_param') + @mock.patch.object(util, 'parse_request') + @mock.patch.object(rpc_client.EngineClient, 'call') + def test_action_update_force_cancel(self, mock_call, mock_parse, + mock_parse_bool, mock_enforce): + self._mock_enforce_setup(mock_enforce, 'update', True) + aid = 'xxxx-yyyy-zzzz' + body = { + 'action': { 'status': 'CANCELLED' + } + } + params = {'force': 'True'} + req = self._patch( + '/actions/%(action_id)s' % {'action_id': aid}, + jsonutils.dumps(body), version='1.12', params=params) + obj = mock.Mock() + mock_parse.return_value = obj + mock_parse_bool.return_value = True + + self.assertRaises(exc.HTTPAccepted, + self.controller.update, req, + action_id=aid, body=body) + + mock_parse.assert_called_once_with( + 'ActionUpdateRequest', req, + { + 'identity': aid, + 'status': 'CANCELLED', + 'force': True }) mock_call.assert_called_once_with(req.context, 'action_update', obj) diff --git a/senlin/tests/unit/api/shared.py b/senlin/tests/unit/api/shared.py index 0b6a3035b..96b4ca5c9 100644 --- a/senlin/tests/unit/api/shared.py +++ b/senlin/tests/unit/api/shared.py @@ -85,10 +85,14 @@ class ControllerTest(object): return self._simple_request(path, params=params, method='DELETE') def _data_request(self, path, data, content_type='application/json', - method='POST', version=None): + method='POST', version=None, params=None): environ = self._environ(path) environ['REQUEST_METHOD'] = method + if params: + qs = "&".join(["=".join([k, str(params[k])]) for k in params]) + environ['QUERY_STRING'] = qs + req = wsgi.Request(environ) req.context = utils.dummy_context('api_test_user', self.project) self.context = req.context @@ -104,10 +108,10 @@ class ControllerTest(object): return self._data_request(path, data, content_type, method='PUT', version=version) - def _patch(self, path, data, content_type='application/json', + def _patch(self, path, data, params=None, content_type='application/json', version=None): return self._data_request(path, data, content_type, method='PATCH', - version=version) + version=version, params=params) def tearDown(self): # Common tearDown to assert that policy enforcement happens for all diff --git a/senlin/tests/unit/engine/actions/test_action_base.py b/senlin/tests/unit/engine/actions/test_action_base.py index f39f513e6..c25664d31 100755 --- a/senlin/tests/unit/engine/actions/test_action_base.py +++ b/senlin/tests/unit/engine/actions/test_action_base.py @@ -606,6 +606,55 @@ class ActionBaseTest(base.SenlinTestCase): action.set_status.assert_not_called() mock_signal.aseert_not_called() + @mock.patch.object(dobj.Dependency, 'get_depended') + def test_force_cancel(self, mock_dobj): + action = ab.Action(OBJID, 'OBJECT_ACTION', self.ctx, id=ACTION_ID) + action.load = mock.Mock() + action.set_status = mock.Mock() + mock_dobj.return_value = None + + action.status = action.RUNNING + action.force_cancel() + + action.load.assert_not_called() + action.set_status.assert_called_once_with( + action.RES_CANCEL, 'Action execution force cancelled') + + @mock.patch.object(dobj.Dependency, 'get_depended') + def test_force_cancel_children(self, mock_dobj): + action = ab.Action(OBJID, 'OBJECT_ACTION', self.ctx, id=ACTION_ID) + child_status_mock = mock.Mock() + children = [] + for child_id in CHILD_IDS: + child = ab.Action(OBJID, 'OBJECT_ACTION', self.ctx, id=child_id) + child.status = child.WAITING_LIFECYCLE_COMPLETION + child.set_status = child_status_mock + children.append(child) + mock_dobj.return_value = CHILD_IDS + action.set_status = mock.Mock() + action.load = mock.Mock() + action.load.side_effect = children + + action.status = action.RUNNING + action.force_cancel() + + mock_dobj.assert_called_once_with(action.context, action.id) + self.assertEqual(2, child_status_mock.call_count) + self.assertEqual(2, action.load.call_count) + + @mock.patch.object(dobj.Dependency, 'get_depended') + def test_force_cancel_immutable(self, mock_dobj): + action = ab.Action(OBJID, 'OBJECT_ACTION', self.ctx, id=ACTION_ID) + action.load = mock.Mock() + action.set_status = mock.Mock() + mock_dobj.return_value = None + + action.status = action.FAILED + self.assertRaises(exception.ActionImmutable, action.force_cancel) + + action.load.assert_not_called() + action.set_status.assert_not_called() + def test_execute_default(self): action = ab.Action.__new__(DummyAction, OBJID, 'BOOM', self.ctx) self.assertRaises(NotImplementedError, diff --git a/senlin/tests/unit/engine/service/test_actions.py b/senlin/tests/unit/engine/service/test_actions.py index c5b230941..addc42184 100644 --- a/senlin/tests/unit/engine/service/test_actions.py +++ b/senlin/tests/unit/engine/service/test_actions.py @@ -215,7 +215,7 @@ class ActionTest(base.SenlinTestCase): mock_load.return_value = x_obj req = orao.ActionUpdateRequest(identity='ACTION_ID', - status='CANCELLED') + status='CANCELLED', force=False) result = self.eng.action_update(self.ctx, req.obj_to_primitive()) self.assertIsNone(result)