From cc422c0a5e1b65a6d24a5ed7bddf4dc81382e8dd Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 9 Feb 2016 12:36:07 +0100 Subject: [PATCH] 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 | 4 ---- ironic_inspector/plugins/base.py | 4 +--- ironic_inspector/plugins/standard.py | 3 +-- ironic_inspector/process.py | 16 ++------------ .../test/test_plugins_standard.py | 12 +++++----- ironic_inspector/test/test_process.py | 22 +------------------ ...googbye-patches-args-071532024b9260bd.yaml | 4 ++++ 7 files changed, 15 insertions(+), 50 deletions(-) create mode 100644 releasenotes/notes/googbye-patches-args-071532024b9260bd.yaml diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 6756a3666..aebaef063 100644 --- a/CONTRIBUTING.rst +++ b/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). diff --git a/ironic_inspector/plugins/base.py b/ironic_inspector/plugins/base.py index caac5c005..6cc252906 100644 --- a/ironic_inspector/plugins/base.py +++ b/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 diff --git a/ironic_inspector/plugins/standard.py b/ironic_inspector/plugins/standard.py index 4de6a517a..7cc61a739 100644 --- a/ironic_inspector/plugins/standard.py +++ b/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: diff --git a/ironic_inspector/process.py b/ironic_inspector/process.py index 1447a0d62..00cf96fdd 100644 --- a/ironic_inspector/process.py +++ b/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): diff --git a/ironic_inspector/test/test_plugins_standard.py b/ironic_inspector/test/test_plugins_standard.py index e18c67de0..2434db854 100644 --- a/ironic_inspector/test/test_plugins_standard.py +++ b/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) diff --git a/ironic_inspector/test/test_process.py b/ironic_inspector/test/test_process.py index 245a25547..aa9cb1986 100644 --- a/ironic_inspector/test/test_process.py +++ b/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) diff --git a/releasenotes/notes/googbye-patches-args-071532024b9260bd.yaml b/releasenotes/notes/googbye-patches-args-071532024b9260bd.yaml new file mode 100644 index 000000000..5e94981e3 --- /dev/null +++ b/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.