vmware: sync one interface per FC port when FC-NVMe is enabled

module/sources/vmware/config.py

@@ -654,8 +654,10 @@ def __init__(self):
                          name, an interface type derived from the adapter model's "<n>G"/"<n>Gb" capability
                          (the first number when several are given) and only falling back to the port speed
                          when the model gives none, the port WWN and the adapter model and driver as
-                         description. Other host bus adapter types (block, PCIe) are ignored, as is an FC
-                         adapter that reports no port WWN. 'host_interface_exclude_filter' applies to the
+                         description. With FC-NVMe enabled ESXi lists each port a second time as an NVMe
+                         adapter, only the SCSI adapter of such a pair becomes an interface. Other host bus
+                         adapter types (block, PCIe) are ignored, as is an FC adapter that reports no port
+                         WWN. 'host_interface_exclude_filter' applies to the
                          adapter names as well. The option is independent of 'skip_host_nics'. Disabled
                          by default.""",
                          default_value=False

module/sources/vmware/connection.py

@@ -935,10 +935,67 @@ def fc_hba_wwn(port_wwn):
         except (TypeError, ValueError):
             return None
 
+        # pyVmomi reads an unset WWN as 0, which is no valid WWN
+        if port_wwn <= 0:
+            return None
+
         hex_wwn = f"{port_wwn:016x}"
 
         return ":".join(hex_wwn[i:i + 2] for i in range(0, 16, 2)).upper()
 
+    @classmethod
+    def fc_hba_ports(cls, host_bus_adapters, host_name=None):
+        """
+        Return the Fibre Channel HBAs of a host, one per port.
+
+        With FC-NVMe enabled ESXi lists every FC port twice: as a SCSI adapter and as an
+        NVMe adapter (storageProtocol "nvme") with the same PCI address and port WWN.
+        Of such a pair only the SCSI adapter is returned. An NVMe adapter without a SCSI
+        counterpart is kept.
+
+        Parameters
+        ----------
+        host_bus_adapters: list
+            the host's config.storageDevice.hostBusAdapter
+        host_name: str
+            name of the host, only used for logging
+
+        Returns
+        -------
+        list: Fibre Channel HBAs in the order vCenter reports them
+        """
+
+        ports = dict()
+        for hba in host_bus_adapters or list():
+
+            if not isinstance(hba, vim.host.FibreChannelHba):
+                continue
+
+            port_wwn = cls.fc_hba_wwn(grab(hba, "portWorldWideName"))
+
+            # adapters without a WWN are passed on as they are, the caller skips them
+            if port_wwn is None:
+                ports[id(hba)] = hba
+                continue
+
+            # both adapters of an FC-NVMe port share the PCI address and the port WWN
+            port = (port_wwn, grab(hba, "pci"))
+
+            current = ports.get(port)
+            if current is None:
+                ports[port] = hba
+                continue
+
+            if grab(current, "storageProtocol") == "nvme" and grab(hba, "storageProtocol") != "nvme":
+                ports[port], dropped = hba, current
+            else:
+                dropped = hba
+
+            log.debug2(f"FC HBA '{grab(dropped, 'device')}' of host '{host_name}' lists the same port "
+                       f"as '{grab(ports[port], 'device')}' (WWN {port_wwn}). Skipping")
+
+        return list(ports.values())
+
     def get_site_name(self, object_type, object_name, cluster_name=""):
         """
         Return a site name for a NBCluster or NBDevice depending on config options
@@ -3001,11 +3058,8 @@ def add_host(self, obj):
         # collect Fibre Channel host bus adapters as interfaces if enabled
         fc_hba_data_dict = dict()
         if self.settings.sync_host_fc_adapters is True:
-            for hba in grab(obj, "config.storageDevice.hostBusAdapter", fallback=list()):
-
-                # only Fibre Channel adapters are synced, block and PCIe adapters are ignored
-                if not isinstance(hba, vim.host.FibreChannelHba):
-                    continue
+            # only Fibre Channel adapters are synced, block and PCIe adapters are ignored
+            for hba in self.fc_hba_ports(grab(obj, "config.storageDevice.hostBusAdapter", fallback=list()), name):
 
                 hba_device = get_string_or_none(grab(hba, "device"))
                 if hba_device is None:

settings-example.ini

@@ -597,10 +597,12 @@ password = super-secret
 ; For each FC HBA an interface is created with the adapter's device name, an interface
 ; type derived from the adapter model's "<n>G"/"<n>Gb" capability (the first number when
 ; several are given) and only falling back to the port speed when the model gives none,
-; the port WWN and the adapter model and driver as description. Other host bus adapter
-; types (block, PCIe) are ignored, as is an FC adapter that reports no port WWN.
-; 'host_interface_exclude_filter' applies to the adapter names as well. The option is
-; independent of 'skip_host_nics'. Disabled by default.
+; the port WWN and the adapter model and driver as description. With FC-NVMe enabled ESXi
+; lists each port a second time as an NVMe adapter, only the SCSI adapter of such a pair
+; becomes an interface. Other host bus adapter types (block, PCIe) are ignored, as is an
+; FC adapter that reports no port WWN. 'host_interface_exclude_filter' applies to the
+; adapter names as well. The option is independent of 'skip_host_nics'. Disabled by
+; default.
 ;sync_host_fc_adapters = False
 
 [source/my-redfish-example]

tests/test_vmware_host_fc_adapters.py

@@ -19,6 +19,7 @@
 from types import SimpleNamespace
 
 import pytest
+from pyVmomi import vim
 
 from module.netbox.object_classes import NBDevice, NBInterface, NBIPAddress, NBMACAddress
 from module.sources import instantiate_sources
@@ -87,6 +88,84 @@ def test_fc_hba_wwn_none_stays_none():
     assert VMWareHandler.fc_hba_wwn(None) is None
 
 
+def test_fc_hba_wwn_zero_is_no_wwn():
+    # what pyVmomi returns for an unset WWN
+    assert VMWareHandler.fc_hba_wwn(0) is None
+
+
+def _fc_hba(device, wwn=None, storage_protocol=None, pci=None):
+    hba = vim.host.FibreChannelHba(device=device, storageProtocol=storage_protocol, pci=pci)
+    if wwn is not None:
+        hba.portWorldWideName = wwn
+    return hba
+
+
+def test_fc_hba_ports_keeps_the_scsi_adapter_of_an_fc_nvme_port():
+    adapters = [
+        _fc_hba("vmhba3", 2378169741649783014, "scsi", "0000:37:00.0"),
+        _fc_hba("vmhba4", 2378169741649783015, "scsi", "0000:37:00.1"),
+        _fc_hba("vmhba64", 2378169741649783014, "nvme", "0000:37:00.0"),
+        _fc_hba("vmhba65", 2378169741649783015, "nvme", "0000:37:00.1"),
+    ]
+
+    assert [hba.device for hba in VMWareHandler.fc_hba_ports(adapters)] == ["vmhba3", "vmhba4"]
+
+
+def test_fc_hba_ports_prefers_scsi_when_nvme_is_listed_first(caplog):
+    adapters = [
+        _fc_hba("vmhba64", 2378169741649783014, "nvme", "0000:37:00.0"),
+        _fc_hba("vmhba3", 2378169741649783014, "scsi", "0000:37:00.0"),
+    ]
+
+    caplog.set_level(1, logger="NetBox-Sync")
+    assert [hba.device for hba in VMWareHandler.fc_hba_ports(adapters, "esxi01")] == ["vmhba3"]
+    assert any("'vmhba64' of host 'esxi01' lists the same port as 'vmhba3'" in record.getMessage()
+               for record in caplog.records), caplog.records
+
+
+def test_fc_hba_ports_keeps_adapters_sharing_a_wwn_on_different_pci_addresses():
+    # not an FC-NVMe pair, both are synced as before
+    adapters = [
+        _fc_hba("vmhba3", 2378169741649783014, "scsi", "0000:37:00.0"),
+        _fc_hba("vmhba7", 2378169741649783014, "scsi", "0000:38:00.0"),
+    ]
+
+    assert [hba.device for hba in VMWareHandler.fc_hba_ports(adapters)] == ["vmhba3", "vmhba7"]
+
+
+def test_fc_hba_ports_keeps_an_nvme_adapter_without_scsi_counterpart():
+    adapters = [_fc_hba("vmhba64", 2378169741649783014, "nvme", "0000:37:00.0")]
+
+    assert [hba.device for hba in VMWareHandler.fc_hba_ports(adapters)] == ["vmhba64"]
+
+
+def test_fc_hba_ports_without_storage_protocol():
+    # vCenter before 7.0 doesn't report storageProtocol
+    adapters = [
+        _fc_hba("vmhba2", 1152921507993664860),
+        _fc_hba("vmhba3", 1152921507993693731),
+    ]
+
+    assert [hba.device for hba in VMWareHandler.fc_hba_ports(adapters)] == ["vmhba2", "vmhba3"]
+
+
+def test_fc_hba_ports_passes_on_adapters_without_wwn():
+    # pyVmomi reads an unset WWN as 0, the caller skips those adapters
+    adapters = [_fc_hba("vmhba3", storage_protocol="scsi"), _fc_hba("vmhba4", storage_protocol="scsi")]
+
+    assert [hba.portWorldWideName for hba in adapters] == [0, 0]
+    assert [hba.device for hba in VMWareHandler.fc_hba_ports(adapters)] == ["vmhba3", "vmhba4"]
+
+
+def test_fc_hba_ports_ignores_other_adapters():
+    adapters = [
+        vim.host.BlockHba(device="vmhba0"),
+        _fc_hba("vmhba3", 2378169741649783014, "scsi"),
+    ]
+
+    assert [hba.device for hba in VMWareHandler.fc_hba_ports(adapters)] == ["vmhba3"]
+
+
 def _matching_source(inventory):
     """A VMware handler with nothing but the state interface matching needs."""
     source = object.__new__(VMWareHandler)
