fix(cli): read J.15 port status, and say why each transport failed
A 2800-series switch on J.15.09 polled over CLI came back with no
interfaces and an empty OS version, while VLANs and ARP parsed fine.
`show interfaces brief` on that firmware has an Intrusion Alert column
between the `|` and Enabled, and puts Mode before MDI rather than after:
Port Type | Alert Enabled Status Mode Mode ...
3-Trk3 100/1000T | No Yes Down 1000FDx MDI ...
The regex read Alert as Enabled, then failed on Yes where it wanted
Up|Down, so no line matched. It now skips the Alert column where there is
one and takes the speed from either position. Trunk members are listed as
`<port>-Trk<n>`; the port is `<port>` and the suffix becomes its
trunk_group, so the per-port `show interfaces 3` is a command the switch
knows.
The empty OS version was the alternatives list in _send_command. It moved
to the next command on "% Invalid" or "Error", but ProCurve rejects an
unknown command with "Invalid input: system-information" -- so that line
was parsed as system information. "Invalid input" now counts as failure.
Getting there took longer than it should have, because the first symptom
was "Authentication failed: Login failed". That was Telnet's error, the
last transport tried; the REST probe and both SSH attempts had failed
before it and said nothing above debug level. In fact the switch had run
out of CLI sessions and closed SSH straight after the password. open()
now records why each transport failed, and both the auth error and the
final "Cannot connect" carry that list.
Fixtures are that switch's output, with hostname, serial and MAC
replaced.
Closes #2
Closes #3
Closes #4
This commit is contained in:
@@ -234,12 +234,13 @@ def parse_model_from_version(output: str) -> tuple[str, str]:
|
||||
# 1 100/1000T | Yes Down Auto Unknown off 0
|
||||
|
||||
_INTF_BRIEF_RE = re.compile(
|
||||
r"^\s*(\S+)\s+" # Port
|
||||
r"^\s*([^\s-]+)" # Port
|
||||
r"(?:-(Trk\d+))?\s+" # Trunk group of a member port ("3-Trk3")
|
||||
r"(\S+)\s+\|" # Type |
|
||||
r"\s+(Yes|No)\s+" # Enabled
|
||||
r"\s+(?:(?:Yes|No)\s+)?" # Intrusion Alert, on firmware that has the column
|
||||
r"(Yes|No)\s+" # Enabled
|
||||
r"(Up|Down)\s+" # Link
|
||||
r"\S+\s+" # MDI (ignored)
|
||||
r"(\S+)", # Mode (speed/duplex)
|
||||
r"(\S+)\s+(\S+)", # Mode and MDI, in either order depending on firmware
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
@@ -255,13 +256,13 @@ def parse_interfaces_brief(output: str) -> Dict[str, Dict]:
|
||||
m = _INTF_BRIEF_RE.match(line)
|
||||
if not m:
|
||||
continue
|
||||
port, itype, enabled, link, mode = (
|
||||
m.group(1), m.group(2), m.group(3), m.group(4), m.group(5)
|
||||
)
|
||||
port, trunk_group, itype, enabled, link, *mode_or_mdi = m.groups()
|
||||
speed = 0.0
|
||||
duplex = ""
|
||||
# Mode examples: "1000FDx", "100HDx", "Unknown", "Auto"
|
||||
sm = re.match(r"(\d+)(FDx|HDx)?", mode, re.I)
|
||||
# Mode examples: "1000FDx", "100HDx", "Unknown"; MDI is "Auto", "MDI", "MDIX", "NA"
|
||||
sm = next(
|
||||
filter(None, (re.match(r"(\d+)(FDx|HDx)?", t, re.I) for t in mode_or_mdi)), None
|
||||
)
|
||||
if sm:
|
||||
speed = float(sm.group(1))
|
||||
duplex = "full" if (sm.group(2) or "").lower() == "fdx" else "half"
|
||||
@@ -276,6 +277,8 @@ def parse_interfaces_brief(output: str) -> Dict[str, Dict]:
|
||||
"mtu": -1,
|
||||
"mac_address": "",
|
||||
}
|
||||
if trunk_group:
|
||||
interfaces[port]["trunk_group"] = trunk_group
|
||||
return interfaces
|
||||
|
||||
|
||||
|
||||
+28
-14
@@ -138,6 +138,8 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver):
|
||||
self._device: Optional[ConnectHandler] = None
|
||||
# REST API backend
|
||||
self._api: Optional[ProcurveApiClient] = None
|
||||
# Why each transport failed during the current open(), as "<transport>: <reason>"
|
||||
self._attempts: List[str] = []
|
||||
|
||||
# Config management state (CLI only)
|
||||
self._candidate_config: Optional[str] = None
|
||||
@@ -159,38 +161,45 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver):
|
||||
4. Telnet
|
||||
"""
|
||||
forced = self.force_transport
|
||||
tried: List[str] = []
|
||||
self._attempts = []
|
||||
|
||||
# --- 1. REST API ---
|
||||
if not forced or forced == "api":
|
||||
if self._try_api():
|
||||
return
|
||||
tried.append("api")
|
||||
|
||||
# --- 2. SSH standard ---
|
||||
if not forced or forced == "ssh":
|
||||
if self._try_ssh(legacy=False):
|
||||
return
|
||||
tried.append("ssh")
|
||||
|
||||
# --- 3. SSH legacy KEX ---
|
||||
if not forced or forced == "ssh_legacy":
|
||||
if self._try_ssh(legacy=True):
|
||||
return
|
||||
tried.append("ssh_legacy")
|
||||
|
||||
# --- 4. Telnet ---
|
||||
if not forced or forced == "telnet":
|
||||
if self._try_telnet():
|
||||
return
|
||||
tried.append("telnet")
|
||||
|
||||
raise ConnectionException(
|
||||
f"Cannot connect to {self.hostname}. "
|
||||
f"Tried transports: {', '.join(tried)}. "
|
||||
f"Tried transports: {'; '.join(self._attempts)}. "
|
||||
"Check connectivity, credentials and whether SSH/Telnet/API is enabled."
|
||||
)
|
||||
|
||||
def _auth_failure(self, transport: str, exc: Exception) -> ConnectionException:
|
||||
"""An authentication error that also says why the earlier transports failed.
|
||||
|
||||
Without them, a Telnet "Login failed" reads as a wrong password when SSH
|
||||
was merely refused and Telnet was the only transport left to answer (#2).
|
||||
"""
|
||||
msg = f"Authentication failed for {self.hostname} via {transport}: {exc}"
|
||||
if self._attempts:
|
||||
msg += f" (earlier: {'; '.join(self._attempts)})"
|
||||
return ConnectionException(msg)
|
||||
|
||||
def close(self) -> None:
|
||||
"""Close the active connection."""
|
||||
if self._api:
|
||||
@@ -234,10 +243,12 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver):
|
||||
|
||||
if not ver:
|
||||
logger.warning("REST API not detected on %s — falling back to CLI", self.hostname)
|
||||
self._attempts.append("api: not detected")
|
||||
return False
|
||||
|
||||
# Try connecting with the requested SSL setting first; if it fails due to a
|
||||
# self-signed certificate (ssl_verify=True), transparently retry unverified.
|
||||
error: Optional[Exception] = None
|
||||
for ssl_verify in ([self.ssl_verify] if not self.ssl_verify else [True, False]):
|
||||
client = ProcurveApiClient(
|
||||
hostname=self.hostname,
|
||||
@@ -252,6 +263,7 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver):
|
||||
client.connect()
|
||||
except Exception as exc:
|
||||
logger.debug("REST API connect failed (ssl_verify=%s): %s", ssl_verify, exc)
|
||||
error = exc
|
||||
continue
|
||||
self._api = client
|
||||
self._transport = "api"
|
||||
@@ -260,6 +272,7 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver):
|
||||
return True
|
||||
|
||||
logger.warning("REST API connect failed for %s — falling back to CLI", self.hostname)
|
||||
self._attempts.append(f"api: {error}")
|
||||
return False
|
||||
|
||||
def _netmiko_kwargs(self, legacy: bool = False) -> dict:
|
||||
@@ -285,22 +298,23 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver):
|
||||
def _try_ssh(self, legacy: bool = False) -> bool:
|
||||
"""Probe and connect via SSH. Returns True on success."""
|
||||
label = "SSH-legacy" if legacy else "SSH"
|
||||
transport = "ssh_legacy" if legacy else "ssh"
|
||||
logger.debug("Trying %s for %s", label, self.hostname)
|
||||
try:
|
||||
conn = ConnectHandler(**self._netmiko_kwargs(legacy))
|
||||
self._device = conn
|
||||
self._transport = "ssh_legacy" if legacy else "ssh"
|
||||
self._transport = transport
|
||||
logger.info("Connected to %s via %s", self.hostname, label)
|
||||
return True
|
||||
except NetmikoAuthenticationException as exc:
|
||||
raise ConnectionException(
|
||||
f"Authentication failed for {self.hostname}: {exc}"
|
||||
) from exc
|
||||
raise self._auth_failure(transport, exc) from exc
|
||||
except NetmikoTimeoutException:
|
||||
logger.debug("%s timeout for %s", label, self.hostname)
|
||||
self._attempts.append(f"{transport}: timed out")
|
||||
return False
|
||||
except Exception as exc:
|
||||
logger.debug("%s failed for %s: %s", label, self.hostname, exc)
|
||||
self._attempts.append(f"{transport}: {exc}")
|
||||
return False
|
||||
|
||||
def _try_telnet(self) -> bool:
|
||||
@@ -321,11 +335,10 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver):
|
||||
logger.info("Connected to %s via Telnet", self.hostname)
|
||||
return True
|
||||
except NetmikoAuthenticationException as exc:
|
||||
raise ConnectionException(
|
||||
f"Authentication failed for {self.hostname}: {exc}"
|
||||
) from exc
|
||||
raise self._auth_failure("telnet", exc) from exc
|
||||
except Exception as exc:
|
||||
logger.debug("Telnet failed for %s: %s", self.hostname, exc)
|
||||
self._attempts.append(f"telnet: {exc}")
|
||||
return False
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
@@ -355,7 +368,8 @@ class ProcurveDriver(ConfigLifecycleMixin, SwitchDriver):
|
||||
last = ""
|
||||
for cmd in command:
|
||||
last = _do(cmd)
|
||||
if "% Invalid" not in last and "Error" not in last:
|
||||
# ProCurve says "Invalid input: …"; other firmware "% Invalid …"
|
||||
if "Invalid input" not in last and "% Invalid" not in last and "Error" not in last:
|
||||
return last
|
||||
return last
|
||||
return _do(command)
|
||||
|
||||
Reference in New Issue
Block a user