Merge pull request 'fix: keep CARP state knowledge out of the base package (4.10.1)' (#39) from fix/carp-helper-to-driver into main

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

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",

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"]

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"

tests/test_carp_virtual_ips.py

@@ -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"