From afada321d80c2e053bd8156972f57dbd4b7b3687 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Wed, 15 Mar 2023 13:06:39 +0100
Subject: [PATCH] Very basic in-band inspection with the "agent" interface

Only port creation/updating/deletion logic has been replicated from
ironic-inspector, as well as the add_ports and keep_ports options.

In the future patches, the added code will become a part of processing
hooks.

Change-Id: I69d6a1a53c5bf9e0f41d1a5bce7215edeea54b22
---
 ironic/common/exception.py                    |   4 +
 ironic/conf/inspector.py                      |  14 +
 ironic/drivers/modules/inspector/agent.py     |   4 +-
 ironic/drivers/modules/inspector/nics.py      | 209 ++++++++++++
 .../drivers/modules/inspector/test_agent.py   |   6 +-
 .../drivers/modules/inspector/test_nics.py    | 311 ++++++++++++++++++
 6 files changed, 545 insertions(+), 3 deletions(-)
 create mode 100644 ironic/drivers/modules/inspector/nics.py
 create mode 100644 ironic/tests/unit/drivers/modules/inspector/test_nics.py

diff --git a/ironic/common/exception.py b/ironic/common/exception.py
index 8286ef58a9..3ef7e3bb04 100644
--- a/ironic/common/exception.py
+++ b/ironic/common/exception.py
@@ -883,3 +883,7 @@ class FirmwareComponentAlreadyExists(Conflict):
 
 class FirmwareComponentNotFound(NotFound):
     _msg_fmt = _("Node %(node)s doesn't have Firmware component %(name)s")
+
+
+class InvalidNodeInventory(Invalid):
+    _msg_fmt = _("Inventory for node %(node)s is invalid: %(reason)s")
diff --git a/ironic/conf/inspector.py b/ironic/conf/inspector.py
index ce9ece205b..e987c7a8e9 100644
--- a/ironic/conf/inspector.py
+++ b/ironic/conf/inspector.py
@@ -55,6 +55,20 @@ opts = [
                        'to False if you need to inspect nodes that are not '
                        'supported by boot interfaces (e.g. because they '
                        'don\'t have ports).')),
+    cfg.StrOpt('add_ports',
+               default='pxe',
+               help=_('Which MAC addresses to add as ports during '
+                      'inspection.'),
+               choices=list(VALID_ADD_PORTS_VALUES.items())),
+    cfg.StrOpt('keep_ports',
+               default='all',
+               help=_('Which ports (already present on a node) to keep after '
+                      'inspection.'),
+               choices=list(VALID_KEEP_PORTS_VALUES.items())),
+    cfg.BoolOpt('update_pxe_enabled',
+                default=True,
+                help=_('Whether to update the ports\' pxe_enabled field '
+                       'according to the inspection data.')),
 ]
 
 
diff --git a/ironic/drivers/modules/inspector/agent.py b/ironic/drivers/modules/inspector/agent.py
index f4886972b8..bc5bc08b1a 100644
--- a/ironic/drivers/modules/inspector/agent.py
+++ b/ironic/drivers/modules/inspector/agent.py
@@ -23,6 +23,7 @@ from ironic.conductor import utils as cond_utils
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules import inspect_utils
 from ironic.drivers.modules.inspector import interface as common
+from ironic.drivers.modules.inspector import nics
 
 LOG = logging.getLogger(__name__)
 
