Remove deprecated support for passing patches lists into hooks

Long time ago we had an idea of batching node updates. However,
it fails miserably when a plugin or an introspection rule has to inspect
existing node properties, because it receives possibly stale values.
We've deprecated support for batching patches back in Liberty, this
patch removes the associated bits from the hook interface.

Change-Id: Ia482ff50ca276ce1ffec631f016c6a6b54d5a4ab
Closes-Bug: #1506348

CONTRIBUTING.rst

@@ -210,10 +210,6 @@ Writing a Plugin
       updated on a node.  Please refer to the docstring for details
       and examples.
 
-      .. note::
-        Keyword arguments node_patches and port_patches are also provided, but
-        should not be used in new plugins.
-
   Make your plugin a setuptools entry point under
   ``ironic_inspector.hooks.processing`` namespace and enable it in the
   configuration file (``processing.processing_hooks`` option).

ironic_inspector/plugins/base.py

@@ -48,9 +48,7 @@ class ProcessingHook(object):  # pragma: no cover
 
         :param introspection_data: processed data from the ramdisk.
         :param node_info: NodeInfo instance.
-        :param kwargs: used for extensibility without breaking existing hooks,
-                       currently deprecated arguments node_patches and
-                       ports_patches are provided here.
+        :param kwargs: used for extensibility without breaking existing hooks.
         :returns: nothing.
 
         [RFC 6902] - http://tools.ietf.org/html/rfc6902

ironic_inspector/plugins/standard.py

@@ -41,8 +41,7 @@ class RootDiskSelectionHook(base.ProcessingHook):
     might not be updated.
     """
 
-    def before_update(self, introspection_data, node_info, node_patches,
-                      ports_patches, **kwargs):
+    def before_update(self, introspection_data, node_info, **kwargs):
         """Detect root disk from root device hints and IPA inventory."""
         hints = node_info.node().properties.get('root_device')
         if not hints:

ironic_inspector/process.py

@@ -17,7 +17,7 @@ import eventlet
 from ironicclient import exceptions
 from oslo_config import cfg
 
-from ironic_inspector.common.i18n import _, _LE, _LI, _LW
+from ironic_inspector.common.i18n import _, _LE, _LI
 from ironic_inspector.common import swift
 from ironic_inspector import firewall
 from ironic_inspector import node_cache
@@ -130,19 +130,7 @@ def _run_post_hooks(node_info, introspection_data):
     hooks = plugins_base.processing_hooks_manager()
 
     for hook_ext in hooks:
-        node_patches = []
-        ports_patches = {}
-        hook_ext.obj.before_update(introspection_data, node_info,
-                                   node_patches=node_patches,
-                                   ports_patches=ports_patches)
-        if node_patches:
-            LOG.warning(_LW('Using node_patches is deprecated'))
-            node_info.patch(node_patches)
-
-        if ports_patches:
-            LOG.warning(_LW('Using ports_patches is deprecated'))
-            for mac, patches in ports_patches.items():
-                node_info.patch_port(mac, patches)
+        hook_ext.obj.before_update(introspection_data, node_info)
 
 
 def _process_node(node, introspection_data, node_info):

ironic_inspector/test/test_plugins_standard.py

@@ -340,7 +340,7 @@ class TestRootDiskSelection(test_base.NodeTest):
                                    **{'node.return_value': self.node})
 
     def test_no_hints(self):
-        self.hook.before_update(self.data, self.node_info, None, None)
+        self.hook.before_update(self.data, self.node_info)
 
         self.assertNotIn('local_gb', self.data)
         self.assertNotIn('root_disk', self.data)
@@ -352,7 +352,7 @@ class TestRootDiskSelection(test_base.NodeTest):
         self.assertRaisesRegexp(utils.Error,
                                 'requires ironic-python-agent',
                                 self.hook.before_update,
-                                self.data, self.node_info, None, None)
+                                self.data, self.node_info)
 
         self.assertNotIn('local_gb', self.data)
         self.assertNotIn('root_disk', self.data)
@@ -364,12 +364,12 @@ class TestRootDiskSelection(test_base.NodeTest):
         self.assertRaisesRegexp(utils.Error,
                                 'No disks found',
                                 self.hook.before_update,
-                                self.data, self.node_info, None, None)
+                                self.data, self.node_info)
 
     def test_one_matches(self):
         self.node.properties['root_device'] = {'size': 10}
 
-        self.hook.before_update(self.data, self.node_info, None, None)
+        self.hook.before_update(self.data, self.node_info)
 
         self.assertEqual(self.matched, self.data['root_disk'])
 
@@ -377,7 +377,7 @@ class TestRootDiskSelection(test_base.NodeTest):
         self.node.properties['root_device'] = {'size': 10,
                                                'model': 'Model 3'}
 
-        self.hook.before_update(self.data, self.node_info, None, None)
+        self.hook.before_update(self.data, self.node_info)
 
         self.assertEqual(self.matched, self.data['root_disk'])
 
@@ -388,7 +388,7 @@ class TestRootDiskSelection(test_base.NodeTest):
         self.assertRaisesRegexp(utils.Error,
                                 'No disks satisfied root device hints',
                                 self.hook.before_update,
-                                self.data, self.node_info, None, None)
+                                self.data, self.node_info)
 
         self.assertNotIn('local_gb', self.data)
         self.assertNotIn('root_disk', self.data)

ironic_inspector/test/test_process.py

@@ -300,9 +300,7 @@ class TestProcessNode(BaseTest):
         self.cli.node.set_power_state.assert_called_once_with(self.uuid, 'off')
         self.assertFalse(self.cli.node.validate.called)
 
-        post_hook_mock.assert_called_once_with(self.data, self.node_info,
-                                               node_patches=mock.ANY,
-                                               ports_patches=mock.ANY)
+        post_hook_mock.assert_called_once_with(self.data, self.node_info)
         finished_mock.assert_called_once_with(mock.ANY)
 
     def test_overwrite_disabled(self, filters_mock, post_hook_mock):
@@ -328,24 +326,6 @@ class TestProcessNode(BaseTest):
                                              address=self.macs[1])
         self.assertCalledWithPatch(self.patch_props, self.cli.node.update)
 
-    def test_hook_patches(self, filters_mock, post_hook_mock):
-        expected_node_patches = [{'path': 'foo', 'op': 'bar'}]
-        expected_port_patch = [{'path': 'foo', 'op': 'baz'}]
-
-        def fake_hook(data, node_info, node_patches, ports_patches):
-            node_patches.extend(expected_node_patches)
-            ports_patches.setdefault(self.macs[1],
-                                     []).extend(expected_port_patch)
-
-        post_hook_mock.side_effect = fake_hook
-
-        self.call()
-
-        self.assertCalledWithPatch(self.patch_props + expected_node_patches,
-                                   self.cli.node.update)
-        self.assertCalledWithPatch(expected_port_patch,
-                                   self.cli.port.update)
-
     def test_set_ipmi_credentials(self, filters_mock, post_hook_mock):
         self.node_info.set_option('new_ipmi_credentials', self.new_creds)
 

releasenotes/notes/googbye-patches-args-071532024b9260bd.yaml

@@ -0,0 +1,4 @@
+---
+upgrade:
+  - Removed deprecated support for passing "node_patches" and "ports_patches"
+    arguments to processing hooks.