diff --git a/.gitignore b/.gitignore index 046f8b119..7d088d6bc 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 6e3fcd709..c33328629 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -140,6 +140,34 @@ 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 with the netbox-manager +# example data, 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 and catches drift of the +# netbox-manager seed data, which is consumed at tip-of-main via +# required-projects (honoring Depends-On). +- 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\.txt$ + - ^tests/e2e/.* + - project: merge-mode: squash-merge default-branch: main @@ -154,6 +182,7 @@ - python-osism-test-setup - python-osism-unit-tests - python-osism-integration-tests + - python-osism-sonic-e2e periodic-daily: jobs: - flake8 @@ -163,6 +192,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 000000000..94765b04f --- /dev/null +++ b/Makefile @@ -0,0 +1,24 @@ +NETBOX_MANAGER_DIR ?= $(abspath ../netbox-manager) + +# SONiC config-generation E2E golden test (see tests/e2e/run.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/run.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/run.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 + +.PHONY: sonic-e2e sonic-e2e-regen sonic-e2e-up sonic-e2e-down diff --git a/osism/settings.py b/osism/settings.py index f6e81cff8..b9ac81118 100644 --- a/osism/settings.py +++ b/osism/settings.py @@ -70,6 +70,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 adf2a7310..3d0816192 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, @@ -2393,14 +2419,14 @@ def _add_snmp_configuration(config, device, oob_ip): in the config_context of the device. """ - # Create vault instance for Custom Field decryption - vault = get_vault() - - # Decrypt secrets Custom Field + # Decrypt the secrets Custom Field. The vault needs the key file and + # Redis, and get_vault() logs ERRORs where those are absent, so it is + # only built when there are secrets to decrypt. node_secrets = device.custom_fields.get("secrets", {}) if node_secrets is None: node_secrets = {} - deep_decrypt(node_secrets, vault) + if node_secrets: + deep_decrypt(node_secrets, get_vault()) # Configure SNMP location and contact location = device.config_context.get("_segment_snmp_server_location", "Data Center") diff --git a/osism/tasks/conductor/sonic/constants.py b/osism/tasks/conductor/sonic/constants.py index 6cbdd0109..4d330ef80 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 2ee32feaa..ef260f00e 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,38 @@ def get_connected_interfaces(device, portchannel_info=None): return _get_connected_interfaces(device, portchannel_info) +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 + base = int(master[len("Ethernet") :]) + # 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 in range(count): + child = f"Ethernet{base + i * lpc}" + 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. @@ -689,6 +766,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 is None or 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() @@ -764,6 +916,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}" @@ -854,6 +1009,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 @@ -951,6 +1110,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 000000000..e7bff8ca1 --- /dev/null +++ b/playbooks/pre-sonic-e2e.yml @@ -0,0 +1,69 @@ +--- +# 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 + + 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/run.sh directly; curl is used by the + # NetBox container's healthcheck (not by run.sh itself), and ensure-docker + # needs it too; 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 000000000..c5dbfaa17 --- /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 + + # run.sh 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 }}" + + # Escape hatch: set `sonic_e2e_seed_parallel: 1` in the job vars to + # seed serially, reproducing the pre-parallel behaviour, if a failure + # is ever suspected of being a seeding race. + export SEED_PARALLEL="{{ sonic_e2e_seed_parallel | default(4) }}" + tests/e2e/run.sh diff --git a/tests/e2e/__init__.py b/tests/e2e/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/e2e/compare.py b/tests/e2e/compare.py new file mode 100644 index 000000000..3ae27e34d --- /dev/null +++ b/tests/e2e/compare.py @@ -0,0 +1,184 @@ +# 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): + """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. + """ + golden_dir = Path(golden_dir) + export_dir = Path(export_dir) + golden_dir.mkdir(parents=True, exist_ok=True) + + 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: + stale.unlink() + for name in sorted(export_names): + config = json.loads((export_dir / name).read_text()) + (golden_dir / name).write_text(_canonical(config)) + + +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", + ) + args = parser.parse_args(argv) + + if args.regenerate: + regenerate(args.golden, args.export) + 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 000000000..6bef74032 --- /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/deploy_netbox.sh b/tests/e2e/deploy_netbox.sh new file mode 100755 index 000000000..c81b3d132 --- /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: run.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 run.sh) + +set -euo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +COMPOSE_FILE="${HERE}/compose.yaml" + +# Only generated when unset, so run.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 <&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 + +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_DEVICETYPE_LIBRARY="${NETBOX_MANAGER_DIR}/example/devicetypes" +export NETBOX_MANAGER_MODULETYPE_LIBRARY="${NETBOX_MANAGER_DIR}/example/moduletypes" +export NETBOX_MANAGER_RESOURCES="${NETBOX_MANAGER_DIR}/example/resources" +export NETBOX_MANAGER_IGNORE_SSL_ERRORS=true + +# netbox-manager sorts resource files by their leading number, runs the groups +# in order, and parallelises only WITHIN a group, so the 000 -> 100 -> 200 -> +# 300 ordering still holds. The 300- group is 16 per-device files and dominates +# the seeding time: measured in CI, --parallel 4 took it from 555s to 250s. +# +# ESCAPE HATCH: set SEED_PARALLEL=1 to seed strictly serially. If a run ever +# fails in a way that looks like a seeding race, re-run with SEED_PARALLEL=1 +# and compare -- that reproduces the original sequential behaviour exactly. +# A race is far more likely to surface as a spurious failure than as a false +# pass, because interleaving would change the generated configs and the golden +# comparison would then catch it. +SEED_PARALLEL="${SEED_PARALLEL:-4}" + +echo ">>> Seeding NetBox with the netbox-manager example data (parallel: ${SEED_PARALLEL})" +"${SEED_VENV}/bin/netbox-manager" run --fail-fast --parallel "${SEED_PARALLEL}" + +# Scenario overlay: a second seeding pass adds the breakout / speed-unit +# regression devices that the base example does not cover. It reuses the +# site / tenant / roles created above and brings its own device type. +echo ">>> Seeding NetBox with the E2E scenario overlay" +export NETBOX_MANAGER_DEVICETYPE_LIBRARY="${REPO_ROOT}/tests/e2e/scenario/devicetypes" +export NETBOX_MANAGER_RESOURCES="${REPO_ROOT}/tests/e2e/scenario/resources" +# Passed for consistency, but expect no gain: the overlay's files sit in +# distinct numeric groups (500-, 600-, 700-) and only files within one group +# run concurrently, so they serialise regardless. Confirmed in CI -- this pass +# measured 49.8s with --parallel 4 against 47.4s serial. +"${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}" + pipenv run python -m tests.e2e.compare \ + --golden "${GOLDEN_DIR}" --export "${EXPORT_DIR}" --regenerate + 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/e2e/scenario/devicetypes/Edgecore/7726-32X-E2E.yaml b/tests/e2e/scenario/devicetypes/Edgecore/7726-32X-E2E.yaml new file mode 100644 index 000000000..4d36d83ca --- /dev/null +++ b/tests/e2e/scenario/devicetypes/Edgecore/7726-32X-E2E.yaml @@ -0,0 +1,22 @@ +--- +# Minimal device type for the SONiC E2E port-channel scenario. +# +# Not a faithful model of the real 7726-32X; it defines only a management +# port and two data ports used as LAG members. Config generation is driven +# by the hwsku custom field (Accton-AS7726-32X on the scenario device), not +# by this device type -- the device type only controls which NetBox +# interfaces exist. The LAG interface itself is created in the resources +# overlay (type: lag) with these two ports as members. +manufacturer: Edgecore +model: 7726-32X-E2E +slug: edgecore-7726-32x-e2e +u_height: 1.0 +is_full_depth: true +interfaces: + - name: eth0 + type: 1000base-t + mgmt_only: true + - name: Ethernet0 + type: 100gbase-x-qsfp28 + - name: Ethernet4 + type: 100gbase-x-qsfp28 diff --git a/tests/e2e/scenario/devicetypes/Edgecore/9726-32D-E2E.yaml b/tests/e2e/scenario/devicetypes/Edgecore/9726-32D-E2E.yaml new file mode 100644 index 000000000..c161befde --- /dev/null +++ b/tests/e2e/scenario/devicetypes/Edgecore/9726-32D-E2E.yaml @@ -0,0 +1,37 @@ +--- +# Minimal device type for the SONiC E2E breakout regression scenarios. +# +# This is not a faithful model of the real 9726-32D; it defines only the +# interfaces the scenarios need. Config generation is driven by the hwsku +# custom field (Accton-AS9726-32D on the scenario devices), not by this +# device type -- the device type only controls which NetBox interfaces +# exist. +# +# The interface layout mirrors how real deployments model breakouts (the +# NetBox EthX/Y/Z sub-port notation, speed derived from the interface +# type rather than set explicitly). On the 8-lane 400G master Ethernet0 +# (Accton-AS9726-32D.ini), the four sub-ports Eth1/1/1..4 map to +# Ethernet0/2/4/6 as a 4x100G breakout; the three non-master sub-ports are +# absent from the .ini and so are generated by _add_missing_breakout_ports, +# where the sub-port speed is read back from the collected NetBox data. +# +# Eth1/5 is a plain (non-breakout) 100G port used for VLAN coverage. +manufacturer: Edgecore +model: 9726-32D-E2E +slug: edgecore-9726-32d-e2e +u_height: 1.0 +is_full_depth: true +interfaces: + - name: eth0 + type: 1000base-t + mgmt_only: true + - name: Eth1/1/1 + type: 100gbase-x-qsfp28 + - name: Eth1/1/2 + type: 100gbase-x-qsfp28 + - name: Eth1/1/3 + type: 100gbase-x-qsfp28 + - name: Eth1/1/4 + type: 100gbase-x-qsfp28 + - name: Eth1/5 + type: 100gbase-x-qsfp28 diff --git a/tests/e2e/scenario/resources/500-breakout-scenarios.yml b/tests/e2e/scenario/resources/500-breakout-scenarios.yml new file mode 100644 index 000000000..42e937c38 --- /dev/null +++ b/tests/e2e/scenario/resources/500-breakout-scenarios.yml @@ -0,0 +1,157 @@ +--- +# 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 the base example run, and live in their own rack so +# they never collide with base devices. +# +# Both 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). + +- vars: + site: Discworld + location: Ankh-Morpork + tenant: Testbed + +- rack: + name: E2E + tenant: "{{ tenant }}" + site: "{{ site }}" + location: "{{ location }}" + u_height: 47 + +- vlan: + name: E2E Data + tenant: "{{ tenant }}" + vid: 200 + site: "{{ site }}" + vlan_role: OOB + +# --- Derived-speed breakout (the real-deployment shape) -------------------- + +- device: + name: e2e-breakout-derived + tenant: "{{ tenant }}" + site: "{{ site }}" + location: "{{ location }}" + rack: E2E + device_type: edgecore-9726-32d-e2e + device_role: leaf + face: front + position: 1 + tags: + - managed-by-metalbox + - managed-by-osism + 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 the base example leaves uncovered. +- device_interface: + device: e2e-breakout-derived + name: Eth1/5 + mode: tagged + tagged_vlans: + - name: E2E Data + site: "{{ site }}" + +# --- Explicit-speed breakout (the other speed unit) ----------------------- + +- device: + name: e2e-breakout-explicit + tenant: "{{ tenant }}" + site: "{{ site }}" + location: "{{ location }}" + rack: E2E + device_type: edgecore-9726-32d-e2e + device_role: leaf + face: front + position: 2 + tags: + - managed-by-metalbox + - managed-by-osism + custom_fields: + sonic_parameters: + hwsku: Accton-AS9726-32D + version: 4.5.0 + +- device_interface: + device: e2e-breakout-explicit + name: Eth1/1/1 + speed: 100000000 + +- device_interface: + device: e2e-breakout-explicit + name: Eth1/1/2 + speed: 100000000 + +- device_interface: + device: e2e-breakout-explicit + name: Eth1/1/3 + speed: 100000000 + +- device_interface: + device: e2e-breakout-explicit + name: Eth1/1/4 + 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: "{{ tenant }}" + site: "{{ site }}" + location: "{{ location }}" + rack: E2E + device_type: edgecore-9726-32d-e2e + device_role: leaf + face: front + position: 5 + tags: + - managed-by-metalbox + - managed-by-osism + 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-scenario.yml b/tests/e2e/scenario/resources/600-portchannel-scenario.yml new file mode 100644 index 000000000..ed96595e5 --- /dev/null +++ b/tests/e2e/scenario/resources/600-portchannel-scenario.yml @@ -0,0 +1,42 @@ +--- +# Port-channel (LAG) scenario for the SONiC E2E golden test. +# +# The base example models 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 / rack / tenant / roles / tags from +# the earlier overlay files (processed first by filename order). + +- 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: 3 + tags: + - managed-by-metalbox + - managed-by-osism + 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-scenario.yml b/tests/e2e/scenario/resources/700-evpn-scenario.yml new file mode 100644 index 000000000..a42aa0cb1 --- /dev/null +++ b/tests/e2e/scenario/resources/700-evpn-scenario.yml @@ -0,0 +1,83 @@ +--- +# EVPN / VXLAN / VRF scenario for the SONiC E2E golden test. +# +# The base example assigns no VRFs, so the whole EVPN/VXLAN/VRF subsystem +# is emitted-but-empty. Config 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. +# +# 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 / +# rack / tenant / roles / tags from the earlier overlay files. + +- 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: 4 + tags: + - managed-by-metalbox + - managed-by-osism + 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 + vrf: VrfStorage + +- device_interface: + device: e2e-evpn + name: Ethernet4 + 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/unit/e2e/__init__.py b/tests/unit/e2e/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit/e2e/test_compare.py b/tests/unit/e2e/test_compare.py new file mode 100644 index 000000000..36351a460 --- /dev/null +++ b/tests/unit/e2e/test_compare.py @@ -0,0 +1,251 @@ +# 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.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": {}} diff --git a/tests/unit/e2e/test_generate.py b/tests/unit/e2e/test_generate.py new file mode 100644 index 000000000..2e3a831da --- /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 e99d8c429..846f57b0d 100644 --- a/tests/unit/tasks/conductor/sonic/_detection_helpers.py +++ b/tests/unit/tasks/conductor/sonic/_detection_helpers.py @@ -8,13 +8,19 @@ from types import SimpleNamespace +_DEFAULT = object() -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 95198aac5..7df13d029 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 @@ -568,3 +574,360 @@ def test_detect_breakout_ports_sonic_standard_speed_resolved_from_port_type( result = detect_breakout_ports(device) 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 == {} 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 c292fe288..6332cce4d 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 4fd03ee0e..047934eeb 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_config_generator_snmp.py b/tests/unit/tasks/conductor/sonic/test_config_generator_snmp.py index d11844b86..f04d1600e 100644 --- a/tests/unit/tasks/conductor/sonic/test_config_generator_snmp.py +++ b/tests/unit/tasks/conductor/sonic/test_config_generator_snmp.py @@ -169,3 +169,42 @@ def test_add_snmp_configuration_secrets_none_treated_as_empty(patch_snmp_secrets "shaKey": "OBFUSCATEDAUTHSECRET", "aesKey": "OBFUSCATEDPRIVSECRET", } + + +def test_add_snmp_configuration_no_secrets_skips_vault(mocker): + """Without encrypted secrets there is nothing to decrypt, so the vault + (which needs the container key file and Redis) must not be built at + all -- ``get_vault`` logs ERRORs in environments without a vault.""" + get_vault_mock = mocker.patch( + "osism.tasks.conductor.sonic.config_generator.get_vault" + ) + deep_decrypt_mock = mocker.patch( + "osism.tasks.conductor.sonic.config_generator.deep_decrypt" + ) + config = {} + + _add_snmp_configuration(config, _snmp_device(), oob_ip=None) + _add_snmp_configuration( + config, _snmp_device(custom_fields={"secrets": None}), oob_ip=None + ) + + get_vault_mock.assert_not_called() + deep_decrypt_mock.assert_not_called() + assert "SNMP_SERVER" in config + + +def test_add_snmp_configuration_with_secrets_uses_vault(mocker): + get_vault_mock = mocker.patch( + "osism.tasks.conductor.sonic.config_generator.get_vault" + ) + deep_decrypt_mock = mocker.patch( + "osism.tasks.conductor.sonic.config_generator.deep_decrypt", + side_effect=lambda data, vault: None, + ) + config = {} + device = _snmp_device(custom_fields={"secrets": {"snmp_auth": "$ANSIBLE_VAULT..."}}) + + _add_snmp_configuration(config, device, oob_ip=None) + + get_vault_mock.assert_called_once() + deep_decrypt_mock.assert_called_once() diff --git a/tests/unit/tasks/conductor/sonic/test_constants.py b/tests/unit/tasks/conductor/sonic/test_constants.py index f65e8cc82..3c4c72bfd 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 4f1df8caa..5d5907c0e 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 # ---------------------------------------------------------------------------