From b31888a4ee4548f0f852e0cf752c77b5b10e23d7 Mon Sep 17 00:00:00 2001
From: Kaifeng Wang <kaifeng.w@gmail.com>
Date: Sat, 20 Jun 2020 20:42:11 +0800
Subject: [PATCH] Fix incorrect pxe-enabled was set during introspection

Morden bare metals usually support PXEs from all NICs, in
our current logic only the port matching mac address in the
BOOTIF will be set to pxe-enabled. This does not work for UEFI
nor user friendly to bonding.

This patch adds a configuration option [processing]update_pxe_enabled
to control whether this field should be updated according to
introspection data, defaults to False to keep backwards compatibility.

Change-Id: I6f3b00180f62dc6f500ac2cdb5d8f8cc7c7190cf
---
 ironic_inspector/conf/processing.py           |  5 +++
 ironic_inspector/plugins/standard.py          |  3 +-
 ironic_inspector/test/functional.py           | 43 +++++++++++++++++--
 .../pxe-enabled-for-pxe-a199e81128557bc0.yaml |  6 +++
 4 files changed, 53 insertions(+), 4 deletions(-)
 create mode 100644 releasenotes/notes/pxe-enabled-for-pxe-a199e81128557bc0.yaml

diff --git a/ironic_inspector/conf/processing.py b/ironic_inspector/conf/processing.py
index db86bb3fe..01f5555c3 100644
--- a/ironic_inspector/conf/processing.py
+++ b/ironic_inspector/conf/processing.py
@@ -103,6 +103,11 @@ _OPTS = [
                 default=False,
                 help=_('Whether to process nodes that are in running '
                        'states.')),
+    cfg.BoolOpt('update_pxe_enabled',
+                default=True,
+                help=_('Whether to update the pxe_enabled value according '
+                       'to the introspection data. This option has no effect '
+                       'if [processing]overwrite_existing is set to False')),
 ]
 
 
diff --git a/ironic_inspector/plugins/standard.py b/ironic_inspector/plugins/standard.py
index e0342dbc9..a0f4143cd 100644
--- a/ironic_inspector/plugins/standard.py
+++ b/ironic_inspector/plugins/standard.py
@@ -294,7 +294,8 @@ class ValidateInterfacesHook(base.ProcessingHook):
                              node_info=node_info, data=introspection_data)
                     node_info.delete_port(port)
 
-        if CONF.processing.overwrite_existing:
+        if (CONF.processing.overwrite_existing
+                and CONF.processing.update_pxe_enabled):
             # Make sure is_pxe_enabled is up-to-date
             ports = node_info.ports()
             for iface in introspection_data['interfaces'].values():
diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py
index 4d38af9ce..60531a183 100644
--- a/ironic_inspector/test/functional.py
+++ b/ironic_inspector/test/functional.py
@@ -310,9 +310,46 @@ class Test(Base):
         ]
         self.cli.create_port.assert_has_calls(calls, any_order=True)
         self.cli.delete_port.assert_called_once_with(uuid_to_delete)
-        self.cli.patch_port.assert_called_once_with(
-            uuid_to_update,
-            [{'op': 'replace', 'path': '/pxe_enabled', 'value': False}])
+
+        status = self.call_get_status(self.uuid)
+        self.check_status(status, finished=True, state=istate.States.finished)
+
+    def test_port_not_update_pxe_enabled(self):
+        cfg.CONF.set_override('add_ports', 'active', 'processing')
+        cfg.CONF.set_override('keep_ports', 'added', 'processing')
+        cfg.CONF.set_override('update_pxe_enabled', False, 'processing')
+
+        uuid_to_update = uuidutils.generate_uuid()
+        # One port with incorrect pxe_enabled.
+        self.cli.ports.return_value = [
+            mock.Mock(address=self.macs[0], id=uuid_to_update,
+                      node_id=self.uuid, extra={}, is_pxe_enabled=False)
+        ]
+        # Two more ports are created, one with client_id. Make sure the
+        # returned object has the same properties as requested in create().
+        self.cli.create_port.side_effect = mock.Mock
+
+        self.call_introspect(self.uuid)
+        eventlet.greenthread.sleep(DEFAULT_SLEEP)
+        self.cli.set_node_power_state.assert_called_once_with(self.uuid,
+                                                              'rebooting')
+
+        status = self.call_get_status(self.uuid)
+        self.check_status(status, finished=False, state=istate.States.waiting)
+
+        res = self.call_continue(self.data)
+        self.assertEqual({'uuid': self.uuid}, res)
+        eventlet.greenthread.sleep(DEFAULT_SLEEP)
+
+        self.cli.patch_node.assert_called_with(self.uuid, mock.ANY)
+        self.assertCalledWithPatch(self.patch, self.cli.patch_node)
+        calls = [
+            mock.call(node_uuid=self.uuid, address=self.macs[2],
+                      extra={'client-id': self.client_id},
+                      is_pxe_enabled=False),
+        ]
+        self.assertFalse(self.cli.patch_port.called)
+        self.cli.create_port.assert_has_calls(calls, any_order=True)
 
         status = self.call_get_status(self.uuid)
         self.check_status(status, finished=True, state=istate.States.finished)
diff --git a/releasenotes/notes/pxe-enabled-for-pxe-a199e81128557bc0.yaml b/releasenotes/notes/pxe-enabled-for-pxe-a199e81128557bc0.yaml
new file mode 100644
index 000000000..807fe13bd
--- /dev/null
+++ b/releasenotes/notes/pxe-enabled-for-pxe-a199e81128557bc0.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Adds a configuration option ``[processing]update_pxe_enabled`` to control
+    whether the pxe_enabled should be updated according to introspection data
+    for ports. The default value is True which is backwards compatible.
\ No newline at end of file