Merge "Don't power off non-deploying iLO nodes in takeover"

ironic/drivers/modules/ilo/deploy.py

@@ -170,7 +170,7 @@ class IloVirtualMediaIscsiDeploy(iscsi_deploy.ISCSIDeploy):
         :param task: a TaskManager instance containing the node to act on.
         :raises: IloOperationError, if some operation on iLO failed.
         """
-        if task.node.provision_state != states.ACTIVE:
+        if task.node.provision_state == states.DEPLOYING:
             _prepare_node_for_deploy(task)
 
         super(IloVirtualMediaIscsiDeploy, self).prepare(task)
@@ -218,7 +218,7 @@ class IloVirtualMediaAgentDeploy(agent.AgentDeploy):
         :param task: a TaskManager instance.
         :raises: IloOperationError, if some operation on iLO failed.
         """
-        if task.node.provision_state != states.ACTIVE:
+        if task.node.provision_state == states.DEPLOYING:
             _prepare_node_for_deploy(task)
 
         super(IloVirtualMediaAgentDeploy, self).prepare(task)
@@ -275,7 +275,7 @@ class IloPXEDeploy(iscsi_deploy.ISCSIDeploy):
         :raises: IloOperationError, if some operation on iLO failed.
         :raises: InvalidParameterValue, if some information is invalid.
         """
-        if task.node.provision_state != states.ACTIVE:
+        if task.node.provision_state == states.DEPLOYING:
             _prepare_node_for_deploy(task)
 
             # Check if 'boot_option' is compatible with 'boot_mode' and image.

ironic/tests/unit/drivers/modules/ilo/test_deploy.py

@@ -263,6 +263,8 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
                        autospec=True)
     def test_prepare(self, func_prepare_node_for_deploy,
                      iscsi_deploy_prepare_mock):
+        self.node.provision_state = states.DEPLOYING
+        self.node.save()
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.deploy.prepare(task)
@@ -275,13 +277,22 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
                        autospec=True)
     def test_prepare_active_node(self, func_prepare_node_for_deploy,
                                  iscsi_deploy_prepare_mock):
-        self.node.provision_state = states.ACTIVE
+        """Ensure nodes in running states are not inadvertently changed"""
-        self.node.save()
+        test_states = list(states.STABLE_STATES)
-        with task_manager.acquire(self.context, self.node.uuid,
+        test_states.extend([states.CLEANING,
-                                  shared=False) as task:
+                           states.CLEANWAIT,
-            task.driver.deploy.prepare(task)
+                           states.INSPECTING])
-            self.assertFalse(func_prepare_node_for_deploy.called)
+        for state in test_states:
-            iscsi_deploy_prepare_mock.assert_called_once_with(mock.ANY, task)
+            self.node.provision_state = state
+            self.node.save()
+            func_prepare_node_for_deploy.reset_mock()
+            iscsi_deploy_prepare_mock.reset_mock()
+            with task_manager.acquire(self.context, self.node.uuid,
+                                      shared=False) as task:
+                task.driver.deploy.prepare(task)
+                self.assertFalse(func_prepare_node_for_deploy.called)
+                iscsi_deploy_prepare_mock.assert_called_once_with(
+                    mock.ANY, task)
 
     @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'prepare_cleaning',
                        spec_set=True, autospec=True)
@@ -360,6 +371,8 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
     def test_prepare(self,
                      agent_prepare_mock,
                      func_prepare_node_for_deploy):
+        self.node.provision_state = states.DEPLOYING
+        self.node.save()
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.deploy.prepare(task)
@@ -373,12 +386,21 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
     def test_prepare_active_node(self,
                                  func_prepare_node_for_deploy,
                                  agent_prepare_mock):
-        with task_manager.acquire(self.context, self.node.uuid,
+        """Ensure nodes in running states are not inadvertently changed"""
-                                  shared=False) as task:
+        test_states = list(states.STABLE_STATES)
-            task.node.provision_state = states.ACTIVE
+        test_states.extend([states.CLEANING,
-            task.driver.deploy.prepare(task)
+                           states.CLEANWAIT,
-            self.assertFalse(func_prepare_node_for_deploy.called)
+                           states.INSPECTING])
-            agent_prepare_mock.assert_called_once_with(mock.ANY, task)
+        for state in test_states:
+            self.node.provision_state = state
+            self.node.save()
+            func_prepare_node_for_deploy.reset_mock()
+            agent_prepare_mock.reset_mock()
+            with task_manager.acquire(self.context, self.node.uuid,
+                                      shared=False) as task:
+                task.driver.deploy.prepare(task)
+                self.assertFalse(func_prepare_node_for_deploy.called)
+                agent_prepare_mock.assert_called_once_with(mock.ANY, task)
 
     @mock.patch.object(deploy_utils, 'agent_get_clean_steps', spec_set=True,
                        autospec=True)
@@ -467,13 +489,15 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
     @mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
                        autospec=True)
     def test_prepare(self,
-                     prepare_node_mock,
+                     prepare_node_for_deploy_mock,
                      pxe_prepare_mock):
+        self.node.provision_state = states.DEPLOYING
+        self.node.save()
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.node.properties['capabilities'] = 'boot_mode:uefi'
             task.driver.deploy.prepare(task)
-            prepare_node_mock.assert_called_once_with(task)
+            prepare_node_for_deploy_mock.assert_called_once_with(task)
             pxe_prepare_mock.assert_called_once_with(mock.ANY, task)
 
     @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'prepare', spec_set=True,
@@ -481,15 +505,24 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
     @mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
                        autospec=True)
     def test_prepare_active_node(self,
-                                 prepare_node_mock,
+                                 prepare_node_for_deploy_mock,
                                  pxe_prepare_mock):
-        with task_manager.acquire(self.context, self.node.uuid,
+        """Ensure nodes in running states are not inadvertently changed"""
-                                  shared=False) as task:
+        test_states = list(states.STABLE_STATES)
-            task.node.provision_state = states.ACTIVE
+        test_states.extend([states.CLEANING,
-            task.node.properties['capabilities'] = 'boot_mode:uefi'
+                           states.CLEANWAIT,
-            task.driver.deploy.prepare(task)
+                           states.INSPECTING])
-            self.assertFalse(prepare_node_mock.called)
+        for state in test_states:
-            pxe_prepare_mock.assert_called_once_with(mock.ANY, task)
+            self.node.provision_state = state
+            self.node.save()
+            prepare_node_for_deploy_mock.reset_mock()
+            pxe_prepare_mock.reset_mock()
+            with task_manager.acquire(self.context, self.node.uuid,
+                                      shared=False) as task:
+                task.node.properties['capabilities'] = 'boot_mode:uefi'
+                task.driver.deploy.prepare(task)
+                self.assertFalse(prepare_node_for_deploy_mock.called)
+                pxe_prepare_mock.assert_called_once_with(mock.ANY, task)
 
     @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'prepare', spec_set=True,
                        autospec=True)
@@ -498,6 +531,8 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
     def test_prepare_uefi_whole_disk_image_fail(self,
                                                 prepare_node_for_deploy_mock,
                                                 pxe_prepare_mock):
+        self.node.provision_state = states.DEPLOYING
+        self.node.save()
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.node.properties['capabilities'] = 'boot_mode:uefi'

releasenotes/notes/ilo-do-not-power-off-non-deploying-nodes-0a3aed7c8ea3940a.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - A bug was identified in the behavior of the ilo driver
+    where nodes that are not active but taking part of a
+    conductor takeover could be powered off.
+    In preparation for new features and functionality,
+    that risk encountering this bug, we are limiting the
+    deployment preparation steps to the ``deploying``
+    state to prevent nodes from being erroniuosly
+    powered off.