From 7f07da6857f8be5f3945804e20996066f5857b22 Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Fri, 25 Sep 2026 10:44:11 +0200 Subject: [PATCH] fix(cli): the trunk gets its own row, as over REST Since f35b59e a CLI poll reports trunk members as `3` with trunk_group `Trk3`, but `show interfaces brief` has no line for Trk3 itself, so the trunk was missing from the interface list. The REST path built that row on its own. Both paths now use napalm_device_types.add_lag_interfaces, and the REST path's copy is gone. One visible change there: `lag_members` is now in port order (7, 10) instead of API order; the description already was. The CLI path does not know the mode yet, so its trunks carry no lag_mode. That needs `show trunks`, whose output is not at hand. Needs napalm-device-types f3fa75b. Closes #5 --- napalm_procurve/api_client.py | 22 ++++---------- napalm_procurve/procurve.py | 10 +++++-- tests/unit/test_driver.py | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/napalm_procurve/api_client.py b/napalm_procurve/api_client.py index 18c1ff4..03ae7cc 100644 --- a/napalm_procurve/api_client.py +++ b/napalm_procurve/api_client.py @@ -15,6 +15,7 @@ import urllib3 from napalm.base import helpers as napalm_helpers from napalm.base.exceptions import ConnectionException, ConnectAuthError +from napalm_device_types import add_lag_interfaces logger = logging.getLogger("napalm_procurve.api") @@ -307,22 +308,11 @@ class ProcurveApiClient: if pid in output: output[pid]["speed"] = float(stat.get("port_speed_mbps", 0)) - # Synthesize a logical interface entry for each configured LAG/trunk - # group so it shows up as its own row alongside its member ports. - for group, members in trunk_groups.items(): - output[group] = { - "is_up": any(output[m]["is_up"] for m in members), - "is_enabled": any(output[m]["is_enabled"] for m in members), - "description": f"LAG ({', '.join(sorted(members, key=lambda s: int(s) if s.isdigit() else 0))})", - "last_flapped": -1.0, - "speed": sum(output[m]["speed"] for m in members), - "mtu": -1, - "mac_address": "", - "lag_members": members, - "lag_mode": "lacp" if trunk_modes.get(group) == "PTT_LACP" else "trunk", - } - - return output + # One row per LAG/trunk group alongside its member ports. + return add_lag_interfaces(output, { + group: "lacp" if trunk_modes.get(group) == "PTT_LACP" else "trunk" + for group in trunk_groups + }) def get_interfaces_ip(self) -> Dict[str, Dict]: """Return NAPALM interfaces IP from the REST API.""" diff --git a/napalm_procurve/procurve.py b/napalm_procurve/procurve.py index c8057ae..1406017 100644 --- a/napalm_procurve/procurve.py +++ b/napalm_procurve/procurve.py @@ -27,7 +27,12 @@ from netmiko.exceptions import ( NetmikoAuthenticationException, NetmikoTimeoutException, ) -from napalm_device_types import ConfigLifecycleMixin, FingerprintRule, SwitchDriver +from napalm_device_types import ( + ConfigLifecycleMixin, + FingerprintRule, + SwitchDriver, + add_lag_interfaces, +) from napalm_device_types.models import InterfaceConfigDict, VlanConfigDict from napalm.base import helpers as napalm_helpers from napalm.base.exceptions import ( @@ -502,7 +507,8 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver): except Exception: pass - return ifaces + # The CLI lists member ports only ("3-Trk3"); the trunk gets its own row. + return add_lag_interfaces(ifaces) # ------------------------------------------------------------------ # NAPALM: get_interfaces_ip diff --git a/tests/unit/test_driver.py b/tests/unit/test_driver.py index 02b29c7..9def7a8 100644 --- a/tests/unit/test_driver.py +++ b/tests/unit/test_driver.py @@ -884,3 +884,58 @@ class TestApiSetInterfaceExistingMembership: drv.set_interface("10", {"mode": "trunk", "trunk_vlans": [10]}) assert "boom" in str(exc.value) api.put.assert_not_called() + + +# =========================================================================== +# LAG rows (#5) +# =========================================================================== + + +class TestApiGetInterfacesLag: + """The REST path's trunk rows, kept as they were when it moved to add_lag_interfaces.""" + + def _client(self, ports): + from napalm_procurve.api_client import ProcurveApiClient + + client = ProcurveApiClient(hostname="192.168.0.1", username="manager", password="secret") + responses = { + "ports": {"port_element": ports}, + "port-statistics": {"port_statistics_element": [ + {"id": p["id"], "port_speed_mbps": 1000} for p in ports + ]}, + "system/status/switch": {}, + } + client.get = MagicMock(side_effect=lambda endpoint, **kw: responses[endpoint]) + return client + + def test_trunk_rows_with_mode(self): + client = self._client([ + {"id": "1", "is_port_up": True, "is_port_enabled": True, "trunk_group": ""}, + {"id": "3", "is_port_up": False, "is_port_enabled": True, "trunk_group": "trk3", "trunk_mode": "PTT_LACP"}, + {"id": "4", "is_port_up": True, "is_port_enabled": True, "trunk_group": "trk3", "trunk_mode": "PTT_LACP"}, + {"id": "10", "is_port_up": False, "is_port_enabled": True, "trunk_group": "trk6"}, + {"id": "7", "is_port_up": False, "is_port_enabled": True, "trunk_group": "trk6"}, + ]) + ifaces = client.get_interfaces() + + assert ifaces["trk3"]["lag_members"] == ["3", "4"] + assert ifaces["trk3"]["lag_mode"] == "lacp" + assert ifaces["trk3"]["is_up"] is True + assert ifaces["trk3"]["speed"] == 2000.0 + assert ifaces["trk6"]["lag_members"] == ["7", "10"] + assert ifaces["trk6"]["lag_mode"] == "trunk" + assert ifaces["trk6"]["description"] == "LAG (7, 10)" + assert ifaces["3"]["trunk_group"] == "trk3" + + +class TestCliGetInterfacesLag: + def test_trunk_rows_from_member_ports(self, driver): + driver._send_command = MagicMock( + side_effect=lambda cmd: SHOW_INTERFACES_BRIEF_INTRUSION if cmd == "show interfaces brief" else "" + ) + ifaces = driver.get_interfaces() + + assert ifaces["Trk3"]["lag_members"] == ["3", "4"] + assert ifaces["Trk6"]["lag_members"] == ["6", "7"] + assert ifaces["Trk6"]["is_up"] is True + assert ifaces["Trk6"]["is_enabled"] is True