openstack
/
senlin
Code Issues Proposed changes

Propagate node creation errors 

When a node fails to create due to an exception returned by
openstacksdk, the exception message is not available to the user after
the action is marked as failed.  The action status_reason is set to the
exception message inside engine/node.py do_create.  However it is
overwritten again in engine/actions/base.py ActionProc.

The fix is to return the error message out of engine/node.py do_create
and so that engine/actions/base.py ActionProc can set the status_reason
to the error message.

Change-Id: Id34d40be3752b3b4f7e12eb28b6fa88741b13d78
This commit is contained in:
Duc Truong 2018-09-07 18:22:47 +00:00
parent 9ac7037c2b
commit e1b8701f13
4 changed files with 24 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
senlin/engine/actions/node_action.py
View File

@ -67,7 +67,7 @@ class NodeAction(base.Action):
{'cluster_id': '', 'status': consts.NS_ERROR})
return self.RES_ERROR, result
res = self.entity.do_create(self.context)
res, reason = self.entity.do_create(self.context)
if cluster_id and self.cause == consts.CAUSE_RPC:
# Update cluster's desired_capacity and re-evaluate its status no
@ -79,7 +79,7 @@ class NodeAction(base.Action):
if res:
return self.RES_OK, 'Node created successfully.'
else:
return self.RES_ERROR, 'Node creation failed.'
return self.RES_ERROR, reason
@profiler.trace('NodeAction.do_delete', hide_args=False)
def do_delete(self):

8
senlin/engine/node.py
View File

@ -211,7 +211,9 @@ class Node(object):
def do_create(self, context):
if self.status != consts.NS_INIT:
LOG.error('Node is in status "%s"', self.status)
return False
self.set_status(context, consts.NS_ERROR,
'Node must be in INIT status')
return False, 'Node must be in INIT status'
self.set_status(context, consts.NS_CREATING, 'Creation in progress')
try:
@ -220,11 +222,11 @@ class Node(object):
physical_id = ex.resource_id
self.set_status(context, consts.NS_ERROR, six.text_type(ex),
physical_id=physical_id)
return False
return False, str(ex)
self.set_status(context, consts.NS_ACTIVE, 'Creation succeeded',
physical_id=physical_id)
return True
return True, None
def do_delete(self, context):
self.set_status(context, consts.NS_DELETING, 'Deletion in progress')

16
senlin/tests/unit/engine/actions/test_node_action.py
View File

@ -37,7 +37,7 @@ class NodeActionTest(base.SenlinTestCase):
def test_do_create_okay(self, mock_load):
node = mock.Mock(id='NID')
node.do_create = mock.Mock(return_value=True)
node.do_create = mock.Mock(return_value=[True, ''])
mock_load.return_value = node
action = node_action.NodeAction(node.id, 'ACTION', self.ctx)
@ -49,7 +49,8 @@ class NodeActionTest(base.SenlinTestCase):
def test_do_create_failed(self, mock_load):
node = mock.Mock(id='NID')
node.do_create = mock.Mock(return_value=False)
node.do_create = mock.Mock(return_value=[False,
'custom error message'])
mock_load.return_value = node
action = node_action.NodeAction(node.id, 'ACTION', self.ctx)
@ -57,7 +58,7 @@ class NodeActionTest(base.SenlinTestCase):
res_code, res_msg = action.do_create()
self.assertEqual(action.RES_ERROR, res_code)
self.assertEqual('Node creation failed.', res_msg)
self.assertEqual('custom error message', res_msg)
node.do_create.assert_called_once_with(action.context)
@mock.patch.object(scaleutils, 'check_size_params')
@ -68,7 +69,7 @@ class NodeActionTest(base.SenlinTestCase):
cluster = mock.Mock(id='CID')
mock_c_load.return_value = cluster
node = mock.Mock(id='NID', cluster_id='CID')
node.do_create = mock.Mock(return_value=True)
node.do_create = mock.Mock(return_value=[True, ''])
mock_load.return_value = node
mock_count.return_value = 11
mock_check.return_value = None
@ -97,7 +98,7 @@ class NodeActionTest(base.SenlinTestCase):
cluster = mock.Mock(id='CID')
mock_c_load.return_value = cluster
node = mock.Mock(id='NID', cluster_id='CID')
node.do_create = mock.Mock(return_value=True)
node.do_create = mock.Mock(return_value=[True, ''])
mock_load.return_value = node
mock_count.return_value = 11
mock_check.return_value = 'overflow'
@ -128,7 +129,8 @@ class NodeActionTest(base.SenlinTestCase):
cluster = mock.Mock(id='CID')
mock_c_load.return_value = cluster
node = mock.Mock(id='NID', cluster_id='CID')
node.do_create = mock.Mock(return_value=False)
node.do_create = mock.Mock(return_value=[False,
'custom error message'])
mock_load.return_value = node
mock_count.return_value = 11
mock_check.return_value = ''
@ -140,7 +142,7 @@ class NodeActionTest(base.SenlinTestCase):
# assertions
self.assertEqual(action.RES_ERROR, res_code)
self.assertEqual('Node creation failed.', res_msg)
self.assertEqual('custom error message', res_msg)
mock_c_load.assert_called_once_with(action.context, 'CID')
mock_count.assert_called_once_with(action.context, 'CID')
mock_check.assert_called_once_with(cluster, 11, None, None, True)

11
senlin/tests/unit/engine/test_node.py
View File

@ -251,11 +251,15 @@ class TestNode(base.SenlinTestCase):
'Creation succeeded',
physical_id=physical_id)
def test_node_create_not_init(self):
@mock.patch.object(nodem.Node, 'set_status')
def test_node_create_not_init(self, mock_status):
node = nodem.Node('node1', PROFILE_ID, CLUSTER_ID, self.context)
node.status = 'NOT_INIT'
res = node.do_create(self.context)
res, reason = node.do_create(self.context)
self.assertFalse(res)
self.assertEqual('Node must be in INIT status', reason)
mock_status.assert_any_call(self.context, consts.NS_ERROR,
'Node must be in INIT status')
@mock.patch.object(nodem.Node, 'set_status')
@mock.patch.object(pb.Profile, 'create_object')
@ -265,9 +269,10 @@ class TestNode(base.SenlinTestCase):
mock_create.side_effect = exception.EResourceCreation(
type='PROFILE', message='Boom', resource_id='test_id')
res = node.do_create(self.context)
res, reason = node.do_create(self.context)
self.assertFalse(res)
self.assertEqual(str(reason), 'Failed in creating PROFILE: Boom.')
mock_status.assert_any_call(self.context, consts.NS_CREATING,
'Creation in progress')
mock_status.assert_any_call(self.context, consts.NS_ERROR,