diff --git a/ironic_inspector/plugins/base.py b/ironic_inspector/plugins/base.py index dbb2a6239..bfd8322ed 100644 --- a/ironic_inspector/plugins/base.py +++ b/ironic_inspector/plugins/base.py @@ -137,17 +137,6 @@ class RuleActionPlugin(WithValidation): # pragma: no cover :raises: utils.Error on failure """ - def rollback(self, node_info, params, **kwargs): - """Rollback action effects from previous run on a failed match. - - Default implementation does nothing. - - :param node_info: NodeInfo object - :param params: parameters as a dictionary - :param kwargs: used for extensibility without breaking existing plugins - :raises: utils.Error on failure - """ - _HOOKS_MGR = None _NOT_FOUND_HOOK_MGR = None @@ -239,10 +228,4 @@ def rule_actions_manager(): _ACTIONS_MGR = stevedore.ExtensionManager( 'ironic_inspector.rules.actions', invoke_on_load=True) - for act in _ACTIONS_MGR: - # a trick to detect if function was overridden - if "rollback" in act.obj.__class__.__dict__: - LOG.warning('Defining "rollback" for introspection rules ' - 'actions is deprecated (action "%s")', - act.name) return _ACTIONS_MGR diff --git a/ironic_inspector/rules.py b/ironic_inspector/rules.py index f9d1d66bd..22bb87987 100644 --- a/ironic_inspector/rules.py +++ b/ironic_inspector/rules.py @@ -182,20 +182,13 @@ class IntrospectionRule(object): node_info=node_info, data=data) return True - def apply_actions(self, node_info, rollback=False, data=None): + def apply_actions(self, node_info, data=None): """Run actions on a node. :param node_info: NodeInfo instance - :param rollback: if True, rollback actions are executed :param data: introspection data """ - if rollback: - method = 'rollback' - else: - method = 'apply' - - LOG.debug('Running %(what)s actions for rule "%(rule)s"', - {'what': method, 'rule': self.description}, + LOG.debug('Running actions for rule "%s"', self.description, node_info=node_info, data=data) ext_mgr = plugins_base.rule_actions_manager() @@ -213,25 +206,16 @@ class IntrospectionRule(object): try: act.params[formatted_param] = value.format(data=data) except KeyError as e: - if rollback: - LOG.warning('Invalid formatting variable key provided:' - ' %(key)s, skipping rollback for action ' - '%(action)s', - {'key': e, 'action': act.action}) - break - else: - raise utils.Error(_('Invalid formatting variable key ' - 'provided: %s') % e, - node_info=node_info, data=data) - else: - LOG.debug('Running %(what)s action `%(action)s %(params)s`', - {'action': act.action, 'params': act.params, - 'what': method}, - node_info=node_info, data=data) - getattr(ext, method)(node_info, act.params) + raise utils.Error(_('Invalid formatting variable key ' + 'provided: %s') % e, + node_info=node_info, data=data) - LOG.debug('Successfully applied %s', - 'rollback actions' if rollback else 'actions', + LOG.debug('Running action `%(action)s %(params)s`', + {'action': act.action, 'params': act.params}, + node_info=node_info, data=data) + ext.apply(node_info, act.params) + + LOG.debug('Successfully applied actions', node_info=node_info, data=data) @@ -425,26 +409,15 @@ def apply(node_info, data): LOG.debug('Applying custom introspection rules', node_info=node_info, data=data) - to_rollback = [] to_apply = [] for rule in rules: if rule.check_conditions(node_info, data): to_apply.append(rule) - else: - to_rollback.append(rule) - - if to_rollback: - LOG.debug('Running rollback actions', node_info=node_info, data=data) - for rule in to_rollback: - rule.apply_actions(node_info, rollback=True, data=data) - else: - LOG.debug('No rollback actions to apply', - node_info=node_info, data=data) if to_apply: LOG.debug('Running actions', node_info=node_info, data=data) for rule in to_apply: - rule.apply_actions(node_info, rollback=False, data=data) + rule.apply_actions(node_info, data=data) else: LOG.debug('No actions to apply', node_info=node_info, data=data) diff --git a/ironic_inspector/test/unit/test_rules.py b/ironic_inspector/test/unit/test_rules.py index 80c2865a6..a2a51d7ec 100644 --- a/ironic_inspector/test/unit/test_rules.py +++ b/ironic_inspector/test/unit/test_rules.py @@ -390,7 +390,6 @@ class TestApplyActions(BaseTest): self.act_mock.apply.assert_any_call(self.node_info, {}) self.assertEqual(len(self.actions_json), self.act_mock.apply.call_count) - self.assertFalse(self.act_mock.rollback.called) def test_apply_data_format_value(self, mock_ext_mgr): self.rule = rules.create(actions_json=[ @@ -404,7 +403,6 @@ class TestApplyActions(BaseTest): self.rule.apply_actions(self.node_info, data=self.data) self.assertEqual(1, self.act_mock.apply.call_count) - self.assertFalse(self.act_mock.rollback.called) def test_apply_data_format_value_fail(self, mock_ext_mgr): self.rule = rules.create( @@ -431,36 +429,6 @@ class TestApplyActions(BaseTest): self.rule.apply_actions(self.node_info, data=self.data) self.assertEqual(1, self.act_mock.apply.call_count) - self.assertFalse(self.act_mock.rollback.called) - - def test_rollback(self, mock_ext_mgr): - mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock - - self.rule.apply_actions(self.node_info, rollback=True) - - self.act_mock.rollback.assert_any_call(self.node_info, - {'message': 'boom!'}) - self.act_mock.rollback.assert_any_call(self.node_info, {}) - self.assertEqual(len(self.actions_json), - self.act_mock.rollback.call_count) - self.assertFalse(self.act_mock.apply.called) - - @mock.patch.object(rules.LOG, 'warning', autospec=True) - def test_rollback_ignores_formatting_failures(self, mock_log, - mock_ext_mgr): - self.rule = rules.create( - actions_json=[ - {'action': 'set-attribute', - 'path': '/driver_info/ipmi_address', - 'value': '{data[foo][bar]}'}], - conditions_json=self.conditions_json - ) - mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock - - self.rule.apply_actions(self.node_info, rollback=True, data=self.data) - - self.assertTrue(mock_log.called) - self.assertFalse(self.act_mock.rollback.called) @mock.patch.object(rules, 'get_all', autospec=True) @@ -475,7 +443,7 @@ class TestApply(BaseTest): rules.apply(self.node_info, self.data) - def test_no_actions(self, mock_get_all): + def test_apply(self, mock_get_all): mock_get_all.return_value = self.rules for idx, rule in enumerate(self.rules): rule.check_conditions.return_value = not bool(idx) @@ -485,44 +453,8 @@ class TestApply(BaseTest): for idx, rule in enumerate(self.rules): rule.check_conditions.assert_called_once_with(self.node_info, self.data) - rule.apply_actions.assert_called_once_with( - self.node_info, rollback=bool(idx), data=self.data) - - def test_actions(self, mock_get_all): - mock_get_all.return_value = self.rules - for idx, rule in enumerate(self.rules): - rule.check_conditions.return_value = not bool(idx) - - rules.apply(self.node_info, self.data) - - for idx, rule in enumerate(self.rules): - rule.check_conditions.assert_called_once_with(self.node_info, - self.data) - rule.apply_actions.assert_called_once_with( - self.node_info, rollback=bool(idx), data=self.data) - - def test_no_rollback(self, mock_get_all): - mock_get_all.return_value = self.rules - for rule in self.rules: - rule.check_conditions.return_value = True - - rules.apply(self.node_info, self.data) - - for rule in self.rules: - rule.check_conditions.assert_called_once_with(self.node_info, - self.data) - rule.apply_actions.assert_called_once_with( - self.node_info, rollback=False, data=self.data) - - def test_only_rollback(self, mock_get_all): - mock_get_all.return_value = self.rules - for rule in self.rules: - rule.check_conditions.return_value = False - - rules.apply(self.node_info, self.data) - - for rule in self.rules: - rule.check_conditions.assert_called_once_with(self.node_info, - self.data) - rule.apply_actions.assert_called_once_with( - self.node_info, rollback=True, data=self.data) + if rule.check_conditions.return_value: + rule.apply_actions.assert_called_once_with( + self.node_info, data=self.data) + else: + self.assertFalse(rule.apply_actions.called) diff --git a/releasenotes/notes/rollback-removal-a03a989e2e9f776b.yaml b/releasenotes/notes/rollback-removal-a03a989e2e9f776b.yaml new file mode 100644 index 000000000..f48386c99 --- /dev/null +++ b/releasenotes/notes/rollback-removal-a03a989e2e9f776b.yaml @@ -0,0 +1,4 @@ +--- +upgrade: + - | + Support for rollback actions in introspection rules was removed.