Merge "Enable priority overrides to enable/disable steps"

ironic/conductor/steps.py

@@ -121,15 +121,22 @@ def _get_steps(task, interfaces, get_method, enabled=False,
     for interface in interfaces:
         interface = getattr(task.driver, interface)
         if interface:
-            interface_steps = [x for x in getattr(interface, get_method)(task)
-                               if not enabled or x['priority'] > 0]
+            # NOTE(janders) get all steps to start with, regardless of whether
+            # enabled is True and priority is zero or not; we need to apply
+            # priority overrides prior to filtering out disabled steps
+            interface_steps = [x for x in getattr(interface, get_method)(task)]
             steps.extend(interface_steps)
+    # Iterate over steps to apply prio overrides if set
     if prio_overrides is not None:
         for step in steps:
             override_key = '%(interface)s.%(step)s' % step
             override_value = prio_overrides.get(override_key)
             if override_value:
                 step["priority"] = int(override_value)
+    # NOTE(janders) If enabled is set to True, we filter out steps with zero
+    # priority now, after applying priority overrides
+    if enabled:
+        steps = [x for x in steps if not (x.get('priority') == 0)]
     if sort_step_key:
         steps = _sorted_steps(steps, sort_step_key)
     return steps

ironic/tests/unit/conductor/test_steps.py

@@ -768,6 +768,38 @@ class NodeCleaningStepsTestCase(db_base.DbTestCase):
 
         self.assertEqual(expected_step_priorities, steps)
 
+    @mock.patch('ironic.drivers.modules.fake.FakeDeploy.get_clean_steps',
+                autospec=True)
+    @mock.patch('ironic.drivers.modules.fake.FakePower.get_clean_steps',
+                autospec=True)
+    def test__get_cleaning_steps_priority_override_disable(self,
+                                                           mock_power_steps,
+                                                           mock_deploy_steps):
+        # Test clean_step_priority_override
+        cfg.CONF.set_override('clean_step_priority_override',
+                              ["deploy.erase_disks:0",
+                               "power.update_firmware:0",
+                               "deploy.update_firmware:0", ],
+                              'conductor')
+
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.CLEANING,
+            target_provision_state=states.AVAILABLE)
+
+        mock_power_steps.return_value = [self.power_update]
+        mock_deploy_steps.return_value = [self.deploy_erase,
+                                          self.deploy_update,
+                                          self.deploy_raid]
+
+        with task_manager.acquire(
+                self.context, node.uuid, shared=True) as task:
+            steps = conductor_steps._get_cleaning_steps(task, enabled=True)
+
+        expected_step_priorities = []
+
+        self.assertEqual(expected_step_priorities, steps)
+
     @mock.patch.object(conductor_steps, '_validate_user_clean_steps',
                        autospec=True)
     @mock.patch.object(conductor_steps, '_get_cleaning_steps', autospec=True)

releasenotes/notes/fix-step-priority-overrides-edecff2a6c68dcac.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Resolve issue where ``[conductor]clean_step_priority_override`` values
+    are applied too late, after disabled steps have been already filtered out.
+    With this change, priority overrides are applied prior to filtering out
+    disabled steps, so that this configuration option can use used to enable
+    or disable steps (in particular clean steps) in addition to changing
+    priorities they are run with.