openstack
/
senlin
Code Issues Proposed changes

Do not set parent action status if stop node failed 

- Add new action input key 'update_parent_status' to skip setting the
  parent action status if an action fails.
- When stopping a node before delete, 'update_parent_status' is set to
 False to avoid setting the parent action status if the stop action
fails.

Change-Id: I5f06b4fba7b8f6efc4c36f3397d94e3ccc1abc55
Closes-Bug: #1812112
This commit is contained in:
Duc Truong 2019-01-17 00:03:22 +00:00
parent 0700a8bf29
commit 0367baa145
4 changed files with 82 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
senlin/db/sqlalchemy/api.py
View File

@ -1250,14 +1250,16 @@ def _mark_failed(action_id, timestamp, reason=None):
'end_time': timestamp,
}
query.update(values, synchronize_session=False)
action = query.all()
query = session.query(models.ActionDependency)
query = query.filter_by(depended=action_id)
dependents = [d.dependent for d in query.all()]
query.delete(synchronize_session=False)
for d in dependents:
_mark_failed(d, timestamp)
if parent_status_update_needed(action):
for d in dependents:
_mark_failed(d, timestamp)
@retry_on_deadlock
@ -1276,14 +1278,16 @@ def _mark_cancelled(session, action_id, timestamp, reason=None):
'end_time': timestamp,
}
query.update(values, synchronize_session=False)
action = query.all()
query = session.query(models.ActionDependency)
query = query.filter_by(depended=action_id)
dependents = [d.dependent for d in query.all()]
query.delete(synchronize_session=False)
for d in dependents:
_mark_cancelled(session, d, timestamp)
if parent_status_update_needed(action):
for d in dependents:
_mark_cancelled(session, d, timestamp)
@retry_on_deadlock
@ -1686,3 +1690,12 @@ def db_sync(engine, version=None):
def db_version(engine):
"""Display the current database version."""
return migration.db_version(engine)
def parent_status_update_needed(action):
"""Return if the status of the parent action needs to be updated
Return value for update_parent_status key in action inputs
"""
return (len(action) > 0 and hasattr(action[0], 'inputs') and
action[0].inputs.get('update_parent_status', True))

9
senlin/engine/actions/cluster_action.py
View File

@ -455,9 +455,11 @@ class ClusterAction(base.Action):
lifecycle_hook = self.data.get('hooks')
if lifecycle_hook:
if stop_node_before_delete:
# set update_parent_status to False so that a failure in stop
# operation is ignored and the parent status is not changed
res, reason = self._remove_nodes_with_hook(
consts.NODE_OPERATION, node_ids, lifecycle_hook,
{'operation': 'stop'})
{'operation': 'stop', 'update_parent_status': False})
if res != self.RES_OK:
LOG.warning('Failure while stopping nodes. '
'Proceed to delete nodes.')
@ -468,8 +470,11 @@ class ClusterAction(base.Action):
action_name, node_ids, lifecycle_hook)
else:
if stop_node_before_delete:
# set update_parent_status to False so that a failure in stop
# operation is ignored and the parent status is not changed
res, reason = self._remove_nodes_normally(
consts.NODE_OPERATION, node_ids, {'operation': 'stop'})
consts.NODE_OPERATION, node_ids,
{'operation': 'stop', 'update_parent_status': False})
if res != self.RES_OK:
LOG.warning('Failure while stopping nodes. '
'Proceed to delete nodes.')

55
senlin/tests/unit/db/test_action_api.py
View File

@ -412,7 +412,8 @@ class DBAPIActionTest(base.SenlinTestCase):
specs = [
{'name': 'A01', 'status': 'INIT', 'target': 'cluster_001'},
{'name': 'A02', 'status': 'INIT', 'target': 'node_001'},
{'name': 'A03', 'status': 'INIT', 'target': 'node_002'},
{'name': 'A03', 'status': 'INIT', 'target': 'node_002',
'inputs': {'update_parent_status': False}},
{'name': 'A04', 'status': 'INIT', 'target': 'node_003'},
{'name': 'A05', 'status': 'INIT', 'target': 'cluster_002'},
{'name': 'A06', 'status': 'INIT', 'target': 'cluster_003'},
@ -424,10 +425,12 @@ class DBAPIActionTest(base.SenlinTestCase):
action = _create_action(self.ctx, **spec)
id_of[spec['name']] = action.id
# A01 has dependents A02, A03, A04
db_api.dependency_add(self.ctx,
[id_of['A02'], id_of['A03'], id_of['A04']],
id_of['A01'])
# A05, A06, A07 each has dependent A01
db_api.dependency_add(self.ctx,
id_of['A01'],
[id_of['A05'], id_of['A06'], id_of['A07']])
@ -517,7 +520,7 @@ class DBAPIActionTest(base.SenlinTestCase):
result = db_api.dependency_get_dependents(self.ctx, id_of['A02'])
self.assertEqual(0, len(result))
def test_action_mark_failed(self):
def test_action(self):
timestamp = time.time()
id_of = self._prepare_action_mark_failed_cancel()
db_api.action_mark_failed(self.ctx, id_of['A01'], timestamp)
@ -530,6 +533,30 @@ class DBAPIActionTest(base.SenlinTestCase):
result = db_api.dependency_get_dependents(self.ctx, id_of['A01'])
self.assertEqual(0, len(result))
def test_action_mark_failed_parent_status_update_needed(self):
timestamp = time.time()
id_of = self._prepare_action_mark_failed_cancel()
db_api.action_mark_failed(self.ctx, id_of['A04'], timestamp)
action = db_api.action_get(self.ctx, id_of['A01'])
self.assertEqual(consts.ACTION_FAILED, action.status)
self.assertEqual(round(timestamp, 6), float(action.end_time))
result = db_api.dependency_get_dependents(self.ctx, id_of['A01'])
self.assertEqual(0, len(result))
def test_action_mark_failed_parent_status_update_not_needed(self):
timestamp = time.time()
id_of = self._prepare_action_mark_failed_cancel()
db_api.action_mark_failed(self.ctx, id_of['A03'], timestamp)
action = db_api.action_get(self.ctx, id_of['A01'])
self.assertEqual(consts.ACTION_WAITING, action.status)
self.assertIsNone(action.end_time)
result = db_api.dependency_get_dependents(self.ctx, id_of['A01'])
self.assertEqual(3, len(result))
def test_action_mark_cancelled(self):
timestamp = time.time()
id_of = self._prepare_action_mark_failed_cancel()
@ -543,6 +570,30 @@ class DBAPIActionTest(base.SenlinTestCase):
result = db_api.dependency_get_dependents(self.ctx, id_of['A01'])
self.assertEqual(0, len(result))
def test_action_mark_cancelled_parent_status_update_needed(self):
timestamp = time.time()
id_of = self._prepare_action_mark_failed_cancel()
db_api.action_mark_cancelled(self.ctx, id_of['A04'], timestamp)
action = db_api.action_get(self.ctx, id_of['A01'])
self.assertEqual(consts.ACTION_CANCELLED, action.status)
self.assertEqual(round(timestamp, 6), float(action.end_time))
result = db_api.dependency_get_dependents(self.ctx, id_of['A01'])
self.assertEqual(0, len(result))
def test_action_mark_cancelled_parent_status_update_not_needed(self):
timestamp = time.time()
id_of = self._prepare_action_mark_failed_cancel()
db_api.action_mark_cancelled(self.ctx, id_of['A03'], timestamp)
action = db_api.action_get(self.ctx, id_of['A01'])
self.assertEqual(consts.ACTION_WAITING, action.status)
self.assertIsNone(action.end_time)
result = db_api.dependency_get_dependents(self.ctx, id_of['A01'])
self.assertEqual(3, len(result))
def test_action_mark_ready(self):
timestamp = time.time()

8
senlin/tests/unit/engine/actions/test_delete.py
View File

@ -98,7 +98,8 @@ class ClusterDeleteTest(base.SenlinTestCase):
mock.call(action.context, 'NODE_ID', 'NODE_OPERATION',
name='node_delete_NODE_ID',
cause='Derived Action',
inputs={'operation': 'stop'}),
inputs={'operation': 'stop',
'update_parent_status': False}),
mock.call(action.context, 'NODE_ID', 'NODE_DELETE',
name='node_delete_NODE_ID',
cause='Derived Action', inputs={})
@ -481,7 +482,8 @@ class ClusterDeleteTest(base.SenlinTestCase):
self.assertEqual(action.RES_OK, res_code)
self.assertEqual({}, action.data)
remove_calls = [
mock.call('NODE_OPERATION', ['NODE_ID'], {'operation': 'stop'}),
mock.call('NODE_OPERATION', ['NODE_ID'],
{'operation': 'stop', 'update_parent_status': False}),
mock.call('NODE_DELETE', ['NODE_ID']),
]
mock_remove.assert_has_calls(remove_calls)
@ -518,7 +520,7 @@ class ClusterDeleteTest(base.SenlinTestCase):
self.assertEqual(action.RES_OK, res_code)
mock_remove_hook.assert_called_once_with(
'NODE_OPERATION', ['NODE_ID'], lifecycle_hook,
{'operation': 'stop'})
{'operation': 'stop', 'update_parent_status': False})
mock_remove_normally.assert_called_once_with('NODE_DELETE',
['NODE_ID'])