From 40a42cbb73ad07a83cdf2104aac0291217b0940d Mon Sep 17 00:00:00 2001 From: Ben Duncan <5178243+benduncan@users.noreply.github.com> Date: Fri, 24 Apr 2026 16:14:30 +1000 Subject: [PATCH 1/2] grub branding tweaks --- cmd/installer/install/install.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/cmd/installer/install/install.go b/cmd/installer/install/install.go index 1bb3eedb3..53268f7f5 100644 --- a/cmd/installer/install/install.go +++ b/cmd/installer/install/install.go @@ -466,12 +466,16 @@ func installBootloader(disk string) error { copyGrubFont(mountRoot) // Kernel cmdline and basic defaults only — graphics/serial handled by 05_spinifex below. + // Confirm requirement + // console=ttyS0,115200n8 + grubDefault := `GRUB_DEFAULT=0 GRUB_TIMEOUT=5 GRUB_DISTRIBUTOR=Spinifex GRUB_CMDLINE_LINUX_DEFAULT="" -GRUB_CMDLINE_LINUX="console=tty0 console=ttyS0,115200n8 systemd.show_status=1" +GRUB_CMDLINE_LINUX="console=tty0 systemd.show_status=1" ` + if err := os.WriteFile(filepath.Join(mountRoot, "etc/default/grub"), []byte(grubDefault), 0o644); err != nil { return fmt.Errorf("write /etc/default/grub: %w", err) } @@ -494,14 +498,14 @@ if serial --unit=0 --speed=115200 --timeout=1; then terminal_input --append serial terminal_output --append serial fi + +# --- Branding --- insmod png -if background_image /boot/grub/splash.png; then - set color_normal=white/black - set color_highlight=black/white -fi +set theme=/boot/grub/theme.txt +export theme ` - if err := os.WriteFile(filepath.Join(mountRoot, "etc/grub.d/05_spinifex"), []byte(grubTheme), 0o755); err != nil { - return fmt.Errorf("write /etc/grub.d/05_spinifex: %w", err) + if err := os.WriteFile(filepath.Join(mountRoot, "etc/grub.d/06_spinifex"), []byte(grubTheme), 0o755); err != nil { + return fmt.Errorf("write /etc/grub.d/06_spinifex: %w", err) } if err := bindChrootMounts(); err != nil { From 94600d7b847f84e70276dd3feaaff418fbdd56d1 Mon Sep 17 00:00:00 2001 From: Ben Duncan <5178243+benduncan@users.noreply.github.com> Date: Mon, 27 Apr 2026 15:48:47 +1000 Subject: [PATCH 2/2] OVN chassis issues, new reconsile function, reboot fixed (#165) --- build/systemd/spinifex-vpcd.service | 4 +- cmd/spinifex/cmd/service.go | 9 - docs/compute/vpc-networking/README.md | 464 +++++++++++------------ docs/install-vm-eval/README.md | 13 +- scripts/setup-ovn.sh | 18 +- spinifex/services/vpcd/mock_ovn.go | 143 ++++++- spinifex/services/vpcd/ovn.go | 108 +++++- spinifex/services/vpcd/ovn_test.go | 70 ++++ spinifex/services/vpcd/reconcile.go | 15 +- spinifex/services/vpcd/reconcile_test.go | 114 +++++- spinifex/services/vpcd/topology.go | 68 ++++ spinifex/services/vpcd/vpcd.go | 23 +- spinifex/services/vpcd/vpcd_test.go | 52 ++- 13 files changed, 786 insertions(+), 315 deletions(-) diff --git a/build/systemd/spinifex-vpcd.service b/build/systemd/spinifex-vpcd.service index c5ce2acf2..4c3582779 100644 --- a/build/systemd/spinifex-vpcd.service +++ b/build/systemd/spinifex-vpcd.service @@ -1,8 +1,8 @@ [Unit] Description=Spinifex VPC Daemon (OVN Networking) PartOf=spinifex.target -After=spinifex-nats.service spinifex-daemon.service systemd-networkd.service openvswitch-switch.service -Wants=spinifex-nats.service +After=spinifex-nats.service spinifex-daemon.service systemd-networkd.service openvswitch-switch.service ovn-controller.service +Wants=spinifex-nats.service ovn-controller.service [Service] Type=simple diff --git a/cmd/spinifex/cmd/service.go b/cmd/spinifex/cmd/service.go index 9376b7018..750d817d9 100644 --- a/cmd/spinifex/cmd/service.go +++ b/cmd/spinifex/cmd/service.go @@ -746,14 +746,6 @@ var vpcdStartCmd = &cobra.Command{ }) } - // Fallback chassis names derived from config node names. vpcd prefers - // discovering chassis from the OVN Southbound DB at startup; these are - // only used if that discovery fails. - var chassisNames []string - for nodeName := range clusterConfig.Nodes { - chassisNames = append(chassisNames, "chassis-"+nodeName) - } - var bootstrap *vpcd.BootstrapVPC if clusterConfig.Bootstrap.VpcId != "" { bootstrap = &vpcd.BootstrapVPC{ @@ -782,7 +774,6 @@ var vpcdStartCmd = &cobra.Command{ Debug: false, ExternalMode: clusterConfig.Network.ExternalMode, ExternalPools: extPools, - ChassisNames: chassisNames, Bootstrap: bootstrap, ExternalInterface: nodeConfig.VPCD.ExternalInterface, DhcpBindBridge: nodeConfig.VPCD.DhcpBindBridge, diff --git a/docs/compute/vpc-networking/README.md b/docs/compute/vpc-networking/README.md index 2b6b8791a..2028ca8f9 100644 --- a/docs/compute/vpc-networking/README.md +++ b/docs/compute/vpc-networking/README.md @@ -227,177 +227,171 @@ define a range (or use `source = "dhcp"`) — no data migration needed. ## Bridge Setup — Physical Network Wiring -OVN needs an OVS bridge for external connectivity. `setup-ovn.sh` auto-detects -the best approach: +The WAN NIC **must** be enslaved to a Linux bridge. This is a hard requirement — +`setup-ovn.sh` will not attach a physical NIC directly to OVS, and macvlan is +no longer supported. The Linux bridge owns the host IP, default route, and any +DHCP lease, so SSH and management traffic stay up while OVS/OVN are configured +underneath. -- **Linux bridge on default route** (e.g. `br-wan` from cloud-init): Creates - OVS bridge `br-ext` and links them with a veth pair. The Linux bridge keeps - its IP and routes — no interruption. Bridge-mapping: `external:br-ext`. -- **OVS bridge on default route**: Uses it directly for bridge-mappings. -- **Physical NIC**: Stops and prints guidance (cannot safely move the NIC). +The full datapath chain looks like this: -## Two Bridges, Two Jobs - -Every Spinifex node has two OVS bridges: - -| Bridge | Purpose | Ports | -| -------- | ------------------------------------ | ---------------------------- | -| `br-int` | VM overlay traffic (Geneve tunnels) | VM TAP devices, tunnel ports | -| `br-wan` | WAN uplink for public subnet traffic | macvlan on WAN NIC | - -`br-int` is always created by `setup-ovn.sh`. `br-wan` is created only when -a WAN bridge is configured (auto-detected or via `--wan-bridge`, required for public subnets / external connectivity). - -The connection between them is logical, not physical — OVN's `localnet` port type -maps the logical external switch to `br-wan` via `ovn-bridge-mappings`. - -

- Data path — VM TAP through br-int, OVN pipeline, localnet, br-wan, WAN -

- -## Bridge Modes - -Spinifex supports two ways to wire `br-wan` to the physical network. -**Direct bridge** is preferred when the host has a dedicated WAN NIC. **Macvlan** -is the fallback for single-NIC hosts where the WAN NIC is also the SSH NIC. - -### Direct Bridge (Preferred) - -The WAN NIC is added directly to `br-wan` as an OVS port. OVS sees all -traffic on the wire — any MAC, any protocol. No filtering, no workarounds. - -

- Direct bridge mode — dedicated WAN NIC added directly to br-wan -

- -**Use when:** The host has a NIC dedicated to external/WAN traffic that is NOT -used for SSH or management. Datacenter servers with separate management + -public NICs. Homelab hosts with 2+ NICs. - -**Benefits over macvlan:** - -- **Distributed NAT**: DNAT processed on the VM's own chassis (no hairpin) -- **No MAC workarounds**: OVS sees all frames, any MAC -- **Host can reach VMs**: No macvlan parent↔child isolation -- **Better multi-node performance**: External traffic stays local -- **DHCP lease stability**: No MAC changes after lease - -```bash -sudo setup-ovn.sh --wan-bridge=br-wan --wan-iface=eth1 +``` +physical NIC (e.g. `wan`) + └─ enslaved to ─▶ br-wan (Linux bridge — host IP, default route, DHCP) + │ + └─ veth pair ─▶ br-ext (OVS bridge — OVN external uplink) + │ + └─ localnet ─▶ br-int (OVS integration bridge) + │ + └─▶ TAP devices (VM NICs) ``` -### Macvlan (Fallback for Single-NIC) +`setup-ovn.sh` auto-detects the Linux bridge that owns the default route +(typically `br-wan`, provisioned by cloud-init / netplan / systemd-networkd). +You can override the detection with `--wan-bridge=`. Once detected, the +script creates the OVS bridge `br-ext` and links it to the WAN bridge with a +veth pair. The Linux bridge keeps its IP and routes — no interruption. +Bridge-mapping is set to `external:br-ext`. -A macvlan sub-interface is created in bridge mode off the WAN NIC, and that -macvlan is added to br-wan. The host keeps its IP on the original NIC — -SSH stays up. +If the default route is on a bare physical NIC (no bridge), `setup-ovn.sh` +stops and prints guidance on how to convert the NIC to a bridge before +re-running. -

- Macvlan mode — WAN NIC shared between host traffic and macvlan child on br-wan -

+### Example: Required `br-wan` State -**Use when:** The WAN NIC is the same NIC used for SSH. Single-NIC homelabs, -edge deployments. Adding the NIC directly to OVS would break host connectivity. +The host must have something resembling this before `setup-ovn.sh` is run: -**Limitations:** - -- Centralized NAT only (macvlan filters unicast not matching its MAC) -- Host cannot reach VMs at their public IPs (macvlan isolation) -- vpcd must align macvlan MAC with OVN router MAC on startup -- Multi-node: all external traffic hairpins through the gateway chassis - -```bash -sudo setup-ovn.sh --macvlan --wan-iface=eth0 +``` +7: br-wan: mtu 1500 qdisc noqueue state UP group default qlen 1000 + link/ether 26:df:3c:de:d0:c2 brd ff:ff:ff:ff:ff:ff + inet 192.168.1.31/23 brd 192.168.1.255 scope global br-wan + valid_lft forever preferred_lft forever + inet6 fe80::24df:3cff:fede:d0c2/64 scope link proto kernel_ll + valid_lft forever preferred_lft forever ``` -### Mode Comparison +The physical NIC (e.g. `wan`, `eth0`, `eno1`) is enslaved to `br-wan` and has +no IP of its own — all L3 state lives on the bridge. -| Capability | Direct bridge | Macvlan | -| ------------------------ | ------------------------ | -------------------------------- | -| NAT mode | Distributed (local DNAT) | Centralized (gateway hairpin) | -| Multi-node external perf | Optimal | Hairpin through gateway chassis | -| Host → VM public IP | Works | Blocked (kernel isolation) | -| MAC alignment needed | No | Yes (fragile startup step) | -| Gratuitous ARPs needed | No | Yes (`nat-addresses=router`) | -| Works on single-NIC | No (breaks SSH) | Yes | -| DHCP lease stability | Clean | Fragile (MAC changes post-lease) | +Example netplan that produces this: -### How setup-ovn.sh Decides +```yaml +network: + version: 2 + ethernets: + wan: + dhcp4: false + bridges: + br-wan: + interfaces: [wan] + dhcp4: true +``` -| Flags | Result | -| -------------------------------------- | ----------------------------------------------- | -| `--wan-bridge=br-wan --wan-iface=eth1` | Direct bridge: NIC added to br-wan | -| `--macvlan --wan-iface=eth0` | Macvlan: sub-interface created, added to br-wan | -| (no WAN bridge) | Only br-int created, no WAN connectivity | +## Three Bridges, Three Jobs -### Setup (Macvlan Mode) +Every Spinifex node has three bridges in the datapath: + +| Bridge | Type | Created By | Purpose | Ports | +| -------- | ------------ | --------------------------- | --------------------------------------------- | ---------------------------- | +| `br-wan` | Linux bridge | Host (cloud-init / netplan) | Host WAN uplink — owns host IP and default route | Physical WAN NIC, veth peer | +| `br-ext` | OVS bridge | `setup-ovn.sh` | OVN external uplink (`localnet`) | veth peer to `br-wan` | +| `br-int` | OVS bridge | `setup-ovn.sh` | VM overlay traffic (Geneve tunnels) | VM TAP devices, tunnel ports | + +`br-wan` is provisioned by your distro's network configuration (cloud-init, +netplan, systemd-networkd, ifupdown). The name is configurable; `br-wan` is +the convention. `br-int` and `br-ext` are always created by `setup-ovn.sh`. + +The link between `br-ext` and the VM datapath is logical, not physical: OVN's +`localnet` port type maps the logical external switch to `br-ext` via +`ovn-bridge-mappings`. Frames egressing a VM travel TAP → `br-int` → OVN +pipeline → `br-ext` → veth → `br-wan` → physical NIC → wire. + +

+ Data path — VM TAP through br-int, OVN pipeline, br-ext, veth pair, br-wan, physical NIC +

+ +## Running setup-ovn.sh + +```bash +# Auto-detect the WAN bridge (recommended) +sudo setup-ovn.sh + +# Explicitly specify the WAN bridge name +sudo setup-ovn.sh --wan-bridge=br-wan +``` In environments where the WAN IP comes from a router's DHCP server (homelab, -small office), add `--dhcp` to obtain a gateway IP from the router automatically: +small office), add `--dhcp` to obtain a gateway IP from the router +automatically: ```bash -sudo setup-ovn.sh --macvlan --wan-iface=eth0 --dhcp +sudo setup-ovn.sh --dhcp ``` This requests an IP from the **router's DHCP** (e.g., 192.168.1.1 serving addresses on the LAN). This is not Spinifex's internal OVN DHCP that assigns private IPs to VMs — it's your network's existing DHCP server. -### Macvlan Internals +| Flags | Result | +| ------------------------------ | --------------------------------------------------------------------- | +| (no flags) | Auto-detect WAN bridge from default route, create `br-int` + `br-ext` | +| `--wan-bridge=` | Use the specified Linux bridge as the WAN uplink | +| `--dhcp` | Obtain the OVN gateway IP from the router's DHCP | -macvlan in `bridge` mode creates a virtual interface that shares the parent NIC's -physical wire but has its own MAC address. The Linux kernel blocks direct L2 -frames between a parent interface and its macvlan children. - -| Path | Works? | Why | -| ------------------------- | ------ | ------------------------------------------------- | -| VM → internet | Yes | SNAT through OVN router → macvlan → WAN NIC → WAN | -| LAN device → VM public IP | Yes | LAN → WAN NIC → macvlan → br-wan → OVN | -| Host → VM public IP | No | macvlan isolation (kernel blocks parent↔child) | -| Host → VM private IP | Yes | Overlay via br-int (unrelated to br-wan) | +If no Linux bridge owns the default route, `setup-ovn.sh` exits with guidance +rather than silently breaking host connectivity. ## Per-Node Configuration -Different nodes in a cluster can have different WAN NICs: +Different nodes in a cluster can have different WAN bridges and NICs: ```toml [nodes.node1.vpcd] -external_interface = "eth1" +external_interface = "br-wan" [nodes.node2.vpcd] -external_interface = "eth0" +external_interface = "br-public" [nodes.node3.vpcd] -external_interface = "bond0" # bonded NIC +external_interface = "br-wan" # br-wan enslaving bond0 ``` -Each node runs `setup-ovn.sh` with its own WAN bridge configuration. OVN doesn't -care which NIC the macvlan is on — only that `ovn-bridge-mappings` is set. +`external_interface` is the **Linux bridge** that owns the WAN uplink on this +node — not the physical NIC. The physical NIC lives underneath the bridge. +Each node runs `setup-ovn.sh` with its own WAN bridge name (or relies on +auto-detection). OVN only requires `ovn-bridge-mappings` to point at `br-ext`. ## Bridge Verification ```bash -# Check br-wan exists -sudo ovs-vsctl br-exists br-wan && echo "OK" || echo "MISSING" +# OVS bridges created by setup-ovn.sh +sudo ovs-vsctl br-exists br-int && echo "br-int OK" || echo "br-int MISSING" +sudo ovs-vsctl br-exists br-ext && echo "br-ext OK" || echo "br-ext MISSING" -# Check the physical port -sudo ovs-vsctl list-ports br-wan -# Multi-NIC: shows "eth1" (or your WAN NIC) -# Single-NIC: shows "spx-ext-eth0" (macvlan name) +# Linux WAN bridge owns the host IP and default route +ip -br addr show br-wan +ip route show default +# Default route's dev should be br-wan (or your WAN bridge name) -# Check bridge mappings +# br-ext should have one veth port linking it to br-wan +sudo ovs-vsctl list-ports br-ext +# Expect: a veth name (e.g. "veth-wan-ovs") + +# Confirm the matching peer is enslaved to the Linux WAN bridge +sudo bridge link show | grep br-wan + +# Bridge mappings must point at br-ext sudo ovs-vsctl get Open_vSwitch . external-ids:ovn-bridge-mappings -# Output: "external:br-wan" +# Output: "external:br-ext" -# Check macvlan (single-NIC only) -ip link show spx-ext-eth0 -# Shows: state UP, type macvlan mode bridge +# Physical NIC is enslaved to br-wan (master should be br-wan) +ip -d link show wan ``` ## Network Flow Diagram

- Bare-metal host — br-int overlay, br-wan WAN bridge with macvlan, OVN NAT pipeline + Bare-metal host — br-int overlay, br-ext OVS uplink, br-wan Linux bridge, physical NIC, OVN NAT pipeline

## Configuration Reference @@ -480,12 +474,12 @@ These are different things: [nodes.spx1.vpcd] ovn_nb_addr = "tcp:10.1.3.181:6641" # OVN Northbound DB ovn_sb_addr = "tcp:10.1.3.181:6642" # OVN Southbound DB -external_interface = "eth1" # WAN NIC name +external_interface = "br-wan" # WAN Linux bridge name ``` -| Field | Description | -| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `external_interface` | Physical NIC for WAN traffic. A macvlan sub-interface is created on this NIC for br-wan. Different servers may have different names (eth1, eno2, enp3s0, bond0). | +| Field | Description | +| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `external_interface` | Linux bridge that owns the WAN uplink on this node (e.g. `br-wan`, `br-public`). The physical NIC is enslaved to this bridge — not configured here. Different nodes may differ. | ## Pool Selection Logic @@ -544,11 +538,13 @@ gateway = "192.168.1.1" prefix_len = 24 [nodes.homelab.vpcd] -external_interface = "eth0" +external_interface = "br-wan" ``` -**Setup:** Change your router's DHCP range to end at .149. Run -`setup-ovn.sh --macvlan --wan-iface=eth0` (or auto-detected if WAN is already a bridge). +**Setup:** Configure `br-wan` to enslave your physical WAN NIC (netplan, +cloud-init, or systemd-networkd). Change your router's DHCP range to end +at .149. Run `sudo setup-ovn.sh` — it auto-detects the WAN bridge from the +default route, or specify it with `--wan-bridge=br-wan`. ## Homelab / Dev (DHCP Pool — No Range Reservation) @@ -570,7 +566,7 @@ prefix_len = 24 dns_servers = ["192.168.1.1", "8.8.8.8"] [nodes.homelab.vpcd] -external_interface = "eth0" +external_interface = "br-wan" ``` **Setup:** No router changes needed. Spinifex requests IPs from the router's @@ -582,7 +578,7 @@ on the host (`apt install isc-dhcp-client`). ``` ISP-assigned: 203.0.113.0/28 (14 usable IPs) ISP gateway: 203.0.113.1 -Servers: 3x with separate mgmt (eth0) and public (eth1) NICs +Servers: 3x with separate mgmt NIC (eth0) and public NIC enslaved to br-wan ``` ```toml @@ -597,17 +593,21 @@ gateway = "203.0.113.1" prefix_len = 28 [nodes.dc1.vpcd] -external_interface = "eth1" +external_interface = "br-wan" # br-wan enslaves eth1 [nodes.dc2.vpcd] -external_interface = "eth1" +external_interface = "br-wan" # br-wan enslaves eth1 [nodes.dc3.vpcd] -external_interface = "eno1" +external_interface = "br-public" # br-public enslaves eno1 ``` ## Enterprise On-Prem (VLAN) +The Linux WAN bridge enslaves a VLAN sub-interface (e.g. `eth1.200`) instead +of a raw NIC. From OVN's perspective nothing changes — `external_interface` +still points at the bridge. + ```toml [network] external_mode = "pool" @@ -620,10 +620,10 @@ gateway = "172.16.0.1" prefix_len = 24 [nodes.srv1.vpcd] -external_interface = "eth1.200" # VLAN-tagged sub-interface +external_interface = "br-dmz" # br-dmz enslaves eth1.200 [nodes.srv2.vpcd] -external_interface = "bond0.200" # Bonded NIC with VLAN tag +external_interface = "br-dmz" # br-dmz enslaves bond0.200 ``` ## Edge / Branch (Outbound Only) @@ -639,7 +639,7 @@ gateway_ip = "10.0.0.50" prefix_len = 24 [nodes.edge1.vpcd] -external_interface = "eth0" +external_interface = "br-wan" ``` ## Multi-Region (Multiple Pools) @@ -800,14 +800,15 @@ ovn-nbctl lr-route-add vpc-{vpcId} 0.0.0.0/0 192.168.1.1 ## Per-Instance Public IP ```bash -# 1:1 NAT (centralized — processed on gateway chassis, uses router MAC) -ovn-nbctl lr-nat-add vpc-{vpcId} dnat_and_snat {public_ip} {private_ip} +# 1:1 NAT — distributed (DNAT processed on the VM's own chassis) +ovn-nbctl lr-nat-add vpc-{vpcId} dnat_and_snat {public_ip} {private_ip} \ + port-{eniId} {vm_mac} ``` -> **Note:** Spinifex uses centralized NAT (no `external_mac` / `logical_port`) -> because macvlan-based external bridges can only receive unicast traffic matching -> the macvlan's own MAC. The router port MAC is set on the macvlan, and OVN uses -> it for all ARP replies. For single-node, there is no performance difference. +With the WAN NIC on a Linux bridge wired to OVS via veth, OVS sees every frame +on the wire regardless of MAC, so OVN can use distributed NAT. The DNAT is +processed on the chassis hosting the VM rather than hairpinning through a +single gateway chassis. ## Security Group @@ -863,8 +864,13 @@ sudo ovn-nbctl pg-get-ports sg-{groupId} ## 1. Set Up OVN Bridges +Make sure the WAN NIC is enslaved to a Linux bridge (e.g. `br-wan`) and that +bridge owns the default route. Then run: + ```bash -sudo setup-ovn.sh --macvlan --wan-iface=eth0 +sudo setup-ovn.sh # auto-detect WAN bridge +# or +sudo setup-ovn.sh --wan-bridge=br-wan # explicit ``` ## 2. Configure External IP Pool @@ -937,7 +943,7 @@ sudo ovn-nbctl list Logical_Switch_Port port-eni-{eniId} # Gateway chassis assignment sudo ovn-nbctl list Logical_Router_Port gw-vpc-{vpcId} -# Localnet port options (check nat-addresses=router) +# Localnet port options (network_name should be "external") sudo ovn-nbctl get Logical_Switch_Port ext-port-vpc-{vpcId} options ``` @@ -987,24 +993,30 @@ sudo ovs-vsctl get Open_vSwitch . external_ids ### Network Interfaces ```bash -# Check macvlan exists and has correct MAC -ip -d link show spx-ext-{nic} +# Physical NIC should be enslaved to br-wan ("master br-wan") +ip -d link show {nic} -# Check macvlan is attached to OVS (look for "openvswitch_slave") -ip -d link show spx-ext-{nic} | grep openvswitch +# Linux WAN bridge — owns the host IP and default route +ip -br addr show br-wan +ip route show default + +# OVS bridges and the veth linking br-ext to br-wan +sudo ovs-vsctl list-ports br-ext +bridge link show | grep br-wan # Interface traffic stats (RX/TX counts, drops) -ip -s link show spx-ext-{nic} +ip -s link show {nic} +ip -s link show br-wan ``` ### Packet Capture ```bash -# Capture on the OVS bridge (sees traffic after OVS processing) -sudo tcpdump -i br-wan -n -e arp +# Capture on the OVS uplink bridge (frames between OVN and br-wan) +sudo tcpdump -i br-ext -n -e arp -# Capture on the macvlan (sees what leaves/enters the wire) -sudo tcpdump -i spx-ext-{nic} -n -e "host {public_ip}" +# Capture on the Linux WAN bridge (frames between br-ext veth and the NIC) +sudo tcpdump -i br-wan -n -e "host {public_ip}" # Capture on the physical NIC (sees everything on the wire) sudo tcpdump -i {nic} -n "host {public_ip}" @@ -1082,12 +1094,18 @@ Work through these checks in order — each eliminates a class of issues. ### 1. Verify OVS wiring ```bash -# br-wan must exist with the macvlan port -sudo ovs-vsctl show | grep -A5 br-wan +# br-int and br-ext must exist +sudo ovs-vsctl show | grep -E "Bridge (br-int|br-ext)" -# Bridge mappings must be set +# br-ext should have a veth port linking it to br-wan +sudo ovs-vsctl list-ports br-ext + +# The Linux WAN bridge should have the physical NIC and the veth peer +bridge link show | grep br-wan + +# Bridge mappings must point at br-ext sudo ovs-vsctl get Open_vSwitch . external-ids:ovn-bridge-mappings -# Expected: "external:br-wan" +# Expected: "external:br-ext" ``` ### 2. Verify chassis and gateway scheduling @@ -1111,7 +1129,8 @@ If these don't match, see "Chassis name mismatch" below. ```bash sudo ovn-nbctl lr-nat-list vpc-$VPC | grep dnat_and_snat # Must show the public IP → private IP mapping -# Must NOT have external_mac or logical_port set (centralized NAT) +# For distributed NAT, external_mac and logical_port should be set +# (the VM's MAC and ENI port name) ``` ### 4. Verify ARP resolution @@ -1122,20 +1141,21 @@ From another host on the same LAN, check if OVN responds to ARP: # On the remote host: ping -c 1 {public_ip} ip neigh show {public_ip} -# Should show the OVN router MAC (02:00:00:xx:xx:xx) +# Should show the VM's ENI MAC (distributed NAT) or the OVN router MAC ``` -If ARP fails, check the macvlan MAC alignment: +If ARP fails, confirm the WAN bridge is forwarding: ```bash -# On the Spinifex host: -# Gateway router MAC -sudo ovn-nbctl get Logical_Router_Port gw-vpc-{vpcId} mac -# Macvlan MAC -ip link show spx-ext-{nic} | grep ether +# Physical NIC must be enslaved to br-wan +ip -d link show {nic} | grep "master br-wan" -# These MUST match. If they don't, vpcd failed to set the macvlan MAC. -# Check vpcd.log for "macvlan MAC aligned" or errors. +# br-wan must be UP and have an IP +ip -br addr show br-wan + +# br-ext must have a veth port whose peer is enslaved to br-wan +sudo ovs-vsctl list-ports br-ext +bridge link show | grep br-wan ``` ### 5. Verify packet flow with tcpdump @@ -1146,15 +1166,16 @@ Capture at each layer to find where packets stop: # Layer 1: Does the ARP/ICMP arrive on the physical NIC? sudo tcpdump -i {nic} -n "host {public_ip}" -# Layer 2: Does it reach br-wan? +# Layer 2: Does it cross the Linux WAN bridge? sudo tcpdump -i br-wan -n "host {public_ip}" -# Layer 3: Does it reach the macvlan output? -sudo tcpdump -i spx-ext-{nic} -n -e "host {public_ip}" +# Layer 3: Does it reach the OVS uplink bridge? +sudo tcpdump -i br-ext -n -e "host {public_ip}" ``` -If traffic arrives on the physical NIC but not br-wan, the macvlan is -filtering it. Check MAC alignment (step 4). +If traffic arrives on the physical NIC but not br-wan, the NIC is not +enslaved to the bridge. If it reaches br-wan but not br-ext, the veth +pair between them is missing or down — re-run `setup-ovn.sh`. ### 6. Use ovn-trace for pipeline debugging @@ -1197,77 +1218,46 @@ sudo systemctl start ovn-central ovn-controller ### Chassis Name Mismatch -**Symptom:** Gateway chassis is set in NB but the SB has no matching chassis. -`ovn-nbctl show` reports a gateway chassis name that doesn't appear in -`ovn-sbctl show`. Gateway traffic (SNAT/DNAT) doesn't work. +After mulga-999, vpcd uses the OVS-managed UUID (persisted at +`/etc/openvswitch/system-id.conf` by the `openvswitch-switch` package and +re-applied on every boot) as the canonical chassis identity. The chassis name +across OVS `external_ids:system-id`, OVN SB `Chassis.name`, and NB +`Gateway_Chassis.chassis_name` is always the same value, so the mismatch +class that this section previously documented can no longer recur. -**Cause:** Three names must align: +`ReconcileFromKV` runs a `reconcileGatewayChassis` pre-step on every vpcd +startup that deletes any stale `Gateway_Chassis` row left over from earlier +installs (where `setup-ovn.sh` fabricated `chassis-$(hostname -s)`) and +re-binds every gateway LRP against the live SBDB chassis. Pre-mulga-999 +brokenness recovers automatically on the first vpcd restart after upgrade — +no manual `ovn-nbctl destroy` required. -1. OVS `external_ids:system-id` → what ovn-controller registers as -2. OVN SB `Chassis.name` → what actually exists in the SB DB -3. vpcd's chassis name → `chassis-{nodeName}` from config +### WAN NIC Not Enslaved to a Bridge -If `setup-ovn.sh` used `chassis-$(hostname -s)` but vpcd uses -`chassis-{nodeName}`, they'll differ on any host where hostname ≠ node name. +**Symptom:** `setup-ovn.sh` exits with an error like "default route is on a +physical NIC, not a bridge" and refuses to continue. -**Diagnosis:** +**Cause:** Spinifex requires the WAN NIC to be enslaved to a Linux bridge +(typically `br-wan`). Macvlan is no longer supported, and attaching the NIC +directly to OVS would break SSH and any other host services using the NIC. -```bash -# What OVS has -sudo ovs-vsctl get Open_vSwitch . external-ids:system-id -# What SB registered -sudo ovn-sbctl list Chassis | grep name -# What vpcd expects -grep node /etc/spinifex/spinifex.toml | head -1 +**Fix:** Move the host IP and default route onto a Linux bridge. Example +netplan: + +```yaml +network: + version: 2 + ethernets: + wan: + dhcp4: false + bridges: + br-wan: + interfaces: [wan] + dhcp4: true ``` -**Fix:** `reset-dev-env.sh` passes `--chassis-id=chassis-node1` to -`setup-ovn.sh`. For manual fix: - -```bash -sudo ovs-vsctl set Open_vSwitch . external_ids:system-id=chassis-node1 -sudo systemctl restart ovn-controller -``` - -### Macvlan Unicast Filtering (Inbound Traffic Lost) - -**Symptom:** VM gets DHCP, outbound works (ping 8.8.8.8 from VM), but inbound -from the LAN to the VM's public IP fails. ARP replies go out but ICMP never -arrives at br-wan. - -**Cause:** macvlan in bridge mode only delivers unicast frames matching its own -MAC. OVN announces the router MAC for external IPs, but the macvlan has a -different (random) MAC. Broadcasts (ARP requests) pass through; unicast return -traffic is filtered by the kernel macvlan subsystem. - -**Diagnosis:** - -```bash -# Compare these two — they must match -sudo ovn-nbctl get Logical_Router_Port gw-vpc-{vpcId} mac -ip link show spx-ext-{nic} | grep ether - -# If they differ, inbound unicast is being dropped -``` - -**Fix:** vpcd automatically aligns the macvlan MAC on startup. If it failed: - -```bash -# Get the gateway MAC from OVN -GW_MAC=$(sudo ovn-nbctl get Logical_Router_Port gw-vpc-{vpcId} mac | tr -d '"') - -# Set it on the macvlan -sudo ip link set spx-ext-{nic} down -sudo ip link set spx-ext-{nic} address $GW_MAC -sudo ip link set spx-ext-{nic} up -``` - -Also ensure the DNAT rule uses centralized NAT (no `external_mac`): - -```bash -sudo ovn-nbctl lr-nat-list vpc-{vpcId} -# The dnat_and_snat row should have EMPTY external_mac and logical_port columns -``` +Apply (`sudo netplan apply`), confirm the host IP is now on `br-wan` +(`ip -br addr show br-wan`), and re-run `setup-ovn.sh`. ### Stale ARP on Remote Hosts @@ -1282,7 +1272,7 @@ sudo ip neigh flush dev {nic} {public_ip} ping {public_ip} # should work now ``` -OVN's `nat-addresses=router` option sends periodic gratuitous ARPs that will +OVN sends periodic gratuitous ARPs from the chassis hosting the VM that will eventually update remote ARP caches, but flushing is faster for testing. ### Security Group Rules Not Taking Effect diff --git a/docs/install-vm-eval/README.md b/docs/install-vm-eval/README.md index 361d88fb4..bc584f868 100644 --- a/docs/install-vm-eval/README.md +++ b/docs/install-vm-eval/README.md @@ -220,15 +220,24 @@ Output: Auto-detected Linux bridge: br-wan (default route) Will create OVS bridge br-ext + veth pair to link them Auto-detected encap IP: 192.168.0.249 -Auto-detected chassis ID: chassis-spinifex === Spinifex OVN Compute Node Setup === Management node: true WAN bridge: br-ext (veth) Linux bridge: br-wan (linked via veth pair) OVN Remote (SB): tcp:127.0.0.1:6642 Encap IP: 192.168.0.249 - Chassis ID: chassis-spinifex ... + system-id: "add0104e-9aec-43f0-af53-5fc89b4d91bf" +... +``` + +The `system-id` is the OVS-managed chassis UUID (read back from +`/etc/openvswitch/system-id.conf`); this is the same value that appears as +`Chassis.name` in `ovn-sbctl show`. Cross-reference with `Chassis.hostname` +when you need the human-readable host: + +```bash +sudo ovn-sbctl --columns=name,hostname list Chassis ``` ## Spinifex Initialize diff --git a/scripts/setup-ovn.sh b/scripts/setup-ovn.sh index f2e73cb39..353197131 100755 --- a/scripts/setup-ovn.sh +++ b/scripts/setup-ovn.sh @@ -26,7 +26,6 @@ # --no-mgmt-bridge Skip mgmt bridge provisioning (for dev-networking hosts) # --ovn-remote=ADDR OVN SB DB address (default: tcp:127.0.0.1:6642) # --encap-ip=IP Geneve tunnel endpoint IP (default: auto-detect) -# --chassis-id=ID OVN chassis identifier (default: chassis-$(hostname -s)) # # WAN Bridge Auto-Detection: # When no --wan-bridge is given, the script checks the default route interface: @@ -65,7 +64,6 @@ MGMT_CIDR="10.15.8.1/24" MGMT_IFACE="" OVN_REMOTE="tcp:127.0.0.1:6642" ENCAP_IP="" -CHASSIS_ID="" # Parse arguments for arg in "$@"; do @@ -81,7 +79,6 @@ for arg in "$@"; do --no-mgmt-bridge) MGMT_BRIDGE_ENABLED=false ;; --ovn-remote=*) OVN_REMOTE="${arg#*=}" ;; --encap-ip=*) ENCAP_IP="${arg#*=}" ;; - --chassis-id=*) CHASSIS_ID="${arg#*=}" ;; --help|-h) head -50 "$0" | tail -48 exit 0 @@ -210,12 +207,6 @@ if [ -z "$ENCAP_IP" ]; then fi fi -# Auto-detect chassis ID if not specified -if [ -z "$CHASSIS_ID" ]; then - CHASSIS_ID="chassis-$(hostname -s)" - echo "Auto-detected chassis ID: $CHASSIS_ID" -fi - echo "=== Spinifex OVN Compute Node Setup ===" echo " Management node: $MANAGEMENT" if [ -n "$WAN_BRIDGE" ]; then @@ -231,7 +222,6 @@ else fi echo " OVN Remote (SB): $OVN_REMOTE" echo " Encap IP: $ENCAP_IP" -echo " Chassis ID: $CHASSIS_ID" echo "" # --- Step 1: Install packages --- @@ -578,7 +568,6 @@ echo "Step 4: Setting OVS external_ids for OVN..." if [ -n "$WAN_BRIDGE" ]; then BRIDGE_MAPPINGS="external:${WAN_BRIDGE}" sudo ovs-vsctl set Open_vSwitch . \ - external_ids:system-id="$CHASSIS_ID" \ external_ids:ovn-remote="$OVN_REMOTE" \ external_ids:ovn-encap-ip="$ENCAP_IP" \ external_ids:ovn-encap-type="geneve" \ @@ -586,13 +575,16 @@ if [ -n "$WAN_BRIDGE" ]; then echo " ovn-bridge-mappings: $BRIDGE_MAPPINGS" else sudo ovs-vsctl set Open_vSwitch . \ - external_ids:system-id="$CHASSIS_ID" \ external_ids:ovn-remote="$OVN_REMOTE" \ external_ids:ovn-encap-ip="$ENCAP_IP" \ external_ids:ovn-encap-type="geneve" fi -echo " system-id: $CHASSIS_ID" +# system-id is owned by the openvswitch-switch package (persisted in +# /etc/openvswitch/system-id.conf and re-applied on every boot). Read it back +# rather than overriding — overriding here would drift from the on-disk value +# and the next reboot would silently flip the chassis identity (mulga-999). +echo " system-id: $(sudo ovs-vsctl get open . external_ids:system-id)" echo " ovn-remote: $OVN_REMOTE" echo " ovn-encap-ip: $ENCAP_IP" echo " ovn-encap-type: geneve" diff --git a/spinifex/services/vpcd/mock_ovn.go b/spinifex/services/vpcd/mock_ovn.go index 29a0e8594..7c5dc9424 100644 --- a/spinifex/services/vpcd/mock_ovn.go +++ b/spinifex/services/vpcd/mock_ovn.go @@ -14,34 +14,44 @@ type MockOVNClient struct { mu sync.Mutex connected bool - switches map[string]*nbdb.LogicalSwitch - ports map[string]*nbdb.LogicalSwitchPort - routers map[string]*nbdb.LogicalRouter - routerPorts map[string]*nbdb.LogicalRouterPort - dhcpOpts map[string]*nbdb.DHCPOptions - nats map[string]*nbdb.NAT // keyed by UUID - staticRoutes map[string]*nbdb.LogicalRouterStaticRoute // keyed by UUID - portGroups map[string]*nbdb.PortGroup // keyed by name - acls map[string]*nbdb.ACL // keyed by UUID + switches map[string]*nbdb.LogicalSwitch + ports map[string]*nbdb.LogicalSwitchPort + routers map[string]*nbdb.LogicalRouter + routerPorts map[string]*nbdb.LogicalRouterPort + dhcpOpts map[string]*nbdb.DHCPOptions + nats map[string]*nbdb.NAT // keyed by UUID + staticRoutes map[string]*nbdb.LogicalRouterStaticRoute // keyed by UUID + portGroups map[string]*nbdb.PortGroup // keyed by name + acls map[string]*nbdb.ACL // keyed by UUID + gatewayChassis map[string]*nbdb.GatewayChassis // keyed by UUID // UpdateLogicalSwitchPortCalls counts UpdateLogicalSwitchPort invocations // so tests can assert idempotent read-before-write paths (e.g. // ensureLocalnetOptions — mulga-998.b Fix 3). UpdateLogicalSwitchPortCalls int + + // SetGatewayChassisCalls / DeleteGatewayChassisCalls / + // UpdateGatewayChassisPriorityCalls let tests distinguish between + // "no-op", "create", "delete", and "priority update" paths through + // reconcileGatewayChassis (mulga-999). + SetGatewayChassisCalls int + DeleteGatewayChassisCalls int + UpdateGatewayChassisPriorityCalls int } // NewMockOVNClient creates a new MockOVNClient for testing. func NewMockOVNClient() *MockOVNClient { return &MockOVNClient{ - switches: make(map[string]*nbdb.LogicalSwitch), - ports: make(map[string]*nbdb.LogicalSwitchPort), - routers: make(map[string]*nbdb.LogicalRouter), - routerPorts: make(map[string]*nbdb.LogicalRouterPort), - dhcpOpts: make(map[string]*nbdb.DHCPOptions), - nats: make(map[string]*nbdb.NAT), - staticRoutes: make(map[string]*nbdb.LogicalRouterStaticRoute), - portGroups: make(map[string]*nbdb.PortGroup), - acls: make(map[string]*nbdb.ACL), + switches: make(map[string]*nbdb.LogicalSwitch), + ports: make(map[string]*nbdb.LogicalSwitchPort), + routers: make(map[string]*nbdb.LogicalRouter), + routerPorts: make(map[string]*nbdb.LogicalRouterPort), + dhcpOpts: make(map[string]*nbdb.DHCPOptions), + nats: make(map[string]*nbdb.NAT), + staticRoutes: make(map[string]*nbdb.LogicalRouterStaticRoute), + portGroups: make(map[string]*nbdb.PortGroup), + acls: make(map[string]*nbdb.ACL), + gatewayChassis: make(map[string]*nbdb.GatewayChassis), } } @@ -580,6 +590,21 @@ func (m *MockOVNClient) ClearACLs(_ context.Context, portGroupName string) error return nil } +// ListLogicalRouterPorts returns every LRP across the mock state. +func (m *MockOVNClient) ListLogicalRouterPorts(_ context.Context) ([]nbdb.LogicalRouterPort, error) { + m.mu.Lock() + defer m.mu.Unlock() + result := make([]nbdb.LogicalRouterPort, 0, len(m.routerPorts)) + for _, lrp := range m.routerPorts { + result = append(result, *lrp) + } + return result, nil +} + +// SetGatewayChassis is the idempotent read-then-decide path mirrored from +// LiveOVNClient. Tests rely on the mock applying the same "no-op when already +// correct" semantics so the call counters distinguish create vs. update vs. +// no-op (mulga-999). func (m *MockOVNClient) SetGatewayChassis(_ context.Context, lrpName string, chassisName string, priority int) error { m.mu.Lock() defer m.mu.Unlock() @@ -588,7 +613,85 @@ func (m *MockOVNClient) SetGatewayChassis(_ context.Context, lrpName string, cha return fmt.Errorf("logical router port %q not found", lrpName) } gcName := lrpName + "-" + chassisName - lrp.GatewayChassis = append(lrp.GatewayChassis, gcName) - _ = priority + for _, gc := range m.gatewayChassis { + if gc.Name != gcName { + continue + } + if gc.Priority == priority { + return nil + } + gc.Priority = priority + m.UpdateGatewayChassisPriorityCalls++ + return nil + } + gc := &nbdb.GatewayChassis{ + UUID: utils.GenerateResourceID("gc"), + Name: gcName, + ChassisName: chassisName, + Priority: priority, + ExternalIDs: map[string]string{}, + Options: map[string]string{}, + } + m.gatewayChassis[gc.UUID] = gc + lrp.GatewayChassis = append(lrp.GatewayChassis, gc.UUID) + m.SetGatewayChassisCalls++ return nil } + +func (m *MockOVNClient) GetGatewayChassisByName(_ context.Context, name string) (*nbdb.GatewayChassis, error) { + m.mu.Lock() + defer m.mu.Unlock() + for _, gc := range m.gatewayChassis { + if gc.Name == name { + result := *gc + return &result, nil + } + } + return nil, nil +} + +func (m *MockOVNClient) ListGatewayChassis(_ context.Context) ([]nbdb.GatewayChassis, error) { + m.mu.Lock() + defer m.mu.Unlock() + result := make([]nbdb.GatewayChassis, 0, len(m.gatewayChassis)) + for _, gc := range m.gatewayChassis { + result = append(result, *gc) + } + return result, nil +} + +func (m *MockOVNClient) DeleteGatewayChassis(_ context.Context, lrpName string, gcUUID string) error { + m.mu.Lock() + defer m.mu.Unlock() + lrp, exists := m.routerPorts[lrpName] + if !exists { + return fmt.Errorf("logical router port %q not found", lrpName) + } + if _, ok := m.gatewayChassis[gcUUID]; !ok { + return fmt.Errorf("gateway_chassis %q not found", gcUUID) + } + for i, u := range lrp.GatewayChassis { + if u == gcUUID { + lrp.GatewayChassis = append(lrp.GatewayChassis[:i], lrp.GatewayChassis[i+1:]...) + break + } + } + delete(m.gatewayChassis, gcUUID) + m.DeleteGatewayChassisCalls++ + return nil +} + +// SeedGatewayChassis lets tests pre-populate a Gateway_Chassis row directly, +// bypassing the idempotent SetGatewayChassis path. Useful for setting up a +// "stale row" scenario for reconcileGatewayChassis tests (mulga-999). +func (m *MockOVNClient) SeedGatewayChassis(lrpName string, gc *nbdb.GatewayChassis) { + m.mu.Lock() + defer m.mu.Unlock() + if gc.UUID == "" { + gc.UUID = utils.GenerateResourceID("gc") + } + m.gatewayChassis[gc.UUID] = gc + if lrp, ok := m.routerPorts[lrpName]; ok { + lrp.GatewayChassis = append(lrp.GatewayChassis, gc.UUID) + } +} diff --git a/spinifex/services/vpcd/ovn.go b/spinifex/services/vpcd/ovn.go index a8ada215f..bd1f6f62d 100644 --- a/spinifex/services/vpcd/ovn.go +++ b/spinifex/services/vpcd/ovn.go @@ -76,6 +76,7 @@ type OVNClient interface { CreateLogicalRouterPort(ctx context.Context, routerName string, lrp *nbdb.LogicalRouterPort) error DeleteLogicalRouterPort(ctx context.Context, routerName string, portName string) error GetLogicalRouterPort(ctx context.Context, name string) (*nbdb.LogicalRouterPort, error) + ListLogicalRouterPorts(ctx context.Context) ([]nbdb.LogicalRouterPort, error) // DHCP Options CreateDHCPOptions(ctx context.Context, opts *nbdb.DHCPOptions) (string, error) @@ -105,6 +106,9 @@ type OVNClient interface { // Gateway Chassis (HA scheduling for gateway router ports) SetGatewayChassis(ctx context.Context, lrpName string, chassisName string, priority int) error + GetGatewayChassisByName(ctx context.Context, name string) (*nbdb.GatewayChassis, error) + ListGatewayChassis(ctx context.Context) ([]nbdb.GatewayChassis, error) + DeleteGatewayChassis(ctx context.Context, lrpName string, gcUUID string) error } // namedUUID generates a valid OVSDB named-uuid from a prefix and name. @@ -464,14 +468,29 @@ func (c *LiveOVNClient) GetLogicalRouterPort(ctx context.Context, name string) ( return &ports[0], nil } +// SetGatewayChassis binds a chassis to an LRP for HA gateway scheduling. Read- +// then-decide so it tolerates re-runs of reconcile/IGW-attach: absent → create +// + LRP-mutate; present + same priority → no-op; present + different priority +// → mutate priority on the existing row. Required for the reconcile-time +// rebind step that recovers from chassis_name drift (mulga-999). func (c *LiveOVNClient) SetGatewayChassis(ctx context.Context, lrpName string, chassisName string, priority int) error { + gcName := lrpName + "-" + chassisName + existing, err := c.GetGatewayChassisByName(ctx, gcName) + if err != nil { + return fmt.Errorf("get existing gateway chassis: %w", err) + } + if existing != nil { + if existing.Priority == priority { + return nil + } + return c.updateGatewayChassisPriority(ctx, existing, priority) + } + lrp, err := c.GetLogicalRouterPort(ctx, lrpName) if err != nil { return fmt.Errorf("get logical router port for gateway chassis: %w", err) } - // Create the Gateway_Chassis row with a deterministic name - gcName := lrpName + "-" + chassisName gc := &nbdb.GatewayChassis{ UUID: namedUUID("gc_", gcName), Name: gcName, @@ -486,7 +505,6 @@ func (c *LiveOVNClient) SetGatewayChassis(ctx context.Context, lrpName string, c return fmt.Errorf("create gateway chassis ops: %w", err) } - // Add the Gateway_Chassis UUID to the router port's gateway_chassis set mutateOps, err := c.client.Where(lrp).Mutate(lrp, model.Mutation{ Field: &lrp.GatewayChassis, Mutator: "insert", @@ -503,6 +521,90 @@ func (c *LiveOVNClient) SetGatewayChassis(ctx context.Context, lrpName string, c return nil } +func (c *LiveOVNClient) updateGatewayChassisPriority(ctx context.Context, gc *nbdb.GatewayChassis, priority int) error { + gc.Priority = priority + ops, err := c.client.Where(gc).Update(gc, &gc.Priority) + if err != nil { + return fmt.Errorf("update gateway_chassis priority ops: %w", err) + } + if err := c.transactOps(ctx, ops); err != nil { + return fmt.Errorf("update gateway_chassis priority transact: %w", err) + } + return nil +} + +// GetGatewayChassisByName looks up a Gateway_Chassis row by its `name` column +// (the deterministic "lrpName-chassisName" form). Returns (nil, nil) when no +// row matches — matches the cache-lookup convention used by GetLogicalSwitch +// et al. +func (c *LiveOVNClient) GetGatewayChassisByName(ctx context.Context, name string) (*nbdb.GatewayChassis, error) { + var rows []nbdb.GatewayChassis + err := c.client.WhereCache(func(gc *nbdb.GatewayChassis) bool { + return gc.Name == name + }).List(ctx, &rows) + if err != nil { + return nil, fmt.Errorf("list gateway_chassis by name: %w", err) + } + if len(rows) == 0 { + return nil, nil + } + return &rows[0], nil +} + +// ListGatewayChassis returns every Gateway_Chassis row. The reconcile loop +// uses this to find rows referencing chassis names that no longer exist in +// the SBDB (e.g. because the OVS system-id changed across a reboot — +// mulga-999). +func (c *LiveOVNClient) ListGatewayChassis(ctx context.Context) ([]nbdb.GatewayChassis, error) { + var rows []nbdb.GatewayChassis + if err := c.client.List(ctx, &rows); err != nil { + return nil, fmt.Errorf("list gateway_chassis: %w", err) + } + return rows, nil +} + +// DeleteGatewayChassis removes a Gateway_Chassis row and detaches it from the +// owning LRP in one transaction. lrpName is required so the mutation targets +// the correct router port; gcUUID identifies the row to remove. +func (c *LiveOVNClient) DeleteGatewayChassis(ctx context.Context, lrpName string, gcUUID string) error { + lrp, err := c.GetLogicalRouterPort(ctx, lrpName) + if err != nil { + return fmt.Errorf("get logical router port for gateway chassis delete: %w", err) + } + + mutateOps, err := c.client.Where(lrp).Mutate(lrp, model.Mutation{ + Field: &lrp.GatewayChassis, + Mutator: "delete", + Value: []string{gcUUID}, + }) + if err != nil { + return fmt.Errorf("mutate logical router port gateway_chassis (delete) ops: %w", err) + } + + gc := &nbdb.GatewayChassis{UUID: gcUUID} + deleteOps, err := c.client.Where(gc).Delete() + if err != nil { + return fmt.Errorf("delete gateway_chassis ops: %w", err) + } + + ops := append(mutateOps, deleteOps...) + if err := c.transactOps(ctx, ops); err != nil { + return fmt.Errorf("delete gateway_chassis transact: %w", err) + } + return nil +} + +// ListLogicalRouterPorts returns every LRP across all routers. Used by the +// reconcile-time gateway-chassis rebind to find every LRP tagged +// external_ids:spinifex:role=gateway (mulga-999). +func (c *LiveOVNClient) ListLogicalRouterPorts(ctx context.Context) ([]nbdb.LogicalRouterPort, error) { + var rows []nbdb.LogicalRouterPort + if err := c.client.List(ctx, &rows); err != nil { + return nil, fmt.Errorf("list logical_router_port: %w", err) + } + return rows, nil +} + func (c *LiveOVNClient) CreateDHCPOptions(ctx context.Context, opts *nbdb.DHCPOptions) (string, error) { ops, err := c.client.Create(opts) if err != nil { diff --git a/spinifex/services/vpcd/ovn_test.go b/spinifex/services/vpcd/ovn_test.go index b181d4cf6..d25be9659 100644 --- a/spinifex/services/vpcd/ovn_test.go +++ b/spinifex/services/vpcd/ovn_test.go @@ -524,3 +524,73 @@ func TestNBDB_FullDatabaseModel(t *testing.T) { } } } + +// --- SetGatewayChassis idempotency (mulga-999) --- + +func TestSetGatewayChassis_Idempotent(t *testing.T) { + mock := NewMockOVNClient() + _ = mock.Connect(context.Background()) + ctx := context.Background() + + if err := mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{Name: "vpc-A"}); err != nil { + t.Fatalf("CreateLogicalRouter: %v", err) + } + if err := mock.CreateLogicalRouterPort(ctx, "vpc-A", &nbdb.LogicalRouterPort{Name: "gw-A"}); err != nil { + t.Fatalf("CreateLogicalRouterPort: %v", err) + } + + if err := mock.SetGatewayChassis(ctx, "gw-A", "chassis-1", 20); err != nil { + t.Fatalf("SetGatewayChassis (1): %v", err) + } + if err := mock.SetGatewayChassis(ctx, "gw-A", "chassis-1", 20); err != nil { + t.Fatalf("SetGatewayChassis (2): %v", err) + } + + rows, err := mock.ListGatewayChassis(ctx) + if err != nil { + t.Fatalf("ListGatewayChassis: %v", err) + } + if len(rows) != 1 { + t.Fatalf("expected 1 row, got %d (%v)", len(rows), rows) + } + if mock.SetGatewayChassisCalls != 1 { + t.Errorf("expected 1 create call, got %d", mock.SetGatewayChassisCalls) + } + if mock.UpdateGatewayChassisPriorityCalls != 0 { + t.Errorf("expected 0 priority updates, got %d", mock.UpdateGatewayChassisPriorityCalls) + } +} + +func TestSetGatewayChassis_UpdatesPriority(t *testing.T) { + mock := NewMockOVNClient() + _ = mock.Connect(context.Background()) + ctx := context.Background() + + if err := mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{Name: "vpc-A"}); err != nil { + t.Fatalf("CreateLogicalRouter: %v", err) + } + if err := mock.CreateLogicalRouterPort(ctx, "vpc-A", &nbdb.LogicalRouterPort{Name: "gw-A"}); err != nil { + t.Fatalf("CreateLogicalRouterPort: %v", err) + } + + if err := mock.SetGatewayChassis(ctx, "gw-A", "chassis-1", 20); err != nil { + t.Fatalf("SetGatewayChassis at 20: %v", err) + } + if err := mock.SetGatewayChassis(ctx, "gw-A", "chassis-1", 15); err != nil { + t.Fatalf("SetGatewayChassis at 15: %v", err) + } + + rows, err := mock.ListGatewayChassis(ctx) + if err != nil { + t.Fatalf("ListGatewayChassis: %v", err) + } + if len(rows) != 1 { + t.Fatalf("expected 1 row (mutated, not duplicated), got %d", len(rows)) + } + if rows[0].Priority != 15 { + t.Errorf("expected priority 15 after update, got %d", rows[0].Priority) + } + if mock.UpdateGatewayChassisPriorityCalls != 1 { + t.Errorf("expected 1 priority update, got %d", mock.UpdateGatewayChassisPriorityCalls) + } +} diff --git a/spinifex/services/vpcd/reconcile.go b/spinifex/services/vpcd/reconcile.go index 2342b03e0..58d3c4cdb 100644 --- a/spinifex/services/vpcd/reconcile.go +++ b/spinifex/services/vpcd/reconcile.go @@ -94,9 +94,22 @@ func Reconcile(ctx context.Context, topo *TopologyHandler, bootstrap *BootstrapV // ReconcileFromKV reads all VPCs, subnets, IGW attachments, and ENIs from NATS KV // and ensures OVN topology matches. This is Pass 2 from the Phase 12 plan — it // handles reboots, OVN DB loss, and any missed events. All operations are idempotent. -func ReconcileFromKV(ctx context.Context, nc *nats.Conn, topo *TopologyHandler) ReconcileResult { +// +// chassisNames is the set of OVN chassis names discovered from the SBDB at +// startup; reconcileGatewayChassis uses it to delete stale Gateway_Chassis +// rows (e.g. left over from a chassis_name change across reboot) and rebind +// every gateway LRP against the live chassis (mulga-999). +func ReconcileFromKV(ctx context.Context, nc *nats.Conn, topo *TopologyHandler, chassisNames []string) ReconcileResult { var result ReconcileResult + // 0. Reconcile gateway_chassis bindings before any per-VPC work. State + // drift here (a gateway_chassis row pointing at a chassis that no longer + // exists) leaves the cr-gw* port unbound — centralized NAT silently + // breaks. Single SB-derived list, applied once. + if err := topo.reconcileGatewayChassis(ctx, chassisNames); err != nil { + slog.Warn("vpcd reconcile-kv: gateway_chassis reconcile failed", "err", err) + } + js, err := nc.JetStream() if err != nil { slog.Warn("vpcd reconcile-kv: failed to get JetStream context", "err", err) diff --git a/spinifex/services/vpcd/reconcile_test.go b/spinifex/services/vpcd/reconcile_test.go index a20c44230..6c9317d46 100644 --- a/spinifex/services/vpcd/reconcile_test.go +++ b/spinifex/services/vpcd/reconcile_test.go @@ -8,6 +8,7 @@ import ( handlers_ec2_igw "github.com/mulgadc/spinifex/spinifex/handlers/ec2/igw" handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" + "github.com/mulgadc/spinifex/spinifex/services/vpcd/nbdb" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" "github.com/stretchr/testify/assert" @@ -233,7 +234,7 @@ func TestReconcileFromKV_CreatesFullTopology(t *testing.T) { }}, ) - result := ReconcileFromKV(ctx, nc, topo) + result := ReconcileFromKV(ctx, nc, topo, nil) assert.Equal(t, 1, result.RoutersCreated) assert.Equal(t, 1, result.SwitchesCreated) assert.Equal(t, 1, result.IGWsCreated) @@ -279,12 +280,12 @@ func TestReconcileFromKV_Idempotent(t *testing.T) { nil, nil, ) - r1 := ReconcileFromKV(ctx, nc, topo) + r1 := ReconcileFromKV(ctx, nc, topo, nil) assert.Equal(t, 1, r1.RoutersCreated) assert.Equal(t, 1, r1.SwitchesCreated) // Second run: everything exists - r2 := ReconcileFromKV(ctx, nc, topo) + r2 := ReconcileFromKV(ctx, nc, topo, nil) assert.Equal(t, 0, r2.RoutersCreated) assert.Equal(t, 0, r2.SwitchesCreated) } @@ -314,7 +315,7 @@ func TestReconcileFromKV_SkipsDetachedIGW(t *testing.T) { nil, ) - result := ReconcileFromKV(ctx, nc, topo) + result := ReconcileFromKV(ctx, nc, topo, nil) assert.Equal(t, 1, result.RoutersCreated) // VPC router created assert.Equal(t, 0, result.IGWsCreated) // Detached IGW skipped @@ -330,7 +331,7 @@ func TestReconcileFromKV_NoBuckets(t *testing.T) { topo := NewTopologyHandler(ovn) // No KV buckets seeded — should handle gracefully - result := ReconcileFromKV(context.Background(), nc, topo) + result := ReconcileFromKV(context.Background(), nc, topo, nil) assert.Equal(t, 0, result.RoutersCreated) assert.Equal(t, 0, result.SwitchesCreated) assert.Equal(t, 0, result.IGWsCreated) @@ -418,7 +419,7 @@ func TestReconcileFromKV_VersionKeysAndBadJSON(t *testing.T) { _, err = eniKV.Put(utils.AccountKey("000000000001", "eni-ver1"), eniData) require.NoError(t, err) - result := ReconcileFromKV(ctx, nc, topo) + result := ReconcileFromKV(ctx, nc, topo, nil) // Valid records should still be reconciled despite bad records assert.Equal(t, 1, result.RoutersCreated) @@ -458,9 +459,108 @@ func TestReconcileFromKV_EmptyBuckets(t *testing.T) { _, err = js.CreateKeyValue(&nats.KeyValueConfig{Bucket: handlers_ec2_vpc.KVBucketENIs, History: 1}) require.NoError(t, err) - result := ReconcileFromKV(context.Background(), nc, topo) + result := ReconcileFromKV(context.Background(), nc, topo, nil) assert.Equal(t, 0, result.RoutersCreated) assert.Equal(t, 0, result.SwitchesCreated) assert.Equal(t, 0, result.IGWsCreated) assert.Equal(t, 0, result.PortsCreated) } + +// --- reconcileGatewayChassis tests (mulga-999) --- + +// seedGatewayLRP creates a router + LRP tagged spinifex:role=gateway so +// reconcileGatewayChassis's rebind step has something to bind. +func seedGatewayLRP(t *testing.T, ovn *MockOVNClient, routerName, lrpName string) { + t.Helper() + ctx := context.Background() + require.NoError(t, ovn.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{Name: routerName})) + require.NoError(t, ovn.CreateLogicalRouterPort(ctx, routerName, &nbdb.LogicalRouterPort{ + Name: lrpName, + ExternalIDs: map[string]string{"spinifex:role": "gateway"}, + })) +} + +func TestReconcileGatewayChassis_RemovesStaleRows(t *testing.T) { + ovn := NewMockOVNClient() + _ = ovn.Connect(context.Background()) + topo := NewTopologyHandler(ovn) + ctx := context.Background() + + seedGatewayLRP(t, ovn, "vpc-A", "gw-A") + // One stale (chassis-old not in valid set), one live (UUID-A is). + ovn.SeedGatewayChassis("gw-A", &nbdb.GatewayChassis{ + Name: "gw-A-chassis-old", + ChassisName: "chassis-old", + Priority: 20, + }) + ovn.SeedGatewayChassis("gw-A", &nbdb.GatewayChassis{ + Name: "gw-A-UUID-A", + ChassisName: "UUID-A", + Priority: 20, + }) + + require.NoError(t, topo.reconcileGatewayChassis(ctx, []string{"UUID-A"})) + + rows, err := ovn.ListGatewayChassis(ctx) + require.NoError(t, err) + require.Len(t, rows, 1, "stale row should be deleted, live row should remain") + assert.Equal(t, "UUID-A", rows[0].ChassisName) + assert.Equal(t, 1, ovn.DeleteGatewayChassisCalls) +} + +func TestReconcileGatewayChassis_RebindsAllGatewayLRPs(t *testing.T) { + ovn := NewMockOVNClient() + _ = ovn.Connect(context.Background()) + topo := NewTopologyHandler(ovn) + ctx := context.Background() + + seedGatewayLRP(t, ovn, "vpc-A", "gw-A") + seedGatewayLRP(t, ovn, "vpc-B", "gw-B") + // Untagged LRP must NOT be rebinded. + require.NoError(t, ovn.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{Name: "vpc-C"})) + require.NoError(t, ovn.CreateLogicalRouterPort(ctx, "vpc-C", &nbdb.LogicalRouterPort{ + Name: "rtr-subnet-C", + ExternalIDs: map[string]string{"spinifex:role": "internal"}, + })) + + require.NoError(t, topo.reconcileGatewayChassis(ctx, []string{"chassis-A", "chassis-B"})) + + // 2 gateway LRPs × 2 chassis = 4 SetGatewayChassis create calls. The + // internal LRP must contribute zero. + assert.Equal(t, 4, ovn.SetGatewayChassisCalls) + rows, err := ovn.ListGatewayChassis(ctx) + require.NoError(t, err) + assert.Len(t, rows, 4) + + // Verify priorities — first chassis gets 20, second 15. + prioByGC := make(map[string]int, len(rows)) + for _, gc := range rows { + prioByGC[gc.Name] = gc.Priority + } + assert.Equal(t, 20, prioByGC["gw-A-chassis-A"]) + assert.Equal(t, 15, prioByGC["gw-A-chassis-B"]) + assert.Equal(t, 20, prioByGC["gw-B-chassis-A"]) + assert.Equal(t, 15, prioByGC["gw-B-chassis-B"]) +} + +func TestReconcileGatewayChassis_NoOpWhenAlreadyCorrect(t *testing.T) { + ovn := NewMockOVNClient() + _ = ovn.Connect(context.Background()) + topo := NewTopologyHandler(ovn) + ctx := context.Background() + + seedGatewayLRP(t, ovn, "vpc-A", "gw-A") + require.NoError(t, topo.reconcileGatewayChassis(ctx, []string{"chassis-A"})) + // Reset counters from the first (creating) pass. + ovn.SetGatewayChassisCalls = 0 + ovn.DeleteGatewayChassisCalls = 0 + ovn.UpdateGatewayChassisPriorityCalls = 0 + + // Second pass: state is already correct; idempotent path must take no + // destructive or mutative action. + require.NoError(t, topo.reconcileGatewayChassis(ctx, []string{"chassis-A"})) + + assert.Equal(t, 0, ovn.SetGatewayChassisCalls, "no new creates expected") + assert.Equal(t, 0, ovn.DeleteGatewayChassisCalls, "no deletes expected") + assert.Equal(t, 0, ovn.UpdateGatewayChassisPriorityCalls, "no priority updates expected") +} diff --git a/spinifex/services/vpcd/topology.go b/spinifex/services/vpcd/topology.go index 823a792ff..88d41a2fd 100644 --- a/spinifex/services/vpcd/topology.go +++ b/spinifex/services/vpcd/topology.go @@ -1291,6 +1291,74 @@ func (h *TopologyHandler) reconcileIGW(ctx context.Context, vpcId, igwId string) return nil } +// reconcileGatewayChassis brings every gateway LRP's HA-scheduling state into +// agreement with the live SBDB chassis list. Two-phase: +// +// 1. Delete Gateway_Chassis rows whose chassis_name no longer matches any +// entry in validNames. These rows are inevitable when the OVS system-id +// changes across a reboot (mulga-999): NB still references the old +// chassis-* name, but the SB Chassis row is now under a UUID, so OVN +// can't resolve the binding and cr-gw* port-bindings sit empty. +// +// 2. Re-bind every LRP tagged spinifex:role=gateway against validNames via +// the idempotent SetGatewayChassis. Existing rows with matching priority +// are no-ops; mismatched priorities are mutated; missing rows are +// created. +// +// Called as the first step of ReconcileFromKV so chassis state is correct +// before any reconcileVPC / reconcileIGW work. Failures are logged and +// returned but do not abort the rest of the reconcile loop — partial +// progress is better than none. +func (h *TopologyHandler) reconcileGatewayChassis(ctx context.Context, validNames []string) error { + valid := make(map[string]struct{}, len(validNames)) + for _, n := range validNames { + valid[n] = struct{}{} + } + + rows, err := h.ovn.ListGatewayChassis(ctx) + if err != nil { + return fmt.Errorf("list gateway_chassis: %w", err) + } + for _, gc := range rows { + if _, ok := valid[gc.ChassisName]; ok { + continue + } + // Gateway_Chassis.Name is "lrpName-chassisName" (see SetGatewayChassis). + // The chassis suffix is unique because chassis names are globally + // unique; trimming it leaves the owning LRP name. + lrpName := strings.TrimSuffix(gc.Name, "-"+gc.ChassisName) + if err := h.ovn.DeleteGatewayChassis(ctx, lrpName, gc.UUID); err != nil { + slog.Warn("vpcd: failed to delete stale gateway_chassis", + "row", gc.Name, "chassis_name", gc.ChassisName, "err", err) + continue + } + slog.Warn("vpcd: deleted stale gateway_chassis", + "row", gc.Name, "chassis_name", gc.ChassisName, "lrp", lrpName) + } + + if len(validNames) == 0 { + return nil + } + + lrps, err := h.ovn.ListLogicalRouterPorts(ctx) + if err != nil { + return fmt.Errorf("list logical_router_port: %w", err) + } + for _, lrp := range lrps { + if lrp.ExternalIDs["spinifex:role"] != "gateway" { + continue + } + for i, name := range validNames { + priority := max(20-(i*5), 1) + if err := h.ovn.SetGatewayChassis(ctx, lrp.Name, name, priority); err != nil { + slog.Warn("vpcd: failed to set gateway chassis on rebind", + "lrp", lrp.Name, "chassis", name, "priority", priority, "err", err) + } + } + } + return nil +} + // --- Helpers --- // subnetGateway computes the gateway IP (.1) from a CIDR string. diff --git a/spinifex/services/vpcd/vpcd.go b/spinifex/services/vpcd/vpcd.go index ef510ef9d..2e85a7eeb 100644 --- a/spinifex/services/vpcd/vpcd.go +++ b/spinifex/services/vpcd/vpcd.go @@ -61,10 +61,6 @@ type Config struct { ExternalMode string // ExternalPools holds the cluster-wide external IP pool configs. ExternalPools []ExternalPoolConfig - // ChassisNames are fallback OVN chassis identifiers for gateway HA scheduling. - // Normally discovered from the OVN Southbound DB at startup. Only used if - // SBDB discovery fails. - ChassisNames []string // Bootstrap holds the default VPC config from spinifex.toml for first-boot reconciliation. Bootstrap *BootstrapVPC // ExternalInterface is the WAN NIC name (e.g., "enp0s3"). Used to align @@ -331,17 +327,20 @@ func launchService(cfg *Config) error { topoOpts = append(topoOpts, WithExternalNetwork(cfg.ExternalMode, cfg.ExternalPools)) slog.Info("External network enabled", "mode", cfg.ExternalMode, "pools", len(cfg.ExternalPools)) } - // Discover chassis from OVN SBDB (source of truth). Fall back to config - // if SBDB query fails (e.g., SBDB address not configured). + // Discover chassis from OVN SBDB. The OVS-managed system-id (persisted at + // /etc/openvswitch/system-id.conf and re-applied on every boot) is the + // canonical chassis identity; SBDB is the only source of truth. Fail-start + // rather than guess — a missing chassis means ovn-controller hasn't + // registered yet (boot race) and systemd's Restart=on-failure will retry. chassisNames, err := discoverChassis(cfg.OVNSBAddr) if err != nil { - slog.Warn("Failed to discover chassis from OVN SBDB, falling back to config", "err", err) - chassisNames = cfg.ChassisNames + return fmt.Errorf("vpcd: discover OVN chassis: %w", err) } - if len(chassisNames) > 0 { - topoOpts = append(topoOpts, WithChassisNames(chassisNames)) - slog.Info("Gateway chassis configured", "source", "sbdb", "chassis", chassisNames) + if len(chassisNames) == 0 { + return fmt.Errorf("vpcd: no OVN chassis registered in SBDB — is ovn-controller running and connected?") } + topoOpts = append(topoOpts, WithChassisNames(chassisNames)) + slog.Info("vpcd: gateway chassis discovered", "chassis", chassisNames) topoOpts = append(topoOpts, WithBridgeMode(bridgeMode)) topo := NewTopologyHandler(liveClient, topoOpts...) @@ -376,7 +375,7 @@ func launchService(cfg *Config) error { // Pass 2: Reconcile from NATS KV (handles reboots, OVN DB loss, missed events). // Runs after subscribing so new events are not missed during reconciliation. - ReconcileFromKV(ctx, nc, topo) + ReconcileFromKV(ctx, nc, topo, chassisNames) // DHCP manager: services vpc.dhcp.acquire/release requests from the // daemon-side ExternalIPAM handlers, runs a renewal goroutine per lease, diff --git a/spinifex/services/vpcd/vpcd_test.go b/spinifex/services/vpcd/vpcd_test.go index 7504a5bce..5d1c09818 100644 --- a/spinifex/services/vpcd/vpcd_test.go +++ b/spinifex/services/vpcd/vpcd_test.go @@ -123,24 +123,58 @@ func TestDiscoverChassis_EmptyOutput(t *testing.T) { } } -func TestDiscoverChassis_Error_FallsBackToConfig(t *testing.T) { +// chassisDiscoveryOrFail mirrors the fail-start branch in launchService so the +// contract is unit-testable without a full NATS + OVN NB stand-up. Any change +// to the fail-start semantics must also update this helper (mulga-999). +func chassisDiscoveryOrFail(sbAddr string) ([]string, error) { + names, err := discoverChassis(sbAddr) + if err != nil { + return nil, fmt.Errorf("vpcd: discover OVN chassis: %w", err) + } + if len(names) == 0 { + return nil, fmt.Errorf("vpcd: no OVN chassis registered in SBDB — is ovn-controller running and connected?") + } + return names, nil +} + +// TestLaunchService_FailsOnDiscoverChassisError pins the fail-start contract: +// when the SBDB query errors, vpcd must surface a wrapped error rather than +// guessing a chassis name (which is what the pre-mulga-999 fallback did). +func TestLaunchService_FailsOnDiscoverChassisError(t *testing.T) { orig := discoverChassis defer func() { discoverChassis = orig }() - discoverChassis = func(sbAddr string) ([]string, error) { return nil, fmt.Errorf("connection refused") } - // Simulate the fallback logic from launchService - _, err := discoverChassis("") + _, err := chassisDiscoveryOrFail("") if err == nil { - t.Fatal("expected error from discoverChassis") + t.Fatal("expected error to propagate from discoverChassis") + } + if !strings.Contains(err.Error(), "discover OVN chassis") { + t.Errorf("expected wrapped error, got: %v", err) + } + if !strings.Contains(err.Error(), "connection refused") { + t.Errorf("expected underlying cause to be preserved, got: %v", err) + } +} + +// TestLaunchService_FailsOnEmptyChassisDiscovery pins the no-fallback +// contract: an empty SBDB chassis list (typical when ovn-controller hasn't +// registered yet) must fail-start rather than silently bind nothing. +func TestLaunchService_FailsOnEmptyChassisDiscovery(t *testing.T) { + orig := discoverChassis + defer func() { discoverChassis = orig }() + discoverChassis = func(sbAddr string) ([]string, error) { + return []string{}, nil } - // Fallback to config (as launchService does) - chassisNames := []string{"chassis-node1"} - if len(chassisNames) != 1 || chassisNames[0] != "chassis-node1" { - t.Errorf("expected fallback to config names, got %v", chassisNames) + _, err := chassisDiscoveryOrFail("") + if err == nil { + t.Fatal("expected error when SBDB has zero chassis registered") + } + if !strings.Contains(err.Error(), "no OVN chassis registered") { + t.Errorf("expected fail-start message, got: %v", err) } }