Merge "Drop rollback actions for set-XX and extend-XX rules actions"

ironic_inspector/plugins/rules.py

@@ -117,16 +117,6 @@ class SetAttributeAction(base.RuleActionPlugin):
         node_info.patch([{'op': 'add', 'path': params['path'],
                           'value': params['value']}])
 
-    def rollback(self, node_info, params, **kwargs):
-        try:
-            node_info.get_by_path(params['path'])
-        except KeyError:
-            LOG.debug('Field %s was not set, no need for rollback',
-                      params['path'], node_info=node_info)
-            return
-
-        node_info.patch([{'op': 'remove', 'path': params['path']}])
-
 
 class SetCapabilityAction(base.RuleActionPlugin):
     REQUIRED_PARAMS = {'name'}
@@ -136,9 +126,6 @@ class SetCapabilityAction(base.RuleActionPlugin):
         node_info.update_capabilities(
             **{params['name']: params.get('value')})
 
-    def rollback(self, node_info, params, **kwargs):
-        node_info.update_capabilities(**{params['name']: None})
-
 
 class ExtendAttributeAction(base.RuleActionPlugin):
     REQUIRED_PARAMS = {'path', 'value'}
@@ -153,9 +140,3 @@ class ExtendAttributeAction(base.RuleActionPlugin):
             return values
 
         node_info.replace_field(params['path'], _replace, default=[])
-
-    def rollback(self, node_info, params, **kwargs):
-        def _replace(values):
-            return [v for v in values if v != params['value']]
-
-        node_info.replace_field(params['path'], _replace, default=[])

ironic_inspector/test/functional.py

@@ -333,11 +333,6 @@ class Test(Base):
         self.call_continue(self.data)
         eventlet.greenthread.sleep(DEFAULT_SLEEP)
 
-        # clean up for second rule
-        self.cli.node.update.assert_any_call(
-            self.uuid,
-            [{'op': 'remove', 'path': '/extra/bar'}])
-        # applying first rule
         self.cli.node.update.assert_any_call(
             self.uuid,
             [{'op': 'add', 'path': '/extra/foo', 'value': 'bar'}])

ironic_inspector/test/test_plugins_rules.py

@@ -144,19 +144,6 @@ class TestSetAttributeAction(test_base.NodeTest):
                                              'path': '/extra/value',
                                              'value': 42}])
 
-    @mock.patch.object(node_cache.NodeInfo, 'patch')
-    def test_rollback_with_existing(self, mock_patch):
-        self.node.extra = {'value': 'value'}
-        self.act.rollback(self.node_info, self.params)
-        mock_patch.assert_called_once_with([{'op': 'remove',
-                                             'path': '/extra/value'}])
-
-    @mock.patch.object(node_cache.NodeInfo, 'patch')
-    def test_rollback_no_existing(self, mock_patch):
-        self.node.extra = {}
-        self.act.rollback(self.node_info, self.params)
-        self.assertFalse(mock_patch.called)
-
 
 class TestSetCapabilityAction(test_base.NodeTest):
     act = rules_plugins.SetCapabilityAction()
@@ -182,23 +169,6 @@ class TestSetCapabilityAction(test_base.NodeTest):
         new_caps = utils.capabilities_to_dict(patch[0]['value'])
         self.assertEqual({'cap1': 'val', 'x': 'y', 'answer': '42'}, new_caps)
 
-    @mock.patch.object(node_cache.NodeInfo, 'patch')
-    def test_rollback_with_existing(self, mock_patch):
-        self.node.properties = {'capabilities': 'foo:bar,cap1:val'}
-        self.act.rollback(self.node_info, self.params)
-        mock_patch.assert_called_once_with(
-            [{'op': 'add', 'path': '/properties/capabilities',
-              'value': 'foo:bar'}])
-
-    @mock.patch.object(node_cache.NodeInfo, 'patch')
-    def test_rollback_no_existing(self, mock_patch):
-        self.node.properties = {'capabilities': 'foo:bar'}
-        self.act.rollback(self.node_info, self.params)
-        # TODO(dtantsur): make sure it's not called at all
-        mock_patch.assert_called_once_with(
-            [{'op': 'add', 'path': '/properties/capabilities',
-              'value': 'foo:bar'}])
-
 
 class TestExtendAttributeAction(test_base.NodeTest):
     act = rules_plugins.ExtendAttributeAction()
@@ -228,17 +198,3 @@ class TestExtendAttributeAction(test_base.NodeTest):
         self.node.extra['value'] = [42]
         self.act.apply(self.node_info, params)
         self.assertFalse(mock_patch.called)
-
-    @mock.patch.object(node_cache.NodeInfo, 'patch')
-    def test_rollback_with_existing(self, mock_patch):
-        self.node.extra['value'] = [1, 42, 0]
-        self.act.rollback(self.node_info, self.params)
-
-        mock_patch.assert_called_once_with(
-            [{'op': 'replace', 'path': '/extra/value', 'value': [1, 0]}])
-
-    @mock.patch.object(node_cache.NodeInfo, 'patch')
-    def test_rollback_no_existing(self, mock_patch):
-        self.node.extra['value'] = [1, 0]
-        self.act.rollback(self.node_info, self.params)
-        self.assertFalse(mock_patch.called)

releasenotes/notes/no-rollback-e15bc7fee0134545.yaml

@@ -0,0 +1,10 @@
+---
+upgrade:
+  - Introspection rules actions 'set-attribute', 'set-capability' and
+    'extend-attribute' no longer have the opposite effect on nodes that do not
+    match a rule.
+fixes:
+  - Dropped rollback actions from 'set-attribute', 'set-capability' and
+    'extend-attribute' introspection rules actions, as they were confusing,
+    completely undocumented and broke some real world use cases
+    (e.g. setting driver field).