From 0330f17a672fbfd8ed2638b416decec3036dec00 Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Fri, 16 Oct 2026 11:11:16 +0200 Subject: [PATCH] fix: keep CARP state knowledge out of the base package (4.10.1) carp_virtual_ip() and its state table are CARP protocol knowledge; under the agreed structure (PR #37, docs/STRUCTURE.md) the base package carries only contracts, data shapes and I/O-free reconcilers. They move to the driver that speaks CARP (napalm-opnsense). The contract stays: VirtualIpsDict.carp and VirtualIpDict.holds. Nothing used the helper yet; it was released in 4.10.0 an hour ago. netOrk #1121. --- README.md | 13 ++-- napalm_device_types/__init__.py | 2 - napalm_device_types/virtual_ips.py | 52 -------------- pyproject.toml | 2 +- tests/test_carp_virtual_ips.py | 111 ----------------------------- 5 files changed, 7 insertions(+), 173 deletions(-) delete mode 100644 tests/test_carp_virtual_ips.py diff --git a/README.md b/README.md index 542f9df..a5c42d7 100644 --- a/README.md +++ b/README.md @@ -128,13 +128,12 @@ A mechanism it could not read completely is `None` ("did not look"), a host with `[]`. The command is about 4 kB, so a driver sends it on an exec channel (`_run_virtual_ips_command(command, privileged=)`), not typed into a shell. -A firewall's CARP addresses are a third mechanism, `carp`, which a firewall driver reads -from its own API and reports under the reading's `carp` key (`vip_manager` and -`keepalived` then None). `carp_virtual_ip(...)` turns one address bound to a VHID into the -entry -- the same for OPNsense and pfSense. Unlike the other two, a CARP entry says who -holds the address (`holds`): every member carries the address in its configuration, and a -firewall's interface list need not show it, but CARP's state names the MASTER. A state it -does not know leaves `holds` None, and the caller falls back on the interfaces. +A firewall's CARP addresses are a third mechanism, `carp`: a firewall driver reads them from +its own API and reports them under the reading's `carp` key (`vip_manager` and `keepalived` +then None). What a CARP state means is protocol knowledge and stays in that driver. The +contract only carries the result: unlike the other two, a CARP entry says who holds the +address (`holds`), because a firewall's interface list need not show it. `holds` None +leaves the holder to the interfaces. `SystemIdentityMixin` (`get_system_identity`) reads a host's SMBIOS identity -- system UUID, product serial and board serial from `/sys/class/dmi/id` -- which is what ties a server to diff --git a/napalm_device_types/__init__.py b/napalm_device_types/__init__.py index 4d901a8..5eb32f9 100644 --- a/napalm_device_types/__init__.py +++ b/napalm_device_types/__init__.py @@ -134,7 +134,6 @@ from napalm_device_types.system_identity import ( from napalm_device_types.virtual_ips import ( VIRTUAL_IPS_COMMAND, VirtualIpsMixin, - carp_virtual_ip, parse_virtual_ips, ) from napalm_device_types.residential_gateway import ResidentialGatewayDriver @@ -222,7 +221,6 @@ __all__ = [ "SshHardeningResult", "plan_ssh_hardening", "parse_virtual_ips", - "carp_virtual_ip", "add_lag_interfaces", "driver_supports_ping", "normalize_cidr", diff --git a/napalm_device_types/virtual_ips.py b/napalm_device_types/virtual_ips.py index 565243f..6733684 100644 --- a/napalm_device_types/virtual_ips.py +++ b/napalm_device_types/virtual_ips.py @@ -42,11 +42,6 @@ caller keeps what it knew. ``[]`` is a host that has none. script goes to the host as one ``sh -c`` argument; when the privileged call brings no report back, it runs again without privilege and reads what it can. -**CARP.** A firewall's CARP addresses come from its own API, not from this -command, so :func:`carp_virtual_ip` only shapes one entry: what is the same for -every CARP firewall is the meaning of the state. CARP's MASTER holds the address -(``holds``), which the interface list of a firewall need not show. - **An exec channel, not a terminal.** With its awk program the command is about 4 kB, past the line an interactive shell reliably takes. A driver sends it the way it runs any command that needs no PTY -- napalm-linux's ``run_command``. @@ -337,53 +332,6 @@ _READERS: Dict[str, Tuple[str, Callable[[str, List[_Record]], List[VirtualIpDict } -#: CARP's state of a VHID on one member -> (running, holds). -_CARP_STATES: Dict[str, Tuple[bool, bool]] = { - "MASTER": (True, True), - "BACKUP": (True, False), - "INIT": (False, False), - "DISABLED": (False, False), -} - - -def carp_virtual_ip( - *, - address: str, - prefix_length: Optional[int], - vhid: object, - state: Optional[str], - instance: str, - interface: Optional[str], -) -> Optional[VirtualIpDict]: - """One address bound to a CARP VHID, as a virtual IP; None for what is no address. - - Who holds it comes from CARP's *state* on this member, not from its - interfaces: MASTER holds it, BACKUP stands by, INIT and DISABLED neither run - nor hold. A state this does not know says nothing (``running`` and ``holds`` - None), and the caller falls back on the interfaces. *address* may carry its - prefix (``"10.0.0.1/24"``), which then wins over *prefix_length*. - """ - parsed = _address(address.strip()) if address else None - if parsed is None: - return None - normalised, written = parsed - prefix = written if written is not None else prefix_length - if prefix is not None and not 0 <= prefix <= ipaddress.ip_address(normalised).max_prefixlen: - prefix = None - vhid_text = str(vhid).strip() if vhid is not None else "" - running, holds = _CARP_STATES.get((state or "").strip().upper(), (None, None)) - return { - "mechanism": "carp", - "instance": instance or f"vhid {vhid_text}", - "address": normalised, - "prefix_length": prefix, - "interface": interface, - "group_key": f"vhid:{vhid_text}" if vhid_text.isdigit() else None, - "running": running, - "holds": holds, - } - - def _order(entry: VirtualIpDict) -> Tuple[int, int, str]: address = ipaddress.ip_address(entry["address"]) return address.version, int(address), entry["instance"] diff --git a/pyproject.toml b/pyproject.toml index b965bed..7dec00f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "napalm-device-types" -version = "4.10.0" +version = "4.10.1" description = "Abstract device-type base classes for NAPALM drivers" readme = "README.md" requires-python = ">=3.10" diff --git a/tests/test_carp_virtual_ips.py b/tests/test_carp_virtual_ips.py deleted file mode 100644 index 3568a5e..0000000 --- a/tests/test_carp_virtual_ips.py +++ /dev/null @@ -1,111 +0,0 @@ -"""carp_virtual_ip: one CARP address as a virtual IP, with its holder in the reading. - -CARP differs from vip-manager and keepalived in what the caller can see. The -address is configured on every member, and the interface list of a firewall need -not show it at all -- so the interfaces cannot say who holds it. CARP's own state -can: the member in MASTER holds it, a BACKUP stands by. Turning that state into a -virtual IP is the same for every CARP firewall (OPNsense, pfSense), so it lives -here; a driver supplies the fields from its API (netOrk #1121). -""" - -from __future__ import annotations - -import pytest - -from napalm_device_types.virtual_ips import carp_virtual_ip - - -def _entry(**overrides): - fields = { - "address": "172.22.8.1", - "prefix_length": 24, - "vhid": "108", - "state": "MASTER", - "instance": "MGMT", - "interface": "vtnet0", - } - fields.update(overrides) - return carp_virtual_ip(**fields) - - -def test_a_master_holds_the_address(): - assert _entry() == { - "mechanism": "carp", - "instance": "MGMT", - "address": "172.22.8.1", - "prefix_length": 24, - "interface": "vtnet0", - "group_key": "vhid:108", - "running": True, - "holds": True, - } - - -def test_a_backup_stands_by(): - entry = _entry(state="BACKUP") - assert entry is not None - assert (entry["running"], entry["holds"]) == (True, False) - - -@pytest.mark.parametrize("state", ["INIT", "DISABLED"]) -def test_a_member_whose_carp_is_not_up_neither_runs_nor_holds(state): - entry = _entry(state=state) - assert entry is not None - assert (entry["running"], entry["holds"]) == (False, False) - - -@pytest.mark.parametrize("state", [None, "", "SOMETHING-NEW"]) -def test_an_unknown_state_says_nothing_about_the_holder(state): - """None leaves the holder to the interfaces, as for the other mechanisms.""" - entry = _entry(state=state) - assert entry is not None - assert (entry["running"], entry["holds"]) == (None, None) - - -def test_the_state_is_read_case_blind(): - entry = _entry(state=" master ") - assert entry is not None - assert entry["holds"] is True - - -def test_the_address_is_normalised(): - entry = _entry(address="FD00::0001", prefix_length=64) - assert entry is not None - assert entry["address"] == "fd00::1" - - -def test_an_address_written_with_its_prefix_takes_that_prefix(): - entry = _entry(address="10.42.100.1/24", prefix_length=None) - assert entry is not None - assert (entry["address"], entry["prefix_length"]) == ("10.42.100.1", 24) - - -@pytest.mark.parametrize("address", ["", "not-an-address", "10.0.0.300"]) -def test_what_is_no_address_is_no_virtual_ip(address): - assert _entry(address=address) is None - - -@pytest.mark.parametrize("prefix", [33, -1]) -def test_an_impossible_prefix_is_dropped(prefix): - entry = _entry(prefix_length=prefix) - assert entry is not None - assert entry["prefix_length"] is None - - -@pytest.mark.parametrize("vhid", [None, "", "abc"]) -def test_without_a_numeric_vhid_there_is_no_group(vhid): - entry = _entry(vhid=vhid) - assert entry is not None - assert entry["group_key"] is None - - -def test_a_vhid_given_as_a_number_groups_the_same_way(): - entry = _entry(vhid=108) - assert entry is not None - assert entry["group_key"] == "vhid:108" - - -def test_an_empty_instance_falls_back_to_the_vhid(): - entry = _entry(instance="") - assert entry is not None - assert entry["instance"] == "vhid 108" -- 2.54.0