diff --git a/ironic_inspector/rules.py b/ironic_inspector/rules.py index 90f139703..f9d1d66bd 100644 --- a/ironic_inspector/rules.py +++ b/ironic_inspector/rules.py @@ -201,6 +201,7 @@ class IntrospectionRule(object): ext_mgr = plugins_base.rule_actions_manager() for act in self._actions: ext = ext_mgr[act.action].obj + for formatted_param in ext.FORMATTED_PARAMS: value = act.params.get(formatted_param) if not value or not isinstance(value, six.string_types): @@ -212,15 +213,22 @@ class IntrospectionRule(object): try: act.params[formatted_param] = value.format(data=data) except KeyError as e: - raise utils.Error(_('Invalid formatting variable key ' - 'provided: %s') % e, - node_info=node_info, data=data) - - 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) + 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) LOG.debug('Successfully applied %s', 'rollback actions' if rollback else 'actions', diff --git a/ironic_inspector/test/unit/test_rules.py b/ironic_inspector/test/unit/test_rules.py index 4164a6c6d..80c2865a6 100644 --- a/ironic_inspector/test/unit/test_rules.py +++ b/ironic_inspector/test/unit/test_rules.py @@ -445,6 +445,23 @@ class TestApplyActions(BaseTest): 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) class TestApply(BaseTest): diff --git a/releasenotes/notes/rollback-formatting-7d61c9af2600d42f.yaml b/releasenotes/notes/rollback-formatting-7d61c9af2600d42f.yaml new file mode 100644 index 000000000..23189527d --- /dev/null +++ b/releasenotes/notes/rollback-formatting-7d61c9af2600d42f.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Do not fail the whole introspection due to a value formatting error during + introspection rules rollback. See `bug 1686942 + `_ for an example + and detailed investigation.