feat(understack_workflows): refactor LIF creation to be more dynamic

The broadcast_domain_name could not be hard coded. What we really want
is to create home ports and the LIFs bound to those home ports based on
the node and the ports that the data traffic is routing over. This takes
those two items as input and then makes the rest dynamically looked up
from there while returning detailed exceptions when things are missing
or incorrect.

Author: cardoe

python/understack-workflows/tests/test_netapp_client.py

@@ -403,6 +403,28 @@ def test_get_or_create_port_failure(self, mock_port_class, netapp_client):
 
         assert exc_info.value.__cause__ is not None
 
+    @patch("understack_workflows.netapp.client.Port")
+    def test_get_broadcast_domain_name_success(self, mock_port_class, netapp_client):
+        """Test successful broadcast domain lookup."""
+        port = MagicMock()
+        port.broadcast_domain.name = "Fabric-A"
+        mock_port_class.get_collection.return_value = [port]
+
+        result = netapp_client.get_broadcast_domain_name("node-01", "e4a")
+
+        assert result == "Fabric-A"
+
+    @patch("understack_workflows.netapp.client.Port")
+    def test_get_broadcast_domain_name_not_found(self, mock_port_class, netapp_client):
+        """Test broadcast domain lookup when no matching port exists."""
+        mock_port_class.get_collection.return_value = []
+
+        with pytest.raises(NetworkOperationError) as exc_info:
+            netapp_client.get_broadcast_domain_name("node-01", "e4a")
+
+        assert exc_info.value.context["node_name"] == "node-01"
+        assert exc_info.value.context["port_name"] == "e4a"
+
     @patch("understack_workflows.netapp.client.Node")
     def test_get_nodes_success(self, mock_node_class, netapp_client):
         """Test successful node retrieval."""
@@ -566,6 +588,7 @@ def test_interface_methods_are_abstract(self):
             "find_volume",
             "get_or_create_ip_interface",
             "get_or_create_port",
+            "get_broadcast_domain_name",
             "get_aggregates",
             "get_nodes",
             "get_namespaces",

python/understack-workflows/tests/test_netapp_lif_service.py

@@ -17,6 +17,8 @@
 from understack_workflows.netapp.value_objects import PortSpec
 from understack_workflows.netapp.value_objects import SvmResult
 
+DYNAMIC_BROADCAST_DOMAIN = "test-domain"
+
 
 class TestLifService:
     """Test cases for LifService class."""
@@ -56,6 +58,7 @@ def test_create_lif_success(self, lif_service, mock_client, sample_config):
             uuid="port-uuid-123", name="e4a-100", node_name="node-01", port_type="vlan"
         )
         mock_client.get_or_create_port.return_value = mock_port
