Merge pull request #2434 from rackerlabs/enable_storage_nic

fix(ironic): enable Ironic’s storage-port discovery/classification

Author: cardoe

components/ironic/cm-trait-based-networking.yaml

@@ -4,9 +4,9 @@ metadata:
   name: ironic-trait-based-networking
 data:
   trait_based_networking.yaml: |
-    # Trait Based Networking (TBN) config for grouping category=network ports
-    # into a dynamic portgroup, and only ever auto-attaching category=network
-    # ports via vif_attach (category=storage ports are left alone).
+    # Trait Based Networking (TBN) config. category=network ports auto-attach via
+    # the CUSTOM_NETWORK_PORTGROUP / default handlers. category=storage ports
+    # attach only when a VIF requests the CUSTOM_STORAGE_ATTACH trait.
     #
     # Applied to a node only when CUSTOM_NETWORK_PORTGROUP appears in the
     # node's instance_info.traits (Nova copies matching flavor/image traits
@@ -31,6 +31,16 @@ data:
           filter: port.category == 'network'
           max_count: 1
 
+    # Attaches a single category=storage port for a storage VIF. Single-port
+    # only for now (A/B grouping is deferred). Require it on a flavor with:
+    #   openstack flavor set --property trait:CUSTOM_STORAGE_ATTACH=required <flavor>
+    CUSTOM_STORAGE_ATTACH:
+      order: 1
+      actions:
+        - action: attach_port
+          filter: port.category == 'storage'
+          max_count: 1
+
     # Overrides Ironic's built-in fallback trait (CUSTOM_DEFAULT_TBN_TRAIT),
     # which normally attaches the first available port or portgroup regardless
     # of category. Redefining it here keeps that fallback scoped to

python/ironic-understack/ironic_understack/hooks/inspect_hook_update_baremetal_ports.py

@@ -14,6 +14,10 @@
 
 LOG = logging.getLogger(__name__)
 
+# Physnet suffixes realized through ML2 + Undersync. Other derived physnets
+# (e.g. "-storage-appliance", "enrol") are nulled so they don't reach binding.
+REALIZED_PHYSNET_SUFFIXES = ("-network", "-storage")
+
 
 class InspectHookUpdateBaremetalPorts(base.InspectionHook):
     """Hook to update ports according to LLDP data."""
@@ -161,7 +165,9 @@ def _set_port_attributes(
             )
             port.local_link_connection = inspected_port.local_link_connection
 
-        if physical_network and not physical_network.endswith("-network"):
+        if physical_network and not physical_network.endswith(
+            REALIZED_PHYSNET_SUFFIXES
+        ):
             physical_network = None
 
         if port.physical_network != physical_network:

python/ironic-understack/ironic_understack/tests/test_inspect_hook_update_baremetal_ports.py

@@ -145,12 +145,60 @@ def test_with_valid_storage_port(mocker, caplog):
         "switch_id": "88:5a:92:ec:54:59",
         "switch_info": "f20-3-1f.iad3.rackspace.net",
     }
-    assert mock_port.physical_network is None
+    # Storage-client physnets are preserved (previously nulled).
+    assert mock_port.physical_network == "f20-3-storage"
     assert mock_port.category == "storage"
     assert mock_port.pxe_enabled is False
     mock_port.save.assert_called()
 
 
+def test_non_realized_physnet_is_nulled(mocker, caplog):
+    """A "-storage-appliance" physnet is still nulled; only its category survives."""
+    caplog.set_level(logging.DEBUG)
+
+    node_uuid = uuidutils.generate_uuid()
+    mock_traits = mocker.Mock()
+    mock_context = mocker.Mock()
+    mock_node = mocker.Mock(id=1234, traits=mock_traits)
+    mock_task = mocker.Mock(node=mock_node, context=mock_context)
+    mock_port = mocker.Mock(
+        uuid=uuidutils.generate_uuid(),
+        node_id=node_uuid,
+        address="33:33:33:33:33:33",
+        local_link_connection={},
+        physical_network="original_value",
+        category=None,
+        pxe_enabled=True,
+    )
+
+    # Derive "f20-3-storage-appliance" from the connected suffixes.
+    appliance_mapping = {
+        "1": "network",
+        "2": "network",
+        "1f": "storage-appliance",
+        "2f": "storage-appliance",
+    }
+
+    mocker.patch(
+        "ironic_understack.hooks.inspect_hook_update_baremetal_ports.ironic_ports_for_node",
+        return_value=[mock_port],
+    )
+    mocker.patch(
+        "ironic_understack.hooks.inspect_hook_update_baremetal_ports.CONF.ironic_understack.switch_name_vlan_group_mapping",
+        appliance_mapping,
+    )
+    mocker.patch(
+        "ironic_understack.hooks.inspect_hook_update_baremetal_ports.objects.TraitList.create"
+    )
+    mock_traits.get_trait_names.return_value = ["CUSTOM_BMC_SWITCH", "bar"]
+
+    InspectHookUpdateBaremetalPorts().__call__(mock_task, _INVENTORY, _PLUGIN_DATA)
+
+    assert mock_port.physical_network is None
+    assert mock_port.category == "appliance"
+    mock_port.save.assert_called()
+
+
 def test_secondary_network_port_has_pxe_disabled(mocker, caplog):
     caplog.set_level(logging.DEBUG)