Merge "Validation before perform node deallocation"

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 = common.get_oneview_info(node)
-        oneview_info.get('applied_server_profile_uri')
+        server_profile_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}
         )
-    except oneview_exception.OneViewException as e:
 
-        msg = (_("Error while deleting applied Server Profile from node "
+        try:
-                 "%(node_uuid)s. Error: %(error)s") %
+            oneview_client.power_off(oneview_info)
-               {'node_uuid': node.uuid, 'error': e})
+            oneview_client.delete_server_profile(server_profile_uuid)
+            _del_applied_server_profile_uri_field(node)
 
-        raise exception.OneViewError(
+            LOG.info(_LI("Server Profile %(server_profile_uuid)s was deleted "
-            node=node.uuid, reason=msg
+                         "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})

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 '

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
+            )

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 = \
+        fake_sh = oneview_models.ServerHardware()
-            oneview_exceptions.OneViewException()
+        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 = \
+        fake_sh = oneview_models.ServerHardware()
-            oneview_exceptions.OneViewException()
+        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)
+            self.info['applied_server_profile_uri'] = sp_uri
-        oneview_client.power_on.assert_called_once_with(self.info)
+            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)
-
+            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)
+            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 = \
+        fake_sh = oneview_models.ServerHardware()
-            oneview_exceptions.OneViewException()
+        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,
+                              self.driver.power.reboot, task)
-                              task)
+            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)
+            self.assertFalse(oneview_client.power_on.called)
-        self.assertFalse(oneview_client.power_on.called)

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.