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.