From 61d41f3d00b74585cbf90fa44b8127ab08b70e2d Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:30:52 +0000 Subject: [PATCH] Commit the lab topology, and let a test guard it `netclab_topology` derives the lab's cabling from the AVD model, and had no test at all: no unit test, and its only consumer is `kind-up.sh`, which CI never runs because cEOS is licensed and cannot be pulled in Actions. So an AVD upgrade that changed the peering would have been invisible -- the topology was regenerated into a temp file on a developer's laptop and never reviewed. Generating the default subset once and committing it makes that diff visible, and `test_topology_golden` fails when the committed file stops matching what the generator produces. It runs offline with the rest of the suite, so it fits the constraint that keeps the cEOS path out of CI. `kind-up.sh` now reads the committed file at the default subset, so the lab boots the reviewed artifact rather than something regenerated beside it. A different `LAB_HOSTS` still regenerates: --hosts is an input to generation, not a filter applied to the result, because a link survives only when both of its ends are selected -- cutting nodes out of a finished topology would leave networks with one end. The generator is fed `examples/fabric/single-dc-l3ls.yaml` directly instead of the rendered `examples/lab` overlay, which drops a `kubectl kustomize` from the path. Checked before relying on it: both inputs produce byte-identical output apart from the provenance header, because the overlay only adds `management_eapi` and `push`, and neither affects cabling. The committed file sits outside `--examples-root=examples/fabric`, so it does not reach the Configuration package -- it has no `kind` and would fail the build. Verified by building it. Co-Authored-By: Claude Opus 5 --- README.md | 8 ++++++ examples/lab/topology.yaml | 22 +++++++++++++++ scripts/kind-up.sh | 31 ++++++++++++++++----- tests/test_topology_golden.py | 52 +++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 examples/lab/topology.yaml create mode 100644 tests/test_topology_golden.py diff --git a/README.md b/README.md index 753a4db..9de036b 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,11 @@ recomputes fabric-wide facts. `scripts/kind-down.sh` tears it all down. itself: AVD resolves the peering, so `avd-topology` derives the netclab topology from the same model that renders the configs, and the two cannot drift apart. +The default subset's topology is generated once and committed as +[`examples/lab/topology.yaml`](examples/lab/topology.yaml), so the lab boots a reviewed +file and an AVD upgrade that changes the cabling fails `test_topology_golden` instead of +passing unnoticed. Regenerate it with the command in that test's docstring. + ```bash WITH_NETCLAB=1 scripts/kind-up.sh # + CNI plugins, Multus, netclab-chart, cEOS nodes kubectl --context kind-avd apply -k examples/lab/ @@ -56,6 +61,9 @@ kubectl --context kind-avd apply -k examples/lab/ cEOS is licensed and cannot be pulled — import it once and it outlives teardown in the registry's data volume; `kind-up.sh` says how if it is missing. `LAB_HOSTS` picks the subset, defaulting to the two that make one link, because all eight is 16Gi of cEOS. +Setting it to anything else regenerates the topology, since `--hosts` is an input to +generation rather than a filter applied to the result — a link survives only when both +of its ends are selected. `examples/lab/` is that same fabric with eAPI bound to the default VRF, which is the one thing a lab genuinely needs and production does not: AVD binds eAPI to VRF MGMT on diff --git a/examples/lab/topology.yaml b/examples/lab/topology.yaml new file mode 100644 index 0000000..ac9bdd4 --- /dev/null +++ b/examples/lab/topology.yaml @@ -0,0 +1,22 @@ +# Generated from examples/fabric/single-dc-l3ls.yaml by avd-topology -- do not edit. +# b1: dc1-leaf1a:Ethernet1 <-> dc1-spine1:Ethernet1 +topology: + networks: + - name: b1 + nodes: + - name: dc1-spine1 + type: ceos + image: localhost:5001/netclab/ceos:4.36.1F + memory: 2Gi + cpu: 1000m + interfaces: + - name: eth1 + network: b1 + - name: dc1-leaf1a + type: ceos + image: localhost:5001/netclab/ceos:4.36.1F + memory: 2Gi + cpu: 1000m + interfaces: + - name: eth1 + network: b1 diff --git a/scripts/kind-up.sh b/scripts/kind-up.sh index 9290cd4..1031269 100755 --- a/scripts/kind-up.sh +++ b/scripts/kind-up.sh @@ -55,7 +55,15 @@ NETCLAB_CHART=${NETCLAB_CHART:-0.5.9} PROVIDER_HTTP=${PROVIDER_HTTP:-v1.0.14} # The subset of the fabric to actually run. All 8 devices is 16Gi of cEOS; two # make a link, and a link is enough to prove the config reached the device. -LAB_HOSTS=${LAB_HOSTS:-dc1-spine1,dc1-leaf1a} +# At the default, the committed topology is used as-is; anything else is +# regenerated from LAB_FABRIC below. +LAB_HOSTS_DEFAULT=dc1-spine1,dc1-leaf1a +LAB_HOSTS=${LAB_HOSTS:-$LAB_HOSTS_DEFAULT} +# The Fabric the lab runs, and the topology derived from it. Keep the two in +# step: regenerate with +# uv run avd-topology "$LAB_FABRIC" --hosts "$LAB_HOSTS_DEFAULT" > "$LAB_TOPOLOGY" +LAB_FABRIC=${LAB_FABRIC:-examples/fabric/single-dc-l3ls.yaml} +LAB_TOPOLOGY=${LAB_TOPOLOGY:-examples/lab/topology.yaml} echo ">> local registry (data volume: ${REG_VOL})" if [ -z "$(docker ps -q -f name="^${REG}$")" ]; then @@ -167,12 +175,21 @@ EOF # The topology is derived from the lab fabric, not written by hand: AVD resolves # the cabling, so the lab cannot be wired differently from the config it runs. - echo ">> lab topology from examples/lab (${LAB_HOSTS})" - TOPO="$(mktemp -t netclab-topology.XXXXXX.yaml)" - LAB_FABRIC="$(mktemp -t lab-fabric.XXXXXX.yaml)" - trap 'rm -f "$TOPO" "$LAB_FABRIC"' EXIT - kubectl kustomize examples/lab > "$LAB_FABRIC" - uv run avd-topology "$LAB_FABRIC" --hosts "$LAB_HOSTS" > "$TOPO" + # + # The default subset is generated once and committed, so this boots a + # reviewed artifact and `test_topology_golden` can fail when an AVD upgrade + # changes the peering. Asking for a different subset regenerates, because + # --hosts is an input to generation and not a filter applied afterwards: a + # link survives only when both of its ends are selected. + if [ "$LAB_HOSTS" = "$LAB_HOSTS_DEFAULT" ]; then + echo ">> lab topology from ${LAB_TOPOLOGY} (${LAB_HOSTS})" + TOPO="$LAB_TOPOLOGY" + else + echo ">> lab topology regenerated for ${LAB_HOSTS}" + TOPO="$(mktemp -t netclab-topology.XXXXXX.yaml)" + trap 'rm -f "$TOPO"' EXIT + uv run avd-topology "$LAB_FABRIC" --hosts "$LAB_HOSTS" > "$TOPO" + fi # cEOS cannot be pulled: it is licensed and needs an Arista login. Fail here # with the tag the topology asks for, rather than as an ImagePullBackOff later. diff --git a/tests/test_topology_golden.py b/tests/test_topology_golden.py new file mode 100644 index 0000000..5aa3486 --- /dev/null +++ b/tests/test_topology_golden.py @@ -0,0 +1,52 @@ +"""`examples/lab/topology.yaml` must be what the generator produces today. + +The lab topology is derived from the AVD model rather than written by hand, so +the cabling cannot drift from the config it runs. Committing the result buys +two things a temp file could not: + +- `scripts/kind-up.sh` boots a reviewed artifact, and consumers outside this + repo can fetch it at a tag instead of running the generator themselves. +- **An AVD upgrade that changes the cabling shows up as a failing test here.** + Without it, the topology is regenerated at bring-up on a developer's laptop + and a changed peering is invisible in review. This is the only guard on + `netclab_topology`, whose sole other consumer is a script CI never runs -- + the cEOS path cannot run in Actions, since the image is licensed. + +If this fails because the model legitimately moved, regenerate: + + uv run avd-topology examples/fabric/single-dc-l3ls.yaml \ + --hosts dc1-spine1,dc1-leaf1a > examples/lab/topology.yaml + +and read the diff before committing it -- that diff is the point. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +GOLDEN = ROOT / "examples" / "lab" / "topology.yaml" + +# The arguments the committed file was generated with. `kind-up.sh` runs the +# same subset by default; `--hosts` is an input to generation, not a filter +# applied afterwards, because a link is kept only when both of its ends are +# selected. +FABRIC = "examples/fabric/single-dc-l3ls.yaml" +HOSTS = "dc1-spine1,dc1-leaf1a" + + +def test_committed_topology_matches_the_generator() -> None: + generated = subprocess.run( + [sys.executable, "-m", "function.netclab_topology", FABRIC, "--hosts", HOSTS], + cwd=ROOT, + capture_output=True, + text=True, + check=True, + ).stdout + + assert generated == GOLDEN.read_text(), ( + f"{GOLDEN.relative_to(ROOT)} is stale -- regenerate it (see this " + f"module's docstring) and review the diff." + )