openstack
/
senlin
Code Issues Proposed changes

Update node recover operation 

- Change recover operation return value
- Change reboot type parameter
- Add handle reboot exception capture.

Change-Id: Ie008293fc5a7df8f8dd06c5e4a2d39d034199c5d
Signed-off-by: Yuanbin.Chen <cybing4@gmail.com>
This commit is contained in:
Yuanbin.Chen 2018-04-04 15:58:06 +08:00
parent 98d8cf30a3
commit 5a92b60339
6 changed files with 81 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
senlin/engine/node.py
View File

@ -373,17 +373,18 @@ class Node(object):
reason='Recovery in progress')
try:
physical_id = pb.Profile.recover_object(context, self, **options)
physical_id, status = pb.Profile.recover_object(context,
self, **options)
except exc.EResourceOperation as ex:
self.set_status(context, consts.NS_ERROR, reason=six.text_type(ex))
return False
if not physical_id:
if not status:
self.set_status(context, consts.NS_ERROR, reason='Recovery failed')
return False
params = {}
if self.physical_id != physical_id:
if physical_id and self.physical_id != physical_id:
self.data['recovery'] = consts.RECOVER_RECREATE
params['data'] = self.data
params['physical_id'] = physical_id

9
senlin/profiles/base.py
View File

@ -487,6 +487,10 @@ class Profile(object):
:param obj: The node object to operate on.
:param options: Keyword arguments for the recover operation.
:return id: New id of the recovered resource or None if recovery
failed.
:return status: True indicates successful recovery, False indicates
failure.
"""
operation = options.pop('operation', None)
@ -496,7 +500,7 @@ class Profile(object):
if operation and operation['name'] != consts.RECOVER_RECREATE:
LOG.error("Recover operation not supported: %s", operation)
return False
return None, False
extra_params = options.get('params', {})
fence_compute = extra_params.get('fence_compute', False)
@ -505,13 +509,12 @@ class Profile(object):
except exc.EResourceDeletion as ex:
raise exc.EResourceOperation(op='recovering', type='node',
id=obj.id, message=six.text_type(ex))
res = None
try:
res = self.do_create(obj)
except exc.EResourceCreation as ex:
raise exc.EResourceOperation(op='recovering', type='node',
id=obj.id, message=six.text_type(ex))
return res
return res, True
def do_validate(self, obj):
"""For subclass to override."""

44
senlin/profiles/os/nova/server.py
View File

@ -1516,6 +1516,10 @@ class ServerProfile(base.Profile):
:param obj: The node object.
:param dict options: A list for operations each of which has a name
and optionally a map from parameter to values.
:return id: New id of the recovered resource or None if recovery
failed.
:return status: True indicates successful recovery, False indicates
failure.
"""
operation = options.get('operation', None)
@ -1526,10 +1530,15 @@ class ServerProfile(base.Profile):
op_name = operation['name']
if op_name.upper() != consts.RECOVER_RECREATE:
op_params = operation.get('params', {})
# nova recover operation always use hard reboot
# vm in error or stop status soft reboot can't succeed
if op_name.upper() == consts.RECOVER_REBOOT:
if self.REBOOT_TYPE not in op_params:
op_params[self.REBOOT_TYPE] = self.REBOOT_HARD
if op_name.lower() not in self.OP_NAMES:
LOG.error("The operation '%s' is not supported",
op_name)
return False
return obj.physical_id, False
method = getattr(self, "handle_" + op_name.lower())
return method(obj, **op_params)
@ -1539,17 +1548,27 @@ class ServerProfile(base.Profile):
def handle_reboot(self, obj, **options):
"""Handler for the reboot operation."""
if not obj.physical_id:
return False
return None, False
server_id = obj.physical_id
reboot_type = options.get(self.REBOOT_TYPE, self.REBOOT_SOFT)
if (not isinstance(reboot_type, six.string_types) or
reboot_type not in self.REBOOT_TYPES):
return False
return server_id, False
self.compute(obj).server_reboot(obj.physical_id, reboot_type)
self.compute(obj).wait_for_server(obj.physical_id,
consts.VS_ACTIVE)
return True
nova_driver = self.compute(obj)
try:
server = nova_driver.server_get(server_id)
if server is None:
return None, False
nova_driver.server_reboot(server_id, reboot_type)
nova_driver.wait_for_server(obj.physical_id,
consts.VS_ACTIVE)
return server_id, True
except exc.InternalError as ex:
raise exc.EResourceOperation(op='rebooting', type='server',
id=server_id,
message=six.text_type(ex))
def handle_rebuild(self, obj, **options):
"""Handler for the rebuild operation.
@ -1557,10 +1576,13 @@ class ServerProfile(base.Profile):
:param obj: The node object.
:param dict options: A list for operations each of which has a name
and optionally a map from parameter to values.
:returns: The server ID if successful or None if failed.
:return id: New id of the recovered resource or None if recovery
failed.
:return status: True indicates successful recovery, False indicates
failure.
"""
if not obj.physical_id:
return None
return None, False
server_id = obj.physical_id
nova_driver = self.compute(obj)
@ -1572,7 +1594,7 @@ class ServerProfile(base.Profile):
message=six.text_type(ex))
if server is None:
return None
return None, False
image_id = self._get_image_id(obj, server, 'rebuilding')
admin_pass = self.properties.get(self.ADMIN_PASS)
@ -1581,7 +1603,7 @@ class ServerProfile(base.Profile):
nova_driver.server_rebuild(server_id, image_id,
name, admin_pass)
nova_driver.wait_for_server(server_id, consts.VS_ACTIVE)
return server_id
return server_id, True
except exc.InternalError as ex:
raise exc.EResourceOperation(op='rebuilding', type='server',
id=server_id,

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

@ -603,7 +603,7 @@ class TestNode(base.SenlinTestCase):
# action = node_action.NodeAction(node.id, 'ACTION', self.ctx)
mock_recover.return_value = new_id
mock_recover.return_value = new_id, True
mock_status.side_effect = set_status
action = mock.Mock()
action.inputs = {'operation': ['SWIM', 'DANCE']}
@ -630,7 +630,7 @@ class TestNode(base.SenlinTestCase):
def test_node_recover_in_place(self, mock_recover, mock_status):
node = nodem.Node('node1', PROFILE_ID, None)
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
mock_recover.return_value = node.physical_id
mock_recover.return_value = node.physical_id, True
action = mock.Mock(inputs={})
res = node.do_recover(self.context, action)
@ -672,7 +672,7 @@ class TestNode(base.SenlinTestCase):
node = nodem.Node('node1', PROFILE_ID, '')
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
new_id = '166db83b-b4a4-49ef-96a8-6c0fdd882d1a'
mock_recover.return_value = new_id
mock_recover.return_value = new_id, True
mock_status.side_effect = set_status
mock_check = self.patchobject(pb.Profile, 'check_object')
mock_check.return_value = False
@ -706,7 +706,7 @@ class TestNode(base.SenlinTestCase):
node = nodem.Node('node1', PROFILE_ID, '', id='fake')
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
new_id = '166db83b-b4a4-49ef-96a8-6c0fdd882d1a'
mock_recover.return_value = new_id
mock_recover.return_value = new_id, True
mock_status.side_effect = set_status
mock_check = self.patchobject(pb.Profile, 'check_object')
mock_check.return_value = False
@ -742,7 +742,7 @@ class TestNode(base.SenlinTestCase):
node = nodem.Node('node1', PROFILE_ID, '', id='fake')
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
new_id = '166db83b-b4a4-49ef-96a8-6c0fdd882d1a'
mock_recover.return_value = new_id
mock_recover.return_value = new_id, True
mock_status.side_effect = set_status
mock_check = self.patchobject(pb.Profile, 'check_object')
mock_check.return_value = False
@ -779,7 +779,7 @@ class TestNode(base.SenlinTestCase):
node = nodem.Node('node1', PROFILE_ID, '')
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
new_id = '166db83b-b4a4-49ef-96a8-6c0fdd882d1a'
mock_recover.return_value = new_id
mock_recover.return_value = new_id, True
mock_status.side_effect = set_status
mock_check = self.patchobject(pb.Profile, 'check_object')
mock_check.side_effect = exception.EResourceOperation(
@ -813,7 +813,7 @@ class TestNode(base.SenlinTestCase):
def test_node_recover_failed_recover(self, mock_recover, mock_status):
node = nodem.Node('node1', PROFILE_ID, None)
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
mock_recover.return_value = None
mock_recover.return_value = node.physical_id, None
action = mock.Mock(inputs={'operation': [{'name': 'RECREATE'}]})
res = node.do_recover(self.context, action)

40
senlin/tests/unit/profiles/test_nova_server.py
View File

@ -1327,14 +1327,26 @@ class TestNovaServerBasic(base.SenlinTestCase):
self.assertEqual(mock_rebuild.return_value, res)
mock_rebuild.assert_called_once_with(node_obj)
@mock.patch.object(server.ServerProfile, 'handle_reboot')
def test_do_recover_reboot(self, mock_reboot):
profile = server.ServerProfile('t', self.spec)
node_obj = mock.Mock(physical_id='FAKE_ID')
res = profile.do_recover(node_obj, operation=[{'name': 'REBOOT'}])
self.assertTrue(res)
self.assertEqual(mock_reboot.return_value, res)
mock_reboot.assert_called_once_with(node_obj, type='HARD')
@mock.patch.object(profiles_base.Profile, 'do_recover')
def test_do_recover_bad_operation(self, mock_base_recover):
profile = server.ServerProfile('t', self.spec)
node_obj = mock.Mock(physical_id='FAKE_ID')
res = profile.do_recover(node_obj, operation=[{'name': 'BLAHBLAH'}])
res, status = profile.do_recover(node_obj,
operation=[{'name': 'BLAHBLAH'}])
self.assertFalse(res)
self.assertFalse(status)
@mock.patch.object(profiles_base.Profile, 'do_recover')
def test_do_recover_fallback(self, mock_base_recover):
@ -1367,9 +1379,9 @@ class TestNovaServerBasic(base.SenlinTestCase):
profile = server.ServerProfile('t', self.spec)
# do it
res = profile.handle_reboot(obj, type='SOFT')
res, status = profile.handle_reboot(obj, type='SOFT')
self.assertFalse(res)
self.assertFalse(status)
def test_handle_reboot_default_type(self):
obj = mock.Mock(physical_id='FAKE_ID')
@ -1392,11 +1404,11 @@ class TestNovaServerBasic(base.SenlinTestCase):
profile._computeclient = mock.Mock()
# do it
res = profile.handle_reboot(obj, type=['foo'])
self.assertFalse(res)
res, status = profile.handle_reboot(obj, type=['foo'])
self.assertFalse(status)
res = profile.handle_reboot(obj, type='foo')
self.assertFalse(res)
res, status = profile.handle_reboot(obj, type='foo')
self.assertFalse(status)
def test_handle_rebuild_with_image(self):
profile = server.ServerProfile('t', self.spec)
@ -1410,7 +1422,7 @@ class TestNovaServerBasic(base.SenlinTestCase):
res = profile.handle_rebuild(node_obj)
self.assertEqual('FAKE_ID', res)
self.assertTrue(res)
cc.server_get.assert_called_with('FAKE_ID')
cc.server_rebuild.assert_called_once_with('FAKE_ID', '123',
'FAKE_SERVER_NAME',
@ -1449,7 +1461,7 @@ class TestNovaServerBasic(base.SenlinTestCase):
res = profile.handle_rebuild(node_obj)
self.assertEqual('FAKE_ID', res)
self.assertTrue(res)
cc.server_get.assert_called_with('FAKE_ID')
cc.server_rebuild.assert_called_once_with('FAKE_ID', '123',
'FAKE_SERVER_NAME',
@ -1527,9 +1539,9 @@ class TestNovaServerBasic(base.SenlinTestCase):
profile._computeclient = cc
node_obj = mock.Mock(physical_id='FAKE_ID')
res = profile.handle_rebuild(node_obj)
res, status = profile.handle_rebuild(node_obj)
self.assertFalse(res)
self.assertFalse(status)
cc.server_get.assert_called_once_with('FAKE_ID')
self.assertEqual(0, cc.server_rebuild.call_count)
self.assertEqual(0, cc.wait_for_server.call_count)
@ -1541,9 +1553,9 @@ class TestNovaServerBasic(base.SenlinTestCase):
test_server = mock.Mock()
test_server.physical_id = None
res = profile.handle_rebuild(test_server)
res, status = profile.handle_rebuild(test_server)
self.assertFalse(res)
self.assertFalse(status)
def test_handle_rebuild_failed_with_name(self):
self.spec['properties']['name'] = None

9
senlin/tests/unit/profiles/test_profile_base.py
View File

@ -751,11 +751,12 @@ class TestProfileBase(base.SenlinTestCase):
self.patchobject(profile, 'do_create', return_value=True)
self.patchobject(profile, 'do_delete', return_value=True)
res = profile.do_recover(mock.Mock())
self.assertTrue(res)
res, status = profile.do_recover(mock.Mock())
self.assertTrue(status)
res = profile.do_recover(mock.Mock(), operation=[{'name': 'bar'}])
self.assertFalse(res)
res, status = profile.do_recover(
mock.Mock(), operation=[{'name': 'bar'}])
self.assertFalse(status)
def test_do_recover_with_fencing(self):
profile = self._create_profile('test-profile')