openstack/ironic
Code Issues Proposed changes

Validation before perform node deallocation

Browse Source 
Introduces a new validation to ensure the node deallocation
process will only be performed to nodes that have Server
Profile applied in OneView. When the allocation process
fails, the machine will have no Server Profile applied in
OneView and in ironic it will have no reference to any
Server Profile. Under these conditions if the deallocation
process be performed the driver will try to delete a
nonexistent Server Profile in OneView, leaving the node
in error state in ironic.

Change-Id: I83cef7fafac4e22ebf5eb4fe83f6da26ec132c99
Closes-Bug: #1657892
This commit is contained in:
Xavier 2016-10-13 11:29:10 -03:00 committed by Fellype Cavalcante
parent 748e93009a
commit 0b23a041f1
5 changed files with 185 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
ironic/drivers/modules/oneview/deploy_utils.py
View File

@ -20,7 +20,7 @@ from oslo_log import log as logging
from oslo_utils import importutils
from ironic.common import exception
from ironic.common.i18n import _, _LE, _LI
from ironic.common.i18n import _, _LE, _LI, _LW
from ironic.common import states
from ironic.drivers.modules.oneview import common
@ -354,30 +354,30 @@ def deallocate_server_hardware_from_ironic(oneview_client, node):
Hardware to ironic
"""
oneview_info = common.get_oneview_info(node)
oneview_client.power_off(oneview_info)
if is_node_in_use_by_ironic(oneview_client, node):
applied_sp_uuid = oneview_utils.get_uuid_from_uri(
oneview_info.get('applied_server_profile_uri')
)
try:
oneview_client.delete_server_profile(applied_sp_uuid)
_del_applied_server_profile_uri_field(node)
LOG.info(
_LI("Server Profile %(server_profile_uuid)s was successfully"
" deleted from node %(node_uuid)s."
),
{"node_uuid": node.uuid, "server_profile_uuid": applied_sp_uuid}
oneview_info = common.get_oneview_info(node)
server_profile_uuid = oneview_utils.get_uuid_from_uri(
oneview_info.get('applied_server_profile_uri')
)
except oneview_exception.OneViewException as e:
msg = (_("Error while deleting applied Server Profile from node "
"%(node_uuid)s. Error: %(error)s") %
{'node_uuid': node.uuid, 'error': e})
try:
oneview_client.power_off(oneview_info)
oneview_client.delete_server_profile(server_profile_uuid)
_del_applied_server_profile_uri_field(node)
raise exception.OneViewError(
node=node.uuid, reason=msg
)
LOG.info(_LI("Server Profile %(server_profile_uuid)s was deleted "
"from node %(node_uuid)s in OneView."),
{'server_profile_uuid': server_profile_uuid,
'node_uuid': node.uuid})
except (ValueError, oneview_exception.OneViewException) as e:
msg = (_("Error while deleting applied Server Profile from node "
"%(node_uuid)s. Error: %(error)s") %
{'node_uuid': node.uuid, 'error': e})
raise exception.OneViewError(error=msg)
else:
LOG.warning(_LW("Cannot deallocate node %(node_uuid)s "
"in OneView because it is not in use by "
"ironic."), {'node_uuid': node.uuid})

8
ironic/drivers/modules/oneview/power.py
View File

@ -114,6 +114,14 @@ class OneViewPower(base.PowerInterface):
:raises: PowerStateFailure if the power couldn't be set to power_state.
:raises: OneViewError if OneView fails setting the power state.
"""
if deploy_utils.is_node_in_use_by_oneview(self.oneview_client,
task.node):
raise exception.PowerStateFailure(_(
"Cannot set power state '%(power_state)s' to node %(node)s. "
"The node is in use by OneView.") %
{'power_state': power_state,
'node': task.node.uuid})
oneview_info = common.get_oneview_info(task.node)
LOG.debug('Setting power state of node %(node_uuid)s to '

58
ironic/tests/unit/drivers/modules/oneview/test_deploy_utils.py
View File

@ -98,7 +98,12 @@ class OneViewDeployUtilsTestCase(db_base.DbTestCase):
"""`tear_down` behavior when node already has Profile applied
"""
oneview_client = mock_get_ov_client()
sp_uri = '/rest/server-profiles/1234556789'
ov_client = mock_get_ov_client()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
ov_client = mock_get_ov_client.return_value
ov_client.get_server_hardware_by_uuid.return_value = fake_sh
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
@ -109,12 +114,12 @@ class OneViewDeployUtilsTestCase(db_base.DbTestCase):
self.assertTrue(
'applied_server_profile_uri' in task.node.driver_info
)
deploy_utils.tear_down(oneview_client, task)
deploy_utils.tear_down(ov_client, task)
self.assertFalse(
'applied_server_profile_uri' in task.node.driver_info
)
self.assertTrue(
oneview_client.delete_server_profile.called
ov_client.delete_server_profile.called
)
# Tests for prepare_cleaning
@ -184,7 +189,12 @@ class OneViewDeployUtilsTestCase(db_base.DbTestCase):
"""Checks if Server Profile was deleted and its uri removed
"""
oneview_client = mock_get_ov_client()
sp_uri = '/rest/server-profiles/1234556789'
ov_client = mock_get_ov_client()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
ov_client = mock_get_ov_client.return_value
ov_client.get_server_hardware_by_uuid.return_value = fake_sh
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
@ -193,10 +203,10 @@ class OneViewDeployUtilsTestCase(db_base.DbTestCase):
task.node.driver_info = driver_info
self.assertIn('applied_server_profile_uri', task.node.driver_info)
deploy_utils.tear_down_cleaning(oneview_client, task)
deploy_utils.tear_down_cleaning(ov_client, task)
self.assertNotIn('applied_server_profile_uri',
task.node.driver_info)
self.assertTrue(oneview_client.delete_server_profile.called)
self.assertTrue(ov_client.delete_server_profile.called)
# Tests for is_node_in_use_by_oneview
def test_is_node_in_use_by_oneview(self, mock_get_ov_client):
@ -400,3 +410,39 @@ class OneViewDeployUtilsTestCase(db_base.DbTestCase):
self.assertTrue(
'applied_server_profile_uri' not in task.node.driver_info
)
@mock.patch.object(objects.Node, 'save')
def test_deallocate_server_hardware_from_ironic_missing_profile_uuid(
self, mock_node_save, mock_get_ov_client
):
"""Test for case when server profile application fails.
Due to an error when applying Server Profile in OneView,
the node will have no Server Profile uuid in the
'applied_server_profile_uri' namespace. When the method
tested is called without Server Profile uuid, the client
will raise a ValueError when trying to delete the profile,
this error is converted to an OneViewError.
"""
ov_client = mock_get_ov_client.return_value
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = 'any/applied_sp_uri/'
ov_client.get_server_hardware_by_uuid.return_value = fake_sh
ov_client.delete_server_profile.side_effect = ValueError
mock_get_ov_client.return_value = ov_client
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = 'any/applied_sp_uri/'
task.node.driver_info = driver_info
self.assertRaises(
exception.OneViewError,
deploy_utils.deallocate_server_hardware_from_ironic,
ov_client,
task.node
)
self.assertTrue(ov_client.delete_server_profile.called)
self.assertTrue(
'applied_server_profile_uri' in task.node.driver_info
)

113
ironic/tests/unit/drivers/modules/oneview/test_power.py
View File

@ -30,6 +30,7 @@ from ironic.tests.unit.db import base as db_base
from ironic.tests.unit.db import utils as db_utils
from ironic.tests.unit.objects import utils as obj_utils
oneview_models = importutils.try_import('oneview_client.models')
oneview_exceptions = importutils.try_import('oneview_client.exceptions')
POWER_ON = 'On'
@ -142,87 +143,163 @@ class OneViewPowerDriverTestCase(db_base.DbTestCase):
)
def test_set_power_on(self, mock_get_ov_client):
sp_uri = '/any/server-profile'
oneview_client = mock_get_ov_client()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
oneview_client = mock_get_ov_client.return_value
oneview_client.get_server_hardware_by_uuid.return_value = fake_sh
oneview_client.power_on.return_value = POWER_ON
self.driver.power.oneview_client = oneview_client
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = sp_uri
task.node.driver_info = driver_info
self.driver.power.set_power_state(task, states.POWER_ON)
oneview_client.power_on.assert_called_once_with(self.info)
self.info['applied_server_profile_uri'] = sp_uri
oneview_client.power_on.assert_called_once_with(self.info)
def test_set_power_off(self, mock_get_ov_client):
sp_uri = '/any/server-profile'
oneview_client = mock_get_ov_client()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
oneview_client = mock_get_ov_client.return_value
oneview_client.get_server_hardware_by_uuid.return_value = fake_sh
oneview_client.power_off.return_value = POWER_OFF
self.driver.power.oneview_client = oneview_client
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = sp_uri
task.node.driver_info = driver_info
self.driver.power.set_power_state(task, states.POWER_OFF)
oneview_client.power_off.assert_called_once_with(self.info)
self.info['applied_server_profile_uri'] = sp_uri
oneview_client.power_off.assert_called_once_with(self.info)
def test_set_power_on_fail(self, mock_get_ov_client):
sp_uri = '/any/server-profile'
oneview_client = mock_get_ov_client()
oneview_client.power_on.side_effect = \
oneview_exceptions.OneViewException()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
oneview_client.get_server_hardware_by_uuid.return_value = fake_sh
exc = oneview_exceptions.OneViewException()
oneview_client.power_on.side_effect = exc
self.driver.power.oneview_client = oneview_client
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = sp_uri
task.node.driver_info = driver_info
self.assertRaises(exception.OneViewError,
self.driver.power.set_power_state, task,
states.POWER_ON)
self.info['applied_server_profile_uri'] = sp_uri
oneview_client.power_on.assert_called_once_with(self.info)
def test_set_power_off_fail(self, mock_get_ov_client):
sp_uri = '/any/server-profile'
oneview_client = mock_get_ov_client()
oneview_client.power_off.side_effect = \
oneview_exceptions.OneViewException()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
oneview_client.get_server_hardware_by_uuid.return_value = fake_sh
exc = oneview_exceptions.OneViewException()
oneview_client.power_off.side_effect = exc
self.driver.power.oneview_client = oneview_client
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = sp_uri
task.node.driver_info = driver_info
self.assertRaises(exception.OneViewError,
self.driver.power.set_power_state, task,
states.POWER_OFF)
self.info['applied_server_profile_uri'] = sp_uri
oneview_client.power_off.assert_called_once_with(self.info)
def test_set_power_invalid_state(self, mock_get_ov_client):
sp_uri = '/any/server-profile'
oneview_client = mock_get_ov_client()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
oneview_client.get_server_hardware_by_uuid.return_value = fake_sh
exc = oneview_exceptions.OneViewException()
oneview_client.power_off.side_effect = exc
self.driver.power.oneview_client = oneview_client
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = sp_uri
task.node.driver_info = driver_info
self.assertRaises(exception.InvalidParameterValue,
self.driver.power.set_power_state, task,
'fake state')
def test_set_power_reboot(self, mock_get_ov_client):
sp_uri = '/any/server-profile'
oneview_client = mock_get_ov_client()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
oneview_client.get_server_hardware_by_uuid.return_value = fake_sh
oneview_client.power_off.return_value = POWER_OFF
oneview_client.power_on.return_value = POWER_ON
self.driver.power.oneview_client = oneview_client
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = sp_uri
task.node.driver_info = driver_info
self.driver.power.set_power_state(task, states.REBOOT)
oneview_client.power_off.assert_called_once_with(self.info)
oneview_client.power_on.assert_called_once_with(self.info)
self.info['applied_server_profile_uri'] = sp_uri
oneview_client.power_off.assert_called_once_with(self.info)
oneview_client.power_off.assert_called_once_with(self.info)
oneview_client.power_on.assert_called_once_with(self.info)
def test_reboot(self, mock_get_ov_client):
sp_uri = '/any/server-profile'
oneview_client = mock_get_ov_client()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
oneview_client.get_server_hardware_by_uuid.return_value = fake_sh
oneview_client.power_off.return_value = POWER_OFF
oneview_client.power_on.return_value = POWER_ON
self.driver.power.oneview_client = oneview_client
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = sp_uri
task.node.driver_info = driver_info
self.driver.power.reboot(task)
oneview_client.power_off.assert_called_once_with(self.info)
oneview_client.power_on.assert_called_once_with(self.info)
self.info['applied_server_profile_uri'] = sp_uri
oneview_client.power_off.assert_called_once_with(self.info)
oneview_client.power_on.assert_called_once_with(self.info)
def test_reboot_fail(self, mock_get_ov_client):
sp_uri = '/any/server-profile'
oneview_client = mock_get_ov_client()
oneview_client.power_off.side_effect = \
oneview_exceptions.OneViewException()
fake_sh = oneview_models.ServerHardware()
fake_sh.server_profile_uri = sp_uri
oneview_client.get_server_hardware_by_uuid.return_value = fake_sh
exc = oneview_exceptions.OneViewException()
oneview_client.power_off.side_effect = exc
self.driver.power.oneview_client = oneview_client
with task_manager.acquire(self.context,
self.node.uuid) as task:
driver_info = task.node.driver_info
driver_info['applied_server_profile_uri'] = sp_uri
task.node.driver_info = driver_info
self.assertRaises(exception.OneViewError,
self.driver.power.reboot,
task)
oneview_client.power_off.assert_called_once_with(self.info)
self.assertFalse(oneview_client.power_on.called)
self.driver.power.reboot, task)
self.info['applied_server_profile_uri'] = sp_uri
oneview_client.power_off.assert_called_once_with(self.info)
self.assertFalse(oneview_client.power_on.called)

7
releasenotes/notes/fix-oneview-deallocate-server-8256e279af837e5d.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
-
Fixes an issue with oneview driver trying to deallocate a node when
an error is encountered while performing server profile application.
Also ensures only those nodes that are managed by ironic can be
deallocated.