Stop stale saved/appended startup-configs from hijacking the management IP#495
Open
alxrxs wants to merge 3 commits into
Open
Stop stale saved/appended startup-configs from hijacking the management IP#495alxrxs wants to merge 3 commits into
alxrxs wants to merge 3 commits into
Conversation
98795f1 to
c49b751
Compare
c49b751 to
b6fe149
Compare
Two additions to the shared launcher library, both used by the
per-vendor launcher changes that follow.
strip_mgmt_interface_config(config_text, mgmt_intf, style) removes the
management interface's address configuration from a chunk of device
config -- a user-supplied or previously-saved startup-config that a
launcher appends to / merges with its own launcher-generated management
stanza. Every vrnetlab launcher configures the node's management
interface itself, each boot, from the container's actual IP; if the
appended config also sets an address on that interface (very likely if
it came from `containerlab save`, which captures the full running-config,
or was hand-written), whichever the device applies last wins, and a
stale value leaves the node "healthy" but unreachable at the address
clab/DNS expect.
It is keyed on the interface NAME, not the address value, deliberately:
the stale address is by definition a *different* value than the one the
launcher just configured, so a value-based filter can't catch it -- only
the interface identity is invariant. It drops every address the appended
config puts on that interface, whatever the value, leaving the launcher's
own address the only one. Structure-aware: "ios" walks the
`interface <name>` block (whitespace/case-insensitive name match, since
IOS-XR accepts a space in the name and different launchers format it both
ways) and drops `ip|ipv4|ipv6 address` leaves while keeping the header;
"junos" handles both hierarchical (`<name> { ... address x; ... }`,
brace-scoped, `address x { ... }` option-blocks dropped whole) and flat
set-style, matching the `family inet[6] address` element so a
`description` mentioning "address" is left alone.
The qemu-guest-agent channel is a virtio-serial chardev on a per-VM unix
socket (/run/qga<n>.sock) wired to org.qemu.guest_agent.0, alongside the
existing monitor/serial chardevs. A unix socket rather than a TCP port
avoids any collision with the host-forwarded mgmt ports regardless of how
many VMs a node runs. It gives an out-of-band path into the guest --
guest-exec over the guest-agent protocol -- that works even when the
node's management plane hasn't come up and the serial console has no
interactive getty. Inert unless qemu-guest-agent is installed in the
guest.
…nfigs
Wire strip_mgmt_interface_config into every launcher that applies a
user/saved startup-config on top of its own launcher-generated management
stanza, passing that platform's own management interface name and config
style so the launcher's address always wins:
ios: xrd-vrouter/xrv9k MgmtEth0/RP0/CPU0/0, xrv MgmtEth0/0/CPU0/0,
csr1000v/c8000v GigabitEthernet1, cat9kv GigabitEthernet0/0,
n9kv/nxos mgmt0, asav Management0/0
junos: vjunosevolved re0:mgmt-0; vjunosrouter/vjunosswitch/vsrx/vmx
fxp0; vqfx em0
Names are taken from each launcher's own generated stanza / init.conf, so
they match whatever the device emits into a saved config. The four
"cat-style" juniper launchers that built juniper.conf via a shell
`cat init.conf $STARTUP_CONFIG_FILE >> juniper.conf` are changed to an
explicit read/filter/write so the filter can run in between.
Deliberately not touched: vios (its entire config, management interface
included, comes from the startup-config alone -- there is no launcher-
owned stanza to protect, so stripping would remove the only mgmt config),
and c8000v's MIME-wrapped startup-config branch (same reason for that
path).
Two supporting bits:
- xrd-vrouter also gains an `ipv6 address` line on its MgmtEth stanza
(when the clab network has IPv6), matching xrv9k/xrv: the strip removes
every ip/ipv4/ipv6 address the appended config sets on MgmtEth, so
without the launcher writing IPv6 too a dual-stack node would lose its
IPv6 management address with nothing re-adding it.
- xrd-vrouter's Dockerfile now names vrnetlab.py in its COPY. The generic
build stages common/vrnetlab.py into the build context, and kinds that
use `COPY *.py /` pick it up automatically, but this Dockerfile lists
files explicitly -- so without naming it, the image kept the older
/vrnetlab.py from the pinned base and launch.py's call into the current
common lib would fail with AttributeError.
guest-init.sh's wait loop only broke early on MgmtEth reaching Up/Up; on the iteration-count timeout it fell through and emitted the CLAB_XRD_READY marker unconditionally anyway. The launcher sets running=True on that marker and writes "0 running" to /health regardless of whether management came up -- so a node whose MgmtEth never got assigned still reported "healthy" to docker/containerlab, with nothing to say why SSH wasn't answering. Now the marker is emitted only once "show ipv4 vrf all interface brief" confirms MgmtEth is Up/Up. On failure guest-init dumps diagnostics to the console instead (show configuration failed startup, show running-config/interfaces/ipv4 for MgmtEth, show logging) and exits without signalling ready, so the node correctly stays unhealthy and the reason is visible in `docker logs` on every boot attempt. make-golden.sh installs and enables qemu-guest-agent in the golden image, so the guest-agent channel added to common/vrnetlab.py has something answering on it for this kind: `guest-exec docker exec xrd /pkg/bin/xr_cli ...` works even when management is down and the golden image's serial console has no interactive getty (serial-getty@ttyS0 is masked by design during provisioning).
b6fe149 to
2301267
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every vrnetlab launcher configures the node's management interface itself, every boot, from the container's actual clab-assigned IP -- that's the real source of truth for management addressing. Several launchers also apply a user-supplied startup-config after that (commonly one produced by
containerlab save, which captures the full running-config including whatever the device currently has on its management interface). When both configure the same interface, the appended one competes with the launcher's: on IOS-family platforms (IOS-XR/IOS-XE/NX-OS/ASA) a later address under the same interface replaces it, so a stale one makes the node unreachable; on Junos it is added alongside (Junos keeps multiple addresses per interface), so the node stays reachable via the launcher's address but carries a stale/foreign one. Either way the launcher's address should be the interface's only one.If that's a stale saved address -- which happens whenever the container is later assigned a different mgmt IP (a partial
--node-filterdeploy, Docker IPAM handing out a different address, etc.), or when a hand-written config carries its own mgmt address -- the node comes up completely healthy and functional but with the wrong (or an extra) address on its management interface -- unreachable at the IP clab/DNS expect on the IOS-family kinds, and carrying a stale/duplicate mgmt address on Junos. Nothing in the container healthcheck or the launcher's readiness signal catches this: from clab's point of view the node is "healthy."I hit and root-caused this on my own lab running the XRd vRouter kind:
containerlab savehad captured a running-config withMgmtEth0/RP0/CPU0/0pinned to one IP; a later redeploy gave the container a different IP; the node reportedhealthyand nothing answered ARP for the address clab reported. I filed a companion save-side fix in containerlab (srl-labs/containerlab#3263) socontainerlab savestops writing the mgmt address into the saved config in the first place -- this PR is the launcher-side half, which also covers configs already saved with an address, or hand-written ones.Approach: strip the mgmt interface's address by name
common/vrnetlab.pygainsstrip_mgmt_interface_config(config_text, mgmt_intf, style), and each launcher calls it on the startup-config it's about to append/merge, passing its own management interface name.It's keyed on the interface name, not the address value, and that distinction is the whole point: the failure is a stale address -- a different value than the one the launcher just configured -- so a value-based filter can never catch it (it would only match when the appended address already equals the current one, i.e. exactly when there's no conflict). The interface identity is the only invariant, so the filter drops every address the appended config puts on that interface, whatever the value, leaving the launcher's own as the only one.
Structure-aware, not a blind line delete:
style="ios"(IOS / IOS-XE / IOS-XR / NX-OS / ASA): walks theinterface <name>block and dropsip/ipv4/ipv6 addressleaves, keeping the interface header and any other per-interface config. The name match is whitespace/case-insensitive (IOS-XR acceptsMgmtEth 0/RP0/CPU0/0andMgmtEth0/RP0/CPU0/0interchangeably and launchers emit both).style="junos": handles both hierarchical (<name> { ... address x; ... }, brace-scoped, withaddress x { ... }option-blocks removed whole so braces stay balanced) and flat set-style, matching thefamily inet[6] addresselement so adescriptionthat merely mentions "address" is left alone.MgmtEth0/RP0/CPU0/0MgmtEth0/0/CPU0/0GigabitEthernet1GigabitEthernet0/0mgmt0Management0/0re0:mgmt-0fxp0em0vios(its entire config, mgmt interface included, comes from the startup-config alone -- no launcher stanza to protect) andc8000v's MIME-wrapped startup-config branch are deliberately left untouched; stripping either would remove the only source of mgmt config.Also in this PR (XRd-vRouter reliability, how I found the bug)
common/vrnetlab.py) -- a virtio-serial chardev on a per-VM unix socket (/run/qga<n>.sock) wired toorg.qemu.guest_agent.0, plus qemu-guest-agent installed in the xrd-vrouter golden image. A unix socket (rather than a TCP port) avoids any collision with the host-forwarded mgmt ports no matter how many VMs a node runs; nothing in the launcher connects to it -- it's reached bydocker exec-ing into the container and talking to the socket. This is what let me pull the actual root cause out of a node that was "healthy" but unreachable, whose serial console has no interactive getty. Inert on other kinds unless they install the agent.guest-init.sh's wait loop emitted the readiness marker on timeout even if MgmtEth never came up, so the launcher reportedhealthyregardless. Now the marker is gated on a real Up/Up check; on failure it dumps diagnostics to the console instead.vrnetlab.pyin itsCOPY. Unlike kinds thatCOPY *.py /, this Dockerfile lists files explicitly, so without it the image kept the older/vrnetlab.pyfrom the pinned base andlaunch.py's calls into the current common lib crashed withAttributeError.Testing
The
strip_mgmt_interface_confighelper was verified against a standalone matrix of adversarial inputs (this repo has no Python test harness, so it is not committed as a CI test): IOS-XR/IOS-XE/NX-OS leaf drops with the header preserved; whitespace-tolerant name match; a stale address different from anything passed in still dropped because the match is on the interface name; other-interface-untouched; a flush-left address leaf (with and without a preceding non-address leaf); Junos hierarchical leaf + dual-stack +address { ... }option-block; Junos set-style; and adescriptioncarrying both the mgmt IP and the word "address" (IOS, Junos hierarchical, and Junos set-style) left alone. (The containerlab companion PR does commit an equivalent Go test,nodes/vr_node_mgmtfilter_test.go.)End-to-end on a real containerlab lab, both platforms I run, from images built via this branch's build path:
MgmtEth0/RP0/CPU0/0v4 and v6 address. Both stripped from the builtfirst-boot.cfg; MgmtEth came up Up/Up at the real container IP on both families;ssh clab@<real-ip>works.re0:mgmt-0address. Stripped from the builtjuniper.conf;re0:mgmt-0kept the launcher value;ssh admin@<real-ip>works.I don't have running instances of the other 13 patched kinds, so those are verified by reading each launcher's actual config-application path before changing it (not assumed from a sibling) plus
python -m aston every file. Happy to adjust anything that doesn't match a maintainer's closer knowledge of a specific platform.