@@ -152,7 +231,8 @@ def test_fc_hba_matches_existing_fc_interface_by_wwn(inventory):
 
 
 def test_fc_hbas_sharing_a_wwn_are_not_merged(inventory):
-    # two ports of one adapter report the same WWN; each still gets its own interface
+    # if two discovered interfaces report the same WWN, only one of them takes over the
+    # existing interface
     source = _matching_source(inventory)
     device = inventory.add_object(NBDevice, data={"name": "esxi01"}, source=source)
 
@@ -288,14 +368,15 @@ def test_fc_adapters_synced_for_vc001(vcsim, inventory, load_config, vmware_sett
     host = _find_device(inventory, "172.29.50.68")
     fc_interfaces = _fc_interfaces(inventory, host)
 
-    assert len(fc_interfaces) == 8
+    assert len(fc_interfaces) == 4
 
     # the host also has a PCIe NVMe controller and two SATA controllers (vmhba0-2); only the
-    # Fibre Channel adapters become interfaces
+    # Fibre Channel adapters become interfaces. FC-NVMe is enabled, so ESXi lists each port
+    # a second time as vmhba64-67, those must not become interfaces of their own.
     hba_interface_names = {interface.data.get("name") for interface in inventory.get_all_interfaces(host)
                            if str(interface.data.get("name", "")).startswith("vmhba")}
-    assert hba_interface_names == {"vmhba3", "vmhba4", "vmhba5", "vmhba6",
-                                   "vmhba64", "vmhba65", "vmhba66", "vmhba67"}
+    assert hba_interface_names == {"vmhba3", "vmhba4", "vmhba5", "vmhba6"}
+    assert len({interface.data.get("wwn") for interface in fc_interfaces}) == 4
 
     # every port is a QLE2700/QLE2800, whose "32/64G" capability wins over the
     # negotiated link speed (16) and over the offline ports' speed 0