diff --git a/python/understack-workflows/tests/test_netapp_client.py b/python/understack-workflows/tests/test_netapp_client.py index 3a047175c..05954ca45 100644 --- a/python/understack-workflows/tests/test_netapp_client.py +++ b/python/understack-workflows/tests/test_netapp_client.py @@ -84,14 +84,17 @@ def test_init_connection_failure( assert exc_info.value.config_path == "/test/config/path" assert exc_info.value.__cause__ is not None + @patch("understack_workflows.netapp.client.Volume") @patch("understack_workflows.netapp.client.Svm") - def test_create_svm_success(self, mock_svm_class, netapp_client): + def test_create_svm_success(self, mock_svm_class, mock_volume_class, netapp_client): """Test successful SVM creation.""" mock_svm_instance = MagicMock() mock_svm_instance.name = "test-svm" mock_svm_instance.uuid = "svm-uuid-123" mock_svm_instance.state = "online" mock_svm_class.return_value = mock_svm_instance + mock_root_volume = MagicMock() + mock_volume_class.get_collection.return_value = [mock_root_volume] svm_spec = SvmSpec(name="test-svm", aggregate_name="test-aggregate") @@ -103,6 +106,18 @@ def test_create_svm_success(self, mock_svm_class, netapp_client): assert result.state == "online" mock_svm_instance.post.assert_called_once() mock_svm_instance.get.assert_called_once() + mock_volume_class.get_collection.assert_called_once_with( + name="test-svm_root", + fields="uuid,name", + **{"svm.name": "test-svm"}, + ) + assert mock_root_volume.size == 1024**3 + assert mock_root_volume.snapshot_policy == {"name": "none"} + assert mock_root_volume.autosize == { + "mode": "grow", + "maximum": 2 * 1024**3, + } + mock_root_volume.patch.assert_called_once() @patch("understack_workflows.netapp.client.Svm") def test_create_svm_failure(self, mock_svm_class, netapp_client): @@ -403,6 +418,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 +603,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", diff --git a/python/understack-workflows/tests/test_netapp_lif_service.py b/python/understack-workflows/tests/test_netapp_lif_service.py index dba5c1112..41670e864 100644 --- a/python/understack-workflows/tests/test_netapp_lif_service.py +++ b/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,9 +216,8 @@ 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 @@ -232,9 +225,8 @@ def test_identify_home_node_exception( """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.""" diff --git a/python/understack-workflows/tests/test_netapp_manager.py b/python/understack-workflows/tests/test_netapp_manager.py index 2d7d6134c..47d030c88 100644 --- a/python/understack-workflows/tests/test_netapp_manager.py +++ b/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: diff --git a/python/understack-workflows/tests/test_netapp_manager_integration.py b/python/understack-workflows/tests/test_netapp_manager_integration.py index 515b7bb81..3c3114200 100644 --- a/python/understack-workflows/tests/test_netapp_manager_integration.py +++ b/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) diff --git a/python/understack-workflows/tests/test_netapp_svm_service.py b/python/understack-workflows/tests/test_netapp_svm_service.py index c46ffbaf1..25d56977d 100644 --- a/python/understack-workflows/tests/test_netapp_svm_service.py +++ b/python/understack-workflows/tests/test_netapp_svm_service.py @@ -56,6 +56,7 @@ def test_create_svm_success(self, svm_service, mock_client): assert call_args.aggregate_name == aggregate_name assert call_args.language == "c.utf_8" assert call_args.allowed_protocols == ["nvme"] + assert call_args.root_volume_name == f"{expected_svm_name}_root" def test_create_svm_already_exists(self, svm_service, mock_client): """Test SVM creation when SVM already exists.""" diff --git a/python/understack-workflows/understack_workflows/netapp/client.py b/python/understack-workflows/understack_workflows/netapp/client.py index f90cdda2f..b64f47f86 100644 --- a/python/understack-workflows/understack_workflows/netapp/client.py +++ b/python/understack-workflows/understack_workflows/netapp/client.py @@ -47,6 +47,9 @@ logger = logging.getLogger(__name__) +SVM_ROOT_VOLUME_SIZE_BYTES = 1024**3 +SVM_ROOT_VOLUME_AUTOSIZE_MAXIMUM_BYTES = 2 * 1024**3 + class NetAppClientInterface(ABC): """Abstract interface for NetApp operations.""" @@ -155,6 +158,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. @@ -261,6 +268,7 @@ def create_svm(self, svm_spec: SvmSpec) -> SvmResult: svm.post() svm.get() # Refresh to get the latest state + self._configure_svm_root_volume(svm_spec) result = SvmResult( name=str(svm.name), @@ -286,6 +294,39 @@ def create_svm(self, svm_spec: SvmSpec) -> SvmResult: }, ) from e + def _configure_svm_root_volume(self, svm_spec: SvmSpec) -> None: + """Apply administrative settings to an SVM root volume.""" + try: + root_volume = cast( + Volume, + next( + iter( + Volume.get_collection( + name=svm_spec.root_volume_name, + fields="uuid,name", + **{"svm.name": svm_spec.name}, # pyright: ignore[reportArgumentType] + ) + ) + ), + ) + except StopIteration as e: + raise SvmOperationError( + "SVM root volume was not found after SVM creation", + svm_name=svm_spec.name, + context={ + "svm_name": svm_spec.name, + "root_volume_name": svm_spec.root_volume_name, + }, + ) from e + + root_volume.size = SVM_ROOT_VOLUME_SIZE_BYTES + root_volume.snapshot_policy = {"name": "none"} + root_volume.autosize = { + "mode": "grow", + "maximum": SVM_ROOT_VOLUME_AUTOSIZE_MAXIMUM_BYTES, + } + root_volume.patch() + def delete_svm(self, svm_name: str) -> bool: """Delete a Storage Virtual Machine (SVM).""" try: @@ -599,6 +640,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: diff --git a/python/understack-workflows/understack_workflows/netapp/lif_service.py b/python/understack-workflows/understack_workflows/netapp/lif_service.py index 03a13d7f1..ffb1a4e56 100644 --- a/python/understack-workflows/understack_workflows/netapp/lif_service.py +++ b/python/understack-workflows/understack_workflows/netapp/lif_service.py @@ -67,8 +67,17 @@ def create_lif(self, project_id: str, config: NetappIPInterfaceConfig) -> None: context={"project_id": project_id, "interface_name": config.name}, ) + home_node = self.identify_home_node(config) + broadcast_domain_name = self._client.get_broadcast_domain_name( + home_node.name, config.base_port_name + ) + # Create the home port first - home_port = self.create_home_port(config) + home_port = self.create_home_port( + config, + home_node, + broadcast_domain_name, + ) # Create interface specification interface_spec = InterfaceSpec( @@ -77,7 +86,7 @@ def create_lif(self, project_id: str, config: NetappIPInterfaceConfig) -> None: netmask=str(config.network.netmask), svm_name=svm_name, home_port_uuid=home_port.uuid, - broadcast_domain_name=config.broadcast_domain_name, + broadcast_domain_name=broadcast_domain_name, service_policy="default-data-nvme-tcp", ) @@ -109,11 +118,18 @@ def create_lif(self, project_id: str, config: NetappIPInterfaceConfig) -> None: }, ) from e - def create_home_port(self, config: NetappIPInterfaceConfig) -> PortResult: # pyright: ignore + def create_home_port( + self, + config: NetappIPInterfaceConfig, + home_node: NodeResult, + broadcast_domain_name: str, + ) -> PortResult: # pyright: ignore """Create a home port for the network interface. Args: config: Network interface configuration + home_node: Pre-resolved node that should host the port + broadcast_domain_name: Pre-resolved broadcast domain name Returns: PortResult: Result of the port creation @@ -129,28 +145,14 @@ def create_home_port(self, config: NetappIPInterfaceConfig) -> PortResult: # py "interface_name": config.name, "vlan_id": config.vlan_id, "base_port": config.base_port_name, - "broadcast_domain": config.broadcast_domain_name, }, ) - # Identify the home node using business logic - home_node = self.identify_home_node(config) - if not home_node: - raise HomeNodeNotFoundError( - f"Could not find home node for interface {config.name}", - interface_name=config.name, - context={ - "desired_node_number": config.desired_node_number, - "vlan_id": config.vlan_id, - }, - ) - - # Create port specification port_spec = PortSpec( node_name=home_node.name, vlan_id=config.vlan_id, base_port_name=config.base_port_name, - broadcast_domain_name=config.broadcast_domain_name, + broadcast_domain_name=broadcast_domain_name, ) # Get or create the port @@ -182,14 +184,14 @@ def create_home_port(self, config: NetappIPInterfaceConfig) -> PortResult: # py }, ) from e - def identify_home_node(self, config: NetappIPInterfaceConfig) -> NodeResult | None: + def identify_home_node(self, config: NetappIPInterfaceConfig) -> NodeResult: """Identify the home node for a network interface using business logic. Args: config: Network interface configuration Returns: - Optional[NodeResult]: The identified home node, or None if not found + NodeResult: The identified home node """ try: logger.debug( @@ -230,15 +232,28 @@ def identify_home_node(self, config: NetappIPInterfaceConfig) -> NodeResult | No }, ) - return None + raise HomeNodeNotFoundError( + f"Could not find home node for interface {config.name}", + interface_name=config.name, + context={ + "desired_node_number": config.desired_node_number, + "vlan_id": config.vlan_id, + "available_nodes": [node.name for node in nodes], + }, + ) + except HomeNodeNotFoundError: + raise except Exception as e: - logger.warning( - "Error identifying home node for interface %(interface_name)s: " - "%(error)s", - {"interface_name": config.name, "error": str(e)}, - ) - return None + raise HomeNodeNotFoundError( + f"Could not find home node for interface {config.name}: {e}", + interface_name=config.name, + context={ + "desired_node_number": config.desired_node_number, + "vlan_id": config.vlan_id, + "original_error": str(e), + }, + ) from e def _get_svm_name(self, project_id: str) -> str: """Generate SVM name using business naming conventions. diff --git a/python/understack-workflows/understack_workflows/netapp/manager.py b/python/understack-workflows/understack_workflows/netapp/manager.py index 2a68d8e57..4d70f3d75 100644 --- a/python/understack-workflows/understack_workflows/netapp/manager.py +++ b/python/understack-workflows/understack_workflows/netapp/manager.py @@ -464,9 +464,15 @@ def create_home_port(self, config: NetappIPInterfaceConfig): Delegates to LifService for port management. """ - return self._lif_service.create_home_port(config) + home_node = self._lif_service.identify_home_node(config) + broadcast_domain_name = self._client.get_broadcast_domain_name( + home_node.name, config.base_port_name + ) + return self._lif_service.create_home_port( + config, home_node, broadcast_domain_name + ) - def identify_home_node(self, config: NetappIPInterfaceConfig) -> NodeResult | None: + def identify_home_node(self, config: NetappIPInterfaceConfig) -> NodeResult: """Identifies the home node for a network interface. Delegates to LifService for node identification. diff --git a/python/understack-workflows/understack_workflows/netapp/value_objects.py b/python/understack-workflows/understack_workflows/netapp/value_objects.py index c467bb513..b80b6cb39 100644 --- a/python/understack-workflows/understack_workflows/netapp/value_objects.py +++ b/python/understack-workflows/understack_workflows/netapp/value_objects.py @@ -162,7 +162,6 @@ class NetappIPInterfaceConfig(BaseModel): side: Extracts side (A or B) from interface name desired_node_number: Extracts node number (1 or 2) from interface name base_port_name: Constructs base port name using NIC slot prefix and side - broadcast_domain_name: Constructs broadcast domain name using side route_nexthop: Calculates next hop IP address from network Example: @@ -254,16 +253,6 @@ def base_port_name(self) -> str: """ return f"{self.nic_slot_prefix}{self.side.lower()}" - @computed_field - @property - def broadcast_domain_name(self) -> str: - """Get the broadcast domain name based on the side. - - Returns: - str: Broadcast domain name (e.g., "Fabric-A", "Fabric-B") - """ - return f"Fabric-{self.side}" - @computed_field @property def route_nexthop(self) -> IPv4Address: