diff --git a/.gitignore b/.gitignore index 046f8b11..7d088d6b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ *.egg-info +.venv-sonic-e2e/ *.pyc *.swp __pycache__ diff --git a/.zuul.yaml b/.zuul.yaml index 6e3fcd70..145287f0 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -140,6 +140,37 @@ pre-run: playbooks/pre.yml run: playbooks/test-integration.yml +# End-to-end golden test for the SONiC config generator: provisions NetBox +# with a docker compose stack, seeds it from the in-repo fixtures under +# tests/e2e/scenario/, runs sync_sonic() and compares the exported config_db +# files against tests/e2e/golden/. In check the job only runs when files +# that can change the generated output (or the harness itself) are touched; +# periodic-daily runs it unconditionally. netbox-manager is still pulled at +# tip-of-main via required-projects because it remains the seeding tool, so +# a Depends-On is honored for its code -- but its example/ seed data is no +# longer used, so this job no longer detects drift in that data. +- job: + name: python-osism-sonic-e2e + nodeset: ubuntu-noble + pre-run: playbooks/pre-sonic-e2e.yml + run: playbooks/test-sonic-e2e.yml + required-projects: + - osism/netbox-manager + timeout: 2400 + files: + - ^\.zuul\.yaml$ + - ^Makefile$ + - ^Pipfile\.lock$ + - ^files/sonic/.* + - ^osism/settings\.py$ + - ^osism/tasks/conductor/.* + - ^osism/utils/.* + - ^playbooks/(pre-|test-)sonic-e2e\.yml$ + - ^requirements\.ansible\.txt$ + - ^requirements\.txt$ + - ^setup\.cfg$ + - ^tests/e2e/.* + - project: merge-mode: squash-merge default-branch: main @@ -154,6 +185,7 @@ - python-osism-test-setup - python-osism-unit-tests - python-osism-integration-tests + - python-osism-sonic-e2e periodic-daily: jobs: - flake8 @@ -163,6 +195,7 @@ - python-osism-test-setup - python-osism-unit-tests - python-osism-integration-tests + - python-osism-sonic-e2e periodic-midnight: jobs: - container-image-python-osism-push diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..f632b2eb --- /dev/null +++ b/Makefile @@ -0,0 +1,30 @@ +NETBOX_MANAGER_DIR ?= $(abspath ../netbox-manager) + +# SONiC config-generation E2E golden test (see tests/e2e/sonic_golden_test.sh). + +# Full cycle: start the NetBox compose stack (an existing stack is reused +# and left in place), seed, generate, compare against tests/e2e/golden/. +sonic-e2e: + NETBOX_MANAGER_DIR=$(NETBOX_MANAGER_DIR) tests/e2e/sonic_golden_test.sh + +# Regenerate the golden files after an intentional generator change, +# then review and commit the diff. +sonic-e2e-regen: + NETBOX_MANAGER_DIR=$(NETBOX_MANAGER_DIR) tests/e2e/sonic_golden_test.sh --regenerate + +# Start the NetBox stack and leave it running for debugging. Export a +# NETBOX_TOKEN beforehand to get a known API token minted. +sonic-e2e-up: + tests/e2e/deploy_netbox.sh + +# Stop the NetBox stack and remove its volumes. +sonic-e2e-down: + docker compose -f tests/e2e/compose.yaml down --volumes --remove-orphans + +# Report config_db table coverage of the golden set (tests/e2e/coverage.py). +# A reporting tool only -- not part of the gating check, which stays the +# golden comparison run by sonic-e2e above. +sonic-e2e-coverage: + pipenv run python -m tests.e2e.coverage + +.PHONY: sonic-e2e sonic-e2e-regen sonic-e2e-up sonic-e2e-down sonic-e2e-coverage diff --git a/README.md b/README.md index 1c9cea93..916a16f0 100644 --- a/README.md +++ b/README.md @@ -44,3 +44,60 @@ REDIS_HOST=localhost REDIS_DB=15 pipenv run pytest tests/integration > above, or `OSISM_ALLOW_DEFAULT_REDIS_DB=1` if the Redis itself is disposable. > `REDIS_DB` moves the direct client, the Celery broker and the result backend > together. + +## Running the SONiC E2E golden test + +The end-to-end test in `tests/e2e/` provisions NetBox with a docker compose +stack, seeds it from the fixtures in `tests/e2e/scenario/`, generates the SONiC +`config_db.json` files and compares them against the goldens in +`tests/e2e/golden/`. Besides the development dependencies it needs docker with +the compose plugin, `openssl`, and a `netbox-manager` checkout for the seeding +CLI — a sibling directory by default, `NETBOX_MANAGER_DIR` otherwise. + +``` +pipenv install --dev +make sonic-e2e +``` + +A cold run takes roughly ten minutes, most of it starting NetBox. To iterate +without paying that each time, bring the stack up separately and leave it +running: + +``` +make sonic-e2e-up # start NetBox and leave it up; sonic-e2e reuses it +make sonic-e2e-down # stop it again and remove its volumes +``` + +After an intentional generator change, rewrite the goldens and review the diff +before committing it. Regeneration deliberately refuses to run against a stack +left over from an earlier run, because applying the fixtures over a populated +database can produce goldens that CI — which always starts fresh — would not +reproduce: + +``` +make sonic-e2e-down +make sonic-e2e-regen +``` + +How much of the generated config the golden set actually covers is reported +separately, because nothing in CI reports it: + +``` +make sonic-e2e-coverage +``` + +That compares the `config_db` tables the generator can emit against the tables +that are non-empty in at least one golden, and names any that no golden covers. +It exits non-zero while that list is non-empty, so it is worth running after +adding a scenario to confirm the new tables landed. It gates nothing on its own +— the golden comparison above is the only check that fails a run. + +`tests/e2e/sonic_golden_test.sh` documents the remaining environment overrides +(`NETBOX_PORT`, `KEEP_STACK`, `SEED_PARALLEL` and the regeneration escape +hatch). + +> **Warning:** Seeding applies *every* file under +> `tests/e2e/scenario/resources/`, tracked or not, so a stray file there joins +> the fixture set — which either breaks the run or silently changes the +> goldens. Check that directory with `git status --ignored` before regenerating +> or debugging a mismatch. diff --git a/osism/settings.py b/osism/settings.py index effb911d..0a800c52 100644 --- a/osism/settings.py +++ b/osism/settings.py @@ -78,6 +78,10 @@ def read_secret(secret_name): SONIC_EXPORT_SUFFIX = os.getenv("SONIC_EXPORT_SUFFIX", "_config_db.json") SONIC_EXPORT_IDENTIFIER = os.getenv("SONIC_EXPORT_IDENTIFIER", "serial-number") +# Directory holding the per-HWSKU port_config .ini files (bundled in the +# repo under files/sonic/port_config and installed by the Dockerfile) +SONIC_PORT_CONFIG_PATH = os.getenv("SONIC_PORT_CONFIG_PATH", "/etc/sonic/port_config") + # SONiC ZTP firmware configuration # # The ZTP firmware install uses a dynamic-url built from diff --git a/osism/tasks/conductor/sonic/config_generator.py b/osism/tasks/conductor/sonic/config_generator.py index c66b2fb6..3d081619 100644 --- a/osism/tasks/conductor/sonic/config_generator.py +++ b/osism/tasks/conductor/sonic/config_generator.py @@ -508,7 +508,11 @@ def generate_sonic_config(device, hwsku, device_as_mapping=None, config_version= if breakout_info["breakout_cfgs"]: config["BREAKOUT_CFG"].update(breakout_info["breakout_cfgs"]) if breakout_info["breakout_ports"]: - config["BREAKOUT_PORTS"].update(breakout_info["breakout_ports"]) + # Project each entry to the SONiC schema ({port: {"master": }}); + # the full stash (declared/lanes/speed) stays in breakout_info for the + # PORT-building helpers and must not leak into the owned table. + for child, entry in breakout_info["breakout_ports"].items(): + config["BREAKOUT_PORTS"][child] = {"master": entry["master"]} # Add port channel configuration _add_portchannel_configuration(config, portchannel_info) @@ -609,33 +613,45 @@ def _add_port_configurations( port_speed = sonic_speed if port_name in breakout_info["breakout_ports"]: + bp = breakout_info["breakout_ports"][port_name] # Get the master port to determine original speed and lanes - master_port = breakout_info["breakout_ports"][port_name]["master"] + master_port = bp["master"] - # Override with individual breakout port speed from NetBox if available - if port_name in netbox_interfaces and netbox_interfaces[port_name]["speed"]: - port_speed = str(int(netbox_interfaces[port_name]["speed"])) - logger.debug( - f"Using NetBox speed {port_speed} Mbps for breakout port {port_name}" + if bp.get("declared"): + # Declared-mode stash is authoritative: it takes precedence over + # the NetBox-speed override and the inferred lane calculation. + port_speed = str(bp["speed"]) + port_lanes = bp["lanes"] + else: + # Override with individual breakout port speed from NetBox if available + if ( + port_name in netbox_interfaces + and netbox_interfaces[port_name]["speed"] + ): + port_speed = str(int(netbox_interfaces[port_name]["speed"])) + logger.debug( + f"Using NetBox speed {port_speed} Mbps for breakout port {port_name}" + ) + elif master_port in breakout_info["breakout_cfgs"]: + # Fallback to extracting speed from breakout mode + brkout_mode = breakout_info["breakout_cfgs"][master_port][ + "brkout_mode" + ] + if "10G" in brkout_mode: + port_speed = "10000" + elif "25G" in brkout_mode: + port_speed = "25000" + elif "50G" in brkout_mode: + port_speed = "50000" + elif "100G" in brkout_mode: + port_speed = "100000" + elif "200G" in brkout_mode: + port_speed = "200000" + + # Calculate individual lane for this breakout port + port_lanes = _calculate_breakout_port_lane( + port_name, master_port, port_config ) - elif master_port in breakout_info["breakout_cfgs"]: - # Fallback to extracting speed from breakout mode - brkout_mode = breakout_info["breakout_cfgs"][master_port]["brkout_mode"] - if "10G" in brkout_mode: - port_speed = "10000" - elif "25G" in brkout_mode: - port_speed = "25000" - elif "50G" in brkout_mode: - port_speed = "50000" - elif "100G" in brkout_mode: - port_speed = "100000" - elif "200G" in brkout_mode: - port_speed = "200000" - - # Calculate individual lane for this breakout port - port_lanes = _calculate_breakout_port_lane( - port_name, master_port, port_config - ) # Generate correct alias based on port name and speed interface_speed = int(port_speed) if port_speed else None @@ -817,32 +833,47 @@ def _add_missing_breakout_ports( for port_name in breakout_info["breakout_ports"]: if port_name not in config["PORT"]: # Get the master port to determine configuration - master_port = breakout_info["breakout_ports"][port_name]["master"] + bp = breakout_info["breakout_ports"][port_name] + master_port = bp["master"] - # Override with individual breakout port speed from NetBox if available - # Note: netbox_interfaces speeds are already normalized to Mbps - if port_name in netbox_interfaces and netbox_interfaces[port_name]["speed"]: - port_speed = str(int(netbox_interfaces[port_name]["speed"])) - logger.debug( - f"Using NetBox speed {port_speed} Mbps for missing breakout port {port_name}" - ) - elif master_port in breakout_info["breakout_cfgs"]: - # Fallback to extracting speed from breakout mode - brkout_mode = breakout_info["breakout_cfgs"][master_port]["brkout_mode"] - if "10G" in brkout_mode: - port_speed = "10000" - elif "25G" in brkout_mode: - port_speed = "25000" - elif "50G" in brkout_mode: - port_speed = "50000" - elif "100G" in brkout_mode: - port_speed = "100000" - elif "200G" in brkout_mode: - port_speed = "200000" + if bp.get("declared"): + port_speed = str(bp["speed"]) + port_lanes = bp["lanes"] + else: + # Override with individual breakout port speed from NetBox if available + # Note: netbox_interfaces speeds are already normalized to Mbps + if ( + port_name in netbox_interfaces + and netbox_interfaces[port_name]["speed"] + ): + port_speed = str(int(netbox_interfaces[port_name]["speed"])) + logger.debug( + f"Using NetBox speed {port_speed} Mbps for missing breakout port {port_name}" + ) + elif master_port in breakout_info["breakout_cfgs"]: + # Fallback to extracting speed from breakout mode + brkout_mode = breakout_info["breakout_cfgs"][master_port][ + "brkout_mode" + ] + if "10G" in brkout_mode: + port_speed = "10000" + elif "25G" in brkout_mode: + port_speed = "25000" + elif "50G" in brkout_mode: + port_speed = "50000" + elif "100G" in brkout_mode: + port_speed = "100000" + elif "200G" in brkout_mode: + port_speed = "200000" + else: + port_speed = "25000" # Default fallback else: port_speed = "25000" # Default fallback - else: - port_speed = "25000" # Default fallback + + # Calculate individual lane for this breakout port + port_lanes = _calculate_breakout_port_lane( + port_name, master_port, port_config + ) # Set admin_status based on connection or port channel membership admin_status = ( @@ -865,11 +896,6 @@ def _add_missing_breakout_ports( if master_port in port_config: port_index = port_config[master_port]["index"] - # Calculate individual lane for this breakout port - port_lanes = _calculate_breakout_port_lane( - port_name, master_port, port_config - ) - port_data = { "admin_status": admin_status, "alias": correct_alias, diff --git a/osism/tasks/conductor/sonic/constants.py b/osism/tasks/conductor/sonic/constants.py index 6cbdd010..4d330ef8 100644 --- a/osism/tasks/conductor/sonic/constants.py +++ b/osism/tasks/conductor/sonic/constants.py @@ -2,6 +2,8 @@ """Constants and mappings for SONiC configuration.""" +from osism import settings + # Tag to add AF L2VPN EVPN to BGP neighbor BGP_AF_L2VPN_EVPN_TAG = "bgp-af-l2vpn-evpn" @@ -87,7 +89,7 @@ } # Path to SONiC port configuration files -PORT_CONFIG_PATH = "/etc/sonic/port_config" +PORT_CONFIG_PATH = settings.SONIC_PORT_CONFIG_PATH # List of supported vendors SUPPORTED_VENDORS = [ diff --git a/osism/tasks/conductor/sonic/interface.py b/osism/tasks/conductor/sonic/interface.py index 71e2af41..ff9d1087 100644 --- a/osism/tasks/conductor/sonic/interface.py +++ b/osism/tasks/conductor/sonic/interface.py @@ -19,6 +19,51 @@ _port_config_cache: dict[str, dict[str, dict[str, str]]] = {} +def get_declared_breakout_modes(device): + cf = getattr(device, "custom_fields", None) + if not isinstance(cf, dict): + return {} + sp = cf.get("sonic_parameters") + if not isinstance(sp, dict): + return {} + bk = sp.get("breakout") + return bk if isinstance(bk, dict) else {} + + +_MODE_RE = re.compile(r"(\d+)x(\d+)G") + + +def _parse_breakout_mode(mode): + if not isinstance(mode, str): + return None + m = _MODE_RE.fullmatch(mode.strip()) + if not m: + return None + count, g = int(m.group(1)), int(m.group(2)) + if count < 2 or g <= 0: + return None + return count, g * 1000 + + +def _parse_lanes(lanes): + if not isinstance(lanes, str): + return [] + s = lanes.strip() + if not s: + return [] + try: + if "," in s: + parts = [p.strip() for p in s.split(",")] + return parts if all(p.isdigit() for p in parts) else [] + if "-" in s: + a, b = s.split("-", 1) + a, b = int(a), int(b) + return [str(n) for n in range(a, b + 1)] if a <= b else [] + return [s] if s.isdigit() else [] + except (ValueError, TypeError): + return [] + + def get_speed_from_port_type(port_type): """Get speed from port type when speed is not provided. @@ -641,6 +686,12 @@ def get_connected_interfaces(device, portchannel_info=None): return _get_connected_interfaces(device, portchannel_info) +def _breakout_child_names(master, count, lanes_per_child): + """Names a breakout of ``master`` into ``count`` children would occupy.""" + base = int(master[len("Ethernet") :]) + return [f"Ethernet{base + i * lanes_per_child}" for i in range(count)] + + def _breakout_child_collisions(children, master, port_config): """Children that are separate ports of this HWSKU rather than free slots. @@ -655,6 +706,49 @@ def _breakout_child_collisions(children, master, port_config): return [c for c in children if c != master and c in port_config] +def _emit_breakout( + master, + count, + speed_mbps, + port_config, + breakout_cfgs, + breakout_ports, + suppressed_masters, +): + lanes = _parse_lanes(port_config[master]["lanes"]) + lpc = len(lanes) // count + # Refuse before mutating anything: a child slot that is a port in its own + # right must keep its own configuration. Leaving breakout_cfgs unset keeps + # the master a normal port, so the whole declaration is dropped rather + # than half-applied. + children = _breakout_child_names(master, count, lpc) + collisions = _breakout_child_collisions(children, master, port_config) + if collisions: + logger.error( + f"Declared breakout {count}x{speed_mbps // 1000}G for {master} " + f"would claim {', '.join(collisions)}, which are separate ports " + f"on this HWSKU; refusing the declaration" + ) + return + # Read the master index and build its breakout_cfgs entry before staging + # any children, so a missing "index" fails cleanly without leaving orphan + # breakout_ports entries behind. + master_cfg = { + "breakout_owner": "MANUAL", + "brkout_mode": f"{count}x{speed_mbps // 1000}G", + "port": f"1/{port_config[master]['index']}", + } + for i, child in enumerate(children): + breakout_ports[child] = { + "master": master, + "declared": True, + "lanes": ",".join(lanes[i * lpc : (i + 1) * lpc]), + "speed": speed_mbps, + } + breakout_cfgs[master] = master_cfg + suppressed_masters.add(master) + + def detect_breakout_ports(device): """Detect breakout ports from NetBox device interfaces using the centralized breakout logic. @@ -703,6 +797,81 @@ def detect_breakout_ports(device): logger.warning(f"Could not load port config for {device_hwsku}: {e}") return {"breakout_cfgs": breakout_cfgs, "breakout_ports": breakout_ports} + suppressed_masters: set = set() + + # Declared-mode pass: honor explicit breakout map before inference + modes = get_declared_breakout_modes(device) + master_to_keys: dict = {} + for key, mode in modes.items(): + try: + if not isinstance(key, str): + master = None + elif re.fullmatch(r"Ethernet\d+", key): + master = key + elif re.fullmatch(r"Eth1/\d+", key): + resolved = _map_interface_name_to_sonic( + key, interface_names, port_config, device_hwsku + ) + master = ( + resolved if re.fullmatch(r"Ethernet\d+", resolved) else None + ) + else: + master = None + master_to_keys.setdefault(master, []).append((key, mode)) + except Exception as e: + logger.error(f"Error normalizing declared breakout key {key!r}: {e}") + + for master, key_mode_list in master_to_keys.items(): + try: + if master is None: + # Multiple keys can normalize to None simply because none of + # them resolves to a known port; that is not a collision. + for key, _mode in key_mode_list: + logger.error( + f"Declared breakout key {key!r} could not be " + f"resolved to a known port" + ) + continue + if len(key_mode_list) >= 2: + suppressed_masters.add(master) + logger.error( + f"Declared breakout collision for {master}: " + f"keys {[k for k, _ in key_mode_list]}" + ) + continue + key, mode = key_mode_list[0] + if master not in port_config: + logger.error( + f"Declared breakout key {key!r} could not be resolved to a known port" + ) + continue + suppressed_masters.add(master) + parsed = _parse_breakout_mode(mode) + if parsed is None: + logger.error( + f"Declared breakout mode {mode!r} for {master} is invalid" + ) + continue + count, speed_mbps = parsed + L = len(_parse_lanes(port_config[master]["lanes"])) + if L == 0 or L % count != 0: + logger.error( + f"Declared breakout {mode!r} for {master}: " + f"{L} lanes not divisible by {count}" + ) + continue + _emit_breakout( + master, + count, + speed_mbps, + port_config, + breakout_cfgs, + breakout_ports, + suppressed_masters, + ) + except Exception as e: + logger.error(f"Error processing declared breakout for {master!r}: {e}") + # Process interfaces that match breakout patterns processed_groups = set() @@ -778,6 +947,9 @@ def detect_breakout_ports(device): ) continue + if master_port in suppressed_masters: + continue + # Calculate physical port number (1/1 -> port 1, 1/2 -> port 2, etc.) physical_port_num = f"{module}/{port}" @@ -879,6 +1051,10 @@ def detect_breakout_ports(device): if len(sonic_400g_breakout_group) == 4: processed_groups.add(group_key_400g) master_port = f"Ethernet{base_port_400g}" + + if master_port in suppressed_masters: + continue + brkout_mode = "4x100G" # Calculate physical port number for 400G ports @@ -990,6 +1166,9 @@ def detect_breakout_ports(device): if not brkout_mode: continue # Skip unsupported speeds + if master_port in suppressed_masters: + continue + # Calculate physical port number (Ethernet0-3 -> port 1/1, Ethernet4-7 -> port 1/2, etc.) physical_port_index = (base_port // 4) + 1 physical_port_num = f"1/{physical_port_index}" diff --git a/playbooks/pre-sonic-e2e.yml b/playbooks/pre-sonic-e2e.yml new file mode 100644 index 00000000..5aa7e7a4 --- /dev/null +++ b/playbooks/pre-sonic-e2e.yml @@ -0,0 +1,74 @@ +--- +# Node preparation for the SONiC config-generation E2E golden test. +# +# The test's NetBox fixture is a docker compose stack (tests/e2e/compose.yaml), +# so the node only needs Docker: ensure-docker installs docker-compose-plugin +# along with docker-ce. No kind, kubectl or helm. netbox-manager's own E2E job +# still provisions NetBox on kind, so there is nothing to keep in sync here. +- name: Prepare the SONiC E2E node + hosts: all + + pre_tasks: + # This CI node is IPv6-only and learns its address and default route via + # SLAAC / Router Advertisements. If Docker enables + # net.ipv6.conf.all.forwarding=1, the kernel stops honouring RAs at the + # default accept_ra=1, so the SLAAC default route expires and the node + # drops off the network a few minutes into the run. accept_ra=2 keeps RAs + # honoured even while forwarding is on, preserving the default route. + # Compose's default bridge network is IPv4-only, so Docker probably will + # not turn on IPv6 forwarding here -- but this guard is cheap insurance + # against a severe failure mode (an unreachable CI node) and is kept + # deliberately; removing it should be its own separate experiment, not a + # side effect of some other change. Set it here -- before Docker enables + # forwarding -- so the route never lapses. See + # https://docs.docker.com/engine/daemon/ipv6/ and + # https://forums.docker.com/t/docker-removes-host-ipv6-default-route/83238 + - name: Keep accepting IPv6 RAs after Docker enables forwarding (preserve default route) + become: true + ansible.builtin.copy: + dest: /etc/sysctl.d/99-sonic-e2e-accept-ra.conf + owner: root + group: root + mode: "0644" + content: | + net.ipv6.conf.all.accept_ra = 2 + net.ipv6.conf.default.accept_ra = 2 + {% if ansible_default_ipv6.interface is defined %} + net.ipv6.conf.{{ ansible_default_ipv6.interface }}.accept_ra = 2 + {% endif %} + + - name: Apply the accept_ra sysctl settings now + become: true + ansible.builtin.command: + cmd: sysctl -p /etc/sysctl.d/99-sonic-e2e-accept-ra.conf + changed_when: true + # A node without an ansible_default_ipv6.interface fact (or otherwise + # missing these IPv6 keys) fails sysctl -p; this is best-effort + # insurance for the common IPv6-only case, not a hard requirement, so + # do not fail pre-run over it. + failed_when: false + + roles: + - ensure-pip + - ensure-pipenv + - ensure-docker + + tasks: + - name: Ensure the Docker service is running + become: true + ansible.builtin.service: + name: docker + state: started + enabled: true + + # openssl is used by tests/e2e/sonic_golden_test.sh directly. curl is used + # by the NetBox container's healthcheck rather than by the script itself, + # but ensure-docker needs it anyway. python3-venv provides the venv module + # the script uses for the seeding venv. + - name: Install required packages + become: true + ansible.builtin.apt: + name: + - curl + - openssl + - python3-venv diff --git a/playbooks/test-sonic-e2e.yml b/playbooks/test-sonic-e2e.yml new file mode 100644 index 00000000..ab8ebfc0 --- /dev/null +++ b/playbooks/test-sonic-e2e.yml @@ -0,0 +1,40 @@ +--- +- name: Run the SONiC config-generation E2E golden test + hosts: all + + vars: + python_venv_dir: /tmp/venv + + tasks: + - name: Install dependencies + ansible.builtin.shell: + executable: /bin/bash + chdir: "{{ zuul.project.src_dir }}" + cmd: | + set -e + set -o pipefail + set -x + + {{ python_venv_dir }}/bin/pipenv install --dev --deploy + {{ python_venv_dir }}/bin/pipenv run pip install . + + - name: Run the E2E golden test + ansible.builtin.shell: + executable: /bin/bash + chdir: "{{ zuul.project.src_dir }}" + cmd: | + set -e + set -o pipefail + set -x + + # The script invokes bare `pipenv`; the netbox-manager checkout comes + # from Zuul's required-projects, so a Depends-On change to it is + # tested against the changed code and seed data. + export PATH="{{ python_venv_dir }}/bin:${PATH}" + export NETBOX_MANAGER_DIR="{{ ansible_user_dir }}/{{ zuul.projects['github.com/osism/netbox-manager'].src_dir }}" + + # Serial by default: concurrent seeding deadlocks on dcim_device FK + # row locks (see tests/e2e/sonic_golden_test.sh, Phase 2). Set + # `sonic_e2e_seed_parallel: 4` in the job vars to opt back in. + export SEED_PARALLEL="{{ sonic_e2e_seed_parallel | default(1) }}" + tests/e2e/sonic_golden_test.sh diff --git a/tests/e2e/__init__.py b/tests/e2e/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/e2e/compare.py b/tests/e2e/compare.py new file mode 100644 index 00000000..e2e734f2 --- /dev/null +++ b/tests/e2e/compare.py @@ -0,0 +1,224 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Golden-file comparator for the SONiC config-generation E2E test. + +Compares exported SONiC ``config_db.json`` files against committed golden +files. config_db is a shallow TABLE -> entry -> attribute tree, so +structural difference paths are rendered as ``TABLE|entry.attribute``. + +Canonicalization sorts dictionary keys only; array order is preserved and +compared, since list order can be part of the generated contract. +""" + +import argparse +import difflib +import json +import sys +from dataclasses import dataclass, field +from pathlib import Path + + +def _walk(golden, actual, path, depth, out): + """Collect difference paths between two parsed JSON values. + + ``depth`` counts dict levels already entered: children of the top level + (tables) attach with ``|``, everything deeper with ``.``. List indices + attach as ``[i]`` and stay at their parent's depth. + """ + if isinstance(golden, dict) and isinstance(actual, dict): + for key in sorted(golden.keys() | actual.keys()): + sep = "|" if depth == 1 else "." + child = f"{path}{sep}{key}" if path else str(key) + if key not in actual: + out.append(f"{child}: only in golden") + elif key not in golden: + out.append(f"{child}: only in actual") + else: + _walk(golden[key], actual[key], child, depth + 1, out) + elif isinstance(golden, list) and isinstance(actual, list): + for i in range(max(len(golden), len(actual))): + child = f"{path}[{i}]" + if i >= len(actual): + out.append(f"{child}: only in golden") + elif i >= len(golden): + out.append(f"{child}: only in actual") + else: + _walk(golden[i], actual[i], child, depth, out) + elif golden != actual: + out.append(f"{path}: golden {golden!r}, actual {actual!r}") + + +def diff_paths(golden, actual): + """Return sorted structural paths of the differences between two configs.""" + out = [] + _walk(golden, actual, "", 0, out) + return sorted(out) + + +def _canonical(config): + """Canonical JSON text: dict keys sorted, arrays untouched.""" + return json.dumps(config, indent=2, sort_keys=True) + "\n" + + +def unified_diff(golden, actual, name): + """Unified diff of the canonicalized golden and actual configs.""" + return "".join( + difflib.unified_diff( + _canonical(golden).splitlines(keepends=True), + _canonical(actual).splitlines(keepends=True), + fromfile=f"golden/{name}", + tofile=f"export/{name}", + ) + ) + + +@dataclass +class Mismatch: + """Content difference of one exported file against its golden file.""" + + paths: list + diff: str + + +@dataclass +class CompareResult: + """Outcome of comparing an export directory against the golden directory.""" + + missing: list = field(default_factory=list) + extra: list = field(default_factory=list) + mismatched: dict = field(default_factory=dict) + + @property + def ok(self): + return not (self.missing or self.extra or self.mismatched) + + def report(self): + lines = [] + for name in self.missing: + lines.append( + f"MISSING {name}: not exported " + "(generation or export failed - see generate log)" + ) + for name in self.extra: + lines.append(f"EXTRA {name}: unexpected extra export (no golden file)") + for name, mismatch in self.mismatched.items(): + lines.append(f"MISMATCH {name}:") + lines.extend(f" {path}" for path in mismatch.paths) + lines.append(mismatch.diff) + if not lines: + lines.append("OK: exports match the golden files") + return "\n".join(lines) + + +def compare_dirs(golden_dir, export_dir): + """Compare all golden ``*.json`` files against the exported ones. + + Requires exact file-set equality: a golden file without an export is + ``missing`` (generation or export failed), an export without a golden + file is ``extra``, and differing content is ``mismatched``. Non-JSON + files (e.g. firmware symlinks in the export directory) are ignored. + """ + golden_dir = Path(golden_dir) + export_dir = Path(export_dir) + golden_names = {p.name for p in golden_dir.glob("*.json")} + export_names = {p.name for p in export_dir.glob("*.json")} + + result = CompareResult( + missing=sorted(golden_names - export_names), + extra=sorted(export_names - golden_names), + ) + for name in sorted(golden_names & export_names): + golden = json.loads((golden_dir / name).read_text()) + actual = json.loads((export_dir / name).read_text()) + paths = diff_paths(golden, actual) + if paths: + result.mismatched[name] = Mismatch( + paths=paths, diff=unified_diff(golden, actual, name) + ) + return result + + +def regenerate(golden_dir, export_dir, allow_coverage_loss=False): + """Rewrite the golden directory from the export directory. + + Every exported ``*.json`` file is stored in canonical form (sorted dict + keys) so regenerated goldens produce minimal, reviewable git diffs; + stale golden files without a matching export are removed. Never run in + CI - regeneration is a deliberate local step after an intentional + generator change. + + Returns a list of entries describing coverage that was lost: either a + table that was populated in the previous golden and would become empty, + or a whole golden file that had no matching export at all and is being + removed. Regeneration is the only path by which coverage can silently + drop -- in the normal path a table going empty (or a whole device + disappearing) changes the golden and fails the comparison -- and either + is invisible inside a several-hundred-line JSON diff. The files are + still written (or removed); reporting is the caller's decision to act + on. + """ + golden_dir = Path(golden_dir) + export_dir = Path(export_dir) + golden_dir.mkdir(parents=True, exist_ok=True) + + def _populated(config): + return {t for t, v in config.items() if v} + + lost = [] + export_names = {p.name for p in export_dir.glob("*.json")} + for stale in golden_dir.glob("*.json"): + if stale.name not in export_names: + n = len(_populated(json.loads(stale.read_text()))) + if n: + lost.append(f"{stale.name}: file removed, had {n} populated tables") + stale.unlink() + + for name in sorted(export_names): + config = json.loads((export_dir / name).read_text()) + previous = golden_dir / name + if previous.exists(): + before = _populated(json.loads(previous.read_text())) + for table in sorted(before - _populated(config)): + lost.append(f"{name}: {table}") + previous.write_text(_canonical(config)) + return lost + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--golden", required=True, help="golden file directory") + parser.add_argument("--export", required=True, help="export directory to check") + parser.add_argument( + "--regenerate", + action="store_true", + help="rewrite the golden files from the export directory", + ) + parser.add_argument( + "--allow-coverage-loss", + action="store_true", + help="regenerate even when a previously populated table becomes empty", + ) + args = parser.parse_args(argv) + + if args.regenerate: + lost = regenerate( + args.golden, args.export, allow_coverage_loss=args.allow_coverage_loss + ) + if lost and not args.allow_coverage_loss: + print("COVERAGE LOSS -- coverage was lost:") + for entry in lost: + print(f" {entry}") + print( + "Goldens were written. Review the diff, then either fix the " + "fixtures or re-run with --allow-coverage-loss to accept it." + ) + return 1 + return 0 + + result = compare_dirs(args.golden, args.export) + print(result.report()) + return 0 if result.ok else 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/e2e/compose.yaml b/tests/e2e/compose.yaml new file mode 100644 index 00000000..6bef7403 --- /dev/null +++ b/tests/e2e/compose.yaml @@ -0,0 +1,93 @@ +--- +# NetBox fixture for the SONiC config-generation E2E golden test. +# +# Three services, no Kubernetes: the test only needs a NetBox REST API, +# its PostgreSQL and its Valkey. See +# docs/superpowers/specs/2026-07-29-sonic-e2e-compose-design.md. +# +# NetBox is configured through the environment variables its own baked +# /etc/netbox/config/configuration.py reads, so no configuration file is +# mounted. API_TOKEN_PEPPERS is deliberately NOT set: the image's +# super_user.py only creates a token when a pepper is configured, and then +# only a v2 one, which pynetbox / netbox.netbox cannot use. deploy_netbox.sh +# mints a v1 token instead. ("No API token will be created" in the netbox +# log is therefore expected, not an error.) +# +# The secret key and superuser password are fixed literals, not +# interpolated variables. This stack is ephemeral, published on loopback +# only, and thrown away after each run, so they are not credentials. Keeping +# them out of ${...} matters because docker compose interpolates the whole +# file on *every* subcommand -- a `${VAR:?}` here would make plain +# `docker compose down` fail whenever the caller's shell lacked the value. +# +# The NetBox version is pinned because the golden files were generated +# against it -- changing it can change generated configs. postgres and +# valkey are pinned to a patch level; they are tags, not digests, so this +# bounds rather than freezes them. + +name: sonic-e2e + +services: + postgres: + image: postgres:17.10-alpine + environment: + # Must match netbox's DB_NAME / DB_USER / DB_PASSWORD below. The + # official image mandates POSTGRES_PASSWORD and would otherwise + # default the database and role to "postgres". + POSTGRES_DB: netbox + POSTGRES_USER: netbox + POSTGRES_PASSWORD: netbox + healthcheck: + test: ["CMD-SHELL", "pg_isready -U netbox -d netbox"] + interval: 5s + timeout: 5s + retries: 24 + + valkey: + image: valkey/valkey:8.1-alpine + # No authentication: valkey is reachable only on the compose-private + # network and publishes no port, so netbox needs no REDIS_PASSWORD. + healthcheck: + test: ["CMD", "valkey-cli", "ping"] + interval: 5s + timeout: 5s + retries: 24 + + netbox: + image: ghcr.io/netbox-community/netbox:v4.5.10 + depends_on: + postgres: + condition: service_healthy + valkey: + condition: service_healthy + environment: + DB_HOST: postgres + DB_NAME: netbox + DB_USER: netbox + DB_PASSWORD: netbox + # NetBox needs two logical Redis databases; one valkey serves both. + REDIS_HOST: valkey + REDIS_PORT: "6379" + REDIS_DATABASE: "0" + REDIS_CACHE_HOST: valkey + REDIS_CACHE_PORT: "6379" + REDIS_CACHE_DATABASE: "1" + # Django wants >= 50 characters. Not a credential -- see the header. + SECRET_KEY: "insecure-sonic-e2e-secret-key-not-used-outside-tests" + SUPERUSER_NAME: admin + SUPERUSER_EMAIL: admin@example.com + SUPERUSER_PASSWORD: "insecure-sonic-e2e-admin-password" + ALLOWED_HOSTS: "*" + ports: + # Published on loopback only; the seeding and generation phases talk + # to http://127.0.0.1:${NETBOX_PORT}. No port-forward involved. + - "127.0.0.1:${NETBOX_PORT:-8080}:8080" + healthcheck: + # First boot runs the full migration set, which takes minutes; the + # start_period covers that without the container being marked + # unhealthy. Subsequent boots skip migrations and come up in ~1 min. + test: ["CMD", "curl", "-fsS", "-o", "/dev/null", "http://localhost:8080/login/"] + interval: 10s + timeout: 5s + retries: 3 + start_period: 600s diff --git a/tests/e2e/coverage.py b/tests/e2e/coverage.py new file mode 100644 index 00000000..a9aded4e --- /dev/null +++ b/tests/e2e/coverage.py @@ -0,0 +1,89 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""config_db table coverage report for the SONiC E2E golden set. + +This is a reporting tool, not a gate: it is not invoked from +sonic_golden_test.sh, and the golden comparison (tests/e2e/compare.py) +remains the only check that actually fails a run. It exists so the "N of M +config_db tables covered" claim made when the golden set is extended can be +re-derived by anyone, at any time, instead of only having existed as an +ad-hoc one-off script run once during development. + +It works in two independent steps: + +1. Derive the set of tables the generator can emit by grepping + osism/tasks/conductor/sonic/ (excluding the generated schema package, + _generated/, which is data rather than emission logic) for direct + ``config["TABLE"]``/``cfg["TABLE"]`` assignments -- including a nested + item assignment such as ``config["ACL_TABLE"]["SSH_ONLY"] = ...`` or a + ``.update(...)`` call, but not a mere read such as + ``"x" in config["VERSIONS"]``. This is a static, syntactic approximation + of "tables the generator can populate", not a guarantee every branch + that reaches it is exercised. +2. Collect the set of tables that are non-empty in at least one file under + tests/e2e/golden/*.json. + +The report prints both counts and, if any emitted table is never non-empty +across the golden set, lists them and exits 1. +""" + +import re +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +GENERATOR_DIR = REPO_ROOT / "osism" / "tasks" / "conductor" / "sonic" +GOLDEN_DIR = REPO_ROOT / "tests" / "e2e" / "golden" + +# Matches config["TABLE"] or cfg["TABLE"], optionally followed by one or +# more ["key"]/[expr] accessors, then either an assignment ("=" but not +# "==") or a .update(...) call -- i.e. the table is a write target, not +# merely read. +_TABLE_ASSIGNMENT_RE = re.compile( + r'(?:config|cfg)\["([A-Z][A-Z0-9_]*)"\](?:\[[^\]]*\])*\s*(?:=(?!=)|\.update\()' +) + + +def emitted_tables(generator_dir=GENERATOR_DIR): + """Tables the generator can emit, derived from source assignments.""" + tables = set() + for path in sorted(Path(generator_dir).rglob("*.py")): + if "_generated" in path.parts: + continue + for line in path.read_text().splitlines(): + if line.lstrip().startswith("#"): + continue + for match in _TABLE_ASSIGNMENT_RE.finditer(line): + tables.add(match.group(1)) + return tables + + +def covered_tables(golden_dir=GOLDEN_DIR): + """Tables that are non-empty in at least one golden file.""" + import json + + tables = set() + for path in sorted(Path(golden_dir).glob("*.json")): + config = json.loads(path.read_text()) + for table, value in config.items(): + if value: + tables.add(table) + return tables + + +def main(argv=None): + emitted = emitted_tables() + covered = covered_tables() + missing = sorted(emitted - covered) + + print(f"generator emits {len(emitted)} tables; {len(covered)} non-empty") + if missing: + print("not covered by any golden file:") + for table in missing: + print(f" {table}") + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/e2e/deploy_netbox.sh b/tests/e2e/deploy_netbox.sh new file mode 100755 index 00000000..ac5a8890 --- /dev/null +++ b/tests/e2e/deploy_netbox.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +# +# Provision NetBox for the SONiC E2E golden test (phase 1), using docker +# compose. See docs/superpowers/specs/2026-07-29-sonic-e2e-compose-design.md. +# +# This script starts the stack and mints the API token. It deliberately +# installs NO teardown trap: sonic_golden_test.sh owns the lifecycle, and a trap here +# would fire when this script exits -- before seeding and generation. +# +# Safe to run standalone for debugging (`make sonic-e2e-up`), which leaves +# the stack running. +# +# The NetBox secret key and superuser password are fixed literals in +# compose.yaml (ephemeral loopback-only fixture); only the API token and the +# published port are parameterised here. +# +# Environment overrides: +# NETBOX_TOKEN v1 API token to mint (default: random) +# NETBOX_PORT host port for the NetBox API (default: 8080) +# PRINT_NETBOX_TOKEN=0 suppress echoing the token (set by sonic_golden_test.sh) + +set -euo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +COMPOSE_FILE="${HERE}/compose.yaml" + +# Only generated when unset, so sonic_golden_test.sh's value wins when it calls us. +export NETBOX_TOKEN="${NETBOX_TOKEN:-$(openssl rand -hex 20)}" +# Guard against two failure modes of a caller-supplied token: it is +# interpolated into a Python string literal in the heredoc below, so a +# quote or newline would break out of that literal; and a token that is +# not 40 hex characters is otherwise accepted here but silently rejected +# by NetBox much later, far from this, the actual cause. +[[ "${NETBOX_TOKEN}" =~ ^[0-9a-f]{40}$ ]] || { + echo "error: NETBOX_TOKEN must be 40 hex characters" >&2 + exit 2 +} +export NETBOX_PORT="${NETBOX_PORT:-8080}" + +compose() { docker compose -f "${COMPOSE_FILE}" "$@"; } + +echo ">>> Starting the NetBox stack (postgres, valkey, netbox)" +# --wait blocks until every service is healthy. First boot runs the full +# NetBox migration set, hence the generous timeout. +compose up --detach --wait --wait-timeout 900 + +# Mint a deterministic v1 API token for the superuser. NetBox 4.5 introduced +# peppered "v2" API tokens; the image's bootstrap only ever creates a v2 one, +# and only when API_TOKEN_PEPPERS is set (compose.yaml leaves it unset, so it +# creates none). pynetbox / netbox.netbox authenticate with +# `Authorization: Token `, i.e. a v1 token, which NetBox accepts through +# v4.6 -- legacy v1 support is removed in v4.7, so re-check this on a bump. +# +# The delete-then-create makes this idempotent: re-running against a reused +# stack replaces the old token instead of colliding with it. Keep the delete. +# +# The script is fed over stdin (not `shell -c`) so the token never lands on a +# command line inside the container. +echo ">>> Creating a deterministic v1 API token for the superuser" +compose exec -T netbox /opt/netbox/netbox/manage.py shell --interface python <leaf, one leaf carrying an access and a trunk +# port, and one VRF assigned to a data port. +# +# oob_ip / Loopback0 values are fixed by the fixture value contract in the +# plan (docs/superpowers/plans/2026-07-30-sonic-e2e-synthetic-fixtures.md): +# the ASN is derived from Loopback0 (4200 + zero-padded 3rd/4th octet), so +# these addresses are not free choices. +# +# Interface names come from the Accton-AS7726-32X hwsku's port_config +# (files/sonic/port_config/Accton-AS7726-32X.ini), not from the minimal +# edgecore-7726-32x-e2e device type -- that device type only defines eth0, +# Ethernet0 and Ethernet4, so every other interface used here is created +# explicitly via device_interface stanzas. + +# ---------------------------------------------------------------- spine -- +- device: + name: e2e-spine-1 + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-7726-32x-e2e + device_role: spine + status: active + face: front + position: 1 + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS7726-32X + version: 4.5.0 + +- device_interface: + device: e2e-spine-1 + name: Loopback0 + type: virtual + enabled: true + +- ip_address: + tenant: Testbed + address: 172.16.10.1/20 + assigned_object: + name: eth0 + device: e2e-spine-1 + +- ip_address: + tenant: Testbed + address: 192.168.20.1/32 + assigned_object: + name: Loopback0 + device: e2e-spine-1 + +- device: + name: e2e-spine-1 + oob_ip: 172.16.10.1/20 + primary_ip4: 192.168.20.1/32 + +# --------------------------------------------------------------- leaf-1 -- +- device: + name: e2e-leaf-1 + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-7726-32x-e2e + device_role: leaf + status: active + face: front + position: 2 + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS7726-32X + version: 4.5.0 + +- device_interface: + device: e2e-leaf-1 + name: Loopback0 + type: virtual + enabled: true + +- ip_address: + tenant: Testbed + address: 172.16.10.11/20 + assigned_object: + name: eth0 + device: e2e-leaf-1 + +- ip_address: + tenant: Testbed + address: 192.168.20.11/32 + assigned_object: + name: Loopback0 + device: e2e-leaf-1 + +- device: + name: e2e-leaf-1 + oob_ip: 172.16.10.11/20 + primary_ip4: 192.168.20.11/32 + +# --------------------------------------------------------------- leaf-2 -- +- device: + name: e2e-leaf-2 + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-7726-32x-e2e + device_role: leaf + status: active + face: front + position: 3 + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS7726-32X + version: 4.5.0 + +- device_interface: + device: e2e-leaf-2 + name: Loopback0 + type: virtual + enabled: true + +- ip_address: + tenant: Testbed + address: 172.16.10.12/20 + assigned_object: + name: eth0 + device: e2e-leaf-2 + +- ip_address: + tenant: Testbed + address: 192.168.20.12/32 + assigned_object: + name: Loopback0 + device: e2e-leaf-2 + +- device: + name: e2e-leaf-2 + oob_ip: 172.16.10.12/20 + primary_ip4: 192.168.20.12/32 + +# ---------------------------------------------------------- fabric links -- +# Point-to-point link addresses must fall inside a prefix with the +# Transfer role, or _get_transfer_role_ipv4_addresses() treats the link as +# IP-unnumbered and BGP_NEIGHBOR/BGP_NEIGHBOR_AF stay empty even though the +# link is cabled and addressed. +- prefix: + family: 4 + prefix: 192.168.30.0/24 + prefix_role: Transfer + tenant: Testbed + +# e2e-spine-1 Ethernet0 <-> e2e-leaf-1 Ethernet0, on a /31. +- cable: + termination_a_type: dcim.interface + termination_a: + device: e2e-spine-1 + name: Ethernet0 + termination_b_type: dcim.interface + termination_b: + device: e2e-leaf-1 + name: Ethernet0 + +- ip_address: + tenant: Testbed + address: 192.168.30.0/31 + assigned_object: + name: Ethernet0 + device: e2e-spine-1 + +- ip_address: + tenant: Testbed + address: 192.168.30.1/31 + assigned_object: + name: Ethernet0 + device: e2e-leaf-1 + +# e2e-spine-1 Ethernet4 <-> e2e-leaf-2 Ethernet0, on a /31. +- cable: + termination_a_type: dcim.interface + termination_a: + device: e2e-spine-1 + name: Ethernet4 + termination_b_type: dcim.interface + termination_b: + device: e2e-leaf-2 + name: Ethernet0 + +- ip_address: + tenant: Testbed + address: 192.168.30.2/31 + assigned_object: + name: Ethernet4 + device: e2e-spine-1 + +- ip_address: + tenant: Testbed + address: 192.168.30.3/31 + assigned_object: + name: Ethernet0 + device: e2e-leaf-2 + +# ------------------------------------------------------ leaf-1 VLAN ports -- +# Access port with untagged VLAN 100 (oob) and a trunk port with tagged +# VLAN 200 (data), so VLAN, VLAN_INTERFACE and VLAN_MEMBER populate. +- device_interface: + device: e2e-leaf-1 + name: Ethernet8 + type: 100gbase-x-qsfp28 + mode: access + untagged_vlan: + name: oob + site: Discworld + +- device_interface: + device: e2e-leaf-1 + name: Ethernet12 + type: 100gbase-x-qsfp28 + mode: tagged + tagged_vlans: + - name: data + site: Discworld + +# VLAN_INTERFACE (SVI) needs an actual NetBox virtual interface named +# Vlan carrying an IP address -- VLAN_MEMBER from the ports above +# does not by itself create one. +- device_interface: + device: e2e-leaf-1 + name: Vlan200 + type: virtual + enabled: true + +- ip_address: + tenant: Testbed + address: 192.168.40.1/24 + assigned_object: + name: Vlan200 + device: e2e-leaf-1 + +# ------------------------------------------------------------------ VRF -- +# vrf99 has no RD, so it takes the table_id-only branch (VRF["Vrf99"] = +# {vrf_table_id: 99}), which is enough to populate VRF, the BGP_GLOBALS* +# family and ROUTE_REDISTRIBUTE without pulling in EVPN/VXLAN. +- vrf: + name: vrf99 + tenant: Testbed + +- device_interface: + device: e2e-leaf-2 + name: Ethernet4 + vrf: vrf99 diff --git a/tests/e2e/scenario/resources/250-oob.yml b/tests/e2e/scenario/resources/250-oob.yml new file mode 100644 index 00000000..bdb24bba --- /dev/null +++ b/tests/e2e/scenario/resources/250-oob.yml @@ -0,0 +1,47 @@ +--- +# Standalone OOB switch. Role "switch" is in DEFAULT_SONIC_ROLES, so this +# device also gets a golden -- it exercises the filter with a role other +# than spine/leaf. + +- device: + name: e2e-oob-1 + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-7726-32x-e2e + device_role: switch + status: active + face: front + position: 4 + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS7726-32X + version: 4.5.0 + +- device_interface: + device: e2e-oob-1 + name: Loopback0 + type: virtual + enabled: true + +- ip_address: + tenant: Testbed + address: 172.16.10.21/20 + assigned_object: + name: eth0 + device: e2e-oob-1 + +- ip_address: + tenant: Testbed + address: 192.168.20.21/32 + assigned_object: + name: Loopback0 + device: e2e-oob-1 + +- device: + name: e2e-oob-1 + oob_ip: 172.16.10.21/20 + primary_ip4: 192.168.20.21/32 diff --git a/tests/e2e/scenario/resources/260-metalbox.yml b/tests/e2e/scenario/resources/260-metalbox.yml new file mode 100644 index 00000000..fbffcc01 --- /dev/null +++ b/tests/e2e/scenario/resources/260-metalbox.yml @@ -0,0 +1,40 @@ +--- +# The metalbox. Role "metalbox" is NOT in DEFAULT_SONIC_ROLES and this +# device is deliberately NOT tagged managed-by-metalbox, so it must never +# get a golden of its own -- if one appears, this device wrongly acquired a +# SONiC role or the tag. +# +# _get_metalbox_ip_for_device() walks every role=metalbox device's +# interfaces and returns the first IPv4 that falls inside the SONiC +# device's OOB network; it does not follow a cable. All four switches use +# /20 oob_ips, so that network is 172.16.0.0/20, and 172.16.10.254 sits +# inside it -- that subnet membership, not any cabling, is what unlocks +# DNS_NAMESERVER and NTP_SERVER. +# +# The address must NOT sit on a mgmt_only interface: +# _load_metalbox_devices_cache() explicitly skips those, so it cannot be on +# eth0 (mgmt_only: true in the device type). Ethernet0 is a plain data port +# on this device type and is otherwise unused here. + +- device: + name: e2e-metalbox-1 + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-7726-32x-e2e + device_role: metalbox + status: active + face: front + position: 5 + +- ip_address: + tenant: Testbed + address: 172.16.10.254/20 + assigned_object: + name: Ethernet0 + device: e2e-metalbox-1 + +- device: + name: e2e-metalbox-1 + primary_ip4: 172.16.10.254/20 diff --git a/tests/e2e/scenario/resources/500-breakout.yml b/tests/e2e/scenario/resources/500-breakout.yml new file mode 100644 index 00000000..11decb90 --- /dev/null +++ b/tests/e2e/scenario/resources/500-breakout.yml @@ -0,0 +1,143 @@ +--- +# Scenario overlay for the SONiC E2E golden test. +# +# The bundled netbox-manager example models no breakout ports and sets no +# explicit interface speeds, so the breakout code paths and the kbps->Mbps +# speed handling are never exercised by it. These devices add that +# coverage. They reuse the site / location / tenant / roles / tags / custom +# fields created by 100-base.yml, and take the next free positions in the +# shared E2E rack so they never collide with the base devices. +# +# The first two devices use the edgecore-9726-32d-e2e device type (hwsku +# Accton-AS9726-32D) and break Eth1/1 into a 4x100G group. They differ +# only in how the sub-port speed reaches NetBox: +# +# e2e-breakout-derived speed derived from the interface type +# (100gbase-x-qsfp28 -> 100000 Mbps); no explicit +# speed. This is how real deployments model +# breakouts. +# e2e-breakout-explicit the same sub-ports with an explicit NetBox speed +# set in kbps (100000000), the other unit the +# collection step must normalise. +# +# Both must yield sub-port speed 100000 in the generated config. +# +# A third device, e2e-breakout-declared, covers the explicit +# sonic_parameters.breakout declaration path (see its block below). + +# --- Derived-speed breakout (the real-deployment shape) -------------------- + +- device: + name: e2e-breakout-derived + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-9726-32d-e2e + device_role: leaf + face: front + position: 6 + status: active + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS9726-32D + version: 4.5.0 + +# Tagged VLAN on the plain (non-breakout) port, exercising the VLAN and +# tagged-VLAN-to-port paths. +- device_interface: + device: e2e-breakout-derived + name: Eth1/5 + type: 100gbase-x-qsfp28 + mode: tagged + tagged_vlans: + - name: data + site: Discworld + +# --- Explicit-speed breakout (the other speed unit) ----------------------- + +- device: + name: e2e-breakout-explicit + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-9726-32d-e2e + device_role: leaf + face: front + position: 7 + status: active + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS9726-32D + version: 4.5.0 + +- device_interface: + device: e2e-breakout-explicit + name: Eth1/1/1 + type: 100gbase-x-qsfp28 + speed: 100000000 + +- device_interface: + device: e2e-breakout-explicit + name: Eth1/1/2 + type: 100gbase-x-qsfp28 + speed: 100000000 + +- device_interface: + device: e2e-breakout-explicit + name: Eth1/1/3 + type: 100gbase-x-qsfp28 + speed: 100000000 + +- device_interface: + device: e2e-breakout-explicit + name: Eth1/1/4 + type: 100gbase-x-qsfp28 + speed: 100000000 + +# --- Declared breakout map (the explicit sonic_parameters.breakout path) --- +# +# e2e-breakout-declared carries an authoritative device-level breakout map +# instead of modelling sub-ports in NetBox. It exercises the declared-mode +# code path end to end on the 8-lane Accton-AS9726-32D: +# +# Ethernet0: 4x100G canonical key; 8 lanes -> 4 children (Ethernet0/2/4/6), +# 2 lanes each (73,74 / 75,76 / 77,78 / 79,80), 100000 +# Ethernet8: 2x50G canonical key; 8 lanes -> 2 children (Ethernet8/12), +# 4 lanes each (65,66,67,68 / 69,70,71,72), 50000 +# Eth1/9: 8x50G physical key normalised to Ethernet64; 8 lanes -> 8 +# children (Ethernet64..71), 1 lane each (41..48), 50000 +# +# The map is the sole breakout signal: no sub-port interfaces are modelled, +# so a golden diff here proves the declared path, not inference. Each +# BREAKOUT_CFG entry must carry breakout_owner MANUAL, the canonical mode +# string, and the port_config index (1/1, 1/2, 1/9 -- correct on this +# mixed-lane platform); each BREAKOUT_PORTS child must contain only its +# master; and the per-child PORT lanes/speed must match the mode. + +- device: + name: e2e-breakout-declared + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-9726-32d-e2e + device_role: leaf + face: front + position: 8 + status: active + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS9726-32D + version: 4.5.0 + breakout: + Ethernet0: 4x100G + Ethernet8: 2x50G + Eth1/9: 8x50G diff --git a/tests/e2e/scenario/resources/600-portchannel.yml b/tests/e2e/scenario/resources/600-portchannel.yml new file mode 100644 index 00000000..2f1b8e6e --- /dev/null +++ b/tests/e2e/scenario/resources/600-portchannel.yml @@ -0,0 +1,42 @@ +--- +# Port-channel (LAG) scenario for the SONiC E2E golden test. +# +# The base fixtures model no LAGs, so PORTCHANNEL / PORTCHANNEL_MEMBER / +# PORTCHANNEL_INTERFACE are emitted-but-empty. This device bonds two data +# ports into PortChannel1: a NetBox LAG interface (type: lag) with the two +# ports referencing it via their lag field, exactly as real deployments +# model it. Reuses the site / location / tenant from 100-base.yml, and +# takes the next free position in the shared E2E rack. + +- device: + name: e2e-portchannel + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-7726-32x-e2e + device_role: leaf + face: front + position: 9 + status: active + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS7726-32X + version: 4.5.0 + +- device_interface: + device: e2e-portchannel + name: PortChannel1 + type: lag + +- device_interface: + device: e2e-portchannel + name: Ethernet0 + lag: PortChannel1 + +- device_interface: + device: e2e-portchannel + name: Ethernet4 + lag: PortChannel1 diff --git a/tests/e2e/scenario/resources/700-evpn.yml b/tests/e2e/scenario/resources/700-evpn.yml new file mode 100644 index 00000000..23fac139 --- /dev/null +++ b/tests/e2e/scenario/resources/700-evpn.yml @@ -0,0 +1,89 @@ +--- +# EVPN / VXLAN / VRF scenario for the SONiC E2E golden test. +# +# The base fixtures assign only one table_id-only VRF (vrf99 in +# 200-fabric.yml), so the whole EVPN/VXLAN subsystem is emitted-but-empty: +# VXLAN_TUNNEL, VXLAN_TUNNEL_MAP, VXLAN_EVPN_NVO, the L2VPN-EVPN +# BGP_GLOBALS_AF and BGP_GLOBALS_ROUTE_ADVERTISE. Generation keys purely on +# NetBox VRF objects assigned to interfaces (interface.vrf); no +# config_context or tags are involved. +# +# VrfStorage (rd 2001, a pure number) -> becomes VNI 2001 and populates +# VRF, VXLAN_TUNNEL, VXLAN_TUNNEL_MAP, VXLAN_EVPN_NVO, +# BGP_GLOBALS_AF (l2vpn_evpn, route-target 2001:1) and +# BGP_GLOBALS_ROUTE_ADVERTISE, plus side effects (Vlan2001, its +# VLAN_INTERFACE, ROUTE_REDISTRIBUTE, per-VRF BGP_GLOBALS). +# vrf42 (name matches vrf, no rd) -> the table_id-only branch: +# VRF["Vrf42"] = {vrf_table_id: 42}, no EVPN/VXLAN. Together with +# vrf99 (200-fabric.yml) this brings VRF coverage from one to three. +# +# The VXLAN tunnel source IP is the device router-id (primary_ip4), so the +# device needs a Loopback0 with an address. Reuses the site / location / +# tenant / roles / tags created by 100-base.yml, and takes the next free +# position in the shared E2E rack. + +- vrf: + name: VrfStorage + rd: 2001 + tenant: Testbed + +- vrf: + name: vrf42 + tenant: Testbed + +- device: + name: e2e-evpn + tenant: Testbed + site: Discworld + location: Ankh-Morpork + rack: E2E + device_type: edgecore-7726-32x-e2e + device_role: leaf + face: front + position: 10 + status: active + tags: + - managed-by-metalbox + custom_fields: + sonic_parameters: + hwsku: Accton-AS7726-32X + version: 4.5.0 + +- device_interface: + device: e2e-evpn + name: Loopback0 + type: virtual + enabled: true + +- ip_address: + tenant: Testbed + address: 192.168.16.40/32 + assigned_object: + name: Loopback0 + device: e2e-evpn + +- ip_address: + tenant: Testbed + address: "fda6:f659:8c2b::192:168:16:40/128" + assigned_object: + name: Loopback0 + device: e2e-evpn + +# VRF-with-VNI on one data port drives the EVPN/VXLAN tables; the +# table_id-only VRF on another exercises the non-EVPN VRF branch. +- device_interface: + device: e2e-evpn + name: Ethernet0 + type: 100gbase-x-qsfp28 + vrf: VrfStorage + +- device_interface: + device: e2e-evpn + name: Ethernet4 + type: 100gbase-x-qsfp28 + vrf: vrf42 + +- device: + name: e2e-evpn + primary_ip4: 192.168.16.40/32 + primary_ip6: "fda6:f659:8c2b::192:168:16:40/128" diff --git a/tests/e2e/sonic_golden_test.sh b/tests/e2e/sonic_golden_test.sh new file mode 100755 index 00000000..ed135154 --- /dev/null +++ b/tests/e2e/sonic_golden_test.sh @@ -0,0 +1,226 @@ +#!/usr/bin/env bash +# +# SONiC config-generation E2E golden test: +# +# 1. start NetBox with docker compose (tests/e2e/deploy_netbox.sh; an +# existing stack is reused and left in place) +# 2. seed it with the fixtures in tests/e2e/scenario/ (netbox-manager is +# the seeding tool; its example/ data is deliberately not used) +# 3. run sync_sonic() via tests/e2e/generate.py +# 4. compare the exported config_db files against tests/e2e/golden/ +# (or rewrite the goldens with --regenerate) +# +# Requirements: docker (with the compose plugin), openssl, a netbox-manager +# checkout (sibling directory or NETBOX_MANAGER_DIR) for the seeding CLI, and +# this repo's pipenv environment (pipenv install --dev). +# +# Phase 2 seeds every file under tests/e2e/scenario/resources/ regardless of +# git status -- a stray untracked file in that directory joins the fixture +# set and gets seeded too. This has broken a run twice; keep that directory +# clean (git status --ignored) before regenerating or debugging a mismatch. +# +# Environment overrides: +# NETBOX_MANAGER_DIR netbox-manager checkout (default: ../netbox-manager) +# NETBOX_TOKEN API token (default: random; also minted in NetBox) +# NETBOX_PORT host port for the NetBox API (default: 8080) +# KEEP_STACK=1 leave a stack created by this run running +# SEED_PARALLEL files seeded concurrently per group (default: 1; +# >1 deadlocks intermittently -- see Phase 2) +# ALLOW_WARM_REGEN=1 let --regenerate run against a reused stack (see +# the CREATED_STACK check below; off by default +# because it can produce goldens CI cannot reproduce) +# +# Usage: sonic_golden_test.sh [--regenerate [--allow-coverage-loss]] +# --regenerate rewrite the golden files from a fresh export +# --allow-coverage-loss with --regenerate, accept a table/device going +# from populated to empty/removed instead of +# failing (forwarded to tests.e2e.compare) + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cd "${REPO_ROOT}" + +REGENERATE=0 +ALLOW_COVERAGE_LOSS=0 +for arg in "$@"; do + case "${arg}" in + --regenerate) REGENERATE=1 ;; + --allow-coverage-loss) ALLOW_COVERAGE_LOSS=1 ;; + *) + echo "usage: $0 [--regenerate [--allow-coverage-loss]]" >&2 + exit 2 + ;; + esac +done + +NETBOX_MANAGER_DIR="${NETBOX_MANAGER_DIR:-${REPO_ROOT}/../netbox-manager}" +NETBOX_MANAGER_DIR="$(cd "${NETBOX_MANAGER_DIR}" 2>/dev/null && pwd)" || { + echo "error: netbox-manager checkout not found; set NETBOX_MANAGER_DIR" >&2 + exit 2 +} + +NETBOX_TOKEN="${NETBOX_TOKEN:-$(openssl rand -hex 20)}" +NETBOX_PORT="${NETBOX_PORT:-8080}" +GOLDEN_DIR="${REPO_ROOT}/tests/e2e/golden" +COMPOSE_FILE="${REPO_ROOT}/tests/e2e/compose.yaml" +export NETBOX_TOKEN NETBOX_PORT + +compose() { docker compose -f "${COMPOSE_FILE}" "$@"; } + +# Only tear down a stack this run actually created -- never a reused debug +# stack (make sonic-e2e-up). --all so a stopped stack still counts as +# pre-existing. +CREATED_STACK=0 +if [[ -z "$(compose ps --all --quiet 2>/dev/null)" ]]; then + CREATED_STACK=1 +fi + +# Regenerating against a reused stack applies each fixture as an UPDATE over +# whatever is already in the database, where a fresh stack creates every +# object cleanly -- the two can produce different goldens with no warning. +# Refuse by default; ALLOW_WARM_REGEN=1 is the escape hatch for someone who +# knows the reused stack still matches the fixtures (e.g. iterating with +# KEEP_STACK=1 and only editing generator code, not the fixtures). +if [[ "${REGENERATE}" == "1" && "${CREATED_STACK}" == "0" && "${ALLOW_WARM_REGEN:-0}" != "1" ]]; then + echo "error: --regenerate refuses to run against a reused NetBox stack." >&2 + echo " Applying fixtures as an UPDATE over a stale database can" >&2 + echo " produce goldens a fresh stack (as CI always uses) would not" >&2 + echo " reproduce. Run 'make sonic-e2e-down' first, or set" >&2 + echo " ALLOW_WARM_REGEN=1 if you know the reused stack still" >&2 + echo " matches the fixtures being regenerated." >&2 + exit 2 +fi + +EXPORT_DIR="" +dump_diagnostics() { + echo "==================== NetBox stack diagnostics ====================" + compose ps --all 2>&1 || true + # The application log is what actually explains a failed start; the stack + # is torn down below, taking it with it, so snapshot it first. Capped to + # the last 200 lines: this fires on any non-zero exit, including the most + # common failure (a golden mismatch in phase 4, where compare.py has + # already printed the useful diff), and an uncapped dump buries that diff + # under megabytes of NetBox first-boot migration and postgres logs. 200 + # lines still covers a genuine boot failure. + compose logs --no-color --timestamps --tail 200 2>&1 || true + echo "=================================================================" +} +cleanup() { + rc=$? + if [[ "${rc}" -ne 0 ]]; then + echo ">>> E2E run failed (exit ${rc}); dumping stack diagnostics" + dump_diagnostics || true + fi + if [[ -n "${EXPORT_DIR}" ]]; then + rm -rf "${EXPORT_DIR}" + fi + if [[ "${CREATED_STACK}" == "1" && "${KEEP_STACK:-0}" != "1" ]]; then + echo ">>> Stopping the NetBox stack" + compose down --volumes --remove-orphans || true + else + echo ">>> Leaving the NetBox stack in place" + fi +} +trap cleanup EXIT + +# --- Phase 1: provision NetBox with docker compose -------------------------- +# The stack publishes the API on 127.0.0.1:${NETBOX_PORT} directly, so there +# is no port-forward to supervise, and `up --wait` has already established +# readiness via the services' healthchecks. A port clash fails at `up`. +PRINT_NETBOX_TOKEN=0 "${REPO_ROOT}/tests/e2e/deploy_netbox.sh" + +# --- Phase 2: seed with netbox-manager ------------------------------------- +# The CLI is installed from the checkout so a Zuul Depends-On on a +# netbox-manager change is honored for code and data alike. It goes into a +# dedicated venv: netbox-manager pins different versions of packages that +# python-osism also pins (e.g. pynetbox), and installing it into the +# project venv would silently mutate those pins. +SEED_VENV="${SEED_VENV:-${REPO_ROOT}/.venv-sonic-e2e}" +if [[ ! -x "${SEED_VENV}/bin/pip" ]]; then + echo ">>> Creating seeding venv ${SEED_VENV}" + python3 -m venv "${SEED_VENV}" +fi +# netbox-manager drives Ansible through ansible-runner, which resolves +# ansible-playbook via PATH -- the venv's bin must therefore be on PATH, +# not merely used for the netbox-manager entry point itself. +export PATH="${SEED_VENV}/bin:${PATH}" +echo ">>> Installing netbox-manager from ${NETBOX_MANAGER_DIR}" +"${SEED_VENV}/bin/pip" install --quiet "${NETBOX_MANAGER_DIR}" + +echo ">>> Installing the netbox.netbox Ansible collection" +"${SEED_VENV}/bin/ansible-galaxy" collection install -r "${NETBOX_MANAGER_DIR}/requirements.yml" + +export NETBOX_MANAGER_URL="http://127.0.0.1:${NETBOX_PORT}" +export NETBOX_MANAGER_TOKEN="${NETBOX_TOKEN}" +export NETBOX_MANAGER_IGNORE_SSL_ERRORS=true + +# Seeding is serial by default because concurrent seeding DEADLOCKS. +# +# netbox-manager sorts resource files by leading number, runs the groups in +# order, and parallelises only within a group. The synthetic fixtures under +# tests/e2e/scenario/ follow the same numeric-group layout the retired example +# data used, so the same parallelism opportunity applies here too. +# +# That object-level analysis was not sufficient. The files share *foreign key +# parents*: every node file creates cables terminating on the shared switches, +# and inserting a row that references a device takes a KEY SHARE lock on that +# device's row for the FK check. Two transactions acquiring those parent locks +# in opposite orders deadlock: +# +# deadlock detected ... while locking tuple (1,3) in relation "dcim_device" +# SELECT 1 FROM ONLY "dcim_device" x WHERE "id" = $1 FOR KEY SHARE OF x +# +# Observed intermittently: two CI runs passed, the third failed this way, so +# roughly a one-in-three flake rate -- unusable as a default. It fails loudly +# rather than corrupting anything: the goldens are never at risk, because a +# deadlock aborts the run instead of silently reordering writes. +# +# The hazard has not gone away with the synthetic fixtures: 200-fabric.yml +# cables both leaves to a shared spine, so files in one numeric group still +# take KEY SHARE locks on the same parent dcim_device rows. Set +# SEED_PARALLEL=4 to opt back in -- worth revisiting only if netbox-manager +# gains deadlock retry. +SEED_PARALLEL="${SEED_PARALLEL:-1}" + +echo ">>> Seeding NetBox with the E2E fixtures (parallel: ${SEED_PARALLEL})" +export NETBOX_MANAGER_DEVICETYPE_LIBRARY="${REPO_ROOT}/tests/e2e/scenario/devicetypes" +export NETBOX_MANAGER_RESOURCES="${REPO_ROOT}/tests/e2e/scenario/resources" +"${SEED_VENV}/bin/netbox-manager" run --fail-fast --skipmtl --parallel "${SEED_PARALLEL}" + +# --- Phase 3: generate SONiC configurations --------------------------------- +# The conductor import chain needs ansible-core, which lives in the +# project's optional [ansible] extra (the container image installs it via +# requirements.ansible.txt). The unit tests stub it out in conftest.py, so +# a venv that runs the unit suite does not necessarily satisfy this import. +echo ">>> Ensuring the osism[ansible] extra is installed" +pipenv run pip install --quiet ".[ansible]" + +EXPORT_DIR="$(mktemp -d)" +export NETBOX_API="http://127.0.0.1:${NETBOX_PORT}" +export SONIC_EXPORT_DIR="${EXPORT_DIR}" +export SONIC_EXPORT_IDENTIFIER="hostname" +export SONIC_PORT_CONFIG_PATH="${REPO_ROOT}/files/sonic/port_config" + +echo ">>> Generating SONiC configurations (tests/e2e/generate.py)" +if [[ "${REGENERATE}" == "1" ]]; then + pipenv run python -m tests.e2e.generate --no-expect +else + pipenv run python -m tests.e2e.generate --golden "${GOLDEN_DIR}" +fi + +# --- Phase 4: compare against (or regenerate) the golden files ------------- +if [[ "${REGENERATE}" == "1" ]]; then + echo ">>> Regenerating golden files in ${GOLDEN_DIR}" + COMPARE_ARGS=(--golden "${GOLDEN_DIR}" --export "${EXPORT_DIR}" --regenerate) + if [[ "${ALLOW_COVERAGE_LOSS}" == "1" ]]; then + COMPARE_ARGS+=(--allow-coverage-loss) + fi + pipenv run python -m tests.e2e.compare "${COMPARE_ARGS[@]}" + echo ">>> Golden files regenerated; review and commit the diff." +else + echo ">>> Comparing exports against ${GOLDEN_DIR}" + pipenv run python -m tests.e2e.compare \ + --golden "${GOLDEN_DIR}" --export "${EXPORT_DIR}" + echo ">>> SONiC E2E golden test passed." +fi diff --git a/tests/unit/e2e/__init__.py b/tests/unit/e2e/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/e2e/test_compare.py b/tests/unit/e2e/test_compare.py new file mode 100644 index 00000000..43a1e60d --- /dev/null +++ b/tests/unit/e2e/test_compare.py @@ -0,0 +1,362 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for the E2E golden-file comparator. + +The comparator itself (tests/e2e/compare.py) is pure logic and independent +of any infrastructure, so it is tested here as part of the regular unit +suite. +""" + +import json + +from tests.e2e import compare +from tests.e2e.compare import compare_dirs, diff_paths, main, regenerate + + +class TestDiffPaths: + def test_equal_configs_yield_no_paths(self): + config = {"PORT": {"Ethernet0": {"speed": "100000"}}} + + assert diff_paths(config, config) == [] + + def test_value_mismatch_reports_table_entry_attribute_path(self): + golden = {"PORT": {"Ethernet4": {"speed": "100000"}}} + actual = {"PORT": {"Ethernet4": {"speed": "100"}}} + + assert diff_paths(golden, actual) == [ + "PORT|Ethernet4.speed: golden '100000', actual '100'" + ] + + def test_entry_only_in_golden(self): + golden = {"PORT": {"Ethernet4": {"speed": "100000"}}} + actual = {"PORT": {}} + + assert diff_paths(golden, actual) == ["PORT|Ethernet4: only in golden"] + + def test_entry_only_in_actual(self): + golden = {"PORT": {}} + actual = {"PORT": {"Ethernet4": {"speed": "100000"}}} + + assert diff_paths(golden, actual) == ["PORT|Ethernet4: only in actual"] + + def test_table_only_in_golden(self): + golden = {"VLAN": {"Vlan100": {}}} + actual = {} + + assert diff_paths(golden, actual) == ["VLAN: only in golden"] + + def test_array_order_is_significant(self): + golden = {"PORT": {"Ethernet0": {"valid_speeds": ["100000", "50000"]}}} + actual = {"PORT": {"Ethernet0": {"valid_speeds": ["50000", "100000"]}}} + + paths = diff_paths(golden, actual) + + assert paths == [ + "PORT|Ethernet0.valid_speeds[0]: golden '100000', actual '50000'", + "PORT|Ethernet0.valid_speeds[1]: golden '50000', actual '100000'", + ] + + def test_array_length_difference_reported(self): + golden = {"PORT": {"Ethernet0": {"valid_speeds": ["100000"]}}} + actual = {"PORT": {"Ethernet0": {"valid_speeds": ["100000", "50000"]}}} + + assert diff_paths(golden, actual) == [ + "PORT|Ethernet0.valid_speeds[1]: only in actual" + ] + + def test_type_mismatch_reported_as_value_difference(self): + golden = {"PORT": {"Ethernet0": {"mtu": "9100"}}} + actual = {"PORT": {"Ethernet0": {"mtu": 9100}}} + + assert diff_paths(golden, actual) == [ + "PORT|Ethernet0.mtu: golden '9100', actual 9100" + ] + + def test_multiple_differences_are_sorted_by_path(self): + golden = { + "PORT": {"Ethernet0": {"speed": "100000"}}, + "VLAN": {"Vlan100": {"vlanid": "100"}}, + } + actual = { + "PORT": {"Ethernet0": {"speed": "100"}}, + "VLAN": {"Vlan100": {"vlanid": "200"}}, + } + + paths = diff_paths(golden, actual) + + assert paths == sorted(paths) + assert len(paths) == 2 + + def test_deeply_nested_values_use_dot_separators(self): + golden = {"BGP_NEIGHBOR": {"10.0.0.1": {"af": {"ipv4": "on"}}}} + actual = {"BGP_NEIGHBOR": {"10.0.0.1": {"af": {"ipv4": "off"}}}} + + assert diff_paths(golden, actual) == [ + "BGP_NEIGHBOR|10.0.0.1.af.ipv4: golden 'on', actual 'off'" + ] + + +class TestCompareDirs: + @staticmethod + def _write(directory, name, config): + directory.mkdir(parents=True, exist_ok=True) + (directory / name).write_text(json.dumps(config)) + + def test_identical_dirs_are_ok(self, tmp_path): + config = {"PORT": {"Ethernet0": {"speed": "100000"}}} + self._write(tmp_path / "golden", "osism_sw1_config_db.json", config) + self._write(tmp_path / "export", "osism_sw1_config_db.json", config) + + result = compare_dirs(tmp_path / "golden", tmp_path / "export") + + assert result.ok + assert result.missing == [] + assert result.extra == [] + assert result.mismatched == {} + + def test_missing_export_is_reported(self, tmp_path): + self._write(tmp_path / "golden", "osism_sw1_config_db.json", {}) + (tmp_path / "export").mkdir() + + result = compare_dirs(tmp_path / "golden", tmp_path / "export") + + assert not result.ok + assert result.missing == ["osism_sw1_config_db.json"] + + def test_extra_export_is_reported(self, tmp_path): + (tmp_path / "golden").mkdir() + self._write(tmp_path / "export", "osism_sw2_config_db.json", {}) + + result = compare_dirs(tmp_path / "golden", tmp_path / "export") + + assert not result.ok + assert result.extra == ["osism_sw2_config_db.json"] + + def test_mismatch_collects_paths_and_unified_diff(self, tmp_path): + name = "osism_sw1_config_db.json" + self._write( + tmp_path / "golden", name, {"PORT": {"Ethernet4": {"speed": "100000"}}} + ) + self._write( + tmp_path / "export", name, {"PORT": {"Ethernet4": {"speed": "100"}}} + ) + + result = compare_dirs(tmp_path / "golden", tmp_path / "export") + + assert not result.ok + mismatch = result.mismatched[name] + assert mismatch.paths == ["PORT|Ethernet4.speed: golden '100000', actual '100'"] + assert '- "speed": "100000"' in mismatch.diff + assert '+ "speed": "100"' in mismatch.diff + + def test_non_json_files_are_ignored(self, tmp_path): + config = {"PORT": {}} + self._write(tmp_path / "golden", "osism_sw1_config_db.json", config) + self._write(tmp_path / "export", "osism_sw1_config_db.json", config) + (tmp_path / "export" / "firmware_sw1.bin").write_text("not json") + + result = compare_dirs(tmp_path / "golden", tmp_path / "export") + + assert result.ok + + def test_report_distinguishes_failure_categories(self, tmp_path): + self._write(tmp_path / "golden", "osism_sw1_config_db.json", {"A": {}}) + self._write(tmp_path / "golden", "osism_sw2_config_db.json", {}) + self._write(tmp_path / "export", "osism_sw1_config_db.json", {"B": {}}) + self._write(tmp_path / "export", "osism_sw3_config_db.json", {}) + + result = compare_dirs(tmp_path / "golden", tmp_path / "export") + report = result.report() + + assert "osism_sw2_config_db.json" in report + assert "generation or export failed" in report + assert "osism_sw3_config_db.json" in report + assert "unexpected extra" in report + assert "osism_sw1_config_db.json" in report + assert "A: only in golden" in report + + +class TestRegenerate: + def test_copies_exports_to_golden_canonically(self, tmp_path): + export = tmp_path / "export" + golden = tmp_path / "golden" + export.mkdir() + (export / "osism_sw1_config_db.json").write_text('{"B": {}, "A": {}}') + + regenerate(golden, export) + + content = (golden / "osism_sw1_config_db.json").read_text() + assert content == '{\n "A": {},\n "B": {}\n}\n' + + def test_removes_stale_golden_files(self, tmp_path): + export = tmp_path / "export" + golden = tmp_path / "golden" + export.mkdir() + golden.mkdir() + (export / "osism_sw1_config_db.json").write_text("{}") + (golden / "osism_gone_config_db.json").write_text("{}") + + regenerate(golden, export) + + assert not (golden / "osism_gone_config_db.json").exists() + assert (golden / "osism_sw1_config_db.json").exists() + + def test_ignores_non_json_exports(self, tmp_path): + export = tmp_path / "export" + golden = tmp_path / "golden" + export.mkdir() + (export / "firmware_sw1.bin").write_text("binary") + (export / "osism_sw1_config_db.json").write_text("{}") + + regenerate(golden, export) + + assert not (golden / "firmware_sw1.bin").exists() + + +class TestMain: + @staticmethod + def _dirs(tmp_path, golden_config, export_config): + golden = tmp_path / "golden" + export = tmp_path / "export" + golden.mkdir() + export.mkdir() + name = "osism_sw1_config_db.json" + (golden / name).write_text(json.dumps(golden_config)) + (export / name).write_text(json.dumps(export_config)) + return golden, export + + def test_matching_dirs_exit_zero(self, tmp_path, capsys): + golden, export = self._dirs(tmp_path, {"A": {}}, {"A": {}}) + + rc = main(["--golden", str(golden), "--export", str(export)]) + + assert rc == 0 + assert "OK" in capsys.readouterr().out + + def test_mismatch_exits_nonzero_and_prints_report(self, tmp_path, capsys): + golden, export = self._dirs(tmp_path, {"A": {}}, {"B": {}}) + + rc = main(["--golden", str(golden), "--export", str(export)]) + + assert rc == 1 + out = capsys.readouterr().out + assert "A: only in golden" in out + + def test_regenerate_rewrites_goldens_and_exits_zero(self, tmp_path): + golden, export = self._dirs(tmp_path, {"A": {}}, {"B": {}}) + + rc = main(["--golden", str(golden), "--export", str(export), "--regenerate"]) + + assert rc == 0 + name = "osism_sw1_config_db.json" + assert json.loads((golden / name).read_text()) == {"B": {}} + + def test_regenerate_coverage_loss_exits_nonzero_without_flag( + self, tmp_path, capsys + ): + """Without --allow-coverage-loss, main() reports the loss and fails.""" + golden, export = self._dirs(tmp_path, {"VLAN": {"Vlan100": {}}}, {"VLAN": {}}) + + rc = main(["--golden", str(golden), "--export", str(export), "--regenerate"]) + + assert rc == 1 + out = capsys.readouterr().out + assert "COVERAGE LOSS" in out + assert "osism_sw1_config_db.json: VLAN" in out + # Goldens are still written even though the run is reported failed. + name = "osism_sw1_config_db.json" + assert json.loads((golden / name).read_text()) == {"VLAN": {}} + + def test_regenerate_coverage_loss_exits_zero_with_flag(self, tmp_path, capsys): + """--allow-coverage-loss makes the same loss a successful regen.""" + golden, export = self._dirs(tmp_path, {"VLAN": {"Vlan100": {}}}, {"VLAN": {}}) + + rc = main( + [ + "--golden", + str(golden), + "--export", + str(export), + "--regenerate", + "--allow-coverage-loss", + ] + ) + + assert rc == 0 + out = capsys.readouterr().out + assert "COVERAGE LOSS" not in out + name = "osism_sw1_config_db.json" + assert json.loads((golden / name).read_text()) == {"VLAN": {}} + + +def test_regenerate_reports_emptied_table(tmp_path): + """A table that was populated and becomes empty is reported.""" + golden = tmp_path / "golden" + export = tmp_path / "export" + golden.mkdir() + export.mkdir() + (golden / "osism_sw_config_db.json").write_text( + json.dumps({"PORT": {"Ethernet0": {}}, "VLAN": {"Vlan100": {}}}) + ) + (export / "osism_sw_config_db.json").write_text( + json.dumps({"PORT": {"Ethernet0": {}}, "VLAN": {}}) + ) + + lost = compare.regenerate(golden, export) + + assert lost == ["osism_sw_config_db.json: VLAN"] + + +def test_regenerate_ignores_tables_empty_in_both(tmp_path): + """A table empty before and after is not a loss.""" + golden = tmp_path / "golden" + export = tmp_path / "export" + golden.mkdir() + export.mkdir() + (golden / "osism_sw_config_db.json").write_text(json.dumps({"NTP_SERVER": {}})) + (export / "osism_sw_config_db.json").write_text(json.dumps({"NTP_SERVER": {}})) + + assert compare.regenerate(golden, export) == [] + + +def test_regenerate_still_writes_when_coverage_lost(tmp_path): + """The guard reports; it does not refuse to write.""" + golden = tmp_path / "golden" + export = tmp_path / "export" + golden.mkdir() + export.mkdir() + (golden / "osism_sw_config_db.json").write_text( + json.dumps({"VLAN": {"Vlan100": {}}}) + ) + (export / "osism_sw_config_db.json").write_text(json.dumps({"VLAN": {}})) + + compare.regenerate(golden, export) + + assert json.loads((golden / "osism_sw_config_db.json").read_text()) == {"VLAN": {}} + + +def test_regenerate_ignores_new_file(tmp_path): + """A device with no previous golden cannot lose coverage.""" + golden = tmp_path / "golden" + export = tmp_path / "export" + golden.mkdir() + export.mkdir() + (export / "osism_new_config_db.json").write_text(json.dumps({"VLAN": {}})) + + assert compare.regenerate(golden, export) == [] + + +def test_regenerate_reports_removed_file_with_populated_tables(tmp_path): + """A device that stops being exported at all is the largest coverage loss.""" + golden = tmp_path / "golden" + export = tmp_path / "export" + golden.mkdir() + export.mkdir() + (golden / "osism_gone_config_db.json").write_text( + json.dumps({"PORT": {"Ethernet0": {}}, "VLAN": {"Vlan100": {}}}) + ) + + lost = compare.regenerate(golden, export) + + assert lost == ["osism_gone_config_db.json: file removed, had 2 populated tables"] + assert not (golden / "osism_gone_config_db.json").exists() diff --git a/tests/unit/e2e/test_coverage.py b/tests/unit/e2e/test_coverage.py new file mode 100644 index 00000000..3471c48a --- /dev/null +++ b/tests/unit/e2e/test_coverage.py @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Gate on the golden set covering every config_db table the generator emits. + +tests/e2e/coverage.py is the human-facing report; this is the check that +actually fails. It lives in the unit suite rather than in the E2E job because +it needs neither NetBox nor a generated config -- only the generator source +and the committed goldens -- so it costs milliseconds and runs on every +change, instead of only when the E2E job's file matcher fires. + +What it catches is the one coverage failure nothing else does: a newly +emitted table that arrives with no golden. Coverage going the other way (a +table that had a golden becoming empty) already fails the golden comparison, +because the golden file itself changes. +""" + +from tests.e2e.coverage import covered_tables, emitted_tables + + +def test_every_emitted_table_has_golden_coverage(): + missing = sorted(emitted_tables() - covered_tables()) + + assert not missing, ( + "the generator can emit these config_db tables, but they are empty in " + "every file under tests/e2e/golden/: " + ", ".join(missing) + ". Seed a " + "device that populates them under tests/e2e/scenario/resources/ and " + "rewrite the goldens with `make sonic-e2e-regen`, or -- if the table " + "cannot be reached by any fixture -- exclude it here with a comment " + "saying why." + ) diff --git a/tests/unit/e2e/test_generate.py b/tests/unit/e2e/test_generate.py new file mode 100644 index 00000000..2e3a831d --- /dev/null +++ b/tests/unit/e2e/test_generate.py @@ -0,0 +1,108 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for the E2E generation driver. + +The driver's assertion logic (expected device set, loguru error capture) +is pure and tested here with an injected sync callable; only the real +``sync_sonic`` import is exercised in the actual E2E run. +""" + +from loguru import logger + +from tests.e2e.generate import expected_devices, main, run_generation + + +class TestExpectedDevices: + def test_strips_prefix_and_suffix(self, tmp_path): + (tmp_path / "osism_testbed-switch-0_config_db.json").write_text("{}") + (tmp_path / "osism_testbed-switch-1_config_db.json").write_text("{}") + + assert expected_devices(tmp_path, "osism_", "_config_db.json") == { + "testbed-switch-0", + "testbed-switch-1", + } + + def test_ignores_non_json_and_foreign_names(self, tmp_path): + (tmp_path / "osism_sw1_config_db.json").write_text("{}") + (tmp_path / "README.md").write_text("") + (tmp_path / "other_sw2.json").write_text("{}") + + assert expected_devices(tmp_path, "osism_", "_config_db.json") == {"sw1"} + + +class TestRunGeneration: + def test_returns_configs_and_no_errors_for_clean_sync(self): + configs, errors = run_generation(lambda: {"sw1": {"PORT": {}}}) + + assert configs == {"sw1": {"PORT": {}}} + assert errors == [] + + def test_captures_loguru_errors_during_sync(self): + def failing_sync(): + logger.error("Failed to sync SONiC configuration for device sw1: boom") + return {} + + configs, errors = run_generation(failing_sync) + + assert configs == {} + assert len(errors) == 1 + assert "boom" in errors[0] + + def test_does_not_capture_non_error_levels(self): + def chatty_sync(): + logger.info("processing") + logger.warning("odd but fine") + return {"sw1": {}} + + _, errors = run_generation(chatty_sync) + + assert errors == [] + + def test_sink_is_removed_after_run(self): + _, errors = run_generation(lambda: {}) + logger.error("logged after the run") + + assert errors == [] + + +class TestMain: + def test_success_returns_zero(self, tmp_path, capsys): + (tmp_path / "osism_sw1_config_db.json").write_text("{}") + + rc = main(["--golden", str(tmp_path)], sync=lambda: {"sw1": {"PORT": {}}}) + + assert rc == 0 + + def test_captured_errors_fail_the_run(self, tmp_path, capsys): + (tmp_path / "osism_sw1_config_db.json").write_text("{}") + + def failing_sync(): + logger.error("device sw1 exploded") + return {"sw1": {}} + + rc = main(["--golden", str(tmp_path)], sync=failing_sync) + + assert rc == 1 + assert "device sw1 exploded" in capsys.readouterr().out + + def test_device_set_mismatch_fails_and_names_devices(self, tmp_path, capsys): + (tmp_path / "osism_sw1_config_db.json").write_text("{}") + (tmp_path / "osism_sw2_config_db.json").write_text("{}") + + rc = main(["--golden", str(tmp_path)], sync=lambda: {"sw1": {"PORT": {}}}) + + assert rc == 1 + assert "sw2" in capsys.readouterr().out + + def test_empty_config_counts_as_failure(self, tmp_path, capsys): + (tmp_path / "osism_sw1_config_db.json").write_text("{}") + + rc = main(["--golden", str(tmp_path)], sync=lambda: {"sw1": {}}) + + assert rc == 1 + assert "sw1" in capsys.readouterr().out + + def test_no_expect_skips_device_set_check(self, capsys): + rc = main(["--no-expect"], sync=lambda: {"sw1": {"PORT": {}}}) + + assert rc == 0 diff --git a/tests/unit/tasks/conductor/sonic/_detection_helpers.py b/tests/unit/tasks/conductor/sonic/_detection_helpers.py index e970a515..d89746a6 100644 --- a/tests/unit/tasks/conductor/sonic/_detection_helpers.py +++ b/tests/unit/tasks/conductor/sonic/_detection_helpers.py @@ -9,6 +9,8 @@ from pathlib import Path from types import SimpleNamespace +_DEFAULT = object() + def repo_root(): """Return the repository root, found by its ``setup.cfg`` marker. @@ -23,12 +25,16 @@ def repo_root(): raise RuntimeError("no repository root with setup.cfg above this file") -def _make_sonic_device(device_id=1, name="sw1", hwsku="TEST-HWSKU"): +def _make_sonic_device( + device_id=1, name="sw1", hwsku="TEST-HWSKU", custom_fields=_DEFAULT +): """Build a NetBox device stub carrying ``custom_fields.sonic_parameters.hwsku``.""" + if custom_fields is _DEFAULT: + custom_fields = {"sonic_parameters": {"hwsku": hwsku}} return SimpleNamespace( id=device_id, name=name, - custom_fields={"sonic_parameters": {"hwsku": hwsku}}, + custom_fields=custom_fields, ) diff --git a/tests/unit/tasks/conductor/sonic/test_breakout_detection.py b/tests/unit/tasks/conductor/sonic/test_breakout_detection.py index 8c34d535..19ed0144 100644 --- a/tests/unit/tasks/conductor/sonic/test_breakout_detection.py +++ b/tests/unit/tasks/conductor/sonic/test_breakout_detection.py @@ -14,7 +14,13 @@ import pytest from osism.tasks.conductor.sonic import interface as interface_module -from osism.tasks.conductor.sonic.interface import detect_breakout_ports +from osism.tasks.conductor.sonic.interface import ( + detect_breakout_ports, + _emit_breakout, + _parse_lanes, + _parse_breakout_mode, + get_declared_breakout_modes, +) from ._detection_helpers import _make_iface, _make_sonic_device, repo_root @@ -570,6 +576,363 @@ def test_detect_breakout_ports_sonic_standard_speed_resolved_from_port_type( assert result["breakout_cfgs"]["Ethernet0"]["brkout_mode"] == "4x25G" +# --------------------------------------------------------------------------- +# _parse_lanes +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "s,out", + [ + ("1,2,3,4", ["1", "2", "3", "4"]), + ( + "73,74,75,76,77,78,79,80", + ["73", "74", "75", "76", "77", "78", "79", "80"], + ), + ("1-4", ["1", "2", "3", "4"]), + ("29", ["29"]), + ("", []), + (" ", []), + ("a,b", []), + ("4-1", []), + ("1-", []), + ("x-3", []), + (None, []), + (29, []), + (["1", "2"], []), + ], +) +def test_parse_lanes(s, out): + assert _parse_lanes(s) == out + + +# --------------------------------------------------------------------------- +# _parse_breakout_mode +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "m,out", + [ + ("4x10G", (4, 10000)), + ("2x50G", (2, 50000)), + ("8x50G", (8, 50000)), + ("4x100G", (4, 100000)), + ("2x200G", (2, 200000)), + ("1x400G", None), + ("0x10G", None), + ("4x0G", None), + ("4x10g", None), + (" 4x10G ", (4, 10000)), + ("bogus", None), + ("4x", None), + ("x10G", None), + ("", None), + (None, None), + (10, None), + ({}, None), + ], +) +def test_parse_breakout_mode(m, out): + assert _parse_breakout_mode(m) == out + + +# --------------------------------------------------------------------------- +# get_declared_breakout_modes +# --------------------------------------------------------------------------- + + +def test_declared_modes_map(): + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": { + "hwsku": "x", + "breakout": {"Ethernet0": "4x10G", "Eth1/9": "2x50G"}, + } + } + ) + assert get_declared_breakout_modes(d) == {"Ethernet0": "4x10G", "Eth1/9": "2x50G"} + + +@pytest.mark.parametrize( + "cf", + [ + {"sonic_parameters": {"hwsku": "x"}}, + {"sonic_parameters": {"breakout": None}}, + {"sonic_parameters": {"breakout": "Ethernet0: 4x10G"}}, + {"sonic_parameters": {"breakout": []}}, + {"sonic_parameters": None}, + {}, + None, + ], +) +def test_declared_modes_malformed_or_absent(cf): + assert get_declared_breakout_modes(_make_sonic_device(custom_fields=cf)) == {} + + +# --------------------------------------------------------------------------- +# detect_breakout_ports — declared-mode pass (Task 5) +# --------------------------------------------------------------------------- + + +_GATE_PC = { + "Ethernet0": { + "lanes": "1,2,3,4", + "alias": "Eth1/1", + "index": "1", + "speed": "25000", + } +} # Eth1/1/2/3 absent -> gate accepts + + +def test_declared_4x10g(patch_breakout_helpers): + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": {"hwsku": "x", "breakout": {"Ethernet0": "4x10G"}} + } + ) + patch_breakout_helpers( + interfaces=[], port_config=_port_config_for_port(lanes="1,2,3,4") + ) + r = detect_breakout_ports(d) + assert r["breakout_cfgs"]["Ethernet0"]["brkout_mode"] == "4x10G" + assert {k: v["lanes"] for k, v in r["breakout_ports"].items()} == { + "Ethernet0": "1", + "Ethernet1": "2", + "Ethernet2": "3", + "Ethernet3": "4", + } + assert all(v["declared"] for v in r["breakout_ports"].values()) + + +def test_declared_2x50g(patch_breakout_helpers): + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": {"hwsku": "x", "breakout": {"Ethernet0": "2x50G"}} + } + ) + patch_breakout_helpers( + interfaces=[], port_config=_port_config_for_port(lanes="1,2,3,4") + ) + assert { + k: v["lanes"] for k, v in detect_breakout_ports(d)["breakout_ports"].items() + } == {"Ethernet0": "1,2", "Ethernet2": "3,4"} + + +def test_declared_8x50g(patch_breakout_helpers): + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": {"hwsku": "x", "breakout": {"Ethernet0": "8x50G"}} + } + ) + patch_breakout_helpers( + interfaces=[], port_config=_port_config_for_port(lanes="1,2,3,4,5,6,7,8") + ) + r = detect_breakout_ports(d) + assert set(r["breakout_ports"]) == {f"Ethernet{n}" for n in range(8)} + assert r["breakout_ports"]["Ethernet0"]["lanes"] == "1" + + +def test_declared_4x100g_8lane(patch_breakout_helpers): + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": {"hwsku": "x", "breakout": {"Ethernet0": "4x100G"}} + } + ) + patch_breakout_helpers( + interfaces=[], port_config=_port_config_for_port(lanes="1,2,3,4,5,6,7,8") + ) + assert { + k: v["lanes"] for k, v in detect_breakout_ports(d)["breakout_ports"].items() + } == { + "Ethernet0": "1,2", + "Ethernet2": "3,4", + "Ethernet4": "5,6", + "Ethernet6": "7,8", + } + + +def test_declared_key_eth1(patch_breakout_helpers): + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": { + "hwsku": "x", + "breakout": {"Eth1/1": "4x10G"}, + } + } + ) + patch_breakout_helpers( + interfaces=[], + port_config=_port_config_for_port(lanes="1,2,3,4", alias="Eth1/1"), + ) + assert ( + detect_breakout_ports(d)["breakout_cfgs"]["Ethernet0"]["brkout_mode"] == "4x10G" + ) + + +def test_declared_mixed_layout_port_index(patch_breakout_helpers): + pc = { + **{ + f"Ethernet{n}": { + "lanes": str(29 + n), + "alias": f"Eth{n + 1}(Port{n + 1})", + "index": str(n + 1), + "speed": "25000", + } + for n in range(12) + }, + "Ethernet12": { + "lanes": "41,42,43,44", + "alias": "Eth13(Port13)", + "index": "13", + "speed": "100000", + }, + } + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": { + "hwsku": "x", + "breakout": {"Ethernet12": "4x25G"}, + } + } + ) + patch_breakout_helpers(interfaces=[], port_config=pc) + assert detect_breakout_ports(d)["breakout_cfgs"]["Ethernet12"]["port"] == "1/13" + + +def test_inference_control(patch_breakout_helpers): + ifaces = [_make_iface(f"Ethernet{n}", speed=25_000_000) for n in range(4)] + patch_breakout_helpers(interfaces=ifaces, port_config=_GATE_PC) + assert ( + detect_breakout_ports(_make_sonic_device())["breakout_cfgs"]["Ethernet0"][ + "brkout_mode" + ] + == "4x25G" + ) + + +def test_invalid_mode_suppresses_inference(patch_breakout_helpers): + ifaces = [_make_iface(f"Ethernet{n}", speed=25_000_000) for n in range(4)] + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": { + "hwsku": "x", + "breakout": {"Ethernet0": "3x25G"}, + } + } + ) # 4 % 3 != 0 + patch_breakout_helpers(interfaces=ifaces, port_config=_GATE_PC) + assert detect_breakout_ports(d) == {"breakout_cfgs": {}, "breakout_ports": {}} + + +def test_collision_fail_closed(patch_breakout_helpers): + ifaces = [_make_iface(f"Ethernet{n}", speed=25_000_000) for n in range(4)] + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": { + "hwsku": "x", + "breakout": {"Ethernet0": "4x25G", "Eth1/1": "4x25G"}, + } + } + ) + patch_breakout_helpers(interfaces=ifaces, port_config=_GATE_PC) + assert detect_breakout_ports(d) == {"breakout_cfgs": {}, "breakout_ports": {}} + + +def test_unresolvable_and_malformed(patch_breakout_helpers): + for bmap in ( + {"Ethernetfoo": "4x10G"}, + {"Eth1/1/1": "4x10G"}, + {"Eth2/1": "4x10G"}, + {"Ethernet99": "4x10G"}, + {123: "4x10G"}, + ): + d = _make_sonic_device( + custom_fields={"sonic_parameters": {"hwsku": "x", "breakout": bmap}} + ) + patch_breakout_helpers( + interfaces=[], port_config=_port_config_for_port(lanes="1,2,3,4") + ) + assert detect_breakout_ports(d) == { + "breakout_cfgs": {}, + "breakout_ports": {}, + } + + +def test_declared_single_lane_master_no_emit_and_suppressed(patch_breakout_helpers): + """A declared 4x25G on a single-lane master fails L%count validation + (L=1). No breakout is emitted, and the master is suppressed so the + NetBox-format inference that would otherwise fire (Eth1/49/1..4 at 25G) + also emits nothing.""" + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": {"hwsku": "x", "breakout": {"Ethernet0": "4x25G"}} + } + ) + interfaces = _netbox_breakout_interfaces(speed=25_000_000) + port_config = _port_config_for_port(lanes="29", speed="25000") + patch_breakout_helpers(interfaces=interfaces, port_config=port_config) + assert detect_breakout_ports(d) == {"breakout_cfgs": {}, "breakout_ports": {}} + + +def test_multiple_unresolvable_keys_logged_per_key(patch_breakout_helpers, loguru_logs): + """Two unresolvable keys both normalize to master=None and land in the + same bucket. That is not a collision: each key should get its own + "could not be resolved" message, not a misleading "collision for None".""" + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": { + "hwsku": "x", + "breakout": {"Ethernetfoo": "4x10G", "Eth2/1": "4x10G"}, + } + } + ) + patch_breakout_helpers( + interfaces=[], port_config=_port_config_for_port(lanes="1,2,3,4") + ) + assert detect_breakout_ports(d) == {"breakout_cfgs": {}, "breakout_ports": {}} + + messages = [r["message"] for r in loguru_logs] + assert not any("collision for None" in m for m in messages) + for key in ("Ethernetfoo", "Eth2/1"): + assert any("could not be resolved" in m and key in m for m in messages), key + + +def test_declared_malformed_master_lanes_no_emit(patch_breakout_helpers): + """Malformed lanes on the declared master parse to [] (L==0): no emit, + master suppressed (the NetBox-format inference is silenced too).""" + d = _make_sonic_device( + custom_fields={ + "sonic_parameters": {"hwsku": "x", "breakout": {"Ethernet0": "4x25G"}} + } + ) + interfaces = _netbox_breakout_interfaces(speed=25_000_000) + port_config = _port_config_for_port(lanes="a,b", speed="25000") + patch_breakout_helpers(interfaces=interfaces, port_config=port_config) + assert detect_breakout_ports(d) == {"breakout_cfgs": {}, "breakout_ports": {}} + + +def test_emit_breakout_missing_index_leaves_no_partial_state(): + """A master lacking ``index`` must fail before any children are staged, + so no orphan breakout_ports entries survive the error.""" + port_config = {"Ethernet0": {"lanes": "1,2,3,4"}} # no "index" + breakout_cfgs: dict = {} + breakout_ports: dict = {} + suppressed: set = set() + with pytest.raises(KeyError): + _emit_breakout( + "Ethernet0", + 2, + 50000, + port_config, + breakout_cfgs, + breakout_ports, + suppressed, + ) + assert breakout_ports == {} + assert breakout_cfgs == {} + + # --------------------------------------------------------------------------- # Child slots occupied by another port # --------------------------------------------------------------------------- @@ -693,3 +1056,56 @@ def test_sonic_400g_breakout_refused_when_child_slot_is_another_port( assert result["breakout_cfgs"] == {} assert result["breakout_ports"] == {} + + +@pytest.mark.parametrize("mode", ["4x25G", "2x50G"]) +def test_declared_breakout_refused_when_child_slot_is_another_port( + patch_breakout_helpers, real_port_config, mode +): + """Ethernet124 on Accton-AS7726-32X carries four lanes, but Ethernet125 + and Ethernet126 are independent 10G SFP+ ports sitting in two of its four + child slots. Emitting the breakout would rewrite their lanes, speed and + alias, so the declaration has to be dropped whole -- no BREAKOUT_CFG entry + either, which keeps Ethernet124 a normal 100G port. + """ + port_config = real_port_config("Accton-AS7726-32X") + assert {"Ethernet124", "Ethernet125", "Ethernet126"} <= set(port_config) + + device = _make_sonic_device( + custom_fields={ + "sonic_parameters": {"hwsku": "x", "breakout": {"Ethernet124": mode}} + } + ) + patch_breakout_helpers(interfaces=[], port_config=port_config) + + result = detect_breakout_ports(device) + + assert result["breakout_ports"] == {} + assert result["breakout_cfgs"] == {} + + +def test_declared_breakout_allowed_when_child_slots_are_free( + patch_breakout_helpers, real_port_config +): + """The guard must not reject an ordinary port on the same HWSKU: the + children of Ethernet0 are Ethernet1-3, none of which is a port in its own + right, so 4x25G there stays valid. + """ + port_config = real_port_config("Accton-AS7726-32X") + + device = _make_sonic_device( + custom_fields={ + "sonic_parameters": {"hwsku": "x", "breakout": {"Ethernet0": "4x25G"}} + } + ) + patch_breakout_helpers(interfaces=[], port_config=port_config) + + result = detect_breakout_ports(device) + + assert result["breakout_cfgs"]["Ethernet0"]["brkout_mode"] == "4x25G" + assert sorted(result["breakout_ports"]) == [ + "Ethernet0", + "Ethernet1", + "Ethernet2", + "Ethernet3", + ] diff --git a/tests/unit/tasks/conductor/sonic/test_config_generator_orchestrator.py b/tests/unit/tasks/conductor/sonic/test_config_generator_orchestrator.py index c292fe28..6332cce4 100644 --- a/tests/unit/tasks/conductor/sonic/test_config_generator_orchestrator.py +++ b/tests/unit/tasks/conductor/sonic/test_config_generator_orchestrator.py @@ -495,6 +495,30 @@ def test_generate_sonic_config_merges_breakout_cfgs_and_ports( assert config["BREAKOUT_PORTS"]["Ethernet0"] == {"master": "Ethernet0"} +def test_generate_sonic_config_breakout_ports_projected_to_master_only( + mocker, patch_orchestrator_helpers, make_orchestrator_device +): + """Declared-mode stash keys must not leak into the owned BREAKOUT_PORTS + table; only ``{"master": }`` belongs in the device config.""" + patch_base_config(mocker) + patch_orchestrator_helpers.detect_breakout_ports.return_value = { + "breakout_cfgs": {"Ethernet0": {"brkout_mode": "2x50G"}}, + "breakout_ports": { + "Ethernet0": { + "master": "Ethernet0", + "declared": True, + "lanes": "1,2", + "speed": 50000, + } + }, + } + device = make_orchestrator_device() + + config = generate_sonic_config(device, "HWSKU") + + assert config["BREAKOUT_PORTS"]["Ethernet0"] == {"master": "Ethernet0"} + + # --------------------------------------------------------------------------- # generate_sonic_config — config_version normalization # --------------------------------------------------------------------------- diff --git a/tests/unit/tasks/conductor/sonic/test_config_generator_ports.py b/tests/unit/tasks/conductor/sonic/test_config_generator_ports.py index 4fd03ee0..047934ee 100644 --- a/tests/unit/tasks/conductor/sonic/test_config_generator_ports.py +++ b/tests/unit/tasks/conductor/sonic/test_config_generator_ports.py @@ -352,6 +352,55 @@ def test_breakout_port_index_copied_from_master( assert config["PORT"]["Ethernet1"]["index"] == "42" + def test_declared_child_in_port_config_uses_declared_speed_lanes( + self, config, device, mocker + ): + """A declared non-master child that also exists in port_config is + processed by the main loop; the declared stash must win over the + NetBox-speed override and the inferred lane calculation.""" + mocker.patch.object( + config_generator, "convert_sonic_interface_to_alias", return_value="a" + ) + port_config = { + "Ethernet0": _port_info(index="1", lanes="1,2,3,4", speed="100000"), + "Ethernet2": _port_info(index="1", lanes="3,4", speed="25000"), + } + breakout_info = { + "breakout_cfgs": { + "Ethernet0": {"breakout_owner": "MANUAL", "brkout_mode": "2x50G"} + }, + "breakout_ports": { + "Ethernet0": { + "master": "Ethernet0", + "declared": True, + "lanes": "1,2", + "speed": 50000, + }, + "Ethernet2": { + "master": "Ethernet0", + "declared": True, + "lanes": "3,4", + "speed": 50000, + }, + }, + } + # STALE explicit NetBox 25G that must be overridden by the declared stash + netbox_interfaces = {"Ethernet2": _nb_iface(speed=25000, speed_explicit=True)} + + _add_port_configurations( + config, + port_config, + connected_interfaces=set(), + portchannel_info={"portchannels": {}, "member_mapping": {}}, + breakout_info=breakout_info, + netbox_interfaces=netbox_interfaces, + vlan_info={"vlan_members": {}}, + device=device, + ) + + assert config["PORT"]["Ethernet2"]["speed"] == "50000" + assert config["PORT"]["Ethernet2"]["lanes"] == "3,4" + def test_default_port_data_keys( self, config, device, mocker, patch_post_loop_hooks ): @@ -850,6 +899,82 @@ def test_alias_called_with_breakout_flag(self, config, mocker): "Ethernet1", 25000, is_breakout=True, port_config=port_config ) + def test_declared_authoritative(self, config): + breakout_info = { + "breakout_cfgs": { + "Ethernet0": { + "breakout_owner": "MANUAL", + "brkout_mode": "2x50G", + "port": "1/1", + } + }, + "breakout_ports": { + "Ethernet0": { + "master": "Ethernet0", + "declared": True, + "lanes": "1,2", + "speed": 50000, + }, + "Ethernet2": { + "master": "Ethernet0", + "declared": True, + "lanes": "3,4", + "speed": 50000, + }, + }, + } + pc = { + "Ethernet0": { + "lanes": "1,2,3,4", + "alias": "Eth1/1", + "index": "1", + "speed": "100000", + } + } + nb = { + "Ethernet0": {"speed": 25000}, + "Ethernet2": {"speed": 25000}, + } # STALE explicit 25G + _add_missing_breakout_ports( + config, + breakout_info, + pc, + connected_interfaces=set(), + portchannel_info={"portchannels": {}, "member_mapping": {}}, + netbox_interfaces=nb, + ) + assert config["PORT"]["Ethernet0"]["speed"] == "50000" + assert config["PORT"]["Ethernet0"]["lanes"] == "1,2" + assert config["PORT"]["Ethernet2"]["speed"] == "50000" + assert config["PORT"]["Ethernet2"]["lanes"] == "3,4" + + def test_inferred_unchanged(self, config, mocker): + mocker.patch.object( + config_generator, "convert_sonic_interface_to_alias", return_value="a" + ) + breakout_info = { + "breakout_cfgs": {"Ethernet0": {"brkout_mode": "4x25G"}}, + "breakout_ports": {"Ethernet1": {"master": "Ethernet0"}}, + } + pc = { + "Ethernet0": { + "lanes": "1,2,3,4", + "alias": "Eth1/1", + "index": "1", + "speed": "100000", + } + } + nb = {"Ethernet1": {"speed": 25000}} + _add_missing_breakout_ports( + config, + breakout_info, + pc, + connected_interfaces=set(), + portchannel_info={"portchannels": {}, "member_mapping": {}}, + netbox_interfaces=nb, + ) + assert config["PORT"]["Ethernet1"]["speed"] == "25000" # existing behavior + # --------------------------------------------------------------------------- # _add_tagged_vlans_to_ports diff --git a/tests/unit/tasks/conductor/sonic/test_constants.py b/tests/unit/tasks/conductor/sonic/test_constants.py index f65e8cc8..3c4c72bf 100644 --- a/tests/unit/tasks/conductor/sonic/test_constants.py +++ b/tests/unit/tasks/conductor/sonic/test_constants.py @@ -1,7 +1,11 @@ # SPDX-License-Identifier: Apache-2.0 +import importlib + import pytest +from osism import settings as settings_module +from osism.tasks.conductor.sonic import constants as constants_module from osism.tasks.conductor.sonic.constants import ( BGP_AF_L2VPN_EVPN_TAG, DEFAULT_LOCAL_AS_PREFIX, @@ -151,3 +155,19 @@ def test_supported_hwskus_entry_invariants(hwsku): assert "-" in hwsku vendor = hwsku.split("-")[0] assert vendor in SUPPORTED_VENDORS + + +# --------------------------------------------------------------------------- +# PORT_CONFIG_PATH settings wiring +# --------------------------------------------------------------------------- + + +def test_port_config_path_follows_settings(): + original = settings_module.SONIC_PORT_CONFIG_PATH + try: + settings_module.SONIC_PORT_CONFIG_PATH = "/custom/port_config" + reloaded = importlib.reload(constants_module) + assert reloaded.PORT_CONFIG_PATH == "/custom/port_config" + finally: + settings_module.SONIC_PORT_CONFIG_PATH = original + importlib.reload(constants_module) diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 4f1df8ca..5d5907c0 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -583,6 +583,20 @@ def test_sonic_export_identifier_override(reload_settings, monkeypatch): assert settings_module.SONIC_EXPORT_IDENTIFIER == "asset-tag" +def test_sonic_port_config_path_default(reload_settings, monkeypatch): + monkeypatch.delenv("SONIC_PORT_CONFIG_PATH", raising=False) + reload_settings() + + assert settings_module.SONIC_PORT_CONFIG_PATH == "/etc/sonic/port_config" + + +def test_sonic_port_config_path_override(reload_settings, monkeypatch): + monkeypatch.setenv("SONIC_PORT_CONFIG_PATH", "/tmp/port_config") + reload_settings() + + assert settings_module.SONIC_PORT_CONFIG_PATH == "/tmp/port_config" + + # --------------------------------------------------------------------------- # NETBOX_SECONDARIES # ---------------------------------------------------------------------------