From 7a89f75edd6c2a912f107330dd2077c9dbbba186 Mon Sep 17 00:00:00 2001 From: wmousa Date: Fri, 16 Oct 2026 17:48:06 +0200 Subject: [PATCH 1/3] fix(port-block): fence only through the SPDK RPC, drop the FirewallClient fallback port_block.set_port sent fences to the legacy firewall webapp (FirewallClient, mgmt_ip:firewall_port) whenever node.spdk_version was empty, and redirected a method-not-found from nvmf_port_block there too. 25.10.x node records never carried spdk_version, and it is only set by a restart that passes --spdk-proxy-image, so a node upgraded to 26.3 kept "" and every fence on it went to a port nothing listens on: the SPDK image no longer ships the webapp or iptables (ultra #218). The fence failed with connection refused. - port_block: set_port / get_blocked_ports_set / is_port_blocked use the SPDK RPCs only; an RPC error, method-not-found included, propagates. - health_controller.check_ports_on_node: drop the per-port legacy branch. - Delete fw_api_client.py; port_block was its only user. - Tests: the ftt2 and dual-FT fixtures patch port_block instead of FirewallClient; the expansion simulator answers nvmf_port_block / nvmf_port_unblock / nvmf_get_blocked_ports instead of stubbing FirewallClient; regression tests in test_port_block_rpc_only.py. Co-Authored-By: Claude Opus 5.5 --- simplyblock_core/AGENTS.md | 2 +- .../controllers/health_controller.py | 7 +- simplyblock_core/fw_api_client.py | 100 ------------------ simplyblock_core/utils/port_block.py | 90 +++------------- tests/integration/expansion_sim/_rpc_sim.py | 82 +++----------- tests/integration/expansion_sim/conftest.py | 11 -- tests/integration/ftt2/conftest.py | 7 +- tests/integration/test_dual_ft_e2e.py | 7 +- tests/unit/test_port_block_leak_k8s_0911.py | 10 +- tests/unit/test_port_block_rpc_only.py | 66 ++++++++++++ 10 files changed, 110 insertions(+), 272 deletions(-) delete mode 100644 simplyblock_core/fw_api_client.py create mode 100644 tests/unit/test_port_block_rpc_only.py diff --git a/simplyblock_core/AGENTS.md b/simplyblock_core/AGENTS.md index 697e921ebe..e4438ee96d 100644 --- a/simplyblock_core/AGENTS.md +++ b/simplyblock_core/AGENTS.md @@ -155,7 +155,7 @@ When adding a new `SecretStr` field to an existing model, grep for `json.dumps`, ## Client Pattern (RPC, SNode, Firewall API) -Clients in `rpc_client.py`, `snode_client.py`, and `fw_api_client.py` accept `SecretStr` parameters and follow the **log-then-unwrap** pattern: +Clients in `rpc_client.py` and `snode_client.py` accept `SecretStr` parameters and follow the **log-then-unwrap** pattern: 1. Log the payload dict containing `SecretStr` wrappers (masked by Pydantic's `__repr__`). 2. Call `unwrap_secrets_for_send(payload)` from `utils/secrets.py` to produce a plaintext dict. diff --git a/simplyblock_core/controllers/health_controller.py b/simplyblock_core/controllers/health_controller.py index ba293119c4..4c4bc6528a 100644 --- a/simplyblock_core/controllers/health_controller.py +++ b/simplyblock_core/controllers/health_controller.py @@ -258,16 +258,13 @@ def check_port_on_node(snode, port_id): def check_ports_on_node(snode, port_ids): """Batch variant of :func:`check_port_on_node`: ONE ``nvmf_get_blocked_ports`` fetch answers every port in ``port_ids``. - Returns ``{port: bool}`` (True = open). Falls back to per-port checks on - legacy nodes without the RPC. Raises on fetch failure — callers treat it - like the single-port failure path.""" + Returns ``{port: bool}`` (True = open). Raises on fetch failure — callers + treat it like the single-port failure path.""" from simplyblock_core.utils import port_block port_ids = list(port_ids) if not port_ids: return {} blocked = port_block.get_blocked_ports_set(snode) - if blocked is None: - return {p: check_port_on_node(snode, p) for p in port_ids} return {p: int(p) not in blocked for p in port_ids} diff --git a/simplyblock_core/fw_api_client.py b/simplyblock_core/fw_api_client.py deleted file mode 100644 index 010d321eb8..0000000000 --- a/simplyblock_core/fw_api_client.py +++ /dev/null @@ -1,100 +0,0 @@ -import json -import logging - -import requests -from requests.adapters import HTTPAdapter -from urllib3 import Retry - -from simplyblock_core.settings import Settings -from simplyblock_core.utils.secrets import unwrap_secrets_for_send - -logger = logging.getLogger() - - -class FirewallClientException(Exception): - def __init__(self, message): - self.message = message - - -class FirewallClient: - - def __init__(self, node, timeout=300, retry=5): - self.node = node - self.ip_address = f"{node.mgmt_ip}:{node.firewall_port}" - self.url = 'http://%s/' % self.ip_address - self.timeout = timeout - self.session = requests.session() - self.session.verify = False - self.session.headers['Content-Type'] = "application/json" - retries = Retry(total=retry, backoff_factor=1, connect=retry, read=retry) - self.session.mount("http://", HTTPAdapter(max_retries=retries)) - - def _request(self, method, path, payload=None): - try: - logger.debug("Requesting path: %s, params: %s", path, payload) - wire_payload = unwrap_secrets_for_send(payload) if payload else None - data = None - params = None - if wire_payload: - if method == "GET": - params = wire_payload - else: - data = json.dumps(wire_payload) - - response = self.session.request(method, self.url+path, data=data, - timeout=self.timeout, params=params) - except Exception as e: - raise FirewallClientException(str(e)) - - if Settings().log_response_bodies: - logger.debug("Response: status_code: %s, content: %s", - response.status_code, response.content) - else: - logger.debug("Response: status_code: %s, content-length: %s", - response.status_code, len(response.content)) - ret_code = response.status_code - - result = None - error = None - if ret_code == 200: - try: - decoded_data = response.json() - except Exception: - return response.content, None - - result = decoded_data.get('results') - error = decoded_data.get('error') - if result is not None or error is not None: - return result, error - else: - return data, None - - if ret_code in [500, 400]: - raise FirewallClientException("Invalid http status: %s" % ret_code) - - if ret_code == 422: - raise FirewallClientException("Request validation failed (status 422)") - - logger.error("Unknown http status: %s", ret_code) - return None, None - - def firewall_set_port(self, port_id, port_type="tcp", action="block", rpc_port=None, is_reject=False): - params = { - "port_id": port_id, - "port_type": "tcp", - "action": action, - "rpc_port": rpc_port, - "is_reject": is_reject, - } - response = self._request("POST", "firewall", params) - - if response and self.node.active_rdma: - if action == "block": - response = self.node.rpc_client().nvmf_port_block_rdma(port_id) - else: - response = self.node.rpc_client().nvmf_port_unblock_rdma(port_id) - return response - - def get_firewall(self, rpc_port=None): - params = {"rpc_port": rpc_port} - return self._request("GET", "firewall", params) diff --git a/simplyblock_core/utils/port_block.py b/simplyblock_core/utils/port_block.py index 9b7e944427..8f24340d00 100644 --- a/simplyblock_core/utils/port_block.py +++ b/simplyblock_core/utils/port_block.py @@ -1,18 +1,12 @@ -"""Port block helpers with RPC-then-iptables fallback. - -Tries the SPDK ``nvmf_port_block`` / ``nvmf_port_unblock`` / -``nvmf_get_blocked_ports`` RPCs first; on JSON-RPC method-not-found -(SPDK build without the new RPCs — in-flight upgrades) falls back to -the legacy iptables-based FirewallClient. +"""Port block helpers over the SPDK ``nvmf_port_block`` / +``nvmf_port_unblock`` / ``nvmf_get_blocked_ports`` RPCs. + +Fencing is done inside SPDK only. There is no iptables fallback: the SPDK +image no longer ships iptables or the firewall webapp (ultra #218), and +25.10.x -> 26.3 upgrades are offline, so no node runs an SPDK build without +these RPCs. A method-not-found error therefore propagates like any other +RPC failure instead of being redirected to a port nothing listens on. """ -import logging - -import jc - -from simplyblock_core.fw_api_client import FirewallClient -from simplyblock_core.rpc_client import RPCErrorCode, RPCRemoteError - -logger = logging.getLogger(__name__) def set_port(node, port, block, is_reject=False, timeout=0.5, retry=1): @@ -32,53 +26,22 @@ def set_port(node, port, block, is_reject=False, timeout=0.5, retry=1): healthy survivor fenced, the client lost its last path at 4.1s, and fio took EIO. A healthy peer answers this RPC in ~11ms, so 0.5s is ~45x the observed latency; two attempts bound the whole call near 1s. - If spdk_version is R26.2-PRE-latest or empty (came from upgrade) then - use FirewallClient(iptables). Default is to use SPDK RPC. - - Tries SPDK ``nvmf_port_block`` / ``nvmf_port_unblock`` first; on - method-not-found falls back to ``FirewallClient.firewall_set_port`` - (iptables). Any other error from the RPC propagates as-is. """ - if node.spdk_version == "" or node.spdk_version == "R26.2-PRE-latest": - fw = FirewallClient(node, timeout=timeout, retry=retry) - action = "block" if block else "allow" - return fw.firewall_set_port(port, "tcp", action, node.rpc_port, is_reject=is_reject) - rpc = node.rpc_client(timeout=timeout, retry=retry) - try: - if block: - return rpc.nvmf_port_block(port, is_reject=is_reject) - return rpc.nvmf_port_unblock(port) - except RPCRemoteError as exc: - if exc.code != RPCErrorCode.method_not_found: - raise - logger.info( - "nvmf_port_%s RPC not available on %s; falling back to iptables", - "block" if block else "unblock", node.get_id()) - - fw = FirewallClient(node, timeout=timeout, retry=retry) - action = "block" if block else "allow" - return fw.firewall_set_port( - port, "tcp", action, node.rpc_port, is_reject=is_reject) + if block: + return rpc.nvmf_port_block(port, is_reject=is_reject) + return rpc.nvmf_port_unblock(port) def get_blocked_ports_set(node, timeout=5, retry=5): - """One ``nvmf_get_blocked_ports`` fetch -> set of blocked port numbers, - or ``None`` when the node's SPDK lacks the method (legacy iptables path — - caller falls back to per-port :func:`is_port_blocked`). + """One ``nvmf_get_blocked_ports`` fetch -> set of blocked port numbers. The health/monitor loops used to call ``is_port_blocked`` per port, re-fetching the identical full list each time — measured 528 of these RPCs per minute cluster-wide at idle (2026-07-21 baseline audit). One fetch per node per cycle answers every port. """ - rpc = node.rpc_client(timeout=timeout, retry=retry) - try: - blocked = rpc.nvmf_get_blocked_ports() - except RPCRemoteError as exc: - if exc.code != RPCErrorCode.method_not_found: - raise - return None + blocked = node.rpc_client(timeout=timeout, retry=retry).nvmf_get_blocked_ports() if not blocked: return set() entries = blocked.get("blocked_ports", []) if isinstance(blocked, dict) else [] @@ -88,36 +51,15 @@ def get_blocked_ports_set(node, timeout=5, retry=5): def is_port_blocked(node, port_id, timeout=5, retry=5): """Return True if ``port_id`` is currently blocked on ``node``. - Delegates the parse to :func:`get_blocked_ports_set`; on method-not-found - (legacy SPDK) falls back to parsing iptables output via - ``FirewallClient``. + Delegates the parse to :func:`get_blocked_ports_set`. This USED to do ``port_id in rpc.nvmf_get_blocked_ports()``. That RPC returns ``{"blocked_ports": [{"port": N, ...}, ...]}``, so the ``in`` tested the dict's KEYS and the function returned False for every port, always -- a blocked port read as open. Only the batched ``get_blocked_ports_set`` parsed the payload, which is why the monitor's - batched path saw the fences and this one never did. It is the fallback - for nodes whose SPDK lacks the batch RPC and for + batched path saw the fences and this one never did. It backs ``check_port_on_node``, so the miss was silent rather than harmless. """ - blocked = get_blocked_ports_set(node, timeout=timeout, retry=retry) - if blocked is None: - return _is_port_blocked_iptables(node, port_id, timeout, retry) - return int(port_id) in blocked - + return int(port_id) in get_blocked_ports_set(node, timeout=timeout, retry=retry) -def _is_port_blocked_iptables(node, port_id, timeout, retry): - """Legacy iptables-based check via FirewallClient + jc parsing.""" - fw = FirewallClient(node, timeout=timeout, retry=retry) - iptables_output, error = fw.get_firewall(node.rpc_port) - if isinstance(iptables_output, str): - iptables_output = [iptables_output] - for rules in iptables_output: - result = jc.parse('iptables', rules) - for chain in result: - if chain['chain'] in ("INPUT", "OUTPUT"): # type: ignore - for rule in chain['rules']: # type: ignore - if str(port_id) in rule['options'] and rule['target'] == 'DROP': # type: ignore - return True - return False diff --git a/tests/integration/expansion_sim/_rpc_sim.py b/tests/integration/expansion_sim/_rpc_sim.py index 53a23c6e2c..5776497a08 100644 --- a/tests/integration/expansion_sim/_rpc_sim.py +++ b/tests/integration/expansion_sim/_rpc_sim.py @@ -273,6 +273,20 @@ def nvmf_subsystem_add_listener(self, nqn, trtype, ip, port, ana_state=None, **_): return self.listeners_create(nqn, trtype, ip, port, ana_state=ana_state) + # -- Port fencing (nvmf_port_block) ------------------------------------ + + def nvmf_port_block(self, port, is_reject=False, **_): + self.firewall[(int(port), "tcp")] = True + return True + + def nvmf_port_unblock(self, port, **_): + self.firewall[(int(port), "tcp")] = False + return True + + def nvmf_get_blocked_ports(self, **_): + return {"blocked_ports": [{"port": port} for (port, _proto), blocked + in self.firewall.items() if blocked]} + # -- NVMe controller (cross-node) --------------------------------------- def bdev_nvme_attach_controller(self, name, nqn, traddr, trsvcid, @@ -474,71 +488,3 @@ def install_rpc_router(modules: list[str]) -> list[tuple[Any, str, Any]]: def restore_rpc_router(saved: list[tuple[Any, str, Any]]) -> None: for mod, attr, value in saved: setattr(mod, attr, value) - - -# --------------------------------------------------------------------------- -# Firewall client stub -# --------------------------------------------------------------------------- - - -class FirewallClientSim: - """In-process replacement for - :class:`simplyblock_core.fw_api_client.FirewallClient`. - - The real client makes HTTP requests to a per-storage-node firewall API - on port 5001. In simulation we route the same intent — set/unset a - port block — into the target :class:`RpcServerSim`'s ``firewall`` - dict, so cross-node validation logic (phase 2) can read it back. - - Constructed with ``(snode, timeout=, retry=)`` to match the production - signature. - """ - - _active_cluster: ClusterSim | None = None - - @classmethod - def set_active_cluster(cls, sim: ClusterSim | None) -> None: - cls._active_cluster = sim - - def __init__(self, snode, timeout=None, retry=None): - self.snode = snode - - def _server(self) -> RpcServerSim | None: - active = type(self)._active_cluster - if active is None: - return None - return active.get_by_endpoint(self.snode.mgmt_ip, self.snode.rpc_port) - - def firewall_set_port(self, port_id, port_type="tcp", - action="block", rpc_port=None, - is_reject=False, **_): - srv = self._server() - if srv is None: - return True # silently noop in tests not using cluster sim - key = (int(port_id), port_type) - srv.firewall[key] = (action == "block") - srv.call_log.append(("firewall_set_port", - {"port": port_id, "proto": port_type, - "action": action})) - return True - - def firewall_get_status(self, *_, **__): - return {"status": "ok"} - - -def install_firewall_stub(modules: list[str]) -> list[tuple[Any, str, Any]]: - """Replace ``FirewallClient`` with :class:`FirewallClientSim` in every - named module. Symmetric with :func:`install_rpc_router`.""" - import importlib - saved = [] - for mod_name in modules: - mod = importlib.import_module(mod_name) - if hasattr(mod, "FirewallClient"): - saved.append((mod, "FirewallClient", mod.FirewallClient)) - mod.FirewallClient = FirewallClientSim - return saved - - -def restore_firewall_stub(saved: list[tuple[Any, str, Any]]) -> None: - for mod, attr, value in saved: - setattr(mod, attr, value) diff --git a/tests/integration/expansion_sim/conftest.py b/tests/integration/expansion_sim/conftest.py index a30616197f..a8ee458ad0 100644 --- a/tests/integration/expansion_sim/conftest.py +++ b/tests/integration/expansion_sim/conftest.py @@ -139,17 +139,13 @@ def patched_rpc_router(): :class:`ClusterSim` the test should populate.""" from tests.integration.expansion_sim._rpc_sim import ( ClusterSim, - FirewallClientSim, RpcRouter, - install_firewall_stub, install_rpc_router, - restore_firewall_stub, restore_rpc_router, ) cluster_sim = ClusterSim() RpcRouter.set_active_cluster(cluster_sim) - FirewallClientSim.set_active_cluster(cluster_sim) # Modules that do `from simplyblock_core.rpc_client import RPCClient`. # Enumerated by `grep -l RPCClient simplyblock_core/**/*.py`. Excludes @@ -169,17 +165,10 @@ def patched_rpc_router(): "simplyblock_core.controllers.backup.device", ] saved_rpc = install_rpc_router(target_modules) - saved_fw = install_firewall_stub([ - "simplyblock_core.storage_node_ops", - "simplyblock_core.controllers.lvol_controller", - "simplyblock_core.controllers.health_controller", - ]) try: yield cluster_sim finally: - restore_firewall_stub(saved_fw) restore_rpc_router(saved_rpc) - FirewallClientSim.set_active_cluster(None) RpcRouter.set_active_cluster(None) diff --git a/tests/integration/ftt2/conftest.py b/tests/integration/ftt2/conftest.py index 032ee10f46..62550ef83c 100644 --- a/tests/integration/ftt2/conftest.py +++ b/tests/integration/ftt2/conftest.py @@ -553,10 +553,9 @@ def patch_externals(): return_value=(True, None)), patch('simplyblock_core.controllers.health_controller._check_node_api', return_value=True), - patch('simplyblock_core.fw_api_client.FirewallClient.firewall_set_port', - return_value=(True, None)), - patch('simplyblock_core.fw_api_client.FirewallClient.get_firewall', - return_value=('', None)), + patch('simplyblock_core.utils.port_block.set_port', return_value=True), + patch('simplyblock_core.utils.port_block.get_blocked_ports_set', + return_value=set()), patch('simplyblock_core.controllers.health_controller.check_port_on_node', return_value=True), patch('simplyblock_core.utils.get_next_port', return_value=9090), diff --git a/tests/integration/test_dual_ft_e2e.py b/tests/integration/test_dual_ft_e2e.py index 55f5f663ac..8e577b6fe2 100644 --- a/tests/integration/test_dual_ft_e2e.py +++ b/tests/integration/test_dual_ft_e2e.py @@ -815,10 +815,9 @@ def _patch_externals(): return_value=(True, None)), patch('simplyblock_core.controllers.health_controller._check_node_api', return_value=True), # Firewall always succeeds - patch('simplyblock_core.fw_api_client.FirewallClient.firewall_set_port', - return_value=(True, None)), - patch('simplyblock_core.fw_api_client.FirewallClient.get_firewall', - return_value=('', None)), + patch('simplyblock_core.utils.port_block.set_port', return_value=True), + patch('simplyblock_core.utils.port_block.get_blocked_ports_set', + return_value=set()), # Port check always passes patch('simplyblock_core.controllers.health_controller.check_port_on_node', return_value=True), diff --git a/tests/unit/test_port_block_leak_k8s_0911.py b/tests/unit/test_port_block_leak_k8s_0911.py index 4bb7945875..56424c711e 100644 --- a/tests/unit/test_port_block_leak_k8s_0911.py +++ b/tests/unit/test_port_block_leak_k8s_0911.py @@ -75,16 +75,16 @@ def test_agrees_with_the_batched_reader(self): self.assertEqual(port_block.is_port_blocked(node, port), port in batched, port) - def test_falls_back_to_iptables_when_rpc_absent(self): + def test_rpc_absent_raises_instead_of_reading_iptables(self): + # There is no iptables fallback any more: the firewall API it went + # to does not exist on 26.3 hosts. See test_port_block_rpc_only. from simplyblock_core.rpc_client import RPCErrorCode, RPCRemoteError node = MagicMock() node.rpc_client.return_value.nvmf_get_blocked_ports.side_effect = ( RPCRemoteError("no such method", RPCErrorCode.method_not_found)) - with patch.object(port_block, "_is_port_blocked_iptables", - return_value=True) as legacy: - self.assertTrue(port_block.is_port_blocked(node, 4442)) - legacy.assert_called_once() + with self.assertRaises(RPCRemoteError): + port_block.is_port_blocked(node, 4442) # -------------------------------------------------------------------------- diff --git a/tests/unit/test_port_block_rpc_only.py b/tests/unit/test_port_block_rpc_only.py new file mode 100644 index 0000000000..a6a2274c0f --- /dev/null +++ b/tests/unit/test_port_block_rpc_only.py @@ -0,0 +1,66 @@ +"""Port fencing goes through the SPDK RPCs only, never the firewall API. + +``port_block.set_port`` used to send fences to the legacy firewall webapp +(``FirewallClient``, ``mgmt_ip:firewall_port``) whenever ``node.spdk_version`` +was empty. 25.10.x records never carried that field, so every node upgraded +to 26.3 took that path -- and a 26.3 host has nothing listening on the +firewall port (the SPDK image no longer ships the webapp or iptables), so +every fence on an upgraded node failed with connection refused. A +method-not-found from the RPC was redirected to the same dead port. +""" +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from simplyblock_core.controllers import health_controller +from simplyblock_core.rpc_client import RPCErrorCode, RPCRemoteError +from simplyblock_core.utils import port_block + + +def _node(rpc, spdk_version=""): + return SimpleNamespace(spdk_version=spdk_version, rpc_client=lambda **_: rpc, + get_id=lambda: "node-1") + + +@pytest.mark.parametrize("spdk_version", ["", "R26.2-PRE-latest", "R26.3"]) +def test_block_and_unblock_use_rpc_whatever_the_recorded_version(spdk_version): + rpc = MagicMock() + node = _node(rpc, spdk_version) + + port_block.set_port(node, 9101, block=True, is_reject=True) + port_block.set_port(node, 9101, block=False) + + rpc.nvmf_port_block.assert_called_once_with(9101, is_reject=True) + rpc.nvmf_port_unblock.assert_called_once_with(9101) + + +def test_method_not_found_propagates_instead_of_falling_back(): + rpc = MagicMock() + rpc.nvmf_port_block.side_effect = RPCRemoteError( + "Method not found", RPCErrorCode.method_not_found) + + with pytest.raises(RPCRemoteError): + port_block.set_port(_node(rpc), 9101, block=True) + + +def test_blocked_ports_come_from_the_rpc(): + rpc = MagicMock() + rpc.nvmf_get_blocked_ports.return_value = { + "blocked_ports": [{"port": 9101}, {"port": 9031}]} + node = _node(rpc) + + assert port_block.get_blocked_ports_set(node) == {9101, 9031} + assert port_block.is_port_blocked(node, 9101) is True + assert port_block.is_port_blocked(node, 9100) is False + assert health_controller.check_ports_on_node(node, [9101, 9100]) == { + 9101: False, 9100: True} + + +def test_blocked_ports_method_not_found_propagates(): + rpc = MagicMock() + rpc.nvmf_get_blocked_ports.side_effect = RPCRemoteError( + "Method not found", RPCErrorCode.method_not_found) + + with pytest.raises(RPCRemoteError): + health_controller.check_ports_on_node(_node(rpc), [9101]) From d3459b902f89828c00a7cead6444da03a56fa178 Mon Sep 17 00:00:00 2001 From: wmousa Date: Fri, 16 Oct 2026 18:07:58 +0200 Subject: [PATCH 2/3] chore: remove the firewall-port plumbing and the jc dependency Nothing has read any of this since fencing moved to the SPDK nvmf_port_block RPC and the SPDK image dropped iptables and the firewall webapp (ultra #218): - StorageNode.firewall_port, the spdk_process_start(firewall_port=) param, the field in both node-API SPDKParams, the FW_PORT env on the SPDK container (docker and the k8s template) and constants.FW_PORT_START. - utils.get_next_fw_port: no callers. - The deprecated node-API routes /snode/firewall_set_port and /snode/get_firewall, which only returned False. - StorageNode.spdk_version: only the removed FirewallClient fallback read it. - The jc dependency (it parsed iptables output); uv.lock regenerated with uv 0.12.22, the version docker/Dockerfile pins. Existing FDB records keep their stored firewall_port / spdk_version keys; BaseModel.from_dict only reads declared attributes, so they are ignored. Co-Authored-By: Claude Opus 5.5 --- pyproject.toml | 1 - simplyblock_core/constants.py | 1 - simplyblock_core/models/storage_node.py | 3 - simplyblock_core/snode_client.py | 4 +- simplyblock_core/storage_node_ops.py | 2 - simplyblock_core/utils/__init__.py | 26 ---- .../api/internal/storage_node/docker.py | 2 - .../api/internal/storage_node/kubernetes.py | 26 ---- .../templates/storage_deploy_spdk.yaml.j2 | 2 - tests/integration/test_lvs_role_assignment.py | 1 - tests/unit/test_port_allocation.py | 120 +----------------- tests/unit/test_storage_deploy_template.py | 1 - uv.lock | 36 +----- 13 files changed, 5 insertions(+), 220 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c6bab74933..cec650f171 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,6 @@ dependencies = [ "graypy==2.1.0", "boto3", "jinja2", - "jc", "prometheus-client", "ec2-metadata", "flask-swagger-ui", diff --git a/simplyblock_core/constants.py b/simplyblock_core/constants.py index b79a29e20e..903f10eba4 100644 --- a/simplyblock_core/constants.py +++ b/simplyblock_core/constants.py @@ -970,7 +970,6 @@ def get_config_var(name, default=None): # Backward compatibility aliases RPC_PORT_RANGE_START = RPC_BASE_PORT -FW_PORT_START = SNODE_API_PORT LVOL_NVMF_PORT_START = NVMF_BASE_PORT NODE_NVMF_PORT_START = NVMF_BASE_PORT NODE_HUBLVOL_PORT_START = NVMF_BASE_PORT diff --git a/simplyblock_core/models/storage_node.py b/simplyblock_core/models/storage_node.py index 61a725a729..64de18fa9d 100644 --- a/simplyblock_core/models/storage_node.py +++ b/simplyblock_core/models/storage_node.py @@ -207,12 +207,9 @@ class StorageNode(BaseNodeObject): active_tcp: bool = True active_rdma: bool = False socket: int = 0 - firewall_port: int = 5001 lvol_poller_mask: str = "" spdk_proxy_image: str = "" transfer_hublvol: HubLVol = None # type: ignore[assignment] - # spdk image tag - spdk_version: str = "" def get_lvol_subsys_port(self, lvs_name=None): """Get the client-facing NVMeoF port for a specific lvstore. diff --git a/simplyblock_core/snode_client.py b/simplyblock_core/snode_client.py index e557c98c49..b938bd656d 100644 --- a/simplyblock_core/snode_client.py +++ b/simplyblock_core/snode_client.py @@ -129,7 +129,7 @@ def recalculate_cores_distribution(self, cores, number_of_alceml_devices): def spdk_process_start(self, l_cores, spdk_mem, spdk_image=None, spdk_debug=None, cluster_ip=None, fdb_connection: SecretStr | None = None, namespace=None, server_ip=None, rpc_port=None, rpc_username=None, rpc_password: SecretStr | None = None, multi_threading_enabled=False, timeout=0, ssd_pcie=None, - total_mem=None, system_mem=None, cluster_mode=None, socket=0, firewall_port=0, cluster_id=None, + total_mem=None, system_mem=None, cluster_mode=None, socket=0, cluster_id=None, spdk_proxy_image=None, mcp_max_unavailable=None): params = { "cluster_ip": cluster_ip, @@ -162,8 +162,6 @@ def spdk_process_start(self, l_cores, spdk_mem, spdk_image=None, spdk_debug=None params["system_mem"] = system_mem if cluster_mode: params["cluster_mode"] = cluster_mode - if firewall_port: - params["firewall_port"] = firewall_port params["socket"] = socket if cluster_id: params["cluster_id"] = cluster_id diff --git a/simplyblock_core/storage_node_ops.py b/simplyblock_core/storage_node_ops.py index 4669c6b837..8ebe5b67db 100644 --- a/simplyblock_core/storage_node_ops.py +++ b/simplyblock_core/storage_node_ops.py @@ -4280,7 +4280,6 @@ def add_node(cluster_id, node_addr, iface_name, data_nics_list, snode.active_tcp = active_tcp snode.active_rdma = active_rdma snode.spdk_proxy_image = spdk_proxy_image - snode.spdk_version = spdk_proxy_image.split(":")[1] if 'cpu_count' in node_info: snode.cpu = node_info['cpu_count'] @@ -8579,7 +8578,6 @@ def _restart_storage_node_impl( if spdk_proxy_image: snode.spdk_proxy_image = spdk_proxy_image - snode.spdk_version = spdk_proxy_image.split(":")[1] if not snode.spdk_proxy_image: snode.spdk_proxy_image = cluster.container_image_prefix + constants.SIMPLY_BLOCK_DOCKER_IMAGE diff --git a/simplyblock_core/utils/__init__.py b/simplyblock_core/utils/__init__.py index 8854e47ed7..528d2631d8 100644 --- a/simplyblock_core/utils/__init__.py +++ b/simplyblock_core/utils/__init__.py @@ -1000,32 +1000,6 @@ def _get_cluster_port_config(cluster_id): return constants.NVMF_BASE_PORT, constants.RPC_BASE_PORT, constants.SNODE_API_PORT -def get_next_fw_port(cluster_id, mgmt_ip=None): - """Get the SNodeAPI/firewall port. One per SPDK storage node. - - In Kubernetes hyper-converged layouts, multiple SPDK pods can land on the - same worker host (one per NUMA socket). Each pod runs its own SnodeAPI - sidecar, which binds `FW_PORT` in its pod network namespace, so two co- - located pods must get DIFFERENT ports or only one sidecar wins the bind - and firewall RPCs for the others fail with ECONNREFUSED. - - `mgmt_ip` is kept for signature compatibility but is no longer used for - port sharing — allocation is strictly per-node. - """ - from simplyblock_core.db_controller import DBController - db_controller = DBController() - - _, _, snode_api_port = _get_cluster_port_config(cluster_id) - used_ports = set() - for node in db_controller.get_storage_nodes_by_cluster_id(cluster_id): - if node.firewall_port > 0: - used_ports.add(node.firewall_port) - next_port = snode_api_port - while next_port in used_ports: - next_port += 1 - return next_port - - def get_node_nvmf_ports(cluster_id): """NVMe-oF ports persisted on the cluster's node records (lvol, hublvol, device, rpc, per-lvstore).""" diff --git a/simplyblock_web/api/internal/storage_node/docker.py b/simplyblock_web/api/internal/storage_node/docker.py index bed72b326e..dac39bba2b 100644 --- a/simplyblock_web/api/internal/storage_node/docker.py +++ b/simplyblock_web/api/internal/storage_node/docker.py @@ -88,7 +88,6 @@ class SPDKParams(BaseModel): cluster_ip: str | None = Field(default=None, pattern=utils.IP_PATTERN) cluster_mode: str socket: int | None = Field(None, ge=0) - firewall_port: int = Field(constants.FW_PORT_START) cluster_id: str mcp_max_unavailable: int | None = Field(None) # OpenShift-only (MCP); ignored here @@ -169,7 +168,6 @@ def spdk_process_start(body: SPDKParams): f"PCI_ALLOWED={ssd_pcie_list}", f"TOTAL_HP={total_mem_mib}", f"NSOCKET={body.socket}", - f"FW_PORT={body.firewall_port}", ] # restart_policy={"Name": "on-failure", "MaximumRetryCount": 99} ) diff --git a/simplyblock_web/api/internal/storage_node/kubernetes.py b/simplyblock_web/api/internal/storage_node/kubernetes.py index a4135e56e0..cd3b81e242 100644 --- a/simplyblock_web/api/internal/storage_node/kubernetes.py +++ b/simplyblock_web/api/internal/storage_node/kubernetes.py @@ -290,7 +290,6 @@ class SPDKParams(BaseModel): cluster_ip: str = Field(pattern=utils.IP_PATTERN) cluster_mode: str socket: int | None = Field(None, ge=0) - firewall_port: int | None = Field(constants.FW_PORT_START) cluster_id: str mcp_max_unavailable: int | None = Field(None) @@ -405,7 +404,6 @@ def spdk_process_start(body: SPDKParams): 'PCI_ALLOWED': ssd_pcie_list, 'TOTAL_HP': total_mem_mib, 'NSOCKET': body.socket, - 'FW_PORT': body.firewall_port, 'CPU_TOPOLOGY_ENABLED': cpu_topology_enabled, 'RESERVED_SYSTEM_CPUS': reserved_system_cpus, 'MCP_MAX_UNAVAILABLE': mcp_max_unavailable, @@ -750,30 +748,6 @@ def write_key_file(body: WriteKeyFileBody): return utils.get_response(key_path) -class _FirewallParams(BaseModel): - port_id: int - port_type: str - action: str - rpc_port: int = Field(ge=1, le=65536) - - -@api.post('/firewall_set_port', responses={ - 200: {'content': {'application/json': {'schema': utils.response_schema({ - 'type': 'string' - })}}}, -}) -def firewall_set_port(body: _FirewallParams): - return utils.get_response(False, "deprecated bath post snode/firewall_set_port") - -@api.get('/get_firewall', responses={ - 200: {'content': {'application/json': {'schema': utils.response_schema({ - 'type': 'string' - })}}}, -}) -def get_firewall(): - return utils.get_response(False, "deprecated bath get snode/get_firewall") - - @api.post('/persist_node_config', responses={ 200: {'content': {'application/json': {'schema': utils.response_schema({ 'type': 'boolean' diff --git a/simplyblock_web/templates/storage_deploy_spdk.yaml.j2 b/simplyblock_web/templates/storage_deploy_spdk.yaml.j2 index 664d4f7bd4..52a5e7ce26 100755 --- a/simplyblock_web/templates/storage_deploy_spdk.yaml.j2 +++ b/simplyblock_web/templates/storage_deploy_spdk.yaml.j2 @@ -105,8 +105,6 @@ spec: value: "{{ RPC_PORT }}" - name: NSOCKET value: "{{ NSOCKET }}" - - name: FW_PORT - value: "{{ FW_PORT }}" lifecycle: postStart: exec: diff --git a/tests/integration/test_lvs_role_assignment.py b/tests/integration/test_lvs_role_assignment.py index 9aa18c0978..df7dcdfb77 100644 --- a/tests/integration/test_lvs_role_assignment.py +++ b/tests/integration/test_lvs_role_assignment.py @@ -77,7 +77,6 @@ def _node(uuid, status=StorageNode.STATUS_ONLINE, cluster_id="cluster-1", n.remote_jm_devices = [] n.nvme_devices = [] n.health_check = True - n.firewall_port = 50001 nic = IFace() nic.ip4_address = mgmt_ip or unique_ip(f"{uuid}-nic") nic.trtype = "TCP" diff --git a/tests/unit/test_port_allocation.py b/tests/unit/test_port_allocation.py index 06e6e6c98f..ae9024a71e 100644 --- a/tests/unit/test_port_allocation.py +++ b/tests/unit/test_port_allocation.py @@ -42,7 +42,7 @@ def _cluster(nvmf_base_port=4420, rpc_base_port=8080, snode_api_port=50001, def _node(uuid, mgmt_ip="10.0.0.1", lvol_subsys_port=0, nvmf_port=0, - hublvol_port=0, rpc_port=0, firewall_port=0, cluster_id="cluster-test-001"): + hublvol_port=0, rpc_port=0, cluster_id="cluster-test-001"): n = StorageNode() n.uuid = uuid n.cluster_id = cluster_id @@ -50,7 +50,6 @@ def _node(uuid, mgmt_ip="10.0.0.1", lvol_subsys_port=0, nvmf_port=0, n.lvol_subsys_port = lvol_subsys_port n.nvmf_port = nvmf_port n.rpc_port = rpc_port - n.firewall_port = firewall_port if hublvol_port > 0: n.hublvol = HubLVol({"nvmf_port": hublvol_port, "uuid": f"hub-{uuid}", "nqn": f"nqn.hub.{uuid}", "bdev_name": "lvs/hublvol"}) @@ -98,7 +97,6 @@ def test_canonical_constants_exist(self): def test_backward_compat_aliases(self): self.assertEqual(constants.RPC_PORT_RANGE_START, constants.RPC_BASE_PORT) - self.assertEqual(constants.FW_PORT_START, constants.SNODE_API_PORT) self.assertEqual(constants.LVOL_NVMF_PORT_START, constants.NVMF_BASE_PORT) self.assertEqual(constants.NODE_NVMF_PORT_START, constants.NVMF_BASE_PORT) self.assertEqual(constants.NODE_HUBLVOL_PORT_START, constants.NVMF_BASE_PORT) @@ -291,104 +289,6 @@ def test_next_free_hublvol_port_delegates(self, mock_nvmf): mock_nvmf.assert_called_once_with("c1") -# --------------------------------------------------------------------------- -# Test get_next_fw_port (SNodeAPI — per-storage-node, unique per SPDK) -# --------------------------------------------------------------------------- - -class TestGetNextFwPort(unittest.TestCase): - - @patch("simplyblock_core.utils._get_cluster_port_config") - @patch("simplyblock_core.db_controller.DBController") - def test_empty_cluster_returns_base(self, mock_db_cls, mock_config): - from simplyblock_core.utils import get_next_fw_port - mock_config.return_value = (4420, 8080, 50001) - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = [] - self.assertEqual(get_next_fw_port("c1"), 50001) - - @patch("simplyblock_core.utils._get_cluster_port_config") - @patch("simplyblock_core.db_controller.DBController") - def test_co_located_nodes_get_distinct_ports(self, mock_db_cls, mock_config): - """Two SPDK nodes on the same mgmt_ip must get DIFFERENT firewall ports. - - In K8s hyper-converged layouts, each SPDK pod runs its own SnodeAPI - sidecar; sharing a port would cause bind conflicts and ECONNREFUSED - on firewall checks. - """ - from simplyblock_core.utils import get_next_fw_port - mock_config.return_value = (4420, 8080, 50001) - nodes = [_node("n1", mgmt_ip="10.0.0.1", firewall_port=50001)] - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = nodes - result = get_next_fw_port("c1", mgmt_ip="10.0.0.1") - self.assertEqual(result, 50002) - - @patch("simplyblock_core.utils._get_cluster_port_config") - @patch("simplyblock_core.db_controller.DBController") - def test_new_port_for_different_host_ip(self, mock_db_cls, mock_config): - """A new host IP should get a new firewall port.""" - from simplyblock_core.utils import get_next_fw_port - mock_config.return_value = (4420, 8080, 50001) - nodes = [_node("n1", mgmt_ip="10.0.0.1", firewall_port=50001)] - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = nodes - result = get_next_fw_port("c1", mgmt_ip="10.0.0.2") - self.assertEqual(result, 50002) - - @patch("simplyblock_core.utils._get_cluster_port_config") - @patch("simplyblock_core.db_controller.DBController") - def test_without_mgmt_ip_allocates_new(self, mock_db_cls, mock_config): - from simplyblock_core.utils import get_next_fw_port - mock_config.return_value = (4420, 8080, 50001) - nodes = [_node("n1", mgmt_ip="10.0.0.1", firewall_port=50001)] - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = nodes - result = get_next_fw_port("c1") - self.assertEqual(result, 50002) - - @patch("simplyblock_core.utils._get_cluster_port_config") - @patch("simplyblock_core.db_controller.DBController") - def test_custom_snode_api_port(self, mock_db_cls, mock_config): - from simplyblock_core.utils import get_next_fw_port - mock_config.return_value = (4420, 8080, 60000) - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = [] - self.assertEqual(get_next_fw_port("c1"), 60000) - - @patch("simplyblock_core.utils._get_cluster_port_config") - @patch("simplyblock_core.db_controller.DBController") - def test_multiple_hosts_sequential_ports(self, mock_db_cls, mock_config): - from simplyblock_core.utils import get_next_fw_port - mock_config.return_value = (4420, 8080, 50001) - nodes = [ - _node("n1", mgmt_ip="10.0.0.1", firewall_port=50001), - _node("n2", mgmt_ip="10.0.0.2", firewall_port=50002), - ] - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = nodes - # Third host gets 50003 - self.assertEqual(get_next_fw_port("c1", mgmt_ip="10.0.0.3"), 50003) - - @patch("simplyblock_core.utils._get_cluster_port_config") - @patch("simplyblock_core.db_controller.DBController") - def test_third_node_same_ip_gets_fresh_port(self, mock_db_cls, mock_config): - """Three SPDK nodes on the same IP: each gets its own port.""" - from simplyblock_core.utils import get_next_fw_port - mock_config.return_value = (4420, 8080, 50001) - nodes = [ - _node("n1", mgmt_ip="10.0.0.1", firewall_port=50001), - _node("n2", mgmt_ip="10.0.0.1", firewall_port=50002), - ] - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = nodes - result = get_next_fw_port("c1", mgmt_ip="10.0.0.1") - self.assertEqual(result, 50003) - - @patch("simplyblock_core.utils._get_cluster_port_config") - @patch("simplyblock_core.db_controller.DBController") - def test_ignores_zero_firewall_port_on_existing_node(self, mock_db_cls, mock_config): - from simplyblock_core.utils import get_next_fw_port - mock_config.return_value = (4420, 8080, 50001) - nodes = [_node("n1", mgmt_ip="10.0.0.1", firewall_port=0)] - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = nodes - # firewall_port=0 is unallocated; should not be treated as reserved - result = get_next_fw_port("c1", mgmt_ip="10.0.0.1") - self.assertEqual(result, 50001) - - # --------------------------------------------------------------------------- # Test unified pool: sequential allocations yield unique ports # --------------------------------------------------------------------------- @@ -481,19 +381,7 @@ def test_node_removed_frees_port(self, mock_ports, mock_config): class TestMultiNodeSameHost(unittest.TestCase): """Test that multiple nodes on the same host IP get distinct ports across - all port types, including SNodeAPI/firewall (one sidecar per SPDK pod).""" - - @patch("simplyblock_core.utils._get_cluster_port_config", return_value=(4420, 8080, 50001)) - @patch("simplyblock_core.db_controller.DBController") - def test_two_nodes_same_ip_get_distinct_fw_ports(self, mock_db_cls, mock_config): - from simplyblock_core.utils import get_next_fw_port - nodes = [ - _node("n1", mgmt_ip="10.0.0.1", firewall_port=50001, - lvol_subsys_port=4420, nvmf_port=4421, hublvol_port=4422), - ] - mock_db_cls.return_value.get_storage_nodes_by_cluster_id.return_value = nodes - # Second SPDK pod on same host gets its own FW port - self.assertEqual(get_next_fw_port("c1", mgmt_ip="10.0.0.1"), 50002) + all port types.""" @patch("simplyblock_core.utils._get_cluster_port_config", return_value=(4420, 8080, 50001)) @patch("simplyblock_core.utils._get_all_nvmf_ports") @@ -678,7 +566,6 @@ def test_storage_node_default_ports(self): n = StorageNode() self.assertEqual(n.lvol_subsys_port, 9090) # legacy default in model self.assertEqual(n.nvmf_port, 4420) - self.assertEqual(n.firewall_port, 5001) self.assertEqual(n.rpc_port, -1) @@ -837,12 +724,11 @@ class TestNodeDisplayPorts(unittest.TestCase): def test_node_port_fields_are_readable(self): n = _node("n1", lvol_subsys_port=4420, nvmf_port=4421, hublvol_port=4422, - rpc_port=8080, firewall_port=50001) + rpc_port=8080) self.assertEqual(n.lvol_subsys_port, 4420) self.assertEqual(n.nvmf_port, 4421) self.assertEqual(n.hublvol.nvmf_port, 4422) self.assertEqual(n.rpc_port, 8080) - self.assertEqual(n.firewall_port, 50001) def test_node_without_hublvol(self): n = _node("n1", lvol_subsys_port=4420, nvmf_port=4421) diff --git a/tests/unit/test_storage_deploy_template.py b/tests/unit/test_storage_deploy_template.py index 2af67b3fa0..c98a6cdc50 100644 --- a/tests/unit/test_storage_deploy_template.py +++ b/tests/unit/test_storage_deploy_template.py @@ -32,7 +32,6 @@ def _render_storage_deploy(tls_provider: str) -> str: PCI_ALLOWED="", TOTAL_HP="", NSOCKET=0, - FW_PORT=50001, CPU_TOPOLOGY_ENABLED=False, MEM_MEGA=1536, MEM2_MEGA=1024, diff --git a/uv.lock b/uv.lock index 90d166f054..c681ebd4d7 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 3 +revision = 5 requires-python = ">=3.11" [manifest] @@ -702,20 +702,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/04/96/92447566d16df59b2a776c0fb82dbc4d9e07cd95062562af01e408583fc4/itsdangerous-2.2.0-py3-none-any.whl", hash = "sha256:c6242fc49e35958c8b15141343aa660db5fc54d4f13a1db01a3f5891b98700ef", size = 16234, upload-time = "2024-04-16T21:28:14.499Z" }, ] -[[package]] -name = "jc" -version = "1.25.7" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "pygments" }, - { name = "ruamel-yaml" }, - { name = "xmltodict" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/a9/da/7827d3389d87fa645556d595a2a437576de83f7a7d4a5fceda9c32208c0b/jc-1.25.7.tar.gz", hash = "sha256:63481e34d92715781e1b094eca7d76d122a14489463d45b7f3e8180f44ea3a10", size = 616951, upload-time = "2026-06-18T17:06:23.49Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/42/59/6eed63d63287e905fe288a1d2791d288e7dee5c80063ec3f5bda4217ab55/jc-1.25.7-py3-none-any.whl", hash = "sha256:5ed6a0da915c931c04693cab806d5c31d9ef14ca998c6c32308460d37cb30baf", size = 705553, upload-time = "2026-06-18T17:06:18.874Z" }, -] - [[package]] name = "jinja2" version = "3.1.6" @@ -1656,15 +1642,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/10/85/48f0abdcef5cce4e034c7a5b0ceeceba0b01bf0d942824f4bb720afe2dec/rpds_py-2026.6.3-pp311-pypy311_pp73-musllinux_1_2_x86_64.whl", hash = "sha256:8e65860d238379ed982fd9ba690579b5e95af2f4840f99c772816dbe573cb826", size = 586486, upload-time = "2026-06-30T07:17:51.141Z" }, ] -[[package]] -name = "ruamel-yaml" -version = "0.19.1" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/c7/3b/ebda527b56beb90cb7652cb1c7e4f91f48649fbcd8d2eb2fb6e77cd3329b/ruamel_yaml-0.19.1.tar.gz", hash = "sha256:53eb66cd27849eff968ebf8f0bf61f46cdac2da1d1f3576dd4ccee9b25c31993", size = 142709, upload-time = "2026-01-02T16:50:31.84Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/b8/0c/51f6841f1d84f404f92463fc2b1ba0da357ca1e3db6b7fbda26956c3b82a/ruamel_yaml-0.19.1-py3-none-any.whl", hash = "sha256:27592957fedf6e0b62f281e96effd28043345e0e66001f97683aa9a40c667c93", size = 118102, upload-time = "2026-01-02T16:50:29.201Z" }, -] - [[package]] name = "s3transfer" version = "0.19.2" @@ -1692,7 +1669,6 @@ dependencies = [ { name = "foundationdb" }, { name = "graypy" }, { name = "hvac" }, - { name = "jc" }, { name = "jinja2" }, { name = "jsonschema" }, { name = "kubernetes" }, @@ -1749,7 +1725,6 @@ requires-dist = [ { name = "foundationdb", specifier = "==7.3.63" }, { name = "graypy", specifier = "==2.1.0" }, { name = "hvac" }, - { name = "jc" }, { name = "jinja2" }, { name = "jsonschema" }, { name = "kubernetes", specifier = "==35.0.0" }, @@ -2118,12 +2093,3 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/68/64/d15740c763dd0ddea2338ad42e3bd4a84f8702e16083e7ff61674c504a13/wrapt-2.4.0-cp315-cp315t-win_arm64.whl", hash = "sha256:36b56a4fba13b34ed8ff307557325fff215de0a58b5dbaef2c50e4d8aa39dbd1", size = 96039, upload-time = "2026-08-30T04:41:29.062Z" }, { url = "https://files.pythonhosted.org/packages/79/c8/fafe0002f572ced999c792cfe8b05d39269c63d8193d15d25bd828bcad7a/wrapt-2.4.0-py3-none-any.whl", hash = "sha256:18aabd9301d06026f5900538051773d6f87f65ae02cdc60de482df978513dc0a", size = 73713, upload-time = "2026-08-30T04:41:49.805Z" }, ] - -[[package]] -name = "xmltodict" -version = "1.0.4" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/19/70/80f3b7c10d2630aa66414bf23d210386700aa390547278c789afa994fd7e/xmltodict-1.0.4.tar.gz", hash = "sha256:6d94c9f834dd9e44514162799d344d815a3a4faec913717a9ecbfa5be1bb8e61", size = 26124, upload-time = "2026-02-22T02:21:22.074Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/38/34/98a2f52245f4d47be93b580dae5f9861ef58977d73a79eb47c58f1ad1f3a/xmltodict-1.0.4-py3-none-any.whl", hash = "sha256:a4a00d300b0e1c59fc2bfccb53d7b2e88c32f200df138a0dd2229f842497026a", size = 13580, upload-time = "2026-02-22T02:21:21.039Z" }, -] From 59a238bf6613cb970d65996eeedfb52fbaad0dbf Mon Sep 17 00:00:00 2001 From: wmousa Date: Fri, 16 Oct 2026 23:51:37 +0200 Subject: [PATCH 3/3] refactor: remove utils.port_block, fence through the RPC client directly With the FirewallClient fallback gone, port_block was a pass-through (review on #1496): - set_port only chose nvmf_port_block or nvmf_port_unblock, and every caller passed block= as a constant: the 13 call sites now call node.rpc_client(timeout=.., retry=..).nvmf_port_block/unblock(port) with the budget each already used. The rationale for the short fence budget moves to RPCClient.nvmf_port_block. - get_blocked_ports_set was the only caller of RPCClient.nvmf_get_blocked_ports: the projection to a set of port numbers moves into that method. - is_port_blocked was a one-line membership test; check_port_on_node does it inline. Tests: they faked fencing by patching port_block.set_port, which also told them which node a fence was for. tests._mocks.route_fences keeps that: it wraps each node's rpc_client so nvmf_port_block/unblock reach the same (node, port, block) callback the tests already used. Source text anchors and "must not fence" assertions now look for the RPC calls, and mocks of nvmf_get_blocked_ports return a set. Co-Authored-By: Claude Opus 5.5 --- simplyblock_core/cluster_ops.py | 6 +- .../controllers/health_controller.py | 6 +- simplyblock_core/rpc_client.py | 27 +++- .../services/storage_node_monitor.py | 3 +- .../services/tasks_runner_port_allow.py | 3 +- simplyblock_core/storage_node_ops.py | 21 ++- simplyblock_core/utils/port_block.py | 65 --------- tests/_mocks.py | 39 +++++ tests/integration/expansion_sim/_rpc_sim.py | 5 +- tests/integration/ftt2/conftest.py | 5 +- ...ctivate_portblock_and_suspend_no_demote.py | 25 ++-- .../test_activation_ana_inaccessible.py | 1 - tests/integration/test_dual_ft_e2e.py | 5 +- .../test_dual_ft_secondary_fixes.py | 27 ++-- .../test_failover_failback_combinations.py | 136 +++++++++--------- tests/integration/test_lvs_role_assignment.py | 27 ++-- tests/integration/test_nvmeof_security.py | 5 +- tests/integration/test_port_allow_recovery.py | 37 ++--- ...reate_lvstore_abort_on_peer_rpc_failure.py | 26 ++-- ...est_recreate_secondary_takeover_cascade.py | 9 +- tests/integration/test_secondary_promotion.py | 26 ++-- .../test_shutdown_no_suspension.py | 12 +- tests/unit/test_connect_hublvol_lvs_node.py | 2 - tests/unit/test_cp_latency_fixes.py | 2 +- .../test_drain_replaced_with_fixed_sleep.py | 2 +- tests/unit/test_failback_listener_ana.py | 2 +- .../unit/test_fence_remediation_down_node.py | 8 +- tests/unit/test_ftt_protection.py | 13 +- tests/unit/test_port_block_leak_k8s_0911.py | 41 +++--- tests/unit/test_port_block_rpc_only.py | 70 +++++---- tests/unit/test_restart_cpu_fixes.py | 4 +- tests/unit/test_restart_portblock_order.py | 16 +-- tests/unit/test_two_node_c3_fixes.py | 5 +- tests/unit/test_two_node_c5_fixes.py | 4 +- tests/unit/test_two_node_f15_port_allow.py | 9 +- 35 files changed, 345 insertions(+), 349 deletions(-) delete mode 100644 simplyblock_core/utils/port_block.py diff --git a/simplyblock_core/cluster_ops.py b/simplyblock_core/cluster_ops.py index c65a8c5de7..a27a8cd43e 100644 --- a/simplyblock_core/cluster_ops.py +++ b/simplyblock_core/cluster_ops.py @@ -66,7 +66,7 @@ from simplyblock_core.release_upgrades import jc_compression_upgrade from simplyblock_core.rpc_client import RPCException from simplyblock_core.settings import Settings -from simplyblock_core.utils import port_block, pull_docker_image_with_retry +from simplyblock_core.utils import pull_docker_image_with_retry logger = utils.get_logger(__name__) @@ -1645,7 +1645,7 @@ def _recreate_non_leader_lvs(node_id) -> bool: if not is_fresh_activation and primary_node.status == StorageNode.STATUS_ONLINE: try: leader_port = primary_node.get_lvol_subsys_port(primary_node.lvstore) - port_block.set_port(primary_node, leader_port, block=True, timeout=3, retry=1) + primary_node.rpc_client(timeout=3, retry=1).nvmf_port_block(leader_port) tcp_ports_events.port_deny(primary_node, leader_port) leader_blocked = True time.sleep(0.5) @@ -1668,7 +1668,7 @@ def _recreate_non_leader_lvs(node_id) -> bool: finally: if leader_blocked: try: - port_block.set_port(primary_node, leader_port, block=False, timeout=3, retry=1) + primary_node.rpc_client(timeout=3, retry=1).nvmf_port_unblock(leader_port) tcp_ports_events.port_allowed(primary_node, leader_port) except Exception as ue: logger.error( diff --git a/simplyblock_core/controllers/health_controller.py b/simplyblock_core/controllers/health_controller.py index 4c4bc6528a..d5dfa2b73d 100644 --- a/simplyblock_core/controllers/health_controller.py +++ b/simplyblock_core/controllers/health_controller.py @@ -251,8 +251,7 @@ def _log_port_check_failure(db_controller, snode, port, exc): def check_port_on_node(snode, port_id): - from simplyblock_core.utils import port_block - return not port_block.is_port_blocked(snode, port_id) + return int(port_id) not in snode.rpc_client(timeout=5, retry=5).nvmf_get_blocked_ports() def check_ports_on_node(snode, port_ids): @@ -260,11 +259,10 @@ def check_ports_on_node(snode, port_ids): ``nvmf_get_blocked_ports`` fetch answers every port in ``port_ids``. Returns ``{port: bool}`` (True = open). Raises on fetch failure — callers treat it like the single-port failure path.""" - from simplyblock_core.utils import port_block port_ids = list(port_ids) if not port_ids: return {} - blocked = port_block.get_blocked_ports_set(snode) + blocked = snode.rpc_client(timeout=5, retry=5).nvmf_get_blocked_ports() return {p: int(p) not in blocked for p in port_ids} diff --git a/simplyblock_core/rpc_client.py b/simplyblock_core/rpc_client.py index 775ede46d2..cbfb5a46e7 100644 --- a/simplyblock_core/rpc_client.py +++ b/simplyblock_core/rpc_client.py @@ -1997,6 +1997,19 @@ def jc_compression_start(self, jm_vuid): return self._request("jc_compression", jm_vuid=jm_vuid) def nvmf_port_block(self, port, is_reject=False): + """Fence ``port``: SPDK stops serving client connections on it. + + Callers fencing a peer's client listener build this client with a + short budget, e.g. ``node.rpc_client(timeout=0.5, retry=1)``. + ``timeout`` is per HTTP attempt and the client retries ``retry`` + times with backoff, so the wall clock is roughly (retry+1) x timeout + plus backoff. While the port is blocked the peer cannot answer + keep-alives, and the client KATO is 4s: a fence window longer than + that is an outage. On 2026-08-31 a failing block on a rebooting peer + with a (5s, 2) budget took 12s, left a healthy survivor fenced, and + the client lost its last path at 4.1s. A healthy peer answers in + ~11ms, so 0.5s is ~45x the observed latency. + """ params = {"port": port} if is_reject: params["reject"] = is_reject @@ -2005,8 +2018,18 @@ def nvmf_port_block(self, port, is_reject=False): def nvmf_port_unblock(self, port): return self._request("nvmf_port_unblock", port=port) - def nvmf_get_blocked_ports(self): - return self._request("nvmf_get_blocked_ports") + def nvmf_get_blocked_ports(self) -> set[int]: + """Ports currently fenced on this SPDK. + + The RPC returns ``{"blocked_ports": [{"port": N, ...}, ...]}``. Test + membership against the projected set, never against the raw reply: + ``port in reply`` tested the dict's keys and read every fenced port as + open (2026-09-11, k8s rapid failover).""" + blocked = self._request("nvmf_get_blocked_ports") + if not blocked: + return set() + entries = blocked.get("blocked_ports", []) if isinstance(blocked, dict) else [] + return {int(e.get("port", -1)) for e in entries} def bdev_raid_get_bdevs(self): return self._request("bdev_raid_get_bdevs", category="online") diff --git a/simplyblock_core/services/storage_node_monitor.py b/simplyblock_core/services/storage_node_monitor.py index 9608034c6e..6bd24b958c 100644 --- a/simplyblock_core/services/storage_node_monitor.py +++ b/simplyblock_core/services/storage_node_monitor.py @@ -1869,8 +1869,7 @@ def _remediate_stale_port_blocks(db, snode, port_results, port_lvs_owner): "nothing released it. Unblocking.", port, snode.get_id(), held, snode.status, owner.lvstore) try: - from simplyblock_core.utils import port_block - port_block.set_port(snode, port, block=False, timeout=5, retry=1) + snode.rpc_client(timeout=5, retry=1).nvmf_port_unblock(port) _blocked_port_since.pop(key, None) except Exception as e: logger.error("Failed to unblock stale port %s on %s: %s", diff --git a/simplyblock_core/services/tasks_runner_port_allow.py b/simplyblock_core/services/tasks_runner_port_allow.py index 290f0957b6..b354aed6cf 100644 --- a/simplyblock_core/services/tasks_runner_port_allow.py +++ b/simplyblock_core/services/tasks_runner_port_allow.py @@ -20,7 +20,6 @@ serve, set_result, ) -from simplyblock_core.utils import port_block logger = utils.get_logger(__name__) @@ -971,7 +970,7 @@ def exec_port_allow_task(task): lvs_ports.append(port_number) logger.info(f"Allow LVS ports {lvs_ports} on node {node.get_id()}") for p in lvs_ports: - port_block.set_port(node, p, block=False, timeout=5, retry=2) + node.rpc_client(timeout=5, retry=2).nvmf_port_unblock(p) tcp_ports_events.port_allowed(node, p) # Deferred ANA failover, AFTER the port release: a secondary recovering diff --git a/simplyblock_core/storage_node_ops.py b/simplyblock_core/storage_node_ops.py index 8ebe5b67db..a9ceb44e0f 100644 --- a/simplyblock_core/storage_node_ops.py +++ b/simplyblock_core/storage_node_ops.py @@ -88,7 +88,6 @@ dial_backoff, hublvol_reconnect, nvmf_listener, - port_block, pull_docker_image_with_retry, rpc_budget, ) @@ -9378,7 +9377,7 @@ def _fenced_secondary_of(snode: StorageNode, not_online_nodes): continue port = sec.get_lvol_subsys_port(snode.lvstore) try: - blocked = port_block.get_blocked_ports_set(sec, timeout=5, retry=1) + blocked = sec.rpc_client(timeout=5, retry=1).nvmf_get_blocked_ports() except (RPCException, SNodeClientException, OSError) as e: logger.warning("Cannot read blocked ports of secondary %s of %s: %s", sec.get_id(), snode.get_id(), e) @@ -12723,7 +12722,7 @@ def _abort_and_unblock(reason): leader_node, leader_lvs_port, primary_node.jm_vuid, db_controller, "Restart abort"): try: - port_block.set_port(leader_node, leader_lvs_port, block=False, timeout=0.5, retry=2) + leader_node.rpc_client(timeout=0.5, retry=2).nvmf_port_unblock(leader_lvs_port) _deferred_port_events.append(("allow", leader_node, leader_lvs_port)) _mark_leader_unblocked() except Exception as ue: @@ -12775,7 +12774,7 @@ def _abort_and_unblock(reason): attempts = 3 for attempt in range(1, attempts + 1): try: - port_block.set_port(leader_node, leader_lvs_port, block=True, timeout=3, retry=1) + leader_node.rpc_client(timeout=3, retry=1).nvmf_port_block(leader_lvs_port) _deferred_port_events.append(("deny", leader_node, leader_lvs_port)) leader_port_blocked = True _mark_leader_blocked() @@ -13084,7 +13083,7 @@ def _lvol_bdev_registered(lv): leader_node, leader_lvs_port, primary_node.jm_vuid, db_controller, "Attach-retry gap"): try: - port_block.set_port(leader_node, leader_lvs_port, block=False, timeout=3, retry=1) + leader_node.rpc_client(timeout=3, retry=1).nvmf_port_unblock(leader_lvs_port) _deferred_port_events.append(("allow", leader_node, leader_lvs_port)) leader_port_blocked = False _mark_leader_unblocked() @@ -13094,7 +13093,7 @@ def _lvol_bdev_registered(lv): "failed: %s", leader_node.get_id(), ue) time.sleep(ATTACH_RETRY_GAP_SEC) try: - port_block.set_port(leader_node, leader_lvs_port, block=True, timeout=3, retry=1) + leader_node.rpc_client(timeout=3, retry=1).nvmf_port_block(leader_lvs_port) _deferred_port_events.append(("deny", leader_node, leader_lvs_port)) leader_port_blocked = True _mark_leader_blocked() @@ -13126,7 +13125,7 @@ def _lvol_bdev_registered(lv): attempts = 3 for attempt in range(1, attempts + 1): try: - port_block.set_port(leader_node, leader_lvs_port, block=False, timeout=3, retry=1) + leader_node.rpc_client(timeout=3, retry=1).nvmf_port_unblock(leader_lvs_port) _deferred_port_events.append(("allow", leader_node, leader_lvs_port)) unblocked = True _mark_leader_unblocked() @@ -13313,7 +13312,7 @@ def _release_lvs_subsys_port_on_peers(lvs_node, exclude_node_id, db_controller): "Defensive unblock after failed recreate of %s" % lvs_node.lvstore): continue - port_block.set_port(peer, port, block=False, timeout=0.5, retry=2) + peer.rpc_client(timeout=0.5, retry=2).nvmf_port_unblock(port) tcp_ports_events.port_allowed(peer, port) logger.info("Defensive unblock: allowed LVS port %s on peer %s after " "failed recreate of %s", port, pid, lvs_node.lvstore) @@ -13859,7 +13858,7 @@ def _unblock_peer_port(peer): pass return try: - port_block.set_port(peer, snode_lvs_port, block=False, timeout=0.5, retry=2) + peer.rpc_client(timeout=0.5, retry=2).nvmf_port_unblock(snode_lvs_port) _deferred_port_events.append(("allow", peer, snode_lvs_port)) _t0 = _block_started.pop(peer.get_id(), None) if _t0 is not None: @@ -14224,7 +14223,7 @@ def _fenced(method, *args, budget=None, **kwargs): if sec_node in blocked_peers: continue try: - port_block.set_port(sec_node, snode_lvs_port, block=True, timeout=0.5, retry=1) + sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port) _deferred_port_events.append(("deny", sec_node, snode_lvs_port)) blocked_peers.append(sec_node) _block_started[sec_node.get_id()] = time.monotonic() @@ -14302,7 +14301,7 @@ def _fenced(method, *args, budget=None, **kwargs): db_controller.atomic_update( current_leader, lambda x: setattr(x, "lvstore_status", "in_creation")) - port_block.set_port(current_leader, snode_lvs_port, block=True, timeout=0.5, retry=1) + current_leader.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port) _deferred_port_events.append(("deny", current_leader, snode_lvs_port)) blocked_peers.append(current_leader) _block_started[current_leader.get_id()] = time.monotonic() diff --git a/simplyblock_core/utils/port_block.py b/simplyblock_core/utils/port_block.py deleted file mode 100644 index 8f24340d00..0000000000 --- a/simplyblock_core/utils/port_block.py +++ /dev/null @@ -1,65 +0,0 @@ -"""Port block helpers over the SPDK ``nvmf_port_block`` / -``nvmf_port_unblock`` / ``nvmf_get_blocked_ports`` RPCs. - -Fencing is done inside SPDK only. There is no iptables fallback: the SPDK -image no longer ships iptables or the firewall webapp (ultra #218), and -25.10.x -> 26.3 upgrades are offline, so no node runs an SPDK build without -these RPCs. A method-not-found error therefore propagates like any other -RPC failure instead of being redirected to a port nothing listens on. -""" - - -def set_port(node, port, block, is_reject=False, timeout=0.5, retry=1): - """Block or unblock ``port`` on ``node``. - - Budget: ``timeout`` is PER HTTP ATTEMPT and RPCClient builds - ``Retry(total=retry, backoff_factor=1, connect=retry)``, so the wall - clock is roughly (retry+1) x timeout plus urllib3 backoff -- not - ``timeout``. The old defaults (5s, 2) therefore cost 12s against a host - that is rebooting and so drops SYNs instead of refusing them - (measured 12.009s = 5 + 5 + 2 on 2026-08-31). - - That matters because blocking a port fences a peer's client listener: - while it is blocked the peer cannot answer keep-alives, and the client - KATO is 4s. A fence window longer than KATO is indistinguishable from an - outage -- on 2026-08-31 a 12s failing block on a rebooting peer left a - healthy survivor fenced, the client lost its last path at 4.1s, and fio - took EIO. A healthy peer answers this RPC in ~11ms, so 0.5s is ~45x the - observed latency; two attempts bound the whole call near 1s. - """ - rpc = node.rpc_client(timeout=timeout, retry=retry) - if block: - return rpc.nvmf_port_block(port, is_reject=is_reject) - return rpc.nvmf_port_unblock(port) - - -def get_blocked_ports_set(node, timeout=5, retry=5): - """One ``nvmf_get_blocked_ports`` fetch -> set of blocked port numbers. - - The health/monitor loops used to call ``is_port_blocked`` per port, - re-fetching the identical full list each time — measured 528 of these - RPCs per minute cluster-wide at idle (2026-07-21 baseline audit). One - fetch per node per cycle answers every port. - """ - blocked = node.rpc_client(timeout=timeout, retry=retry).nvmf_get_blocked_ports() - if not blocked: - return set() - entries = blocked.get("blocked_ports", []) if isinstance(blocked, dict) else [] - return {int(e.get("port", -1)) for e in entries} - - -def is_port_blocked(node, port_id, timeout=5, retry=5): - """Return True if ``port_id`` is currently blocked on ``node``. - - Delegates the parse to :func:`get_blocked_ports_set`. - - This USED to do ``port_id in rpc.nvmf_get_blocked_ports()``. That RPC - returns ``{"blocked_ports": [{"port": N, ...}, ...]}``, so the ``in`` - tested the dict's KEYS and the function returned False for every port, - always -- a blocked port read as open. Only the batched - ``get_blocked_ports_set`` parsed the payload, which is why the monitor's - batched path saw the fences and this one never did. It backs - ``check_port_on_node``, so the miss was silent rather than harmless. - """ - return int(port_id) in get_blocked_ports_set(node, timeout=timeout, retry=retry) - diff --git a/tests/_mocks.py b/tests/_mocks.py index 2b84609e6b..a0ae9d1f1a 100644 --- a/tests/_mocks.py +++ b/tests/_mocks.py @@ -77,3 +77,42 @@ def unique_ip(host_key): ip = f"10.200.{index // 254}.{index % 254 + 1}" _UNIQUE_IPS[host_key] = ip return ip + + +class _FenceRoutingClient: + """An RPC client that sends fences to a test callback and everything else + to the wrapped client.""" + + def __init__(self, client, node, fence): + self._client = client + self._node = node + self._fence = fence + + def nvmf_port_block(self, port, is_reject=False): + return self._fence(self._node, port, block=True, is_reject=is_reject) + + def nvmf_port_unblock(self, port): + return self._fence(self._node, port, block=False) + + def __getattr__(self, name): + return getattr(self._client, name) + + +def route_fences(nodes, fence): + """Send every fence issued through ``node.rpc_client(...)`` for ``nodes`` + to ``fence(node, port, block=..., is_reject=...)``. + + Production fences a peer with ``node.rpc_client(timeout=.., + retry=..).nvmf_port_block(port)`` / ``.nvmf_port_unblock(port)``. Tests + usually hand every node the same mocked RPC client, which loses which + node a fence was meant for; this keeps it. Each node's ``rpc_client`` is + wrapped, so call it after the test has installed its own ``rpc_client`` + on the nodes. Every other RPC goes to the client the node returned. + """ + for node in nodes: + original = node.rpc_client + + def _client(*args, _original=original, _node=node, **kwargs): + return _FenceRoutingClient(_original(*args, **kwargs), _node, fence) + + node.rpc_client = _client diff --git a/tests/integration/expansion_sim/_rpc_sim.py b/tests/integration/expansion_sim/_rpc_sim.py index 5776497a08..f4f1057920 100644 --- a/tests/integration/expansion_sim/_rpc_sim.py +++ b/tests/integration/expansion_sim/_rpc_sim.py @@ -284,8 +284,9 @@ def nvmf_port_unblock(self, port, **_): return True def nvmf_get_blocked_ports(self, **_): - return {"blocked_ports": [{"port": port} for (port, _proto), blocked - in self.firewall.items() if blocked]} + # RpcRouter stands in for RPCClient, whose method already projects + # the SPDK reply to a set of port numbers. + return {port for (port, _proto), blocked in self.firewall.items() if blocked} # -- NVMe controller (cross-node) --------------------------------------- diff --git a/tests/integration/ftt2/conftest.py b/tests/integration/ftt2/conftest.py index 62550ef83c..f21431ca3c 100644 --- a/tests/integration/ftt2/conftest.py +++ b/tests/integration/ftt2/conftest.py @@ -553,8 +553,9 @@ def patch_externals(): return_value=(True, None)), patch('simplyblock_core.controllers.health_controller._check_node_api', return_value=True), - patch('simplyblock_core.utils.port_block.set_port', return_value=True), - patch('simplyblock_core.utils.port_block.get_blocked_ports_set', + patch('simplyblock_core.rpc_client.RPCClient.nvmf_port_block', return_value=True), + patch('simplyblock_core.rpc_client.RPCClient.nvmf_port_unblock', return_value=True), + patch('simplyblock_core.rpc_client.RPCClient.nvmf_get_blocked_ports', return_value=set()), patch('simplyblock_core.controllers.health_controller.check_port_on_node', return_value=True), diff --git a/tests/integration/test_activate_portblock_and_suspend_no_demote.py b/tests/integration/test_activate_portblock_and_suspend_no_demote.py index b371781a1b..8f65fa8664 100644 --- a/tests/integration/test_activate_portblock_and_suspend_no_demote.py +++ b/tests/integration/test_activate_portblock_and_suspend_no_demote.py @@ -24,6 +24,7 @@ from simplyblock_core.models.cluster import Cluster from simplyblock_core.models.iface import IFace from simplyblock_core.models.storage_node import StorageNode +from tests._mocks import route_fences # --------------------------------------------------------------------------- # Helpers @@ -156,8 +157,8 @@ def _primary_for(node_id): return [primary] if node_id == secondary.get_id() else [] db.get_primary_storage_nodes_by_secondary_node_id.side_effect = _primary_for - # port_block.set_port: record block/allow calls. (Port blocking moved - # off the directly-imported FirewallClient onto port_block.set_port.) + # Fences (nvmf_port_block / nvmf_port_unblock on node.rpc_client): + # record block/allow calls per node. fw_calls = [] def _fake_set_port(node, port, block, is_reject=False, timeout=5, retry=2): @@ -202,7 +203,6 @@ def _port_allowed(node, port): patches = [ patch.object(cluster_ops, "db_controller", db), patch.object(cluster_ops, "DBController", return_value=db), - patch("simplyblock_core.utils.port_block.set_port", _fake_set_port), patch.object(cluster_ops.tcp_ports_events, "port_deny", _port_deny), patch.object(cluster_ops.tcp_ports_events, "port_allowed", _port_allowed), patch.object(cluster_ops.tasks_controller, "add_port_allow_task", @@ -236,6 +236,7 @@ def _port_allowed(node, port): n.create_secondary_hublvol = MagicMock(return_value=True) n.connect_to_hublvol = MagicMock(return_value=True) n.client = MagicMock() + route_fences((primary, secondary), _fake_set_port) # Primary's per-LVS port lookup — already populated in _node(). # Make is_qos_set on the cluster return False so the QOS branch is skipped. cluster.is_qos_set = lambda: False @@ -363,7 +364,7 @@ class TestSuspendIsDeprecatedNoop(unittest.TestCase): (Loop 1 device-unavailable + Loop 2 detach-remote-controllers). Here we just verify that `suspend_storage_node` is a noop - (returns True without invoking FirewallClient). The detailed + (returns True without fencing any port). The detailed coverage for the new shutdown flow lives in tests/test_shutdown_no_suspension.py. """ @@ -373,8 +374,10 @@ def test_suspend_is_noop_and_does_not_touch_firewall(self): snode = _node("node-A", lvstore="LVS_A", status=StorageNode.STATUS_ONLINE) - with patch.object(storage_node_ops, "port_block") as pb, \ - patch.object(storage_node_ops, "set_node_status"), \ + snode.rpc_client = MagicMock() + fence = MagicMock() + route_fences([snode], fence) + with patch.object(storage_node_ops, "set_node_status"), \ patch.object(storage_node_ops, "DBController") as _db: # The callee does DBController().get_storage_node_by_id(...), so the # node has to hang off the INSTANCE mock. Stubbing the class mock @@ -384,20 +387,22 @@ def test_suspend_is_noop_and_does_not_touch_firewall(self): _db.return_value.get_storage_node_by_id.return_value = snode self.assertTrue( storage_node_ops.suspend_storage_node(snode.get_id())) - pb.set_port.assert_not_called() + fence.assert_not_called() def test_resume_is_noop_and_does_not_touch_firewall(self): from simplyblock_core import storage_node_ops snode = _node("node-A", lvstore="LVS_A", status=StorageNode.STATUS_SUSPENDED) - with patch.object(storage_node_ops, "port_block") as pb, \ - patch.object(storage_node_ops, "set_node_status"), \ + snode.rpc_client = MagicMock() + fence = MagicMock() + route_fences([snode], fence) + with patch.object(storage_node_ops, "set_node_status"), \ patch.object(storage_node_ops, "DBController") as _db: _db.return_value.get_storage_node_by_id.return_value = snode self.assertTrue( storage_node_ops.resume_storage_node(snode.get_id())) - pb.set_port.assert_not_called() + fence.assert_not_called() if __name__ == "__main__": diff --git a/tests/integration/test_activation_ana_inaccessible.py b/tests/integration/test_activation_ana_inaccessible.py index 1ec89fa5da..d634c17148 100644 --- a/tests/integration/test_activation_ana_inaccessible.py +++ b/tests/integration/test_activation_ana_inaccessible.py @@ -191,7 +191,6 @@ def _set_status(cid, status): patches = [ patch.object(cluster_ops, "db_controller", db), patch.object(cluster_ops, "DBController", return_value=db), - patch("simplyblock_core.utils.port_block.set_port", lambda *a, **k: None), patch.object(cluster_ops.tcp_ports_events, "port_deny", lambda *a, **k: None), patch.object(cluster_ops.tcp_ports_events, "port_allowed", lambda *a, **k: None), patch.object(cluster_ops.tasks_controller, "add_port_allow_task", lambda *a, **k: None), diff --git a/tests/integration/test_dual_ft_e2e.py b/tests/integration/test_dual_ft_e2e.py index 8e577b6fe2..2d129866fc 100644 --- a/tests/integration/test_dual_ft_e2e.py +++ b/tests/integration/test_dual_ft_e2e.py @@ -815,8 +815,9 @@ def _patch_externals(): return_value=(True, None)), patch('simplyblock_core.controllers.health_controller._check_node_api', return_value=True), # Firewall always succeeds - patch('simplyblock_core.utils.port_block.set_port', return_value=True), - patch('simplyblock_core.utils.port_block.get_blocked_ports_set', + patch('simplyblock_core.rpc_client.RPCClient.nvmf_port_block', return_value=True), + patch('simplyblock_core.rpc_client.RPCClient.nvmf_port_unblock', return_value=True), + patch('simplyblock_core.rpc_client.RPCClient.nvmf_get_blocked_ports', return_value=set()), # Port check always passes patch('simplyblock_core.controllers.health_controller.check_port_on_node', diff --git a/tests/integration/test_dual_ft_secondary_fixes.py b/tests/integration/test_dual_ft_secondary_fixes.py index 0a7ee5ae0d..b30beb967f 100644 --- a/tests/integration/test_dual_ft_secondary_fixes.py +++ b/tests/integration/test_dual_ft_secondary_fixes.py @@ -17,7 +17,7 @@ from simplyblock_core.models.iface import IFace from simplyblock_core.models.lvol_model import LVol from simplyblock_core.models.storage_node import StorageNode -from tests._mocks import assert_hublvol_wired, unique_ip +from tests._mocks import assert_hublvol_wired, route_fences, unique_ip # --------------------------------------------------------------------------- # Helpers @@ -223,16 +223,16 @@ def _build_4node_cluster(self): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_both_secondaries_get_firewall_blocked( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_storage_events, mock_tcp_events, + mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """Both sec1 and sec2 should have their ports blocked during primary restart.""" + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore nodes = self._build_4node_cluster() @@ -281,6 +281,7 @@ def make_fw(node, port, block, is_reject=False, timeout=5, retry=2): n.recreate_hublvol = MagicMock() n.connect_to_hublvol = MagicMock() n.write_to_db = MagicMock() + route_fences(nodes.values(), mock_fw_cls) mock_recreate_on_non_leader.return_value = True mock_health.check_bdev.return_value = True @@ -311,16 +312,16 @@ def make_fw(node, port, block, is_reject=False, timeout=5, retry=2): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_both_secondaries_get_hublvol_connection( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_storage_events, mock_tcp_events, + mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """Both secondaries should connect to hublvol after primary restart.""" + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore nodes = self._build_4node_cluster() @@ -362,6 +363,7 @@ def get_node(nid): # HublvolReconnectCoordinator. n.add_hublvol_failover_path = MagicMock(return_value=True) n.write_to_db = MagicMock() + route_fences(nodes.values(), mock_fw_cls) mock_recreate_on_non_leader.return_value = True mock_health.check_bdev.return_value = True @@ -391,16 +393,16 @@ def get_node(nid): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_both_secondaries_get_lvstore_status_ready( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_storage_events, mock_tcp_events, + mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """Both online secondaries should get lvstore_status='ready' at the end.""" + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore nodes = self._build_4node_cluster() @@ -453,6 +455,7 @@ def get_node(nid): n.wait_for_jm_rep_tasks_to_finish = MagicMock(return_value=True) n.recreate_hublvol = MagicMock() n.connect_to_hublvol = MagicMock() + route_fences(nodes.values(), mock_fw_cls) mock_recreate_on_non_leader.return_value = True mock_health.check_bdev.return_value = True @@ -471,17 +474,17 @@ def get_node(nid): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_offline_secondary2_skipped_gracefully( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_storage_events, mock_tcp_events, + mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_recreate_on_non_leader, _mock_phase, _mock_handle): """If tertiary_node_id is offline, it should be skipped for firewall/hublvol but not crash.""" + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore nodes = self._build_4node_cluster() @@ -524,6 +527,7 @@ def get_node(nid): n.connect_to_hublvol = MagicMock() n.create_secondary_hublvol = MagicMock() n.write_to_db = MagicMock() + route_fences(nodes.values(), mock_fw_cls) mock_recreate_on_non_leader.return_value = True mock_health.check_bdev.return_value = True @@ -550,16 +554,16 @@ def _disc_side_effect(peer, **kwargs): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_suspend_when_any_secondary_unreachable( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_storage_events, mock_tcp_events, + mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """If any secondary is UNREACHABLE, primary should be suspended.""" + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore nodes = self._build_4node_cluster() @@ -602,6 +606,7 @@ def get_node(nid): n.recreate_hublvol = MagicMock() n.connect_to_hublvol = MagicMock() n.write_to_db = MagicMock() + route_fences(nodes.values(), mock_fw_cls) mock_recreate_on_non_leader.return_value = True mock_health.check_bdev.return_value = True diff --git a/tests/integration/test_failover_failback_combinations.py b/tests/integration/test_failover_failback_combinations.py index 066df33fac..9aed18c9ed 100644 --- a/tests/integration/test_failover_failback_combinations.py +++ b/tests/integration/test_failover_failback_combinations.py @@ -28,7 +28,7 @@ from simplyblock_core.models.iface import IFace from simplyblock_core.models.lvol_model import LVol from simplyblock_core.models.storage_node import StorageNode -from tests._mocks import unique_ip +from tests._mocks import route_fences, unique_ip # --------------------------------------------------------------------------- # Helpers @@ -135,13 +135,13 @@ def _mock_rpc(): def _mock_fw_factory(): - """A ``port_block.set_port`` replacement that tracks per-node spies. + """A fence callback (see ``tests._mocks.route_fences``) that tracks + per-node spies. - Port blocking moved off the directly-imported ``FirewallClient`` onto - ``port_block.set_port(node, port, block=...)`` (RPC-first with iptables - fallback). To keep these tests' intent ("a block/allow landed on node - X") and their assertions intact, this shim lazily creates one spy per - target node and records each call in the *legacy* shape + Fences are ``nvmf_port_block`` / ``nvmf_port_unblock`` on + ``node.rpc_client(...)``. To keep these tests' intent ("a block/allow + landed on node X") and their assertions intact, this shim lazily creates + one spy per target node and records each call in the *legacy* shape ``firewall_set_port(port, "tcp", action, rpc_port)`` where ``action`` is "block"/"allow". Returns ``(set_port_impl, fw_instances)``.""" instances = [] @@ -163,10 +163,11 @@ def make_fw(node, port, block, is_reject=False, timeout=5, retry=2): def _set_port_to_fw(fw): - """Return a ``port_block.set_port`` replacement that routes every call - into a single ``fw.firewall_set_port(port, "tcp", action, rpc_port)`` - spy, so side_effects (raising) and call_count assertions on that one - spy keep working after the port_block migration.""" + """Return a fence callback (see ``tests._mocks.route_fences``) that + routes every call into a single + ``fw.firewall_set_port(port, "tcp", action, rpc_port)`` spy, so + side_effects (raising) and call_count assertions on that one spy keep + working.""" def _impl(node, port, block, is_reject=False, timeout=5, retry=2): action = "block" if block else "allow" return fw.firewall_set_port(port, "tcp", action, getattr(node, "rpc_port", None)) @@ -419,19 +420,6 @@ def test_ftt2_failback_first_sec_restarts_second_sec_offline(self): # recreate_lvstore Tests (primary failback) # =========================================================================== -_RECREATE_PATCHES = [ - "simplyblock_core.storage_node_ops.recreate_lvstore_on_non_leader", - "simplyblock_core.storage_node_ops.health_controller", - "simplyblock_core.storage_node_ops.tcp_ports_events", - "simplyblock_core.storage_node_ops.storage_events", - "simplyblock_core.storage_node_ops.tasks_controller", - "simplyblock_core.utils.port_block.set_port", - "simplyblock_core.models.storage_node.RPCClient", - "simplyblock_core.storage_node_ops._connect_to_remote_jm_devs", - "simplyblock_core.storage_node_ops._create_bdev_stack", - "simplyblock_core.storage_node_ops.DBController", -] - class TestRecreateLvstoreFTT1(unittest.TestCase): """FTT=1: recreate_lvstore on primary restart with single secondary.""" @@ -440,21 +428,21 @@ class TestRecreateLvstoreFTT1(unittest.TestCase): side_effect=lambda peer, **kw: peer.status in ["offline"]) @patch("simplyblock_core.storage_node_ops._set_restart_phase") @patch("simplyblock_core.storage_node_ops._handle_rpc_failure_on_peer", return_value="skip") - @patch(*_RECREATE_PATCHES[:1]) - @patch(*_RECREATE_PATCHES[1:2]) - @patch(*_RECREATE_PATCHES[2:3]) - @patch(*_RECREATE_PATCHES[3:4]) - @patch(*_RECREATE_PATCHES[4:5]) - @patch(*_RECREATE_PATCHES[5:6]) - @patch(*_RECREATE_PATCHES[6:7]) - @patch(*_RECREATE_PATCHES[7:8]) - @patch(*_RECREATE_PATCHES[8:9]) - @patch(*_RECREATE_PATCHES[9:10]) + @patch("simplyblock_core.storage_node_ops.recreate_lvstore_on_non_leader") + @patch("simplyblock_core.storage_node_ops.health_controller") + @patch("simplyblock_core.storage_node_ops.tcp_ports_events") + @patch("simplyblock_core.storage_node_ops.storage_events") + @patch("simplyblock_core.storage_node_ops.tasks_controller") + @patch("simplyblock_core.models.storage_node.RPCClient") + @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") + @patch("simplyblock_core.storage_node_ops._create_bdev_stack") + @patch("simplyblock_core.storage_node_ops.DBController") def test_ftt1_failback_blocks_and_drops_leadership_on_secondary( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, mock_health, mock_recreate_on_non_leader, _mock_handle, _mock_phase, _mock_disc): + mock_fw_cls = MagicMock() nodes = _build_ftt1_nodes() db = _make_db_mock(nodes) mock_db_cls.return_value = db @@ -466,6 +454,7 @@ def test_ftt1_failback_blocks_and_drops_leadership_on_secondary( make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) from simplyblock_core.storage_node_ops import recreate_lvstore snode = nodes["node-1"] @@ -504,15 +493,15 @@ class TestRecreateLvstoreFTT2(unittest.TestCase): @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tasks_controller") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_ftt2_failback_blocks_both_secondaries( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): + mock_fw_cls = MagicMock() nodes = _build_ftt2_nodes() db = _make_db_mock(nodes) mock_db_cls.return_value = db @@ -524,6 +513,7 @@ def test_ftt2_failback_blocks_both_secondaries( make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) from simplyblock_core.storage_node_ops import recreate_lvstore snode = nodes["node-1"] @@ -551,16 +541,16 @@ def test_ftt2_failback_blocks_both_secondaries( @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tasks_controller") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_ftt2_failback_second_sec_offline_skipped( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """Primary restarts, second secondary offline → only first sec processed.""" + mock_fw_cls = MagicMock() nodes = _build_ftt2_nodes() nodes["node-3"].status = StorageNode.STATUS_OFFLINE db = _make_db_mock(nodes) @@ -573,6 +563,7 @@ def test_ftt2_failback_second_sec_offline_skipped( make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) from simplyblock_core.storage_node_ops import recreate_lvstore result = recreate_lvstore(nodes["node-1"]) @@ -602,15 +593,15 @@ class TestRecreateLvstoreOnSecPrimaryOnline(unittest.TestCase): @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tasks_controller") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_primary_online_port_blocked_drain_io_no_leadership_drop( self, mock_db_cls, mock_create_bdev, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): """When primary is online, recreate_lvstore_on_non_leader must: block port, drain inflight IO, examine, then allow port. Leadership must NOT be dropped.""" + mock_fw_cls = MagicMock() nodes = _build_ftt2_nodes() # node-2 is the secondary being rebuilt; node-1 is its primary (online) secondary = nodes["node-2"] @@ -627,6 +618,7 @@ def test_primary_online_port_blocked_drain_io_no_leadership_drop( make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader result = recreate_lvstore_on_non_leader(secondary, leader_node=primary, primary_node=primary) @@ -663,7 +655,6 @@ class TestRecreateLvstoreOnSecPrimaryOffline(unittest.TestCase): @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tasks_controller") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") @@ -673,10 +664,11 @@ class TestRecreateLvstoreOnSecPrimaryOffline(unittest.TestCase): return_value=True) def test_primary_offline_first_sec_restarts_drops_leadership_on_second_sec( self, mock_quorum, mock_snode_client, mock_health, mock_db_cls, mock_create_bdev, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, mock_storage_events, + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): """Primary offline, first sec restarts → must drop leadership on second sec to prevent writer conflict when JC connects to remote JMs.""" + mock_fw_cls = MagicMock() nodes = _build_ftt2_nodes() nodes["node-1"].status = StorageNode.STATUS_OFFLINE # primary offline secondary = nodes["node-2"] # first secondary, restarting @@ -695,6 +687,7 @@ def test_primary_offline_first_sec_restarts_drops_leadership_on_second_sec( make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader result = recreate_lvstore_on_non_leader(secondary, leader_node=leader, primary_node=primary) @@ -729,7 +722,6 @@ def test_primary_offline_first_sec_restarts_drops_leadership_on_second_sec( @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tasks_controller") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") @@ -737,10 +729,11 @@ def test_primary_offline_first_sec_restarts_drops_leadership_on_second_sec( @patch("simplyblock_core.storage_node_ops.SNodeClient") def test_primary_offline_second_sec_also_offline_no_failback_for_that_group( self, mock_snode_client, mock_health, mock_db_cls, mock_create_bdev, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): """Primary offline, second sec also offline → no port block for THAT group. (Node may still get failback calls for other groups it's secondary for.)""" # Use a minimal 3-node topology where node-2 is ONLY secondary for node-1 + mock_fw_cls = MagicMock() nodes = { "node-1": _node("node-1", lvstore="LVS_100", jm_vuid=100, status=StorageNode.STATUS_OFFLINE, @@ -771,6 +764,7 @@ def test_primary_offline_second_sec_also_offline_no_failback_for_that_group( make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader primary = nodes["node-1"] @@ -794,15 +788,15 @@ def test_primary_offline_second_sec_also_offline_no_failback_for_that_group( @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tasks_controller") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_second_sec_restarts_primary_offline_no_failback_on_first_sec_for_that_group( self, mock_db_cls, mock_create_bdev, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): """Second secondary restarts, primary offline → sibling (first sec) gets port blocked unconditionally. Uses minimal topology where node-3 is ONLY secondary for node-1.""" + mock_fw_cls = MagicMock() nodes = { "node-1": _node("node-1", lvstore="LVS_100", jm_vuid=100, status=StorageNode.STATUS_OFFLINE, @@ -834,6 +828,7 @@ def test_second_sec_restarts_primary_offline_no_failback_on_first_sec_for_that_g make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader result = recreate_lvstore_on_non_leader(secondary, leader_node=leader, primary_node=primary) @@ -857,7 +852,6 @@ class TestRecreateLvstoreOnSecANAFailback(unittest.TestCase): @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tasks_controller") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") @@ -865,8 +859,9 @@ class TestRecreateLvstoreOnSecANAFailback(unittest.TestCase): @patch("simplyblock_core.storage_node_ops.SNodeClient") def test_no_ana_failback_on_sec2_when_primary_offline( self, mock_snode_client, mock_health, mock_db_cls, mock_create_bdev, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): """sec_2 is always non_optimized — no ANA failback to inaccessible needed.""" + mock_fw_cls = MagicMock() nodes = _build_ftt2_nodes() nodes["node-1"].status = StorageNode.STATUS_OFFLINE secondary = nodes["node-2"] # first secondary restarting @@ -883,6 +878,7 @@ def test_no_ana_failback_on_sec2_when_primary_offline( make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader primary = nodes["node-1"] @@ -913,20 +909,20 @@ class TestSequentialFailbackScenario(unittest.TestCase): @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tasks_controller") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_primary_failback_then_second_sec_restart( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_tasks, mock_tcp_events, + mock_rpc_cls, mock_tasks, mock_tcp_events, mock_storage_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """ 1. Primary restarts with second sec offline → failback from first sec only 2. Then second sec comes online → recreate_lvstore_on_non_leader(second_sec, ...) Both operations should succeed without conflicts. """ + mock_fw_cls = MagicMock() nodes = _build_ftt2_nodes() nodes["node-3"].status = StorageNode.STATUS_OFFLINE # sec2 offline initially @@ -940,6 +936,7 @@ def test_primary_failback_then_second_sec_restart( make_fw, fw_instances = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) # Step 1: Primary restarts (second sec offline) from simplyblock_core.storage_node_ops import recreate_lvstore @@ -1004,7 +1001,7 @@ def test_failback_sits_behind_the_gates_and_before_unblock(self): i_own_gate = src.find("_reconnect_own_sec_tert_hublvols(node)") i_failback = src.find( "failback_ok, failback_msg = _failback_leadership_to_primary(") - i_unblock = src.find("port_block.set_port(node, p, block=False") + i_unblock = src.find("node.rpc_client(timeout=5, retry=2).nvmf_port_unblock(p)") self.assertGreater(i_own_gate, 0, "own-hublvol gate must exist") self.assertGreater(i_peer_gate, 0, "peer hublvol gate must exist") self.assertGreater( @@ -1019,7 +1016,7 @@ def test_demote_is_minimal_no_blocks_no_drain(self): src = self._read_source() fn = self._failback_fn(src) self.assertNotIn( - "block=True", fn, + ".nvmf_port_block(", fn, "the failback must NOT port-block the acting leader or peers — " "LVS ports carry hublvol/redirect and journal traffic (2026-07-06)") self.assertNotIn( @@ -1158,6 +1155,7 @@ def _setup(self, mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, make_fw, _ = _mock_fw_factory() mock_fw_cls.side_effect = make_fw _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) return secondary, primary, rpc @patch("simplyblock_core.storage_node_ops._check_peer_disconnected", @@ -1169,17 +1167,17 @@ def _setup(self, mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, @patch("simplyblock_core.storage_node_ops.tasks_controller") @patch("simplyblock_core.storage_node_ops.set_node_status") @patch("simplyblock_core.storage_node_ops.SNodeClient") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_missing_lvol_bdev_aborts_when_not_force( - self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_fw_cls, + self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_snode_client, mock_set_status, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): """Without force=True, restart must abort when an expected lvol bdev isn't in the SPDK bdev registry after examine.""" # Empty bdev registry — the expected LVS_100/bdev_test isn't there + mock_fw_cls = MagicMock() secondary, primary, _rpc = self._setup( mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, present_bdevs=[], @@ -1204,16 +1202,16 @@ def test_missing_lvol_bdev_aborts_when_not_force( @patch("simplyblock_core.storage_node_ops.tasks_controller") @patch("simplyblock_core.storage_node_ops.set_node_status") @patch("simplyblock_core.storage_node_ops.SNodeClient") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_missing_lvol_bdev_succeeds_with_force( - self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_fw_cls, + self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_snode_client, mock_set_status, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): """With force=True, restart proceeds even when lvol bdev is missing (operator explicitly accepts that the peer won't serve it).""" + mock_fw_cls = MagicMock() secondary, primary, _rpc = self._setup( mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, present_bdevs=[], @@ -1232,16 +1230,16 @@ def test_missing_lvol_bdev_succeeds_with_force( @patch("simplyblock_core.storage_node_ops.tasks_controller") @patch("simplyblock_core.storage_node_ops.set_node_status") @patch("simplyblock_core.storage_node_ops.SNodeClient") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_lvol_present_passes( - self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_fw_cls, + self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_snode_client, mock_set_status, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle): """Happy path: expected lvol bdev is in the registry, restart succeeds without force.""" + mock_fw_cls = MagicMock() secondary, primary, _rpc = self._setup( mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, present_bdevs=["LVS_100/bdev_test"], @@ -1279,7 +1277,7 @@ def _setup(self, mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, mock_create_bdev.return_value = (True, None) # Single firewall spy whose firewall_set_port raises as directed. - # port_block.set_port is routed into it so call_count / side_effect + # Every fence is routed into it so call_count / side_effect # assertions on the retry-then-abort path keep working. fw = MagicMock() if fw_set_port_side_effect is not None: @@ -1287,6 +1285,7 @@ def _setup(self, mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, mock_fw_cls.side_effect = _set_port_to_fw(fw) _setup_node_methods(nodes, rpc) + route_fences(nodes.values(), mock_fw_cls) return secondary, primary, rpc, fw @patch("simplyblock_core.storage_node_ops.time.sleep", return_value=None) @@ -1299,12 +1298,11 @@ def _setup(self, mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, @patch("simplyblock_core.storage_node_ops.tasks_controller") @patch("simplyblock_core.storage_node_ops.set_node_status") @patch("simplyblock_core.storage_node_ops.SNodeClient") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_block_fails_all_attempts_aborts_without_force( - self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_fw_cls, + self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_snode_client, mock_set_status, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle, _mock_sleep): """All port-block attempts fail → restart aborts, leader kept @@ -1318,6 +1316,7 @@ def test_block_fails_all_attempts_aborts_without_force( before we ever reach this code; the reduced budget protects the remaining fabric-partition-while-mgmt-reachable case. """ + mock_fw_cls = MagicMock() secondary, primary, _rpc, fw = self._setup( mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, fw_set_port_side_effect=ConnectionRefusedError("Connection refused"), @@ -1344,17 +1343,17 @@ def test_block_fails_all_attempts_aborts_without_force( @patch("simplyblock_core.storage_node_ops.tasks_controller") @patch("simplyblock_core.storage_node_ops.set_node_status") @patch("simplyblock_core.storage_node_ops.SNodeClient") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_block_fails_all_attempts_proceeds_with_force( - self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_fw_cls, + self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_snode_client, mock_set_status, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle, _mock_sleep): """With force=True, all failed block attempts don't abort — restart proceeds despite the race risk (operator explicit choice). Attempt count was tightened from 5 to 3 (PR #996).""" + mock_fw_cls = MagicMock() secondary, primary, _rpc, fw = self._setup( mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, fw_set_port_side_effect=ConnectionRefusedError("Connection refused"), @@ -1378,17 +1377,17 @@ def test_block_fails_all_attempts_proceeds_with_force( @patch("simplyblock_core.storage_node_ops.tasks_controller") @patch("simplyblock_core.storage_node_ops.set_node_status") @patch("simplyblock_core.storage_node_ops.SNodeClient") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_block_succeeds_on_retry( - self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_fw_cls, + self, mock_db_cls, mock_create_bdev, mock_rpc_cls, mock_snode_client, mock_set_status, mock_tasks, mock_tcp_events, mock_storage_events, _mock_disc, _mock_phase, _mock_handle, _mock_sleep): """Transient ConnectionRefused — first 2 attempts fail, 3rd succeeds. Restart must keep going (no abort, no force needed).""" # side_effect: raise twice, then succeed (return None = call succeeds) + mock_fw_cls = MagicMock() secondary, primary, _rpc, fw = self._setup( mock_rpc_cls, mock_db_cls, mock_create_bdev, mock_fw_cls, fw_set_port_side_effect=[ @@ -1440,6 +1439,8 @@ def _run(self, jc_disable_side=None, jc_disable_return=True): rpc.jc_disable_replication.return_value = jc_disable_return _setup_node_methods(nodes, rpc) + make_fw, fw_instances = _mock_fw_factory() + route_fences(nodes.values(), make_fw) # SPDK-kill sink for the abort path. spdk_process_is_up returns a # (up, err) tuple — report down so _kill_spdk_until_dead returns fast. @@ -1449,8 +1450,6 @@ def _run(self, jc_disable_side=None, jc_disable_return=True): for n in nodes.values(): n.client = MagicMock(return_value=snode_api) - make_fw, fw_instances = _mock_fw_factory() - patches = [ patch("simplyblock_core.storage_node_ops.DBController", return_value=db), patch("simplyblock_core.storage_node_ops._create_bdev_stack", return_value=(True, None)), @@ -1466,7 +1465,6 @@ def _run(self, jc_disable_side=None, jc_disable_return=True): side_effect=lambda peer, **kw: peer.status in ["offline"]), patch("simplyblock_core.storage_node_ops.time.sleep", return_value=None), patch("simplyblock_core.models.storage_node.RPCClient", return_value=rpc), - patch("simplyblock_core.utils.port_block.set_port", side_effect=make_fw), ] for p in patches: p.start() diff --git a/tests/integration/test_lvs_role_assignment.py b/tests/integration/test_lvs_role_assignment.py index df7dcdfb77..f3998f67aa 100644 --- a/tests/integration/test_lvs_role_assignment.py +++ b/tests/integration/test_lvs_role_assignment.py @@ -18,7 +18,7 @@ from simplyblock_core.models.hublvol import HubLVol from simplyblock_core.models.iface import IFace from simplyblock_core.models.storage_node import StorageNode -from tests._mocks import assert_hublvol_wired, unique_ip +from tests._mocks import assert_hublvol_wired, route_fences, unique_ip # --------------------------------------------------------------------------- # Helpers (shared with test_dual_ft_secondary_fixes.py) @@ -271,15 +271,15 @@ def _build_cluster(self): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_primary_gets_primary_role( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_storage_events, mock_tcp_events, + mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore nodes = self._build_cluster() @@ -315,6 +315,7 @@ def get_node(nid): n.connect_to_hublvol = MagicMock() n.create_secondary_hublvol = MagicMock() n.write_to_db = MagicMock() + route_fences(nodes.values(), mock_fw_cls) mock_recreate_on_non_leader.return_value = True mock_health.check_bdev.return_value = True @@ -339,16 +340,16 @@ def get_node(nid): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_sec1_secondary_sec2_tertiary( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_storage_events, mock_tcp_events, + mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """sec1 gets role='secondary', sec2 gets role='tertiary'.""" + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore nodes = self._build_cluster() @@ -388,6 +389,7 @@ def get_node(nid): # HublvolReconnectCoordinator. n.add_hublvol_failover_path = MagicMock(return_value=True) n.write_to_db = MagicMock() + route_fences(nodes.values(), mock_fw_cls) mock_recreate_on_non_leader.return_value = True mock_health.check_bdev.return_value = True @@ -416,16 +418,16 @@ def get_node(nid): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_single_secondary_gets_secondary_role( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_storage_events, mock_tcp_events, + mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """With only one secondary (FTT=1), it should get role='secondary'.""" + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore nodes = self._build_cluster() @@ -463,6 +465,7 @@ def get_node(nid): n.connect_to_hublvol = MagicMock() n.create_secondary_hublvol = MagicMock() n.write_to_db = MagicMock() + route_fences(nodes.values(), mock_fw_cls) mock_recreate_on_non_leader.return_value = True mock_health.check_bdev.return_value = True @@ -509,14 +512,14 @@ def _build_nodes(self): @patch("simplyblock_core.storage_node_ops._set_restart_phase") @patch("simplyblock_core.storage_node_ops._handle_rpc_failure_on_peer", return_value="skip") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_first_secondary_gets_secondary_role( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_tcp_events, _mock_disc, _mock_phase, _mock_handle): + mock_rpc_cls, mock_tcp_events, _mock_disc, _mock_phase, _mock_handle): + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader nodes = self._build_nodes() @@ -552,6 +555,7 @@ def get_node(nid): n.create_secondary_hublvol = MagicMock() n.write_to_db = MagicMock() n.wait_for_jm_rep_tasks_to_finish = MagicMock(return_value=True) + route_fences(nodes.values(), mock_fw_cls) recreate_lvstore_on_non_leader(sec1, leader_node=primary, primary_node=primary) @@ -563,14 +567,14 @@ def get_node(nid): @patch("simplyblock_core.storage_node_ops._set_restart_phase") @patch("simplyblock_core.storage_node_ops._handle_rpc_failure_on_peer", return_value="skip") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.DBController") def test_second_secondary_gets_tertiary_role( self, mock_db_cls, mock_create_bdev, mock_connect_jm, - mock_rpc_cls, mock_fw_cls, mock_tcp_events, _mock_disc, _mock_phase, _mock_handle): + mock_rpc_cls, mock_tcp_events, _mock_disc, _mock_phase, _mock_handle): + mock_fw_cls = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader nodes = self._build_nodes() @@ -606,6 +610,7 @@ def get_node(nid): n.create_secondary_hublvol = MagicMock() n.write_to_db = MagicMock() n.wait_for_jm_rep_tasks_to_finish = MagicMock(return_value=True) + route_fences(nodes.values(), mock_fw_cls) recreate_lvstore_on_non_leader(sec2, leader_node=primary, primary_node=primary) diff --git a/tests/integration/test_nvmeof_security.py b/tests/integration/test_nvmeof_security.py index c26f183e46..f8f7b44088 100644 --- a/tests/integration/test_nvmeof_security.py +++ b/tests/integration/test_nvmeof_security.py @@ -1006,14 +1006,13 @@ class TestRecreateSubsystemSecurity(unittest.TestCase): @patch("simplyblock_core.storage_node_ops._reapply_allowed_hosts") @patch("simplyblock_core.storage_node_ops.add_lvol_thread") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.storage_node_ops._create_bdev_stack") @patch("simplyblock_core.storage_node_ops.tasks_controller") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops.DBController") def test_recreate_lvstore_on_non_leader_passes_allow_any_false( self, MockDB, MockRPC, mock_tasks, mock_bdev_stack, - MockFW, mock_tcp_events, mock_add_thread, mock_reapply, _mock_disc, _mock_phase, _mock_handle): + mock_tcp_events, mock_add_thread, mock_reapply, _mock_disc, _mock_phase, _mock_handle): """recreate_lvstore_on_non_leader sets allow_any_host=False for lvols with allowed_hosts.""" dhchap_host = {"nqn": "nqn:secured", "dhchap_key": "DHHC-1:01:x:"} @@ -1082,8 +1081,6 @@ def test_recreate_lvstore_on_non_leader_passes_allow_any_false( with patch.object(sec_node, 'connect_to_hublvol'): with patch.object(primary_node, 'write_to_db'), patch.object(sec_node, 'write_to_db'): mock_bdev_stack.return_value = (True, None) - mock_fw_inst = MagicMock() - MockFW.return_value = mock_fw_inst snode_ops.recreate_lvstore_on_non_leader(sec_node, leader_node=primary_node, primary_node=primary_node) diff --git a/tests/integration/test_port_allow_recovery.py b/tests/integration/test_port_allow_recovery.py index 11a34eec3b..f5bd326989 100644 --- a/tests/integration/test_port_allow_recovery.py +++ b/tests/integration/test_port_allow_recovery.py @@ -10,7 +10,7 @@ port-blocked the acting leader during failback; blocking LVS ports mid-IO severed redirect chains (DISTRIBD n_unavail_read) and JC paths (history_append n_success=0 -> node abort). The runner's ONLY - ``port_block.set_port`` call is the final unblock of the recovering + fence call is the final ``nvmf_port_unblock`` of the recovering node's own port. 2. **Device-state refresh before unblock** (replaces the old full @@ -63,6 +63,7 @@ from simplyblock_core.models.nvme_device import NVMeDevice from simplyblock_core.models.storage_node import StorageNode from simplyblock_core.services.task_runner_base import TaskAbort, TaskDefer +from tests._mocks import route_fences def _make_lvol(uuid, status=None): @@ -172,13 +173,15 @@ def _side(*a, **kw): return _side getattr(rpc, method).side_effect = _make_side() - # port_block.set_port spy. Record each call in the legacy - # ("firewall_set_port", target_uuid, port, action) shape so the + # Fence spy. Record each nvmf_port_block / nvmf_port_unblock in the + # legacy ("firewall_set_port", target_uuid, port, action) shape so the # assertions below keep working unchanged. def _set_port_side_effect(node, port, block, *a, **kw): action = "block" if block else "allow" self.calls.append(("firewall_set_port", node.uuid, port, action)) return True + self._fence = _set_port_side_effect + route_fences([self.node, self.sec], self._fence) self.db_mock = MagicMock( get_task_by_id=MagicMock(return_value=self.task), @@ -257,10 +260,6 @@ def _check_sec_hublvol_side(peer_node, auto_fix=True, primary_node_id=None): "simplyblock_core.services.tasks_runner_port_allow.tasks_controller.get_active_node_restart_task", return_value=None, ), - patch( - "simplyblock_core.utils.port_block.set_port", - side_effect=_set_port_side_effect, - ), patch( "simplyblock_core.services.tasks_runner_port_allow.tcp_ports_events.port_allowed", side_effect=lambda n, p: self.calls.append(("port_allowed", n.uuid, p)), @@ -664,6 +663,7 @@ def setUp(self): self.node.tertiary_node_id = self.tert.uuid self.tert_rpc = MagicMock(name="tert_rpc") self.tert.rpc_client = MagicMock(return_value=self.tert_rpc) + route_fences([self.tert], self._fence) self.tert.wait_for_jm_rep_tasks_to_finish = MagicMock(return_value=True) self.tert.connect_to_hublvol = MagicMock(return_value=True) self.tert.get_lvol_subsys_port = MagicMock(return_value=self.port) @@ -787,7 +787,7 @@ def test_no_port_is_ever_blocked(self): if c[0] == "firewall_set_port" and c[3] == "block"] self.assertEqual( blocks, [], - "the runner must NEVER call port_block.set_port with block=True — " + "the runner must NEVER call nvmf_port_block — " "on any node, any port, any phase. LVS ports carry hublvol/" "redirect and journal traffic; blocking them mid-IO produced " "DISTRIBD n_unavail_read errors and JC history_append failures " @@ -828,7 +828,7 @@ def test_failback_present_and_ordered(self): i_failback_call = self.src.find( "failback_ok, failback_msg = _failback_leadership_to_primary(") i_unblock = self.src.find( - "port_block.set_port(node, p, block=False") + "node.rpc_client(timeout=5, retry=2).nvmf_port_unblock(p)") self.assertGreater(i_gate, 0) self.assertGreater(i_failback_call, i_gate, "failback must run only after the peer hublvol gate") @@ -840,7 +840,7 @@ def test_failback_is_minimal(self): # poll, no bs_nonleadership / force_to_non_leader — exactly one # plain demote of the acting leader. fn = self._failback_fn() - self.assertNotIn("block=True", fn) + self.assertNotIn(".nvmf_port_block(", fn) self.assertNotIn("bdev_distrib_check_inflight_io", fn) self.assertNotIn("bs_nonleadership", fn) self.assertNotIn("bdev_distrib_force_to_non_leader", fn) @@ -863,15 +863,15 @@ def test_runner_never_takes_leadership_and_never_blocks(self): # A management-forced leader=True caused the 2026-07-06 # stale-blob-metadata corruption; _take_leadership_on_primary is # deleted. And the runner never blocks any port: exactly one - # port_block.set_port callsite, the final block=False unblock. + # fence callsite, the final nvmf_port_unblock. self.assertNotIn("def _take_leadership_on_primary", self.src) self.assertNotIn("leader=True)", self.src) - self.assertNotIn("block=True", self.src) + self.assertNotIn(".nvmf_port_block(", self.src) import re - set_port_calls = re.findall(r"port_block\.set_port\(", self.src) - self.assertEqual(len(set_port_calls), 1, - "exactly one set_port callsite: the final unblock") - self.assertIn("port_block.set_port(node, p, block=False", self.src) + fence_calls = re.findall(r"\.nvmf_port_(?:un)?block\(", self.src) + self.assertEqual(len(fence_calls), 1, + "exactly one fence callsite: the final unblock") + self.assertIn("node.rpc_client(timeout=5, retry=2).nvmf_port_unblock(p)", self.src) def test_fencing_recommit_and_map_push_are_gone(self): for gone in ( @@ -897,7 +897,7 @@ def test_deferred_ana_promotion_is_post_unblock_and_offline_gated(self): "exactly one ANA callsite in the runner: the deferred " "post-unblock promotion") i_unblock = self.src.find( - "port_block.set_port(node, p, block=False") + "node.rpc_client(timeout=5, retry=2).nvmf_port_unblock(p)") i_event = self.src.find("tcp_ports_events.port_allowed") i_gate = self.src.find("ana_primary.status == StorageNode.STATUS_OFFLINE") i_ana = self.src.find("storage_node_ops._set_lvol_ana_on_node(") @@ -919,7 +919,7 @@ def test_device_refresh_present_and_ordered(self): i_broadcast = self.src.find("distr_controller.send_dev_status_event(") i_targeted = self.src.find("target_node=node") i_unblock = self.src.find( - "port_block.set_port(node, p, block=False") + "node.rpc_client(timeout=5, retry=2).nvmf_port_unblock(p)") self.assertGreater(i_readmit, 0) self.assertGreater(i_broadcast, i_readmit, "FDB re-admit before the broadcast") @@ -1341,6 +1341,7 @@ def setUp(self): self.prim_rpc = MagicMock(name="prim_rpc") self.prim_rpc.get_lvstore.return_value = {"lvs leadership": True} self.prim.rpc_client = MagicMock(return_value=self.prim_rpc) + route_fences([self.prim], self._fence) def _get_node(self, uuid): if uuid == self.prim.uuid: diff --git a/tests/integration/test_recreate_lvstore_abort_on_peer_rpc_failure.py b/tests/integration/test_recreate_lvstore_abort_on_peer_rpc_failure.py index 00cd5c6f11..c06fcd4821 100644 --- a/tests/integration/test_recreate_lvstore_abort_on_peer_rpc_failure.py +++ b/tests/integration/test_recreate_lvstore_abort_on_peer_rpc_failure.py @@ -22,7 +22,7 @@ - replication-wait raise -> abort - jc_compression check raise -> abort -External dependencies (FDB, RPC, SPDK, FirewallClient) are mocked. +External dependencies (FDB, RPC, SPDK) are mocked. """ import unittest @@ -32,7 +32,7 @@ from simplyblock_core.models.hublvol import HubLVol from simplyblock_core.models.iface import IFace from simplyblock_core.models.storage_node import StorageNode -from tests._mocks import unique_ip +from tests._mocks import route_fences, unique_ip # -------------------------------------------------------------------------- # Helpers (kept aligned with tests/test_dual_ft_secondary_fixes.py) @@ -160,7 +160,6 @@ def _patches(self): patch("simplyblock_core.storage_node_ops.health_controller"), patch("simplyblock_core.storage_node_ops.tcp_ports_events"), patch("simplyblock_core.storage_node_ops.storage_events"), - patch("simplyblock_core.utils.port_block.set_port"), patch("simplyblock_core.models.storage_node.RPCClient"), patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs"), patch("simplyblock_core.storage_node_ops._connect_to_remote_devs"), @@ -181,13 +180,15 @@ def _enter_patches(self, patches): "health": mocks[4], "tcp_events": mocks[5], "storage_events": mocks[6], - "fw_cls": mocks[7], - "rpc_cls": mocks[8], - "connect_jm": mocks[9], - "connect_devs": mocks[10], - "create_bdev": mocks[11], - "failback_ana": mocks[12], - "db_cls": mocks[13], + "rpc_cls": mocks[7], + "connect_jm": mocks[8], + "connect_devs": mocks[9], + "create_bdev": mocks[10], + "failback_ana": mocks[11], + "db_cls": mocks[12], + # Fence spy: route_fences() in _setup_common sends every node's + # nvmf_port_block / nvmf_port_unblock here. + "fw_cls": MagicMock(), } def _setup_common(self, m): @@ -226,6 +227,7 @@ def get_node(nid): rpc.bdev_distrib_check_inflight_io.return_value = False m["rpc_cls"].return_value = rpc _attach_node_helpers(nodes, rpc) + route_fences(nodes.values(), m["fw_cls"]) # snode.client() returns the SnodeAPI used by _kill_app snode_api = _make_snode_api() @@ -238,7 +240,7 @@ def get_node(nid): # Bug repro: leader port-block FW timeout → abort, no leader promotion # ---------------------------------------------------------------------- def test_leader_port_block_fw_failure_aborts_and_does_not_promote(self): - """LVS_6609 incident: FirewallClient.firewall_set_port raises on the + """LVS_6609 incident: the port block raises on the leader. recreate_lvstore must abort (not silent-skip), so snode is NOT promoted and SPDK on snode is killed.""" from simplyblock_core import storage_node_ops @@ -247,7 +249,7 @@ def test_leader_port_block_fw_failure_aborts_and_does_not_promote(self): m = self._enter_patches(patches) nodes, rpc, snode_api = self._setup_common(m) - # FirewallClient: raises only when called against the secondary (leader) + # Fence: raises only when called against the secondary (leader) fw_calls = [] def make_fw(node, port, block, is_reject=False, timeout=5, retry=2): diff --git a/tests/integration/test_recreate_secondary_takeover_cascade.py b/tests/integration/test_recreate_secondary_takeover_cascade.py index 0efafee0c5..e1150afa4f 100644 --- a/tests/integration/test_recreate_secondary_takeover_cascade.py +++ b/tests/integration/test_recreate_secondary_takeover_cascade.py @@ -33,7 +33,7 @@ from simplyblock_core.models.hublvol import HubLVol from simplyblock_core.models.iface import IFace from simplyblock_core.models.storage_node import StorageNode -from tests._mocks import unique_ip +from tests._mocks import route_fences, unique_ip # -------------------------------------------------------------------------- # Helpers @@ -327,7 +327,6 @@ def _build_takeover_nodes(self, snode_role): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") @patch("simplyblock_core.storage_node_ops.storage_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs", return_value=[]) @patch("simplyblock_core.storage_node_ops._connect_to_remote_devs", return_value=[]) @@ -335,9 +334,10 @@ def _build_takeover_nodes(self, snode_role): @patch("simplyblock_core.storage_node_ops.DBController") def _run_takeover(self, snode_role, mock_db_cls, mock_create_bdev, mock_connect_devs, - mock_connect_jm, mock_rpc_cls, mock_fw_cls, + mock_connect_jm, mock_rpc_cls, mock_storage_events, mock_tcp_events, mock_health, mock_failback, mock_phase, mock_disc): + mock_fw_cls = MagicMock() from simplyblock_core import storage_node_ops snode, lvs_owner = self._build_takeover_nodes(snode_role) @@ -379,6 +379,7 @@ def get_node(nid): n.adopt_hublvol = MagicMock() n.write_to_db = MagicMock() n.client = MagicMock(return_value=MagicMock()) + route_fences(nodes.values(), mock_fw_cls) storage_node_ops.recreate_lvstore(snode, lvs_primary=lvs_owner) @@ -581,8 +582,6 @@ def fake_connect(self_node, primary_node, failover_node=None, patch("simplyblock_core.storage_node_ops.health_controller") as mh, \ patch("simplyblock_core.storage_node_ops.tcp_ports_events"), \ patch("simplyblock_core.storage_node_ops.storage_events"), \ - patch("simplyblock_core.utils.port_block.set_port", - return_value=MagicMock()), \ patch("simplyblock_core.rpc_client.RPCClient", return_value=MagicMock( get_lvstore=MagicMock(return_value= diff --git a/tests/integration/test_secondary_promotion.py b/tests/integration/test_secondary_promotion.py index 74d13b525e..4c36cf11fe 100644 --- a/tests/integration/test_secondary_promotion.py +++ b/tests/integration/test_secondary_promotion.py @@ -20,7 +20,7 @@ from simplyblock_core.models.iface import IFace from simplyblock_core.models.lvol_model import LVol from simplyblock_core.models.storage_node import StorageNode -from tests._mocks import unique_ip +from tests._mocks import route_fences, unique_ip # --------------------------------------------------------------------------- # Helpers @@ -189,7 +189,6 @@ class TestSecondaryPromotion(unittest.TestCase): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack", return_value=(True, None)) @@ -197,10 +196,10 @@ class TestSecondaryPromotion(unittest.TestCase): @patch("simplyblock_core.services.storage_node_monitor.is_node_data_plane_disconnected_quorum", return_value=True) def test_promotes_secondary_when_primary_offline( - self, mock_quorum, mock_db_cls, mock_create_stack, mock_connect_jm, mock_rpc_cls, - mock_fw, mock_tcp_events, mock_storage_events, + self, mock_quorum, mock_db_cls, mock_create_stack, mock_connect_jm, mock_rpc_cls, mock_tcp_events, mock_storage_events, mock_health, mock_executor_cls, mock_add_lvol, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): + mock_fw = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore db = mock_db_cls.return_value @@ -248,6 +247,7 @@ def test_promotes_secondary_when_primary_offline( n.connect_to_hublvol = MagicMock() n.write_to_db = MagicMock() n.wait_for_jm_rep_tasks_to_finish = MagicMock(return_value=True) + route_fences([secondary, tertiary, primary], mock_fw) mock_health.check_bdev.return_value = True mock_recreate_on_non_leader.return_value = True @@ -277,13 +277,12 @@ def test_promotes_secondary_when_primary_offline( @patch("simplyblock_core.storage_node_ops._set_restart_phase") @patch("simplyblock_core.storage_node_ops._handle_rpc_failure_on_peer", return_value="skip") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack", return_value=(True, None)) @patch("simplyblock_core.storage_node_ops.DBController") def test_no_promotion_when_primary_online( - self, mock_db_cls, mock_create_stack, mock_rpc_cls, - mock_fw, mock_tcp_events, _mock_disc, _mock_phase, _mock_handle): + self, mock_db_cls, mock_create_stack, mock_rpc_cls, mock_tcp_events, _mock_disc, _mock_phase, _mock_handle): + mock_fw = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader db = mock_db_cls.return_value @@ -323,6 +322,7 @@ def test_no_promotion_when_primary_online( n.connect_to_hublvol = MagicMock() n.write_to_db = MagicMock() n.wait_for_jm_rep_tasks_to_finish = MagicMock(return_value=True) + route_fences([primary, secondary], mock_fw) result = recreate_lvstore_on_non_leader(secondary, leader_node=primary, primary_node=primary) self.assertTrue(result) @@ -346,14 +346,13 @@ def test_no_promotion_when_primary_online( @patch("simplyblock_core.storage_node_ops._set_restart_phase") @patch("simplyblock_core.storage_node_ops._handle_rpc_failure_on_peer", return_value="skip") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._create_bdev_stack", return_value=(True, None)) @patch("simplyblock_core.storage_node_ops.DBController") def test_always_creates_secondary_hublvol_on_sec1( - self, mock_db_cls, mock_create_stack, mock_rpc_cls, - mock_fw, mock_tcp_events, _mock_disc, _mock_phase, _mock_handle): + self, mock_db_cls, mock_create_stack, mock_rpc_cls, mock_tcp_events, _mock_disc, _mock_phase, _mock_handle): """sec_1 should always create secondary hublvol regardless of primary status.""" + mock_fw = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore_on_non_leader db = mock_db_cls.return_value @@ -394,6 +393,7 @@ def test_always_creates_secondary_hublvol_on_sec1( n.connect_to_hublvol = MagicMock() n.write_to_db = MagicMock() n.wait_for_jm_rep_tasks_to_finish = MagicMock(return_value=True) + route_fences([primary, secondary], mock_fw) recreate_lvstore_on_non_leader(secondary, leader_node=primary, primary_node=primary) @@ -417,17 +417,16 @@ class TestPrimaryEscalation(unittest.TestCase): @patch("simplyblock_core.storage_node_ops.health_controller") @patch("simplyblock_core.storage_node_ops.storage_events") @patch("simplyblock_core.storage_node_ops.tcp_ports_events") - @patch("simplyblock_core.utils.port_block.set_port") @patch("simplyblock_core.models.storage_node.RPCClient") @patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs") @patch("simplyblock_core.storage_node_ops._create_bdev_stack", return_value=(True, None)) @patch("simplyblock_core.storage_node_ops.DBController") def test_escalates_unreachable_primary( - self, mock_db_cls, mock_create_stack, mock_connect_jm, mock_rpc_cls, - mock_fw, mock_tcp_events, mock_storage_events, + self, mock_db_cls, mock_create_stack, mock_connect_jm, mock_rpc_cls, mock_tcp_events, mock_storage_events, mock_health, mock_executor_cls, mock_add_lvol, mock_recreate_on_non_leader, _mock_disc, _mock_phase, _mock_handle): """_check_data_plane_and_escalate should be called for unreachable primary.""" + mock_fw = MagicMock() from simplyblock_core.storage_node_ops import recreate_lvstore # Mock the lazy import of _check_data_plane_and_escalate @@ -483,6 +482,7 @@ def test_escalates_unreachable_primary( n.connect_to_hublvol = MagicMock() n.write_to_db = MagicMock() n.wait_for_jm_rep_tasks_to_finish = MagicMock(return_value=True) + route_fences([secondary, primary_offline, primary_unreachable], mock_fw) mock_health.check_bdev.return_value = True mock_recreate_on_non_leader.return_value = True diff --git a/tests/integration/test_shutdown_no_suspension.py b/tests/integration/test_shutdown_no_suspension.py index dcce9239da..239a10a173 100644 --- a/tests/integration/test_shutdown_no_suspension.py +++ b/tests/integration/test_shutdown_no_suspension.py @@ -31,6 +31,7 @@ from simplyblock_core.models.iface import IFace from simplyblock_core.models.nvme_device import JMDevice, NVMeDevice from simplyblock_core.models.storage_node import StorageNode +from tests._mocks import route_fences # --------------------------------------------------------------------------- # Fixtures @@ -320,7 +321,7 @@ def test_returns_zero_when_no_peers(self): class TestShutdownStorageNodeGraceful(unittest.TestCase): """Verify the graceful path runs Loop 1 (device-unavailable) and - Loop 2 (detach), does NOT call FirewallClient, and finishes with + Loop 2 (detach), does NOT fence any port, and finishes with a SPDK kill + offline transition.""" def _patch(self, force=False): @@ -396,9 +397,8 @@ def _set_status(nid, status, caused_by="monitor"): for p in patches: p.start() self.addCleanup(p.stop) - fw_patch = patch.object(sno, "port_block") # MUST NOT be used - fw_mock = fw_patch.start() - self.addCleanup(fw_patch.stop) + fw_mock = MagicMock() # MUST NOT be called: shutdown fences nothing + route_fences([snode, peer], fw_mock) return { "snode": snode, "peer": peer, "db": db, @@ -434,12 +434,12 @@ def test_graceful_runs_both_loops_then_kills(self): self.assertEqual(statuses[-1], StorageNode.STATUS_OFFLINE) def test_graceful_never_invokes_firewall(self): - """The suspension phase is removed; no FirewallClient call must + """The suspension phase is removed; no port may be fenced happen during graceful shutdown.""" from simplyblock_core import storage_node_ops as sno env = self._patch() sno.shutdown_storage_node(env["snode"].get_id(), force=False) - env["fw_mock"].set_port.assert_not_called() + env["fw_mock"].assert_not_called() def test_force_skips_loops(self): """--force keeps the legacy "go straight to kill" semantics: diff --git a/tests/unit/test_connect_hublvol_lvs_node.py b/tests/unit/test_connect_hublvol_lvs_node.py index a2ec02f55b..e3efdcf8e9 100644 --- a/tests/unit/test_connect_hublvol_lvs_node.py +++ b/tests/unit/test_connect_hublvol_lvs_node.py @@ -462,8 +462,6 @@ def disc(node, lvs_peer_ids=None): patch("simplyblock_core.storage_node_ops.health_controller"), \ patch("simplyblock_core.storage_node_ops.tcp_ports_events"), \ patch("simplyblock_core.storage_node_ops.storage_events"), \ - patch("simplyblock_core.utils.port_block.set_port", - return_value=MagicMock()), \ patch("simplyblock_core.rpc_client.RPCClient", return_value=rpc), \ patch("simplyblock_core.storage_node_ops._connect_to_remote_jm_devs", return_value=[]), \ diff --git a/tests/unit/test_cp_latency_fixes.py b/tests/unit/test_cp_latency_fixes.py index 9445ee2451..119fc3d2c5 100644 --- a/tests/unit/test_cp_latency_fixes.py +++ b/tests/unit/test_cp_latency_fixes.py @@ -321,7 +321,7 @@ def test_one_fetch_answers_all_ports(self): def get_blocked(): calls["n"] += 1 - return {"blocked_ports": [{"port": 4500}]} + return {4500} rpc = types.SimpleNamespace(nvmf_get_blocked_ports=get_blocked) res = health_controller.check_ports_on_node( self._node(rpc), [4500, 4502, 4504]) diff --git a/tests/unit/test_drain_replaced_with_fixed_sleep.py b/tests/unit/test_drain_replaced_with_fixed_sleep.py index 3774b464af..f5d2fd0555 100644 --- a/tests/unit/test_drain_replaced_with_fixed_sleep.py +++ b/tests/unit/test_drain_replaced_with_fixed_sleep.py @@ -221,7 +221,7 @@ def test_failback_order_jc_then_hublvols_then_demote(self): def test_failback_never_blocks_ports(self): self.assertNotIn( - "block=True", self.fn, + ".nvmf_port_block(", self.fn, "the failback must NOT port-block the acting leader or peers: " "the LVS ports carry hublvol/redirect and journal traffic; " "blocking them mid-IO produced DISTRIBD n_unavail_read errors " diff --git a/tests/unit/test_failback_listener_ana.py b/tests/unit/test_failback_listener_ana.py index d24abc583a..ae352cb82a 100644 --- a/tests/unit/test_failback_listener_ana.py +++ b/tests/unit/test_failback_listener_ana.py @@ -225,7 +225,7 @@ def test_fallback_follows_the_probe_and_precedes_the_quiesce(self): i_fallback = src.index("_presumed_acting_leader(") i_compression = src.index("_jc_compression_status_with_retry(current_leader, lvs_jm_vuid)") i_fence = src.index( - "port_block.set_port(sec_node, snode_lvs_port, block=True") + "sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") i_disable = src.index(".jc_disable_replication(lvs_jm_vuid)") i_examine = src.index('_fenced("bdev_distrib_force_to_non_leader"') self.assertLess(i_probe, i_fallback) diff --git a/tests/unit/test_fence_remediation_down_node.py b/tests/unit/test_fence_remediation_down_node.py index 1a7bcb2470..bf87b2225e 100644 --- a/tests/unit/test_fence_remediation_down_node.py +++ b/tests/unit/test_fence_remediation_down_node.py @@ -24,7 +24,7 @@ from simplyblock_core.models.storage_node import StorageNode from simplyblock_core.services import storage_node_monitor -from simplyblock_core.utils import port_block +from tests._mocks import route_fences OWN_PORT = 4442 @@ -51,10 +51,9 @@ def __init__(self, status, hublvol_ok=True, restart_owns=False): def run(self): storage_node_monitor._blocked_port_since.clear() hc = storage_node_monitor.health_controller + route_fences([self.node], lambda n, p, **kw: self.unblocked.append(p)) with patch.object(hc, "_restart_owns_lvs", return_value=self.restart_owns), \ patch.object(hc, "_check_node_hublvol", return_value=self.hublvol_ok), \ - patch.object(port_block, "set_port", - side_effect=lambda n, p, **kw: self.unblocked.append(p)), \ patch.object(storage_node_monitor.time, "monotonic", side_effect=[0.0, 100.0]): # First reading stamps first_seen, second measures how long it has @@ -106,10 +105,9 @@ def test_a_fence_younger_than_the_threshold_is_left_alone(self): h = _Harness(StorageNode.STATUS_DOWN) storage_node_monitor._blocked_port_since.clear() hc = storage_node_monitor.health_controller + route_fences([h.node], lambda n, p, **kw: h.unblocked.append(p)) with patch.object(hc, "_restart_owns_lvs", return_value=False), \ patch.object(hc, "_check_node_hublvol", return_value=True), \ - patch.object(port_block, "set_port", - side_effect=lambda n, p, **kw: h.unblocked.append(p)), \ patch.object(storage_node_monitor.time, "monotonic", side_effect=[0.0, 1.0]): storage_node_monitor._remediate_stale_port_blocks( diff --git a/tests/unit/test_ftt_protection.py b/tests/unit/test_ftt_protection.py index 04ec997312..ddbdd07086 100644 --- a/tests/unit/test_ftt_protection.py +++ b/tests/unit/test_ftt_protection.py @@ -986,10 +986,7 @@ def _fence(node, port): """Make ``node`` report ``port`` fenced in reject mode and serve every lvstore on that port.""" node.get_lvol_subsys_port = MagicMock(return_value=port) - node.rpc_client().nvmf_get_blocked_ports = MagicMock(return_value={ - "total_blocked_ports": 1, - "blocked_ports": [{"port": port, "is_reject": True}], - }) + node.rpc_client().nvmf_get_blocked_ports = MagicMock(return_value={port}) class TestFencedOwnSecondary(unittest.TestCase): @@ -1038,8 +1035,7 @@ def test_fence_before_the_shutdown_stays_a_passing_refusal(self): def test_down_secondary_without_a_fence_stays_a_passing_refusal(self): nodes = self._nodes() nodes[1].get_lvol_subsys_port = MagicMock(return_value=4440) - nodes[1].rpc_client().nvmf_get_blocked_ports = MagicMock( - return_value={"total_blocked_ports": 0, "blocked_ports": []}) + nodes[1].rpc_client().nvmf_get_blocked_ports = MagicMock(return_value=set()) db = _db(_cluster(npcs=1, ft=1), nodes) with patch(EVENT) as event: allowed, reason = _check_ftt_allows_node_removal("n1", db) @@ -1064,10 +1060,7 @@ def test_a_fence_on_another_lvstore_port_is_not_ours(self): nodes = self._nodes() nodes[1].get_lvol_subsys_port = MagicMock( side_effect=lambda lvs: {"LVS_10": 4440, "LVS_3": 4434}[lvs]) - nodes[1].rpc_client().nvmf_get_blocked_ports = MagicMock(return_value={ - "total_blocked_ports": 1, - "blocked_ports": [{"port": 4434, "is_reject": True}], - }) + nodes[1].rpc_client().nvmf_get_blocked_ports = MagicMock(return_value={4434}) db = _db(_cluster(npcs=1, ft=1), nodes) allowed, reason = _check_ftt_allows_node_removal("n1", db) self.assertFalse(allowed) diff --git a/tests/unit/test_port_block_leak_k8s_0911.py b/tests/unit/test_port_block_leak_k8s_0911.py index 56424c711e..bb374b8c09 100644 --- a/tests/unit/test_port_block_leak_k8s_0911.py +++ b/tests/unit/test_port_block_leak_k8s_0911.py @@ -19,9 +19,10 @@ on the observation: a fence nobody is looking at is a fence nobody can remediate. -3. ``port_block.is_port_blocked`` tested ``port_id in +3. The single-port check tested ``port_id in rpc.nvmf_get_blocked_ports()``, a ``{"blocked_ports": [...]}`` dict, so it compared against the dict's keys and returned False for every port. + ``RPCClient.nvmf_get_blocked_ports`` now returns the set of ports itself. The client budget this races is ctrl_loss_tmo = 30 x 2s = 60s, after which the kernel deletes the controller and the namespace fails IO. @@ -32,8 +33,8 @@ from simplyblock_core.controllers import health_controller from simplyblock_core.models.storage_node import StorageNode +from simplyblock_core.rpc_client import RPCClient from simplyblock_core.services import storage_node_monitor -from simplyblock_core.utils import port_block # -------------------------------------------------------------------------- # 3. is_port_blocked parsed the wrong shape @@ -42,38 +43,42 @@ class TestIsPortBlockedParsesPayload(unittest.TestCase): """``nvmf_get_blocked_ports`` answers with a dict, not a list of ports.""" - @staticmethod - def _node(payload): + def _node(self, payload): + from pydantic import SecretStr + + client = RPCClient("127.0.0.1", 8080, "user", SecretStr("pass"), timeout=1, retry=0) + request = patch.object(client, "_request", return_value=payload) + request.start() + self.addCleanup(request.stop) node = MagicMock() - node.rpc_client.return_value.nvmf_get_blocked_ports.return_value = payload + node.rpc_client.return_value = client return node def test_blocked_port_reads_blocked(self): node = self._node({"blocked_ports": [{"port": 4442, "is_reject": False}]}) - self.assertTrue(port_block.is_port_blocked(node, 4442)) + self.assertFalse(health_controller.check_port_on_node(node, 4442)) def test_open_port_reads_open(self): node = self._node({"blocked_ports": [{"port": 4436}]}) - self.assertFalse(port_block.is_port_blocked(node, 4442)) + self.assertTrue(health_controller.check_port_on_node(node, 4442)) def test_no_blocked_ports_reads_open(self): - self.assertFalse(port_block.is_port_blocked(self._node({}), 4442)) - self.assertFalse( - port_block.is_port_blocked(self._node({"blocked_ports": []}), 4442)) + self.assertTrue(health_controller.check_port_on_node(self._node({}), 4442)) + self.assertTrue(health_controller.check_port_on_node( + self._node({"blocked_ports": []}), 4442)) def test_string_port_is_accepted(self): node = self._node({"blocked_ports": [{"port": 4442}]}) - self.assertTrue(port_block.is_port_blocked(node, "4442")) + self.assertFalse(health_controller.check_port_on_node(node, "4442")) def test_agrees_with_the_batched_reader(self): # The batched path always parsed this correctly, which is why the - # monitor saw the fences and this function never did. - payload = {"blocked_ports": [{"port": 4442}, {"port": 4436}]} - node = self._node(payload) - batched = port_block.get_blocked_ports_set(node) + # monitor saw the fences and the single-port check never did. + node = self._node({"blocked_ports": [{"port": 4442}, {"port": 4436}]}) + batched = health_controller.check_ports_on_node(node, [4434, 4436, 4442]) for port in (4434, 4436, 4442): - self.assertEqual(port_block.is_port_blocked(node, port), - port in batched, port) + self.assertEqual(health_controller.check_port_on_node(node, port), + batched[port], port) def test_rpc_absent_raises_instead_of_reading_iptables(self): # There is no iptables fallback any more: the firewall API it went @@ -84,7 +89,7 @@ def test_rpc_absent_raises_instead_of_reading_iptables(self): node.rpc_client.return_value.nvmf_get_blocked_ports.side_effect = ( RPCRemoteError("no such method", RPCErrorCode.method_not_found)) with self.assertRaises(RPCRemoteError): - port_block.is_port_blocked(node, 4442) + health_controller.check_port_on_node(node, 4442) # -------------------------------------------------------------------------- diff --git a/tests/unit/test_port_block_rpc_only.py b/tests/unit/test_port_block_rpc_only.py index a6a2274c0f..a2007d6402 100644 --- a/tests/unit/test_port_block_rpc_only.py +++ b/tests/unit/test_port_block_rpc_only.py @@ -1,66 +1,62 @@ """Port fencing goes through the SPDK RPCs only, never the firewall API. -``port_block.set_port`` used to send fences to the legacy firewall webapp -(``FirewallClient``, ``mgmt_ip:firewall_port``) whenever ``node.spdk_version`` -was empty. 25.10.x records never carried that field, so every node upgraded -to 26.3 took that path -- and a 26.3 host has nothing listening on the -firewall port (the SPDK image no longer ships the webapp or iptables), so -every fence on an upgraded node failed with connection refused. A -method-not-found from the RPC was redirected to the same dead port. +Fences used to go through ``utils.port_block``, which sent them to the +legacy firewall webapp (``FirewallClient``, ``mgmt_ip:firewall_port``) +whenever ``node.spdk_version`` was empty. 25.10.x records never carried that +field, so every node upgraded to 26.3 took that path -- and a 26.3 host has +nothing listening on the firewall port (the SPDK image no longer ships the +webapp or iptables), so every fence on an upgraded node failed with +connection refused. Callers now use the SPDK RPCs on ``node.rpc_client(...)`` +directly; there is no other path to fall back to. """ from types import SimpleNamespace -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest +from pydantic import SecretStr from simplyblock_core.controllers import health_controller -from simplyblock_core.rpc_client import RPCErrorCode, RPCRemoteError -from simplyblock_core.utils import port_block +from simplyblock_core.rpc_client import RPCClient, RPCErrorCode, RPCRemoteError -def _node(rpc, spdk_version=""): - return SimpleNamespace(spdk_version=spdk_version, rpc_client=lambda **_: rpc, - get_id=lambda: "node-1") +def _client(): + return RPCClient("127.0.0.1", 8080, "user", SecretStr("pass"), timeout=1, retry=0) -@pytest.mark.parametrize("spdk_version", ["", "R26.2-PRE-latest", "R26.3"]) -def test_block_and_unblock_use_rpc_whatever_the_recorded_version(spdk_version): - rpc = MagicMock() - node = _node(rpc, spdk_version) - - port_block.set_port(node, 9101, block=True, is_reject=True) - port_block.set_port(node, 9101, block=False) - - rpc.nvmf_port_block.assert_called_once_with(9101, is_reject=True) - rpc.nvmf_port_unblock.assert_called_once_with(9101) +@pytest.mark.parametrize("reply, expected", [ + ({"blocked_ports": [{"port": 9101}, {"port": "9031"}]}, {9101, 9031}), + ({"blocked_ports": []}, set()), + ({}, set()), + (None, set()), +]) +def test_blocked_ports_are_projected_to_a_set(reply, expected): + client = _client() + with patch.object(client, "_request", return_value=reply) as request: + assert client.nvmf_get_blocked_ports() == expected + request.assert_called_once_with("nvmf_get_blocked_ports") -def test_method_not_found_propagates_instead_of_falling_back(): - rpc = MagicMock() - rpc.nvmf_port_block.side_effect = RPCRemoteError( - "Method not found", RPCErrorCode.method_not_found) - - with pytest.raises(RPCRemoteError): - port_block.set_port(_node(rpc), 9101, block=True) +def _node(rpc): + return SimpleNamespace(rpc_client=lambda **_: rpc, get_id=lambda: "node-1") -def test_blocked_ports_come_from_the_rpc(): +def test_port_checks_read_the_rpc(): rpc = MagicMock() - rpc.nvmf_get_blocked_ports.return_value = { - "blocked_ports": [{"port": 9101}, {"port": 9031}]} + rpc.nvmf_get_blocked_ports.return_value = {9101, 9031} node = _node(rpc) - assert port_block.get_blocked_ports_set(node) == {9101, 9031} - assert port_block.is_port_blocked(node, 9101) is True - assert port_block.is_port_blocked(node, 9100) is False + assert health_controller.check_port_on_node(node, 9101) is False + assert health_controller.check_port_on_node(node, "9100") is True assert health_controller.check_ports_on_node(node, [9101, 9100]) == { 9101: False, 9100: True} -def test_blocked_ports_method_not_found_propagates(): +def test_method_not_found_propagates_instead_of_falling_back(): rpc = MagicMock() rpc.nvmf_get_blocked_ports.side_effect = RPCRemoteError( "Method not found", RPCErrorCode.method_not_found) with pytest.raises(RPCRemoteError): health_controller.check_ports_on_node(_node(rpc), [9101]) + with pytest.raises(RPCRemoteError): + health_controller.check_port_on_node(_node(rpc), 9101) diff --git a/tests/unit/test_restart_cpu_fixes.py b/tests/unit/test_restart_cpu_fixes.py index 77de27791c..b54f6419ac 100644 --- a/tests/unit/test_restart_cpu_fixes.py +++ b/tests/unit/test_restart_cpu_fixes.py @@ -456,10 +456,10 @@ def test_impls_stamp_and_report(self): assert "_mark_leader_blocked()" in src_n assert "_mark_leader_unblocked()" in src_n assert "Longest client-port block" in src_n - # every block=True in the non-leader impl is stamped + # every fence in the non-leader impl is stamped # (+1: the def line itself contains the token) assert src_n.count("_mark_leader_blocked()") == src_n.count( - "block=True, timeout=3") + 1 + ").nvmf_port_block(") + 1 def test_coordinator_cap_raised(self): from simplyblock_core import constants diff --git a/tests/unit/test_restart_portblock_order.py b/tests/unit/test_restart_portblock_order.py index db22bc4c9d..3102078975 100644 --- a/tests/unit/test_restart_portblock_order.py +++ b/tests/unit/test_restart_portblock_order.py @@ -36,7 +36,7 @@ def test_nonleader_block_precedes_leader_block(self): # pre-fence JM-replication wait guards on that same condition and is # deliberately earlier, so the string is not unique. i_nonleader = src.index( - "port_block.set_port(sec_node, snode_lvs_port, block=True") + "sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") i_leader_open = src.index("_REPL_SUSPEND_MAX_ATTEMPTS = 10") assert i_nonleader < i_leader_open, \ "non-leader ports must be blocked before the leader block opens" @@ -44,7 +44,7 @@ def test_nonleader_block_precedes_leader_block(self): def test_demote_starts_after_all_ports_blocked(self): src = _code(_src()) i_nonleader = src.index( - "port_block.set_port(sec_node, snode_lvs_port, block=True") + "sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") # the demote == suspend replication then drop leadership i_disable_repl = src.index(".jc_disable_replication(lvs_jm_vuid)") i_drop = src.index("bdev_lvol_set_leader(lvs_name, leader=False") @@ -67,7 +67,7 @@ def test_the_order_is_documented(self): def test_nonleader_block_still_aborts_on_failure(self): """The reorder must keep the abort-on-failure behaviour.""" src = _code(_src()) - i = src.index("port_block.set_port(sec_node, snode_lvs_port, block=True") + i = src.index("sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") window = src[i:i + 1100] assert "_abort_restart_and_unblock(" in window @@ -75,7 +75,7 @@ def test_nonleader_block_skips_leader_and_disconnected_and_already_blocked(self) src = _code(_src()) # anchor on the non-leader block's own port_block, then look at the # loop head just above it (there are several sec_node loops). - j = src.index("port_block.set_port(sec_node, snode_lvs_port, block=True") + j = src.index("sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") i = src.rfind("for sec_node in sec_nodes:", 0, j) window = src[i:j] assert "sec_node is current_leader" in window @@ -86,7 +86,7 @@ def test_no_duplicate_nonleader_block_remains(self): """The block must have MOVED, not been copied.""" src = _src() assert src.count( - "port_block.set_port(sec_node, snode_lvs_port, block=True") == 1 + "sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") == 1 class TestJmReplicationWaitStaysOutsideTheFence: @@ -113,7 +113,7 @@ def test_patient_wait_runs_before_any_port_is_fenced(self): lines and its exact formatting is not the invariant.""" src = _code(_src()) i_fence = src.index( - "port_block.set_port(sec_node, snode_lvs_port, block=True") + "sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") patient = [ m.start() for m in re.finditer( r"wait_for_jm_rep_tasks_to_finish\((.*?)\)", src, re.DOTALL) @@ -130,7 +130,7 @@ def test_patient_wait_runs_before_any_port_is_fenced(self): def test_every_wait_inside_the_fence_is_explicitly_bounded(self): src = _code(_src()) i_fence = src.index( - "port_block.set_port(sec_node, snode_lvs_port, block=True") + "sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") calls = re.findall( r"wait_for_jm_rep_tasks_to_finish\((.*?)\)", src[i_fence:], re.DOTALL) @@ -144,7 +144,7 @@ def test_fence_clock_is_checked_before_the_demote_begins(self): """The clock went unchecked from the fence to the inflight drain.""" src = _code(_src()) i_fence = src.index( - "port_block.set_port(sec_node, snode_lvs_port, block=True") + "sec_node.rpc_client(timeout=0.5, retry=1).nvmf_port_block(snode_lvs_port)") i_check = src.index('_check_fence_deadline("jm replication confirm")') i_disable = src.index(".jc_disable_replication(lvs_jm_vuid)") assert i_fence < i_check < i_disable, ( diff --git a/tests/unit/test_two_node_c3_fixes.py b/tests/unit/test_two_node_c3_fixes.py index dfea885178..ae8ec97682 100644 --- a/tests/unit/test_two_node_c3_fixes.py +++ b/tests/unit/test_two_node_c3_fixes.py @@ -24,6 +24,7 @@ LVS_SOLO, ) from simplyblock_core.models.storage_node import StorageNode +from tests._mocks import route_fences from tests.unit.test_two_node_arbiter import ( # noqa: F401 - fixture CL, calls, @@ -365,8 +366,8 @@ def test_defensive_unblock_skips_the_stale_peer_only(monkeypatch): stale = _Peer(B, [{"jm_vuid": 4, "state": "normal"}]) db = _DB(rebuild={B: {"queued": True}}, peers=(survivor, stale)) opened = [] - monkeypatch.setattr(sno.port_block, "set_port", - lambda node, port, block, **kw: opened.append((node.get_id(), port, block))) + route_fences([survivor, stale], + lambda node, port, block, **kw: opened.append((node.get_id(), port, block))) monkeypatch.setattr(sno.tcp_ports_events, "port_allowed", lambda *a, **k: None) class LvsNode: diff --git a/tests/unit/test_two_node_c5_fixes.py b/tests/unit/test_two_node_c5_fixes.py index b79dd0d09d..bc1d931f19 100644 --- a/tests/unit/test_two_node_c5_fixes.py +++ b/tests/unit/test_two_node_c5_fixes.py @@ -369,7 +369,7 @@ def _monitor_run(monkeypatch, arbitrated, ha_state): from simplyblock_core import storage_node_ops as sno from simplyblock_core.services import storage_node_monitor as mon - from simplyblock_core.utils import port_block + from tests._mocks import route_fences node = MagicMock() node.get_id.return_value = "n1" node.cluster_id = "cl-mon-%s" % arbitrated @@ -387,9 +387,9 @@ def _monitor_run(monkeypatch, arbitrated, ha_state): unblocked = [] mon._blocked_port_since.clear() hc = mon.health_controller + route_fences([node], lambda n, p, **kw: unblocked.append(p)) with patch.object(hc, "_restart_owns_lvs", return_value=False), \ patch.object(hc, "_check_node_hublvol", return_value=True), \ - patch.object(port_block, "set_port", side_effect=lambda n, p, **kw: unblocked.append(p)), \ patch.object(mon.time, "monotonic", side_effect=[0.0, 100.0]): mon._remediate_stale_port_blocks(db, node, {4426: False}, {4426: "n1"}) return unblocked, rpc diff --git a/tests/unit/test_two_node_f15_port_allow.py b/tests/unit/test_two_node_f15_port_allow.py index 5beeb15823..51f3df2a95 100644 --- a/tests/unit/test_two_node_f15_port_allow.py +++ b/tests/unit/test_two_node_f15_port_allow.py @@ -9,6 +9,7 @@ """ from types import SimpleNamespace +from unittest.mock import MagicMock import pytest @@ -16,6 +17,7 @@ from simplyblock_core.models.storage_node import StorageNode from simplyblock_core.services import tasks_runner_port_allow as pa from simplyblock_core.services.task_runner_base import TaskDefer +from tests._mocks import route_fences class _RPC: @@ -81,7 +83,7 @@ def _run(monkeypatch, remote_jms, keep): node = SimpleNamespace( status=StorageNode.STATUS_ONLINE, mgmt_ip="10.0.0.1", data_nics=[], cluster_id="CL", remote_devices=[], remote_jm_devices=[], - write_to_db=lambda: None) + write_to_db=lambda: None, rpc_client=lambda **_: MagicMock()) monkeypatch.setattr(pa, "db", _DB(node)) monkeypatch.setattr(pa.health_controller, "_check_node_ping", lambda ip: True) monkeypatch.setattr(sno, "_connect_to_remote_devs", lambda n, reattach=False: ["dev"]) @@ -109,8 +111,7 @@ def past(*a, **k): def test_a_fenced_two_node_task_defers_before_any_unblock(monkeypatch): task, asked, _ = _run(monkeypatch, ["jm-peer"], "LVS 6 is fenced on the node") unblocked = [] - monkeypatch.setattr(pa.port_block, "set_port", - lambda *a, **k: unblocked.append((a, k))) + route_fences([pa.db.node], lambda *a, **k: unblocked.append((a, k))) with pytest.raises(TaskDefer, match="Client ports stay blocked: LVS 6 is fenced"): pa.exec_port_allow_task(task) assert len(asked) == 1 and unblocked == [] @@ -118,7 +119,6 @@ def test_a_fenced_two_node_task_defers_before_any_unblock(monkeypatch): def test_two_remote_jms_never_reach_the_gate(monkeypatch): task, asked, past = _run(monkeypatch, ["jm-a", "jm-b"], "LVS 6 is fenced on the node") - monkeypatch.setattr(pa.port_block, "set_port", lambda *a, **k: None) try: pa.exec_port_allow_task(task) except (TaskDefer, past, Exception): @@ -128,7 +128,6 @@ def test_two_remote_jms_never_reach_the_gate(monkeypatch): def test_an_unfenced_two_node_task_passes_the_gate(monkeypatch): task, asked, past = _run(monkeypatch, ["jm-peer"], "") - monkeypatch.setattr(pa.port_block, "set_port", lambda *a, **k: None) try: pa.exec_port_allow_task(task) except TaskDefer as e: