From 78dbcf603a623de16b3614187cd2c6687d2b9225 Mon Sep 17 00:00:00 2001
From: Serena Ziviani <serena.ziviani@cern.ch>
Date: Mon, 2 Dec 2019 22:59:00 +0100
Subject: [PATCH] Do not update/delete ports upon inspection of active nodes

Short-circuit the port update logic for active nodes to allow only
for the addition of new ports.

Story: #2006956
Task: #37645

Change-Id: Ifd870a8465b6643578299dc86e2eae023db6ae43
---
 ironic_inspector/plugins/standard.py              | 10 ++++++++++
 .../test/unit/test_plugins_standard.py            | 15 +++++++++++++++
 ...te_for_manual_inspection-5dc296df9d409c69.yaml |  8 ++++++++
 3 files changed, 33 insertions(+)
 create mode 100644 releasenotes/notes/short_circuit_port_update_for_manual_inspection-5dc296df9d409c69.yaml

diff --git a/ironic_inspector/plugins/standard.py b/ironic_inspector/plugins/standard.py
index db498374f..a8600a495 100644
--- a/ironic_inspector/plugins/standard.py
+++ b/ironic_inspector/plugins/standard.py
@@ -20,7 +20,9 @@ from oslo_utils import netutils
 from oslo_utils import units
 import six
 
+
 from ironic_inspector.common.i18n import _
+from ironic_inspector.common import ironic as ir_utils
 from ironic_inspector.plugins import base
 from ironic_inspector import utils
 
@@ -274,6 +276,14 @@ class ValidateInterfacesHook(base.ProcessingHook):
         elif CONF.processing.keep_ports == 'added':
             expected_macs = set(introspection_data['macs'])
 
+        node_provision_state = node_info.node().provision_state
+        if node_provision_state in ir_utils.VALID_ACTIVE_STATES:
+            LOG.debug('Node %(node)s is %(state)s; '
+                      'not modifying / deleting its ports',
+                      {'node': node_info.uuid,
+                       'state': node_provision_state})
+            return
+
         if CONF.processing.keep_ports != 'all':
             # list is required as we modify underlying dict
             for port in list(node_info.ports().values()):
diff --git a/ironic_inspector/test/unit/test_plugins_standard.py b/ironic_inspector/test/unit/test_plugins_standard.py
index f23b4b09d..cc5725171 100644
--- a/ironic_inspector/test/unit/test_plugins_standard.py
+++ b/ironic_inspector/test/unit/test_plugins_standard.py
@@ -322,6 +322,15 @@ class TestValidateInterfacesHookBeforeUpdateDeletion(test_base.NodeTest):
         mock_delete_port.assert_any_call(self.node_info,
                                          self.existing_ports[1])
 
+    def test_active_do_not_delete(self, mock_create_ports, mock_delete_port):
+        CONF.set_override('permit_active_introspection', True, 'processing')
+        CONF.set_override('keep_ports', 'present', 'processing')
+        self.data['all_interfaces'] = self.all_interfaces
+        self.node_info.node().provision_state = 'active'
+        self.hook.before_update(self.data, self.node_info)
+        mock_create_ports.assert_called_once_with(self.node_info, mock.ANY)
+        self.assertFalse(mock_delete_port.called)
+
 
 @mock.patch.object(node_cache.NodeInfo, 'patch_port', autospec=True)
 @mock.patch.object(node_cache.NodeInfo, 'create_ports', autospec=True)
@@ -351,6 +360,12 @@ class TestValidateInterfacesHookBeforeUpdatePXEEnabled(test_base.NodeTest):
             self.node_info, self.existing_ports[1],
             [{'op': 'replace', 'path': '/pxe_enabled', 'value': False}])
 
+    def test_active_do_not_modify(self, mock_create_ports, mock_patch_port):
+        CONF.set_override('permit_active_introspection', True, 'processing')
+        self.node_info.node().provision_state = 'active'
+        self.hook.before_update(self.data, self.node_info)
+        self.assertFalse(mock_patch_port.called)
+
     def test_no_overwrite(self, mock_create_ports, mock_patch_port):
         CONF.set_override('overwrite_existing', False, 'processing')
         self.hook.before_update(self.data, self.node_info)
diff --git a/releasenotes/notes/short_circuit_port_update_for_manual_inspection-5dc296df9d409c69.yaml b/releasenotes/notes/short_circuit_port_update_for_manual_inspection-5dc296df9d409c69.yaml
new file mode 100644
index 000000000..0493a9205
--- /dev/null
+++ b/releasenotes/notes/short_circuit_port_update_for_manual_inspection-5dc296df9d409c69.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixes an issue happening during manual inspection of
+    active nodes where the code attempts to delete or update
+    ports, while the only modification allowed for active
+    nodes is updating the MAC address if the node is in
+    maintenance.