@@ -78,6 +79,5 @@ class AgentInspect(common.Common):
         :param plugin_data: optional plugin-specific data.
         """
         # TODO(dtantsur): migrate the whole pipeline from ironic-inspector
-        LOG.error('Meaningful inspection is not implemented yet in the "agent"'
-                  ' inspect interface')
+        nics.process_interfaces(task, inventory, plugin_data)
         common.clean_up(task, finish=False)
diff --git a/ironic/drivers/modules/inspector/nics.py b/ironic/drivers/modules/inspector/nics.py
new file mode 100644
index 0000000000..12dc00dbe5
--- /dev/null
+++ b/ironic/drivers/modules/inspector/nics.py
@@ -0,0 +1,209 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from oslo_log import log as logging
+from oslo_utils import netutils
+
+from ironic.common import exception
+from ironic.common.i18n import _
+from ironic.common import utils
+from ironic.conf import CONF
+from ironic import objects
+
+LOG = logging.getLogger(__name__)
+
+
+def get_pxe_mac(inventory):
+    """Get MAC address of the PXE interface."""
+    pxe_mac = inventory.get('boot', {}).get('pxe_interface')
+    if pxe_mac and '-' in pxe_mac:
+        # pxelinux format: 01-aa-bb-cc-dd-ee-ff
+        pxe_mac = pxe_mac.split('-', 1)[1]
+        pxe_mac = pxe_mac.replace('-', ':').lower()
+    return pxe_mac
+
+
+def get_interfaces(node, inventory):
+    """Convert inventory to a dict with interfaces.
+
+    :return: dict interface name -> interface (for valid interfaces).
+    """
+    result = {}
+    pxe_mac = get_pxe_mac(inventory)
+
+    for iface in inventory['interfaces']:
+        name = iface.get('name')
+        mac = iface.get('mac_address')
+        ipv4_address = iface.get('ipv4_address')
+        ipv6_address = iface.get('ipv6_address')
+        # NOTE(kaifeng) ipv6 address may in the form of fd00::1%enp2s0,
+        # which is not supported by netaddr, remove the suffix if exists.
+        if ipv6_address and '%' in ipv6_address:
+            ipv6_address = ipv6_address.split('%')[0]
+        ip = ipv4_address or ipv6_address
+        client_id = iface.get('client_id')
+
+        if not name:
+            LOG.error('Malformed interface record for node %(node)s: %(if)s',
+                      {'node': node.uuid, 'if': iface})
+            continue
+
+        if not mac:
+            LOG.debug('Skipping interface %(if)s for node %(node)s without '
+                      'link information',
+                      {'node': node.uuid, 'if': iface})
+            continue
+
+        if not netutils.is_valid_mac(mac):
+            LOG.warning('MAC address of interface %(if)s of node %(node)s '
+                        'is not valid, skipping',
+                        {'node': node.uuid, 'if': iface})
+            continue
+
+        mac = mac.lower()
+
+        LOG.debug('Found interface %(name)s with MAC "%(mac)s", '
+                  'IP address "%(ip)s" and client_id "%(client_id)s" '
+                  'for node %(node)s',
+                  {'name': name, 'mac': mac, 'ip': ip,
+                   'client_id': client_id, 'node': node.uuid})
+        result[name] = dict(iface, pxe_enabled=(mac == pxe_mac),
+                            # IPv6 address without scope.
+                            ipv6_address=ipv6_address)
+
+    return result
+
+
+def validate_interfaces(node, inventory, interfaces):
+    """Validate interfaces on correctness and suitability.
+
+    :return: dict interface name -> interface.
+    """
+    if not interfaces:
+        raise exception.InvalidNodeInventory(
+            node=node.uuid,
+            reason=_('no valid network interfaces'))
+
+    pxe_mac = get_pxe_mac(inventory)
+    if not pxe_mac and CONF.inspector.add_ports == 'pxe':
+        LOG.warning('No boot interface provided in the inventory for node '
+                    '%s, will add all ports with IP addresses', node.uuid)
+
+    result = {}
+
+    for name, iface in interfaces.items():
+        ip = iface.get('ipv4_address') or iface.get('ipv6_address')
+        pxe = iface.get('pxe_enabled', True)
+
+        if name == 'lo' or (ip and utils.is_loopback(ip)):
+            LOG.debug('Skipping local interface %(iface)s for node %(node)s',
+                      {'iface': name, 'node': node.uuid})
+            continue
+
+        if CONF.inspector.add_ports == 'pxe' and pxe_mac and not pxe:
+            LOG.debug('Skipping interface %(iface)s for node %(node)s as it '
+                      'was not PXE booting and add_ports is set to "pxe"',
+                      {'iface': name, 'node': node.uuid})
+            continue
+
+        if CONF.inspector.add_ports == 'active' and not ip:
+            LOG.debug('Skipping interface %(iface)s for node %(node)s as it '
+                      'did not have an IP address assigned during the ramdisk '
+                      'run and add_ports is set to "active"',
+                      {'iface': name, 'node': node.uuid})
+            continue
+
+        result[name] = iface.copy()
+
+    if not result:
+        raise exception.InvalidNodeInventory(
+            node=node.uuid,
+            reason=_('no network interfaces match the configuration '
+                     '(add_ports set to "%s")') % CONF.inspector.add_ports)
+    return result
+
+
+def add_ports(task, interfaces):
+    """Add ports for all previously validated interfaces."""
+    for iface in interfaces.values():
+        mac = iface['mac_address']
+        extra = {}
+        if iface.get('client_id'):
+            extra['client-id'] = iface['client_id']
+        port_dict = {'address': mac, 'node_id': task.node.id,
+                     'pxe_enabled': iface['pxe_enabled'], 'extra': extra}
+        port = objects.Port(task.context, **port_dict)
+        try:
+            port.create()
+            LOG.info("Port created for MAC address %(address)s for "
+                     "node %(node)s%(pxe)s",
+                     {'address': mac, 'node': task.node.uuid,
+                      'pxe': ' (PXE booting)' if iface['pxe_enabled'] else ''})
+        except exception.MACAlreadyExists:
+            LOG.info("Port already exists for MAC address %(address)s "
+                     "for node %(node)s",
+                     {'address': mac, 'node': task.node.uuid})
+
+
+def update_ports(task, all_interfaces, valid_macs):
+    """Update ports to match the valid MACs.
+
+    Depending on the value of ``[inspector]keep_ports``, some ports may be
+    removed.
+    """
+    # TODO(dtantsur): no port update for active nodes (when supported)
+
+    if CONF.inspector.keep_ports == 'present':
+        expected_macs = {iface['mac_address']
+                         for iface in all_interfaces.values()}
+    elif CONF.inspector.keep_ports == 'added':
+        expected_macs = set(valid_macs)
+    else:
+        expected_macs = None  # unused
+
+    pxe_macs = {iface['mac_address'] for iface in all_interfaces.values()
+                if iface['pxe_enabled']}
+
+    for port in objects.Port.list_by_node_id(task.context, task.node.id):
+        if expected_macs and port.address not in expected_macs:
+            expected_str = ', '.join(sorted(expected_macs))
+            LOG.info("Deleting port %(port)s of node %(node)s as its MAC "
+                     "%(mac)s is not in the expected MAC list [%(expected)s]",
+                     {'port': port.uuid, 'mac': port.address,
+                      'node': task.node.uuid, 'expected': expected_str})
+            port.destroy()
+        elif CONF.inspector.update_pxe_enabled:
+            pxe_enabled = port.address in pxe_macs
+            if pxe_enabled != port.pxe_enabled:
+                LOG.debug('Changing pxe_enabled=%(val)s on port %(port)s '
+                          'of node %(node)s to match the inventory',
+                          {'port': port.address, 'val': pxe_enabled,
+                           'node': task.node.uuid})
+                port.pxe_enabled = pxe_enabled
+                port.save()
+
+
+def process_interfaces(task, inventory, plugin_data):
+    """Process network interfaces in the inventory."""
+    # TODO(dtantsur): this function will become two hooks in the future.
+    all_interfaces = get_interfaces(task.node, inventory)
+    interfaces = validate_interfaces(task.node, inventory, all_interfaces)
+    valid_macs = [iface['mac_address'] for iface in interfaces.values()]
+
+    plugin_data['all_interfaces'] = all_interfaces
+    plugin_data['valid_interfaces'] = interfaces
+    plugin_data['macs'] = valid_macs
+
+    if CONF.inspector.add_ports != 'disabled':
+        add_ports(task, interfaces)
+
+    update_ports(task, all_interfaces, valid_macs)
diff --git a/ironic/tests/unit/drivers/modules/inspector/test_agent.py b/ironic/tests/unit/drivers/modules/inspector/test_agent.py
index fbd08b9013..6eea58e05e 100644
--- a/ironic/tests/unit/drivers/modules/inspector/test_agent.py
+++ b/ironic/tests/unit/drivers/modules/inspector/test_agent.py
@@ -21,6 +21,7 @@ from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules import inspect_utils
 from ironic.drivers.modules.inspector import agent as inspector
 from ironic.drivers.modules.inspector import interface as common
+from ironic.drivers.modules.inspector import nics
 from ironic.tests.unit.db import base as db_base
 from ironic.tests.unit.objects import utils as obj_utils
 
@@ -102,6 +103,7 @@ class InspectHardwareTestCase(db_base.DbTestCase):
 
 
 @mock.patch.object(common, 'tear_down_managed_boot', autospec=True)
+@mock.patch.object(nics, 'process_interfaces', autospec=True)
 class ContinueInspectionTestCase(db_base.DbTestCase):
     def setUp(self):
         super().setUp()
@@ -113,11 +115,13 @@ class ContinueInspectionTestCase(db_base.DbTestCase):
             provision_state=states.INSPECTING)
         self.iface = inspector.AgentInspect()
 
-    def test(self, mock_tear_down):
+    def test(self, mock_process, mock_tear_down):
         mock_tear_down.return_value = None
         with task_manager.acquire(self.context, self.node.id) as task:
             result = self.iface.continue_inspection(
                 task, mock.sentinel.inventory, mock.sentinel.plugin_data)
+            mock_process.assert_called_once_with(
+                task, mock.sentinel.inventory, mock.sentinel.plugin_data)
             mock_tear_down.assert_called_once_with(task)
             self.assertEqual(states.INSPECTING, task.node.provision_state)
 
diff --git a/ironic/tests/unit/drivers/modules/inspector/test_nics.py b/ironic/tests/unit/drivers/modules/inspector/test_nics.py
new file mode 100644
index 0000000000..7a567322dc
--- /dev/null
+++ b/ironic/tests/unit/drivers/modules/inspector/test_nics.py
@@ -0,0 +1,311 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import copy
+from unittest import mock
+
+from oslo_utils import uuidutils
+
+from ironic.common import exception
+from ironic.conductor import task_manager
+from ironic.conf import CONF
+from ironic.drivers.modules.inspector import nics
+from ironic import objects
+from ironic.tests.unit.db import base as db_base
+from ironic.tests.unit.objects import utils as obj_utils
+
+
+_PXE_INTERFACE = 'aa:bb:cc:dd:ee:ff'
+
+
+_INVENTORY = {
+    'boot': {
+        'pxe_interface': _PXE_INTERFACE,
+    },
+    'interfaces': [
+        {
+            'name': 'lo',
+            'mac_address': '00:00:00:00:00:00',
+            'ipv4_address': '127.0.0.1',
+            'ipv6_address': '::1',
+            'has_carrier': True,
+        },
+        {
+            'name': 'em0',
+            'mac_address': _PXE_INTERFACE,
+            'ipv4_address': '192.0.2.1',
+            'ipv6_address': '2001:db8::1',
+            'has_carrier': True,
+        },
+        {
+            'name': 'em1',
+            'mac_address': '11:11:11:11:11:11',
+            'ipv4_address': '192.0.2.2',
+            'ipv6_address': '2001:db8::2',
+            'has_carrier': True,
+        },
+        {
+            'name': 'em2',
+            'mac_address': '22:22:22:22:22:22',
+            'ipv4_address': None,
+            'ipv6_address': '2001:db8::3',
+            'has_carrier': True,
+        },
+        {
+            'name': 'em3',
+            'mac_address': '33:33:33:33:33:33',
+            'ipv4_address': '192.0.2.4',
+            'ipv6_address': '2001:db8::4%em4',
+            'has_carrier': True,
+        },
+        {
+            'name': 'em4',
+            'mac_address': '44:44:44:44:44:44',
+            'ipv4_address': None,
+            'ipv6_address': None,
+            'has_carrier': False,
+        },
+    ],
+}
+
+
+_VALID = {
+    'lo': {
+        'name': 'lo',
+        'mac_address': '00:00:00:00:00:00',
+        'ipv4_address': '127.0.0.1',
+        'ipv6_address': '::1',
+        'has_carrier': True,
+        'pxe_enabled': False,
+    },
+    'em0': {
+        'name': 'em0',
+        'mac_address': _PXE_INTERFACE,
+        'ipv4_address': '192.0.2.1',
+        'ipv6_address': '2001:db8::1',
+        'has_carrier': True,
+        'pxe_enabled': True,
+    },
+    'em1': {
+        'name': 'em1',
+        'mac_address': '11:11:11:11:11:11',
+        'ipv4_address': '192.0.2.2',
+        'ipv6_address': '2001:db8::2',
+        'has_carrier': True,
+        'pxe_enabled': False,
+    },
+    'em2': {
+        'name': 'em2',
+        'mac_address': '22:22:22:22:22:22',
+        'ipv4_address': None,
+        'ipv6_address': '2001:db8::3',
+        'has_carrier': True,
+        'pxe_enabled': False,
+    },
+    'em3': {
+        'name': 'em3',
+        'mac_address': '33:33:33:33:33:33',
+        'ipv4_address': '192.0.2.4',
+        'ipv6_address': '2001:db8::4',  # note: no scope
+        'has_carrier': True,
+        'pxe_enabled': False,
+    },
+    'em4': {
+        'name': 'em4',
+        'mac_address': '44:44:44:44:44:44',
+        'ipv4_address': None,
+        'ipv6_address': None,
+        'has_carrier': False,
+        'pxe_enabled': False,
+    },
+}
+
+
+class GetInterfacesTestCase(db_base.DbTestCase):
+    def setUp(self):
+        super().setUp()
+        self.node = obj_utils.create_test_node(self.context,
+                                               inspect_interface='agent')
+        self.inventory = copy.deepcopy(_INVENTORY)
+        self.inventory['interfaces'].extend([
+            # Broken records
+            {
+                'mac_address': '55:55:55:55:55:55',
+                'ipv4_address': None,
+                'ipv6_address': None,
+                'has_carrier': False,
+            },
+            {
+                'name': 'broken',
+                'mac_address': 'banana!',
+                'ipv4_address': None,
+                'ipv6_address': None,
+                'has_carrier': False,
+            },
+        ])
+
+    def test_get_interfaces(self):
+        result = nics.get_interfaces(self.node, self.inventory)
+        self.assertEqual(_VALID, result)
+
+
+class ValidateInterfacesTestCase(db_base.DbTestCase):
+    def setUp(self):
+        super().setUp()
+        self.node = obj_utils.create_test_node(self.context,
+                                               inspect_interface='agent')
+        self.inventory = copy.deepcopy(_INVENTORY)
+        self.interfaces = copy.deepcopy(_VALID)
+
+    def test_pxe_only(self):
+        expected = {'em0': _VALID['em0']}
+        result = nics.validate_interfaces(
+            self.node, self.inventory, self.interfaces)
+        self.assertEqual(expected, result)
+        # Make sure we don't modify the initial structures
+        self.assertEqual(_INVENTORY, self.inventory)
+        self.assertEqual(_VALID, self.interfaces)
+
+    def test_all_interfaces(self):
+        CONF.set_override('add_ports', 'all', group='inspector')
+        expected = {key: value.copy() for key, value in _VALID.items()
+                    if key != 'lo'}
+        result = nics.validate_interfaces(
+            self.node, self.inventory, self.interfaces)
+        self.assertEqual(expected, result)
+
+    @mock.patch.object(nics.LOG, 'warning', autospec=True)
+    def test_no_pxe_fallback_to_all(self, mock_warn):
+        del self.inventory['boot']
+        expected = {key: value.copy() for key, value in _VALID.items()
+                    if key != 'lo'}
+        result = nics.validate_interfaces(
+            self.node, self.inventory, self.interfaces)
+        self.assertEqual(expected, result)
+        self.assertTrue(mock_warn.called)
+
+    def test_active_interfaces(self):
+        CONF.set_override('add_ports', 'active', group='inspector')
+        expected = {key: value.copy() for key, value in _VALID.items()
+                    if key not in ('lo', 'em4')}
+        result = nics.validate_interfaces(
+            self.node, self.inventory, self.interfaces)
+        self.assertEqual(expected, result)
+
+    def test_nothing_to_add(self):
+        CONF.set_override('add_ports', 'active', group='inspector')
+        self.interfaces = {key: value.copy() for key, value in _VALID.items()
+                           if key in ('lo', 'em4')}
+        self.assertRaises(exception.InvalidNodeInventory,
+                          nics.validate_interfaces,
+                          self.node, self.inventory, self.interfaces)
+
+
+class AddPortsTestCase(db_base.DbTestCase):
+    def setUp(self):
+        super().setUp()
+        CONF.set_override('enabled_inspect_interfaces',
+                          ['agent', 'no-inspect'])
+        self.node = obj_utils.create_test_node(self.context,
+                                               inspect_interface='agent')
+        self.interfaces = {key: value.copy() for key, value in _VALID.items()
+                           if key not in ('lo', 'em4')}
+        self.macs = {
+            _PXE_INTERFACE,
+            '11:11:11:11:11:11',
+            '22:22:22:22:22:22',
+            '33:33:33:33:33:33',
+        }
+
+    def test_add(self):
+        with task_manager.acquire(self.context, self.node.id) as task:
+            nics.add_ports(task, self.interfaces)
+        ports = objects.Port.list_by_node_id(self.context, self.node.id)
+        self.assertEqual(
+            {port.address: port.pxe_enabled for port in ports},
+            {mac: (mac == _PXE_INTERFACE) for mac in self.macs})
+
+    def test_duplicates(self):
+        obj_utils.create_test_port(self.context,
+                                   node_id=self.node.id,
+                                   address=_PXE_INTERFACE,
+                                   pxe_enabled=False)
+        with task_manager.acquire(self.context, self.node.id) as task:
+            nics.add_ports(task, self.interfaces)
+        ports = objects.Port.list_by_node_id(self.context, self.node.id)
+        self.assertEqual(
+            {port.address: port.pxe_enabled for port in ports},
+            # Always False because the PXE port already existed
+            {mac: False for mac in self.macs})
+
+
+class UpdatePortsTestCase(db_base.DbTestCase):
+    def setUp(self):
+        super().setUp()
+        CONF.set_override('enabled_inspect_interfaces',
+                          ['agent', 'no-inspect'])
+        self.node = obj_utils.create_test_node(self.context,
+                                               inspect_interface='agent')
+        self.macs = {
+            _PXE_INTERFACE,
+            '11:11:11:11:11:11',
+            '22:22:22:22:22:22',
+            '33:33:33:33:33:33',
+        }
+        self.present_macs = {i['mac_address'] for i in _VALID.values()}
+        self.extra_mac = '00:11:00:11:00:11'
+        self.all_macs = self.present_macs | {self.extra_mac}
+        for mac in self.all_macs:
+            obj_utils.create_test_port(self.context,
+                                       uuid=uuidutils.generate_uuid(),
+                                       node_id=self.node.id,
+                                       address=mac,
+                                       pxe_enabled=(mac == self.extra_mac))
+
+    def test_keep_all(self):
+        with task_manager.acquire(self.context, self.node.id) as task:
+            nics.update_ports(task, _VALID, self.macs)
+        ports = objects.Port.list_by_node_id(self.context, self.node.id)
+        self.assertEqual(
+            {port.address: port.pxe_enabled for port in ports},
+            # Nothing is removed by default, pxe_enabled updated
+            {mac: (mac == _PXE_INTERFACE) for mac in self.all_macs})
+
+    def test_keep_pxe_enabled(self):
+        CONF.set_override('update_pxe_enabled', False, group='inspector')
+        with task_manager.acquire(self.context, self.node.id) as task:
+            nics.update_ports(task, _VALID, self.macs)
+        ports = objects.Port.list_by_node_id(self.context, self.node.id)
+        self.assertEqual(
+            {port.address: port.pxe_enabled for port in ports},
+            # Nothing is removed by default, pxe_enabled kept intact
+            {mac: (mac == self.extra_mac) for mac in self.all_macs})
+
+    def test_keep_added(self):
+        CONF.set_override('keep_ports', 'added', group='inspector')
+        with task_manager.acquire(self.context, self.node.id) as task:
+            nics.update_ports(task, _VALID, self.macs)
+        ports = objects.Port.list_by_node_id(self.context, self.node.id)
+        self.assertEqual(
+            {port.address: port.pxe_enabled for port in ports},
+            # Extra ports removed, pxe_enabled updated
+            {mac: (mac == _PXE_INTERFACE) for mac in self.macs})
+
+    def test_keep_present(self):
+        CONF.set_override('keep_ports', 'present', group='inspector')
+        with task_manager.acquire(self.context, self.node.id) as task:
+            nics.update_ports(task, _VALID, self.macs)
+        ports = objects.Port.list_by_node_id(self.context, self.node.id)
+        self.assertEqual(
+            {port.address: port.pxe_enabled for port in ports},
+            # Extra port removed, pxe_enabled updated
+            {mac: (mac == _PXE_INTERFACE) for mac in self.present_macs})