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

@@ -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

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
 

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.

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,13 +121,34 @@ 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):
+        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']}])
 

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
 

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):

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``.