diff options
| -rw-r--r-- | ironic/conductor/steps.py | 11 | ||||
| -rw-r--r-- | ironic/tests/unit/conductor/test_steps.py | 32 | ||||
| -rw-r--r-- | releasenotes/notes/fix-step-priority-overrides-edecff2a6c68dcac.yaml | 9 |
3 files changed, 50 insertions, 2 deletions
diff --git a/ironic/conductor/steps.py b/ironic/conductor/steps.py index 88df4a4bb..4d7570406 100644 --- a/ironic/conductor/steps.py +++ b/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 diff --git a/ironic/tests/unit/conductor/test_steps.py b/ironic/tests/unit/conductor/test_steps.py index 33de7566f..1cba2c7c0 100644 --- a/ironic/tests/unit/conductor/test_steps.py +++ b/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) diff --git a/releasenotes/notes/fix-step-priority-overrides-edecff2a6c68dcac.yaml b/releasenotes/notes/fix-step-priority-overrides-edecff2a6c68dcac.yaml new file mode 100644 index 000000000..46610d824 --- /dev/null +++ b/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. |