Fix exception handling in server check
This patch fixes the logic in server health check. Change-Id: I01b35debfce9844eda436d3d0aa603869d8c48ab
This commit is contained in:
parent
c10ae69ffb
commit
3ba7dc4ce7
|
@ -28,10 +28,6 @@ import os
|
|||
import subprocess
|
||||
import sys
|
||||
|
||||
# TODO(Graham Hayes): Remove the following block of code when os-api-ref is
|
||||
# using openstackdocstheme
|
||||
|
||||
import os_api_ref
|
||||
import openstackdocstheme # noqa
|
||||
|
||||
extensions = [
|
||||
|
|
|
@ -77,17 +77,6 @@ class NodeAction(base.Action):
|
|||
else:
|
||||
return self.RES_ERROR, _('Node creation failed.')
|
||||
|
||||
def do_check(self):
|
||||
"""Handler for the NODE_check action.
|
||||
|
||||
:returns: A tuple containing the result and the corresponding reason.
|
||||
"""
|
||||
res = self.node.do_check(self.context)
|
||||
if res:
|
||||
return self.RES_OK, _('Node status is ACTIVE.')
|
||||
else:
|
||||
return self.RES_ERROR, _('Node status is not ACTIVE.')
|
||||
|
||||
def do_delete(self):
|
||||
"""Handler for the NODE_DELETE action.
|
||||
|
||||
|
@ -178,6 +167,17 @@ class NodeAction(base.Action):
|
|||
else:
|
||||
return self.RES_ERROR, _('Node failed in leaving cluster.')
|
||||
|
||||
def do_check(self):
|
||||
"""Handler for the NODE_check action.
|
||||
|
||||
:returns: A tuple containing the result and the corresponding reason.
|
||||
"""
|
||||
res = self.node.do_check(self.context)
|
||||
if res:
|
||||
return self.RES_OK, _('Node status is ACTIVE.')
|
||||
else:
|
||||
return self.RES_ERROR, _('Node status is not ACTIVE.')
|
||||
|
||||
def do_recover(self):
|
||||
"""Handler for the NODE_RECOVER action.
|
||||
|
||||
|
|
|
@ -207,8 +207,14 @@ class Node(object):
|
|||
return node_dict
|
||||
|
||||
def set_status(self, context, status, reason=None, **params):
|
||||
'''Set status of the node.'''
|
||||
"""Set status of the node.
|
||||
|
||||
:param context: The request context.
|
||||
:param status: New status for the node.
|
||||
:param reason: The reason that leads the node to its current status.
|
||||
:param kwargs params: Other properties that need an update.
|
||||
:returns: ``None``.
|
||||
"""
|
||||
values = {}
|
||||
now = timeutils.utcnow(True)
|
||||
if status == self.ACTIVE and self.status == self.CREATING:
|
||||
|
@ -343,12 +349,17 @@ class Node(object):
|
|||
if not self.physical_id:
|
||||
return False
|
||||
|
||||
res = pb.Profile.check_object(context, self)
|
||||
res = True
|
||||
try:
|
||||
res = pb.Profile.check_object(context, self)
|
||||
except exc.EResourceOperation as ex:
|
||||
self.set_status(self.ERROR, six.text_type(ex))
|
||||
return False
|
||||
|
||||
if not res:
|
||||
self.status = 'ERROR'
|
||||
self.status_reason = _('Physical node is not ACTIVE!')
|
||||
self.store(context)
|
||||
if res:
|
||||
self.set_status(self.ACTIVE, _("Check: Node is ACTIVE."))
|
||||
else:
|
||||
self.set_status(self.ERROR, _("Check: Node is not ACTIVE."))
|
||||
|
||||
return res
|
||||
|
||||
|
|
|
@ -668,13 +668,12 @@ class ServerProfile(base.Profile):
|
|||
if not obj.physical_id:
|
||||
return False
|
||||
|
||||
self.server_id = obj.physical_id
|
||||
|
||||
try:
|
||||
server = self.nova(obj).server_get(self.server_id)
|
||||
except Exception as ex:
|
||||
LOG.error('Error: %s' % six.text_type(ex))
|
||||
return False
|
||||
server = self.nova(obj).server_get(obj.physical_id)
|
||||
except exc.InternalError as ex:
|
||||
raise exc.EResourceOperation(op='checking', type='server',
|
||||
id=obj.physical_id,
|
||||
message=six.text_type(ex))
|
||||
|
||||
if (server is None or server.status != 'ACTIVE'):
|
||||
return False
|
||||
|
|
|
@ -544,8 +544,9 @@ class TestNode(base.SenlinTestCase):
|
|||
self.assertIsNone(node.updated_at)
|
||||
self.assertEqual(1, node.index)
|
||||
|
||||
@mock.patch.object(nodem.Node, 'set_status')
|
||||
@mock.patch.object(pb.Profile, 'check_object')
|
||||
def test_node_check(self, mock_check):
|
||||
def test_node_check(self, mock_check, mock_status):
|
||||
node = nodem.Node('node1', PROFILE_ID, '')
|
||||
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
|
||||
mock_check.return_value = True
|
||||
|
@ -554,10 +555,12 @@ class TestNode(base.SenlinTestCase):
|
|||
|
||||
self.assertTrue(res)
|
||||
mock_check.assert_called_once_with(self.context, node)
|
||||
mock_status.assert_called_once_with(node.ACTIVE,
|
||||
'Check: Node is ACTIVE.')
|
||||
|
||||
@mock.patch.object(nodem.Node, 'store')
|
||||
@mock.patch.object(nodem.Node, 'set_status')
|
||||
@mock.patch.object(pb.Profile, 'check_object')
|
||||
def test_node_check_failed_check(self, mock_check, mock_store):
|
||||
def test_node_check_check_return_false(self, mock_check, mock_status):
|
||||
node = nodem.Node('node1', PROFILE_ID, '')
|
||||
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
|
||||
mock_check.return_value = False
|
||||
|
@ -565,7 +568,25 @@ class TestNode(base.SenlinTestCase):
|
|||
res = node.do_check(self.context)
|
||||
|
||||
self.assertFalse(res)
|
||||
self.assertEqual('ERROR', node.status)
|
||||
mock_status.assert_called_once_with(node.ERROR,
|
||||
'Check: Node is not ACTIVE.')
|
||||
|
||||
@mock.patch.object(nodem.Node, 'set_status')
|
||||
@mock.patch.object(pb.Profile, 'check_object')
|
||||
def test_node_check_check_with_exc(self, mock_check, mock_status):
|
||||
node = nodem.Node('node1', PROFILE_ID, '')
|
||||
node.physical_id = 'd94d6333-82e6-4f87-b7ab-b786776df9d1'
|
||||
err = exception.EResourceOperation(op='checking', type='server',
|
||||
id=node.physical_id,
|
||||
message='failed get')
|
||||
mock_check.side_effect = err
|
||||
|
||||
res = node.do_check(self.context)
|
||||
|
||||
self.assertFalse(res)
|
||||
mock_status.assert_called_once_with(
|
||||
node.ERROR,
|
||||
'Failed in checking server %s: failed get' % node.physical_id)
|
||||
|
||||
def test_node_check_no_physical_id(self):
|
||||
node = nodem.Node('node1', PROFILE_ID, '')
|
||||
|
|
Loading…
Reference in New Issue