Merge "Reject explicit requests to power off nodes with disable_power_off"

ironic/api/controllers/v1/node.py

@@ -884,6 +884,10 @@ class NodeStatesController(rest.RestController):
                 action=target, node=node_ident,
                 state=rpc_node.provision_state)
 
+        elif (target in (ir_states.POWER_OFF, ir_states.SOFT_POWER_OFF)
+              and rpc_node.disable_power_off):
+            raise exception.PowerStateFailure(pstate=target)
+
         api.request.rpcapi.change_node_power_state(api.request.context,
                                                    rpc_node.uuid, target,
                                                    timeout=timeout,

ironic/conductor/manager.py

@@ -4133,7 +4133,8 @@ def do_sync_power_state(task, count):
         return count
 
     if (CONF.conductor.force_power_state_during_sync
-            and task.driver.power.supports_power_sync(task)):
+            and task.driver.power.supports_power_sync(task)
+            and not node.disable_power_off):
         LOG.warning("During sync_power_state, node %(node)s state "
                     "'%(actual)s' does not match expected state. "
                     "Changing hardware state to '%(state)s'.",

ironic/conductor/utils.py

@@ -297,6 +297,12 @@ def node_power_action(task, new_state, timeout=None):
         new_state)
     node = task.node
 
+    target_state = _calculate_target_state(new_state)
+    if target_state == states.POWER_OFF and node.disable_power_off:
+        LOG.error("Refusing to power off node %s with disable_power_off "
+                  "flag set", node.uuid)
+        raise exception.PowerStateFailure(pstate=new_state)
+
     if _can_skip_state_change(task, new_state):
         # NOTE(TheJulia): Even if we are not changing the power state,
         # we need to wipe the token out, just in case for some reason
@@ -306,7 +312,6 @@ def node_power_action(task, new_state, timeout=None):
             wipe_internal_info_on_power_off(node)
             node.save()
         return
-    target_state = _calculate_target_state(new_state)
 
     # Set the target_power_state and clear any last_error, if we're
     # starting a new operation. This will expose to other processes

ironic/tests/unit/api/controllers/v1/test_node.py

@@ -5548,6 +5548,18 @@ class TestPut(test_api_base.BaseApiTest):
         self._test_power_state_failure(
             states.SOFT_POWER_OFF, http_client.NOT_ACCEPTABLE, 0, "1.26")
 
+    def test_power_state_power_off_with_disable_power_off(self):
+        self.node.disable_power_off = True
+        self.node.save()
+        self._test_power_state_failure(
+            states.POWER_OFF, http_client.CONFLICT, None, None)
+
+    def test_power_state_soft_power_off_with_disable_power_off(self):
+        self.node.disable_power_off = True
+        self.node.save()
+        self._test_power_state_failure(
+            states.SOFT_POWER_OFF, http_client.CONFLICT, None, "1.27")
+
     def test_power_state_by_name_unsupported(self):
         response = self.put_json('/nodes/%s/states/power' % self.node.name,
                                  {'target': states.POWER_ON},

ironic/tests/unit/conductor/test_manager.py

@@ -5569,6 +5569,22 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase):
         mock_power_update.assert_called_once_with(
             self.task.context, self.node.instance_uuid, states.POWER_OFF)
 
+    @mock.patch.object(nova, 'power_update', autospec=True)
+    def test_state_changed_no_sync_with_disable_power_off(self,
+                                                          mock_power_update,
+                                                          node_power_action):
+        self.config(force_power_state_during_sync=True, group='conductor')
+        self.node.disable_power_off = True
+        self._do_sync_power_state(states.POWER_ON, states.POWER_OFF)
+
+        self.power.validate.assert_not_called()
+        self.power.get_power_state.assert_called_once_with(self.task)
+        node_power_action.assert_not_called()
+        self.assertEqual(states.POWER_OFF, self.node.power_state)
+        self.task.upgrade_lock.assert_called_once_with()
+        mock_power_update.assert_called_once_with(
+            self.task.context, self.node.instance_uuid, states.POWER_OFF)
+
     @mock.patch('ironic.objects.node.NodeCorrectedPowerStateNotification',
                 autospec=True)
     @mock.patch.object(nova, 'power_update', autospec=True)

ironic/tests/unit/conductor/test_utils.py

@@ -355,7 +355,8 @@ class NodePowerActionTestCase(db_base.DbTestCase):
 
     @mock.patch.object(fake.FakePower, 'reboot', autospec=True)
     @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True)
-    def test_node_power_action_power_reboot(self, get_power_mock, reboot_mock):
+    def _test_node_power_action_power_reboot(self, get_power_mock, reboot_mock,
+                                             disable_power_off=False):
         """Test for reboot a node."""
         dii = {'agent_secret_token': 'token',
                'agent_cached_deploy_steps': ['steps']}
@@ -363,7 +364,8 @@ class NodePowerActionTestCase(db_base.DbTestCase):
                                           uuid=uuidutils.generate_uuid(),
                                           driver='fake-hardware',
                                           power_state=states.POWER_ON,
-                                          driver_internal_info=dii)
+                                          driver_internal_info=dii,
+                                          disable_power_off=disable_power_off)
         task = task_manager.TaskManager(self.context, node.uuid)
 
         conductor_utils.node_power_action(task, states.REBOOT)
@@ -376,6 +378,12 @@ class NodePowerActionTestCase(db_base.DbTestCase):
         self.assertIsNone(node['last_error'])
         self.assertNotIn('agent_secret_token', node['driver_internal_info'])
 
+    def test_node_power_action_power_reboot(self):
+        self._test_node_power_action_power_reboot()
+
+    def test_node_power_action_power_reboot_with_disable_power_off(self):
+        self._test_node_power_action_power_reboot(disable_power_off=True)
+
     @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True)
     def test_node_power_action_invalid_state(self, get_power_mock):
         """Test for exception when changing to an invalid power state."""
@@ -405,6 +413,30 @@ class NodePowerActionTestCase(db_base.DbTestCase):
         self.assertIsNone(node['target_power_state'])
         self.assertIsNone(node['last_error'])
 
+    @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True)
+    def test_node_power_action_disable_power_off(self, get_power_mock):
+        """Test for exception when power off is disabled."""
+        node = obj_utils.create_test_node(self.context,
+                                          uuid=uuidutils.generate_uuid(),
+                                          driver='fake-hardware',
+                                          power_state=states.POWER_ON,
+                                          disable_power_off=True)
+        task = task_manager.TaskManager(self.context, node.uuid)
+
+        get_power_mock.return_value = states.POWER_ON
+
+        self.assertRaises(exception.PowerStateFailure,
+                          conductor_utils.node_power_action,
+                          task, states.POWER_OFF)
+
+        node.refresh()
+        get_power_mock.assert_not_called()
+        self.assertEqual(states.POWER_ON, node['power_state'])
+        self.assertIsNone(node['target_power_state'])
+        # Not updating last_error - this condition should be caught on the API
+        # level and reported immediately.
+        self.assertIsNone(node['last_error'])
+
     @mock.patch('ironic.objects.node.NodeSetPowerStateNotification',
                 autospec=True)
     @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True)