+        mock_client.get_broadcast_domain_name.return_value = DYNAMIC_BROADCAST_DOMAIN
 
         # Mock interface creation
         mock_interface = InterfaceResult(
@@ -71,6 +74,7 @@ def test_create_lif_success(self, lif_service, mock_client, sample_config):
         # Mock node identification
         mock_node = NodeResult(name="node-01", uuid="node-uuid-1")
         mock_client.get_nodes.return_value = [mock_node]
+        mock_client.get_broadcast_domain_name.return_value = DYNAMIC_BROADCAST_DOMAIN
 
         lif_service.create_lif(project_id, sample_config)
 
@@ -91,6 +95,7 @@ def test_create_lif_success(self, lif_service, mock_client, sample_config):
         assert interface_call_args.name == sample_config.name
         assert interface_call_args.svm_name == expected_svm_name
         assert interface_call_args.home_port_uuid == mock_port.uuid
+        mock_client.get_broadcast_domain_name.assert_called_once_with("node-01", "e4a")
 
     def test_create_lif_svm_not_found(self, lif_service, mock_client, sample_config):
         """Test LIF creation when SVM is not found."""
@@ -125,6 +130,7 @@ def test_create_lif_port_creation_error(
         # Mock node identification
         mock_node = NodeResult(name="node-01", uuid="node-uuid-1")
         mock_client.get_nodes.return_value = [mock_node]
+        mock_client.get_broadcast_domain_name.return_value = DYNAMIC_BROADCAST_DOMAIN
 
         # Mock port creation failure
         mock_client.get_or_create_port.side_effect = Exception("Port creation failed")
@@ -139,15 +145,17 @@ def test_create_home_port_success(self, lif_service, mock_client, sample_config)
         """Test successful home port creation."""
         # Mock node identification
         mock_node = NodeResult(name="node-01", uuid="node-uuid-1")
-        mock_client.get_nodes.return_value = [mock_node]
 
         # Mock port creation
         mock_port = PortResult(
             uuid="port-uuid-123", name="e4a-100", node_name="node-01", port_type="vlan"
         )
         mock_client.get_or_create_port.return_value = mock_port
+        mock_client.get_broadcast_domain_name.return_value = DYNAMIC_BROADCAST_DOMAIN
 
-        result = lif_service.create_home_port(sample_config)
+        result = lif_service.create_home_port(
+            sample_config, mock_node, DYNAMIC_BROADCAST_DOMAIN
+        )
 
         assert result == mock_port
 
@@ -158,21 +166,7 @@ def test_create_home_port_success(self, lif_service, mock_client, sample_config)
         assert call_args.node_name == "node-01"
         assert call_args.vlan_id == 100
         assert call_args.base_port_name == sample_config.base_port_name
-        assert call_args.broadcast_domain_name == sample_config.broadcast_domain_name
-
-    def test_create_home_port_no_node(self, lif_service, mock_client, sample_config):
-        """Test home port creation when no suitable node is found."""
-        # Mock no matching nodes
-        mock_client.get_nodes.return_value = [
-            NodeResult(name="node-03", uuid="node-uuid-3"),
-            NodeResult(name="node-04", uuid="node-uuid-4"),
-        ]
-
-        with pytest.raises(HomeNodeNotFoundError, match="Could not find home node"):
-            lif_service.create_home_port(sample_config)
-
-        # Verify no port creation was attempted
-        mock_client.get_or_create_port.assert_not_called()
+        assert call_args.broadcast_domain_name == DYNAMIC_BROADCAST_DOMAIN
 
     def test_identify_home_node_success(self, lif_service, mock_client, sample_config):
         """Test successful node identification."""
@@ -222,19 +216,17 @@ def test_identify_home_node_not_found(
         ]
         mock_client.get_nodes.return_value = mock_nodes
 
-        result = lif_service.identify_home_node(sample_config)
-
-        assert result is None
+        with pytest.raises(HomeNodeNotFoundError, match="Could not find home node"):
+            lif_service.identify_home_node(sample_config)
 
     def test_identify_home_node_exception(
         self, lif_service, mock_client, sample_config
     ):
         """Test node identification when client raises an exception."""
         mock_client.get_nodes.side_effect = Exception("NetApp error")
 
-        result = lif_service.identify_home_node(sample_config)
-
-        assert result is None
+        with pytest.raises(HomeNodeNotFoundError, match="Could not find home node"):
+            lif_service.identify_home_node(sample_config)
 
     def test_svm_name_generation(self, lif_service):
         """Test SVM name generation follows naming convention."""
@@ -260,6 +252,7 @@ def test_interface_spec_creation(self, lif_service, mock_client, sample_config):
             uuid="port-uuid-123", name="e4a-100", node_name="node-01", port_type="vlan"
         )
         mock_client.get_or_create_port.return_value = mock_port
+        mock_client.get_broadcast_domain_name.return_value = DYNAMIC_BROADCAST_DOMAIN
 
         # Mock interface creation
         mock_client.get_or_create_ip_interface.return_value = InterfaceResult(
@@ -273,6 +266,7 @@ def test_interface_spec_creation(self, lif_service, mock_client, sample_config):
         # Mock node identification
         mock_node = NodeResult(name="node-01", uuid="node-uuid-1")
         mock_client.get_nodes.return_value = [mock_node]
+        mock_client.get_broadcast_domain_name.return_value = DYNAMIC_BROADCAST_DOMAIN
 
         lif_service.create_lif(project_id, sample_config)
 
@@ -283,33 +277,27 @@ def test_interface_spec_creation(self, lif_service, mock_client, sample_config):
         assert interface_call_args.netmask == str(sample_config.network.netmask)
         assert interface_call_args.svm_name == expected_svm_name
         assert interface_call_args.home_port_uuid == mock_port.uuid
-        assert (
-            interface_call_args.broadcast_domain_name
-            == sample_config.broadcast_domain_name
-        )
+        assert interface_call_args.broadcast_domain_name == DYNAMIC_BROADCAST_DOMAIN
         assert interface_call_args.service_policy == "default-data-nvme-tcp"
+        mock_client.get_broadcast_domain_name.assert_called_once_with("node-01", "e4a")
 
     def test_port_spec_creation(self, lif_service, mock_client, sample_config):
         """Test that port specification is created correctly."""
         # Mock node identification
         mock_node = NodeResult(name="node-01", uuid="node-uuid-1")
-        mock_client.get_nodes.return_value = [mock_node]
 
         # Mock port creation
         mock_client.get_or_create_port.return_value = PortResult(
             uuid="port-uuid-123", name="e4a-100", node_name="node-01", port_type="vlan"
         )
-
-        lif_service.create_home_port(sample_config)
+        lif_service.create_home_port(sample_config, mock_node, DYNAMIC_BROADCAST_DOMAIN)
 
         # Verify the port spec is created correctly
         port_call_args = mock_client.get_or_create_port.call_args[0][0]
         assert port_call_args.node_name == "node-01"
         assert port_call_args.vlan_id == sample_config.vlan_id
         assert port_call_args.base_port_name == sample_config.base_port_name
-        assert (
-            port_call_args.broadcast_domain_name == sample_config.broadcast_domain_name
-        )
+        assert port_call_args.broadcast_domain_name == DYNAMIC_BROADCAST_DOMAIN
 
     def test_node_number_extraction_logic(self, lif_service, mock_client):
         """Test the node number extraction logic with various node names."""

python/understack-workflows/tests/test_netapp_manager.py

@@ -12,6 +12,7 @@
 from understack_workflows.netapp.manager import NetAppManager
 from understack_workflows.netapp.value_objects import AggregateResult
 from understack_workflows.netapp.value_objects import NetappIPInterfaceConfig
+from understack_workflows.netapp.value_objects import NodeResult
 
 
 class TestNetAppManagerOrchestration:
@@ -392,7 +393,12 @@ def test_public_api_contract_maintained(
         manager._volume_service.get_mapped_namespaces = MagicMock(return_value=[])
         manager._lif_service.create_lif = MagicMock()
         manager._lif_service.create_home_port = MagicMock()
-        manager._lif_service.identify_home_node = MagicMock()
+        manager._lif_service.identify_home_node = MagicMock(
+            return_value=NodeResult(name="node-01", uuid="node-uuid-1")
+        )
+        manager._client.get_broadcast_domain_name = MagicMock(
+            return_value="test-domain"
+        )
 
         # Test all public methods can be called with expected signatures
         try:

python/understack-workflows/tests/test_netapp_manager_integration.py

@@ -10,6 +10,7 @@
 from understack_workflows.netapp.exceptions import SvmOperationError
 from understack_workflows.netapp.exceptions import VolumeOperationError
 from understack_workflows.netapp.manager import NetAppManager
+from understack_workflows.netapp.value_objects import NodeResult
 
 
 class TestNetAppManagerIntegration:
@@ -445,6 +446,12 @@ def test_backward_compatibility_maintained(
                 network=ipaddress.IPv4Network("192.168.1.0/24"),
                 vlan_id=100,
             )
+            manager._lif_service.identify_home_node = MagicMock(
+                return_value=NodeResult(name="node-03", uuid="node-uuid-3")
+            )
+            manager._client.get_broadcast_domain_name = MagicMock(
+                return_value="test-domain"
+            )
             manager.create_lif("project", config_obj)
             manager.create_home_port(config_obj)
             manager.identify_home_node(config_obj)

python/understack-workflows/understack_workflows/netapp/client.py

@@ -155,6 +155,10 @@ def get_or_create_port(self, port_spec: PortSpec) -> PortResult:
             NetworkOperationError: If port creation or load fails
         """
 
+    @abstractmethod
+    def get_broadcast_domain_name(self, node_name: str, port_name: str) -> str:
+        """Get the broadcast domain name for a port."""
+
     @abstractmethod
     def get_nodes(self) -> list[NodeResult]:
         """Get all nodes in the cluster.
@@ -599,6 +603,36 @@ def get_or_create_port(self, port_spec: PortSpec) -> PortResult:
                 },
             ) from e
 
+    def get_broadcast_domain_name(self, node_name: str, port_name: str) -> str:
+        """Get the broadcast domain name for a port."""
+        try:
+            ports = Port.get_collection(
+                name=port_name,
+                fields="node.name,name,broadcast_domain",
+                **{"node.name": node_name},  # pyright: ignore[reportArgumentType]
+            )
+
+            port = cast(Port, next(iter(ports)))
+            return str(port.broadcast_domain.name)
+
+        except NetAppRestError as e:
+            raise NetworkOperationError(
+                f"NetApp broadcast domain lookup failed: {e}",
+                context={
+                    "node_name": node_name,
+                    "port_name": port_name,
+                    "netapp_error": str(e),
+                },
+            ) from e
+        except StopIteration:
+            raise NetworkOperationError(
+                "No broadcast domain found for the requested port",
+                context={
+                    "node_name": node_name,
+                    "port_name": port_name,
+                },
+            ) from None
+
     def get_nodes(self) -> list[NodeResult]:
         """Get all nodes in the cluster."""
         try: