From 802ba30d8a11520b37b3cecd6aa936d95f9d0dad Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:16:21 +0000 Subject: [PATCH 1/8] Resolve a fabric's inputs per device, the way Ansible resolves group_vars A Fabric will name several input XRs, each carrying a fragment of the eos_designs document plus the devices that see it. This lands the resolution: per device, layer the inputs that apply, in order, with dict.update(). Nothing is merged. Two NodeSets carrying the same node-type key never meet, because no device sees both -- a dual-DC fabric's leaves each see their own l3leaf.defaults. That is what pyavd.get_avd_facts already expects to be handed; engine.render_fabric_design flattens it today by giving every device the same document, and xr.fabric_design_from_inputs exists to squeeze many hostvars back into one. Neither is needed on this path, so MergeOnSchema, a duplicate-key conflict rule and the defaults push-down are all avoided -- and with them any dependency on pyavd's private API. Two things that look like one are kept apart: which devices an input *declares* (the union of these is the fabric's device list, and there is no second list) and which devices *see* it (appliesTo). They coincide in simple topologies and diverge in a 5-stage CLOS, where a DC's super_spine block names four devices but is visible to all sixteen of that DC. Measured against AVD's own corpus rather than argued: the hostvars this produces are byte-identical to faithfully reproduced Ansible across all 8 bundled examples and every molecule scenario with an inventory of its own -- 25 inventories, up to 501 devices, in five seconds because nothing renders. The test is stricter than a render comparison on purpose, so a divergence cannot hide until it matters. Co-Authored-By: Claude Opus 5 --- function/kinds.py | 167 ++++++++++++++++++++ function/verify_kinds.py | 263 ++++++++++++++++++++++++++++++++ pyproject.toml | 1 + tests/test_kinds_equivalence.py | 89 +++++++++++ 4 files changed, 520 insertions(+) create mode 100644 function/kinds.py create mode 100644 function/verify_kinds.py create mode 100644 tests/test_kinds_equivalence.py diff --git a/function/kinds.py b/function/kinds.py new file mode 100644 index 0000000..7497fd4 --- /dev/null +++ b/function/kinds.py @@ -0,0 +1,167 @@ +"""The input-kind model: several XRs layered into per-device AVD inputs. + +A ``Fabric`` names its inputs in ``spec.requires``. Each input XR carries a +fragment of the eos_designs document in ``spec.design`` plus ``spec.appliesTo`` +saying which devices see it. Per device, the inputs that apply are layered in +``requires`` order with ``dict.update()`` -- Ansible's default +``hash_behaviour=replace``, which is what group_vars resolution does and what +``pyavd.get_avd_facts`` expects to be handed. + +**Nothing is merged.** Two NodeSets carrying the same node-type key never meet, +because no device sees both: in a dual-DC fabric a DC1 leaf sees DC1's +``l3leaf.defaults`` and a DC2 leaf sees DC2's. That is why this path needs +neither a fabric-wide document nor the fold in :mod:`function.xr`. + +Two things are separate that look like one: + +* which devices an input *declares* (``spec.declares``, plus the nodes its + blocks name) -- the union of these is the fabric's device list, and there is + no second list; +* which devices *see* it (``spec.appliesTo``). They coincide in simple + topologies and diverge in a 5-stage CLOS, where a DC's ``super_spine`` block + names four devices but is visible to every device of that DC. + +Measured against AVD's own corpus: the hostvars this produces are byte-identical +to faithfully reproduced Ansible for all 8 bundled examples and every eos_designs +molecule scenario -- 25 inventories, up to 501 devices. See +:mod:`function.verify_kinds`. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any + +KINDS = ("NodeSet", "NetworkServices", "ConnectedEndpoints", "Settings") + + +def is_node_block(value: Any) -> bool: + """A node-type block is a dict carrying ``nodes`` and/or ``node_groups``.""" + return isinstance(value, dict) and ("nodes" in value or "node_groups" in value) + + +def hosts_in_blocks(design: dict) -> set[str]: + """Device names a design's node-type blocks mention.""" + hosts: set[str] = set() + for value in design.values(): + if not is_node_block(value): + continue + groups = list(value.get("node_groups") or []) + for nodes in [value.get("nodes") or []] + [g.get("nodes") or [] for g in groups]: + for node in nodes: + if isinstance(node, dict) and node.get("name"): + hosts.add(node["name"]) + return hosts + + +def classify(design: dict) -> str: + """Which kind a fragment belongs to. + + Advisory: the kinds exist for ownership (RBAC is granted per kind), not as a + partition the schema could enforce -- eos_designs' top-level key names come + from its own content, so no OpenAPI schema can describe them. + """ + if any(is_node_block(v) for v in design.values()): + return "NodeSet" + if {"tenants", "network_services_keys"} & design.keys(): + return "NetworkServices" + if { + "servers", "firewalls", "routers", "load_balancers", "storage_arrays", + "cpes", "workstations", "access_points", "phones", "printers", + "generic_devices", "port_profiles", "network_ports", + "connected_endpoints_keys", "custom_connected_endpoints_keys", + } & design.keys(): + return "ConnectedEndpoints" + return "Settings" + + +@dataclass +class Input: + """One input XR, reduced to what resolution needs.""" + + name: str + kind: str + design: dict + # spec.appliesTo -- exactly one of the three + all_devices: bool = False + node_sets: list[str] = field(default_factory=list) + hosts: list[str] = field(default_factory=list) + # spec.declares -- devices this input brings into the fabric + declares: list[str] = field(default_factory=list) + + @classmethod + def from_xr(cls, xr: dict) -> "Input": + """Build from an XR as ``required_resources`` delivers it.""" + spec = xr.get("spec") or {} + applies = spec.get("appliesTo") or {} + design = spec.get("design") or {} + declares = list(spec.get("declares") or []) + kind = xr.get("kind") or classify(design) + if kind == "NodeSet" and not declares: + # A NodeSet that declares nothing explicitly declares what its + # blocks name -- the common case, where the two coincide. + declares = sorted(hosts_in_blocks(design)) + return cls( + name=(xr.get("metadata") or {}).get("name", ""), + kind=kind, + design=design, + all_devices=bool(applies.get("all")), + node_sets=list(applies.get("nodeSets") or []), + hosts=list(applies.get("hosts") or []), + declares=declares, + ) + + def scope(self, declared_by: dict[str, set[str]], devices: set[str]) -> set[str]: + """Devices that see this input.""" + if self.all_devices: + return devices + if self.node_sets: + named: set[str] = set() + for name in self.node_sets: + named |= declared_by.get(name, set()) + return devices & named + return devices & set(self.hosts) + + +def resolve(inputs: list[Input]) -> dict[str, dict]: + """Layer ordered inputs into ``{hostname: hostvars}`` for ``get_avd_facts``. + + List order is precedence order: later inputs overwrite earlier ones key by + key, whole-key, exactly as Ansible resolves group_vars. An overwrite is + therefore intentional -- it is what the fabric owner declared by ordering -- + and belongs on status as a warning, never as an error. + """ + declared_by = {i.name: set(i.declares) for i in inputs if i.declares} + devices: set[str] = set() + for hosts in declared_by.values(): + devices |= hosts + + out: dict[str, dict] = {host: {} for host in devices} + for inp in inputs: + for host in inp.scope(declared_by, devices): + out[host].update(inp.design) + return out + + +def overwrites(inputs: list[Input]) -> list[tuple[str, str, str, str]]: + """``(device, key, earlier input, later input)`` for every value replaced. + + Ansible resolves these silently. Here the ordering is written down by a + person, so surfacing them is cheap and worth doing -- on status, as a + warning. + """ + declared_by = {i.name: set(i.declares) for i in inputs if i.declares} + devices: set[str] = set() + for hosts in declared_by.values(): + devices |= hosts + + seen: dict[tuple[str, str], tuple[str, Any]] = {} + found: list[tuple[str, str, str, str]] = [] + for inp in inputs: + for host in inp.scope(declared_by, devices): + for key, value in inp.design.items(): + previous = seen.get((host, key)) + if previous is not None and previous[1] != value: + found.append((host, key, previous[0], inp.name)) + seen[(host, key)] = (inp.name, value) + return found diff --git a/function/verify_kinds.py b/function/verify_kinds.py new file mode 100644 index 0000000..bc92174 --- /dev/null +++ b/function/verify_kinds.py @@ -0,0 +1,263 @@ +"""Prove the input-kind model reproduces Ansible's variable resolution. + +Translates an AVD inventory into input XRs, resolves them with +:func:`function.kinds.resolve` -- which never looks at the inventory again -- +and compares the resulting hostvars against faithfully reproduced Ansible. + +Byte equality is the assertion. It is stricter than necessary (a hostvar AVD +never reads cannot change a rendered config) and that is deliberate: it fails +before a render can hide a difference. + +The translation reads the inventory; the resolution does not. Only the second +half is the model. What it establishes: + +* ``spec.requires`` order reproduces Ansible precedence. Ansible sorts groups by + (depth, name), which is a *global* total order, so one flat list restricted to + the inputs that apply to a device reproduces that device's view. +* ``appliesTo`` reproduces group membership without groups. +* One device list -- what NodeSets declare -- reproduces the inventory. + +Usage: + uv run avd-verify-kinds [EXAMPLE_DIR ...] # default: every bundled example +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +from .ansible_inputs import ( + ALL_GROUP, + AnsibleInventory, + _load_group_vars, + _strip_ansible_keys, + _yaml_load, +) +from .kinds import Input, classify, hosts_in_blocks, is_node_block, resolve + +EXAMPLES_ROOT = Path("avd/ansible_collections/arista/avd/examples") +MOLECULE_ROOT = Path("avd/ansible_collections/arista/avd/extensions/molecule") + +# Examples this harness cannot translate, with the reason. Expected-failure +# semantics, as in verify_xr: one that starts passing IS reported, so a deferral +# can never rot silently. +DEFERRED: dict[str, str] = {} + + +def inline_host_vars(inventory_file: Path) -> dict[str, dict]: + """Host variables written straight into the inventory. + + ``AnsibleInventory`` iterates the keys under ``hosts:`` and drops the values, + so ``dc1-spine1: {type: spine}`` is invisible to it. Several molecule + scenarios declare device types that way and nothing else does. + """ + found: dict[str, dict] = {} + + def walk(node: object) -> None: + if not isinstance(node, dict): + return + for host, hostvars in (node.get("hosts") or {}).items(): + if isinstance(hostvars, dict): + stripped = _strip_ansible_keys(hostvars) + if stripped: + found.setdefault(host, {}).update(stripped) + for child in (node.get("children") or {}).values(): + walk(child) + + for value in (_yaml_load(inventory_file) or {}).values(): + walk(value) + return found + + +def _layout(root: Path) -> tuple[Path, Path]: + """(directory holding group_vars, inventory file) for either layout.""" + if (root / "inventory.yml").is_file(): + return root, root / "inventory.yml" + return root / "inventory", root / "inventory" / "hosts.yml" + + +def ansible_hostvars(root: Path) -> dict[str, dict]: + """What ansible-playbook would hand to AVD, from every source it reads.""" + var_dir, inventory_file = _layout(root) + inventory = AnsibleInventory.from_file(inventory_file) + inline = inline_host_vars(inventory_file) + host_var_dir = var_dir / "host_vars" + files = ( + {f.stem: _strip_ansible_keys(_yaml_load(f)) for f in host_var_dir.glob("*.yml")} + if host_var_dir.is_dir() + else {} + ) + cache: dict[str, dict] = {} + out: dict[str, dict] = {} + for host in sorted(inventory.hosts()): + hostvars: dict = {} + for group in inventory.groups_for_host(host): + if group not in cache: + cache[group] = _load_group_vars(var_dir / "group_vars", group) + hostvars.update(cache[group]) + hostvars = _strip_ansible_keys(hostvars) + hostvars.update(inline.get(host, {})) + hostvars.update(files.get(host, {})) + out[host] = hostvars + return out + + +def inputs_from_inventory(root: Path) -> list[Input]: + """Translate an AVD inventory into ordered input XRs -- a migration. + + Each group_vars file becomes one or two inputs: a ``NodeSet`` for its + node-type blocks and one input for the rest. The split is not cosmetic -- + the two halves have different scopes whenever a block names fewer devices + than the group holds, which is what a 5-stage CLOS does. + """ + var_dir, inventory_file = _layout(root) + inventory = AnsibleInventory.from_file(inventory_file) + group_var_dir = var_dir / "group_vars" + every_device = inventory.hosts() + + groups = [] + if group_var_dir.is_dir(): + named = {p.stem for p in group_var_dir.glob("*.yml")} | { + d.name for d in group_var_dir.iterdir() if d.is_dir() + } + groups = [g for g in named if g in inventory.depth or g == ALL_GROUP] + # Ansible precedence: `all` first, then (depth, name). This is requires order. + ordered = sorted(groups, key=lambda g: (inventory.depth.get(g, 0), g)) + + def group_devices(group: str) -> set[str]: + if group == ALL_GROUP: + return set(every_device) + return {h for h in every_device if group in inventory.groups_for_host(h)} + + designs = {g: _strip_ansible_keys(_load_group_vars(group_var_dir, g)) for g in ordered} + declared_by: dict[str, set[str]] = {} + for group, design in designs.items(): + blocks = {k: v for k, v in design.items() if is_node_block(v)} + if blocks: + declared_by[group] = hosts_in_blocks(blocks) & set(every_device) + + # Devices no block mentions -- fixtures with no node-type blocks at all, and + # hosts that only the inventory knows about. Declared at the narrowest group + # holding each one rather than in a single fabric-wide NodeSet, so the + # resulting NodeSets line up with real groups and other inputs can name them. + device_sets: dict[str, set[str]] = {} + for host in sorted(set(every_device) - set().union(*declared_by.values() or [set()])): + deepest = inventory.groups_for_host(host)[-1] # already (depth, name) sorted + device_sets.setdefault(f"{deepest}-devices", set()).add(host) + declared_by.update(device_sets) + + inputs = [ + Input(name, "NodeSet", {}, node_sets=[name], declares=sorted(hosts)) + for name, hosts in sorted(device_sets.items()) + ] + + for group in ordered: + design = designs[group] + if not design: + continue + blocks = {k: v for k, v in design.items() if is_node_block(v)} + rest = {k: v for k, v in design.items() if not is_node_block(v)} + want = group_devices(group) + + def scoped(name: str, kind: str, payload: dict, want: set[str] = want) -> Input: + inp = Input(name=name, kind=kind, design=payload) + if want == set(every_device): + inp.all_devices = True + else: + cover = [n for n, hs in declared_by.items() if hs and hs <= want] + covered: set[str] = set() + for n in cover: + covered |= declared_by[n] + if covered == want: + inp.node_sets = sorted(cover) + else: + # No union of NodeSets is this group -- name the devices. + inp.hosts = sorted(want) + return inp + + if blocks: + node_set = scoped(group, "NodeSet", blocks) + node_set.declares = sorted(declared_by[group]) + inputs.append(node_set) + if rest: + inputs.append(scoped(f"{group}-settings" if blocks else group, classify(rest), rest)) + + # host_vars last, as Ansible does: inventory inline first, then files. + for host, design in sorted(inline_host_vars(inventory_file).items()): + inputs.append(Input(f"{host}-inline", classify(design), design, hosts=[host])) + host_var_dir = var_dir / "host_vars" + if host_var_dir.is_dir(): + for f in sorted(host_var_dir.glob("*.yml")): + design = _strip_ansible_keys(_yaml_load(f)) + if design: + inputs.append(Input(f.stem, classify(design), design, hosts=[f.stem])) + return inputs + + +def verify_one(root: Path) -> tuple[str, int]: + """Return (status, difference count). status in {ok, differs, error}.""" + try: + from_kinds = resolve(inputs_from_inventory(root)) + from_ansible = ansible_hostvars(root) + except Exception as err: # noqa: BLE001 - surface any translation failure + return f"error: {type(err).__name__}: {str(err)[:70]}", -1 + + notes: list[str] = [] + for host in sorted(set(from_kinds) | set(from_ansible)): + if host not in from_kinds: + notes.append(f"{host}: missing") + continue + if host not in from_ansible: + notes.append(f"{host}: extra") + continue + a, b = from_kinds[host], from_ansible[host] + notes += [f"{host}.{k}" for k in sorted(set(a) | set(b)) if a.get(k) != b.get(k)] + if notes: + return f"differs ({len(notes)}): {', '.join(notes[:3])}", len(notes) + return "ok", 0 + + +def _discover(root: Path) -> list[Path]: + return sorted(d for d in root.iterdir() if (d / "inventory.yml").is_file()) + + +def _discover_molecule(root: Path = MOLECULE_ROOT) -> list[Path]: + """Molecule scenarios carrying an inventory of their own. + + The wider corpus, and the harder one: these reach 501 devices and lean on + inventory-inline host vars, which the examples barely use. + """ + if not root.is_dir(): + return [] + return sorted( + d + for d in root.iterdir() + if (d / "inventory" / "hosts.yml").is_file() and (d / "inventory" / "group_vars").is_dir() + ) + + +def main() -> int: + roots = [Path(a) for a in sys.argv[1:]] or _discover(EXAMPLES_ROOT) + failures = deferred = 0 + for root in roots: + status, _ = verify_one(root) + ok = status == "ok" + reason = DEFERRED.get(root.name) + if reason and not ok: + mark, deferred = "DEFER", deferred + 1 + status = f"deferred: {reason}" + elif reason and ok: + mark, failures = "XPASS", failures + 1 + status = "resolves now -- remove from DEFERRED" + elif ok: + mark = "OK " + else: + mark, failures = "FAIL", failures + 1 + print(f"[{mark}] {root.name:26s} {status}") + expected = len(roots) - deferred + print(f"\n{expected - failures}/{expected} inventories resolve identically to Ansible.") + return 1 if failures else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/pyproject.toml b/pyproject.toml index 824f76a..4854142 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,7 @@ dependencies = [ [project.scripts] avd-verify = "function.verify_example:main" avd-verify-xr = "function.verify_xr:main" +avd-verify-kinds = "function.verify_kinds:main" avd-function = "function.main:main" avd-topology = "function.netclab_topology:main" diff --git a/tests/test_kinds_equivalence.py b/tests/test_kinds_equivalence.py new file mode 100644 index 0000000..23956c7 --- /dev/null +++ b/tests/test_kinds_equivalence.py @@ -0,0 +1,89 @@ +"""The input-kind model resolves exactly as Ansible does. + +Offline -- no cluster, no pyavd render. Guards :func:`function.kinds.resolve` +and the migration that feeds it, over AVD's own corpus: the 8 bundled examples +and every molecule scenario with an inventory of its own, up to 501 devices. + +This is the regression net for the collect path. It is stricter than a render +comparison on purpose: it fails on a hostvar difference even where AVD would +have rendered the same config, so a divergence cannot hide until it matters. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from function.kinds import Input, resolve +from function.verify_kinds import ( + DEFERRED, + EXAMPLES_ROOT, + _discover, + _discover_molecule, + verify_one, +) + +CORPUS = _discover(EXAMPLES_ROOT) + _discover_molecule() + + +@pytest.mark.parametrize("root", CORPUS, ids=lambda p: p.name) +def test_resolves_identically_to_ansible(root: Path) -> None: + status, _ = verify_one(root) + + if root.name in DEFERRED: + # Expected failure. Asserting it still fails keeps a deferral from + # rotting: if it starts resolving, this fails and says to drop it. + assert status != "ok", ( + f"{root.name} resolves now -- remove it from verify_kinds.DEFERRED " + f"(was deferred: {DEFERRED[root.name]})" + ) + return + + assert status == "ok", f"{root.name}: {status}" + + +def test_corpus_is_not_empty() -> None: + # The submodule is optional in a fresh worktree; an empty parametrisation + # would make this whole file pass while testing nothing. + assert len(CORPUS) >= 8, f"expected the AVD corpus, found {len(CORPUS)} inventories" + + +def test_later_input_overwrites_earlier() -> None: + """Precedence is list order, and replacement is whole-key.""" + inputs = [ + Input("nodes", "NodeSet", {"l3leaf": {"nodes": [{"name": "leaf1"}]}}, + node_sets=["nodes"], declares=["leaf1"]), + Input("base", "Settings", {"ntp_settings": {"servers": ["a"]}}, all_devices=True), + Input("narrow", "Settings", {"ntp_settings": {"servers": ["b"]}}, hosts=["leaf1"]), + ] + assert resolve(inputs)["leaf1"]["ntp_settings"] == {"servers": ["b"]} + + +def test_input_applies_only_where_scoped() -> None: + """A device sees an input only if appliesTo names it -- this is what + replaces group membership, and what keeps two DCs' node blocks apart.""" + inputs = [ + Input("dc1", "NodeSet", {"l3leaf": {"defaults": {"loopback_ipv4_pool": "10.0.0.0/24"}}}, + node_sets=["dc1"], declares=["leaf1"]), + Input("dc2", "NodeSet", {"l3leaf": {"defaults": {"loopback_ipv4_pool": "10.1.0.0/24"}}}, + node_sets=["dc2"], declares=["leaf2"]), + ] + out = resolve(inputs) + assert out["leaf1"]["l3leaf"]["defaults"]["loopback_ipv4_pool"] == "10.0.0.0/24" + assert out["leaf2"]["l3leaf"]["defaults"]["loopback_ipv4_pool"] == "10.1.0.0/24" + + +def test_undeclared_node_is_not_a_device() -> None: + """A block may name a node the fabric does not declare -- AVD's own + anta_runner does -- and it must not become a device.""" + inputs = [ + Input( + "leaves", + "NodeSet", + {"l3leaf": {"nodes": [{"name": "leaf1"}, {"name": "ghost"}]}}, + node_sets=["leaves"], + declares=["leaf1"], + ) + ] + assert set(resolve(inputs)) == {"leaf1"} From 0c0a1572d7ce7951bfa91d0de24f37f8a5b91ccd Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:31:34 +0000 Subject: [PATCH 2/8] Render through the kinds path too, and diff against golden Resolution equivalence already proves the model: matching hostvars render identically, because it is the same function on the same input. So this adds nothing there, and that is not its job. It guards the pair (this path, this pyavd) -- an AVD upgrade that changes output slips past equivalence and fails here. That is what test_xr_fold does today through the fold, and this is its successor: the fold reaches 6 of the 8 examples, this reaches 7. campus-fabric is among the two the fold defers, with the reason "aaa_settings.radius differs by role; no node-scoped equivalent" -- there is no equivalent to find when nothing is folded, so it simply renders. Both nets run in parallel for now; removing the older one is a separate change, so the swap is visible in a diff rather than taken on trust. cv-pathfinder is deferred: its credentials are ansible-vault, and credentials cannot live in an XR spec. It carries XPASS semantics, so it will report itself the day that is fixed. Examples only -- the molecule scenarios need AVD features this path does not carry yet. Costs 5s: the offline suite goes 61 tests in 10.7s to 69 in 15.7s. Checked that the test can actually fail, by perturbing a golden value and watching it go red rather than by trusting that it would. Co-Authored-By: Claude Opus 5 --- function/verify_kinds.py | 58 ++++++++++++++++++++++++++++++--- tests/test_kinds_equivalence.py | 27 +++++++++++++++ 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/function/verify_kinds.py b/function/verify_kinds.py index bc92174..8485898 100644 --- a/function/verify_kinds.py +++ b/function/verify_kinds.py @@ -43,6 +43,13 @@ # can never rot silently. DEFERRED: dict[str, str] = {} +# The same, for --render. Resolution and rendering fail for different reasons: +# resolution is settled, rendering still runs into AVD features this path does +# not carry yet. +DEFERRED_RENDER: dict[str, str] = { + "cv-pathfinder": "ansible-vault secrets; credentials cannot live in an XR spec", +} + def inline_host_vars(inventory_file: Path) -> dict[str, dict]: """Host variables written straight into the inventory. @@ -217,6 +224,40 @@ def verify_one(root: Path) -> tuple[str, int]: return "ok", 0 +def render_one(root: Path) -> tuple[str, int]: + """Render through the kinds path and diff against the checked-in golden. + + This does **not** test the model -- if the hostvars match Ansible, and + :func:`verify_one` asserts they do, the render must match too. What it tests + is the pair (this path, this pyavd): an AVD upgrade that changes output slips + past resolution equivalence and fails here. That is the job `test_xr_fold` + does today through the fold, and this is its successor -- the fold reaches + 6 of 8 examples, this reaches 7. + """ + import yaml + + from .engine import render_structured_configs + from .verify_example import _diff + + golden = root / "intended" / "structured_configs" + if not golden.is_dir(): + return "no golden", -1 + try: + rendered = render_structured_configs(resolve(inputs_from_inventory(root))) + except Exception as err: # noqa: BLE001 - surface any AVD/render failure + return f"error: {type(err).__name__}: {str(err)[:70]}", -1 + + total = 0 + for hostname in sorted(rendered): + golden_file = golden / f"{hostname}.yml" + if not golden_file.is_file(): + continue # a scenario may render hosts it keeps no golden for + out: list[str] = [] + _diff(hostname, rendered[hostname], yaml.safe_load(golden_file.read_text()) or {}, out) + total += len(out) + return ("ok" if total == 0 else f"diff ({total})"), total + + def _discover(root: Path) -> list[Path]: return sorted(d for d in root.iterdir() if (d / "inventory.yml").is_file()) @@ -237,25 +278,32 @@ def _discover_molecule(root: Path = MOLECULE_ROOT) -> list[Path]: def main() -> int: - roots = [Path(a) for a in sys.argv[1:]] or _discover(EXAMPLES_ROOT) + args = [a for a in sys.argv[1:] if a != "--render"] + rendering = "--render" in sys.argv[1:] + roots = [Path(a) for a in args] or _discover(EXAMPLES_ROOT) + check = render_one if rendering else verify_one + deferrals = DEFERRED_RENDER if rendering else DEFERRED + failures = deferred = 0 for root in roots: - status, _ = verify_one(root) + status, _ = check(root) ok = status == "ok" - reason = DEFERRED.get(root.name) + reason = deferrals.get(root.name) if reason and not ok: mark, deferred = "DEFER", deferred + 1 status = f"deferred: {reason}" elif reason and ok: mark, failures = "XPASS", failures + 1 - status = "resolves now -- remove from DEFERRED" + status = "passes now -- remove from the DEFERRED map" elif ok: mark = "OK " else: mark, failures = "FAIL", failures + 1 print(f"[{mark}] {root.name:26s} {status}") + expected = len(roots) - deferred - print(f"\n{expected - failures}/{expected} inventories resolve identically to Ansible.") + what = "reproduce golden" if rendering else "resolve identically to Ansible" + print(f"\n{expected - failures}/{expected} inventories {what} ({deferred} deferred).") return 1 if failures else 0 diff --git a/tests/test_kinds_equivalence.py b/tests/test_kinds_equivalence.py index 23956c7..27376c8 100644 --- a/tests/test_kinds_equivalence.py +++ b/tests/test_kinds_equivalence.py @@ -18,13 +18,16 @@ from function.kinds import Input, resolve from function.verify_kinds import ( DEFERRED, + DEFERRED_RENDER, EXAMPLES_ROOT, _discover, _discover_molecule, + render_one, verify_one, ) CORPUS = _discover(EXAMPLES_ROOT) + _discover_molecule() +EXAMPLES = _discover(EXAMPLES_ROOT) @pytest.mark.parametrize("root", CORPUS, ids=lambda p: p.name) @@ -43,6 +46,30 @@ def test_resolves_identically_to_ansible(root: Path) -> None: assert status == "ok", f"{root.name}: {status}" +@pytest.mark.parametrize("root", EXAMPLES, ids=lambda p: p.name) +def test_render_reproduces_golden(root: Path) -> None: + """Rendered configs still match the checked-in golden. + + Redundant as a check on the model -- matching hostvars render identically -- + and that is not what it is for. It is the guard against pyavd itself + changing: an AVD upgrade slips past resolution equivalence and fails here. + + Examples only. The molecule scenarios need AVD features this path does not + carry yet (templates loaded from files, ID pools, custom Python classes), so + they stay on the equivalence test until those land. + """ + status, _ = render_one(root) + + if root.name in DEFERRED_RENDER: + assert status != "ok", ( + f"{root.name} renders clean now -- remove it from " + f"verify_kinds.DEFERRED_RENDER (was: {DEFERRED_RENDER[root.name]})" + ) + return + + assert status == "ok", f"{root.name}: {status}" + + def test_corpus_is_not_empty() -> None: # The submodule is optional in a fresh worktree; an empty parametrisation # would make this whole file pass while testing nothing. From b259c236fa5aff2e777032fcd5e123c8e3146e3b Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:31:28 +0000 Subject: [PATCH 3/8] Give a fabric its input kinds, and the API to name them Four XRDs -- NodeSet, NetworkServices, ConnectedEndpoints, Settings -- each carrying a fragment of the eos_designs document in an open spec.design, plus spec.appliesTo saying which devices see it. Fabric gains spec.requires, an ordered list naming its inputs. The kinds separate ownership, not content: RBAC is granted per kind, and eos_designs' top-level key names come from its own content, so no schema could partition them anyway. Only NodeSet declares devices. spec.declares is the fabric's device list and there is no second one -- a node named in a block the fabric does not declare is not a device. Never a pattern, either: visibility may be matched, existence may not, since a typo would silently drop devices. appliesTo takes all / nodeSets / hosts / matchHostnames. The pattern form copies AVD's own hostname matching from default_node_types -- and from the code, not the description: shared_utils/node_type.py anchors the pattern for you, while the schema's wording reads as though the author must. Copying the wording would have made the same pattern mean two things. A pattern matching nothing is an error rather than an empty set; kinds.unmatched_patterns() surfaces it, because a pattern is silent about matching nothing and the render is pushed as a full config replacement. Secret is in the requires enum from this first version. It is not implemented yet, but the mechanism it enables -- a Secret layered like any other input -- needs no other schema footprint, and adding the enum value after release would be a schema change to a published API. Credentials are not hypothetical here: the bundled examples carry sha512_password and type-7 BGP passwords, and type 7 is reversible, pyavd ships bgp_decrypt. All six XRDs gain categories [crossplane, netclab]. function-avd had them nowhere while netclab-xp carries them on all twelve, so `kubectl get netclab` returned nothing in the avd namespace. This lands the fix for Fabric and Device as well -- though it will only show on a fresh install, since Crossplane's dependency manager installs but does not upgrade. test_apis_consistency guards what a build cannot: that the XRDs and the kinds fn.py reconciles are the same set, that every XRD carries categories, and that each defaultCompositionRef resolves to a Composition for that kind. Checked it can fail, by dropping a categories block and watching it go red. Co-Authored-By: Claude Opus 5 --- apis/connectedendpoints/composition.yaml | 16 +++ apis/connectedendpoints/xrd.yaml | 103 ++++++++++++++++++ apis/device/xrd.yaml | 3 + apis/fabric/xrd.yaml | 42 +++++++- apis/networkservices/composition.yaml | 16 +++ apis/networkservices/xrd.yaml | 104 +++++++++++++++++++ apis/nodeset/composition.yaml | 16 +++ apis/nodeset/xrd.yaml | 126 +++++++++++++++++++++++ apis/settings/composition.yaml | 16 +++ apis/settings/xrd.yaml | 103 ++++++++++++++++++ function/fn.py | 32 ++++++ function/kinds.py | 58 +++++++++-- tests/test_apis_consistency.py | 81 +++++++++++++++ 13 files changed, 705 insertions(+), 11 deletions(-) create mode 100644 apis/connectedendpoints/composition.yaml create mode 100644 apis/connectedendpoints/xrd.yaml create mode 100644 apis/networkservices/composition.yaml create mode 100644 apis/networkservices/xrd.yaml create mode 100644 apis/nodeset/composition.yaml create mode 100644 apis/nodeset/xrd.yaml create mode 100644 apis/settings/composition.yaml create mode 100644 apis/settings/xrd.yaml create mode 100644 tests/test_apis_consistency.py diff --git a/apis/connectedendpoints/composition.yaml b/apis/connectedendpoints/composition.yaml new file mode 100644 index 0000000..8cc38ae --- /dev/null +++ b/apis/connectedendpoints/composition.yaml @@ -0,0 +1,16 @@ +# Composition for ConnectedEndpoints: validate this fragment and report on its own status. +# Uses the same function image as Fabric and Device, which dispatches on the +# composite kind. +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: connectedendpoints-avd +spec: + compositeTypeRef: + apiVersion: avd.netclab.dev/v1alpha1 + kind: ConnectedEndpoints + mode: Pipeline + pipeline: + - step: validate-input + functionRef: + name: netclab-function-avd diff --git a/apis/connectedendpoints/xrd.yaml b/apis/connectedendpoints/xrd.yaml new file mode 100644 index 0000000..c4affc6 --- /dev/null +++ b/apis/connectedendpoints/xrd.yaml @@ -0,0 +1,103 @@ +# CompositeResourceDefinition for what connects to the fabric. +# +# Carries `connected_endpoints_keys.key` lists -- `servers`, `firewalls` and the +# rest -- plus `port_profiles` and `network_ports`. Its own kind for the same +# reason as NetworkServices: whoever attaches servers is rarely whoever owns the +# fabric, and RBAC is granted per kind. +# +# Named ConnectedEndpoints, not Endpoints, because Endpoints is a core/v1 kind +# and `kubectl get endpoints` would become ambiguous. +apiVersion: apiextensions.crossplane.io/v2 +kind: CompositeResourceDefinition +metadata: + name: connectedendpoints.avd.netclab.dev +spec: + scope: Namespaced + group: avd.netclab.dev + names: + kind: ConnectedEndpoints + plural: connectedendpoints + categories: + - crossplane + - netclab + defaultCompositionRef: + name: connectedendpoints-avd + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + design: + type: object + description: >- + The endpoints this input contributes. Structurally open: + the top-level key names come from connected_endpoints_keys, + so they are decided by the document's own content and no + OpenAPI schema can describe them. + x-kubernetes-preserve-unknown-fields: true + appliesTo: + type: object + description: >- + Devices that see this input. Defaults to every device in the + fabric. + properties: + all: + type: boolean + nodeSets: + type: array + description: Devices declared by the named NodeSets. + items: + type: string + hosts: + type: array + description: Devices named directly. + items: + type: string + matchHostnames: + type: array + description: >- + Regular expressions matched against device names. Same + semantics as AVD's own default_node_types, which matches + hostnames this way: the pattern is anchored for you, so + `dc1-leaf.*` matches the whole name. A pattern matching + no device is an error, not an empty set -- a pattern is + silent about both matching nothing and matching too much, + and the render is pushed as a full config replacement. + items: + type: string + required: + - design + status: + type: object + properties: + keys: + type: array + description: Top-level eos_designs keys this input contributes. + items: + type: string + devices: + type: array + description: >- + Devices this input resolved to. Reported rather than left to + be inferred, because appliesTo may be a pattern and a pattern + does not say what it matched. + items: + type: string + validation: + type: object + description: >- + Result of validating this fragment. Reported here rather than + on the Fabric so the team that owns this object sees its own + error on its own object. + properties: + ok: + type: boolean + message: + type: string + x-kubernetes-preserve-unknown-fields: true diff --git a/apis/device/xrd.yaml b/apis/device/xrd.yaml index 8c85079..1f46d8c 100644 --- a/apis/device/xrd.yaml +++ b/apis/device/xrd.yaml @@ -19,6 +19,9 @@ spec: names: kind: Device plural: devices + categories: + - crossplane + - netclab # Devices are composed by the Fabric function without an explicit composition # selector; pin the default so selection is deterministic. defaultCompositionRef: diff --git a/apis/fabric/xrd.yaml b/apis/fabric/xrd.yaml index 8f02562..441ebfd 100644 --- a/apis/fabric/xrd.yaml +++ b/apis/fabric/xrd.yaml @@ -22,6 +22,9 @@ spec: names: kind: Fabric plural: fabrics + categories: + - crossplane + - netclab defaultCompositionRef: name: fabric-avd versions: @@ -46,8 +49,45 @@ spec: description: >- Fabric-wide AVD eos_designs input document (node-type blocks, default_node_types, tenants, connected endpoints, ...). - Validated by pyavd; violations reported on status. + Validated by pyavd; violations reported on status. A fabric + may instead be assembled from input objects listed in + spec.requires, in which case this carries only what is + fabric-wide and the inputs carry the rest. x-kubernetes-preserve-unknown-fields: true + requires: + type: array + description: >- + The input objects composing this fabric. Only objects listed + here take part in the render, however they are labelled -- + which is what makes the rendered document a function of this + Fabric rather than of whatever else exists in the namespace, + and it matters because the render is pushed as a full config + replacement. An entry that does not resolve leaves the Fabric + not ready, naming the object it could not find. List order is + the merge order: a later input replaces an earlier one's keys, + which is how a setting is narrowed to part of the fabric. + items: + type: object + properties: + kind: + type: string + enum: + - NodeSet + - NetworkServices + - ConnectedEndpoints + - Settings + - Secret + name: + type: string + minLength: 1 + namespace: + type: string + description: >- + Namespace holding the object. Defaults to the Fabric's + own namespace. + required: + - kind + - name push: type: object description: >- diff --git a/apis/networkservices/composition.yaml b/apis/networkservices/composition.yaml new file mode 100644 index 0000000..81dc745 --- /dev/null +++ b/apis/networkservices/composition.yaml @@ -0,0 +1,16 @@ +# Composition for NetworkServices: validate this fragment and report on its own status. +# Uses the same function image as Fabric and Device, which dispatches on the +# composite kind. +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: networkservices-avd +spec: + compositeTypeRef: + apiVersion: avd.netclab.dev/v1alpha1 + kind: NetworkServices + mode: Pipeline + pipeline: + - step: validate-input + functionRef: + name: netclab-function-avd diff --git a/apis/networkservices/xrd.yaml b/apis/networkservices/xrd.yaml new file mode 100644 index 0000000..2a21b7f --- /dev/null +++ b/apis/networkservices/xrd.yaml @@ -0,0 +1,104 @@ +# CompositeResourceDefinition for a fabric's network services. +# +# Carries the tenants -- `network_services_keys.name`, `tenants` by default -- +# with their VRFs, SVIs and L2 VLANs. Its own kind because network services are +# owned by whoever runs the services, not by whoever owns the spines, and RBAC +# is granted per kind. +# +# It normally applies to every device: AVD decides per node which services land +# there, through `filter.tenants` and `filter.tags` on the node, so this input +# does not have to be scoped by hand. +apiVersion: apiextensions.crossplane.io/v2 +kind: CompositeResourceDefinition +metadata: + name: networkservices.avd.netclab.dev +spec: + scope: Namespaced + group: avd.netclab.dev + names: + kind: NetworkServices + plural: networkservices + categories: + - crossplane + - netclab + defaultCompositionRef: + name: networkservices-avd + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + design: + type: object + description: >- + The tenants this input contributes. Structurally open: + the top-level key names come from network_services_keys, so + they are decided by the document's own content and no OpenAPI + schema can describe them. + x-kubernetes-preserve-unknown-fields: true + appliesTo: + type: object + description: >- + Devices that see this input. Defaults to every device in the + fabric. + properties: + all: + type: boolean + nodeSets: + type: array + description: Devices declared by the named NodeSets. + items: + type: string + hosts: + type: array + description: Devices named directly. + items: + type: string + matchHostnames: + type: array + description: >- + Regular expressions matched against device names. Same + semantics as AVD's own default_node_types, which matches + hostnames this way: the pattern is anchored for you, so + `dc1-leaf.*` matches the whole name. A pattern matching + no device is an error, not an empty set -- a pattern is + silent about both matching nothing and matching too much, + and the render is pushed as a full config replacement. + items: + type: string + required: + - design + status: + type: object + properties: + keys: + type: array + description: Top-level eos_designs keys this input contributes. + items: + type: string + devices: + type: array + description: >- + Devices this input resolved to. Reported rather than left to + be inferred, because appliesTo may be a pattern and a pattern + does not say what it matched. + items: + type: string + validation: + type: object + description: >- + Result of validating this fragment. Reported here rather than + on the Fabric so the team that owns this object sees its own + error on its own object. + properties: + ok: + type: boolean + message: + type: string + x-kubernetes-preserve-unknown-fields: true diff --git a/apis/nodeset/composition.yaml b/apis/nodeset/composition.yaml new file mode 100644 index 0000000..936031d --- /dev/null +++ b/apis/nodeset/composition.yaml @@ -0,0 +1,16 @@ +# Composition for NodeSet: validate this fragment and report on its own status. +# Uses the same function image as Fabric and Device, which dispatches on the +# composite kind. +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: nodeset-avd +spec: + compositeTypeRef: + apiVersion: avd.netclab.dev/v1alpha1 + kind: NodeSet + mode: Pipeline + pipeline: + - step: validate-input + functionRef: + name: netclab-function-avd diff --git a/apis/nodeset/xrd.yaml b/apis/nodeset/xrd.yaml new file mode 100644 index 0000000..6b4f284 --- /dev/null +++ b/apis/nodeset/xrd.yaml @@ -0,0 +1,126 @@ +# CompositeResourceDefinition for a set of fabric nodes. +# +# A NodeSet carries one node-type block's share of the eos_designs input -- the +# shape of a single group_vars file: `.defaults`, `.nodes[]`, +# `.node_groups[]`. One NodeSet is what `DC1_L3_LEAVES.yml` is in an AVD +# inventory, which is normally several `node_groups`, not one. +# +# It is the only kind that brings devices into existence. `spec.declares` is the +# fabric's device list, and there is no second list: a node named in a block the +# fabric does not declare is not a device. That is deliberate -- AVD inventories +# keep the inventory and the model as two lists that may disagree, and a device +# list in two places with nothing reconciling them is how `spec.push.hosts` +# went wrong. +# +# `spec.appliesTo` is a separate question from what a NodeSet declares: it says +# which devices *see* this input. The two coincide in simple topologies and +# diverge in a 5-stage CLOS, where a DC's super_spine block names four devices +# but is visible to every device of that DC. +apiVersion: apiextensions.crossplane.io/v2 +kind: CompositeResourceDefinition +metadata: + name: nodesets.avd.netclab.dev +spec: + scope: Namespaced + group: avd.netclab.dev + names: + kind: NodeSet + plural: nodesets + categories: + - crossplane + - netclab + defaultCompositionRef: + name: nodeset-avd + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + design: + type: object + description: >- + This input's share of the eos_designs document, in the shape + of a group_vars file. Structurally open: the top-level key + names come from node_type_keys, so they are decided by the + document's own content and no OpenAPI schema can describe + them. Validated by pyavd; violations reported on status. + x-kubernetes-preserve-unknown-fields: true + declares: + type: array + description: >- + Devices this NodeSet brings into the fabric. Defaults to the + devices its node-type blocks name. Set it explicitly to + declare devices no block mentions -- their node type then + comes from default_node_types -- or to exclude a node the + blocks name but the fabric does not contain. + items: + type: string + appliesTo: + type: object + description: >- + Devices that see this input. Defaults to every device in the + fabric. + properties: + all: + type: boolean + nodeSets: + type: array + description: Devices declared by the named NodeSets. + items: + type: string + hosts: + type: array + description: Devices named directly. + items: + type: string + matchHostnames: + type: array + description: >- + Regular expressions matched against device names. Same + semantics as AVD's own default_node_types, which matches + hostnames this way: the pattern is anchored for you, so + `dc1-leaf.*` matches the whole name. A pattern matching + no device is an error, not an empty set -- a pattern is + silent about both matching nothing and matching too much, + and the render is pushed as a full config replacement. + items: + type: string + required: + - design + status: + type: object + properties: + keys: + type: array + description: Top-level eos_designs keys this input contributes. + items: + type: string + devices: + type: array + description: >- + Devices this input resolved to. Reported rather than left to + be inferred, because appliesTo may be a pattern and a pattern + does not say what it matched. + items: + type: string + deviceCount: + type: integer + description: Devices this input declares. + validation: + type: object + description: >- + Result of validating this fragment. Reported here rather than + on the Fabric so the team that owns this object sees its own + error on its own object. + properties: + ok: + type: boolean + message: + type: string + x-kubernetes-preserve-unknown-fields: true diff --git a/apis/settings/composition.yaml b/apis/settings/composition.yaml new file mode 100644 index 0000000..e3fd526 --- /dev/null +++ b/apis/settings/composition.yaml @@ -0,0 +1,16 @@ +# Composition for Settings: validate this fragment and report on its own status. +# Uses the same function image as Fabric and Device, which dispatches on the +# composite kind. +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: settings-avd +spec: + compositeTypeRef: + apiVersion: avd.netclab.dev/v1alpha1 + kind: Settings + mode: Pipeline + pipeline: + - step: validate-input + functionRef: + name: netclab-function-avd diff --git a/apis/settings/xrd.yaml b/apis/settings/xrd.yaml new file mode 100644 index 0000000..59c3c52 --- /dev/null +++ b/apis/settings/xrd.yaml @@ -0,0 +1,103 @@ +# CompositeResourceDefinition for fabric settings at any scope. +# +# Everything that is not node-scoped and not a service: routing protocol choices, +# `bgp_peer_groups`, `default_interfaces`, `aaa_settings`, `dns_settings`, +# `ntp_settings`, `management_eapi`. Fabric-wide settings live on the Fabric +# itself; this kind carries the same keys narrowed to part of the fabric, which +# is what an AVD inventory expresses by putting them in a DC or role group. +# +# The key categories across the input kinds are a convention, not a partition the +# schema could enforce -- eos_designs' top-level key names come from its own +# content. What the kinds separate is ownership. +apiVersion: apiextensions.crossplane.io/v2 +kind: CompositeResourceDefinition +metadata: + name: settings.avd.netclab.dev +spec: + scope: Namespaced + group: avd.netclab.dev + names: + kind: Settings + plural: settings + categories: + - crossplane + - netclab + defaultCompositionRef: + name: settings-avd + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + design: + type: object + description: >- + The settings this input contributes, in the shape of a + group_vars file. Structurally open, like every input's design. + x-kubernetes-preserve-unknown-fields: true + appliesTo: + type: object + description: >- + Devices that see this input. Defaults to every device in the + fabric. + properties: + all: + type: boolean + nodeSets: + type: array + description: Devices declared by the named NodeSets. + items: + type: string + hosts: + type: array + description: Devices named directly. + items: + type: string + matchHostnames: + type: array + description: >- + Regular expressions matched against device names. Same + semantics as AVD's own default_node_types, which matches + hostnames this way: the pattern is anchored for you, so + `dc1-leaf.*` matches the whole name. A pattern matching + no device is an error, not an empty set -- a pattern is + silent about both matching nothing and matching too much, + and the render is pushed as a full config replacement. + items: + type: string + required: + - design + status: + type: object + properties: + keys: + type: array + description: Top-level eos_designs keys this input contributes. + items: + type: string + devices: + type: array + description: >- + Devices this input resolved to. Reported rather than left to + be inferred, because appliesTo may be a pattern and a pattern + does not say what it matched. + items: + type: string + validation: + type: object + description: >- + Result of validating this fragment. Reported here rather than + on the Fabric so the team that owns this object sees its own + error on its own object. + properties: + ok: + type: boolean + message: + type: string + x-kubernetes-preserve-unknown-fields: true diff --git a/function/fn.py b/function/fn.py index 53897b5..aff6b9a 100644 --- a/function/fn.py +++ b/function/fn.py @@ -31,6 +31,7 @@ device_roles_from_design, render_fabric_design, ) +from .kinds import KINDS, hosts_in_blocks API_VERSION = "avd.netclab.dev/v1alpha1" @@ -94,10 +95,41 @@ async def RunFunction( # noqa: N802 (gRPC method name) self._reconcile_fabric(req, rsp, observed) elif kind == "Device": self._reconcile_device(req, rsp, observed) + elif kind in KINDS: + self._reconcile_input(rsp, observed) else: response.fatal(rsp, f"unsupported composite kind: {kind!r}") return rsp + # -- Inputs: a fragment of the design; they compose nothing --------------- + + def _reconcile_input(self, rsp: fnv1.RunFunctionResponse, observed: dict) -> None: + """Report what this fragment contributes, on its own object. + + An input composes nothing -- a Fabric collects it. This reconcile exists + so the team that owns the object sees its own shape here rather than + buried in someone else's Fabric status. + + It cannot report `status.devices`: an input does not know the fabric's + device list, so `appliesTo` only resolves where the inputs are collected. + The Fabric fills that in. Validation is deliberately not attempted + either -- whether pyavd can validate a fragment standalone is unsettled, + and a green validation that never ran is worse than none. + """ + spec = observed.get("spec") or {} + design = spec.get("design") or {} + status: dict = {"keys": sorted(design)} + + if observed.get("kind") == "NodeSet": + declared = spec.get("declares") + devices = set(declared) if declared is not None else hosts_in_blocks(design) + status["deviceCount"] = len(devices) + + resource.update_status(rsp.desired.composite, status) + response.normal( + rsp, f"{observed.get('kind')} contributes {len(design)} top-level key(s)" + ) + # -- Fabric: fabric-wide model -> one Device XR per host ------------------ def _reconcile_fabric( diff --git a/function/kinds.py b/function/kinds.py index 7497fd4..ebd5f04 100644 --- a/function/kinds.py +++ b/function/kinds.py @@ -29,12 +29,26 @@ from __future__ import annotations +import re from dataclasses import dataclass, field from typing import Any KINDS = ("NodeSet", "NetworkServices", "ConnectedEndpoints", "Settings") +def matches(pattern: str, hostname: str) -> bool: + """AVD's own hostname-matching semantics, copied from the code not the docs. + + ``shared_utils/node_type.py`` resolves ``default_node_types`` with + ``search(f"^{regex}$", hostname)`` -- **AVD anchors the pattern for you**, so + ``dc1-leaf.*`` matches a whole name. The schema's description reads as though + the author must anchor it; the code does it for them. Copying the description + instead of the code would make the same pattern mean different things in the + two places. + """ + return re.search(f"^{pattern}$", hostname) is not None + + def is_node_block(value: Any) -> bool: """A node-type block is a dict carrying ``nodes`` and/or ``node_groups``.""" return isinstance(value, dict) and ("nodes" in value or "node_groups" in value) @@ -82,11 +96,14 @@ class Input: name: str kind: str design: dict - # spec.appliesTo -- exactly one of the three + # spec.appliesTo -- the criteria are unioned; none set means every device all_devices: bool = False node_sets: list[str] = field(default_factory=list) hosts: list[str] = field(default_factory=list) - # spec.declares -- devices this input brings into the fabric + match_hostnames: list[str] = field(default_factory=list) + # spec.declares -- devices this input brings into the fabric. Never a + # pattern: visibility may be matched, existence may not. A typo in a pattern + # would silently drop devices from the fabric. declares: list[str] = field(default_factory=list) @classmethod @@ -108,19 +125,20 @@ def from_xr(cls, xr: dict) -> "Input": all_devices=bool(applies.get("all")), node_sets=list(applies.get("nodeSets") or []), hosts=list(applies.get("hosts") or []), + match_hostnames=list(applies.get("matchHostnames") or []), declares=declares, ) def scope(self, declared_by: dict[str, set[str]], devices: set[str]) -> set[str]: - """Devices that see this input.""" - if self.all_devices: + """Devices that see this input. The criteria are unioned.""" + if self.all_devices or not (self.node_sets or self.hosts or self.match_hostnames): return devices - if self.node_sets: - named: set[str] = set() - for name in self.node_sets: - named |= declared_by.get(name, set()) - return devices & named - return devices & set(self.hosts) + named: set[str] = set() + for name in self.node_sets: + named |= declared_by.get(name, set()) + named |= set(self.hosts) + named |= {h for h in devices for p in self.match_hostnames if matches(p, h)} + return devices & named def resolve(inputs: list[Input]) -> dict[str, dict]: @@ -143,6 +161,26 @@ def resolve(inputs: list[Input]) -> dict[str, dict]: return out +def unmatched_patterns(inputs: list[Input]) -> list[tuple[str, str]]: + """``(input name, pattern)`` for every ``matchHostnames`` entry matching no device. + + A pattern is silent in both directions: a typo matches nothing and the input + quietly reaches no device, while a wide pattern quietly reaches devices it + was not meant to. The second is visible on status (`devices`); the first is + not, so the caller is expected to treat this as an error and refuse to + render -- the render is pushed as a full config replacement. + """ + devices: set[str] = set() + for inp in inputs: + devices |= set(inp.declares) + return [ + (inp.name, pattern) + for inp in inputs + for pattern in inp.match_hostnames + if not any(matches(pattern, host) for host in devices) + ] + + def overwrites(inputs: list[Input]) -> list[tuple[str, str, str, str]]: """``(device, key, earlier input, later input)`` for every value replaced. diff --git a/tests/test_apis_consistency.py b/tests/test_apis_consistency.py new file mode 100644 index 0000000..bf4f09f --- /dev/null +++ b/tests/test_apis_consistency.py @@ -0,0 +1,81 @@ +"""The published API and the code that serves it do not drift apart. + +Offline. Cheap checks over `apis/`, each guarding a failure that is silent: + +* a new input kind gets an XRD but `fn.py` never learns to reconcile it (or the + reverse), and the XR sits unready with "unsupported composite kind"; +* an XRD ships without `categories`, so `kubectl get netclab` does not list it -- + the exact defect this repo carried in Fabric and Device until it was found by + running the command, not by reading the file; +* an XRD points `defaultCompositionRef` at a Composition that is not there, or at + one built for a different kind, so nothing selects it. + +None of these break a build. They break in a cluster, one release later. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +import yaml + +from function.kinds import KINDS + +APIS = Path("apis") +# Fabric and Device are composed, not collected -- they are not input kinds. +COMPOSED_KINDS = {"Fabric", "Device"} + + +def _xrds() -> dict[str, dict]: + return {p.parent.name: yaml.safe_load(p.read_text()) for p in sorted(APIS.glob("*/xrd.yaml"))} + + +XRDS = _xrds() + + +def test_apis_directory_is_not_empty() -> None: + assert len(XRDS) >= 6, f"expected the six XRDs, found {sorted(XRDS)}" + + +@pytest.mark.parametrize("name", sorted(XRDS), ids=str) +def test_xrd_declares_categories(name: str) -> None: + """Every XRD is reachable by `kubectl get netclab` and `kubectl get crossplane`.""" + names = XRDS[name]["spec"]["names"] + assert names.get("categories") == ["crossplane", "netclab"], ( + f"{name}: categories are {names.get('categories')!r}; netclab-xp's twelve " + f"XRDs all carry ['crossplane', 'netclab'] and these must match" + ) + + +@pytest.mark.parametrize("name", sorted(XRDS), ids=str) +def test_xrd_has_its_composition(name: str) -> None: + """`defaultCompositionRef` resolves, and to a Composition for this kind.""" + xrd = XRDS[name]["spec"] + wanted = xrd["defaultCompositionRef"]["name"] + composition = yaml.safe_load((APIS / name / "composition.yaml").read_text()) + assert composition["metadata"]["name"] == wanted + assert composition["spec"]["compositeTypeRef"]["kind"] == xrd["names"]["kind"] + + +def test_input_kinds_match_the_function() -> None: + """The XRDs that exist and the kinds fn.py reconciles are the same set.""" + from_apis = {x["spec"]["names"]["kind"] for x in XRDS.values()} - COMPOSED_KINDS + assert from_apis == set(KINDS), ( + f"apis/ serves {sorted(from_apis)} but function.kinds.KINDS is " + f"{sorted(KINDS)} -- fn.py would answer 'unsupported composite kind'" + ) + + +def test_fabric_requires_accepts_every_input_kind_and_secret() -> None: + """A Fabric can name each input kind, plus a Secret carrying credentials. + + Secret is in the enum from the first version deliberately: adding it later + would be a schema change to a released API, and the mechanism it enables -- + a Secret layered like any other input -- needs no other schema footprint. + """ + spec = XRDS["fabric"]["spec"]["versions"][0]["schema"]["openAPIV3Schema"] + enum = spec["properties"]["spec"]["properties"]["requires"]["items"]["properties"]["kind"][ + "enum" + ] + assert set(enum) == set(KINDS) | {"Secret"} From 264cb7c005e4393eb83d8edc976e75ca19b42e03 Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:57:32 +0000 Subject: [PATCH 4/8] Collect the inputs a Fabric names, and refuse to render without them The Fabric asks Crossplane for each entry in spec.requires by kind, name and namespace, layers what comes back, and renders per device. A Fabric with no requires takes the released path unchanged: one document handed to every device. The gate is the point. Requirements are answered on the *next* reconcile, so the first one always arrives with nothing at all -- rendering then would push a fabric short of its inputs, as a full config replacement, with no Delete. So an unresolved requires composes nothing. The proto settles a question this design could not answer before: "not yet" and "never" are distinguishable. Crossplane sends an empty Resources for a requirement it looked for and did not find, and omits the key entirely when it has not fetched yet. Both gate, but they are different states and the condition says which -- WaitingForInputs against InputsMissing. Two refusals rather than a silent render, both because the alternative reaches a device. A matchHostnames pattern that matches nothing is fatal: a pattern is silent about matching nothing, so it cannot be allowed to be. A Secret named in requires is fatal too -- it is in the enum so the mechanism can land without a schema change, but rendering a fabric whose credentials are quietly absent is worse than not rendering. Values replaced by a later input are reported as a warning, never an error: the order is declared by whoever wrote requires, so an override is intentional. First tests in this repo to drive RunFunction. They cover both gate states, both refusals, that resolved inputs compose devices, that each input kind reconciles and reports its own keys, and that a fabric with only spec.design still composes -- the last one guarding the refactor that put both paths through render_structured_configs, since v0.1.6 is published and netclab-xp pins it. Co-Authored-By: Claude Opus 5 --- function/fn.py | 170 ++++++++++++++++++++++++++++++-- tests/test_fabric_collect.py | 181 +++++++++++++++++++++++++++++++++++ 2 files changed, 345 insertions(+), 6 deletions(-) create mode 100644 tests/test_fabric_collect.py diff --git a/function/fn.py b/function/fn.py index aff6b9a..0ccc657 100644 --- a/function/fn.py +++ b/function/fn.py @@ -29,9 +29,17 @@ from .engine import ( InputValidationError, device_roles_from_design, - render_fabric_design, + hostnames_from_design, + render_structured_configs, +) +from .kinds import ( + KINDS, + Input, + hosts_in_blocks, + overwrites, + resolve, + unmatched_patterns, ) -from .kinds import KINDS, hosts_in_blocks API_VERSION = "avd.netclab.dev/v1alpha1" @@ -142,12 +150,24 @@ def _reconcile_fabric( namespace = meta.get("namespace", "default") xr_name = meta.get("name") or (fabric_name or "fabric").lower() - if not fabric_name or not design: - response.fatal(rsp, "spec.fabricName and spec.design are required") + requires = spec.get("requires") or [] + if not fabric_name or not (design or requires): + response.fatal(rsp, "spec.fabricName and spec.design or spec.requires are required") return + if requires: + all_inputs = self._collect(req, rsp, observed, requires, design, fabric_name) + if all_inputs is None: + return # gated -- _collect reported why + else: + # The released path: one fabric-wide document handed to every device, + # with AVD resolving roles from the node-type blocks. + document = dict(design) + document["fabric_name"] = fabric_name + all_inputs = {host: document for host in hostnames_from_design(document)} + try: - structured_configs = render_fabric_design(design, fabric_name) + structured_configs = render_structured_configs(all_inputs) except InputValidationError as err: resource.update_status( rsp.desired.composite, @@ -171,7 +191,12 @@ def _reconcile_fabric( "urlTemplate", "https://{hostname}.{namespace}.svc/command-api" ) - roles = device_roles_from_design(design) + # Roles come from each device's own view: with inputs, a leaf in DC1 sees + # DC1's node-type block and nothing of DC2's. + roles = { + host: device_roles_from_design(hostvars).get(host) or hostvars.get("type", "") + for host, hostvars in all_inputs.items() + } observed_devices = req.observed.resources # keyed by composition-resource-name (hostname) devices = [] for hostname, structured_config in structured_configs.items(): @@ -227,6 +252,139 @@ def _reconcile_fabric( rsp, f"Composed {len(structured_configs)} Device(s) for fabric {fabric_name}" ) + # -- Collecting the inputs a Fabric names --------------------------------- + + def _collect( # noqa: PLR0913 + self, + req: fnv1.RunFunctionRequest, + rsp: fnv1.RunFunctionResponse, + observed: dict, + requires: list[dict], + design: dict, + fabric_name: str, + ) -> dict[str, dict] | None: + """Ask Crossplane for the named inputs; layer them once they arrive. + + Returns per-device inputs, or ``None`` when the fabric must not render -- + the gate. That gate is not only a guard against a slow operator: + requirements are answered on the *next* reconcile, so the first one + always arrives with nothing at all, and rendering then would push a + fabric short of its inputs as a full config replacement. + """ + namespace = (observed.get("metadata") or {}).get("namespace", "default") + + # State the requirements on every reconcile. Crossplane fetches what the + # latest response asked for, so leaving them out once drops the inputs. + keys: list[tuple[str, dict]] = [] + for index, entry in enumerate(requires): + kind = entry["kind"] + key = f"{index:03d}-{kind.lower()}-{entry['name']}" + keys.append((key, entry)) + response.require_resources( + rsp, + name=key, + api_version="v1" if kind == "Secret" else API_VERSION, + kind=kind, + match_name=entry["name"], + namespace=entry.get("namespace", namespace), + ) + + pending: list[str] = [] + absent: list[str] = [] + inputs: list[Input] = [] + for key, entry in keys: + named = f"{entry['kind']}/{entry.get('namespace', namespace)}/{entry['name']}" + if entry["kind"] == "Secret": + # In the API from the first version so the mechanism can land + # without a schema change, but not implemented. Refuse rather + # than render a fabric whose credentials are silently absent. + response.fatal(rsp, f"Secret inputs are not implemented yet: {named}") + return None + if key not in req.required_resources: + pending.append(named) + continue + items = req.required_resources[key].items + if not items: + # Crossplane looked and found nothing. The proto distinguishes + # this from "not fetched yet" by sending an empty Resources, and + # that is what lets a Fabric tell "waiting" from "missing" -- + # the one thing this design was previously unable to do. + absent.append(named) + continue + inputs.append( + Input.from_xr(_normalize_numbers(resource.struct_to_dict(items[0].resource))) + ) + + if pending or absent: + detail = [] + if absent: + detail.append(f"not found: {', '.join(absent)}") + if pending: + detail.append(f"not fetched yet: {', '.join(pending)}") + message = "; ".join(detail) + response.set_conditions( + rsp, + resource.Condition( + typ="InputsResolved", + status="False", + reason="InputsMissing" if absent else "WaitingForInputs", + message=message[:400], + ), + ) + resource.update_status( + rsp.desired.composite, + {"fabricName": fabric_name, "validation": {"ok": False, "message": message}}, + ) + # Missing is a real problem; not-fetched-yet is the normal first pass. + report = response.warning if absent else response.normal + report(rsp, f"fabric {fabric_name} is waiting on inputs -- {message}") + return None + + # The Fabric's own design is the first input: fabric-wide, seen by every + # device, and declaring whatever devices its own blocks name so a Fabric + # that carries both a design and a requires list still has its devices. + document = dict(design) + document["fabric_name"] = fabric_name + inputs.insert( + 0, + Input( + name="fabric", + kind="Settings", + design=document, + all_devices=True, + declares=sorted(hosts_in_blocks(document)), + ), + ) + + if stray := unmatched_patterns(inputs): + listed = ", ".join(f"{name}: {pattern!r}" for name, pattern in stray) + response.fatal( + rsp, + f"appliesTo.matchHostnames matched no device ({listed}) -- " + f"a pattern that matches nothing is silent, so it is refused", + ) + return None + + if replaced := overwrites(inputs): + shown = ", ".join(f"{key} on {host} ({first} -> {second})" + for host, key, first, second in replaced[:5]) + response.warning( + rsp, + f"{len(replaced)} value(s) replaced by a later input: {shown}" + + (" ..." if len(replaced) > 5 else ""), + ) + + response.set_conditions( + rsp, + resource.Condition( + typ="InputsResolved", + status="True", + reason="AllInputsResolved", + message=f"{len(inputs)} input(s)", + ), + ) + return resolve(inputs) + # -- Device: validate + render one device's config ----------------------- def _reconcile_device( diff --git a/tests/test_fabric_collect.py b/tests/test_fabric_collect.py new file mode 100644 index 0000000..68da053 --- /dev/null +++ b/tests/test_fabric_collect.py @@ -0,0 +1,181 @@ +"""A Fabric collects the inputs it names, and refuses to render without them. + +Offline -- drives RunFunction directly, with no cluster and no Crossplane. The +gate is the most safety-critical piece in the collect path: requirements are +answered on the *next* reconcile, so the first one always arrives with nothing, +and a fabric rendered short of its inputs would be pushed to devices as a full +config replacement. +""" + +from __future__ import annotations + +import asyncio + +import pytest +from crossplane.function import resource +from crossplane.function.proto.v1 import run_function_pb2 as fnv1 + +from function.fn import FunctionRunner + +API = "avd.netclab.dev/v1alpha1" + + +def _run(req: fnv1.RunFunctionRequest) -> fnv1.RunFunctionResponse: + return asyncio.run(FunctionRunner().RunFunction(req, None)) + + +def _fabric(requires: list[dict], design: dict | None = None) -> dict: + return { + "apiVersion": API, + "kind": "Fabric", + "metadata": {"name": "fabric", "namespace": "avd"}, + "spec": {"fabricName": "FABRIC", "design": design or {}, "requires": requires}, + } + + +def _input_xr(kind: str, name: str, spec: dict) -> dict: + return { + "apiVersion": API, + "kind": kind, + "metadata": {"name": name, "namespace": "avd"}, + "spec": spec, + } + + +def _request(xr: dict, required: dict[str, list[dict]] | None = None) -> fnv1.RunFunctionRequest: + req = fnv1.RunFunctionRequest() + req.observed.composite.resource.CopyFrom(resource.dict_to_struct(xr)) + for key, objects in (required or {}).items(): + # An empty list is Crossplane saying "I looked and found nothing", which + # the proto distinguishes from a key that is absent entirely. + entry = req.required_resources[key] + for obj in objects: + entry.items.add().resource.CopyFrom(resource.dict_to_struct(obj)) + return req + + +def _condition(rsp: fnv1.RunFunctionResponse, typ: str): + return next((c for c in rsp.conditions if c.type == typ), None) + + +# A spine rather than a leaf, only because a leaf defaults to being a VTEP and +# would drag in the VXLAN pools -- this fixture is about the collect path, not +# about exercising AVD. +SPINES = _input_xr( + "NodeSet", + "spines", + { + # `type` rides in the same input: AVD needs it (or default_node_types) + # to know what the device is, and it applies to whoever sees this input. + "design": { + "type": "spine", + "spine": { + "defaults": {"loopback_ipv4_pool": "10.255.0.0/27"}, + "nodes": [{"name": "spine1", "id": 1, "bgp_as": 65100}], + }, + } + }, +) + + +def test_first_reconcile_asks_and_renders_nothing() -> None: + """Requirements are answered next time round, so the first pass is empty. + + This is the case the gate exists for: not a slow operator, but the protocol. + """ + rsp = _run(_request(_fabric([{"kind": "NodeSet", "name": "spines"}]))) + + assert set(rsp.requirements.resources) == {"000-nodeset-spines"} + selector = rsp.requirements.resources["000-nodeset-spines"] + assert (selector.kind, selector.match_name, selector.namespace) == ("NodeSet", "spines", "avd") + + assert not rsp.desired.resources, "nothing may be composed before the inputs arrive" + condition = _condition(rsp, "InputsResolved") + assert condition.reason == "WaitingForInputs" + + +def test_missing_input_is_distinguished_from_not_yet_fetched() -> None: + """An empty Resources means Crossplane looked and found nothing.""" + rsp = _run( + _request( + _fabric([{"kind": "NodeSet", "name": "spines"}]), + required={"000-nodeset-spines": []}, + ) + ) + + assert not rsp.desired.resources + assert _condition(rsp, "InputsResolved").reason == "InputsMissing" + assert "not found" in _condition(rsp, "InputsResolved").message + + +def test_resolved_inputs_compose_devices() -> None: + rsp = _run( + _request( + _fabric([{"kind": "NodeSet", "name": "spines"}]), + required={"000-nodeset-spines": [SPINES]}, + ) + ) + + assert _condition(rsp, "InputsResolved").status == fnv1.STATUS_CONDITION_TRUE + assert set(rsp.desired.resources) == {"spine1"} + + +def test_design_without_requires_still_composes() -> None: + """The released path is untouched: one document, handed to every device. + + Guards the refactor that put both paths through render_structured_configs -- + v0.1.6 is published and netclab-xp pins it, so this must keep working with no + inputs in sight. + """ + rsp = _run(_request(_fabric(requires=[], design=SPINES["spec"]["design"]))) + + assert not any(r.severity == fnv1.SEVERITY_FATAL for r in rsp.results) + assert set(rsp.desired.resources) == {"spine1"} + assert not rsp.requirements.resources, "a fabric with no requires asks for nothing" + + +def test_secret_input_is_refused_until_implemented() -> None: + """It is in the enum so the mechanism can land without a schema change. + + Rendering a fabric whose credentials are silently absent would push a config + without them, so refusing is the only safe placeholder. + """ + rsp = _run(_request(_fabric([{"kind": "Secret", "name": "creds"}]))) + + assert any(r.severity == fnv1.SEVERITY_FATAL for r in rsp.results) + assert not rsp.desired.resources + + +def test_pattern_matching_no_device_is_refused() -> None: + """A pattern is silent about matching nothing, so it cannot be allowed to.""" + settings = _input_xr( + "Settings", + "typo", + # The fabric holds only spine1, so this pattern matches nothing. + {"design": {"ntp_settings": {}}, "appliesTo": {"matchHostnames": ["leaf.*"]}}, + ) + rsp = _run( + _request( + _fabric( + [ + {"kind": "NodeSet", "name": "spines"}, + {"kind": "Settings", "name": "typo"}, + ] + ), + required={"000-nodeset-spines": [SPINES], "001-settings-typo": [settings]}, + ) + ) + + fatal = [r for r in rsp.results if r.severity == fnv1.SEVERITY_FATAL] + assert fatal and "matched no device" in fatal[0].message + assert not rsp.desired.resources + + +@pytest.mark.parametrize("kind", ["NodeSet", "NetworkServices", "ConnectedEndpoints", "Settings"]) +def test_input_kinds_reconcile_and_report_their_keys(kind: str) -> None: + """Each input reports its own shape on its own object, composing nothing.""" + rsp = _run(_request(_input_xr(kind, "an-input", {"design": {"ntp_settings": {}, "type": "x"}}))) + + assert not any(r.severity == fnv1.SEVERITY_FATAL for r in rsp.results) + status = resource.struct_to_dict(rsp.desired.composite.resource).get("status", {}) + assert status["keys"] == ["ntp_settings", "type"] From aa789bc5015aace60f83dcf6838d045d58e95dc2 Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Thu, 13 Aug 2026 15:27:34 +0000 Subject: [PATCH 5/8] Let the lab script build, and install what the package actually ships Two defects in scripts/kind-up.sh, both found by running it rather than reading it, and both invisible to CI because CI does not run this script. The xpkg build passed no --examples-root. That is not "no examples": it means everything under examples/, including examples/lab/topology.yaml, which is helm values with no `kind`. So the build died with "Object 'Kind' is missing" -- the same failure that killed the v0.1.4 release. CI and the release workflow have named examples/fabric explicitly since, in four places, with comments citing that release. This script was the build path that never got the fix, so a local bring-up has been broken since the topology was committed in #19. And it installed two named XRDs while the package root ships whatever is under apis/, so a cluster built by this script no longer matched the package it was built from -- with the input kinds missing exactly where a Fabric that names them is being tested. It applies apis/*/ now. Co-Authored-By: Claude Opus 5 --- scripts/kind-up.sh | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/scripts/kind-up.sh b/scripts/kind-up.sh index 6213d31..725f1e9 100755 --- a/scripts/kind-up.sh +++ b/scripts/kind-up.sh @@ -101,7 +101,15 @@ done echo ">> build + push function image/xpkg (tag ${TAG})" docker build --provenance=false -t "${IMG}:${TAG}" . -crossplane xpkg build --package-root=package --embed-runtime-image="${IMG}:${TAG}" -o "function-avd-${TAG}.xpkg" +# --examples-root is stated even though `package/` is the package root: it +# defaults to ./examples, which is not "no examples" but *everything* under +# examples/ -- including examples/lab/topology.yaml, which is helm values with no +# `kind` and fails the build with "Object 'Kind' is missing". --ignore is no help, +# it does not reach --examples-root. CI and the release workflow have named it +# since v0.1.4 failed on exactly this; this script was the build path that did +# not, so a local bring-up broke the day the topology was committed. +crossplane xpkg build --package-root=package --examples-root=examples/fabric \ + --embed-runtime-image="${IMG}:${TAG}" -o "function-avd-${TAG}.xpkg" crossplane xpkg push -f "function-avd-${TAG}.xpkg" "localhost:${REG_PORT}/netclab/function-avd:${TAG}" echo ">> install Crossplane (chart ${XP_CHART})" @@ -122,10 +130,14 @@ spec: EOF kubectl --context "$CTX" wait --for=condition=Healthy function.pkg.crossplane.io/netclab-function-avd --timeout=180s -echo ">> install XRDs + Compositions (Fabric + Device)" -kubectl --context "$CTX" apply -f apis/fabric/xrd.yaml -f apis/device/xrd.yaml -kubectl --context "$CTX" wait --for=condition=Established xrd/fabrics.avd.netclab.dev xrd/devices.avd.netclab.dev --timeout=60s -kubectl --context "$CTX" apply -f apis/fabric/composition.yaml -f apis/device/composition.yaml +# Every API under apis/, not a named pair: the package root ships whatever is +# there, so a script naming two of them installs a cluster that does not match +# the package it was built from -- and the input kinds would be missing exactly +# where a Fabric that names them is being tested. +echo ">> install XRDs + Compositions (everything under apis/)" +kubectl --context "$CTX" apply -f apis/*/xrd.yaml +kubectl --context "$CTX" wait --for=condition=Established xrd --all --timeout=60s +kubectl --context "$CTX" apply -f apis/*/composition.yaml if [ "$WITH_NETCLAB" = "1" ]; then echo ">> provider-http ${PROVIDER_HTTP} (config push over eAPI)" From d7ce83884e1952b666b8f8ed0a63b83641db4c02 Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Wed, 19 Aug 2026 16:01:59 +0000 Subject: [PATCH 6/8] Make the input kinds a Set family, and the device list the play's Five changes to the input-kind model, all found by generating the XRs the migration produces and reading them. Renamed to a Set family while renaming is still free: NetworkServiceSet, ConnectedEndpointSet, SettingSet. The three plural kinds declared plurals identical to their own lower-cased kind, so nothing on the CLI told one object from all of them. Kubernetes kinds are singular, and its own plural kind -- core/v1 Endpoints -- is the one it superseded with EndpointSlice. These XRDs are unpublished; after a tag this would be a breaking change with no in-place path. classify learned the native network_services spelling. AVD 6.x reads it as well as the dynamic keys named by network_services_keys, so a document written the new way became a SettingSet, carrying its BGP and OSPF passwords into the kind that owns fabric-wide settings. Latent: no XR in AVD's corpus changes kind. appliesTo on a NodeSet now defaults to what it declares. Defaulting to the whole fabric made every migrated NodeSet name itself, 26 of 26 across the examples, because Ansible has no unscoped group_vars file. The widened case stays expressible and is still needed by a 5-stage CLOS. A group's vars become one input per category, merged as Ansible merges them and split after. cv-pathfinder's group_vars/WAN/ is four files the author had already separated; merged and classified whole, its single tenants key decided the kind for 22. 517 of 1117 XRs mixed categories, now none. The device list is the play's hosts, not the inventory's. cv-pathfinder's inventory holds cloudvision so cv_deploy can reach the CloudVision API server; it is not a switch, and a declared device with no node type fails the whole fabric's render. All 25 inventories name their hosts in a playbook, so this reads what AVD acts on rather than guessing. Gated on measuring peers: the 16 references to dropped hosts are all in the negative fixtures, where AVD itself runs each case without them. Suite 101 green. Each guard was verified to fail without its change. Co-Authored-By: Claude Opus 5 --- .../composition.yaml | 6 +- .../xrd.yaml | 19 ++- apis/fabric/xrd.yaml | 6 +- .../composition.yaml | 6 +- .../xrd.yaml | 10 +- apis/nodeset/xrd.yaml | 9 +- .../{settings => settingset}/composition.yaml | 6 +- apis/{settings => settingset}/xrd.yaml | 10 +- function/fn.py | 2 +- function/kinds.py | 30 +++- function/verify_kinds.py | 149 +++++++++++++++--- tests/test_fabric_collect.py | 8 +- tests/test_kinds_equivalence.py | 143 ++++++++++++++++- 13 files changed, 337 insertions(+), 67 deletions(-) rename apis/{connectedendpoints => connectedendpointset}/composition.yaml (68%) rename apis/{connectedendpoints => connectedendpointset}/xrd.yaml (85%) rename apis/{networkservices => networkserviceset}/composition.yaml (69%) rename apis/{networkservices => networkserviceset}/xrd.yaml (95%) rename apis/{settings => settingset}/composition.yaml (72%) rename apis/{settings => settingset}/xrd.yaml (95%) diff --git a/apis/connectedendpoints/composition.yaml b/apis/connectedendpointset/composition.yaml similarity index 68% rename from apis/connectedendpoints/composition.yaml rename to apis/connectedendpointset/composition.yaml index 8cc38ae..32167a8 100644 --- a/apis/connectedendpoints/composition.yaml +++ b/apis/connectedendpointset/composition.yaml @@ -1,14 +1,14 @@ -# Composition for ConnectedEndpoints: validate this fragment and report on its own status. +# Composition for ConnectedEndpointSet: validate this fragment and report on its own status. # Uses the same function image as Fabric and Device, which dispatches on the # composite kind. apiVersion: apiextensions.crossplane.io/v1 kind: Composition metadata: - name: connectedendpoints-avd + name: connectedendpointset-avd spec: compositeTypeRef: apiVersion: avd.netclab.dev/v1alpha1 - kind: ConnectedEndpoints + kind: ConnectedEndpointSet mode: Pipeline pipeline: - step: validate-input diff --git a/apis/connectedendpoints/xrd.yaml b/apis/connectedendpointset/xrd.yaml similarity index 85% rename from apis/connectedendpoints/xrd.yaml rename to apis/connectedendpointset/xrd.yaml index c4affc6..ada1e89 100644 --- a/apis/connectedendpoints/xrd.yaml +++ b/apis/connectedendpointset/xrd.yaml @@ -2,26 +2,29 @@ # # Carries `connected_endpoints_keys.key` lists -- `servers`, `firewalls` and the # rest -- plus `port_profiles` and `network_ports`. Its own kind for the same -# reason as NetworkServices: whoever attaches servers is rarely whoever owns the -# fabric, and RBAC is granted per kind. +# reason as NetworkServiceSet: whoever attaches servers is rarely whoever owns +# the fabric, and RBAC is granted per kind. # -# Named ConnectedEndpoints, not Endpoints, because Endpoints is a core/v1 kind -# and `kubectl get endpoints` would become ambiguous. +# Named ConnectedEndpointSet, not Endpoints: `Endpoints` is a core/v1 kind, so +# `kubectl get endpoints` would be ambiguous. The `Set` suffix is the family's -- +# every input kind is a set of design entries of one category, carrying a scope -- +# and it keeps the kind singular, so `connectedendpointset` and +# `connectedendpointsets` say different things the way `nodeset` does. apiVersion: apiextensions.crossplane.io/v2 kind: CompositeResourceDefinition metadata: - name: connectedendpoints.avd.netclab.dev + name: connectedendpointsets.avd.netclab.dev spec: scope: Namespaced group: avd.netclab.dev names: - kind: ConnectedEndpoints - plural: connectedendpoints + kind: ConnectedEndpointSet + plural: connectedendpointsets categories: - crossplane - netclab defaultCompositionRef: - name: connectedendpoints-avd + name: connectedendpointset-avd versions: - name: v1alpha1 served: true diff --git a/apis/fabric/xrd.yaml b/apis/fabric/xrd.yaml index 441ebfd..2dd85f2 100644 --- a/apis/fabric/xrd.yaml +++ b/apis/fabric/xrd.yaml @@ -73,9 +73,9 @@ spec: type: string enum: - NodeSet - - NetworkServices - - ConnectedEndpoints - - Settings + - NetworkServiceSet + - ConnectedEndpointSet + - SettingSet - Secret name: type: string diff --git a/apis/networkservices/composition.yaml b/apis/networkserviceset/composition.yaml similarity index 69% rename from apis/networkservices/composition.yaml rename to apis/networkserviceset/composition.yaml index 81dc745..b6b4b1a 100644 --- a/apis/networkservices/composition.yaml +++ b/apis/networkserviceset/composition.yaml @@ -1,14 +1,14 @@ -# Composition for NetworkServices: validate this fragment and report on its own status. +# Composition for NetworkServiceSet: validate this fragment and report on its own status. # Uses the same function image as Fabric and Device, which dispatches on the # composite kind. apiVersion: apiextensions.crossplane.io/v1 kind: Composition metadata: - name: networkservices-avd + name: networkserviceset-avd spec: compositeTypeRef: apiVersion: avd.netclab.dev/v1alpha1 - kind: NetworkServices + kind: NetworkServiceSet mode: Pipeline pipeline: - step: validate-input diff --git a/apis/networkservices/xrd.yaml b/apis/networkserviceset/xrd.yaml similarity index 95% rename from apis/networkservices/xrd.yaml rename to apis/networkserviceset/xrd.yaml index 2a21b7f..274a587 100644 --- a/apis/networkservices/xrd.yaml +++ b/apis/networkserviceset/xrd.yaml @@ -1,4 +1,4 @@ -# CompositeResourceDefinition for a fabric's network services. +# CompositeResourceDefinition for a set of a fabric's network services. # # Carries the tenants -- `network_services_keys.name`, `tenants` by default -- # with their VRFs, SVIs and L2 VLANs. Its own kind because network services are @@ -11,18 +11,18 @@ apiVersion: apiextensions.crossplane.io/v2 kind: CompositeResourceDefinition metadata: - name: networkservices.avd.netclab.dev + name: networkservicesets.avd.netclab.dev spec: scope: Namespaced group: avd.netclab.dev names: - kind: NetworkServices - plural: networkservices + kind: NetworkServiceSet + plural: networkservicesets categories: - crossplane - netclab defaultCompositionRef: - name: networkservices-avd + name: networkserviceset-avd versions: - name: v1alpha1 served: true diff --git a/apis/nodeset/xrd.yaml b/apis/nodeset/xrd.yaml index 6b4f284..ee40c07 100644 --- a/apis/nodeset/xrd.yaml +++ b/apis/nodeset/xrd.yaml @@ -64,8 +64,13 @@ spec: appliesTo: type: object description: >- - Devices that see this input. Defaults to every device in the - fabric. + Devices that see this input. Defaults to the devices this + NodeSet declares, which is the common case -- a node-type + block is read by its own group, the way a group_vars file + is. Set it to widen that: in a 5-stage CLOS a DC's + super_spine block declares four devices and is seen by every + device of that DC. Every other kind defaults to the whole + fabric instead, having nothing of its own to be scoped to. properties: all: type: boolean diff --git a/apis/settings/composition.yaml b/apis/settingset/composition.yaml similarity index 72% rename from apis/settings/composition.yaml rename to apis/settingset/composition.yaml index e3fd526..0b900c6 100644 --- a/apis/settings/composition.yaml +++ b/apis/settingset/composition.yaml @@ -1,14 +1,14 @@ -# Composition for Settings: validate this fragment and report on its own status. +# Composition for SettingSet: validate this fragment and report on its own status. # Uses the same function image as Fabric and Device, which dispatches on the # composite kind. apiVersion: apiextensions.crossplane.io/v1 kind: Composition metadata: - name: settings-avd + name: settingset-avd spec: compositeTypeRef: apiVersion: avd.netclab.dev/v1alpha1 - kind: Settings + kind: SettingSet mode: Pipeline pipeline: - step: validate-input diff --git a/apis/settings/xrd.yaml b/apis/settingset/xrd.yaml similarity index 95% rename from apis/settings/xrd.yaml rename to apis/settingset/xrd.yaml index 59c3c52..ded6ac7 100644 --- a/apis/settings/xrd.yaml +++ b/apis/settingset/xrd.yaml @@ -1,4 +1,4 @@ -# CompositeResourceDefinition for fabric settings at any scope. +# CompositeResourceDefinition for a set of settings at any scope. # # Everything that is not node-scoped and not a service: routing protocol choices, # `bgp_peer_groups`, `default_interfaces`, `aaa_settings`, `dns_settings`, @@ -12,18 +12,18 @@ apiVersion: apiextensions.crossplane.io/v2 kind: CompositeResourceDefinition metadata: - name: settings.avd.netclab.dev + name: settingsets.avd.netclab.dev spec: scope: Namespaced group: avd.netclab.dev names: - kind: Settings - plural: settings + kind: SettingSet + plural: settingsets categories: - crossplane - netclab defaultCompositionRef: - name: settings-avd + name: settingset-avd versions: - name: v1alpha1 served: true diff --git a/function/fn.py b/function/fn.py index 0ccc657..be9c942 100644 --- a/function/fn.py +++ b/function/fn.py @@ -349,7 +349,7 @@ def _collect( # noqa: PLR0913 0, Input( name="fabric", - kind="Settings", + kind="SettingSet", design=document, all_devices=True, declares=sorted(hosts_in_blocks(document)), diff --git a/function/kinds.py b/function/kinds.py index ebd5f04..8813c96 100644 --- a/function/kinds.py +++ b/function/kinds.py @@ -33,7 +33,7 @@ from dataclasses import dataclass, field from typing import Any -KINDS = ("NodeSet", "NetworkServices", "ConnectedEndpoints", "Settings") +KINDS = ("NodeSet", "NetworkServiceSet", "ConnectedEndpointSet", "SettingSet") def matches(pattern: str, hostname: str) -> bool: @@ -77,16 +77,20 @@ def classify(design: dict) -> str: """ if any(is_node_block(v) for v in design.values()): return "NodeSet" - if {"tenants", "network_services_keys"} & design.keys(): - return "NetworkServices" + # Both spellings. AVD 6.x reads a native `network_services` list as well as + # the dynamic keys named by `network_services_keys` (default `tenants`) -- + # `shared_utils/filtered_tenants.py` reads one after the other. A custom key + # name is only recognisable when `network_services_keys` travels with it. + if {"network_services", "tenants", "network_services_keys"} & design.keys(): + return "NetworkServiceSet" if { "servers", "firewalls", "routers", "load_balancers", "storage_arrays", "cpes", "workstations", "access_points", "phones", "printers", "generic_devices", "port_profiles", "network_ports", "connected_endpoints_keys", "custom_connected_endpoints_keys", } & design.keys(): - return "ConnectedEndpoints" - return "Settings" + return "ConnectedEndpointSet" + return "SettingSet" @dataclass @@ -130,9 +134,21 @@ def from_xr(cls, xr: dict) -> "Input": ) def scope(self, declared_by: dict[str, set[str]], devices: set[str]) -> set[str]: - """Devices that see this input. The criteria are unioned.""" - if self.all_devices or not (self.node_sets or self.hosts or self.match_hostnames): + """Devices that see this input. The criteria are unioned. + + Omitting ``appliesTo`` means the whole fabric -- except on a ``NodeSet``, + where it means the devices that NodeSet declares. A node-type block seen + fabric-wide is not a thing Ansible can express: a ``group_vars`` file is + read by its group. Every NodeSet in AVD's 8 examples is scoped to exactly + what it declares, 26 of 26, so the default carries the common case and + ``appliesTo`` is left to say the uncommon one -- which is real: in a + 5-stage CLOS a DC's ``super_spine`` block declares 4 devices and is seen + by all 16 of that DC. + """ + if self.all_devices: return devices + if not (self.node_sets or self.hosts or self.match_hostnames): + return devices & set(self.declares) if self.kind == "NodeSet" else devices named: set[str] = set() for name in self.node_sets: named |= declared_by.get(name, set()) diff --git a/function/verify_kinds.py b/function/verify_kinds.py index 8485898..d25b897 100644 --- a/function/verify_kinds.py +++ b/function/verify_kinds.py @@ -26,6 +26,8 @@ import sys from pathlib import Path +import yaml + from .ansible_inputs import ( ALL_GROUP, AnsibleInventory, @@ -33,7 +35,7 @@ _strip_ansible_keys, _yaml_load, ) -from .kinds import Input, classify, hosts_in_blocks, is_node_block, resolve +from .kinds import KINDS, Input, classify, hosts_in_blocks, is_node_block, resolve EXAMPLES_ROOT = Path("avd/ansible_collections/arista/avd/examples") MOLECULE_ROOT = Path("avd/ansible_collections/arista/avd/extensions/molecule") @@ -83,8 +85,68 @@ def _layout(root: Path) -> tuple[Path, Path]: return root / "inventory", root / "inventory" / "hosts.yml" +def _play_targets(root: Path) -> tuple[set[str], set[str]]: + """``(what every play targets, what the eos_designs plays target)``. + + Both, because they differ and only the second is the fabric. A directory's + playbooks also carry plays that are not a render: molecule's ``create.yml`` + makes output folders, ``howto`` has a ``localhost`` play, and + ``deploy.yml`` pushes what was already built. + """ + every: set[str] = set() + designs: set[str] = set() + for playbook in sorted(root.glob("*.yml")) + sorted(root.glob("converge.yml")): + if playbook.name == "inventory.yml": + continue + document = _yaml_load(playbook) + if not isinstance(document, list): + continue + for play in document: + if not isinstance(play, dict) or "hosts" not in play: + continue + every.add(str(play["hosts"])) + # The plays name the role outright: `arista.avd.eos_designs`. + if "eos_designs" in yaml.safe_dump(play): + designs.add(str(play["hosts"])) + return every, designs + + +def play_hosts(root: Path, inventory: AnsibleInventory) -> list[str]: + """The hosts eos_designs is run on -- which is what a fabric's devices are. + + **An inventory is not a device list.** Ansible has two lists and AVD renders + the second: `cv-pathfinder`'s `build.yml` says `hosts: WAN`, while its + inventory also holds `cloudvision` -- the CloudVision API server, a host so + that `cv_deploy` can reach it, and not a switch. Declaring it composes a + Device for it, and a device with no node type fails the whole fabric's + render, not just its own. + + A Fabric *is* the play, so this is the list `declares` reproduces. Reading it + is not a heuristic: it is the statement AVD itself acts on. + + Two fallbacks, in order, each for a real case in the corpus: + + * no play runs eos_designs -- the three `eos_cli_config_gen` scenarios carry + structured config directly. Their hosts are still devices, so fall back to + what any play targets rather than declaring none of them; + * no playbook at all -- an inventory handed to the migration on its own is + still worth translating, so fall back to the whole inventory. + """ + hosts = inventory.hosts() + every, designs = _play_targets(root) + targets = designs or every + if not targets or "all" in targets: + return hosts + return [h for h in hosts if targets & (set(inventory.groups_for_host(h)) | {h})] + + def ansible_hostvars(root: Path) -> dict[str, dict]: - """What ansible-playbook would hand to AVD, from every source it reads.""" + """What ansible-playbook would hand to AVD, from every source it reads. + + The play's hosts, not the inventory's -- see :func:`play_hosts`. Both this + and the migration restrict to the same set, so the comparison stays an + equality over what AVD is actually given. + """ var_dir, inventory_file = _layout(root) inventory = AnsibleInventory.from_file(inventory_file) inline = inline_host_vars(inventory_file) @@ -96,7 +158,7 @@ def ansible_hostvars(root: Path) -> dict[str, dict]: ) cache: dict[str, dict] = {} out: dict[str, dict] = {} - for host in sorted(inventory.hosts()): + for host in sorted(play_hosts(root, inventory)): hostvars: dict = {} for group in inventory.groups_for_host(host): if group not in cache: @@ -109,18 +171,53 @@ def ansible_hostvars(root: Path) -> dict[str, dict]: return out +_SUFFIX = { + "NetworkServiceSet": "services", + "ConnectedEndpointSet": "endpoints", + "SettingSet": "settings", +} + + +def _by_category(base: str, design: dict) -> list[tuple[str, str, dict]]: + """Partition one group's vars into ``(name, kind, design)`` per category. + + **Merge first, split after** -- the order matters and it is Ansible's. + A ``group_vars`` *directory* is merged into one namespace for the group, so + two files setting the same top-level key resolve alphabetically-last-wins + and never coexist. Splitting by file instead would preserve a boundary + Ansible does not keep, and emit two inputs claiming one key. + + Splitting by category afterwards costs nothing and is what the kinds are + for: `cv-pathfinder`'s `group_vars/WAN/` holds four files the author already + separated -- settings, interface profiles, management, tenants -- which + merged into one fragment whose single `tenants` key decided the kind for the + other 21. Ownership only; `resolve` never reads a kind, and the parts are + disjoint and consecutive, so the hostvars are unchanged either way. + """ + buckets: dict[str, dict] = {} + for key, value in design.items(): + buckets.setdefault(classify({key: value}), {})[key] = value + parts = [(kind, buckets[kind]) for kind in KINDS if kind in buckets] + return [ + (base if len(parts) == 1 or kind == "NodeSet" else f"{base}-{_SUFFIX[kind]}", kind, payload) + for kind, payload in parts + ] + + def inputs_from_inventory(root: Path) -> list[Input]: """Translate an AVD inventory into ordered input XRs -- a migration. - Each group_vars file becomes one or two inputs: a ``NodeSet`` for its - node-type blocks and one input for the rest. The split is not cosmetic -- - the two halves have different scopes whenever a block names fewer devices - than the group holds, which is what a 5-stage CLOS does. + Each group's vars become one input per content category: a ``NodeSet`` for + its node-type blocks, and one each for services, endpoints and settings. + The node-type split is not cosmetic -- the two halves have different scopes + whenever a block names fewer devices than the group holds, which is what a + 5-stage CLOS does. The rest is ownership: see :func:`_by_category`. """ var_dir, inventory_file = _layout(root) inventory = AnsibleInventory.from_file(inventory_file) group_var_dir = var_dir / "group_vars" - every_device = inventory.hosts() + # The play's hosts, not the inventory's -- see play_hosts. + every_device = play_hosts(root, inventory) groups = [] if group_var_dir.is_dir(): @@ -154,7 +251,7 @@ def group_devices(group: str) -> set[str]: declared_by.update(device_sets) inputs = [ - Input(name, "NodeSet", {}, node_sets=[name], declares=sorted(hosts)) + Input(name, "NodeSet", {}, declares=sorted(hosts)) for name, hosts in sorted(device_sets.items()) ] @@ -162,8 +259,6 @@ def group_devices(group: str) -> set[str]: design = designs[group] if not design: continue - blocks = {k: v for k, v in design.items() if is_node_block(v)} - rest = {k: v for k, v in design.items() if not is_node_block(v)} want = group_devices(group) def scoped(name: str, kind: str, payload: dict, want: set[str] = want) -> Input: @@ -182,22 +277,36 @@ def scoped(name: str, kind: str, payload: dict, want: set[str] = want) -> Input: inp.hosts = sorted(want) return inp - if blocks: - node_set = scoped(group, "NodeSet", blocks) - node_set.declares = sorted(declared_by[group]) - inputs.append(node_set) - if rest: - inputs.append(scoped(f"{group}-settings" if blocks else group, classify(rest), rest)) + for name, kind, payload in _by_category(group, design): + inp = scoped(name, kind, payload) + if kind == "NodeSet": + inp.declares = sorted(declared_by[group]) + # A group with no host in the play, whose blocks declare nothing + # either, reaches nobody -- and "reaches nobody" is not expressible + # in appliesTo, where saying nothing means every device. Emitting it + # would invert its meaning. Only reachable since the device list + # became the play's rather than the inventory's. + if not want and not (kind == "NodeSet" and inp.declares): + continue + if kind == "NodeSet": + if want == set(inp.declares): + # The default -- a NodeSet is seen by what it declares. + # Saying it again would make every NodeSet name itself. + inp.all_devices = False + inp.node_sets = [] + inp.hosts = [] + inputs.append(inp) # host_vars last, as Ansible does: inventory inline first, then files. for host, design in sorted(inline_host_vars(inventory_file).items()): - inputs.append(Input(f"{host}-inline", classify(design), design, hosts=[host])) + for name, kind, payload in _by_category(f"{host}-inline", design): + inputs.append(Input(name, kind, payload, hosts=[host])) host_var_dir = var_dir / "host_vars" if host_var_dir.is_dir(): for f in sorted(host_var_dir.glob("*.yml")): design = _strip_ansible_keys(_yaml_load(f)) - if design: - inputs.append(Input(f.stem, classify(design), design, hosts=[f.stem])) + for name, kind, payload in _by_category(f.stem, design): + inputs.append(Input(name, kind, payload, hosts=[f.stem])) return inputs diff --git a/tests/test_fabric_collect.py b/tests/test_fabric_collect.py index 68da053..8adc70e 100644 --- a/tests/test_fabric_collect.py +++ b/tests/test_fabric_collect.py @@ -149,7 +149,7 @@ def test_secret_input_is_refused_until_implemented() -> None: def test_pattern_matching_no_device_is_refused() -> None: """A pattern is silent about matching nothing, so it cannot be allowed to.""" settings = _input_xr( - "Settings", + "SettingSet", "typo", # The fabric holds only spine1, so this pattern matches nothing. {"design": {"ntp_settings": {}}, "appliesTo": {"matchHostnames": ["leaf.*"]}}, @@ -159,10 +159,10 @@ def test_pattern_matching_no_device_is_refused() -> None: _fabric( [ {"kind": "NodeSet", "name": "spines"}, - {"kind": "Settings", "name": "typo"}, + {"kind": "SettingSet", "name": "typo"}, ] ), - required={"000-nodeset-spines": [SPINES], "001-settings-typo": [settings]}, + required={"000-nodeset-spines": [SPINES], "001-settingset-typo": [settings]}, ) ) @@ -171,7 +171,7 @@ def test_pattern_matching_no_device_is_refused() -> None: assert not rsp.desired.resources -@pytest.mark.parametrize("kind", ["NodeSet", "NetworkServices", "ConnectedEndpoints", "Settings"]) +@pytest.mark.parametrize("kind", ["NodeSet", "NetworkServiceSet", "ConnectedEndpointSet", "SettingSet"]) def test_input_kinds_reconcile_and_report_their_keys(kind: str) -> None: """Each input reports its own shape on its own object, composing nothing.""" rsp = _run(_request(_input_xr(kind, "an-input", {"design": {"ntp_settings": {}, "type": "x"}}))) diff --git a/tests/test_kinds_equivalence.py b/tests/test_kinds_equivalence.py index 27376c8..f7c1fbf 100644 --- a/tests/test_kinds_equivalence.py +++ b/tests/test_kinds_equivalence.py @@ -15,13 +15,15 @@ import pytest -from function.kinds import Input, resolve +from function.kinds import Input, classify, resolve from function.verify_kinds import ( DEFERRED, DEFERRED_RENDER, EXAMPLES_ROOT, + _by_category, _discover, _discover_molecule, + inputs_from_inventory, render_one, verify_one, ) @@ -81,8 +83,8 @@ def test_later_input_overwrites_earlier() -> None: inputs = [ Input("nodes", "NodeSet", {"l3leaf": {"nodes": [{"name": "leaf1"}]}}, node_sets=["nodes"], declares=["leaf1"]), - Input("base", "Settings", {"ntp_settings": {"servers": ["a"]}}, all_devices=True), - Input("narrow", "Settings", {"ntp_settings": {"servers": ["b"]}}, hosts=["leaf1"]), + Input("base", "SettingSet", {"ntp_settings": {"servers": ["a"]}}, all_devices=True), + Input("narrow", "SettingSet", {"ntp_settings": {"servers": ["b"]}}, hosts=["leaf1"]), ] assert resolve(inputs)["leaf1"]["ntp_settings"] == {"servers": ["b"]} @@ -101,6 +103,141 @@ def test_input_applies_only_where_scoped() -> None: assert out["leaf2"]["l3leaf"]["defaults"]["loopback_ipv4_pool"] == "10.1.0.0/24" +def test_network_services_is_classified_under_either_spelling() -> None: + """AVD 6.x reads two spellings for the same content, and both are services. + + `shared_utils/filtered_tenants.py` reads `inputs.network_services` and then + the dynamic keys named by `network_services_keys` (default `tenants`). + Classifying the native spelling as `SettingSet` would put a services + document -- its BGP passwords and OSPF auth keys with it -- in the kind that + owns fabric-wide settings, and RBAC is granted per kind. + + Latent when written: of 25 inventories in AVD's corpus only + `eos_designs_unit_tests` uses the native spelling, and there it travels with + `network_services_keys`, which already classified it correctly. + """ + tenants = [{"name": "TENANT_A", "vrfs": [{"name": "VRF10"}]}] + assert classify({"tenants": tenants}) == "NetworkServiceSet" + assert classify({"network_services": tenants}) == "NetworkServiceSet" + + +def test_a_node_block_outranks_every_other_category() -> None: + """Classification is per fragment, first match wins -- a fragment mixing a + node block with anything else is a NodeSet, whole. + + Written down because it is what AVD's own host_vars do: one file carrying + `wan_router` alongside `bgp_peer_groups` and `wan_ipsec_profiles` becomes a + single NodeSet carrying those credentials. Ownership only -- `resolve` never + reads a kind, so no render changes with it. + """ + design = { + "wan_router": {"nodes": [{"name": "wan1"}]}, + "bgp_peer_groups": {"wan_overlay_peers": {"password": "x"}}, + "tenants": [{"name": "TENANT_A"}], + } + assert classify(design) == "NodeSet" + + +def test_a_group_splits_into_one_input_per_category() -> None: + """Merged as Ansible merges, then split -- so each XR carries one category. + + `cv-pathfinder`'s `group_vars/WAN/` is four files whose author had already + separated settings, interface profiles, management and tenants. Ansible + merges a group_vars directory into one namespace, and classifying that whole + let a single `tenants` key decide the kind for 22. + """ + parts = _by_category( + "WAN", + { + "l3leaf": {"nodes": [{"name": "leaf1"}]}, + "tenants": [{"name": "TENANT_A"}], + "aaa_settings": {"local_users": [{"name": "admin"}]}, + }, + ) + assert [(name, kind) for name, kind, _ in parts] == [ + ("WAN", "NodeSet"), + ("WAN-services", "NetworkServiceSet"), + ("WAN-settings", "SettingSet"), + ] + assert [sorted(design) for _, _, design in parts] == [ + ["l3leaf"], ["tenants"], ["aaa_settings"] + ] + + +def test_a_group_of_one_category_keeps_its_bare_name() -> None: + """No suffix where there is nothing to tell apart -- `NETWORK_SERVICES.yml` + stays `NETWORK_SERVICES`, not `NETWORK_SERVICES-services`.""" + design = {"tenants": [{"name": "TENANT_A"}]} + assert _by_category("NETWORK_SERVICES", design) == [ + ("NETWORK_SERVICES", "NetworkServiceSet", design) + ] + + +def test_a_host_outside_the_play_is_not_a_device() -> None: + """`cv-pathfinder` holds `cloudvision`, which is not a switch. + + Its inventory carries the CloudVision API server so `cv_deploy` can reach + it; `build.yml` runs eos_designs on `hosts: WAN`, and there is no + `cloudvision.cfg` in the example's intended configs. Declaring it would + compose a Device for it -- and a declared device with no node type fails the + whole fabric's render with AVD's `No device type found`, not just its own. + + This is the only host in the 8 bundled examples where the inventory and the + play disagree, which is why the model collapsed the two lists for so long. + """ + root = EXAMPLES_ROOT / "cv-pathfinder" + if not root.is_dir(): + pytest.skip("AVD submodule not initialised") + + declared = {host for inp in inputs_from_inventory(root) for host in inp.declares} + + assert declared, "cv-pathfinder should still declare its WAN devices" + assert "cloudvision" not in declared, ( + "cloudvision is an API server, not a fabric device -- play_hosts should " + "have kept it out of the device list" + ) + assert "pf1" in declared, "the play's own devices must survive the restriction" + + +def test_unscoped_nodeset_reaches_only_what_it_declares() -> None: + """An omitted `appliesTo` means the whole fabric -- except on a NodeSet. + + Ansible has no unscoped group_vars file: a node-type block is read by its + own group. Defaulting a NodeSet to the whole fabric made every migrated + NodeSet name itself in `appliesTo`, 26 of 26 across AVD's examples. + """ + inputs = [ + Input("spines", "NodeSet", {"spine": {"nodes": [{"name": "spine1"}]}}, + declares=["spine1"]), + Input("leaves", "NodeSet", {"l3leaf": {"nodes": [{"name": "leaf1"}]}}, + declares=["leaf1"]), + Input("base", "SettingSet", {"ntp_settings": {"servers": ["a"]}}), + ] + out = resolve(inputs) + assert "spine" in out["spine1"] and "spine" not in out["leaf1"] + assert "l3leaf" in out["leaf1"] and "l3leaf" not in out["spine1"] + # Every other kind still defaults to the whole fabric. + assert out["spine1"]["ntp_settings"] == out["leaf1"]["ntp_settings"] + + +def test_a_nodeset_may_be_seen_wider_than_it_declares() -> None: + """The case the default must not swallow, and it is real. + + `eos_designs-twodc-5stage-clos` has a DC-level NodeSet declaring 4 + super_spines and visible to all 16 devices of that DC. Getting this wrong + cost 96 hostvar diffs once. + """ + inputs = [ + Input("dc1", "NodeSet", {"super_spine": {"nodes": [{"name": "ss1"}]}}, + declares=["ss1"], node_sets=["dc1", "dc1-pod1"]), + Input("dc1-pod1", "NodeSet", {"l3leaf": {"nodes": [{"name": "leaf1"}]}}, + declares=["leaf1"]), + ] + out = resolve(inputs) + assert "super_spine" in out["leaf1"], "a widened NodeSet must still reach the pod" + assert "l3leaf" not in out["ss1"], "the pod's own block stays in the pod" + + def test_undeclared_node_is_not_a_device() -> None: """A block may name a node the fabric does not declare -- AVD's own anta_runner does -- and it must not become a device.""" From 6006b0cc019e76e2a837d7348f947ea2ea7dd6ad Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Thu, 20 Aug 2026 12:00:39 +0000 Subject: [PATCH 7/8] Ask Ansible what an inventory means, instead of reimplementing it The migration had its own reader of inventories, group_vars and host_vars, and verify_kinds compared it against a second reader of the same files. Two of our readers on both sides of one comparison: a source neither read vanished from both and the test said EQUAL. Four gaps sat behind that green result and only ever surfaced when a render disagreed with AVD's own golden -- host_vars/*.yaml, host_vars// directories, inline group vars: blocks, and ansible-vault. function/ansible_cli.py asks Ansible instead. `ansible-inventory --list --export` states which group carries which variable -- the ownership boundary an input XR maps to, with every one of those four shapes already merged. An ad-hoc `debug` run states what a play resolves those variables to. `ansible-playbook --list-hosts --list-tasks` states which play runs eos_designs and on which devices, resolving the host pattern with Ansible's own engine. Five subprocesses per inventory, none of them parsing YAML we could misread. function/migrate.py turns that into one Fabric per play plus one input XR per ownership fragment per category. The one rule the Ansible CLI does not print is group order (depth, then name); the migration layers the fragments with it and refuses to emit anything unless the result equals what Ansible reports. Proven by reversing the order on campus-fabric -- and worth knowing that reversing it on single-dc-l3ls or dual-dc-l3ls changes nothing, because precedence is load-bearing in only two of the eight examples. Values are what the play produced, not what the file says. The trigger is not "does this look like Jinja" but "did the play produce something else", which needs no pattern and catches vault too: ansible-inventory does NOT decrypt, it emits {"__ansible_vault": "$ANSIBLE_VAULT;1.1;..."}, which reads like a resolved value in a diff and is not. Read back only on devices where that fragment wins the key, since a device some later fragment overrode carries someone else's value. Measured across the whole corpus: no group-level value resolves to different things on different devices, so one XR can carry one value. classify now reads its vocabulary from pyavd's public schema rather than from a literal. The literal was already short -- AVD ships `cameras` in connected_endpoints_keys and it was missing. What stays a literal is the handful of keys that are not settings, and tests/test_categories.py holds it against documentation_options.table in AVD's own schema, in both directions. `type` is the commonest key in the corpus at 639 fragments and AVD gives it a table of its own; calling it a setting is what forced synthetic -devices NodeSets. avd-migrate names what it cannot carry rather than dropping it. Two shapes of .j2 path look alike and are not: an interface_descriptions template decides a description string, an ip_addressing template decides an address -- eos_designs-twodc-5stage-clos computes its P2P uplink IPs that way. So --drop-description-templates drops only the first, and reports what it dropped. Measured on evpn_underlay_ebgp_overlay_ebgp: all 16 devices render and differ from golden in 168 places, every one of them a `description`. The fold path goes with it -- xr.py, ansible_inputs.py, verify_xr.py, verify_example.py and their two test files. verify_kinds --render was already its successor; this replaces that in turn, reaching every example rather than seven of eight. Measured against AVD's own corpus, with Ansible as the reference: hostvars byte-identical for all 8 examples and 19 molecule scenarios, up to 501 devices in one play. Rendered configs match AVD's checked-in golden for 8 of 8 examples (was 7) and 8 of 12 molecule scenarios shipping one (was 5). The four that remain are upstream, not the model: pyavd implements no Jinja templating (templar=None, NotImplementedError -- AVD's own action plugin reaches into pyavd's internal API to hand in Ansible's templar), pool_manager needs somewhere for a Fabric to keep an ID pool, and one scenario loads a custom Python module. 79 offline tests green in ~50s; 31 more behind -m corpus in ~3.5min. Each new guard was verified to fail without its change. Co-Authored-By: Claude Opus 5 --- README.md | 54 +-- function/__init__.py | 10 +- function/ansible_cli.py | 357 +++++++++++++++++++ function/ansible_inputs.py | 148 -------- function/kinds.py | 146 ++++++-- function/migrate.py | 601 ++++++++++++++++++++++++++++++++ function/verify_example.py | 106 ------ function/verify_kinds.py | 420 ---------------------- function/verify_xr.py | 103 ------ function/xr.py | 165 --------- pyproject.toml | 19 +- tests/test_categories.py | 126 +++++++ tests/test_engine_fidelity.py | 48 --- tests/test_kinds_equivalence.py | 469 ++++++++++++++++++------- tests/test_xr_fold.py | 29 -- uv.lock | 31 +- 16 files changed, 1634 insertions(+), 1198 deletions(-) create mode 100644 function/ansible_cli.py delete mode 100644 function/ansible_inputs.py create mode 100644 function/migrate.py delete mode 100644 function/verify_example.py delete mode 100644 function/verify_kinds.py delete mode 100644 function/verify_xr.py delete mode 100644 function/xr.py create mode 100644 tests/test_categories.py delete mode 100644 tests/test_engine_fidelity.py delete mode 100644 tests/test_xr_fold.py diff --git a/README.md b/README.md index 9de036b..1dc746b 100644 --- a/README.md +++ b/README.md @@ -83,8 +83,9 @@ provider's poll interval, the same rhythm that paces the rest of the model. **No cluster?** The engine and the function both run locally: ```bash -uv run avd-verify # pyavd vs AVD's own golden structured configs -uv run avd-verify-xr # same, through the Fabric-XR fold +uv run avd-migrate # AVD's own inventories -> Fabric + input XRs +uv run avd-migrate avd/ansible_collections/arista/avd/examples/single-dc-l3ls \ + --emit /tmp/xrs # ... and write them out as manifests crossplane render examples/fabric/single-dc-l3ls.yaml \ apis/fabric/composition.yaml dev/function-render.yaml ``` @@ -157,31 +158,42 @@ against the example's checked-in `intended/structured_configs`. Zero diffs everywhere it is ticked, across both group_vars layouts (per-group directories and flat files) and explicit and implicit `all` inventories. The fold -(`function/xr.py`) unions each DC's node-type blocks and pushes per-DC/per-pod -`defaults` down to node_groups/nodes (which override defaults in AVD), so multi-DC -fabrics collapse losslessly into one document. - -The two deferrals are understood, not mysterious: - -- **campus-fabric** — leaves carry RADIUS in `aaa_settings`, spines don't: a - fabric-global key that genuinely differs by role, with no node-scoped equivalent. -- **cv-pathfinder** — SD-WAN multi-site (a WAN gateway across 2 routers) plus - ansible-vault secrets. - -They live in `verify_xr.DEFERRED` as *strict* expected failures: if one starts folding, -the suite fails and says to remove it, so a deferral can't quietly rot. +An inventory becomes one `Fabric` per eos_designs play, plus one input XR per +ownership fragment per category. Nothing is folded into a single document and +nothing is merged: two NodeSets carrying the same node-type key never meet, +because no device sees both. + +**The migration reimplements no part of Ansible.** `function/ansible_cli.py` +asks it instead -- `ansible-inventory --list --export` for which group carries +which variable, and an ad-hoc `debug` run for what a play resolves those +variables to. The one rule the Ansible CLI does not print is group order +(depth, then name); the migration layers the fragments with it and **refuses to +emit anything** unless the result equals what Ansible reports. + +Measured over AVD's own corpus: byte-identical hostvars for all 8 bundled +examples and 19 molecule scenarios, up to 501 devices in one play; and rendered +configs matching AVD's checked-in golden for 8 of 8 examples and 8 of 12 +molecule scenarios that ship one. + +What the remaining four meet is upstream, not the model: pyavd implements no +Jinja templating (`templar=None`, `NotImplementedError`), `pool_manager` needs +somewhere for a Fabric to keep an ID pool, and one scenario loads a custom +Python module. `avd-migrate` names each of them rather than dropping it -- +`--drop-description-templates` is the one exception, and only because dropping +those was measured to cost `description` fields and nothing else. ## Testing ```bash -uv run pytest # offline: engine fidelity, the XR fold, the Struct gotcha (~7s) +uv run pytest # offline: resolution vs Ansible, goldens, the Struct gotcha (~50s) +uv run pytest -m corpus # the whole molecule corpus: 501 devices, 71 plays (~3min) uv run pytest -m e2e # live cluster: needs kind-up.sh + an applied fabric (~2min) ``` | Path | Covers | |------|---------| -| `tests/test_engine_fidelity.py` | the examples above reproduce golden structured config | -| `tests/test_xr_fold.py` | the Ansible→XR fold, over every discovered example | +| `tests/test_kinds_equivalence.py` | the inputs resolve exactly as Ansible does, and render golden | +| `tests/test_categories.py` | which kind a key belongs to still matches AVD's own schema | | `tests/test_normalize_numbers.py` | the protobuf-`Struct` double→int coercion | | `tests/test_push.py` | the eAPI push Request builders: session contents, digest provenance | | `tests/test_e2e_device_layer.py` | drift/reclaim + steady-state idempotency, on a cluster | @@ -357,9 +369,9 @@ The non-obvious things this repo encodes, each of which cost a debugging session | `function/fn.py` | the Crossplane composite function (FunctionRunner: Fabric + Device) | | `function/engine.py` | pyavd pipeline wrapper; `render_fabric_design` is the function's core | | `function/push.py` | eAPI push protocol: the provider-http `Request` builders | -| `function/xr.py` | fold an Ansible example into a `Fabric` document (block union + defaults push-down) | -| `function/ansible_inputs.py` | rebuild `all_inputs` from an Ansible example (inventory + group_vars merge) | -| `function/verify_example.py`, `verify_xr.py` | golden-diff harnesses (`avd-verify`, `avd-verify-xr`) | +| `function/kinds.py` | the input-kind model: `Input`, `resolve`, and which kind a key belongs to | +| `function/ansible_cli.py` | Ansible asked rather than reimplemented (dev-only; the runtime never imports it) | +| `function/migrate.py` | an AVD inventory -> `Fabric` + input XRs (`avd-migrate`) | | `apis/fabric/`, `apis/device/` | XRD + Composition for each layer | | `apis/crossplane.yaml` | Configuration package metadata — `apis/` is that package's root | | `dev/` | `Function` manifests for local use (kind install, `crossplane render`); outside `apis/` so the Configuration build needs no exclusions | diff --git a/function/__init__.py b/function/__init__.py index 98fa0f1..6f462e3 100644 --- a/function/__init__.py +++ b/function/__init__.py @@ -1,11 +1,11 @@ """function-avd: a living AVD supermodel driven by Crossplane XRs. -Exposes the pyavd pipeline fed from AVD Ansible examples, which proves the -engine reproduces AVD's golden structured configs -- the same engine the -Crossplane composite function (`fn.py`) wraps. +Exposes the pyavd pipeline the Crossplane composite function (`fn.py`) wraps. +Nothing here reaches the migration harness: the runtime is handed input XRs and +never sees an inventory, so `ansible_cli` and `migrate` are imported by the +tools that need them and by nothing else. """ -from .ansible_inputs import build_all_inputs from .engine import render_structured_configs, validate_all -__all__ = ["build_all_inputs", "render_structured_configs", "validate_all"] +__all__ = ["render_structured_configs", "validate_all"] diff --git a/function/ansible_cli.py b/function/ansible_cli.py new file mode 100644 index 0000000..4e584a9 --- /dev/null +++ b/function/ansible_cli.py @@ -0,0 +1,357 @@ +"""Ansible, asked rather than reimplemented. + +Everything about an inventory that the migration needs is something Ansible +already states, so nothing here parses a ``group_vars`` tree, expands an +inventory pattern or opens a vault. Four questions, three subprocesses: + +``ansible-inventory --list --export`` + Which group carries which variable -- the ownership boundary an input XR + maps to. Inline ``vars:`` blocks, ``group_vars/.yml``, + ``group_vars//*.yml`` and their ``.yaml`` spellings are already merged + into one namespace per group, exactly as one XR carries one fragment. + +``ansible -m debug -a var=hostvars[inventory_hostname]`` + The merged per-host variables **after templating**. Not a source -- an + oracle. The migration layers the fragments itself and refuses to emit + anything unless the result equals this, so its precedence model can never be + silently wrong. + + An ad-hoc run rather than ``ansible-inventory --list`` because + ``ansible-inventory`` does not template and has no flag that makes it: + `{{ playbook_dir }}`, `{{ spine_bgp_defaults }}` and cv-pathfinder's + `bgp_password | arista.avd.encrypt` come out as literal strings, and pyavd + then meets a `Str` where it wants a `List`. A play templates; this is the + cheapest thing that is a play. `--list` remains available for a caller that + wants the raw text. + +``ansible-playbook --list-tasks`` / ``--list-hosts`` + Which play runs eos_designs, and which devices it runs on. AVD renders a + play, not an inventory, and Ansible resolves the host pattern with its own + engine -- including ``!`` exclusions and ``:&`` intersections, which reading + the playbook's ``hosts:`` string cannot do. + +Only the *harness* depends on this. The runtime takes XRs and never sees an +inventory, so ``ansible-core`` is a development dependency and reaches neither +the image nor the published package. +""" + +from __future__ import annotations + +import json +import os +import re +import shutil +import subprocess +import tempfile +from dataclasses import dataclass, field +from pathlib import Path + +ALL_GROUP = "all" +TIMEOUT = 900 + +#: Transport variables that make an ad-hoc `debug` fail although it never +#: connects. `ansible_connection` in the inventory outranks `-c`, and AVD's +#: examples set it to `ansible.netcommon.httpapi` with `become_method: enable`, +#: neither of which is installed here. Extra vars are the only precedence level +#: above inventory vars. +_ADHOC_OVERRIDES = ("ansible_connection=local", "ansible_become=false") + +# `play #3 (FABRIC): Build Configurations TAGS: []` +_PLAY = re.compile(r"^\s*play #(\d+) \((?P.*)\): (?P.*)\tTAGS:") +# ` arista.avd.eos_designs : Validate eos_designs inputs TAGS: []` +_TASK = re.compile(r"^\s+(?P[\w.]+) : .*\tTAGS:") + + +class AnsibleError(RuntimeError): + """An ansible CLI call failed. Carries what it printed.""" + + +@dataclass(frozen=True) +class Play: + """One play, as ``ansible-playbook`` reports it.""" + + playbook: str + index: int + name: str + pattern: str + hosts: tuple[str, ...] + roles: frozenset[str] + + @property + def runs_eos_designs(self) -> bool: + return any(role.endswith("eos_designs") for role in self.roles) + + +@dataclass +class Inventory: + """An inventory as Ansible describes it, at both levels.""" + + group_vars: dict[str, dict] = field(default_factory=dict) + host_vars: dict[str, dict] = field(default_factory=dict) + children: dict[str, set[str]] = field(default_factory=dict) + direct_hosts: dict[str, set[str]] = field(default_factory=dict) + #: merged per-host variables -- the oracle, never a source + hostvars: dict[str, dict] = field(default_factory=dict) + #: whether `hostvars` had its Jinja evaluated + templated: bool = False + + @property + def depth(self) -> dict[str, int]: + """Longest path from ``all``, which is what orders Ansible's groups. + + The one rule here that Ansible does not print. It is never trusted: + :func:`function.migrate.layer` reproduces ``hostvars`` with it, and the + migration refuses to emit XRs when that fails. + """ + out = {g: 0 for g in self.children} + changed = True + while changed: + changed = False + for parent, kids in self.children.items(): + for kid in kids: + if out.get(parent, 0) + 1 > out.get(kid, 0): + out[kid] = out[parent] + 1 + changed = True + return out + + def members(self, group: str) -> set[str]: + """Every host in the group, transitively.""" + seen: set[str] = set() + stack = [group] + hosts: set[str] = set() + while stack: + current = stack.pop() + if current in seen: + continue + seen.add(current) + hosts |= self.direct_hosts.get(current, set()) + stack.extend(self.children.get(current, ())) + return hosts + + def groups_of(self, host: str) -> list[str]: + """The host's groups, in Ansible's precedence order.""" + mine = {g for g in self.children if host in self.members(g)} | {ALL_GROUP} + depth = self.depth + return sorted(mine, key=lambda g: (depth.get(g, 0), g)) + + @property + def hosts(self) -> set[str]: + return self.members(ALL_GROUP) | set(self.host_vars) + + +def _strip(data: dict) -> dict: + """Drop Ansible's own transport variables -- not part of the AVD model.""" + return {k: v for k, v in data.items() if not k.startswith("ansible_")} + + +def _env(collections: Path | None) -> dict[str, str]: + env = dict(os.environ) + if collections is not None: + env["ANSIBLE_COLLECTIONS_PATH"] = str(collections) + return env + + +def _run(binary: str, args: list[str], cwd: Path, collections: Path | None) -> str: + """Run an ansible CLI in the inventory's own directory. + + The directory matters: ``ansible.cfg`` is read from the working directory, + and it is what points cv-pathfinder at its vault password file. + """ + path = shutil.which(binary) + if path is None: + raise AnsibleError( + f"{binary} not found. This is a development dependency; install it " + f"with `uv run --with ansible-core ...` or add it to the dev group." + ) + result = subprocess.run( + [path, *args], cwd=cwd, capture_output=True, text=True, + env=_env(collections), timeout=TIMEOUT, + ) + if result.returncode != 0: + raise AnsibleError( + f"{binary} {' '.join(args)} (in {cwd}) exited {result.returncode}:\n" + f"{(result.stderr or result.stdout).strip()[:2000]}" + ) + return result.stdout + + +def find_inventory(root: Path) -> Path: + """The inventory file, in either layout AVD ships. + + Absolute, always. Every call runs with ``cwd`` set to the inventory's own + directory, and a relative ``-i`` that does not resolve from there makes + ``ansible-inventory`` **exit 0 with an empty inventory** rather than fail -- + a silent wrong answer, not an error. + """ + root = Path(root).resolve() + for candidate in (root / "inventory.yml", root / "inventory" / "hosts.yml"): + if candidate.is_file(): + return candidate + raise AnsibleError(f"no inventory.yml or inventory/hosts.yml under {root}") + + +def templated_hostvars(root: Path, inventory: Path, collections: Path | None, + known: set[str]) -> dict[str, dict]: + """Per-host variables as a play sees them -- Jinja evaluated. + + ``--tree`` writes one JSON file per host, which is the only machine-readable + output an ad-hoc run offers. + + ⚠ **A value source, never a key source.** The dump carries Ansible's magic + variables (`groups`, `inventory_hostname`, `playbook_dir`, ...) beside the + inventory's own, and those are not part of any document. Rather than name + them -- a literal that would go stale the way every literal here has -- keep + only the keys the inventory itself declares, which ``--export`` already said. + """ + out: dict[str, dict] = {} + with tempfile.TemporaryDirectory() as tree: + _run( + "ansible", + ["all", "-i", str(inventory), "-m", "ansible.builtin.debug", + "-a", "var=hostvars[inventory_hostname]", + *[arg for override in _ADHOC_OVERRIDES for arg in ("-e", override)], + "--tree", tree], + root, collections, + ) + for path in sorted(Path(tree).iterdir()): + try: + body = json.loads(path.read_text()) + except (OSError, ValueError): + continue + data = body.get("hostvars[inventory_hostname]") + if isinstance(data, dict): + out[path.name] = {k: v for k, v in data.items() if k in known} + return out + + +def read_inventory(root: Path, inventory: Path | None = None, + collections: Path | None = None, templated: bool = True) -> Inventory: + """Both levels of the inventory, in two subprocesses. + + ``templated`` decides what the oracle is: what a play would hand AVD + (default), or the raw text ``ansible-inventory --list`` prints. Two calls + either way -- the templated oracle replaces ``--list`` rather than joining it. + """ + root = Path(root).resolve() + inventory = Path(inventory).resolve() if inventory else find_inventory(root) + args = ["-i", str(inventory), "--playbook-dir", str(root), "--list"] + export = json.loads(_run("ansible-inventory", [*args, "--export"], root, collections)) + merged = ( + {} if templated + else json.loads(_run("ansible-inventory", args, root, collections)) + ) + + inv = Inventory() + for group, body in export.items(): + if group == "_meta": + continue + inv.group_vars[group] = _strip(body.get("vars") or {}) + inv.children[group] = set(body.get("children") or []) + inv.direct_hosts[group] = set(body.get("hosts") or []) + for group in list(inv.children): + for kid in inv.children[group]: + inv.children.setdefault(kid, set()) + inv.direct_hosts.setdefault(kid, set()) + inv.children.setdefault(ALL_GROUP, set()) + + for host, data in (export.get("_meta", {}).get("hostvars") or {}).items(): + inv.host_vars[host] = _strip(data) + if templated: + known = {k for design in inv.group_vars.values() for k in design} + known |= {k for design in inv.host_vars.values() for k in design} + inv.hostvars = templated_hostvars(root, inventory, collections, known) + inv.templated = True + else: + for host, data in (merged.get("_meta", {}).get("hostvars") or {}).items(): + inv.hostvars[host] = _strip(data) + # A host carrying no variables at all is omitted by both; it still exists, + # and its merged view is empty. + for host in inv.hosts: + inv.hostvars.setdefault(host, {}) + return inv + + +def _parse_plays(text: str, playbook: str) -> dict[int, dict]: + """Split ``--list-hosts --list-tasks`` output into plays by index. + + The two flags combine, so one subprocess answers both questions: which + devices a play targets, and which roles it runs. Asking separately cost + twice as many processes and told us nothing more. + """ + plays: dict[int, dict] = {} + current: int | None = None + section: str | None = None + for line in text.splitlines(): + header = _PLAY.match(line) + if header: + current = int(header.group(1)) + section = None + plays[current] = { + "playbook": playbook, + "name": header.group("name").strip(), + "pattern": header.group("pattern"), + "hosts": [], + "tasks": [], + } + continue + if current is None: + continue + stripped = line.strip() + if stripped.startswith("hosts (") and stripped.endswith("):"): + section = "hosts" + continue + if stripped == "tasks:": + section = "tasks" + continue + if section == "hosts" and stripped and not stripped.endswith(":"): + plays[current]["hosts"].append(stripped) + elif section == "tasks": + task = _TASK.match(line) + if task: + plays[current]["tasks"].append(task.group("role")) + return plays + + +def plays(root: Path, collections: Path | None = None, + inventory: Path | None = None) -> list[Play]: + """Every play in every playbook beside the inventory, with hosts and roles. + + One subprocess per playbook. They can be passed together in a single call, + but a playbook that does not resolve then takes every other one down with + it -- AVD's examples ship `deploy.yml`, which needs the `arista.eos` + collection -- so the batch is not worth the failure it introduces. + + Playbooks that do not parse are skipped rather than fatal: a molecule + scenario keeps ``molecule.yml`` next to its playbooks and that is a config + file, not a play. + """ + root = Path(root).resolve() + inventory = Path(inventory).resolve() if inventory else find_inventory(root) + found: list[Play] = [] + for playbook in sorted(root.glob("*.yml")) + sorted(root.glob("*.yaml")): + if playbook.resolve() == inventory.resolve() or playbook.name == "molecule.yml": + continue + try: + output = _run( + "ansible-playbook", + ["-i", str(inventory), playbook.name, "--list-hosts", "--list-tasks"], + root, collections, + ) + except AnsibleError: + continue + for index, play in sorted(_parse_plays(output, playbook.name).items()): + found.append(Play( + playbook=playbook.name, + index=index, + name=play["name"], + pattern=play["pattern"], + hosts=tuple(play["hosts"]), + roles=frozenset(play["tasks"]), + )) + return found + + +def design_plays(root: Path, collections: Path | None = None, + inventory: Path | None = None) -> list[Play]: + """The plays that run eos_designs -- one fabric each.""" + return [p for p in plays(root, collections, inventory) if p.runs_eos_designs] diff --git a/function/ansible_inputs.py b/function/ansible_inputs.py deleted file mode 100644 index 2d79640..0000000 --- a/function/ansible_inputs.py +++ /dev/null @@ -1,148 +0,0 @@ -"""Reconstruct pyavd ``all_inputs`` from an AVD Ansible example directory. - -This replicates the parts of Ansible we rely on to feed :mod:`pyavd`: - -* ``inventory.yml`` provides the group hierarchy and host membership. -* ``group_vars//*.yml`` provide the layered AVD data model. - -Ansible merges group_vars **per host**, in precedence order (``all`` first, -then groups sorted by depth and name, host_vars last), with default -``hash_behaviour = replace`` (top-level keys override wholesale). We reproduce -exactly that so the resulting hostvars equal what ``ansible-playbook`` would -hand to AVD. - -The output is ``{hostname: hostvars}`` -- the ``all_inputs`` mapping consumed by -``pyavd.get_avd_facts``. -""" - -from __future__ import annotations - -from pathlib import Path - -import yaml - -ALL_GROUP = "all" - - -class _AnsibleLoader(yaml.SafeLoader): - """SafeLoader that tolerates Ansible-specific tags (e.g. ``!vault``). - - Vault-encrypted values are kept as opaque strings -- enough to parse the - file. Reproducing configs that embed them still needs the vault password. - """ - - -_AnsibleLoader.add_constructor( - "!vault", lambda loader, node: loader.construct_scalar(node) -) - - -def _yaml_load(path: Path): - return yaml.load(path.read_text(), Loader=_AnsibleLoader) or {} - - -class AnsibleInventory: - """Group hierarchy + host membership parsed from an ``inventory.yml`` tree.""" - - def __init__(self) -> None: - self.children: dict[str, set[str]] = {ALL_GROUP: set()} - self.direct_hosts: dict[str, set[str]] = {} - self.depth: dict[str, int] = {ALL_GROUP: 0} - - @classmethod - def from_file(cls, inventory_path: Path) -> "AnsibleInventory": - data = _yaml_load(inventory_path) - inv = cls() - if ALL_GROUP in data: - root = data[ALL_GROUP] or {} - else: - # No explicit `all:` root -> every top-level key is an implicit - # child group of `all` (standard Ansible inventory behaviour). - root = {"children": {k: v for k, v in data.items() if k != "_meta"}} - inv._walk(ALL_GROUP, root) - inv._compute_depths() - return inv - - def _walk(self, group: str, body: dict | None) -> None: - body = body or {} - self.children.setdefault(group, set()) - for host in (body.get("hosts") or {}): - self.direct_hosts.setdefault(group, set()).add(host) - for child, child_body in (body.get("children") or {}).items(): - self.children[group].add(child) - self._walk(child, child_body) - - def _compute_depths(self) -> None: - # Ansible depth = longest path from `all`. Iterate to a fixpoint. - for g in self.children: - self.depth.setdefault(g, 0) - changed = True - while changed: - changed = False - for parent, kids in self.children.items(): - for kid in kids: - d = self.depth[parent] + 1 - if d > self.depth.get(kid, 0): - self.depth[kid] = d - changed = True - - def hosts(self) -> set[str]: - return {h for hs in self.direct_hosts.values() for h in hs} - - def groups_for_host(self, host: str) -> list[str]: - """All groups the host belongs to (transitively), in Ansible merge order.""" - direct = {g for g, hs in self.direct_hosts.items() if host in hs} - groups: set[str] = {ALL_GROUP} - for g in direct: - groups.add(g) - groups |= self._ancestors(g) - return sorted(groups, key=lambda g: (self.depth.get(g, 0), g)) - - def _ancestors(self, group: str) -> set[str]: - parents = {p for p, kids in self.children.items() if group in kids} - result = set(parents) - for p in parents: - result |= self._ancestors(p) - return result - - -def _strip_ansible_keys(data: dict) -> dict: - """Drop Ansible transport vars (ansible_connection, ansible_user, ...). - - They are not part of the AVD data model and will not exist on our XRs. - """ - return {k: v for k, v in data.items() if not k.startswith("ansible_")} - - -def _load_group_vars(group_vars_dir: Path, group: str) -> dict: - """Merge every ``*.yml`` under ``group_vars//`` (alphabetically).""" - merged: dict = {} - dir_path = group_vars_dir / group - single_file = group_vars_dir / f"{group}.yml" - files: list[Path] = [] - if dir_path.is_dir(): - files = sorted(dir_path.glob("*.yml")) + sorted(dir_path.glob("*.yaml")) - elif single_file.is_file(): - files = [single_file] - for f in files: - data = _yaml_load(f) - merged.update(data) # hash_behaviour = replace - return merged - - -def build_all_inputs(example_dir: str | Path) -> dict[str, dict]: - """Return ``{hostname: hostvars}`` for a single-DC AVD example directory.""" - example_dir = Path(example_dir) - inventory = AnsibleInventory.from_file(example_dir / "inventory.yml") - group_vars_dir = example_dir / "group_vars" - - group_cache: dict[str, dict] = {} - all_inputs: dict[str, dict] = {} - for host in sorted(inventory.hosts()): - hostvars: dict = {} - for group in inventory.groups_for_host(host): - if group not in group_cache: - group_cache[group] = _load_group_vars(group_vars_dir, group) - hostvars.update(group_cache[group]) # replace semantics, in precedence order - all_inputs[host] = _strip_ansible_keys(hostvars) - return all_inputs diff --git a/function/kinds.py b/function/kinds.py index 8813c96..e529aba 100644 --- a/function/kinds.py +++ b/function/kinds.py @@ -9,8 +9,8 @@ **Nothing is merged.** Two NodeSets carrying the same node-type key never meet, because no device sees both: in a dual-DC fabric a DC1 leaf sees DC1's -``l3leaf.defaults`` and a DC2 leaf sees DC2's. That is why this path needs -neither a fabric-wide document nor the fold in :mod:`function.xr`. +``l3leaf.defaults`` and a DC2 leaf sees DC2's. So there is no fabric-wide +document to assemble and no conflict to resolve. Two things are separate that look like one: @@ -22,15 +22,16 @@ names four devices but is visible to every device of that DC. Measured against AVD's own corpus: the hostvars this produces are byte-identical -to faithfully reproduced Ansible for all 8 bundled examples and every eos_designs -molecule scenario -- 25 inventories, up to 501 devices. See -:mod:`function.verify_kinds`. +to what **Ansible itself** reports -- all 8 bundled examples and 19 molecule +scenarios, up to 501 devices in one play. :mod:`function.migrate` builds the +inputs and :mod:`function.ansible_cli` supplies the reference. """ from __future__ import annotations import re from dataclasses import dataclass, field +from functools import lru_cache from typing import Any KINDS = ("NodeSet", "NetworkServiceSet", "ConnectedEndpointSet", "SettingSet") @@ -68,31 +69,132 @@ def hosts_in_blocks(design: dict) -> set[str]: return hosts -def classify(design: dict) -> str: - """Which kind a fragment belongs to. +@lru_cache(maxsize=1) +def _default_vocabulary() -> "Vocabulary": + """The dynamic key names AVD invents when a document says nothing. - Advisory: the kinds exist for ownership (RBAC is granted per kind), not as a - partition the schema could enforce -- eos_designs' top-level key names come - from its own content, so no OpenAPI schema can describe them. + Read from pyavd's own public schema rather than copied into a literal: the + three generators ship defaults (13 node types, 12 endpoint kinds, `tenants`), + and a hand-kept copy goes stale silently. It already had -- `cameras` was + added upstream and the literal this replaces never grew it. """ - if any(is_node_block(v) for v in design.values()): + from pyavd.api.schemas import AVDDesign + + design = AVDDesign() + return Vocabulary( + node_types=frozenset(e.key for e in design.node_type_keys), + network_services=frozenset(e.name for e in design.network_services_keys), + connected_endpoints=frozenset(e.key for e in design.connected_endpoints_keys), + ) + + +@dataclass(frozen=True) +class Vocabulary: + """The top-level key names in force for a document. + + eos_designs generates key names from its own content -- `node_type_keys`, + `network_services_keys` and `connected_endpoints_keys` each name a family of + top-level keys. So no static map can classify every key, and this is that + map made per document instead: AVD's defaults, extended by whatever + generators the document carries. + """ + + node_types: frozenset[str] + network_services: frozenset[str] + connected_endpoints: frozenset[str] + + @classmethod + def default(cls) -> "Vocabulary": + return _default_vocabulary() + + def extend(self, design: dict) -> "Vocabulary": + """Add the key names this document's own generators declare.""" + + def named(source: str, field_name: str) -> frozenset[str]: + entries = design.get(source) + if not isinstance(entries, list): + return frozenset() + return frozenset( + str(e[field_name]) for e in entries + if isinstance(e, dict) and e.get(field_name) + ) + + return Vocabulary( + node_types=self.node_types | named("node_type_keys", "key") + | named("custom_node_type_keys", "key"), + network_services=self.network_services | named("network_services_keys", "name"), + connected_endpoints=self.connected_endpoints + | named("connected_endpoints_keys", "key") + | named("custom_connected_endpoints_keys", "key"), + ) + + +# The keys eos_designs names itself that are *not* settings. Everything else in +# its schema is, so only the exceptions are listed -- and `test_categories` +# checks each one against `documentation_options.table` in AVD's own schema, so +# an upstream recategorisation fails the suite instead of drifting. +_NODE_SET_KEYS = frozenset({ + "type", # table: type-setting -- the device's node type + "node_type_keys", # table: node-type-keys -- names the node families + "custom_node_type_keys", + "l3_interface_profiles", # table: node-type-l3-interfaces-configuration +}) +_NETWORK_SERVICE_KEYS = frozenset({ + "network_services", # table: network-services + "network_services_keys", + "evpn_vlan_bundles", # table: evpn-vlan-bundles + "l2vlan_profiles", # table: network-services-l2vlans-settings + "mlag_ibgp_peering_vrfs", # table: network-services-vrfs-settings +}) +_CONNECTED_ENDPOINT_KEYS = frozenset({ + "connected_endpoints_keys", # table: connected-endpoints-keys + "custom_connected_endpoints_keys", + "default_connected_endpoints_description", + "default_connected_endpoints_port_channel_description", + "default_network_ports_description", + "default_network_ports_port_channel_description", + # AVD tags neither of these; they are endpoint content by their own reading + # and this repo places them here. Nothing upstream contradicts it. + "port_profiles", + "network_ports", +}) + + +def kind_of(key: str, value: Any, vocabulary: "Vocabulary | None" = None) -> str: + """Which kind one top-level key belongs to.""" + vocabulary = vocabulary or Vocabulary.default() + if key in vocabulary.node_types or key in _NODE_SET_KEYS or is_node_block(value): return "NodeSet" - # Both spellings. AVD 6.x reads a native `network_services` list as well as - # the dynamic keys named by `network_services_keys` (default `tenants`) -- - # `shared_utils/filtered_tenants.py` reads one after the other. A custom key - # name is only recognisable when `network_services_keys` travels with it. - if {"network_services", "tenants", "network_services_keys"} & design.keys(): + if key in vocabulary.network_services or key in _NETWORK_SERVICE_KEYS: return "NetworkServiceSet" - if { - "servers", "firewalls", "routers", "load_balancers", "storage_arrays", - "cpes", "workstations", "access_points", "phones", "printers", - "generic_devices", "port_profiles", "network_ports", - "connected_endpoints_keys", "custom_connected_endpoints_keys", - } & design.keys(): + if key in vocabulary.connected_endpoints or key in _CONNECTED_ENDPOINT_KEYS: return "ConnectedEndpointSet" return "SettingSet" +def by_kind(design: dict, vocabulary: "Vocabulary | None" = None) -> dict[str, dict]: + """Partition a fragment into ``{kind: design}``, keys in their own order.""" + vocabulary = (vocabulary or Vocabulary.default()).extend(design) + parts: dict[str, dict] = {} + for key, value in design.items(): + parts.setdefault(kind_of(key, value, vocabulary), {})[key] = value + return {kind: parts[kind] for kind in KINDS if kind in parts} + + +def classify(design: dict, vocabulary: "Vocabulary | None" = None) -> str: + """The kind a whole fragment belongs to: the one holding most of its keys. + + Advisory. The kinds exist for ownership -- RBAC is granted per kind -- not + as a partition a schema could enforce, because eos_designs' top-level key + names come from its own content. A fragment spanning categories is split by + :func:`by_kind` rather than resolved by this. + """ + parts = by_kind(design, vocabulary) + if not parts: + return "SettingSet" + return max(parts, key=lambda kind: len(parts[kind])) + + @dataclass class Input: """One input XR, reduced to what resolution needs.""" diff --git a/function/migrate.py b/function/migrate.py new file mode 100644 index 0000000..4bcb658 --- /dev/null +++ b/function/migrate.py @@ -0,0 +1,601 @@ +"""Turn an AVD Ansible inventory into a Fabric and its input XRs. + +The translation reads nothing itself: :mod:`function.ansible_cli` asks Ansible +which group carries which variable, which devices a play runs on, and what the +merged result is. This module only decides how those fragments become XRs. + +Four rules, and the last is what makes the others safe: + +* **One fabric per play.** AVD renders a play, not an inventory. Where the two + differ the inventory is wider -- cv-pathfinder carries `cloudvision`, an API + server that is not a switch -- and a declared device with no node type fails + the *whole* fabric's render, not just its own. +* **One input per ownership fragment per category.** A group's variables are one + fragment because Ansible merges them into one namespace; splitting them by + file would preserve a boundary Ansible does not keep. Splitting the merged + fragment by category afterwards costs nothing and is what the kinds are for. +* **Values are what the play produced, not what the file says.** An XR has no + templating engine, no vault and no playbook directory, so anything Ansible + resolves at play time is resolved before it is written down. See + :func:`templated`. +* **The precedence model is checked, not trusted.** Group order is (depth, name), + the one rule the Ansible CLI does not print. :func:`migrate` layers the + fragments with it and refuses to emit anything unless the result equals the + per-device variables Ansible reports. A wrong order cannot reach an XR. +""" + +from __future__ import annotations + +import json +import re +from dataclasses import dataclass, field +from pathlib import Path + +from .ansible_cli import ALL_GROUP, Inventory, Play, design_plays, plays, read_inventory +from .kinds import Input, KINDS, Vocabulary, by_kind, hosts_in_blocks, is_node_block + +#: kinds whose fragment gets a name suffix; a NodeSet keeps the plain name +SUFFIX = { + "NetworkServiceSet": "services", + "ConnectedEndpointSet": "endpoints", + "SettingSet": "settings", +} + + +class MigrationError(RuntimeError): + """The migration could not produce inputs it is able to stand behind.""" + + +def slug(name: str) -> str: + """A Kubernetes object name from an Ansible group or host name. + + Ansible group names are conventionally SHOUTED and use underscores; neither + survives RFC 1123. Collisions are the caller's to detect -- see + :func:`_unique`. + """ + out = re.sub(r"[^a-z0-9-]+", "-", name.lower()).strip("-") + return out or "unnamed" + + +@dataclass +class Fabric: + """One play, translated.""" + + name: str + devices: tuple[str, ...] + inputs: list[Input] = field(default_factory=list) + play: Play | None = None + #: AVD's own `fabric_name`, promoted to `spec.fabricName` + fabric_name: str = "" + #: things the XRs cannot express, said out loud rather than dropped + notes: list[str] = field(default_factory=list) + + @property + def requires(self) -> list[tuple[str, str]]: + """``spec.requires`` as (kind, name), in precedence order.""" + return [(i.kind, i.name) for i in self.inputs] + + +@dataclass(frozen=True) +class Fragment: + """One ownership unit: a group's variables, or a host's.""" + + name: str + design: dict + #: devices that see it + scope: frozenset[str] + + +def _fragments(inv: Inventory, devices: frozenset[str]) -> list[Fragment]: + """Every fragment that reaches a device, in Ansible's precedence order. + + `all` first, then groups by (depth, name), then host variables. That is a + global total order, so restricting one flat list per device reproduces that + device's view -- which is why `spec.requires` can be a single list. + """ + depth = inv.depth + ordered = sorted( + (g for g, v in inv.group_vars.items() if v), + key=lambda g: (depth.get(g, 0), g), + ) + out = [ + Fragment(group, inv.group_vars[group], + frozenset(devices if group == ALL_GROUP else inv.members(group) & devices)) + for group in ordered + ] + out += [ + Fragment(host, inv.host_vars[host], frozenset({host})) + for host in sorted(inv.host_vars) + if host in devices and inv.host_vars[host] + ] + return out + + +def _winners(fragments: list[Fragment]) -> dict[tuple[int, str], set[str]]: + """``(fragment index, key) -> devices where that fragment's value survives``. + + Layering is last-wins, so a fragment's own value is only observable on the + devices no later fragment overrides it for. That is exactly where a + templated value may be read back from the play's output. + """ + last: dict[str, dict[str, int]] = {} + for index, fragment in enumerate(fragments): + for device in fragment.scope: + for key in fragment.design: + last.setdefault(device, {})[key] = index + out: dict[tuple[int, str], set[str]] = {} + for device, keys in last.items(): + for key, index in keys.items(): + out.setdefault((index, key), set()).add(device) + return out + + +def templated(fragments: list[Fragment], hostvars: dict[str, dict]) -> list[Fragment]: + """Replace each fragment value with what the play actually produced. + + An XR has no templating engine and no playbook directory, so whatever + Ansible resolves at play time has to be resolved before the value is + written down. The trigger is **not** "does this look like Jinja" -- it is + "did the play produce something else", which needs no pattern and catches + every mechanism at once: + + * Jinja -- `{{ spine_bgp_defaults }}` reaches pyavd as a `Str` where AVD + wants a `List`; + * ansible-vault -- ⚠ `ansible-inventory` does **not** decrypt. Both `--list` + and `--export` emit `{"__ansible_vault": "$ANSIBLE_VAULT;1.1;AES256..."}`, + which looks resolved in a diff and is not. A play decrypts it. + + Two rules keep it honest: + + * a value is only read back on devices where **this** fragment wins the key + (:func:`_winners`); reading it off a device some later fragment overrode + would copy the wrong fragment's value; + * measured over AVD's whole corpus, no group-level value resolves to + different things on different devices. An input XR carries one value for + many devices, so if that ever stops holding the fragment is not + expressible and this refuses rather than picking one. + """ + winners = _winners(fragments) + out: list[Fragment] = [] + for index, fragment in enumerate(fragments): + design = dict(fragment.design) + for key, value in fragment.design.items(): + raw = json.dumps(value, sort_keys=True, default=str) + seen: dict[str, object] = {} + for device in sorted(winners.get((index, key), ())): + if key in hostvars.get(device, {}): + seen[json.dumps(hostvars[device][key], sort_keys=True, default=str)] = ( + hostvars[device][key] + ) + if len(seen) > 1: + raise MigrationError( + f"{fragment.name}.{key} resolves to {len(seen)} different values " + f"across the devices that see it; one input XR carries one value" + ) + if seen and next(iter(seen)) != raw: + design[key] = next(iter(seen.values())) + out.append(Fragment(fragment.name, design, fragment.scope)) + return out + + +def layer(fragments: list[Fragment], devices: frozenset[str]) -> dict[str, dict]: + """Apply the fragments in order -- Ansible's ``hash_behaviour=replace``.""" + out: dict[str, dict] = {device: {} for device in devices} + for fragment in fragments: + for device in fragment.scope: + out[device].update(fragment.design) + return out + + +def _node_owner(fragments: list[Fragment], devices: frozenset[str], + vocabulary: Vocabulary) -> dict[str, str]: + """Which fragment declares each device: the narrowest one with node content. + + "Narrowest" is last in precedence order, which is what depth already sorts + by. A device typed only through `default_node_types` matches no fragment and + is left for the caller to place. + """ + owner: dict[str, str] = {} + for fragment in fragments: + parts = by_kind(fragment.design, vocabulary) + if "NodeSet" not in parts: + continue + named = hosts_in_blocks(fragment.design) & devices + for device in fragment.scope | named: + if device in devices: + owner[device] = fragment.name + return owner + + +def _unique(name: str, taken: set[str]) -> str: + candidate, n = name, 2 + while candidate in taken: + candidate, n = f"{name}-{n}", n + 1 + taken.add(candidate) + return candidate + + +def _applies_to(inp: Input, scope: frozenset[str], devices: frozenset[str], + declared_by: dict[str, set[str]]) -> None: + """Say which devices see the input, as narrowly as it can be said. + + Silence means the whole fabric -- except on a NodeSet, where it means the + devices it declares. Both defaults carry the common case, so `appliesTo` + appears only where the answer is unusual: a DC-wide node block seen by more + devices than it declares. + """ + if inp.kind == "NodeSet" and scope == set(inp.declares): + return + if scope == devices: + inp.all_devices = True + return + cover = sorted(n for n, hosts in declared_by.items() if hosts and hosts <= scope) + covered: set[str] = set() + for name in cover: + covered |= declared_by[name] + if covered == scope: + inp.node_sets = cover + else: + inp.hosts = sorted(scope) + + +def _inputs(fragments: list[Fragment], devices: frozenset[str], + vocabulary: Vocabulary) -> list[Input]: + """Every input XR for one fabric, in precedence order. + + Two passes, because `appliesTo` may name a NodeSet that comes later in the + order: a group at depth 1 is commonly scoped to node sets defined at depth 2. + """ + owner = _node_owner(fragments, devices, vocabulary) + declares: dict[str, set[str]] = {} + for device, name in owner.items(): + declares.setdefault(name, set()).add(device) + + taken: set[str] = set() + declared_by: dict[str, set[str]] = {} + planned: list[tuple[Fragment, Input]] = [] + + # A device no fragment types is reached by `default_node_types`, by pattern. + # It still has to exist, so give it a NodeSet of its own, named after the + # narrowest fragment that reaches it. + orphans: dict[str, set[str]] = {} + for device in sorted(devices - set(owner)): + reaching = [f.name for f in fragments if device in f.scope] + orphans.setdefault(reaching[-1] if reaching else device, set()).add(device) + for base, hosts in sorted(orphans.items()): + name = _unique(f"{slug(base)}-devices", taken) + inp = Input(name, "NodeSet", {}, declares=sorted(hosts)) + declared_by[name] = set(hosts) + planned.append((Fragment(base, {}, frozenset(hosts)), inp)) + + for fragment in fragments: + parts = by_kind(fragment.design, vocabulary) + if not parts: + continue + base = slug(fragment.name) + for kind, design in parts.items(): + plain = len(parts) == 1 or kind == "NodeSet" + name = _unique(base if plain else f"{base}-{SUFFIX[kind]}", taken) + inp = Input(name=name, kind=kind, design=design) + if kind == "NodeSet": + inp.declares = sorted(declares.get(fragment.name, ())) + declared_by[name] = set(inp.declares) + if not fragment.scope and not inp.declares: + # Reaches nobody -- and "nobody" is not expressible in + # appliesTo, where silence means everybody. Emitting it would + # invert its meaning. + taken.discard(name) + declared_by.pop(name, None) + continue + planned.append((fragment, inp)) + + for fragment, inp in planned: + _applies_to(inp, fragment.scope, devices, declared_by) + return [inp for _, inp in planned] + + +def _fabric_name(root: Path, play: Play, many: bool) -> str: + if not many: + return slug(root.name) + return slug(f"{root.name}-{play.pattern}") or slug(f"{root.name}-{play.index}") + + +def migrate(root: Path, collections: Path | None = None, inventory: Path | None = None, + inv: Inventory | None = None, drop_descriptions: bool = False) -> list[Fabric]: + """Translate every eos_designs play under ``root`` into a Fabric. + + Raises :class:`MigrationError` when the layered fragments disagree with the + per-device variables Ansible reports. That is the whole safety property: the + only rule this module supplies is the group order, and it is never allowed + to be wrong silently. + """ + root = Path(root).resolve() + # Reading an inventory costs two subprocesses; a caller that already has one + # (a harness measuring the whole corpus) may hand it over. + inv = inv if inv is not None else read_inventory(root, inventory, collections) + found = design_plays(root, collections, inventory) + if not found: + # No play runs eos_designs -- eos_cli_config_gen scenarios carry + # structured config directly and render at the device layer. Say so + # rather than inventing a fabric out of the inventory. + raise MigrationError( + f"{root.name}: no play runs eos_designs " + f"({len(plays(root, collections, inventory))} plays seen)" + ) + + vocabulary = Vocabulary.default() + fabrics: list[Fabric] = [] + for play in found: + devices = frozenset(play.hosts) + if not devices: + continue + fragments = _fragments(inv, devices) + if inv.templated: + fragments = templated(fragments, inv.hostvars) + + expected = {d: inv.hostvars.get(d, {}) for d in devices} + if layer(fragments, devices) != expected: + differing = sorted( + f"{d}.{k}" for d in devices + for k in set(expected[d]) | set(layer(fragments, devices)[d]) + if expected[d].get(k) != layer(fragments, devices)[d].get(k) + ) + raise MigrationError( + f"{root.name} play #{play.index}: layering the fragments does not " + f"reproduce what Ansible reports ({len(differing)} differences, " + f"first: {', '.join(differing[:5])})" + ) + + fabric = Fabric( + name=_fabric_name(root, play, len(found) > 1), + devices=tuple(sorted(devices)), + inputs=_inputs(fragments, devices, vocabulary), + play=play, + ) + _fabric_name_of(fabric, inv.hostvars) + # Only now, with the translation proven faithful. Dropping anything + # before the comparison above would weaken the one gate this module has. + _report_unsupported(fabric, drop_descriptions) + fabrics.append(fabric) + return fabrics + + +def _report_unsupported(fabric: Fabric, drop_descriptions: bool) -> None: + """Note -- and optionally drop -- what pyavd will not honour.""" + if drop_descriptions: + dropped = [ + f"{inp.name}.{path}" + for inp in fabric.inputs + for path in drop_description_templates(inp.design) + ] + if dropped: + fabric.notes.append( + f"dropped {len(dropped)} interface-description template(s); AVD's own " + f"descriptions apply instead: {', '.join(dropped[:3])}" + + (" ..." if len(dropped) > 3 else "") + ) + + found: dict[str, list[str]] = {} + for inp in fabric.inputs: + for owner, paths in unsupported(inp.design).items(): + found.setdefault(owner, []).extend(f"{inp.name}.{p}" for p in paths) + for owner, paths in sorted(found.items()): + what = ( + "descriptions only -- `--drop-description-templates` renders without them" + if owner == COSMETIC + else "this decides addresses, not wording; dropping it would emit a " + "different network" + if owner == "ip_addressing" + else "custom code; a function image is immutable and loads no arbitrary Python" + if owner == "python_module" + else "pyavd implements no Jinja templating" + ) + fabric.notes.append(f"{len(paths)} x {owner} pyavd cannot honour ({what}): {paths[0]}") + + +#: A value pyavd cannot honour, and whether losing it is cosmetic. +#: +#: pyavd implements no Jinja templating: `get_device_structured_config` passes +#: `templar=None` and the call raises `NotImplementedError`. AVD's own Ansible +#: action plugin reaches into pyavd's internal API precisely to hand in a +#: templar built from Ansible's own -- which needs Ansible at render time, and +#: there is none in a cluster. +#: +#: So a template path cannot travel. What differs is what is lost with it: +#: an `interface_descriptions` template decides a `description` string, while an +#: `ip_addressing` template decides an address. Measured on +#: `evpn_underlay_ebgp_overlay_ebgp`: dropping its description templates renders +#: all 16 devices and differs from AVD's golden in 168 places, **all of them a +#: `description` field**. Dropping an addressing template would silently emit a +#: different network. +COSMETIC = "interface_descriptions" + + +def unsupported(design: dict) -> dict[str, list[str]]: + """Values in a fragment that pyavd cannot honour, by the key that owns them. + + Detected by shape rather than by a list of key names: a Jinja template is a + string naming a `.j2` file, and custom logic is a `python_module`. Both are + code paths into a filesystem that a cluster does not have. + """ + found: dict[str, list[str]] = {} + + def walk(node: object, path: list[str], owner: str | None) -> None: + if isinstance(node, dict): + for key, value in node.items(): + walk(value, [*path, str(key)], + str(key) if str(key) in (COSMETIC, "ip_addressing") else owner) + if key == "python_module" and isinstance(value, str): + found.setdefault("python_module", []).append(".".join([*path, str(key)])) + elif isinstance(node, list): + for index, value in enumerate(node): + walk(value, [*path, f"[{index}]"], owner) + elif isinstance(node, str) and node.endswith(".j2"): + found.setdefault(owner or "template", []).append(".".join(path)) + + walk(design, [], None) + return found + + +def drop_description_templates(design: dict) -> list[str]: + """Remove interface-description templates so the rest can render. + + Returns the paths removed, which the caller is expected to report -- a + migration that quietly drops input is worse than one that refuses, because + the render is pushed to a device as a full configuration replacement. + + AVD falls back to its own built-in descriptions, so what is lost is exactly + the wording. Nothing else in the document depends on it. + """ + removed: list[str] = [] + + def walk(node: object, path: list[str]) -> None: + if isinstance(node, dict): + for key in list(node): + value = node[key] + if key == COSMETIC and isinstance(value, dict): + gone = [n for n, v in value.items() if isinstance(v, str) and v.endswith(".j2")] + for name in gone: + del value[name] + removed.append(".".join([*path, key, name])) + if not value: + del node[key] + continue + walk(value, [*path, str(key)]) + elif isinstance(node, list): + for index, value in enumerate(node): + walk(value, [*path, f"[{index}]"]) + + walk(design, []) + return removed + + +def _fabric_name_of(fabric: Fabric, hostvars: dict[str, dict]) -> None: + """Fill ``spec.fabricName`` and note it when the devices disagree. + + `fn.py` writes one `fabric_name` into every device's document, so a Fabric + has exactly one. Ansible does not: `eos_designs_unit_tests` runs one + eos_designs play over 501 devices carrying **six** different `fabric_name` + values. That is not expressible, and quietly picking one would render every + device under a name AVD never gave it -- so it is named as a note. + """ + seen: dict[str, int] = {} + for device in fabric.devices: + name = hostvars.get(device, {}).get("fabric_name") + if isinstance(name, str) and name: + seen[name] = seen.get(name, 0) + 1 + if not seen: + return + fabric.fabric_name = max(seen, key=lambda n: seen[n]) + if len(seen) > 1: + others = ", ".join(f"{n} ({c})" for n, c in sorted(seen.items(), key=lambda kv: -kv[1])) + fabric.notes.append( + f"devices disagree on fabric_name and a Fabric carries one: {others}" + ) + + +def to_manifests(fabric: Fabric, namespace: str = "default") -> list[dict]: + """The Fabric and its inputs, as manifests in ``spec.requires`` order.""" + group = "avd.netclab.dev/v1alpha1" + out: list[dict] = [] + for inp in fabric.inputs: + spec: dict = {"design": inp.design} + if inp.declares: + spec["declares"] = inp.declares + applies: dict = {} + if inp.all_devices: + applies["all"] = True + if inp.node_sets: + applies["nodeSets"] = inp.node_sets + if inp.hosts: + applies["hosts"] = inp.hosts + if inp.match_hostnames: + applies["matchHostnames"] = inp.match_hostnames + if applies: + spec["appliesTo"] = applies + out.append({ + "apiVersion": group, "kind": inp.kind, + "metadata": {"name": inp.name, "namespace": namespace}, + "spec": spec, + }) + out.append({ + "apiVersion": group, "kind": "Fabric", + "metadata": {"name": fabric.name, "namespace": namespace}, + "spec": { + "fabricName": fabric.fabric_name or fabric.name, + "requires": [ + {"kind": i.kind, "name": i.name, "namespace": namespace} + for i in fabric.inputs + ], + }, + }) + return out + + +def _discover(root: Path) -> list[Path]: + """Every inventory under a directory, in either layout AVD ships.""" + if (root / "inventory.yml").is_file() or (root / "inventory" / "hosts.yml").is_file(): + return [root] + return sorted( + d for d in root.iterdir() + if d.is_dir() + and ((d / "inventory.yml").is_file() or (d / "inventory" / "hosts.yml").is_file()) + ) + + +def main() -> int: + """``avd-migrate [ROOT ...] [--emit DIR] [--namespace NS]``""" + import argparse + + import yaml + + parser = argparse.ArgumentParser( + prog="avd-migrate", + description="Translate AVD Ansible inventories into Fabric and input XRs.", + ) + parser.add_argument("roots", nargs="*", type=Path, + default=[Path("avd/ansible_collections/arista/avd/examples")]) + parser.add_argument("--emit", type=Path, help="write manifests under this directory") + parser.add_argument("--namespace", default="default") + parser.add_argument( + "--drop-description-templates", action="store_true", + help="drop interface-description templates so the rest renders. Cosmetic " + "by measurement -- on AVD's own corpus it costs `description` fields " + "and nothing else. Addressing templates are never dropped.", + ) + parser.add_argument("--collections", type=Path, + default=Path("avd"), help="ANSIBLE_COLLECTIONS_PATH") + args = parser.parse_args() + + collections = args.collections.resolve() if args.collections.is_dir() else None + failures = 0 + for top in args.roots: + for root in _discover(top): + try: + fabrics = migrate(root, collections=collections, + drop_descriptions=args.drop_description_templates) + except MigrationError as err: + print(f"[skip] {root.name:38s} {str(err).split(': ', 1)[-1]}") + continue + except Exception as err: # noqa: BLE001 - surface any ansible failure + failures += 1 + print(f"[FAIL] {root.name:38s} {type(err).__name__}: " + f"{str(err).splitlines()[0][:70]}") + continue + for fabric in fabrics: + kinds = {k: sum(1 for i in fabric.inputs if i.kind == k) for k in KINDS} + shape = " ".join(f"{k[:-3] if k.endswith('Set') else k}={v}" + for k, v in kinds.items() if v) + print(f"[ ok ] {fabric.name:38s} {len(fabric.devices):4d} devices {shape}") + for note in fabric.notes: + print(f" ! {note}") + if args.emit: + target = args.emit / f"{fabric.name}.yaml" + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(yaml.safe_dump_all( + to_manifests(fabric, args.namespace), sort_keys=False)) + return 1 if failures else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/function/verify_example.py b/function/verify_example.py deleted file mode 100644 index 686e0a4..0000000 --- a/function/verify_example.py +++ /dev/null @@ -1,106 +0,0 @@ -"""Milestone 1: prove the pyavd path reproduces AVD's golden output. - -Reads an AVD Ansible example, rebuilds ``all_inputs`` the way Ansible would, -runs the pyavd pipeline, and deep-diffs the resulting structured configs against -the example's checked-in ``intended/structured_configs/*.yml``. - -Green = the engine our Crossplane function will wrap is faithful to AVD. - -Usage: - uv run avd-verify [EXAMPLE_DIR] -""" - -from __future__ import annotations - -import sys -from pathlib import Path - -import yaml - -from .ansible_inputs import build_all_inputs -from .engine import render_structured_configs - -DEFAULT_EXAMPLE = ( - "avd/ansible_collections/arista/avd/examples/single-dc-l3ls" -) - - -def _diff(path: str, ours, gold, out: list[str]) -> None: - """Collect human-readable differences between two nested structures.""" - if type(ours) is not type(gold) and not ( - isinstance(ours, (int, float)) and isinstance(gold, (int, float)) - ): - out.append(f"{path}: type {type(ours).__name__} != {type(gold).__name__}") - return - if isinstance(gold, dict): - for k in sorted(set(ours) | set(gold)): - if k not in ours: - out.append(f"{path}.{k}: missing (only in golden)") - elif k not in gold: - out.append(f"{path}.{k}: extra (only in ours)") - else: - _diff(f"{path}.{k}", ours[k], gold[k], out) - elif isinstance(gold, list): - if len(ours) != len(gold): - out.append(f"{path}: list len {len(ours)} != {len(gold)}") - for i, (a, b) in enumerate(zip(ours, gold)): - _diff(f"{path}[{i}]", a, b, out) - elif ours != gold: - out.append(f"{path}: {ours!r} != {gold!r}") - - -def verify(example_dir: str | Path) -> int: - example_dir = Path(example_dir) - golden_dir = example_dir / "intended" / "structured_configs" - - all_inputs = build_all_inputs(example_dir) - rendered = render_structured_configs(all_inputs) - - # Guard: the set of rendered devices must match the golden set, otherwise a - # transcoder that produces zero devices would falsely report "all match". - golden_hosts = {p.stem for p in golden_dir.glob("*.yml")} - rendered_hosts = set(rendered) - if rendered_hosts != golden_hosts: - missing = sorted(golden_hosts - rendered_hosts) - extra = sorted(rendered_hosts - golden_hosts) - print( - f"DEVICE SET MISMATCH: rendered {len(rendered_hosts)} vs golden " - f"{len(golden_hosts)}" - ) - if missing: - print(f" not rendered (in golden): {missing}") - if extra: - print(f" rendered but no golden: {extra}") - print() - print("MISMATCH: device sets differ, cannot claim reproduction.") - return 1 - - total_diffs = 0 - for hostname in sorted(rendered): - golden_file = golden_dir / f"{hostname}.yml" - gold = yaml.safe_load(golden_file.read_text()) or {} - diffs: list[str] = [] - _diff(hostname, rendered[hostname], gold, diffs) - status = "OK " if not diffs else "DIFF" - print(f"[{status}] {hostname} ({len(diffs)} diffs)") - for d in diffs[:20]: - print(f" {d}") - if len(diffs) > 20: - print(f" ... and {len(diffs) - 20} more") - total_diffs += len(diffs) - - print() - if total_diffs == 0: - print(f"MATCH: all {len(rendered)} devices reproduce golden structured config.") - return 0 - print(f"MISMATCH: {total_diffs} total diffs across the fabric.") - return 1 - - -def main() -> int: - example = sys.argv[1] if len(sys.argv) > 1 else DEFAULT_EXAMPLE - return verify(example) - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/function/verify_kinds.py b/function/verify_kinds.py deleted file mode 100644 index d25b897..0000000 --- a/function/verify_kinds.py +++ /dev/null @@ -1,420 +0,0 @@ -"""Prove the input-kind model reproduces Ansible's variable resolution. - -Translates an AVD inventory into input XRs, resolves them with -:func:`function.kinds.resolve` -- which never looks at the inventory again -- -and compares the resulting hostvars against faithfully reproduced Ansible. - -Byte equality is the assertion. It is stricter than necessary (a hostvar AVD -never reads cannot change a rendered config) and that is deliberate: it fails -before a render can hide a difference. - -The translation reads the inventory; the resolution does not. Only the second -half is the model. What it establishes: - -* ``spec.requires`` order reproduces Ansible precedence. Ansible sorts groups by - (depth, name), which is a *global* total order, so one flat list restricted to - the inputs that apply to a device reproduces that device's view. -* ``appliesTo`` reproduces group membership without groups. -* One device list -- what NodeSets declare -- reproduces the inventory. - -Usage: - uv run avd-verify-kinds [EXAMPLE_DIR ...] # default: every bundled example -""" - -from __future__ import annotations - -import sys -from pathlib import Path - -import yaml - -from .ansible_inputs import ( - ALL_GROUP, - AnsibleInventory, - _load_group_vars, - _strip_ansible_keys, - _yaml_load, -) -from .kinds import KINDS, Input, classify, hosts_in_blocks, is_node_block, resolve - -EXAMPLES_ROOT = Path("avd/ansible_collections/arista/avd/examples") -MOLECULE_ROOT = Path("avd/ansible_collections/arista/avd/extensions/molecule") - -# Examples this harness cannot translate, with the reason. Expected-failure -# semantics, as in verify_xr: one that starts passing IS reported, so a deferral -# can never rot silently. -DEFERRED: dict[str, str] = {} - -# The same, for --render. Resolution and rendering fail for different reasons: -# resolution is settled, rendering still runs into AVD features this path does -# not carry yet. -DEFERRED_RENDER: dict[str, str] = { - "cv-pathfinder": "ansible-vault secrets; credentials cannot live in an XR spec", -} - - -def inline_host_vars(inventory_file: Path) -> dict[str, dict]: - """Host variables written straight into the inventory. - - ``AnsibleInventory`` iterates the keys under ``hosts:`` and drops the values, - so ``dc1-spine1: {type: spine}`` is invisible to it. Several molecule - scenarios declare device types that way and nothing else does. - """ - found: dict[str, dict] = {} - - def walk(node: object) -> None: - if not isinstance(node, dict): - return - for host, hostvars in (node.get("hosts") or {}).items(): - if isinstance(hostvars, dict): - stripped = _strip_ansible_keys(hostvars) - if stripped: - found.setdefault(host, {}).update(stripped) - for child in (node.get("children") or {}).values(): - walk(child) - - for value in (_yaml_load(inventory_file) or {}).values(): - walk(value) - return found - - -def _layout(root: Path) -> tuple[Path, Path]: - """(directory holding group_vars, inventory file) for either layout.""" - if (root / "inventory.yml").is_file(): - return root, root / "inventory.yml" - return root / "inventory", root / "inventory" / "hosts.yml" - - -def _play_targets(root: Path) -> tuple[set[str], set[str]]: - """``(what every play targets, what the eos_designs plays target)``. - - Both, because they differ and only the second is the fabric. A directory's - playbooks also carry plays that are not a render: molecule's ``create.yml`` - makes output folders, ``howto`` has a ``localhost`` play, and - ``deploy.yml`` pushes what was already built. - """ - every: set[str] = set() - designs: set[str] = set() - for playbook in sorted(root.glob("*.yml")) + sorted(root.glob("converge.yml")): - if playbook.name == "inventory.yml": - continue - document = _yaml_load(playbook) - if not isinstance(document, list): - continue - for play in document: - if not isinstance(play, dict) or "hosts" not in play: - continue - every.add(str(play["hosts"])) - # The plays name the role outright: `arista.avd.eos_designs`. - if "eos_designs" in yaml.safe_dump(play): - designs.add(str(play["hosts"])) - return every, designs - - -def play_hosts(root: Path, inventory: AnsibleInventory) -> list[str]: - """The hosts eos_designs is run on -- which is what a fabric's devices are. - - **An inventory is not a device list.** Ansible has two lists and AVD renders - the second: `cv-pathfinder`'s `build.yml` says `hosts: WAN`, while its - inventory also holds `cloudvision` -- the CloudVision API server, a host so - that `cv_deploy` can reach it, and not a switch. Declaring it composes a - Device for it, and a device with no node type fails the whole fabric's - render, not just its own. - - A Fabric *is* the play, so this is the list `declares` reproduces. Reading it - is not a heuristic: it is the statement AVD itself acts on. - - Two fallbacks, in order, each for a real case in the corpus: - - * no play runs eos_designs -- the three `eos_cli_config_gen` scenarios carry - structured config directly. Their hosts are still devices, so fall back to - what any play targets rather than declaring none of them; - * no playbook at all -- an inventory handed to the migration on its own is - still worth translating, so fall back to the whole inventory. - """ - hosts = inventory.hosts() - every, designs = _play_targets(root) - targets = designs or every - if not targets or "all" in targets: - return hosts - return [h for h in hosts if targets & (set(inventory.groups_for_host(h)) | {h})] - - -def ansible_hostvars(root: Path) -> dict[str, dict]: - """What ansible-playbook would hand to AVD, from every source it reads. - - The play's hosts, not the inventory's -- see :func:`play_hosts`. Both this - and the migration restrict to the same set, so the comparison stays an - equality over what AVD is actually given. - """ - var_dir, inventory_file = _layout(root) - inventory = AnsibleInventory.from_file(inventory_file) - inline = inline_host_vars(inventory_file) - host_var_dir = var_dir / "host_vars" - files = ( - {f.stem: _strip_ansible_keys(_yaml_load(f)) for f in host_var_dir.glob("*.yml")} - if host_var_dir.is_dir() - else {} - ) - cache: dict[str, dict] = {} - out: dict[str, dict] = {} - for host in sorted(play_hosts(root, inventory)): - hostvars: dict = {} - for group in inventory.groups_for_host(host): - if group not in cache: - cache[group] = _load_group_vars(var_dir / "group_vars", group) - hostvars.update(cache[group]) - hostvars = _strip_ansible_keys(hostvars) - hostvars.update(inline.get(host, {})) - hostvars.update(files.get(host, {})) - out[host] = hostvars - return out - - -_SUFFIX = { - "NetworkServiceSet": "services", - "ConnectedEndpointSet": "endpoints", - "SettingSet": "settings", -} - - -def _by_category(base: str, design: dict) -> list[tuple[str, str, dict]]: - """Partition one group's vars into ``(name, kind, design)`` per category. - - **Merge first, split after** -- the order matters and it is Ansible's. - A ``group_vars`` *directory* is merged into one namespace for the group, so - two files setting the same top-level key resolve alphabetically-last-wins - and never coexist. Splitting by file instead would preserve a boundary - Ansible does not keep, and emit two inputs claiming one key. - - Splitting by category afterwards costs nothing and is what the kinds are - for: `cv-pathfinder`'s `group_vars/WAN/` holds four files the author already - separated -- settings, interface profiles, management, tenants -- which - merged into one fragment whose single `tenants` key decided the kind for the - other 21. Ownership only; `resolve` never reads a kind, and the parts are - disjoint and consecutive, so the hostvars are unchanged either way. - """ - buckets: dict[str, dict] = {} - for key, value in design.items(): - buckets.setdefault(classify({key: value}), {})[key] = value - parts = [(kind, buckets[kind]) for kind in KINDS if kind in buckets] - return [ - (base if len(parts) == 1 or kind == "NodeSet" else f"{base}-{_SUFFIX[kind]}", kind, payload) - for kind, payload in parts - ] - - -def inputs_from_inventory(root: Path) -> list[Input]: - """Translate an AVD inventory into ordered input XRs -- a migration. - - Each group's vars become one input per content category: a ``NodeSet`` for - its node-type blocks, and one each for services, endpoints and settings. - The node-type split is not cosmetic -- the two halves have different scopes - whenever a block names fewer devices than the group holds, which is what a - 5-stage CLOS does. The rest is ownership: see :func:`_by_category`. - """ - var_dir, inventory_file = _layout(root) - inventory = AnsibleInventory.from_file(inventory_file) - group_var_dir = var_dir / "group_vars" - # The play's hosts, not the inventory's -- see play_hosts. - every_device = play_hosts(root, inventory) - - groups = [] - if group_var_dir.is_dir(): - named = {p.stem for p in group_var_dir.glob("*.yml")} | { - d.name for d in group_var_dir.iterdir() if d.is_dir() - } - groups = [g for g in named if g in inventory.depth or g == ALL_GROUP] - # Ansible precedence: `all` first, then (depth, name). This is requires order. - ordered = sorted(groups, key=lambda g: (inventory.depth.get(g, 0), g)) - - def group_devices(group: str) -> set[str]: - if group == ALL_GROUP: - return set(every_device) - return {h for h in every_device if group in inventory.groups_for_host(h)} - - designs = {g: _strip_ansible_keys(_load_group_vars(group_var_dir, g)) for g in ordered} - declared_by: dict[str, set[str]] = {} - for group, design in designs.items(): - blocks = {k: v for k, v in design.items() if is_node_block(v)} - if blocks: - declared_by[group] = hosts_in_blocks(blocks) & set(every_device) - - # Devices no block mentions -- fixtures with no node-type blocks at all, and - # hosts that only the inventory knows about. Declared at the narrowest group - # holding each one rather than in a single fabric-wide NodeSet, so the - # resulting NodeSets line up with real groups and other inputs can name them. - device_sets: dict[str, set[str]] = {} - for host in sorted(set(every_device) - set().union(*declared_by.values() or [set()])): - deepest = inventory.groups_for_host(host)[-1] # already (depth, name) sorted - device_sets.setdefault(f"{deepest}-devices", set()).add(host) - declared_by.update(device_sets) - - inputs = [ - Input(name, "NodeSet", {}, declares=sorted(hosts)) - for name, hosts in sorted(device_sets.items()) - ] - - for group in ordered: - design = designs[group] - if not design: - continue - want = group_devices(group) - - def scoped(name: str, kind: str, payload: dict, want: set[str] = want) -> Input: - inp = Input(name=name, kind=kind, design=payload) - if want == set(every_device): - inp.all_devices = True - else: - cover = [n for n, hs in declared_by.items() if hs and hs <= want] - covered: set[str] = set() - for n in cover: - covered |= declared_by[n] - if covered == want: - inp.node_sets = sorted(cover) - else: - # No union of NodeSets is this group -- name the devices. - inp.hosts = sorted(want) - return inp - - for name, kind, payload in _by_category(group, design): - inp = scoped(name, kind, payload) - if kind == "NodeSet": - inp.declares = sorted(declared_by[group]) - # A group with no host in the play, whose blocks declare nothing - # either, reaches nobody -- and "reaches nobody" is not expressible - # in appliesTo, where saying nothing means every device. Emitting it - # would invert its meaning. Only reachable since the device list - # became the play's rather than the inventory's. - if not want and not (kind == "NodeSet" and inp.declares): - continue - if kind == "NodeSet": - if want == set(inp.declares): - # The default -- a NodeSet is seen by what it declares. - # Saying it again would make every NodeSet name itself. - inp.all_devices = False - inp.node_sets = [] - inp.hosts = [] - inputs.append(inp) - - # host_vars last, as Ansible does: inventory inline first, then files. - for host, design in sorted(inline_host_vars(inventory_file).items()): - for name, kind, payload in _by_category(f"{host}-inline", design): - inputs.append(Input(name, kind, payload, hosts=[host])) - host_var_dir = var_dir / "host_vars" - if host_var_dir.is_dir(): - for f in sorted(host_var_dir.glob("*.yml")): - design = _strip_ansible_keys(_yaml_load(f)) - for name, kind, payload in _by_category(f.stem, design): - inputs.append(Input(name, kind, payload, hosts=[f.stem])) - return inputs - - -def verify_one(root: Path) -> tuple[str, int]: - """Return (status, difference count). status in {ok, differs, error}.""" - try: - from_kinds = resolve(inputs_from_inventory(root)) - from_ansible = ansible_hostvars(root) - except Exception as err: # noqa: BLE001 - surface any translation failure - return f"error: {type(err).__name__}: {str(err)[:70]}", -1 - - notes: list[str] = [] - for host in sorted(set(from_kinds) | set(from_ansible)): - if host not in from_kinds: - notes.append(f"{host}: missing") - continue - if host not in from_ansible: - notes.append(f"{host}: extra") - continue - a, b = from_kinds[host], from_ansible[host] - notes += [f"{host}.{k}" for k in sorted(set(a) | set(b)) if a.get(k) != b.get(k)] - if notes: - return f"differs ({len(notes)}): {', '.join(notes[:3])}", len(notes) - return "ok", 0 - - -def render_one(root: Path) -> tuple[str, int]: - """Render through the kinds path and diff against the checked-in golden. - - This does **not** test the model -- if the hostvars match Ansible, and - :func:`verify_one` asserts they do, the render must match too. What it tests - is the pair (this path, this pyavd): an AVD upgrade that changes output slips - past resolution equivalence and fails here. That is the job `test_xr_fold` - does today through the fold, and this is its successor -- the fold reaches - 6 of 8 examples, this reaches 7. - """ - import yaml - - from .engine import render_structured_configs - from .verify_example import _diff - - golden = root / "intended" / "structured_configs" - if not golden.is_dir(): - return "no golden", -1 - try: - rendered = render_structured_configs(resolve(inputs_from_inventory(root))) - except Exception as err: # noqa: BLE001 - surface any AVD/render failure - return f"error: {type(err).__name__}: {str(err)[:70]}", -1 - - total = 0 - for hostname in sorted(rendered): - golden_file = golden / f"{hostname}.yml" - if not golden_file.is_file(): - continue # a scenario may render hosts it keeps no golden for - out: list[str] = [] - _diff(hostname, rendered[hostname], yaml.safe_load(golden_file.read_text()) or {}, out) - total += len(out) - return ("ok" if total == 0 else f"diff ({total})"), total - - -def _discover(root: Path) -> list[Path]: - return sorted(d for d in root.iterdir() if (d / "inventory.yml").is_file()) - - -def _discover_molecule(root: Path = MOLECULE_ROOT) -> list[Path]: - """Molecule scenarios carrying an inventory of their own. - - The wider corpus, and the harder one: these reach 501 devices and lean on - inventory-inline host vars, which the examples barely use. - """ - if not root.is_dir(): - return [] - return sorted( - d - for d in root.iterdir() - if (d / "inventory" / "hosts.yml").is_file() and (d / "inventory" / "group_vars").is_dir() - ) - - -def main() -> int: - args = [a for a in sys.argv[1:] if a != "--render"] - rendering = "--render" in sys.argv[1:] - roots = [Path(a) for a in args] or _discover(EXAMPLES_ROOT) - check = render_one if rendering else verify_one - deferrals = DEFERRED_RENDER if rendering else DEFERRED - - failures = deferred = 0 - for root in roots: - status, _ = check(root) - ok = status == "ok" - reason = deferrals.get(root.name) - if reason and not ok: - mark, deferred = "DEFER", deferred + 1 - status = f"deferred: {reason}" - elif reason and ok: - mark, failures = "XPASS", failures + 1 - status = "passes now -- remove from the DEFERRED map" - elif ok: - mark = "OK " - else: - mark, failures = "FAIL", failures + 1 - print(f"[{mark}] {root.name:26s} {status}") - - expected = len(roots) - deferred - what = "reproduce golden" if rendering else "resolve identically to Ansible" - print(f"\n{expected - failures}/{expected} inventories {what} ({deferred} deferred).") - return 1 if failures else 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/function/verify_xr.py b/function/verify_xr.py deleted file mode 100644 index bbfdeb4..0000000 --- a/function/verify_xr.py +++ /dev/null @@ -1,103 +0,0 @@ -"""Milestone 2 harness: prove the Fabric-XR path reproduces AVD's golden output. - -For each AVD example, fold it into a single fabric document (`spec.design`), -render it the way the composite function will (`render_fabric_design`), and -deep-diff against the checked-in `intended/structured_configs`. - -This is the XR-level analogue of `avd-verify`, and doubles as the regression net -for the Ansible->XR fold. - -Usage: - uv run avd-verify-xr [EXAMPLE_DIR ...] # default: every bundled example -""" - -from __future__ import annotations - -import sys -from pathlib import Path - -import yaml - -from .ansible_inputs import build_all_inputs -from .engine import render_fabric_design -from .verify_example import _diff -from .xr import fabric_design_from_inputs - -EXAMPLES_ROOT = Path("avd/ansible_collections/arista/avd/examples") - -# Examples that are known not to fold into a single fabric document, with the -# reason. Expected-failure semantics: a deferred example that fails is not a -# regression, but one that starts passing IS reported (the deferral is stale and -# should be removed), so this list can never silently hide a fixed example. -DEFERRED = { - "campus-fabric": "aaa_settings.radius differs by role; no node-scoped equivalent", - "cv-pathfinder": "SD-WAN: WAN gateway across 2 routers + ansible-vault secrets", -} - - -def _discover(root: Path) -> list[Path]: - return sorted( - d - for d in root.iterdir() - if (d / "inventory.yml").is_file() - and (d / "intended" / "structured_configs").is_dir() - ) - - -def verify_one(example_dir: Path) -> tuple[str, int]: - """Return (status, diff_count). status in {ok, diff, conflict, error}.""" - per_host = build_all_inputs(example_dir) - fabric_name, design, conflicts = fabric_design_from_inputs(per_host) - try: - rendered = render_fabric_design(design, fabric_name) - except Exception as exc: # noqa: BLE001 - surface any AVD/render failure - detail = f"conflicts={sorted(conflicts)} " if conflicts else "" - return f"error: {detail}{type(exc).__name__}: {str(exc)[:80]}", -1 - - golden_dir = example_dir / "intended" / "structured_configs" - total = 0 - for hostname in sorted(rendered): - gold = yaml.safe_load((golden_dir / f"{hostname}.yml").read_text()) or {} - out: list[str] = [] - _diff(hostname, rendered[hostname], gold, out) - total += len(out) - if total: - return f"diff ({total})", total - return "ok", total - - -def main() -> int: - args = [Path(a) for a in sys.argv[1:]] - examples = args or _discover(EXAMPLES_ROOT) - failures = 0 - deferred = 0 - for example_dir in examples: - status, diffs = verify_one(example_dir) - n = len(list((example_dir / "intended" / "structured_configs").glob("*.yml"))) - ok = status.startswith("ok") - reason = DEFERRED.get(example_dir.name) - if reason and not ok: - mark, deferred = "DEFER", deferred + 1 - status = f"deferred: {reason}" - elif reason and ok: - # Stale deferral: it folds now, so drop it from DEFERRED. - mark = "XPASS" - failures += 1 - status = "folds now -- remove from DEFERRED" - elif ok: - mark = "OK " - else: - mark = "FAIL" - failures += 1 - print(f"[{mark}] {example_dir.name:26s} devices={n:2d} {status}") - print() - expected = len(examples) - deferred - print( - f"{expected - failures}/{expected} expected examples reproduce golden via the " - f"Fabric-XR fold ({deferred} deferred)." - ) - return 1 if failures else 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/function/xr.py b/function/xr.py deleted file mode 100644 index be58fe1..0000000 --- a/function/xr.py +++ /dev/null @@ -1,165 +0,0 @@ -"""Build a ``Fabric`` XR (custom resource) from an AVD Ansible example. - -Bridges Milestone 1 (Ansible example -> proven pyavd output) to Milestone 2 -(Crossplane XR -> pyavd output): it folds the per-host Ansible inputs into a -single fabric-wide AVD design document -- the shape carried by -``Fabric.spec.design`` -- with device roles expressed via ``default_node_types`` -instead of Ansible's per-group ``type``. - -The resulting XR is a self-contained fixture for ``crossplane render`` and for -regression-testing the composite function. -""" - -from __future__ import annotations - -import copy -import re -from pathlib import Path - -import yaml - -from .ansible_inputs import build_all_inputs - -API_VERSION = "avd.netclab.dev/v1alpha1" -KIND = "Fabric" - - -def _is_node_type_block(value: object) -> bool: - return isinstance(value, dict) and ("nodes" in value or "node_groups" in value) - - -def _canon(value: object) -> str: - return yaml.safe_dump(value, sort_keys=True, default_flow_style=True) - - -def _conflicting_block_defaults(per_host: dict[str, dict]) -> set[tuple[str, str]]: - """Return ``{(block, default_key)}`` whose value differs across DCs. - - These are per-DC/per-pod settings (pools, ASNs, uplinks) kept in a group's - ``defaults``; they cannot share one block-level ``defaults`` and are instead - pushed down to the node_groups/nodes of their originating DC (which override - defaults in AVD, so effective values are unchanged). - """ - seen: dict[tuple[str, str], set[str]] = {} - for hostvars in per_host.values(): - for key, value in hostvars.items(): - if _is_node_type_block(value): - for dk, dv in (value.get("defaults") or {}).items(): - seen.setdefault((key, dk), set()).add(_canon(dv)) - return {kd for kd, values in seen.items() if len(values) > 1} - - -def _append_unique(dst_list: list, items, id_key: str, pushdown: dict) -> None: - """Append node/node_group dicts not already present, baking pushdown defaults.""" - seen = {e[id_key] for e in dst_list if isinstance(e, dict) and id_key in e} - for item in items or []: - if isinstance(item, dict) and item.get(id_key) not in seen: - item = copy.deepcopy(item) # group_vars dicts are shared across hosts - for pk, pv in pushdown.items(): - item.setdefault(pk, pv) - dst_list.append(item) - seen.add(item.get(id_key)) - - -def fabric_design_from_inputs( - per_host: dict[str, dict], -) -> tuple[str, dict, set[str]]: - """Fold ``{hostname: hostvars}`` into ``(fabric_name, design, conflicts)``. - - Per-host hostvars differ by ``type`` (re-expressed as ``default_node_types``) - and by per-DC node-type blocks. Node-type blocks are unioned; per-DC - ``defaults`` that disagree are pushed down to that DC's node_groups/nodes so a - single document stays lossless. Remaining ``conflicts`` are fabric-level keys - (e.g. ``aaa_settings``) that are not node-scoped and need an explicit call. - """ - fabric_name = "FABRIC" - design: dict = {} - roles: dict[str, list[str]] = {} - conflicts: set[str] = set() - conflict_defaults = _conflicting_block_defaults(per_host) - - for hostname, hostvars in per_host.items(): - for key, value in hostvars.items(): - if key == "type": - roles.setdefault(value, []).append(hostname) - elif key == "fabric_name": - fabric_name = value - elif _is_node_type_block(value): - block = design.setdefault(key, {}) - shared_defaults = block.setdefault("defaults", {}) - pushdown: dict = {} - for dk, dv in (value.get("defaults") or {}).items(): - if (key, dk) in conflict_defaults: - pushdown[dk] = dv # per-DC -> node_group/node level - else: - shared_defaults[dk] = dv # uniform -> stays shared - _append_unique(block.setdefault("nodes", []), value.get("nodes"), "name", pushdown) - _append_unique( - block.setdefault("node_groups", []), value.get("node_groups"), "group", pushdown - ) - for bk, bv in value.items(): - if bk not in ("defaults", "nodes", "node_groups"): - block[bk] = bv - else: - if key in design and _canon(design[key]) != _canon(value): - conflicts.add(key) - design[key] = value - - # Drop empty scaffolding left by setdefault. - for block in design.values(): - if isinstance(block, dict): - for empty_key in ("defaults", "nodes", "node_groups"): - if empty_key in block and not block[empty_key]: - del block[empty_key] - - # Roles come from Ansible's per-group `type`. If the example instead assigns - # roles fabric-wide via its own `default_node_types` (e.g. multipod), that - # value already flowed into `design` -- keep it rather than clobbering it. - if roles: - design["default_node_types"] = [ - { - "node_type": node_type, - "match_hostnames": [f"^{re.escape(h)}$" for h in sorted(hosts)], - } - for node_type, hosts in sorted(roles.items()) - ] - return fabric_name, design, conflicts - - -class FabricFoldConflict(Exception): - """Raised when per-host Ansible data cannot collapse into one fabric document. - - Carries the conflicting keys -- typically per-DC/per-pod settings kept in - group ``defaults`` that must be relocated to ``node_groups`` level to be - representable in a single ``spec.design``. - """ - - -def fabric_xr_from_example( - example_dir: str | Path, - *, - name: str | None = None, - namespace: str = "default", - strict: bool = True, -) -> dict: - """Return a ``Fabric`` XR dict for an AVD Ansible example directory. - - With ``strict`` (default), raises :class:`FabricFoldConflict` if the example's - Ansible group-vars cannot be folded losslessly into a single fabric document. - """ - per_host = build_all_inputs(example_dir) - fabric_name, design, conflicts = fabric_design_from_inputs(per_host) - if conflicts and strict: - raise FabricFoldConflict(sorted(conflicts)) - return { - "apiVersion": API_VERSION, - "kind": KIND, - "metadata": { - "name": name or fabric_name.lower().replace("_", "-"), - "namespace": namespace, - }, - "spec": { - "fabricName": fabric_name, - "design": design, - }, - } diff --git a/pyproject.toml b/pyproject.toml index 4854142..b0cc558 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,20 +14,27 @@ dependencies = [ ] [project.scripts] -avd-verify = "function.verify_example:main" -avd-verify-xr = "function.verify_xr:main" -avd-verify-kinds = "function.verify_kinds:main" +avd-migrate = "function.migrate:main" avd-function = "function.main:main" avd-topology = "function.netclab_topology:main" [dependency-groups] -dev = ["pytest>=8.0"] +# `ansible-core` is the migration harness's reference implementation of Ansible: +# `function/ansible_cli.py` asks the CLI what an inventory resolves to instead of +# reimplementing it. The runtime never imports it -- `fn.py` takes XRs and there +# is no inventory in a cluster -- so it reaches neither the image nor the +# published package. Keeping Ansible out of the *runtime* is the rule; keeping it +# out of the *harness* only bought a second, wrong implementation of Ansible. +dev = ["pytest>=8.0", "ansible-core>=2.18"] [tool.pytest.ini_options] testpaths = ["tests"] # e2e needs a live cluster, so it is opt-in: `uv run pytest -m e2e`. -addopts = "-m 'not e2e'" -markers = ["e2e: requires a cluster from scripts/kind-up.sh with a fabric applied"] +addopts = "-m 'not e2e and not corpus'" +markers = [ + "e2e: requires a cluster from scripts/kind-up.sh with a fabric applied", + "corpus: the whole molecule corpus -- 501 devices in one play, 71 plays in one scenario", +] [build-system] requires = ["uv_build>=0.11.7,<0.12.0"] diff --git a/tests/test_categories.py b/tests/test_categories.py new file mode 100644 index 0000000..704804a --- /dev/null +++ b/tests/test_categories.py @@ -0,0 +1,126 @@ +"""The kind a key belongs to is AVD's statement, not ours. + +`function.kinds` classifies a top-level eos_designs key two ways, and this +guards both against upstream moving under us: + +* the **generated** part -- which key names exist at all -- comes from pyavd's + public schema at runtime, so a family gaining a member (upstream added + `cameras` to `connected_endpoints_keys`) needs no edit here. Nothing to guard; + it cannot drift. +* the **named** part -- the handful of keys that are not settings -- is a + literal, because AVD publishes that categorisation only as documentation + metadata. This test reads `documentation_options.table` out of AVD's own + schema and requires the two to agree in both directions. + +Offline, and reads the `avd` submodule the same way the golden tests do. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +import yaml + +from function.kinds import Vocabulary, kind_of + +SCHEMA = Path("avd/python-avd/pyavd/_eos_designs/schema/eos_designs.schema.yml") + +#: AVD's documentation table -> the kind that owns it. The one editorial step, +#: and it is made on AVD's names rather than on any key's content. +TABLE_KIND = { + "node-type-structure": "NodeSet", + "type-setting": "NodeSet", + "node-type-keys": "NodeSet", + "node-type-l3-interfaces-configuration": "NodeSet", + "network-services": "NetworkServiceSet", + "network-services-l2vlans-settings": "NetworkServiceSet", + "network-services-vrfs-settings": "NetworkServiceSet", + "evpn-vlan-bundles": "NetworkServiceSet", + "connected-endpoints": "ConnectedEndpointSet", + "connected-endpoints-keys": "ConnectedEndpointSet", + "default-connected-endpoints-description": "ConnectedEndpointSet", + "default-network-ports-description": "ConnectedEndpointSet", +} + +#: Keys AVD tags with no table at all, placed here by this repo. Listed so the +#: test states them rather than silently tolerating them. +UNTAGGED = { + "port_profiles": "ConnectedEndpointSet", + "network_ports": "ConnectedEndpointSet", + "network_services_keys": "NetworkServiceSet", +} + + +def _schema() -> dict: + if not SCHEMA.is_file(): + pytest.skip(f"{SCHEMA} missing -- run `git submodule update --init`") + return yaml.safe_load(SCHEMA.read_text()) + + +def _tables() -> dict[str, str]: + """Top-level key -> AVD's documentation table.""" + return { + key: (body.get("documentation_options") or {}).get("table") or "" + for key, body in (_schema().get("keys") or {}).items() + } + + +def test_every_key_avd_categorises_lands_in_that_kind() -> None: + """AVD tags a key; we must agree. This is the direction that catches a + *new* upstream key we have never seen.""" + vocabulary = Vocabulary.default() + wrong = { + key: (kind_of(key, None, vocabulary), TABLE_KIND[table]) + for key, table in _tables().items() + if table in TABLE_KIND + } + wrong = {k: v for k, v in wrong.items() if v[0] != v[1]} + assert not wrong, f"kind_of disagrees with AVD's own table: {wrong}" + + +def test_no_key_is_promoted_out_of_settings_without_avd_saying_so() -> None: + """The other direction: nothing is quietly special-cased. A key we do not + call a setting must be one AVD categorises, a dynamic key name, or listed in + UNTAGGED with a reason.""" + vocabulary = Vocabulary.default() + dynamic = vocabulary.node_types | vocabulary.network_services | vocabulary.connected_endpoints + tables = _tables() + unexplained = { + key: kind_of(key, None, vocabulary) + for key in tables + if kind_of(key, None, vocabulary) != "SettingSet" + and key not in dynamic + and tables[key] not in TABLE_KIND + and key not in UNTAGGED + } + assert not unexplained, f"promoted out of SettingSet with nothing backing it: {unexplained}" + + +def test_untagged_placements_are_still_untagged_upstream() -> None: + """If AVD starts tagging one of these, the entry moves to TABLE_KIND and + stops being this repo's opinion.""" + tables = _tables() + now_tagged = {k: tables[k] for k in UNTAGGED if tables.get(k)} + assert not now_tagged, f"AVD now categorises these; move them to TABLE_KIND: {now_tagged}" + + +def test_the_dynamic_families_come_from_pyavd_not_from_a_literal() -> None: + """The defaults are read, so they cannot be short. `cameras` is the case + that proves it: it exists upstream and the literal this replaced lacked it.""" + vocabulary = Vocabulary.default() + schema_defaults = { + source: { + str(entry[field]) + for entry in ((_schema()["keys"].get(source) or {}).get("default") or []) + if isinstance(entry, dict) and entry.get(field) + } + for source, field in ( + ("node_type_keys", "key"), + ("network_services_keys", "name"), + ("connected_endpoints_keys", "key"), + ) + } + assert schema_defaults["node_type_keys"] <= vocabulary.node_types + assert schema_defaults["network_services_keys"] <= vocabulary.network_services + assert schema_defaults["connected_endpoints_keys"] <= vocabulary.connected_endpoints diff --git a/tests/test_engine_fidelity.py b/tests/test_engine_fidelity.py deleted file mode 100644 index f6804ea..0000000 --- a/tests/test_engine_fidelity.py +++ /dev/null @@ -1,48 +0,0 @@ -"""Milestone 1: the pyavd engine reproduces AVD's golden structured config. - -Offline -- no cluster. This is `avd-verify` as assertions instead of a report. -""" - -from __future__ import annotations - -from pathlib import Path - -import pytest -import yaml - -from function.ansible_inputs import build_all_inputs -from function.engine import render_structured_configs -from function.verify_example import _diff - -EXAMPLES_ROOT = Path("avd/ansible_collections/arista/avd/examples") - -# Every bundled example that reproduces golden from full Ansible inputs. -# cv-pathfinder is deferred (ansible-vault secrets); `common` holds shared vars -# and is not a runnable fabric. -EXAMPLES = [ - "single-dc-l3ls", - "single-dc-l3ls-ipv6", - "single-dc-multipod-l3ls", - "dual-dc-l3ls", - "campus-fabric", - "l2ls-fabric", - "isis-ldp-ipvpn", -] - - -@pytest.mark.parametrize("example", EXAMPLES) -def test_reproduces_golden_structured_config(example: str) -> None: - example_dir = EXAMPLES_ROOT / example - golden_dir = example_dir / "intended" / "structured_configs" - - rendered = render_structured_configs(build_all_inputs(example_dir)) - - # The device sets must match first: "zero diffs" over an empty render would - # otherwise pass vacuously. - assert set(rendered) == {p.stem for p in golden_dir.glob("*.yml")} - - diffs: list[str] = [] - for hostname in sorted(rendered): - gold = yaml.safe_load((golden_dir / f"{hostname}.yml").read_text()) or {} - _diff(hostname, rendered[hostname], gold, diffs) - assert diffs == [], "\n".join(diffs[:20]) diff --git a/tests/test_kinds_equivalence.py b/tests/test_kinds_equivalence.py index f7c1fbf..9c78061 100644 --- a/tests/test_kinds_equivalence.py +++ b/tests/test_kinds_equivalence.py @@ -1,12 +1,20 @@ """The input-kind model resolves exactly as Ansible does. -Offline -- no cluster, no pyavd render. Guards :func:`function.kinds.resolve` -and the migration that feeds it, over AVD's own corpus: the 8 bundled examples -and every molecule scenario with an inventory of its own, up to 501 devices. - -This is the regression net for the collect path. It is stricter than a render -comparison on purpose: it fails on a hostvar difference even where AVD would -have rendered the same config, so a divergence cannot hide until it matters. +The reference side is **real Ansible** -- `ansible-inventory --list`, run by +:mod:`function.ansible_cli`. That matters more than it sounds. The previous +version of this test compared two of our own readers against each other, so a +source neither read vanished from both sides and the comparison agreed. Four +such gaps sat behind a green result and surfaced only when a render disagreed +with AVD's own golden: `host_vars/*.yaml`, `host_vars//` directories, +inline group `vars:` blocks, and ansible-vault. + +Byte equality is the assertion. It is stricter than necessary -- a hostvar AVD +never reads cannot change a rendered config -- and that is deliberate: it fails +before a render can hide a difference. + +The 8 bundled examples run by default. The molecule corpus is behind +``-m corpus``: it reaches 501 devices in one play and 71 plays in one scenario, +and every one of them costs an ansible subprocess. """ from __future__ import annotations @@ -15,37 +23,154 @@ import pytest -from function.kinds import Input, classify, resolve -from function.verify_kinds import ( - DEFERRED, - DEFERRED_RENDER, - EXAMPLES_ROOT, - _by_category, +from function.ansible_cli import read_inventory +from function.kinds import Input, Vocabulary, by_kind, classify, resolve +from function.migrate import ( + Fabric, + MigrationError, _discover, - _discover_molecule, - inputs_from_inventory, - render_one, - verify_one, + drop_description_templates, + migrate, + to_manifests, + unsupported, ) -CORPUS = _discover(EXAMPLES_ROOT) + _discover_molecule() -EXAMPLES = _discover(EXAMPLES_ROOT) +AVD = Path("avd/ansible_collections/arista/avd") +EXAMPLES_ROOT = AVD / "examples" +MOLECULE_ROOT = AVD / "extensions" / "molecule" +COLLECTIONS = Path("avd").resolve() + +EXAMPLES = _discover(EXAMPLES_ROOT) if EXAMPLES_ROOT.is_dir() else [] +MOLECULE = _discover(MOLECULE_ROOT) if MOLECULE_ROOT.is_dir() else [] + +# Scenarios with no eos_designs play at all. Not deferrals -- correct answers: +# the three `eos_cli_config_gen` scenarios carry structured config directly and +# render at the device layer, and cv_deploy/cv_workflow push what was built. +NO_DESIGN_PLAY = { + "cv_deploy", "cv_workflow", "eos_cli_config_gen", + "eos_cli_config_gen_deprecated_vars", "eos_cli_config_gen_negative_unit_tests", +} + +# Renders that still meet an AVD feature this path does not carry. Expected- +# failure semantics: one that starts passing is reported, so a deferral cannot +# rot silently. **All eight examples render clean**, so this covers the molecule +# corpus only -- and every entry is one of three causes, not eight. +DEFERRED_RENDER: dict[str, str] = {} + +DEFERRED_RENDER_CORPUS = { + # pyavd implements no Jinja templating: `get_device_structured_config` passes + # `templar=None` and the call raises `NotImplementedError`. Neither public + # entry point accepts a templar, so carrying the .j2 files on an XR would + # change nothing -- nothing would read them. Upstream, not ours. + "ansible_only": "custom ip_addressing template -- pyavd implements no Jinja templating", + "evpn_underlay_ebgp_overlay_ebgp": "custom interface_descriptions templates -- " + "pyavd implements no Jinja templating", + # Persistent mutable state: an ID pool must survive reconciliation or devices + # get new identifiers every pass. `pyavd.api.pool_manager` is public; what is + # missing is somewhere for a Fabric to keep the pool. + "eos_designs-twodc-5stage-clos": "fabric_numbering pool_manager -- makes a Fabric stateful", + # Code, not data: a function image is immutable and must not load arbitrary + # Python. ⚠ And this scenario is not a fabric -- 59 unrelated feature groups + # under one play, with six different `fabric_name` values. + "eos_designs_unit_tests": "templates.*.python_module -- code, not data", +} + + +def _differences(path: str, ours: object, golden: object, out: list[str]) -> None: + """Collect readable differences between a rendered config and its golden.""" + if type(ours) is not type(golden) and not ( + isinstance(ours, (int, float)) and isinstance(golden, (int, float)) + ): + out.append(f"{path}: type {type(ours).__name__} != {type(golden).__name__}") + elif isinstance(golden, dict): + assert isinstance(ours, dict) + for key in sorted(set(ours) | set(golden)): + if key not in ours: + out.append(f"{path}.{key}: only in golden") + elif key not in golden: + out.append(f"{path}.{key}: only in ours") + else: + _differences(f"{path}.{key}", ours[key], golden[key], out) + elif isinstance(golden, list): + assert isinstance(ours, list) + if len(ours) != len(golden): + out.append(f"{path}: list len {len(ours)} != {len(golden)}") + for index, (a, b) in enumerate(zip(ours, golden)): + _differences(f"{path}[{index}]", a, b, out) + elif ours != golden: + out.append(f"{path}: {ours!r} != {golden!r}") + + +def _fabrics(root: Path) -> tuple[list[Fabric], dict[str, dict]]: + inv = read_inventory(root, collections=COLLECTIONS) + return migrate(root, collections=COLLECTIONS, inv=inv), inv.hostvars + + +def _assert_equivalent(root: Path) -> None: + if root.name in NO_DESIGN_PLAY: + with pytest.raises(MigrationError): + migrate(root, collections=COLLECTIONS) + return + + fabrics, hostvars = _fabrics(root) + assert fabrics, f"{root.name}: no fabric produced" + for fabric in fabrics: + got = resolve(fabric.inputs) + want = {device: hostvars.get(device, {}) for device in fabric.devices} + differing = sorted( + f"{host}.{key}" + for host in set(got) | set(want) + for key in set(got.get(host, {})) | set(want.get(host, {})) + if got.get(host, {}).get(key) != want.get(host, {}).get(key) + ) + assert set(got) == set(want), ( + f"{fabric.name}: device sets differ " + f"(missing {sorted(set(want) - set(got))[:5]}, " + f"extra {sorted(set(got) - set(want))[:5]})" + ) + assert not differing, f"{fabric.name}: {len(differing)} differ: {differing[:5]}" -@pytest.mark.parametrize("root", CORPUS, ids=lambda p: p.name) +@pytest.mark.parametrize("root", EXAMPLES, ids=lambda p: p.name) def test_resolves_identically_to_ansible(root: Path) -> None: - status, _ = verify_one(root) - - if root.name in DEFERRED: - # Expected failure. Asserting it still fails keeps a deferral from - # rotting: if it starts resolving, this fails and says to drop it. - assert status != "ok", ( - f"{root.name} resolves now -- remove it from verify_kinds.DEFERRED " - f"(was deferred: {DEFERRED[root.name]})" + _assert_equivalent(root) + + +@pytest.mark.corpus +@pytest.mark.parametrize("root", MOLECULE, ids=lambda p: p.name) +def test_resolves_identically_to_ansible_over_the_molecule_corpus(root: Path) -> None: + _assert_equivalent(root) + + +def _assert_renders(root: Path, deferrals: dict[str, str]) -> None: + import yaml + + from function.engine import render_structured_configs + + golden = root / "intended" / "structured_configs" + if not golden.is_dir(): + pytest.skip(f"{root.name} ships no golden") + + fabrics, _ = _fabrics(root) + total: list[str] = [] + try: + for fabric in fabrics: + rendered = render_structured_configs(resolve(fabric.inputs)) + for hostname, structured in sorted(rendered.items()): + target = golden / f"{hostname}.yml" + if target.is_file(): + _differences(hostname, structured, + yaml.safe_load(target.read_text()) or {}, total) + except Exception as err: # noqa: BLE001 - a render failure is a difference too + total.append(f"{type(err).__name__}: {err}") + + if root.name in deferrals: + assert total, ( + f"{root.name} renders clean now -- drop its deferral " + f"(was: {deferrals[root.name]})" ) return - - assert status == "ok", f"{root.name}: {status}" + assert not total, f"{root.name}: {len(total)} differences: {total[:5]}" @pytest.mark.parametrize("root", EXAMPLES, ids=lambda p: p.name) @@ -53,29 +178,103 @@ def test_render_reproduces_golden(root: Path) -> None: """Rendered configs still match the checked-in golden. Redundant as a check on the model -- matching hostvars render identically -- - and that is not what it is for. It is the guard against pyavd itself - changing: an AVD upgrade slips past resolution equivalence and fails here. - - Examples only. The molecule scenarios need AVD features this path does not - carry yet (templates loaded from files, ID pools, custom Python classes), so - they stay on the equivalence test until those land. + and that is not what it is for. It guards pyavd itself changing: an AVD + upgrade slips past resolution equivalence and fails here. """ - status, _ = render_one(root) + _assert_renders(root, DEFERRED_RENDER) - if root.name in DEFERRED_RENDER: - assert status != "ok", ( - f"{root.name} renders clean now -- remove it from " - f"verify_kinds.DEFERRED_RENDER (was: {DEFERRED_RENDER[root.name]})" - ) - return - assert status == "ok", f"{root.name}: {status}" +@pytest.mark.corpus +@pytest.mark.parametrize("root", MOLECULE, ids=lambda p: p.name) +def test_render_reproduces_golden_over_the_molecule_corpus(root: Path) -> None: + if root.name in NO_DESIGN_PLAY: + pytest.skip("no play runs eos_designs") + if root.name == "eos_designs_negative_unit_tests": + # Every fixture is deliberately invalid and every play asserts a specific + # failure message. Rendering it clean would mean AVD's own negative + # suite had stopped working. + pytest.skip("AVD's negative corpus -- these are meant to fail") + _assert_renders(root, DEFERRED_RENDER_CORPUS) def test_corpus_is_not_empty() -> None: # The submodule is optional in a fresh worktree; an empty parametrisation # would make this whole file pass while testing nothing. - assert len(CORPUS) >= 8, f"expected the AVD corpus, found {len(CORPUS)} inventories" + assert len(EXAMPLES) == 8, f"expected AVD's 8 examples, found {len(EXAMPLES)}" + + +def test_the_migration_refuses_an_order_it_cannot_stand_behind(monkeypatch) -> None: + """The safety property, exercised rather than described. + + Group order (depth, name) is the one rule the Ansible CLI does not print, so + the migration layers the fragments with it and compares the result against + `ansible-inventory --list`. Break the order and nothing is emitted. + + ⚠ `campus-fabric` specifically, and the reason is worth keeping: reversing + the order changes **nothing** in `single-dc-l3ls` or `dual-dc-l3ls`, because + no two fragments there set the same key for the same device. Precedence is + load-bearing in exactly two of AVD's eight examples -- `campus-fabric`'s + role-level `aaa_settings` and `cv-pathfinder`'s `ipv4_acls`. A guard written + against either of the other six would pass while proving nothing. + """ + root = EXAMPLES_ROOT / "campus-fabric" + if not root.is_dir(): + pytest.skip("AVD submodule not initialised") + + import function.migrate as migrate_module + + original = migrate_module._fragments + + def reversed_order(inv, devices): + return list(reversed(original(inv, devices))) + + monkeypatch.setattr(migrate_module, "_fragments", reversed_order) + with pytest.raises(MigrationError) as refused: + migrate(root, collections=COLLECTIONS) + + # Two gates catch a wrong order, and the earlier one is the more useful: + # reading a resolved value back off the devices where a fragment wins finds + # `aaa_settings` disagreeing and names it, before the layering comparison + # gets to count differences. + assert "aaa_settings" in str(refused.value), str(refused.value) + + +def test_a_host_outside_the_play_is_not_a_device() -> None: + """`cv-pathfinder` holds `cloudvision`, which is not a switch. + + Its inventory carries the CloudVision API server so `cv_deploy` can reach + it; `build.yml` runs eos_designs on `hosts: WAN`. Declaring it would compose + a Device for it -- and a declared device with no node type fails the whole + fabric's render with AVD's `No device type found`, not just its own. + """ + root = EXAMPLES_ROOT / "cv-pathfinder" + if not root.is_dir(): + pytest.skip("AVD submodule not initialised") + + fabrics = migrate(root, collections=COLLECTIONS) + devices = {device for fabric in fabrics for device in fabric.devices} + assert "cloudvision" not in devices, "an API server is not a fabric device" + assert "pf1" in devices, "the play's own devices must survive the restriction" + + +def test_manifests_carry_every_input_in_requires_order() -> None: + fabric = Fabric( + name="f", devices=("leaf1",), fabric_name="FABRIC", + inputs=[ + Input("leaves", "NodeSet", {"l3leaf": {"nodes": [{"name": "leaf1"}]}}, + declares=["leaf1"]), + Input("base", "SettingSet", {"ntp_settings": {}}, all_devices=True), + ], + ) + manifests = to_manifests(fabric, namespace="avd") + assert [m["kind"] for m in manifests] == ["NodeSet", "SettingSet", "Fabric"] + assert manifests[-1]["spec"]["requires"] == [ + {"kind": "NodeSet", "name": "leaves", "namespace": "avd"}, + {"kind": "SettingSet", "name": "base", "namespace": "avd"}, + ] + assert manifests[0]["spec"]["declares"] == ["leaf1"] + assert "appliesTo" not in manifests[0]["spec"], "a NodeSet seen by what it declares says nothing" + assert manifests[1]["spec"]["appliesTo"] == {"all": True} def test_later_input_overwrites_earlier() -> None: @@ -108,103 +307,72 @@ def test_network_services_is_classified_under_either_spelling() -> None: `shared_utils/filtered_tenants.py` reads `inputs.network_services` and then the dynamic keys named by `network_services_keys` (default `tenants`). - Classifying the native spelling as `SettingSet` would put a services - document -- its BGP passwords and OSPF auth keys with it -- in the kind that - owns fabric-wide settings, and RBAC is granted per kind. - - Latent when written: of 25 inventories in AVD's corpus only - `eos_designs_unit_tests` uses the native spelling, and there it travels with - `network_services_keys`, which already classified it correctly. """ tenants = [{"name": "TENANT_A", "vrfs": [{"name": "VRF10"}]}] assert classify({"tenants": tenants}) == "NetworkServiceSet" assert classify({"network_services": tenants}) == "NetworkServiceSet" -def test_a_node_block_outranks_every_other_category() -> None: - """Classification is per fragment, first match wins -- a fragment mixing a - node block with anything else is a NodeSet, whole. - - Written down because it is what AVD's own host_vars do: one file carrying - `wan_router` alongside `bgp_peer_groups` and `wan_ipsec_profiles` becomes a - single NodeSet carrying those credentials. Ownership only -- `resolve` never - reads a kind, so no render changes with it. +def test_a_bare_type_is_node_content() -> None: + """`type` is the commonest key in AVD's whole corpus -- 639 fragments -- and + it says which node-type block a device belongs to. AVD gives it a table of + its own (`type-setting`). A group whose only variable is `type` is the + purest NodeSet there is, and calling it a setting is what forced the + migration to invent synthetic `-devices` NodeSets beside it. """ - design = { - "wan_router": {"nodes": [{"name": "wan1"}]}, - "bgp_peer_groups": {"wan_overlay_peers": {"password": "x"}}, - "tenants": [{"name": "TENANT_A"}], + assert classify({"type": "spine"}) == "NodeSet" + assert by_kind({"type": "spine", "ntp_settings": {}}) == { + "NodeSet": {"type": "spine"}, + "SettingSet": {"ntp_settings": {}}, } - assert classify(design) == "NodeSet" -def test_a_group_splits_into_one_input_per_category() -> None: +def test_a_fragment_is_split_by_category_not_won_by_one() -> None: """Merged as Ansible merges, then split -- so each XR carries one category. `cv-pathfinder`'s `group_vars/WAN/` is four files whose author had already separated settings, interface profiles, management and tenants. Ansible - merges a group_vars directory into one namespace, and classifying that whole - let a single `tenants` key decide the kind for 22. + merges a group_vars directory into one namespace, so the fragment arrives + carrying all four -- and one `tenants` key among 22 others must not decide + what the other 21 are. """ - parts = _by_category( - "WAN", - { - "l3leaf": {"nodes": [{"name": "leaf1"}]}, - "tenants": [{"name": "TENANT_A"}], - "aaa_settings": {"local_users": [{"name": "admin"}]}, - }, - ) - assert [(name, kind) for name, kind, _ in parts] == [ - ("WAN", "NodeSet"), - ("WAN-services", "NetworkServiceSet"), - ("WAN-settings", "SettingSet"), - ] - assert [sorted(design) for _, _, design in parts] == [ + parts = by_kind({ + "l3leaf": {"nodes": [{"name": "leaf1"}]}, + "tenants": [{"name": "TENANT_A"}], + "aaa_settings": {"local_users": [{"name": "admin"}]}, + }) + assert list(parts) == ["NodeSet", "NetworkServiceSet", "SettingSet"] + assert [sorted(design) for design in parts.values()] == [ ["l3leaf"], ["tenants"], ["aaa_settings"] ] -def test_a_group_of_one_category_keeps_its_bare_name() -> None: - """No suffix where there is nothing to tell apart -- `NETWORK_SERVICES.yml` - stays `NETWORK_SERVICES`, not `NETWORK_SERVICES-services`.""" - design = {"tenants": [{"name": "TENANT_A"}]} - assert _by_category("NETWORK_SERVICES", design) == [ - ("NETWORK_SERVICES", "NetworkServiceSet", design) - ] +def test_a_custom_dynamic_key_is_classified_when_its_generator_travels_along() -> None: + """The open-kinds limit, and the half of it that is not a limit. - -def test_a_host_outside_the_play_is_not_a_device() -> None: - """`cv-pathfinder` holds `cloudvision`, which is not a switch. - - Its inventory carries the CloudVision API server so `cv_deploy` can reach - it; `build.yml` runs eos_designs on `hosts: WAN`, and there is no - `cloudvision.cfg` in the example's intended configs. Declaring it would - compose a Device for it -- and a declared device with no node type fails the - whole fabric's render with AVD's `No device type found`, not just its own. - - This is the only host in the 8 bundled examples where the inventory and the - play disagree, which is why the model collapsed the two lists for so long. + Top-level key names come from the document's own content, so no static map + can name them all. But a fragment carrying its own `network_services_keys` + says what its keys mean, and then they are classifiable. """ - root = EXAMPLES_ROOT / "cv-pathfinder" - if not root.is_dir(): - pytest.skip("AVD submodule not initialised") + design = {"network_services_keys": [{"name": "tenant_a"}], "tenant_a": [{"name": "T"}]} + assert by_kind(design) == {"NetworkServiceSet": design} + # Without the generator there is nothing to read it by. + assert by_kind({"tenant_a": [{"name": "T"}]}) == {"SettingSet": {"tenant_a": [{"name": "T"}]}} - declared = {host for inp in inputs_from_inventory(root) for host in inp.declares} - assert declared, "cv-pathfinder should still declare its WAN devices" - assert "cloudvision" not in declared, ( - "cloudvision is an API server, not a fabric device -- play_hosts should " - "have kept it out of the device list" - ) - assert "pf1" in declared, "the play's own devices must survive the restriction" +def test_the_endpoint_family_is_read_from_pyavd_not_from_a_literal() -> None: + """`cameras` is the case that proves it: AVD ships it in + `connected_endpoints_keys`, and the hand-written list this replaced never + grew it. Nothing here names the members.""" + assert "cameras" in Vocabulary.default().connected_endpoints + assert classify({"cameras": [{"name": "cam1"}]}) == "ConnectedEndpointSet" def test_unscoped_nodeset_reaches_only_what_it_declares() -> None: """An omitted `appliesTo` means the whole fabric -- except on a NodeSet. Ansible has no unscoped group_vars file: a node-type block is read by its - own group. Defaulting a NodeSet to the whole fabric made every migrated - NodeSet name itself in `appliesTo`, 26 of 26 across AVD's examples. + own group. """ inputs = [ Input("spines", "NodeSet", {"spine": {"nodes": [{"name": "spine1"}]}}, @@ -216,17 +384,12 @@ def test_unscoped_nodeset_reaches_only_what_it_declares() -> None: out = resolve(inputs) assert "spine" in out["spine1"] and "spine" not in out["leaf1"] assert "l3leaf" in out["leaf1"] and "l3leaf" not in out["spine1"] - # Every other kind still defaults to the whole fabric. assert out["spine1"]["ntp_settings"] == out["leaf1"]["ntp_settings"] def test_a_nodeset_may_be_seen_wider_than_it_declares() -> None: - """The case the default must not swallow, and it is real. - - `eos_designs-twodc-5stage-clos` has a DC-level NodeSet declaring 4 - super_spines and visible to all 16 devices of that DC. Getting this wrong - cost 96 hostvar diffs once. - """ + """`eos_designs-twodc-5stage-clos` has a DC-level NodeSet declaring 4 + super_spines and visible to all 16 devices of that DC.""" inputs = [ Input("dc1", "NodeSet", {"super_spine": {"nodes": [{"name": "ss1"}]}}, declares=["ss1"], node_sets=["dc1", "dc1-pod1"]), @@ -243,11 +406,69 @@ def test_undeclared_node_is_not_a_device() -> None: anta_runner does -- and it must not become a device.""" inputs = [ Input( - "leaves", - "NodeSet", + "leaves", "NodeSet", {"l3leaf": {"nodes": [{"name": "leaf1"}, {"name": "ghost"}]}}, - node_sets=["leaves"], - declares=["leaf1"], + node_sets=["leaves"], declares=["leaf1"], ) ] assert set(resolve(inputs)) == {"leaf1"} + + +def test_only_description_templates_are_droppable() -> None: + """Both are `.j2` paths pyavd cannot honour; only one is safe to lose. + + An `interface_descriptions` template decides a `description` string -- on + `evpn_underlay_ebgp_overlay_ebgp`, dropping them renders all 16 devices and + differs from AVD's golden in 168 places, every one of them a `description`. + An `ip_addressing` template decides an address: `eos_designs-twodc-5stage-clos` + computes its P2P uplink IPs that way. Dropping that would emit a different + network without saying so, which is why one flag cannot cover both. + """ + design = { + "node_type_keys": [{ + "key": "spine", + "interface_descriptions": {"underlay_ethernet_interfaces": "d/eth.j2"}, + "ip_addressing": {"p2p_uplinks_ip": "a/p2p.j2"}, + }], + } + assert unsupported(design) == { + "interface_descriptions": ["node_type_keys.[0].interface_descriptions." + "underlay_ethernet_interfaces"], + "ip_addressing": ["node_type_keys.[0].ip_addressing.p2p_uplinks_ip"], + } + + removed = drop_description_templates(design) + assert removed == ["node_type_keys.[0].interface_descriptions." + "underlay_ethernet_interfaces"] + entry = design["node_type_keys"][0] + assert "interface_descriptions" not in entry, "an emptied key is removed, not left bare" + assert entry["ip_addressing"] == {"p2p_uplinks_ip": "a/p2p.j2"}, ( + "addressing must survive the flag that drops descriptions" + ) + + +def test_custom_python_modules_are_reported_and_never_dropped() -> None: + """Code, not data. A function image is immutable and loads no arbitrary + Python, so this cannot travel and cannot be quietly discarded either.""" + design = {"templates": {"ip_addressing": {"python_module": "custom_ip_addressing"}}} + assert unsupported(design) == { + "python_module": ["templates.ip_addressing.python_module"] + } + assert drop_description_templates(design) == [] + + +def test_a_migration_says_what_it_could_not_carry() -> None: + """Reported without the flag too -- someone migrating a real inventory has + to learn this from the tool, not from a diff on a device.""" + root = MOLECULE_ROOT / "evpn_underlay_ebgp_overlay_ebgp" + if not root.is_dir(): + pytest.skip("AVD submodule not initialised") + + fabric = migrate(root, collections=COLLECTIONS)[0] + assert any("interface_descriptions" in note for note in fabric.notes), fabric.notes + assert all("dropped" not in note for note in fabric.notes), ( + "nothing may be dropped unless asked" + ) + + dropped = migrate(root, collections=COLLECTIONS, drop_descriptions=True)[0] + assert any(note.startswith("dropped ") for note in dropped.notes), dropped.notes diff --git a/tests/test_xr_fold.py b/tests/test_xr_fold.py deleted file mode 100644 index 3a9c425..0000000 --- a/tests/test_xr_fold.py +++ /dev/null @@ -1,29 +0,0 @@ -"""Milestone 2: the Ansible -> Fabric-XR fold still reproduces golden. - -Offline -- no cluster. Guards `xr.fabric_design_from_inputs` (block union + -defaults push-down), which is the part most likely to silently lose a value. -""" - -from __future__ import annotations - -from pathlib import Path - -import pytest - -from function.verify_xr import DEFERRED, EXAMPLES_ROOT, _discover, verify_one - - -@pytest.mark.parametrize("example_dir", _discover(EXAMPLES_ROOT), ids=lambda p: p.name) -def test_fold_reproduces_golden(example_dir: Path) -> None: - status, _ = verify_one(example_dir) - - if example_dir.name in DEFERRED: - # Expected failure. Asserting it still fails means a deferral can never - # rot: if the example starts folding, this fails and tells us to drop it. - assert not status.startswith("ok"), ( - f"{example_dir.name} folds now -- remove it from verify_xr.DEFERRED " - f"(was deferred: {DEFERRED[example_dir.name]})" - ) - return - - assert status == "ok", f"{example_dir.name}: {status}" diff --git a/uv.lock b/uv.lock index a39fcc7..07c467c 100644 --- a/uv.lock +++ b/uv.lock @@ -11,6 +11,22 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/78/b6/6307fbef88d9b5ee7421e68d78a9f162e0da4900bc5f5793f6d3d0e34fb8/annotated_types-0.7.0-py3-none-any.whl", hash = "sha256:1f02e8b43a8fbbc3f3e0d4f0f4bfc8131bcb4eebe8849b8e5c773f3a1c582a53", size = 13643, upload-time = "2024-05-20T21:33:24.1Z" }, ] +[[package]] +name = "ansible-core" +version = "2.21.3" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cryptography" }, + { name = "jinja2" }, + { name = "packaging" }, + { name = "pyyaml" }, + { name = "resolvelib" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/1c/11/cb53834d320c38d739e756e2458852d6e74a6c7018a9ab9f6d4ab5e5196e/ansible_core-2.21.3.tar.gz", hash = "sha256:4194fbd82273cbacfd06d86d74d2d7168c3c4b8426c03e93562cd7217f811ae1", size = 3397615, upload-time = "2026-08-10T16:46:14.554Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/a3/fa/938fe0504372377af2bef42abe0d8e463fdcd011ad803eaf300e4577dd7a/ansible_core-2.21.3-py3-none-any.whl", hash = "sha256:9e7dd367f7dc5d5e9fc5ae1baf8af9c4edc09e916a73a40108a3f32e3ad93f10", size = 2446988, upload-time = "2026-08-10T16:46:12.947Z" }, +] + [[package]] name = "anta" version = "1.8.0" @@ -337,6 +353,7 @@ dependencies = [ [package.dev-dependencies] dev = [ + { name = "ansible-core" }, { name = "pytest" }, ] @@ -348,7 +365,10 @@ requires-dist = [ ] [package.metadata.requires-dev] -dev = [{ name = "pytest", specifier = ">=8.0" }] +dev = [ + { name = "ansible-core", specifier = ">=2.18" }, + { name = "pytest", specifier = ">=8.0" }, +] [[package]] name = "grpcio" @@ -1020,6 +1040,15 @@ socks = [ { name = "pysocks" }, ] +[[package]] +name = "resolvelib" +version = "1.2.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/1d/14/4669927e06631070edb968c78fdb6ce8992e27c9ab2cde4b3993e22ac7af/resolvelib-1.2.1.tar.gz", hash = "sha256:7d08a2022f6e16ce405d60b68c390f054efcfd0477d4b9bd019cc941c28fad1c", size = 24575, upload-time = "2025-10-11T01:07:44.582Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/e2/23/c941a0d0353681ca138489983c4309e0f5095dfd902e1357004f2357ddf2/resolvelib-1.2.1-py3-none-any.whl", hash = "sha256:fb06b66c8da04172d9e72a21d7d06186d8919e32ae5ab5cdf5b9d920be805ac2", size = 18737, upload-time = "2025-10-11T01:07:43.081Z" }, +] + [[package]] name = "rich" version = "14.3.4" From 0c577210ee149fbe5b1c75ae4a7bb55a686e6e43 Mon Sep 17 00:00:00 2001 From: mbakalarski <64490638+mbakalarski@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:55:09 +0000 Subject: [PATCH 8/8] Keep a fabric's node IDs on the cluster, and check the whole chain there MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everything here was found or proven by applying things to a real cluster. The offline suite drives RunFunction directly, so it never meets the API server and never runs scripts/kind-up.sh -- and that turned out to hide a whole class of failure, including one that made the central shape of this branch unusable. Three bugs a cluster found and no offline test could. Every migrated Fabric was unapplyable. The XRD listed spec.design as required while fn.py has always accepted "design or requires", and avd-migrate emits {fabricName, requires} with no design at all. A hand-written test fabric hid it by happening to carry design; the corpus-derived one did not. Test with what the tool emits, not with something shaped like it. Uppercase hostnames could not be composed: the hostname went straight into the composed resource name and Kubernetes rejects it. Two of AVD's eight bundled examples -- campus-fabric and l2ls-fabric -- write hostnames entirely in capitals, so a quarter of the canonical corpus failed to compose. _dns_name now spells the object name while the hostname itself stays untouched in spec and labels, with a collision check: child_name hashes what it is given, so SPINE1 and spine1 would have produced one Device for two switches. scripts/kind-up.sh installed one XRD out of six. `apply -f apis/*/xrd.yaml` takes the first path the glob expands to and kubectl rejects the rest. It worked at two API directories and broke silently at six, unseen because `e2e on kind` is workflow_dispatch-only and nothing else in CI runs that script. The node-ID pool now has somewhere to live. A fabric setting fabric_numbering.node_id.algorithm to pool_manager reads its previous assignments from a ConfigMap it composes and writes the updated pool back -- the pool is an output that is also the next run's input, the one place a render stops being a pure function of its inputs. spec.nodeIdPool.seedConfigMapName carries assignments in from a fabric that was already running elsewhere, gated like the named inputs because requirements are answered on the next reconcile; without that gate the first pass would assign a fresh set and the seed would arrive too late. Named-and-absent is fatal: carrying on renumbers every device. ⚠ Three of the four pool tests pass on an implementation that ignores the ConfigMap entirely, because assignment is deterministic from the device set. Only test_the_pool_decides_the_ids -- hand the fabric a pool holding id 7 and require the render to follow -- proves anything. Measured the same way on a cluster: deleting the pool with the device set unchanged costs nothing, and deleting it after a device left renumbered the survivor from 2 to 1. function/avd_compat.py transcribes AVD's v2.x spine addressing as a subclass of the public AvdIpAddressing, which is the route AVD's own schema offers where pyavd implements no Jinja templating. A module shipped in this image is importable by dotted path, so this loads no arbitrary code. avd-migrate says more about what it cannot carry: a pool's assignments, and variables set on the play itself. Play vars outrank group and host vars and ansible-inventory cannot see them -- eos_designs-twodc-5stage-clos runs eos_designs twice and the second play sets avd_digital_twin_mode, so migrated without it the two fabrics come out identical and one is wrong. Reported rather than carried, because the oracle this migration checks itself against is Ansible's hostvars, which do not include them. Fabric names are unique per play now: both plays target the same host pattern, so --emit wrote one file and the first fabric was silently lost. Proven on a cluster: all eight bundled AVD examples, migrated to XRs, applied, and rendering structured config identical to AVD's checked-in golden -- every device, not a sample. ⚠ One namespace per scenario, because single-dc-l3ls, -ipv6 and -multipod all name their devices dc1-leaf1a and a lookup by label compared one scenario's render against another's golden. ⚠ tests/test_e2e_node_id_pool.py is committed strict-xfail. The same twodc scenario renders 26 of 26 clean offline and 12 differences on a cluster, all of them a service_profile key. Not diagnosed; the lead is that the scenario's two fabrics share 47 of 48 input XR names while their contents differ. Recorded with the evidence rather than left out. 91 offline tests green; each new guard verified to fail without its change. Co-Authored-By: Claude Opus 5 --- apis/fabric/xrd.yaml | 36 +++- function/avd_compat.py | 59 ++++++ function/engine.py | 12 +- function/fn.py | 82 +++++++- function/migrate.py | 76 +++++++- function/pools.py | 146 ++++++++++++++ scripts/kind-up.sh | 13 +- tests/test_avd_compat.py | 148 ++++++++++++++ tests/test_e2e_migrated_corpus.py | 160 ++++++++++++++++ tests/test_e2e_node_id_pool.py | 221 +++++++++++++++++++++ tests/test_fabric_collect.py | 309 ++++++++++++++++++++++++++++++ tests/test_kinds_equivalence.py | 12 +- 12 files changed, 1258 insertions(+), 16 deletions(-) create mode 100644 function/avd_compat.py create mode 100644 function/pools.py create mode 100644 tests/test_avd_compat.py create mode 100644 tests/test_e2e_migrated_corpus.py create mode 100644 tests/test_e2e_node_id_pool.py diff --git a/apis/fabric/xrd.yaml b/apis/fabric/xrd.yaml index 2dd85f2..310bcfb 100644 --- a/apis/fabric/xrd.yaml +++ b/apis/fabric/xrd.yaml @@ -88,6 +88,35 @@ spec: required: - kind - name + nodeIdPool: + type: object + description: >- + Where this fabric's node IDs come from when the design sets + fabric_numbering.node_id.algorithm to pool_manager. AVD then + hands out IDs instead of reading them off each node, and + keeps the assignments in a file. There is no such file in a + cluster, so the Fabric composes a ConfigMap and reads it back + on the next reconcile. Nothing here is needed for a fabric + that numbers its nodes itself. + properties: + seedConfigMapName: + type: string + description: >- + A ConfigMap holding assignments to start from, read once + when this fabric has no pool of its own yet. It exists + for a fabric that was already running elsewhere: AVD + generated its IDs and they are in a file, and a fabric + that starts over assigns different ones - which reaches + every device as a full configuration replacement. Not + needed for a new fabric, and ignored once the composed + pool exists, so it seeds rather than overrides. + minLength: 1 + seedKey: + type: string + description: >- + Key within that ConfigMap. Defaults to the key the + composed pool uses. + minLength: 1 push: type: object description: >- @@ -126,9 +155,14 @@ spec: description: cEOS serves a self-signed cert; default true. required: - credentialsSecretName + # `design` is NOT required: a fabric may be assembled entirely from + # the inputs named in `spec.requires`, which is what `avd-migrate` + # emits and what `fn.py` has always accepted ("design or requires"). + # Requiring it here made every migrated Fabric unapplyable, and no + # offline test could see it -- they drive RunFunction directly and + # never meet the API server's schema. required: - fabricName - - design status: type: object properties: diff --git a/function/avd_compat.py b/function/avd_compat.py new file mode 100644 index 0000000..aa1c352 --- /dev/null +++ b/function/avd_compat.py @@ -0,0 +1,59 @@ +"""AVD behaviours pyavd cannot reach, written as the classes AVD asks for. + +pyavd implements **no Jinja templating**: `get_device_structured_config` passes +`templar=None` and the call raises `NotImplementedError`, and no public entry +point accepts a templar. So a design pinning a `.j2` path cannot render here, +wherever the file is carried. + +The same schema blocks — `node_type_keys[].ip_addressing` and +`.interface_descriptions` — take `python_module` / `python_class_name` instead, +and **that route pyavd supports**: `load_python_class` imports the module by +dotted path and checks it against the public base class. A module shipped inside +this package is importable by dotted path, so pointing a design at +`function.avd_compat` loads no arbitrary code — it loads ours. + +This is not a general answer. It reproduces one specific, published scheme. +Anyone whose fabric uses a template of their own writes their own class and +builds their own function image on this one. +""" + +from __future__ import annotations + +import ipaddress + +from pyavd.api.ip_addressing import AvdIpAddressing + + +class AvdIpAddressingV2Spine(AvdIpAddressing): + """AVD v2.x spine-to-super-spine P2P addressing. + + A transcription of the two templates `eos_designs-twodc-5stage-clos` pins, + which say what they are: *"In AVD v2.x the spine to super-spine links used + this special IP addressing scheme. This file may still be used by older + inventories."* + + ⚠ The comment describes where they came from, not that they are inert. The + scheme divides the uplink pool by `max_uplink_switches` so that adding a + spine does not move existing addresses — which is why that fabric's golden + puts a spine's two super-spine uplinks 64 apart rather than adjacent. AVD's + native algorithm packs them contiguously and the two are not interchangeable. + + Everything else falls through to `AvdIpAddressing`, so a fabric selecting + this class changes only its P2P uplinks. + """ + + def _v2_p2p(self, uplink_switch_index: int, last: int) -> str: + pool = ipaddress.ip_network(self._uplink_ipv4_pool, strict=False) + offset = (self._id - 1) % self._max_parallel_uplinks + index = ( + (pool.num_addresses // self._max_uplink_switches) * int(uplink_switch_index) + + ((self._id - 1) * self._max_parallel_uplinks + offset) * 2 + + last + ) + return str(pool.network_address + index) + + def p2p_uplinks_ip(self, uplink_switch_index: int) -> str: + return self._v2_p2p(uplink_switch_index, 1) + + def p2p_uplinks_peer_ip(self, uplink_switch_index: int) -> str: + return self._v2_p2p(uplink_switch_index, 0) diff --git a/function/engine.py b/function/engine.py index e97970b..f91176a 100644 --- a/function/engine.py +++ b/function/engine.py @@ -85,19 +85,27 @@ def validate_all(all_inputs: dict[str, dict]) -> dict[str, list]: def render_structured_configs( - all_inputs: dict[str, dict], *, validate: bool = True + all_inputs: dict[str, dict], *, validate: bool = True, pool_manager: object = None ) -> dict[str, dict]: """Run facts + per-device structured config for the whole fabric. ``get_avd_facts`` is fabric-wide (needs every device at once); the structured config is then derived per device from those shared facts. + + ``pool_manager`` is required only by a fabric setting + ``fabric_numbering.node_id.algorithm: pool_manager``, which asks AVD to + assign node IDs from a pool instead of reading them off each node. ⚠ The + pool is **a file** (`pyavd.api.pool_manager.PoolManager(output_dir)`), and + the assignments have to survive between runs or every device is renumbered — + so nothing composes one yet, and a Fabric that asks for it fails with AVD's + own message until a Fabric has somewhere to keep it. """ if validate: violations = validate_all(all_inputs) if violations: raise InputValidationError(violations) - avd_facts = pyavd.get_avd_facts(all_inputs) + avd_facts = pyavd.get_avd_facts(all_inputs, pool_manager=pool_manager) return { hostname: pyavd.get_device_structured_config( hostname, inputs, avd_facts=avd_facts diff --git a/function/fn.py b/function/fn.py index be9c942..346e8fc 100644 --- a/function/fn.py +++ b/function/fn.py @@ -17,6 +17,7 @@ from __future__ import annotations import hashlib +import re from datetime import datetime, timezone import pyavd @@ -25,7 +26,7 @@ from crossplane.function.proto.v1 import run_function_pb2 as fnv1 from crossplane.function.proto.v1 import run_function_pb2_grpc as grpcv1 -from . import push +from . import pools, push from .engine import ( InputValidationError, device_roles_from_design, @@ -64,6 +65,19 @@ def _normalize_numbers(obj): return obj +def _dns_name(hostname: str) -> str: + """A hostname as a Kubernetes object name may spell it. + + AVD hostnames are free text and two of its eight bundled examples -- + `campus-fabric` and `l2ls-fabric` -- write them entirely in capitals. A + composed resource named after one is rejected outright: *"invalid name + ... Must be a valid RFC 1123 subdomain name"*, so the whole fabric fails to + compose. The hostname itself is untouched; only the object's name is spelled + this way. + """ + return re.sub(r"[^a-z0-9-]+", "-", hostname.lower()).strip("-") or "device" + + def _now() -> str: return datetime.now(timezone.utc).isoformat(timespec="seconds") @@ -166,8 +180,47 @@ def _reconcile_fabric( document["fabric_name"] = fabric_name all_inputs = {host: document for host in hostnames_from_design(document)} + # A fabric that asks for pool-assigned node IDs needs its assignments + # back before it renders, or every device is renumbered on every pass. + keeps_a_pool = pools.wanted_by(all_inputs) + pool = "" try: - structured_configs = render_structured_configs(all_inputs) + if keeps_a_pool: + previous = pools.observed_pool(req.observed.resources) + if not previous.strip(): + # Only before this fabric has a pool of its own. Once it has, + # the seed is history and must not override it. + state, seeded = pools.seed(req, rsp, spec, namespace) + if state == "pending": + # The normal first pass, exactly as for the named inputs. + response.set_conditions( + rsp, + resource.Condition( + typ="InputsResolved", + status="False", + reason="WaitingForSeed", + message="waiting for the node-ID pool seed ConfigMap", + ), + ) + response.normal(rsp, "waiting for the node-ID pool seed") + return + if state == "missing": + response.fatal( + rsp, + "spec.nodeIdPool.seedConfigMapName names a ConfigMap that " + "does not exist; rendering without it would renumber every " + "device", + ) + return + previous = seeded + with pools.pool_manager(all_inputs, previous) as (manager, pool_file): + structured_configs = render_structured_configs( + all_inputs, pool_manager=manager + ) + manager.save_updated_pools() + pool = pool_file.read_text() if pool_file.is_file() else previous + else: + structured_configs = render_structured_configs(all_inputs) except InputValidationError as err: resource.update_status( rsp.desired.composite, @@ -179,6 +232,15 @@ def _reconcile_fabric( response.fatal(rsp, f"AVD render failed: {type(err).__name__}: {err}") return + if keeps_a_pool: + # Composed after the render, so a failed render never overwrites a + # good pool with a partial one. + resource.update( + rsp.desired.resources[pools.RESOURCE_NAME], + pools.configmap(xr_name, namespace, fabric_name, pool), + ) + rsp.desired.resources[pools.RESOURCE_NAME].ready = fnv1.READY_TRUE + push_spec = spec.get("push") or {} if push_spec and not push_spec.get("credentialsSecretName"): response.fatal(rsp, "spec.push.credentialsSecretName is required when push is set") @@ -197,6 +259,20 @@ def _reconcile_fabric( host: device_roles_from_design(hostvars).get(host) or hostvars.get("type", "") for host, hostvars in all_inputs.items() } + # Two hostnames that differ only in case, or only in a character a + # Kubernetes name cannot carry, would compose a single Device between + # them -- one config silently standing in for two switches. + spelled: dict[str, str] = {} + for hostname in sorted(structured_configs): + clash = spelled.setdefault(_dns_name(hostname), hostname) + if clash != hostname: + response.fatal( + rsp, + f"devices {clash!r} and {hostname!r} both need the object name " + f"{_dns_name(hostname)!r}; rename one", + ) + return + observed_devices = req.observed.resources # keyed by composition-resource-name (hostname) devices = [] for hostname, structured_config in structured_configs.items(): @@ -207,7 +283,7 @@ def _reconcile_fabric( "apiVersion": API_VERSION, "kind": "Device", "metadata": { - "name": resource.child_name(xr_name, hostname), + "name": resource.child_name(xr_name, _dns_name(hostname)), "namespace": namespace, "labels": { "avd.netclab.dev/fabric": fabric_name, diff --git a/function/migrate.py b/function/migrate.py index 4bcb658..4250da1 100644 --- a/function/migrate.py +++ b/function/migrate.py @@ -294,10 +294,20 @@ def _inputs(fragments: list[Fragment], devices: frozenset[str], return [inp for _, inp in planned] -def _fabric_name(root: Path, play: Play, many: bool) -> str: +def _fabric_name(root: Path, play: Play, many: bool, taken: set[str]) -> str: + """A name per play, and never the same one twice. + + ⚠ The pattern does not always tell two plays apart: + `eos_designs-twodc-5stage-clos` runs eos_designs twice over the same + `hosts: TWODC_5STAGE_CLOS`, so naming by pattern gave both fabrics one name — + and `--emit` wrote one file, the second silently overwriting the first. + """ if not many: - return slug(root.name) - return slug(f"{root.name}-{play.pattern}") or slug(f"{root.name}-{play.index}") + return _unique(slug(root.name), taken) + base = slug(f"{root.name}-{play.pattern}") or slug(root.name) + if base in taken: + base = slug(f"{base}-{play.playbook.removesuffix('.yml')}-{play.index}") + return _unique(base, taken) def migrate(root: Path, collections: Path | None = None, inventory: Path | None = None, @@ -325,6 +335,7 @@ def migrate(root: Path, collections: Path | None = None, inventory: Path | None vocabulary = Vocabulary.default() fabrics: list[Fabric] = [] + named: set[str] = set() for play in found: devices = frozenset(play.hosts) if not devices: @@ -347,12 +358,13 @@ def migrate(root: Path, collections: Path | None = None, inventory: Path | None ) fabric = Fabric( - name=_fabric_name(root, play, len(found) > 1), + name=_fabric_name(root, play, len(found) > 1, named), devices=tuple(sorted(devices)), inputs=_inputs(fragments, devices, vocabulary), play=play, ) _fabric_name_of(fabric, inv.hostvars) + _report_play_vars(fabric, root, play) # Only now, with the translation proven faithful. Dropping anything # before the comparison above would weaken the one gate this module has. _report_unsupported(fabric, drop_descriptions) @@ -360,8 +372,64 @@ def migrate(root: Path, collections: Path | None = None, inventory: Path | None return fabrics +def _pooled_ids(design: dict) -> dict: + node_id = (design.get("fabric_numbering") or {}).get("node_id") + if isinstance(node_id, dict) and node_id.get("algorithm") == "pool_manager": + return node_id + return {} + + +def _report_play_vars(fabric: Fabric, root: Path, play: Play) -> None: + """Variables set on the play itself, which no input XR carries. + + ⚠ A source `ansible-inventory` cannot see, and one that changes the render: + `eos_designs-twodc-5stage-clos` runs eos_designs twice over the same hosts + and the second play sets `avd_digital_twin_mode: true`, producing a + different config into a different golden directory. Migrated without it, the + two fabrics come out identical and one of them is wrong. + + **Reported, not carried.** Play vars outrank group and host vars, but the + oracle this migration checks itself against is per-host hostvars, which do + not include them -- so carrying them would silently weaken the one gate this + module has. Whoever migrates such a play adds the keys to an input by hand. + """ + import yaml as _yaml + + playbook = root / play.playbook + try: + document = _yaml.safe_load(playbook.read_text()) + except (OSError, _yaml.YAMLError): + return + if not isinstance(document, list) or play.index > len(document): + return + variables = document[play.index - 1].get("vars") if isinstance( + document[play.index - 1], dict) else None + if isinstance(variables, dict) and variables: + fabric.notes.append( + f"{play.playbook} play #{play.index} sets {len(variables)} variable(s) on the " + f"play itself ({', '.join(sorted(variables))}); play vars outrank group and " + f"host vars and are NOT carried into any input" + ) + + def _report_unsupported(fabric: Fabric, drop_descriptions: bool) -> None: """Note -- and optionally drop -- what pyavd will not honour.""" + # State, not settings. An inventory already running `pool_manager` keeps its + # node IDs in a file AVD generated; this translates the *setting* and leaves + # the *assignments* behind. Applied to a fabric that is already deployed, + # that renumbers every device -- and a render reaches a switch as a full + # configuration replacement. + for inp in fabric.inputs: + pooled = _pooled_ids(inp.design) + if pooled: + where = pooled.get("pools_file") or "/intended/data/-ids.yml" + fabric.notes.append( + f"node IDs come from a pool; its assignments live in {where} and do " + f"NOT travel with this migration. Seed them into the fabric " + f"(spec.nodeIdPool.seedConfigMapName) or every device is renumbered" + ) + break + if drop_descriptions: dropped = [ f"{inp.name}.{path}" diff --git a/function/pools.py b/function/pools.py new file mode 100644 index 0000000..431b137 --- /dev/null +++ b/function/pools.py @@ -0,0 +1,146 @@ +"""Where a Fabric keeps its node-ID pool. + +`fabric_numbering.node_id.algorithm: pool_manager` asks AVD to hand out node IDs +instead of reading them off each node. AVD keeps those assignments in **a file**, +and the render is only reproducible while that file survives — lose it and every +device is renumbered, which reaches a switch as a full configuration replacement. + +There is no such file in a cluster, so the Fabric keeps the pool in a ConfigMap +it composes and reads back on the next reconcile. The pool is therefore an +**output that is also the next run's input** — the one place a render stops being +a pure function of its inputs. + +⚠ **`spec.design`'s own `pools_file` is overridden, not honoured.** It names a +path relative to a working directory, which is a statement about somebody's +laptop; the same design in a cluster has nowhere to point. The value is replaced +with a path inside a scratch directory that exists only for the length of one +reconcile, and the ConfigMap is the real home. +""" + +from __future__ import annotations + +import tempfile +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path +from typing import Any + +#: composition-resource-name of the ConfigMap, and the key inside it +RESOURCE_NAME = "id-pool" +DATA_KEY = "node-id-pools.yml" + +_ALGORITHM_PATH = ("fabric_numbering", "node_id") +_POOL_FILE = "pools.yml" + + +def wanted_by(all_inputs: dict[str, dict]) -> bool: + """Does any device ask for pool-assigned node IDs? + + Read per device rather than fabric-wide because the setting is an ordinary + input key: nothing stops one input from narrowing it to part of the fabric, + and one device asking is enough to need a pool. + """ + return any(_node_id(hostvars).get("algorithm") == "pool_manager" + for hostvars in all_inputs.values()) + + +def _node_id(hostvars: dict) -> dict: + node = hostvars + for key in _ALGORITHM_PATH: + node = node.get(key) if isinstance(node, dict) else None + if node is None: + return {} + return node if isinstance(node, dict) else {} + + +def observed_pool(observed_resources: Any) -> str: + """The pool as the cluster last saw it, or empty on the first reconcile.""" + entry = (observed_resources or {}).get(RESOURCE_NAME) + if entry is None: + return "" + from crossplane.function import resource + + data = (resource.struct_to_dict(entry.resource) or {}).get("data") or {} + return data.get(DATA_KEY) or "" + + +@contextmanager +def pool_manager(all_inputs: dict[str, dict], previous: str) -> Iterator[tuple[Any, Path]]: + """A ``PoolManager`` backed by ``previous``, and the file it ends up in. + + Rewrites every device's `pools_file` to the scratch copy first: AVD resolves + that value verbatim against the working directory, so leaving the design's + own value in place would read a path that does not exist here and silently + assign a fresh set of IDs. + """ + from pyavd.api.pool_manager import PoolManager + + with tempfile.TemporaryDirectory() as scratch: + path = Path(scratch) / _POOL_FILE + if previous.strip(): + path.write_text(previous) + for hostvars in all_inputs.values(): + node_id = _node_id(hostvars) + if node_id.get("algorithm") == "pool_manager": + node_id["pools_file"] = str(path) + yield PoolManager(Path(scratch)), path + + +#: requirement key for a seed ConfigMap named by the Fabric +SEED_NAME = "id-pool-seed" + + +def seed(req: Any, rsp: Any, spec: dict, namespace: str) -> tuple[str, str]: + """Assignments to start from, when this fabric has no pool of its own yet. + + Returns ``(state, text)`` where state is one of: + + * ``none`` -- no seed named; a new fabric assigns its own IDs; + * ``pending`` -- asked for, not delivered yet. ⚠ **The caller must not + render.** Requirements are answered on the *next* reconcile, so the first + one always arrives with nothing; rendering then would assign a fresh set + of IDs and compose them as the pool, and the seed would never be read; + * ``missing`` -- Crossplane looked and it is not there. Named and absent is + an error, not an empty pool: proceeding renumbers every device, which is + the one thing this field exists to prevent; + * ``ok`` -- the text. + """ + from crossplane.function import resource, response + + pool_spec = spec.get("nodeIdPool") or {} + name = pool_spec.get("seedConfigMapName") + if not name: + return "none", "" + + response.require_resources( + rsp, name=SEED_NAME, api_version="v1", kind="ConfigMap", + match_name=name, namespace=namespace, + ) + if SEED_NAME not in req.required_resources: + return "pending", "" + items = req.required_resources[SEED_NAME].items + if not items: + return "missing", "" + + data = (resource.struct_to_dict(items[0].resource) or {}).get("data") or {} + return "ok", data.get(pool_spec.get("seedKey") or DATA_KEY) or "" + + +def configmap(xr_name: str, namespace: str, fabric_name: str, pool: str) -> dict: + """The ConfigMap the Fabric composes to keep its assignments.""" + from crossplane.function import resource + + return { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": resource.child_name(xr_name, "id-pool"), + "namespace": namespace, + "labels": {"avd.netclab.dev/fabric": fabric_name}, + "annotations": { + "avd.netclab.dev/description": + "AVD node-ID assignments. Deleting this renumbers the fabric.", + }, + }, + "data": {DATA_KEY: pool}, + } diff --git a/scripts/kind-up.sh b/scripts/kind-up.sh index 725f1e9..630cd31 100755 --- a/scripts/kind-up.sh +++ b/scripts/kind-up.sh @@ -135,9 +135,18 @@ kubectl --context "$CTX" wait --for=condition=Healthy function.pkg.crossplane.io # the package it was built from -- and the input kinds would be missing exactly # where a Fabric that names them is being tested. echo ">> install XRDs + Compositions (everything under apis/)" -kubectl --context "$CTX" apply -f apis/*/xrd.yaml +# One -f per file: `apply -f apis/*/xrd.yaml` reads only the first path the +# glob expands to and passes the rest as positional args, which kubectl rejects. +# It worked while there were two API directories and broke silently at six -- +# and nothing caught it, because `e2e on kind` is workflow_dispatch-only and +# nothing else in CI runs this script. +for manifest in apis/*/xrd.yaml; do + kubectl --context "$CTX" apply -f "$manifest" +done kubectl --context "$CTX" wait --for=condition=Established xrd --all --timeout=60s -kubectl --context "$CTX" apply -f apis/*/composition.yaml +for manifest in apis/*/composition.yaml; do + kubectl --context "$CTX" apply -f "$manifest" +done if [ "$WITH_NETCLAB" = "1" ]; then echo ">> provider-http ${PROVIDER_HTTP} (config push over eAPI)" diff --git a/tests/test_avd_compat.py b/tests/test_avd_compat.py new file mode 100644 index 0000000..face1d7 --- /dev/null +++ b/tests/test_avd_compat.py @@ -0,0 +1,148 @@ +"""The two things `eos_designs-twodc-5stage-clos` needs, proven against golden. + +That scenario is the only fabric in AVD's corpus that turns on `pool_manager`, +and it also pins two `.j2` addressing templates. Both are unreachable through +pyavd's public API as written — Jinja is not implemented, and nothing composes a +pool — so it renders as a failure and the reason it reports is whichever one it +meets first. + +This is the offline oracle for both. With the pool supplied and the templates +replaced by the class AVD's own schema offers instead, the render must reproduce +AVD's checked-in golden **exactly**. Without that, a later failure in a cluster +could be the pool, the class or the render, and nothing would say which. + +⚠ It does not claim the *migrated* design renders. It pins `.j2` and always +will; the substitution below is what a fabric would carry instead. +""" + +from __future__ import annotations + +import shutil +import tempfile +from pathlib import Path + +import pytest +import yaml + +from function.kinds import resolve +from function.migrate import migrate + +pytestmark = pytest.mark.corpus + +SCENARIO = Path( + "avd/ansible_collections/arista/avd/extensions/molecule/eos_designs-twodc-5stage-clos" +) +COLLECTIONS = Path("avd").resolve() + +#: what the scenario's own group_vars name, relative to the working directory +POOL = Path("intended/data/test-ids.yml") + + +def _differences(path: str, ours: object, golden: object, out: list[str]) -> None: + if type(ours) is not type(golden) and not ( + isinstance(ours, (int, float)) and isinstance(golden, (int, float)) + ): + out.append(f"{path}: type {type(ours).__name__} != {type(golden).__name__}") + elif isinstance(golden, dict): + assert isinstance(ours, dict) + for key in sorted(set(ours) | set(golden)): + if key not in ours: + out.append(f"{path}.{key}: only in golden") + elif key not in golden: + out.append(f"{path}.{key}: only in ours") + else: + _differences(f"{path}.{key}", ours[key], golden[key], out) + elif isinstance(golden, list): + assert isinstance(ours, list) + if len(ours) != len(golden): + out.append(f"{path}: list len {len(ours)} != {len(golden)}") + for index, (a, b) in enumerate(zip(ours, golden)): + _differences(f"{path}[{index}]", a, b, out) + elif ours != golden: + out.append(f"{path}: {ours!r} != {golden!r}") + + +def _use_compat_class(design: dict) -> int: + """Point the node-type block at the class instead of the templates.""" + swapped = 0 + for entry in design.get("node_type_keys") or []: + block = entry.get("ip_addressing") if isinstance(entry, dict) else None + if isinstance(block, dict) and any(str(v).endswith(".j2") for v in block.values()): + entry["ip_addressing"] = { + "python_module": "function.avd_compat", + "python_class_name": "AvdIpAddressingV2Spine", + } + swapped += 1 + return swapped + + +def _point_at(design: dict, pool_file: Path) -> bool: + """Redirect `pools_file` at a copy of the pool. + + ⚠ When `pools_file` is set it is used **verbatim**, relative to the current + working directory — it is *not* joined to the PoolManager's output_dir, which + only supplies the default path. Getting that wrong silently assigns fresh IDs + and every address moves. + """ + node_id = (design.get("fabric_numbering") or {}).get("node_id") + if isinstance(node_id, dict) and node_id.get("pools_file"): + node_id["pools_file"] = str(pool_file) + return True + return False + + +def test_the_pool_and_the_compat_class_reproduce_avds_golden() -> None: + if not SCENARIO.is_dir(): + pytest.skip("AVD submodule not initialised") + + from pyavd.api.pool_manager import PoolManager + + from function.engine import render_structured_configs + + fabric = migrate(SCENARIO, collections=COLLECTIONS)[0] + assert sum(_use_compat_class(i.design) for i in fabric.inputs) == 1, ( + "expected exactly one node-type block pinning .j2 addressing" + ) + + with tempfile.TemporaryDirectory() as workdir: + # A copy, so a run can never write into AVD's own tree. + pool_file = Path(workdir) / POOL.name + shutil.copy(SCENARIO / POOL, pool_file) + assert any(_point_at(i.design, pool_file) for i in fabric.inputs), ( + "the scenario should still name a pools_file" + ) + + rendered = render_structured_configs( + resolve(fabric.inputs), pool_manager=PoolManager(Path(workdir)) + ) + + golden_dir = SCENARIO / "intended" / "structured_configs" + differences: list[str] = [] + compared = 0 + for hostname, structured in sorted(rendered.items()): + target = golden_dir / f"{hostname}.yml" + if not target.is_file(): + continue + compared += 1 + _differences(hostname, structured, + yaml.safe_load(target.read_text()) or {}, differences) + + assert compared == 26, f"expected AVD's 26 devices, compared {compared}" + assert not differences, ( + f"{len(differences)} differences: {differences[:5]}" + ) + + +def test_the_compat_class_only_changes_p2p_uplinks() -> None: + """Everything else falls through to AVD's own implementation, so selecting + the class cannot quietly move an address it was not written for.""" + from pyavd.api.ip_addressing import AvdIpAddressing + + from function.avd_compat import AvdIpAddressingV2Spine + + overridden = { + name for name in vars(AvdIpAddressingV2Spine) + if not name.startswith("_") and callable(getattr(AvdIpAddressingV2Spine, name)) + } + assert overridden == {"p2p_uplinks_ip", "p2p_uplinks_peer_ip"}, overridden + assert issubclass(AvdIpAddressingV2Spine, AvdIpAddressing) diff --git a/tests/test_e2e_migrated_corpus.py b/tests/test_e2e_migrated_corpus.py new file mode 100644 index 0000000..553bce2 --- /dev/null +++ b/tests/test_e2e_migrated_corpus.py @@ -0,0 +1,160 @@ +"""What `avd-migrate` emits applies to a cluster and renders AVD's own output. + +Requires a cluster from `scripts/kind-up.sh`: + + uv run pytest -m e2e tests/test_e2e_migrated_corpus.py -s + +The offline suite proves the migration reproduces Ansible's hostvars and that +pyavd turns them into AVD's golden. Neither of those meets the API server. This +does the whole chain -- AVD inventory -> XRs -> Crossplane -> rendered config -- +and compares the result against AVD's checked-in golden. + +⚠ It exists because a hand-written fabric is not the same shape as a migrated +one. The first e2e here carried `spec.design`, passed, and hid that the Fabric +XRD marked `design` **required** -- so every Fabric the migration emits, which +names its inputs in `spec.requires` and has no `design` at all, was rejected by +the API server. No offline test could see it: they drive RunFunction directly. + +No devices are pushed to: these fabrics have no `spec.push`, so nothing boots. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import tempfile +import time +from pathlib import Path + +import pytest +import yaml + +pytestmark = pytest.mark.e2e + +CTX = os.getenv("AVD_KUBE_CONTEXT", "kind-avd") +# ⚠ One namespace per scenario, and it is not tidiness. `single-dc-l3ls`, +# `single-dc-l3ls-ipv6` and `single-dc-multipod-l3ls` all name their devices +# `dc1-leaf1a`, `dc1-spine1` and so on. Applied side by side, a lookup by +# `avd.netclab.dev/device=` label matched whichever came first and compared one +# scenario's render against another's golden -- 420 differences that said +# nothing about the code. +NS_PREFIX = "migrated" +EXAMPLES = Path("avd/ansible_collections/arista/avd/examples") +COLLECTIONS = Path("avd").resolve() +TIMEOUT = 300 + +# Every bundled example. They render clean offline, so anything that fails here +# is about the cluster -- which is the whole point of running them here. +# `campus-fabric` and `l2ls-fabric` write their hostnames in capitals, which is +# what found the RFC 1123 bug; `cv-pathfinder` carries ansible-vault and Jinja, +# resolved by the migration before the XRs are written. +SCENARIOS = [ + "single-dc-l3ls", + "single-dc-l3ls-ipv6", + "single-dc-multipod-l3ls", + "dual-dc-l3ls", + "l2ls-fabric", + "campus-fabric", + "isis-ldp-ipvpn", + "cv-pathfinder", +] + + +def _kubectl(namespace: str, *args: str, check: bool = True) -> str: + proc = subprocess.run( + ["kubectl", "--context", CTX, "-n", namespace, *args], + capture_output=True, text=True, check=check, + ) + return proc.stdout.strip() + + +def _differences(path: str, ours: object, golden: object, out: list[str]) -> None: + if type(ours) is not type(golden) and not ( + isinstance(ours, (int, float)) and isinstance(golden, (int, float)) + ): + out.append(f"{path}: type {type(ours).__name__} != {type(golden).__name__}") + elif isinstance(golden, dict): + assert isinstance(ours, dict) + for key in sorted(set(ours) | set(golden)): + if key not in ours: + out.append(f"{path}.{key}: only in golden") + elif key not in golden: + out.append(f"{path}.{key}: only in ours") + else: + _differences(f"{path}.{key}", ours[key], golden[key], out) + elif isinstance(golden, list): + assert isinstance(ours, list) + if len(ours) != len(golden): + out.append(f"{path}: list len {len(ours)} != {len(golden)}") + for index, (a, b) in enumerate(zip(ours, golden)): + _differences(f"{path}[{index}]", a, b, out) + elif ours != golden: + out.append(f"{path}: {ours!r} != {golden!r}") + + +def _wait_ready(namespace: str, fabric: str) -> str: + deadline = time.monotonic() + TIMEOUT + last = "" + while time.monotonic() < deadline: + last = _kubectl( + namespace, "get", "fabric", fabric, + "-o", 'jsonpath={.status.conditions[?(@.type=="Ready")].status}', + check=False, + ) + if last == "True": + return last + time.sleep(5) + pytest.fail(f"{fabric} never went Ready within {TIMEOUT}s; last saw {last!r}") + + +@pytest.mark.parametrize("scenario", SCENARIOS) +def test_migrated_xrs_render_the_golden_on_a_cluster(scenario: str) -> None: + root = EXAMPLES / scenario + if not root.is_dir(): + pytest.skip("AVD submodule not initialised") + + namespace = f"{NS_PREFIX}-{scenario}" + subprocess.run( + ["kubectl", "--context", CTX, "apply", "-f", "-"], + input=yaml.safe_dump({"apiVersion": "v1", "kind": "Namespace", + "metadata": {"name": namespace}}), + capture_output=True, text=True, check=True, + ) + + with tempfile.TemporaryDirectory() as out: + subprocess.run( + ["uv", "run", "avd-migrate", str(root), "--emit", out, + "--namespace", namespace, "--collections", str(COLLECTIONS)], + capture_output=True, text=True, check=True, + ) + manifests = sorted(Path(out).glob("*.yaml")) + assert len(manifests) == 1, f"expected one fabric, emitted {manifests}" + subprocess.run( + ["kubectl", "--context", CTX, "apply", "-f", str(manifests[0])], + capture_output=True, text=True, check=True, + ) + fabric = yaml.safe_load(manifests[0].read_text().split("---")[-1])["metadata"]["name"] + + _wait_ready(namespace, fabric) + + golden_dir = root / "intended" / "structured_configs" + differences: list[str] = [] + compared = 0 + for golden_file in sorted(golden_dir.glob("*.yml")): + hostname = golden_file.stem + raw = _kubectl( + namespace, "get", "device", "-l", f"avd.netclab.dev/device={hostname}", + "-o", "jsonpath={.items[0].spec.structuredConfig}", check=False, + ) + if not raw: + differences.append(f"{hostname}: no Device composed") + continue + compared += 1 + _differences(hostname, json.loads(raw), + yaml.safe_load(golden_file.read_text()) or {}, differences) + + assert compared == len(list(golden_dir.glob("*.yml"))), ( + f"{scenario}: rendered {compared} of {len(list(golden_dir.glob('*.yml')))} devices" + ) + assert not differences, f"{scenario}: {len(differences)} differences: {differences[:5]}" diff --git a/tests/test_e2e_node_id_pool.py b/tests/test_e2e_node_id_pool.py new file mode 100644 index 0000000..ccbb62b --- /dev/null +++ b/tests/test_e2e_node_id_pool.py @@ -0,0 +1,221 @@ +"""A Fabric keeps its node-ID pool on a cluster, proven against AVD's own output. + +Requires a cluster from `scripts/kind-up.sh`: + + uv run pytest -m e2e tests/test_e2e_node_id_pool.py -s + +The scenario is **AVD's own**, not one written for the test. +`eos_designs-twodc-5stage-clos` is the only fabric in AVD's corpus that turns on +`pool_manager`, and it ships the assignments AVD generated -- 26 of them, in +`intended/data/test-ids.yml`, beside the golden configs those assignments +produced. That makes the strongest possible assertion available: **seed the pool +and the render must reproduce the golden exactly**. If the seed is ignored, or +the pool is not read back, ids move and every derived address moves with them. + +Two substitutions the migration deliberately does not make, both stated by +`avd-migrate` in its own output rather than done silently: + +* the design pins two `.j2` addressing templates, and pyavd implements no Jinja + templating -- swapped here for `function.avd_compat`, which is the route AVD's + schema offers instead (`python_module`) and which this image ships; +* the assignments live in a file that does not travel with a migration -- carried + in as a seed ConfigMap. + +No devices are pushed to: the migrated fabric has no `spec.push`, so 26 devices +cost no cEOS at all. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import time +from pathlib import Path + +import pytest +import yaml + +pytestmark = [ + pytest.mark.e2e, + # ⚠ RED, knowingly, and recorded rather than hidden. Both assertions below + # hold offline -- `tests/test_avd_compat.py` renders this scenario 26 of 26 + # clean against the same golden -- and fail on a cluster with 12 differences, + # every one a `service_profile` key present in ours and absent in the golden, + # on the P2P links between super-spines. + # + # Not diagnosed. The lead is that the scenario's two fabrics share 47 of 48 + # input XR names while their emitted contents differ, so one apply can + # overwrite the other's inputs. See "OPEN -- twodc renders 12 differences on + # a cluster and 0 offline" in .claude/STATE.md. + # + # strict, so the day it starts passing this fails and says to drop the mark. + pytest.mark.xfail(strict=True, reason="twodc: 12 service_profile diffs on a cluster, 0 offline"), +] + +CTX = os.getenv("AVD_KUBE_CONTEXT", "kind-avd") +NS = "twodc" +SCENARIO = Path( + "avd/ansible_collections/arista/avd/extensions/molecule/eos_designs-twodc-5stage-clos" +) +COLLECTIONS = Path("avd").resolve() +SEED_NAME = "twodc-seed-ids" +TIMEOUT = 300 + + +def _kubectl(*args: str, check: bool = True) -> str: + proc = subprocess.run( + ["kubectl", "--context", CTX, "-n", NS, *args], + capture_output=True, text=True, check=check, + ) + return proc.stdout.strip() + + +def _apply(document: object) -> None: + subprocess.run( + ["kubectl", "--context", CTX, "apply", "-f", "-"], + input=yaml.safe_dump_all(document if isinstance(document, list) else [document]), + capture_output=True, text=True, check=True, + ) + + +def _differences(path: str, ours: object, golden: object, out: list[str]) -> None: + if type(ours) is not type(golden) and not ( + isinstance(ours, (int, float)) and isinstance(golden, (int, float)) + ): + out.append(f"{path}: type {type(ours).__name__} != {type(golden).__name__}") + elif isinstance(golden, dict): + assert isinstance(ours, dict) + for key in sorted(set(ours) | set(golden)): + if key not in ours: + out.append(f"{path}.{key}: only in golden") + elif key not in golden: + out.append(f"{path}.{key}: only in ours") + else: + _differences(f"{path}.{key}", ours[key], golden[key], out) + elif isinstance(golden, list): + assert isinstance(ours, list) + if len(ours) != len(golden): + out.append(f"{path}: list len {len(ours)} != {len(golden)}") + for index, (a, b) in enumerate(zip(ours, golden)): + _differences(f"{path}[{index}]", a, b, out) + elif ours != golden: + out.append(f"{path}: {ours!r} != {golden!r}") + + +def _use_compat_class(document: dict) -> int: + swapped = 0 + for entry in (document.get("spec", {}).get("design", {}).get("node_type_keys") or []): + block = entry.get("ip_addressing") if isinstance(entry, dict) else None + if isinstance(block, dict) and any(str(v).endswith(".j2") for v in block.values()): + entry["ip_addressing"] = { + "python_module": "function.avd_compat", + "python_class_name": "AvdIpAddressingV2Spine", + } + swapped += 1 + return swapped + + +@pytest.fixture(scope="module") +def fabric() -> str: + if not SCENARIO.is_dir(): + pytest.skip("AVD submodule not initialised") + + from function import pools + + # A namespace still Terminating from a previous run refuses new objects, and + # re-running a test right after cleaning up is the normal case. + deadline = time.monotonic() + 60 + while True: + try: + _apply({"apiVersion": "v1", "kind": "Namespace", "metadata": {"name": NS}}) + break + except subprocess.CalledProcessError: + if time.monotonic() > deadline: + raise + time.sleep(3) + _apply({ + "apiVersion": "v1", "kind": "ConfigMap", + "metadata": {"name": SEED_NAME, "namespace": NS}, + "data": {pools.DATA_KEY: (SCENARIO / "intended/data/test-ids.yml").read_text()}, + }) + + # ⚠ The first play, by index, not the first file by name. This scenario runs + # eos_designs twice over the same hosts and the **second** play sets + # `avd_digital_twin_mode: true`, rendering a different config into a + # different golden directory. Sorting the emitted filenames picked that one + # and compared it against the other's golden -- 12 differences that were + # entirely the test's fault. + from function.migrate import migrate, to_manifests + + fabrics = migrate(SCENARIO, collections=COLLECTIONS) + first = min(fabrics, key=lambda f: (f.play.playbook, f.play.index)) + documents = to_manifests(first, namespace=NS) + + swapped = sum(_use_compat_class(d) for d in documents if d["kind"] == "NodeSet") + assert swapped == 1, f"expected one node-type block pinning .j2 addressing, got {swapped}" + + name = "" + for document in documents: + if document["kind"] == "Fabric": + document["spec"]["nodeIdPool"] = {"seedConfigMapName": SEED_NAME} + name = document["metadata"]["name"] + assert name, "no Fabric emitted" + + _apply(documents) + deadline = time.monotonic() + TIMEOUT + while time.monotonic() < deadline: + if _kubectl("get", "fabric", name, "-o", + 'jsonpath={.status.conditions[?(@.type=="Ready")].status}', + check=False) == "True": + return name + time.sleep(5) + pytest.fail(f"{name} never went Ready; status: {_kubectl('get', 'fabric', name, '-o', 'jsonpath={.status}', check=False)[:400]}") + + +def test_the_seeded_fabric_renders_avds_golden(fabric: str) -> None: + """⚠ The strongest assertion available, and it needs every piece at once. + + The golden was produced with the assignments in the seed. Reproducing it + means the seed was read, the pool was composed and read back, the compat + class computed the v2.x addresses, and the migration carried the rest -- + any one of those failing moves an address and fails this. + """ + golden_dir = SCENARIO / "intended" / "structured_configs" + goldens = sorted(golden_dir.glob("*.yml")) + differences: list[str] = [] + compared = 0 + for golden_file in goldens: + hostname = golden_file.stem + raw = _kubectl( + "get", "device", "-l", f"avd.netclab.dev/device={hostname}", + "-o", "jsonpath={.items[0].spec.structuredConfig}", check=False, + ) + if not raw: + differences.append(f"{hostname}: no Device composed") + continue + compared += 1 + _differences(hostname, json.loads(raw), + yaml.safe_load(golden_file.read_text()) or {}, differences) + + assert compared == len(goldens), f"rendered {compared} of {len(goldens)} devices" + assert not differences, f"{len(differences)} differences: {differences[:5]}" + + +def test_the_fabric_kept_the_seeded_assignments(fabric: str) -> None: + """The pool is composed from the seed, not started over beside it.""" + from function import pools + + name = _kubectl( + "get", "cm", "-l", "avd.netclab.dev/fabric=TWODC_5STAGE_CLOS", + "-o", "jsonpath={.items[*].metadata.name}", + ).split() + assert name, "the fabric composed no pool" + + body = yaml.safe_load( + _kubectl("get", "cm", name[0], "-o", rf"jsonpath={{.data.{pools.DATA_KEY.replace('.', chr(92) + '.')}}}") + ) or {} + seeded = yaml.safe_load((SCENARIO / "intended/data/test-ids.yml").read_text()) or {} + assert body.get("node_id_pools") == seeded.get("node_id_pools"), ( + "the composed pool differs from the seed it was given" + ) diff --git a/tests/test_fabric_collect.py b/tests/test_fabric_collect.py index 8adc70e..5a2eca3 100644 --- a/tests/test_fabric_collect.py +++ b/tests/test_fabric_collect.py @@ -179,3 +179,312 @@ def test_input_kinds_reconcile_and_report_their_keys(kind: str) -> None: assert not any(r.severity == fnv1.SEVERITY_FATAL for r in rsp.results) status = resource.struct_to_dict(rsp.desired.composite.resource).get("status", {}) assert status["keys"] == ["ntp_settings", "type"] + + +# --- node-ID pools ---------------------------------------------------------- + +def _pooled(*, static_id: int | None) -> dict: + """The spine design with node IDs coming from a pool. + + ⚠ `static_id` is the whole point of having two variants. AVD *reserves* an + id written on the node, so a pool holding a different number for that device + is refused rather than applied. Only a device with no id of its own takes + what the pool holds. + """ + node = {"name": "spine1", "bgp_as": 65100} + if static_id is not None: + node["id"] = static_id + return { + "type": "spine", + "spine": { + "defaults": {"loopback_ipv4_pool": "10.255.0.0/27"}, + "nodes": [node], + }, + "fabric_numbering": { + "node_id": {"algorithm": "pool_manager", "pools_file": "intended/data/x-ids.yml"} + }, + } + + +POOLED = _pooled(static_id=None) + + +def _pool_configmap(pool: str) -> dict: + from function import pools + + return { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": {"name": "fabric-id-pool", "namespace": "avd"}, + "data": {pools.DATA_KEY: pool}, + } + + +def _with_observed(req: fnv1.RunFunctionRequest, name: str, obj: dict) -> fnv1.RunFunctionRequest: + req.observed.resources[name].resource.CopyFrom(resource.dict_to_struct(obj)) + return req + + +def test_a_fabric_asking_for_a_pool_composes_one() -> None: + """`pool_manager` keeps its assignments in a file. There is no file in a + cluster, so the Fabric composes a ConfigMap and reads it back next time.""" + from function import pools + + rsp = _run(_request(_fabric(requires=[], design=POOLED))) + + assert not any(r.severity == fnv1.SEVERITY_FATAL for r in rsp.results), rsp.results + assert pools.RESOURCE_NAME in rsp.desired.resources, "no pool was composed" + + composed = resource.struct_to_dict(rsp.desired.resources[pools.RESOURCE_NAME].resource) + body = composed["data"][pools.DATA_KEY] + assert "spine1" in body, body + # `child_name` appends a hash, as it does for every composed resource. + assert composed["metadata"]["name"].startswith("fabric-id-pool") + + +def test_a_fabric_without_a_pool_composes_none() -> None: + """Nothing is created for a fabric that never asked -- an empty object with + a warning about renumbering would be worse than no object.""" + from function import pools + + rsp = _run(_request(_fabric(requires=[], design=SPINES["spec"]["design"]))) + assert pools.RESOURCE_NAME not in rsp.desired.resources + + +def test_assignments_survive_the_next_reconcile() -> None: + """Two identical reconciles produce the same pool and the same config. + + ⚠ **Weak on its own, deliberately kept.** Assignment is deterministic from + the device set, so this passes even when the observed pool is ignored + entirely -- verified by making `observed_pool` return nothing. It guards + idempotency; `test_the_pool_decides_the_ids` is what proves the ConfigMap is + read at all. + """ + from function import pools + + first = _run(_request(_fabric(requires=[], design=POOLED))) + pool = resource.struct_to_dict( + first.desired.resources[pools.RESOURCE_NAME].resource + )["data"][pools.DATA_KEY] + + second = _run( + _with_observed( + _request(_fabric(requires=[], design=POOLED)), + pools.RESOURCE_NAME, + _pool_configmap(pool), + ) + ) + again = resource.struct_to_dict( + second.desired.resources[pools.RESOURCE_NAME].resource + )["data"][pools.DATA_KEY] + + assert again == pool, "the pool moved between two identical reconciles" + + def rendered(rsp: fnv1.RunFunctionResponse) -> str: + cm = resource.struct_to_dict(rsp.desired.resources["spine1"].resource) + return str(cm) + + assert rendered(second) == rendered(first), "the device changed although its ID did not" + + +def test_the_designs_own_pools_file_is_not_followed() -> None: + """It names a path relative to somebody's working directory. Honouring it in + a cluster would read nothing and quietly assign a fresh set of IDs.""" + from function import pools + + rsp = _run(_request(_fabric(requires=[], design=POOLED))) + body = resource.struct_to_dict( + rsp.desired.resources[pools.RESOURCE_NAME].resource + )["data"][pools.DATA_KEY] + assert body.strip(), "the pool came back empty -- the file was read from the wrong place" + + +def test_the_pool_decides_the_ids() -> None: + """The pool is read, not merely written. + + A fabric handed a pool that assigns `spine1` the id 7 must render the device + with id 7 -- AVD reserves an existing assignment rather than handing out the + next free number. Without this, every test here passes on a function that + throws the ConfigMap away and reassigns from scratch, because a fresh pool + over an unchanged device set produces the same numbers. + """ + from function import pools + + fresh = _run(_request(_fabric(requires=[], design=POOLED))) + assert _loopback(fresh) == "10.255.0.1/32", "unexpected baseline" + + + moved = _pool_configmap( + "node_id_pools:\n" + " fabric_name=FABRIC/type=spine:\n" + " hostname=spine1: 7\n" + ) + rsp = _run( + _with_observed( + _request(_fabric(requires=[], design=POOLED)), pools.RESOURCE_NAME, moved + ) + ) + + assert _loopback(rsp) == "10.255.0.7/32", ( + "the device did not take the id the pool holds -- the pool was not read" + ) + kept = resource.struct_to_dict( + rsp.desired.resources[pools.RESOURCE_NAME].resource + )["data"][pools.DATA_KEY] + assert "hostname=spine1: 7" in kept, kept + + +def _loopback(rsp: fnv1.RunFunctionResponse) -> str: + device = resource.struct_to_dict(rsp.desired.resources["spine1"].resource) + loopbacks = device["spec"]["structuredConfig"].get("loopback_interfaces") or [] + return next(i["ip_address"] for i in loopbacks if i["name"] == "Loopback0") + + +def test_an_id_written_on_the_node_outranks_the_pool() -> None: + """AVD reserves a statically set id; the pool does not get to move it. + + Worth a test because the opposite is the natural expectation -- "the pool + assigns ids" -- and getting it backwards would mean quietly renumbering a + device whose id somebody wrote down on purpose. + """ + from function import pools + + design = _pooled(static_id=1) + rsp = _run( + _with_observed( + _request(_fabric(requires=[], design=design)), + pools.RESOURCE_NAME, + _pool_configmap( + "node_id_pools:\n" + " fabric_name=FABRIC/type=spine:\n" + " hostname=spine1: 7\n" + ), + ) + ) + + assert _loopback(rsp) == "10.255.0.1/32", "the pool overrode an id set on the node" + + +def _seeded(design: dict, name: str = "old-ids") -> dict: + xr = _fabric(requires=[], design=design) + xr["spec"]["nodeIdPool"] = {"seedConfigMapName": name} + return xr + + +def test_a_named_seed_gates_the_first_reconcile() -> None: + """⚠ Without this gate the seed could never work. + + Requirements are answered on the *next* reconcile, so the first one arrives + with nothing. Rendering then would assign a fresh set of IDs and compose + them as the pool, and the seed would be read into a fabric that had already + renumbered itself. + """ + from function import pools + + rsp = _run(_request(_seeded(POOLED))) + + assert pools.RESOURCE_NAME not in rsp.desired.resources, "a pool was composed anyway" + assert not rsp.desired.resources, "nothing may be composed before the seed arrives" + assert _condition(rsp, "InputsResolved").reason == "WaitingForSeed" + assert pools.SEED_NAME in rsp.requirements.resources + + +def test_a_seed_that_does_not_exist_is_fatal() -> None: + """Named and absent is an error, not an empty pool: carrying on renumbers + every device, which is the one thing the field exists to prevent.""" + rsp = _run(_request(_seeded(POOLED), required={"id-pool-seed": []})) + + assert any(r.severity == fnv1.SEVERITY_FATAL for r in rsp.results) + assert "renumber" in " ".join(r.message for r in rsp.results) + + +def test_a_seed_supplies_the_first_pool() -> None: + """The migration case: a fabric that was already running elsewhere keeps the + IDs AVD gave it, instead of starting over.""" + from function import pools + + seed = { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": {"name": "old-ids", "namespace": "avd"}, + "data": { + pools.DATA_KEY: "node_id_pools:\n" + " fabric_name=FABRIC/type=spine:\n" + " hostname=spine1: 9\n" + }, + } + rsp = _run(_request(_seeded(POOLED), required={"id-pool-seed": [seed]})) + + assert _loopback(rsp) == "10.255.0.9/32", "the seeded assignment was not used" + kept = resource.struct_to_dict( + rsp.desired.resources[pools.RESOURCE_NAME].resource + )["data"][pools.DATA_KEY] + assert "hostname=spine1: 9" in kept + + +def test_a_seed_never_overrides_a_pool_the_fabric_already_has() -> None: + """It seeds, it does not steer. Once the fabric keeps its own assignments, + an old ConfigMap left lying around must not pull them back.""" + from function import pools + + seed = { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": {"name": "old-ids", "namespace": "avd"}, + "data": { + pools.DATA_KEY: "node_id_pools:\n" + " fabric_name=FABRIC/type=spine:\n" + " hostname=spine1: 9\n" + }, + } + req = _request(_seeded(POOLED), required={"id-pool-seed": [seed]}) + _with_observed( + req, + pools.RESOURCE_NAME, + _pool_configmap( + "node_id_pools:\n fabric_name=FABRIC/type=spine:\n hostname=spine1: 3\n" + ), + ) + rsp = _run(req) + + assert _loopback(rsp) == "10.255.0.3/32", "the seed overrode the fabric's own pool" + + +# --- hostnames a Kubernetes name cannot spell ------------------------------- + +def _spines(*names: str) -> dict: + return { + "type": "spine", + "spine": { + "defaults": {"loopback_ipv4_pool": "10.255.0.0/27"}, + "nodes": [{"name": n, "id": i + 1, "bgp_as": 65100 + i} + for i, n in enumerate(names)], + }, + } + + +def test_an_uppercase_hostname_still_composes() -> None: + """AVD hostnames are free text, and two of its eight bundled examples -- + `campus-fabric` and `l2ls-fabric` -- write them entirely in capitals. + + ⚠ Found on a cluster, not here: a composed resource named after one is + rejected outright (*"invalid name ... Must be a valid RFC 1123 subdomain + name"*) and the whole fabric fails to compose. Offline tests never met it + because they never reach an API server. + """ + rsp = _run(_request(_fabric(requires=[], design=_spines("SPINE2")))) + + assert not any(r.severity == fnv1.SEVERITY_FATAL for r in rsp.results), rsp.results + device = resource.struct_to_dict(rsp.desired.resources["SPINE2"].resource) + assert device["metadata"]["name"].startswith("fabric-spine2-"), device["metadata"]["name"] + # The hostname itself is untouched -- only the object's name is spelled + # differently, or the render would be for a device that does not exist. + assert device["spec"]["hostname"] == "SPINE2" + + +def test_two_hostnames_needing_one_object_name_are_refused() -> None: + """One Device standing in for two switches would push one config to both.""" + rsp = _run(_request(_fabric(requires=[], design=_spines("SPINE1", "spine1")))) + + assert any(r.severity == fnv1.SEVERITY_FATAL for r in rsp.results), rsp.results + assert "rename one" in " ".join(r.message for r in rsp.results) diff --git a/tests/test_kinds_equivalence.py b/tests/test_kinds_equivalence.py index 9c78061..6265fb0 100644 --- a/tests/test_kinds_equivalence.py +++ b/tests/test_kinds_equivalence.py @@ -65,10 +65,14 @@ "ansible_only": "custom ip_addressing template -- pyavd implements no Jinja templating", "evpn_underlay_ebgp_overlay_ebgp": "custom interface_descriptions templates -- " "pyavd implements no Jinja templating", - # Persistent mutable state: an ID pool must survive reconciliation or devices - # get new identifiers every pass. `pyavd.api.pool_manager` is public; what is - # missing is somewhere for a Fabric to keep the pool. - "eos_designs-twodc-5stage-clos": "fabric_numbering pool_manager -- makes a Fabric stateful", + # Two blockers, not one: `pool_manager` needs a pool that survives + # reconciliation, and the design also pins .j2 addressing. The *migrated* + # design always fails here, because it faithfully carries both. + # ⚠ Both are solved and proven in `tests/test_avd_compat.py`, which renders + # this scenario clean against AVD's golden with the pool supplied and the + # templates replaced by the class AVD's schema offers instead. What is left + # is somewhere for a Fabric to *keep* the pool. + "eos_designs-twodc-5stage-clos": "pool_manager + .j2 addressing -- see test_avd_compat", # Code, not data: a function image is immutable and must not load arbitrary # Python. ⚠ And this scenario is not a fabric -- 59 unrelated feature groups # under one play, with six different `fabric_name` values.