openstack
/
ironic
Code Issues Proposed changes

Avoid name errors in oneview periodics 

Solves a bug that can brake the periodic task execution in
oneview drivers. The problem occurs when the method to check
if a node is in use by oneview raise an exception.

When that happens, the periodic task will break its execution,
leaving the node with a wrong provision state and wrong
maintenance mode too. The expected behavior when this occurs
is skip processing the current node, iterate over the next
node and process it.

This patch changes the periodic task execution flow, ignoring
the exception and processing the next node. In the next
periodic task execution, the not processed node can be
processed and updated accordingly.

Co-Authored-By: Xavier <marcusrafael@lsd.ufcg.edu.br>

Change-Id: I4b4b350d82d9e6863e8a4f334d2f9e7f7c192c6e
Closes-Bug: 1629051
(cherry picked from commit 53521b6fd8)
This commit is contained in:
Vladyslav Drok 2016-09-22 20:31:19 +03:00 committed by Xavier
parent fd2e2fa310
commit 45e954d057
3 changed files with 185 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
ironic/drivers/modules/oneview/deploy.py
View File

@ -70,11 +70,17 @@ class OneViewPeriodicTasks(object):
try:
oneview_using = deploy_utils.is_node_in_use_by_oneview(node)
except exception.OneViewError as e:
# NOTE(xavierr): Skip this node and process the
# remaining nodes. This node will be checked in
# the next periodic call.
LOG.error(_LE("Error while determining if node "
"%(node_uuid)s is in use by OneView. "
"Error: %(error)s"),
{'node_uuid': node.uuid, 'error': e})
continue
if oneview_using:
purpose = (_LI('Updating node %(node_uuid)s in use '
'by OneView from %(provision_state)s state '
@ -126,11 +132,17 @@ class OneViewPeriodicTasks(object):
node
)
except exception.OneViewError as e:
# NOTE(xavierr): Skip this node and process the
# remaining nodes. This node will be checked in
# the next periodic call.
LOG.error(_LE("Error while determining if node "
"%(node_uuid)s is in use by OneView. "
"Error: %(error)s"),
{'node_uuid': node.uuid, 'error': e})
continue
if not oneview_using:
purpose = (_LI('Bringing node %(node_uuid)s back from '
'use by OneView from %(provision_state)s '

245
ironic/tests/unit/drivers/modules/oneview/test_deploy.py
View File

@ -19,10 +19,11 @@ import mock
from oslo_utils import importutils
from ironic.common import driver_factory
from ironic.common import exception
from ironic.common import states
from ironic.drivers.modules.oneview import common
from ironic.drivers.modules.oneview import deploy
from ironic.drivers.modules.oneview import deploy_utils
from ironic import objects
from ironic.tests.unit.conductor import mgr_utils
from ironic.tests.unit.db import base as db_base
from ironic.tests.unit.db import utils as db_utils
@ -30,8 +31,57 @@ from ironic.tests.unit.objects import utils as obj_utils
oneview_models = importutils.try_import('oneview_client.models')
METHODS = ['iter_nodes', 'update_node', 'do_provisioning_action']
@mock.patch.object(common, 'get_oneview_client', spec_set=True, autospec=True)
oneview_error = common.SERVER_HARDWARE_ALLOCATION_ERROR
maintenance_reason = common.NODE_IN_USE_BY_ONEVIEW
driver_internal_info = {'oneview_error': oneview_error}
nodes_taken_by_oneview = [(1, 'fake_oneview')]
nodes_freed_by_oneview = [(1, 'fake_oneview', maintenance_reason)]
nodes_taken_on_cleanfail = [(1, 'fake_oneview', driver_internal_info)]
nodes_taken_on_cleanfail_no_info = [(1, 'fake_oneview', {})]
def _setup_node_in_available_state(node):
node.provision_state = states.AVAILABLE
node.maintenance = False
node.maintenance_reason = None
node.save()
def _setup_node_in_manageable_state(node):
node.provision_state = states.MANAGEABLE
node.maintenance = True
node.maintenance_reason = common.NODE_IN_USE_BY_ONEVIEW
node.save()
def _setup_node_in_cleanfailed_state_with_oneview_error(node):
node.provision_state = states.CLEANFAIL
node.maintenance = False
node.maintenance_reason = None
driver_internal_info = node.driver_internal_info
oneview_error = common.SERVER_HARDWARE_ALLOCATION_ERROR
driver_internal_info['oneview_error'] = oneview_error
node.driver_internal_info = driver_internal_info
node.save()
def _setup_node_in_cleanfailed_state_without_oneview_error(node):
node.provision_state = states.CLEANFAIL
node.maintenance = False
node.maintenance_reason = None
node.save()
class OneViewDriverDeploy(deploy.OneViewPeriodicTasks):
oneview_driver = 'fake_oneview'
@mock.patch('ironic.objects.Node', spec_set=True, autospec=True)
@mock.patch.object(deploy_utils, 'is_node_in_use_by_oneview',
spec_set=True, autospec=True)
class OneViewPeriodicTasks(db_base.DbTestCase):
def setUp(self):
@ -41,104 +91,147 @@ class OneViewPeriodicTasks(db_base.DbTestCase):
self.config(password='password', group='oneview')
mgr_utils.mock_the_extension_manager(driver='fake_oneview')
self.driver = driver_factory.get_driver('fake_oneview')
self.driver = driver_factory.get_driver('fake_oneview')
self.deploy = OneViewDriverDeploy()
self.manager = mock.MagicMock(spec=METHODS)
self.node = obj_utils.create_test_node(
self.context, driver='fake_oneview',
properties=db_utils.get_test_oneview_properties(),
driver_info=db_utils.get_test_oneview_driver_info(),
)
self.info = common.get_oneview_info(self.node)
@mock.patch.object(objects.Node, 'get')
@mock.patch.object(deploy_utils, 'is_node_in_use_by_oneview')
def test__periodic_check_nodes_taken_by_oneview(
self, mock_is_node_in_use_by_oneview, mock_get_node,
mock_get_ov_client
def test_node_manageable_maintenance_when_in_use_by_oneview(
self, mock_is_node_in_use_by_oneview, mock_node_get
):
manager = mock.MagicMock(
spec=['iter_nodes', 'update_node', 'do_provisioning_action']
)
manager.iter_nodes.return_value = [
(self.node.uuid, 'fake_oneview')
]
mock_get_node.return_value = self.node
mock_node_get.get.return_value = self.node
_setup_node_in_available_state(self.node)
self.manager.iter_nodes.return_value = nodes_taken_by_oneview
mock_is_node_in_use_by_oneview.return_value = True
class OneViewDriverDeploy(deploy.OneViewPeriodicTasks):
oneview_driver = 'fake_oneview'
oneview_driver_deploy = OneViewDriverDeploy()
oneview_driver_deploy._periodic_check_nodes_taken_by_oneview(
manager, self.context
self.deploy._periodic_check_nodes_taken_by_oneview(
self.manager, self.context
)
self.assertTrue(manager.update_node.called)
self.assertTrue(manager.do_provisioning_action.called)
mock_is_node_in_use_by_oneview.assert_called_once_with(self.node)
self.assertTrue(self.manager.update_node.called)
self.assertTrue(self.manager.do_provisioning_action.called)
self.assertTrue(self.node.maintenance)
self.assertEqual(common.NODE_IN_USE_BY_ONEVIEW,
self.node.maintenance_reason)
@mock.patch.object(deploy_utils, 'is_node_in_use_by_oneview')
def test__periodic_check_nodes_freed_by_oneview(
self, mock_is_node_in_use_by_oneview, mock_get_ov_client
def test_node_stay_available_when_not_in_use_by_oneview(
self, mock_is_node_in_use_by_oneview, mock_node_get
):
manager = mock.MagicMock(
spec=['iter_nodes', 'update_node', 'do_provisioning_action']
)
manager.iter_nodes.return_value = [
(self.node.uuid, 'fake_oneview',
common.NODE_IN_USE_BY_ONEVIEW)
]
mock_node_get.get.return_value = self.node
_setup_node_in_available_state(self.node)
mock_node_get.return_value = self.node
mock_is_node_in_use_by_oneview.return_value = False
class OneViewDriverDeploy(deploy.OneViewPeriodicTasks):
oneview_driver = 'fake_oneview'
oneview_driver_deploy = OneViewDriverDeploy()
oneview_driver_deploy._periodic_check_nodes_freed_by_oneview(
manager, self.context
self.manager.iter_nodes.return_value = nodes_taken_by_oneview
self.deploy._periodic_check_nodes_taken_by_oneview(
self.manager, self.context
)
self.assertTrue(manager.update_node.called)
self.assertTrue(manager.do_provisioning_action.called)
mock_is_node_in_use_by_oneview.assert_called_once_with(self.node)
self.assertFalse(self.manager.update_node.called)
self.assertFalse(self.manager.do_provisioning_action.called)
self.assertFalse(self.node.maintenance)
self.assertIsNone(self.node.maintenance_reason)
@mock.patch.object(objects.Node, 'get')
def test__periodic_check_nodes_taken_on_cleanfail(
self, mock_get_node, mock_get_ov_client
def test_node_stay_available_when_raise_exception(
self, mock_is_node_in_use_by_oneview, mock_node_get
):
driver_internal_info = {
'oneview_error': common.SERVER_HARDWARE_ALLOCATION_ERROR
}
manager = mock.MagicMock(
spec=['iter_nodes', 'update_node', 'do_provisioning_action']
mock_node_get.get.return_value = self.node
_setup_node_in_available_state(self.node)
side_effect = exception.OneViewError('boom')
mock_is_node_in_use_by_oneview.side_effect = side_effect
self.manager.iter_nodes.return_value = nodes_taken_by_oneview
self.deploy._periodic_check_nodes_taken_by_oneview(
self.manager, self.context
)
mock_is_node_in_use_by_oneview.assert_called_once_with(self.node)
self.assertFalse(self.manager.update_node.called)
self.assertFalse(self.manager.do_provisioning_action.called)
self.assertFalse(self.node.maintenance)
self.assertNotEqual(common.NODE_IN_USE_BY_ONEVIEW,
self.node.maintenance_reason)
manager.iter_nodes.return_value = [
(self.node.uuid, 'fake_oneview', driver_internal_info)
]
self.node.driver_internal_info = driver_internal_info
mock_get_node.return_value = self.node
class OneViewDriverDeploy(deploy.OneViewPeriodicTasks):
oneview_driver = 'fake_oneview'
oneview_driver_deploy = OneViewDriverDeploy()
oneview_driver_deploy._periodic_check_nodes_taken_on_cleanfail(
manager, self.context
def test_node_available_when_not_in_use_by_oneview(
self, mock_is_node_in_use_by_oneview, mock_node_get
):
mock_node_get.get.return_value = self.node
_setup_node_in_manageable_state(self.node)
self.manager.iter_nodes.return_value = nodes_freed_by_oneview
mock_is_node_in_use_by_oneview.return_value = False
self.deploy._periodic_check_nodes_freed_by_oneview(
self.manager, self.context
)
self.assertTrue(manager.update_node.called)
self.assertTrue(manager.do_provisioning_action.called)
mock_is_node_in_use_by_oneview.assert_called_once_with(self.node)
self.assertTrue(self.manager.update_node.called)
self.assertTrue(self.manager.do_provisioning_action.called)
self.assertFalse(self.node.maintenance)
self.assertIsNone(self.node.maintenance_reason)
def test_node_stay_manageable_when_in_use_by_oneview(
self, mock_is_node_in_use_by_oneview, mock_node_get
):
mock_node_get.get.return_value = self.node
_setup_node_in_manageable_state(self.node)
mock_is_node_in_use_by_oneview.return_value = True
self.manager.iter_nodes.return_value = nodes_freed_by_oneview
self.deploy._periodic_check_nodes_freed_by_oneview(
self.manager, self.context
)
mock_is_node_in_use_by_oneview.assert_called_once_with(self.node)
self.assertFalse(self.manager.update_node.called)
self.assertFalse(self.manager.do_provisioning_action.called)
self.assertTrue(self.node.maintenance)
self.assertEqual(common.NODE_IN_USE_BY_ONEVIEW,
self.node.maintenance_reason)
self.assertEqual({}, self.node.driver_internal_info)
def test_node_stay_manageable_maintenance_when_raise_exception(
self, mock_is_node_in_use_by_oneview, mock_node_get
):
mock_node_get.get.return_value = self.node
_setup_node_in_manageable_state(self.node)
side_effect = exception.OneViewError('boom')
mock_is_node_in_use_by_oneview.side_effect = side_effect
self.manager.iter_nodes.return_value = nodes_freed_by_oneview
self.deploy._periodic_check_nodes_freed_by_oneview(
self.manager, self.context
)
mock_is_node_in_use_by_oneview.assert_called_once_with(self.node)
self.assertFalse(self.manager.update_node.called)
self.assertFalse(self.manager.do_provisioning_action.called)
self.assertTrue(self.node.maintenance)
self.assertEqual(common.NODE_IN_USE_BY_ONEVIEW,
self.node.maintenance_reason)
def test_node_manageable_maintenance_when_oneview_error(
self, mock_is_node_in_use_by_oneview, mock_node_get
):
mock_node_get.get.return_value = self.node
_setup_node_in_cleanfailed_state_with_oneview_error(self.node)
self.manager.iter_nodes.return_value = nodes_taken_on_cleanfail
self.deploy._periodic_check_nodes_taken_on_cleanfail(
self.manager, self.context
)
self.assertTrue(self.manager.update_node.called)
self.assertTrue(self.manager.do_provisioning_action.called)
self.assertTrue(self.node.maintenance)
self.assertEqual(common.NODE_IN_USE_BY_ONEVIEW,
self.node.maintenance_reason)
self.assertFalse('oneview_error' in self.node.driver_internal_info)
def test_node_stay_clean_failed_when_no_oneview_error(
self, mock_is_node_in_use_by_oneview, mock_node_get
):
mock_node_get.get.return_value = self.node
_setup_node_in_cleanfailed_state_without_oneview_error(self.node)
self.manager.iter_nodes.return_value = nodes_taken_on_cleanfail_no_info
self.deploy._periodic_check_nodes_taken_on_cleanfail(
self.manager, self.context
)
self.assertFalse(self.manager.update_node.called)
self.assertFalse(self.manager.do_provisioning_action.called)
self.assertFalse(self.node.maintenance)
self.assertNotEqual(common.NODE_IN_USE_BY_ONEVIEW,
self.node.maintenance_reason)
self.assertFalse('oneview_error' in self.node.driver_internal_info)

4
releasenotes/notes/fix-oneview-periodics-0f535fe7a0ad83cd.yaml Normal file
View File

@ -0,0 +1,4 @@
---
fixes:
- Fixes a bug in the oneview driver where the periodic task to
check if a node is in use by oneview may end prematurely.