From ada6b106c523984d37713dff4779c4eb090d3c1f Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 6 Mar 2019 13:20:32 +0100 Subject: [PATCH] Pass reset_interfaces when updating a driver from the rules Otherwise updating a driver requires listing all interfaces with their new values. This patch is designed to be backportable, so it has a fall-back to the previous behavior if the required API version and/or ironicclient versions are not available. Story: #2005148 Task: #29854 Depends-On: https://review.openstack.org/#/c/643264/ Change-Id: I115b92b9d7bcdbb1c03f0d259dbba6efd2baec2a --- devstack/plugin.sh | 1 + ironic_inspector/common/ironic.py | 3 +- ironic_inspector/node_cache.py | 5 +-- ironic_inspector/plugins/rules.py | 31 +++++++++++++++++-- ironic_inspector/test/unit/test_node_cache.py | 10 ++++++ .../test/unit/test_plugins_rules.py | 31 +++++++++++++++++++ .../reset-interfaces-ff78d50b9f05d47d.yaml | 10 ++++++ 7 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/reset-interfaces-ff78d50b9f05d47d.yaml diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 936dc426a..ff54c109c 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -447,6 +447,7 @@ elif [[ "$1" == "stack" && "$2" == "test-config" ]]; then if [ -n "$IRONIC_INSPECTOR_NODE_NOT_FOUND_HOOK" ]; then iniset $TEMPEST_CONFIG baremetal_introspection auto_discovery_feature True iniset $TEMPEST_CONFIG baremetal_introspection auto_discovery_default_driver fake-hardware + iniset $TEMPEST_CONFIG baremetal_introspection auto_discovery_target_driver ipmi fi fi fi diff --git a/ironic_inspector/common/ironic.py b/ironic_inspector/common/ironic.py index 26ce965b8..621542f0d 100644 --- a/ironic_inspector/common/ironic.py +++ b/ironic_inspector/common/ironic.py @@ -31,11 +31,12 @@ VALID_STATES = {'enroll', 'manageable', 'inspecting', 'inspect wait', 'inspect failed'} # 1.38 is the latest API version in the Queens release series, 10.1.0. +# 1.46 is the latest API version in the Rocky release series, 11.1.0. # NOTE(mgoddard): This should be updated with each release to ensure that # inspector is able to use the latest ironic API. In particular, this version # is used when processing introspection rules, and is the default version used # by processing plugins. -DEFAULT_IRONIC_API_VERSION = '1.38' +DEFAULT_IRONIC_API_VERSION = ['1.38', '1.46'] IRONIC_SESSION = None diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 0cf7932d6..07126d989 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -409,13 +409,14 @@ class NodeInfo(object): else: self._ports[mac] = port - def patch(self, patches, ironic=None): + def patch(self, patches, ironic=None, **kwargs): """Apply JSON patches to a node. Refreshes cached node instance. :param patches: JSON patches to apply :param ironic: Ironic client to use instead of self.ironic + :param kwargs: Arguments to pass to ironicclient. :raises: ironicclient exceptions """ ironic = ironic or self.ironic @@ -426,7 +427,7 @@ class NodeInfo(object): patch['path'] = '/' + patch['path'] LOG.debug('Updating node with patches %s', patches, node_info=self) - self._node = ironic.node.update(self.uuid, patches) + self._node = ironic.node.update(self.uuid, patches, **kwargs) def patch_port(self, port, patches, ironic=None): """Apply JSON patches to a port. diff --git a/ironic_inspector/plugins/rules.py b/ironic_inspector/plugins/rules.py index 02a6b18ae..f9f270b30 100644 --- a/ironic_inspector/plugins/rules.py +++ b/ironic_inspector/plugins/rules.py @@ -16,6 +16,7 @@ import operator import re +from ironicclient import exceptions as ironic_exc import netaddr from ironic_inspector.common.i18n import _ @@ -23,6 +24,9 @@ from ironic_inspector.plugins import base from ironic_inspector import utils +LOG = utils.getProcessingLogger(__name__) + + def coerce(value, expected): if isinstance(expected, float): return float(value) @@ -117,15 +121,36 @@ class FailAction(base.RuleActionPlugin): class SetAttributeAction(base.RuleActionPlugin): # NOTE(iurygregory): set as optional to accept None as value, check # that the key 'value' is present, otherwise will raise ValueError. - OPTIONAL_PARAMS = {'value'} + OPTIONAL_PARAMS = {'value', 'reset_interfaces'} REQUIRED_PARAMS = {'path'} # TODO(dtantsur): proper validation of path FORMATTED_PARAMS = ['value'] def apply(self, node_info, params, **kwargs): - node_info.patch([{'op': 'add', 'path': params['path'], - 'value': params['value']}]) + kwargs = {} + if (params['path'].strip('/') == 'driver' + and ('reset_interfaces' not in params + or params['reset_interfaces'])): + # Using at least Rocky + kwargs['reset_interfaces'] = True + + try: + node_info.patch([{'op': 'add', 'path': params['path'], + 'value': params['value']}], **kwargs) + except (TypeError, ironic_exc.NotAcceptable): + # TODO(dtantsur): remove support for old ironicclient and Queens + if 'reset_interfaces' in params: + # An explicit request, report an error. + raise utils.Error( + _('Cannot pass reset_interfaces to set-attribute, ' + 'requires API 1.46 and ironicclient >= 2.5.0')) + + LOG.warning('Not passing reset_interfaces to Ironic, since ' + ' API 1.46 and/or ironicclient >= 2.5.0 are ' + 'not available', node_info=node_info) + node_info.patch([{'op': 'add', 'path': params['path'], + 'value': params['value']}]) def validate(self, params, **kwargs): if 'value' in params: diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index 477bf5145..7f45eacca 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/ironic_inspector/test/unit/test_node_cache.py @@ -666,6 +666,16 @@ class TestUpdate(test_base.NodeTest): self.ironic.node.update.assert_called_once_with(self.uuid, patch) self.assertIs(mock.sentinel.node, self.node_info.node()) + def test_patch_with_args(self): + self.ironic.node.update.return_value = mock.sentinel.node + + self.node_info.patch([{'patch': 'patch'}], reset_interfaces=True) + + self.ironic.node.update.assert_called_once_with(self.uuid, + [{'patch': 'patch'}], + reset_interfaces=True) + self.assertIs(mock.sentinel.node, self.node_info.node()) + def test_update_properties(self): self.ironic.node.update.return_value = mock.sentinel.node diff --git a/ironic_inspector/test/unit/test_plugins_rules.py b/ironic_inspector/test/unit/test_plugins_rules.py index c21911659..3124a7854 100644 --- a/ironic_inspector/test/unit/test_plugins_rules.py +++ b/ironic_inspector/test/unit/test_plugins_rules.py @@ -168,6 +168,37 @@ class TestSetAttributeAction(test_base.NodeTest): 'path': '/extra/value', 'value': 42}]) + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_apply_driver(self, mock_patch): + params = {'path': '/driver', 'value': 'ipmi'} + self.act.apply(self.node_info, params) + mock_patch.assert_called_once_with([{'op': 'add', + 'path': '/driver', + 'value': 'ipmi'}], + reset_interfaces=True) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_apply_driver_no_reset_interfaces(self, mock_patch): + params = {'path': '/driver', 'value': 'ipmi', + 'reset_interfaces': False} + self.act.apply(self.node_info, params) + mock_patch.assert_called_once_with([{'op': 'add', + 'path': '/driver', + 'value': 'ipmi'}]) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_apply_driver_not_supported(self, mock_patch): + for exc in (TypeError, exceptions.NotAcceptable): + mock_patch.reset_mock() + mock_patch.side_effect = [exc, None] + params = {'path': '/driver', 'value': 'ipmi'} + self.act.apply(self.node_info, params) + mock_patch.assert_has_calls([ + mock.call([{'op': 'add', 'path': '/driver', 'value': 'ipmi'}], + reset_interfaces=True), + mock.call([{'op': 'add', 'path': '/driver', 'value': 'ipmi'}]) + ]) + @mock.patch('ironic_inspector.common.ironic.get_client', new=mock.Mock()) class TestSetCapabilityAction(test_base.NodeTest): diff --git a/releasenotes/notes/reset-interfaces-ff78d50b9f05d47d.yaml b/releasenotes/notes/reset-interfaces-ff78d50b9f05d47d.yaml new file mode 100644 index 000000000..e19952cda --- /dev/null +++ b/releasenotes/notes/reset-interfaces-ff78d50b9f05d47d.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + The ``set-attribute`` action now automatically sets ``reset_interfaces`` to + ``True`` if the driver is updated. If it's not desired, set it explicitly + to ``False``. +fixes: + - | + Fixes updating a driver with the ``set-attribute`` introspection rule + action by providing ``reset_interfaces``.