diff --git a/ironic/drivers/modules/oneview/deploy_utils.py b/ironic/drivers/modules/oneview/deploy_utils.py
index 25aa15d0a7..4a032aefe7 100644
--- a/ironic/drivers/modules/oneview/deploy_utils.py
+++ b/ironic/drivers/modules/oneview/deploy_utils.py
@@ -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})
diff --git a/ironic/drivers/modules/oneview/power.py b/ironic/drivers/modules/oneview/power.py
index 82c05693bd..3ea0d94b43 100644
--- a/ironic/drivers/modules/oneview/power.py
+++ b/ironic/drivers/modules/oneview/power.py
@@ -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 '
diff --git a/ironic/tests/unit/drivers/modules/oneview/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/oneview/test_deploy_utils.py
index c19a613996..10e727bff6 100644
--- a/ironic/tests/unit/drivers/modules/oneview/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/oneview/test_deploy_utils.py
@@ -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
+ )
diff --git a/ironic/tests/unit/drivers/modules/oneview/test_power.py b/ironic/tests/unit/drivers/modules/oneview/test_power.py
index 5800a43a9b..7e99dbe91d 100644
--- a/ironic/tests/unit/drivers/modules/oneview/test_power.py
+++ b/ironic/tests/unit/drivers/modules/oneview/test_power.py
@@ -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)
diff --git a/releasenotes/notes/fix-oneview-deallocate-server-8256e279af837e5d.yaml b/releasenotes/notes/fix-oneview-deallocate-server-8256e279af837e5d.yaml
new file mode 100644
index 0000000000..15cecc2792
--- /dev/null
+++ b/releasenotes/notes/fix-oneview-deallocate-server-8256e279af837e5d.yaml
@@ -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.