From 8fc8ee654961f2dfb11883d8f7fdc9775560e9ed Mon Sep 17 00:00:00 2001 From: Duc Truong Date: Thu, 5 Nov 2020 00:44:38 +0000 Subject: [PATCH] Release locks when action is cancelled When an action is cancelled, it should release any associated cluster or node locks so that new operations can be performed on the cluster or node. Remove action status check for action force operation. Change-Id: I6a520a90feed7ab1e9c99a595863ca3ff90aef40 --- senlin/engine/actions/base.py | 12 ++++++----- senlin/engine/actions/cluster_action.py | 6 ++++++ senlin/engine/actions/node_action.py | 12 +++++++++++ .../unit/engine/actions/test_action_base.py | 20 +++++++------------ 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/senlin/engine/actions/base.py b/senlin/engine/actions/base.py index 82b41f024..b5f296534 100755 --- a/senlin/engine/actions/base.py +++ b/senlin/engine/actions/base.py @@ -379,14 +379,11 @@ class Action(object): :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') + self.release_lock() + depended = dobj.Dependency.get_depended(self.context, self.id) if not depended: return @@ -400,6 +397,7 @@ class Action(object): LOG.debug('Forcing action %s to cancel.', action.id) action.set_status(action.RES_CANCEL, 'Action execution force cancelled') + action.release_lock() def execute(self, **kwargs): """Execute the action. @@ -411,6 +409,10 @@ class Action(object): """ raise NotImplementedError + def release_lock(self): + """Release the lock associated with the action.""" + raise NotImplementedError + def set_status(self, result, reason=None): """Set action status based on return value from execute.""" diff --git a/senlin/engine/actions/cluster_action.py b/senlin/engine/actions/cluster_action.py index 5ab94857c..ada70d3b1 100755 --- a/senlin/engine/actions/cluster_action.py +++ b/senlin/engine/actions/cluster_action.py @@ -1215,6 +1215,12 @@ class ClusterAction(base.Action): """Handler to cancel the execution of action.""" return self.RES_OK + def release_lock(self): + """Handler to release the lock.""" + senlin_lock.cluster_lock_release(self.target, self.id, + senlin_lock.CLUSTER_SCOPE) + return self.RES_OK + def CompleteLifecycleProc(context, action_id): """Complete lifecycle process.""" diff --git a/senlin/engine/actions/node_action.py b/senlin/engine/actions/node_action.py index 645b86ab1..f4463f1b9 100755 --- a/senlin/engine/actions/node_action.py +++ b/senlin/engine/actions/node_action.py @@ -283,3 +283,15 @@ class NodeAction(base.Action): def cancel(self): """Handler for cancelling the action.""" return self.RES_OK + + def release_lock(self): + """Handler to release the lock.""" + senlin_lock.node_lock_release(self.entity.id, self.id) + + # only release cluster lock if it was locked as part of this + # action (i.e. it's a user intiated action aka CAUSE_RPC from + # senlin API and a not a CAUSED_DERIVED) + if self.cause == consts.CAUSE_RPC: + senlin_lock.cluster_lock_release(self.entity.cluster_id, self.id, + senlin_lock.NODE_SCOPE) + return self.RES_OK diff --git a/senlin/tests/unit/engine/actions/test_action_base.py b/senlin/tests/unit/engine/actions/test_action_base.py index 9d6df4f1c..f07dcce9c 100755 --- a/senlin/tests/unit/engine/actions/test_action_base.py +++ b/senlin/tests/unit/engine/actions/test_action_base.py @@ -635,6 +635,7 @@ class ActionBaseTest(base.SenlinTestCase): action = ab.Action(OBJID, 'OBJECT_ACTION', self.ctx, id=ACTION_ID) action.load = mock.Mock() action.set_status = mock.Mock() + action.release_lock = mock.Mock() mock_dobj.return_value = None action.status = action.RUNNING @@ -643,19 +644,23 @@ class ActionBaseTest(base.SenlinTestCase): action.load.assert_not_called() action.set_status.assert_called_once_with( action.RES_CANCEL, 'Action execution force cancelled') + self.assertEqual(1, action.release_lock.call_count) @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() + child_release_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 + child.release_lock = child_release_mock children.append(child) mock_dobj.return_value = CHILD_IDS action.set_status = mock.Mock() + action.release_lock = mock.Mock() action.load = mock.Mock() action.load.side_effect = children @@ -664,20 +669,9 @@ class ActionBaseTest(base.SenlinTestCase): mock_dobj.assert_called_once_with(action.context, action.id) self.assertEqual(2, child_status_mock.call_count) + self.assertEqual(2, child_release_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() + self.assertEqual(1, action.release_lock.call_count) def test_execute_default(self): action = ab.Action.__new__(DummyAction, OBJID, 'BOOM', self.ctx)