From d8128ec2f38be694093eb7c1307494071c7701b6 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Thu, 21 May 2026 11:37:42 +1000 Subject: [PATCH] refactor(vpcd): remove macvlan bridge mode Macvlan was a single-NIC SSH-safe hack that predated veth mode and was never selected by any active config path. Deletion is Phase 0 item 1 of the network layer redesign (mulga-siv-125.1). Removed: - BridgeModeMacvlan const + setMacvlanMAC + ifaceIsMacvlan probe - Macvlan MAC alignment block in launchService (only fired on dead path) - Macvlan branch in detectBridgeMode + matching tests - Macvlan duplicates of IGW/AddNAT centralized-NAT tests (veth versions already cover the same path) - --macvlan flag + macvlan case branch in setup-ovn.sh Refactored: - useCentralizedNAT() now == BridgeModeVeth (was != BridgeModeDirect); verifyBridgeMode already rejects empty/unknown at startup - Three IGW-pool tests now explicitly WithBridgeMode(BridgeModeVeth) (previously relied on empty-mode defaulting to centralized) Kept intentionally: - Deprecation notes in docs/compute/vpc-networking/README.md - Legacy macvlan-interface cleanup in reset-dev-env.sh + ansible teardown role (migration for operator systems with stale spx-ext-* interfaces from pre-veth installs) --- cmd/installer/install/install.go | 7 +- cmd/spinifex/cmd/admin.go | 6 +- scripts/reset-dev-env.sh | 5 +- scripts/setup-ovn.sh | 78 ++-------------- spinifex/admin/admin.go | 2 +- spinifex/config/config.go | 2 +- spinifex/services/vpcd/topology.go | 37 ++++---- spinifex/services/vpcd/topology_test.go | 115 +----------------------- spinifex/services/vpcd/vpcd.go | 60 ++----------- spinifex/services/vpcd/vpcd_test.go | 28 ++---- tests/e2e/TEST_COVERAGE.md | 2 +- tests/e2e/run-e2e.sh | 2 +- tests/e2e/run-reboot-e2e.sh | 2 +- tests/e2e/single/lifecycle_test.go | 4 +- 14 files changed, 57 insertions(+), 293 deletions(-) diff --git a/cmd/installer/install/install.go b/cmd/installer/install/install.go index d62174af0..3f01c86a1 100644 --- a/cmd/installer/install/install.go +++ b/cmd/installer/install/install.go @@ -304,9 +304,10 @@ func installSpinifex(cfg *Config) error { return err } - // dhcpcd-base is present on the installed system (used by setup-ovn.sh for - // macvlan mode). Mask the standalone dhcpcd.service so it never auto-starts - // and races with systemd-networkd's built-in DHCP client on br-wan. + // dhcpcd-base is present on the installed system (used by setup-ovn.sh + // for WAN DHCP acquisition). Mask the standalone dhcpcd.service so it + // never auto-starts and races with systemd-networkd's built-in DHCP + // client on br-wan. if err := maskSystemdUnit(mountRoot, "dhcpcd.service"); err != nil { slog.Warn("installSpinifex: failed to mask dhcpcd.service", "err", err) } diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index 7c4208480..8780fb873 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -893,9 +893,9 @@ func runAdminInit(cmd *cobra.Command, args []string) { fmt.Printf(" %-14s %-18s %-20s %-16s %s\n", iface.Name, iface.IP, iface.Subnet, gw, strings.ToUpper(iface.Role)) } if detected.LANCount == 0 { - fmt.Println("\n Mode: single-NIC (macvlan for external bridge)") + fmt.Println("\n Mode: single-NIC (veth-bridged external)") } else { - fmt.Printf("\n Mode: %d LAN + 1 WAN (macvlan for external bridge)\n", detected.LANCount) + fmt.Printf("\n Mode: %d LAN + 1 WAN (veth-bridged external)\n", detected.LANCount) } // Apply auto-detected values when flags not explicitly set @@ -984,7 +984,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { } // For pool/dhcp source, the gateway IP is obtained via DHCP on the - // macvlan/bridge interface (no static pool range or explicit gateway-ip). + // WAN bridge interface (no static pool range or explicit gateway-ip). useExternalDHCP := externalMode == "pool" && externalSource == "dhcp" && gatewayIP == "" // Validate IP address format diff --git a/scripts/reset-dev-env.sh b/scripts/reset-dev-env.sh index a8b5a0ac3..d0dcc8661 100755 --- a/scripts/reset-dev-env.sh +++ b/scripts/reset-dev-env.sh @@ -170,9 +170,10 @@ if [ -e /etc/systemd/network/15-spinifex-veth-wan.netdev ] || \ sudo networkctl reload 2>/dev/null || true fi -# Remove macvlan interfaces created by setup-ovn.sh +# Legacy: clean up macvlan interfaces left over from pre-veth setup-ovn.sh +# runs. Safe to remove once no operator system retains these. for iface in $(ip -o link show type macvlan 2>/dev/null | awk -F': ' '{print $2}' | grep '^spx-ext-'); do - echo " Deleting macvlan: $iface" + echo " Deleting legacy macvlan: $iface" sudo ip link del "$iface" 2>/dev/null || true done diff --git a/scripts/setup-ovn.sh b/scripts/setup-ovn.sh index 223cf4036..be9bd789b 100755 --- a/scripts/setup-ovn.sh +++ b/scripts/setup-ovn.sh @@ -16,9 +16,7 @@ # Options: # --management Also start OVN central services (NB DB, SB DB, ovn-northd) # --wan-bridge=NAME OVS bridge for WAN traffic (default: auto-detect from default route) -# --wan-iface=NAME Physical NIC to add to the WAN bridge (use with --wan-bridge or --macvlan) -# --macvlan Create macvlan off --wan-iface instead of moving NIC directly. -# SSH-safe for single-NIC hosts where WAN NIC carries SSH. +# --wan-iface=NAME Physical NIC to add to the WAN bridge (use with --wan-bridge) # --dhcp Obtain gateway IP via DHCP on the WAN bridge interface # --mgmt-bridge=NAME OVS bridge for system-instance control plane (default: br-mgmt) # --mgmt-cidr=CIDR IPv4 CIDR to assign on the mgmt bridge (default: 10.15.8.1/24) @@ -41,9 +39,6 @@ # # Dedicated WAN NIC (not your SSH NIC — you take responsibility): # ./scripts/setup-ovn.sh --management --wan-bridge=br-wan --wan-iface=eth1 # -# # Single-NIC host (SSH-safe macvlan): -# ./scripts/setup-ovn.sh --management --macvlan --wan-iface=eth0 -# # # Compute node joining an existing cluster: # ./scripts/setup-ovn.sh --ovn-remote=tcp:10.0.0.1:6642 --encap-ip=10.0.0.2 # @@ -56,7 +51,6 @@ set -e MANAGEMENT=false WAN_BRIDGE="" WAN_IFACE="" -MACVLAN_MODE=false EXTERNAL_DHCP=false MGMT_BRIDGE_ENABLED=true MGMT_BRIDGE="br-mgmt" @@ -69,7 +63,6 @@ ENCAP_IP="" for arg in "$@"; do case "$arg" in --management) MANAGEMENT=true ;; - --macvlan) MACVLAN_MODE=true ;; --dhcp) EXTERNAL_DHCP=true ;; --wan-bridge=*) WAN_BRIDGE="${arg#*=}" ;; --wan-iface=*) WAN_IFACE="${arg#*=}" ;; @@ -92,15 +85,13 @@ done # --- WAN bridge auto-detection --- # Determine the WAN bridge name and how to set it up. -WAN_BRIDGE_MODE="" # "existing", "veth", "direct", "macvlan", or "" +WAN_BRIDGE_MODE="" # "existing", "veth", "direct", or "" LINUX_BRIDGE="" # Set when WAN_BRIDGE_MODE="veth" — the Linux bridge behind the veth pair detect_wan_bridge() { # If --wan-bridge was explicitly given, use it if [ -n "$WAN_BRIDGE" ]; then - if [ "$MACVLAN_MODE" = true ] && [ -n "$WAN_IFACE" ]; then - WAN_BRIDGE_MODE="macvlan" - elif [ -n "$WAN_IFACE" ]; then + if [ -n "$WAN_IFACE" ]; then WAN_BRIDGE_MODE="direct" elif sudo ovs-vsctl br-exists "$WAN_BRIDGE" 2>/dev/null; then WAN_BRIDGE_MODE="existing" @@ -111,17 +102,6 @@ detect_wan_bridge() { return fi - # If --macvlan was given without --wan-bridge, we need --wan-iface - if [ "$MACVLAN_MODE" = true ]; then - if [ -z "$WAN_IFACE" ]; then - echo "ERROR: --macvlan requires --wan-iface=" - exit 1 - fi - WAN_BRIDGE="br-wan" - WAN_BRIDGE_MODE="macvlan" - return - fi - # Auto-detect: find the default route interface local default_dev default_dev=$(ip -4 route show default 2>/dev/null | awk '{for(i=1;i<=NF;i++) if($i=="dev") print $(i+1)}' | head -1) @@ -176,10 +156,7 @@ detect_wan_bridge() { echo " 2. Dedicated WAN NIC (NOT your SSH connection):" echo " ./scripts/setup-ovn.sh --management --wan-bridge=br-wan --wan-iface=$default_dev" echo "" - echo " 3. Single-NIC host (SSH-safe macvlan):" - echo " ./scripts/setup-ovn.sh --management --macvlan --wan-iface=$default_dev" - echo "" - echo " 4. No external networking (overlay-only):" + echo " 3. No external networking (overlay-only):" echo " ./scripts/setup-ovn.sh --management --encap-ip=$wan_ip" echo "============================================================" echo "" @@ -477,36 +454,6 @@ NETWORK echo " $WAN_BRIDGE: direct bridge on $WAN_IFACE" echo " NOTE: $WAN_IFACE is now an OVS port — no host IP on this NIC" ;; - - macvlan) - # Create a macvlan sub-interface in bridge mode off the WAN NIC. - # The host keeps its IP on the parent NIC — SSH-safe. OVN localnet - # traffic flows through the macvlan to the physical wire. - if ! ip link show "$WAN_IFACE" >/dev/null 2>&1; then - echo " ERROR: interface $WAN_IFACE does not exist" - echo " Available interfaces:" - ip -o link show | awk -F': ' '{print " " $2}' - exit 1 - fi - - MACVLAN_NAME="spx-ext-${WAN_IFACE}" - - sudo ovs-vsctl --may-exist add-br "$WAN_BRIDGE" - sudo ip link set "$WAN_BRIDGE" up - - if ip link show "$MACVLAN_NAME" >/dev/null 2>&1; then - echo " macvlan $MACVLAN_NAME already exists" - else - sudo ip link add "$MACVLAN_NAME" link "$WAN_IFACE" type macvlan mode bridge - echo " created macvlan: $MACVLAN_NAME (bridge mode) on $WAN_IFACE" - fi - - sudo ip link set "$MACVLAN_NAME" up - sudo ovs-vsctl --may-exist add-port "$WAN_BRIDGE" "$MACVLAN_NAME" - echo " $WAN_BRIDGE: macvlan port $MACVLAN_NAME on $WAN_IFACE" - echo " NOTE: host keeps its IP on $WAN_IFACE (SSH-safe)" - echo " QUIRK: host cannot reach VMs at their public IPs (macvlan isolation)" - ;; esac # --- DHCP: obtain gateway IP for OVN SNAT --- @@ -514,13 +461,8 @@ NETWORK echo "" echo "Step 3c: Obtaining external gateway IP via DHCP..." - # For macvlan mode, DHCP on the macvlan interface (it has L2 access to WAN). - # For direct/existing bridge, DHCP on the bridge itself. - if [ "$WAN_BRIDGE_MODE" = "macvlan" ]; then - DHCP_IFACE="spx-ext-${WAN_IFACE}" - else - DHCP_IFACE="$WAN_BRIDGE" - fi + # DHCP on the WAN bridge itself (direct/existing/veth). + DHCP_IFACE="$WAN_BRIDGE" # Run DHCP client to get a lease if command -v dhcpcd >/dev/null 2>&1; then @@ -815,14 +757,6 @@ if [ -n "$WAN_BRIDGE" ]; then echo " direct bridge: FAILED ($WAN_IFACE not on $WAN_BRIDGE)" OK=false fi - elif [ "$WAN_BRIDGE_MODE" = "macvlan" ]; then - MACVLAN_NAME="spx-ext-${WAN_IFACE}" - if ip link show "$MACVLAN_NAME" >/dev/null 2>&1; then - echo " macvlan: OK ($MACVLAN_NAME)" - else - echo " macvlan: FAILED ($MACVLAN_NAME not found)" - OK=false - fi fi else echo " $WAN_BRIDGE:$(printf '%*s' $((15 - ${#WAN_BRIDGE})) '') FAILED" diff --git a/spinifex/admin/admin.go b/spinifex/admin/admin.go index 130c9f9c9..fea3fd251 100644 --- a/spinifex/admin/admin.go +++ b/spinifex/admin/admin.go @@ -73,7 +73,7 @@ type ConfigSettings struct { ExternalMode string // "pool" or "" (disabled) ExternalIface string // WAN NIC name (e.g., "eth0", "eth1") DhcpBindBridge string // Bridge where the DHCP AF_PACKET socket binds (Linux bridge in veth mode, OVS bridge in direct mode; never "br-ext") - ExternalDHCP bool // Obtain gateway IP via DHCP on macvlan/bridge + ExternalDHCP bool // Obtain gateway IP via DHCP on the WAN bridge PoolName string // External pool name (e.g., "wan") PoolSource string // IP source: "static" (default) or "dhcp" (from router DHCP) PoolStart string // First IP in external pool range (static source only) diff --git a/spinifex/config/config.go b/spinifex/config/config.go index 9d18fba18..3b5a5960c 100644 --- a/spinifex/config/config.go +++ b/spinifex/config/config.go @@ -32,7 +32,7 @@ type ExternalPool struct { Region string `mapstructure:"region"` // Scope to region (optional — empty means any region) AZ string `mapstructure:"az"` // Scope to AZ (optional — empty means any AZ in region) // GwLrpRangeStart/End reserve a sub-range of the LAN for OVN gateway LRP IPs - // in centralized NAT mode (veth/macvlan). Each VPC consumes one IP from this + // in centralized NAT mode (veth). Each VPC consumes one IP from this // range so its gateway router port can ARP with a sender IP on the LAN // subnet — link-local 169.254.0.1/30 makes upstream routers reject ARP per // RFC 826 (mulga-siv-36). Must NOT overlap [RangeStart, RangeEnd] or IPAM diff --git a/spinifex/services/vpcd/topology.go b/spinifex/services/vpcd/topology.go index 60d7bb488..d49430d6d 100644 --- a/spinifex/services/vpcd/topology.go +++ b/spinifex/services/vpcd/topology.go @@ -74,7 +74,7 @@ const ( // chassis so the LRP IP itself never goes on the wire. The default route's // OutputPort is set explicitly so the WAN nexthop need not be on this subnet. // -// Centralized-NAT (veth/macvlan) bypasses this constant: the gateway LRP is +// Centralized-NAT (veth) bypasses this constant: the gateway LRP is // the on-wire egress point and must hold a WAN-subnet IP from the pool's // gw_lrp_range, otherwise the upstream router silently drops ARP requests // from an off-subnet sender (RFC 826) and the default route never resolves @@ -144,16 +144,13 @@ type NATGatewayEvent struct { // Bridge mode constants for external connectivity. const ( - // BridgeModeMacvlan uses a macvlan sub-interface on the WAN NIC. SSH-safe - // for single-NIC hosts but requires centralized NAT and MAC alignment. - BridgeModeMacvlan = "macvlan" // BridgeModeDirect adds the WAN NIC directly to br-external as an OVS port. - // Enables distributed NAT and avoids macvlan workarounds. Only safe when - // the WAN NIC is NOT the SSH/management NIC. + // Enables distributed NAT. Only safe when the WAN NIC is NOT the SSH/ + // management NIC. BridgeModeDirect = "direct" // BridgeModeVeth uses a veth pair to link a Linux bridge (br-wan) to an - // OVS bridge (br-ext). Requires centralized NAT like macvlan because the - // Linux bridge intermediary breaks distributed NAT hairpin routing. + // OVS bridge (br-ext). Requires centralized NAT because the Linux bridge + // intermediary breaks distributed NAT hairpin routing. BridgeModeVeth = "veth" // OvnExternalBridge is the OVS bridge that ovn-bridge-mappings targets // for the "external" localnet. Owned by setup-ovn.sh's ovn-bridge-mappings @@ -169,7 +166,7 @@ type TopologyHandler struct { externalMode string externalPools []ExternalPoolConfig chassisNames []string // OVN chassis names for gateway HA scheduling - bridgeMode string // "direct" or "macvlan" — controls NAT mode and localnet options + bridgeMode string // "direct" or "veth" — controls NAT mode and localnet options // nc is used to talk to vpcd's DHCPManager via vpc.dhcp.acquire / // vpc.dhcp.release for gateway LRP IPs in centralized NAT on // source="dhcp" pools (mulga-siv-38). nil when no DHCP pool is wired or @@ -204,9 +201,8 @@ func WithChassisNames(names []string) TopologyOption { } } -// WithBridgeMode sets the external bridge mode ("direct", "macvlan", or "veth"). -// Direct bridge enables distributed NAT; macvlan and veth use centralized NAT. -// Defaults to macvlan if not set (backward-compatible). +// WithBridgeMode sets the external bridge mode ("direct" or "veth"). +// Direct bridge enables distributed NAT; veth uses centralized NAT. func WithBridgeMode(mode string) TopologyOption { return func(h *TopologyHandler) { h.bridgeMode = mode @@ -223,11 +219,10 @@ func WithNATSConn(nc *nats.Conn) TopologyOption { } // useCentralizedNAT returns true if the bridge mode requires centralized NAT. -// Macvlan and veth modes both need centralized NAT — macvlan because of MAC -// filtering, veth because the Linux bridge intermediary breaks distributed NAT -// hairpin routing. Only direct bridge mode supports distributed NAT. +// Veth mode needs centralized NAT because the Linux bridge intermediary breaks +// distributed NAT hairpin routing. Only direct bridge mode supports distributed NAT. func (h *TopologyHandler) useCentralizedNAT() bool { - return h.bridgeMode != BridgeModeDirect + return h.bridgeMode == BridgeModeVeth } // ensureLocalnetOptions aligns options on an existing localnet port with the @@ -239,7 +234,7 @@ func (h *TopologyHandler) useCentralizedNAT() bool { // // network_name=external is required regardless of bridge mode — it binds the // port to the ovn-bridge-mappings entry for the external OVS bridge. -// Centralized mode (veth/macvlan): options:nat-addresses=router must be set. +// Centralized mode (veth): options:nat-addresses=router must be set. // Distributed mode (direct): options:nat-addresses must be absent. func (h *TopologyHandler) ensureLocalnetOptions(ctx context.Context, extPortName string) error { lsp, err := h.ovn.GetLogicalSwitchPort(ctx, extPortName) @@ -1053,9 +1048,9 @@ func (h *TopologyHandler) handleIGWAttach(msg *nats.Msg) { "network_name": "external", } // nat-addresses=router: OVN sends gratuitous ARPs for all NAT external IPs - // using the router port MAC. Required for centralized NAT modes (macvlan, - // veth) so that ARP replies for NAT IPs reach hosts correctly. Not needed - // for direct bridge since OVS sees all traffic on the wire without MAC + // using the router port MAC. Required for centralized NAT (veth mode) so + // that ARP replies for NAT IPs reach hosts correctly. Not needed for + // direct bridge since OVS sees all traffic on the wire without MAC // filtering and distributed NAT handles ARP per-chassis. if h.useCentralizedNAT() { localnetOpts["nat-addresses"] = "router" @@ -1083,7 +1078,7 @@ func (h *TopologyHandler) handleIGWAttach(msg *nats.Msg) { // dnat_and_snat row with external_mac/logical_port on every chassis, so // the LRP IP itself never goes on the wire. // - // Centralized mode (veth/macvlan): the LRP is the on-wire egress, so it + // Centralized mode (veth): the LRP is the on-wire egress, so it // must hold a WAN-subnet IP. Allocate one from pool.GwLrpRange (or // auto-derive from the WAN subnet) so each VPC gets a distinct sender // IP (mulga-siv-36). Without this the upstream router silently drops diff --git a/spinifex/services/vpcd/topology_test.go b/spinifex/services/vpcd/topology_test.go index ec4075c0c..8de856a45 100644 --- a/spinifex/services/vpcd/topology_test.go +++ b/spinifex/services/vpcd/topology_test.go @@ -1075,7 +1075,7 @@ func TestTopologyHandler_IGWAttach_WithExternalPool(t *testing.T) { PrefixLen: 24, }, } - topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools)) + topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools), WithBridgeMode(BridgeModeVeth)) subs, err := topo.Subscribe(nc) if err != nil { t.Fatalf("subscribe: %v", err) @@ -1169,7 +1169,7 @@ func TestTopologyHandler_IGWAttach_PoolWithGatewayIP(t *testing.T) { PrefixLen: 28, }, } - topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools)) + topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools), WithBridgeMode(BridgeModeVeth)) subs, _ := topo.Subscribe(nc) defer func() { for _, s := range subs { @@ -1232,7 +1232,7 @@ func TestTopologyHandler_IGWAttach_MultiVPC_NoIPCollision(t *testing.T) { PrefixLen: 24, }, } - topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools)) + topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools), WithBridgeMode(BridgeModeVeth)) subs, err := topo.Subscribe(nc) if err != nil { t.Fatalf("subscribe: %v", err) @@ -1319,7 +1319,7 @@ func TestTopologyHandler_IGWAttach_MultiVPC_NoIPCollision(t *testing.T) { } // TestTopologyHandler_IGWAttach_CentralizedNATAllocatesGwLrpIP guards -// mulga-siv-36: in centralized NAT (veth/macvlan) the gateway LRP must +// mulga-siv-36: in centralized NAT (veth) the gateway LRP must // hold a WAN-subnet IP from pool.GwLrpRange so it can ARP the upstream // nexthop. Three VPCs sharing one pool must each get a distinct IP and // persist it on external_ids:spinifex:gateway_ip. @@ -1340,8 +1340,6 @@ func TestTopologyHandler_IGWAttach_CentralizedNATAllocatesGwLrpIP(t *testing.T) GwLrpRangeEnd: "192.168.3.29", }, } - // Default bridge mode is centralized (useCentralizedNAT returns true - // when bridgeMode != "direct"). Explicit veth here to be unambiguous. topo := NewTopologyHandler(mock, WithExternalNetwork("pool", pools), WithBridgeMode(BridgeModeVeth), @@ -2651,52 +2649,6 @@ func TestTopologyHandler_IGWAttach_DirectBridge_NoNatAddresses(t *testing.T) { } } -func TestTopologyHandler_IGWAttach_MacvlanMode_HasNatAddresses(t *testing.T) { - // In macvlan mode (default), the localnet port SHOULD have nat-addresses=router - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock, WithBridgeMode(BridgeModeMacvlan)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create VPC router - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-mv1", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-mv1", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - // Attach IGW - evt := types.IGWEvent{InternetGatewayId: "igw-mv1", VpcId: "vpc-mv1"} - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.igw-attach: %v", err) - } - assertSuccess(t, resp, "attach IGW macvlan") - - // Verify localnet port HAS nat-addresses=router - port, err := mock.GetLogicalSwitchPort(ctx, "ext-port-vpc-mv1") - if err != nil { - t.Fatalf("expected localnet port: %v", err) - } - if port.Options["nat-addresses"] != "router" { - t.Errorf("macvlan mode should have nat-addresses=router, got %q", port.Options["nat-addresses"]) - } -} - func TestTopologyHandler_AddNAT_DirectBridge_DistributedNAT(t *testing.T) { // In direct bridge mode, DNAT rules should have ExternalMAC and LogicalPort set _, nc := startTestNATS(t) @@ -2756,65 +2708,6 @@ func TestTopologyHandler_AddNAT_DirectBridge_DistributedNAT(t *testing.T) { } } -func TestTopologyHandler_AddNAT_MacvlanMode_CentralizedNAT(t *testing.T) { - // In macvlan mode, DNAT rules should NOT have ExternalMAC/LogicalPort - _, nc := startTestNATS(t) - mock := NewMockOVNClient() - _ = mock.Connect(context.Background()) - ctx := context.Background() - - topo := NewTopologyHandler(mock, WithBridgeMode(BridgeModeMacvlan)) - subs, err := topo.Subscribe(nc) - if err != nil { - t.Fatalf("subscribe: %v", err) - } - defer func() { - for _, s := range subs { - _ = s.Unsubscribe() - } - }() - - // Pre-create VPC router - _ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{ - Name: "vpc-vpc-cnat1", - ExternalIDs: map[string]string{ - "spinifex:vpc_id": "vpc-cnat1", - "spinifex:cidr": "10.0.0.0/16", - }, - }) - - // Add NAT with MAC and port — macvlan mode should ignore them - evt := NATEvent{ - VpcId: "vpc-cnat1", - ExternalIP: "192.168.1.201", - LogicalIP: "10.0.1.5", - PortName: "port-eni-5678", - MAC: "02:00:00:11:22:33", - } - data, _ := json.Marshal(evt) - resp, err := nc.Request(TopicAddNAT, data, 5_000_000_000) - if err != nil { - t.Fatalf("request vpc.add-nat: %v", err) - } - assertSuccess(t, resp, "add NAT macvlan") - - // Verify the NAT rule does NOT have ExternalMAC/LogicalPort (centralized NAT) - router, err := mock.GetLogicalRouter(ctx, "vpc-vpc-cnat1") - if err != nil { - t.Fatalf("expected router: %v", err) - } - if len(router.NAT) != 1 { - t.Fatalf("expected 1 NAT rule, got %d", len(router.NAT)) - } - nat := mock.nats[router.NAT[0]] - if nat.ExternalMAC != nil { - t.Errorf("macvlan mode should NOT have ExternalMAC, got %v", *nat.ExternalMAC) - } - if nat.LogicalPort != nil { - t.Errorf("macvlan mode should NOT have LogicalPort, got %v", *nat.LogicalPort) - } -} - func TestTopologyHandler_IGWAttach_VethMode_HasNatAddresses(t *testing.T) { // In veth mode, the localnet port SHOULD have nat-addresses=router (centralized NAT) _, nc := startTestNATS(t) diff --git a/spinifex/services/vpcd/vpcd.go b/spinifex/services/vpcd/vpcd.go index f18b5dc90..5d1914d8b 100644 --- a/spinifex/services/vpcd/vpcd.go +++ b/spinifex/services/vpcd/vpcd.go @@ -64,18 +64,17 @@ type Config struct { ExternalPools []ExternalPoolConfig // 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 - // the macvlan MAC with the OVN gateway MAC for inbound traffic. + // ExternalInterface is the WAN NIC name (e.g., "enp0s3"). Used by + // detectBridgeMode for veth/direct auto-detection. ExternalInterface string // DhcpBindBridge is the bridge where the DHCP client binds its AF_PACKET // socket. In veth mode this is the Linux bridge that holds the WAN NIC // (e.g. "br-wan"); in direct mode this is the OVS bridge holding the // WAN NIC. Never the OVN-side "br-ext" — that never sees LAN DHCP. DhcpBindBridge string - // BridgeMode is "direct", "macvlan", or "veth". Direct bridge adds the WAN - // NIC directly to the OVS bridge; macvlan creates a sub-interface; veth uses - // a veth pair to link a Linux bridge to OVS. When empty, auto-detected at - // startup. + // BridgeMode is "direct" or "veth". Direct bridge adds the WAN NIC + // directly to the OVS bridge; veth uses a veth pair to link a Linux bridge + // to OVS. When empty, auto-detected at startup. BridgeMode string } @@ -364,21 +363,6 @@ func launchService(cfg *Config) error { holder, _ := os.Hostname() releaseLeader, isLeader := AcquireReconcileLeader(nc, holder) - // Macvlan mode only: align macvlan MAC with the OVN gateway router MAC. - // The macvlan only delivers inbound unicast matching its own MAC. With - // centralized NAT, OVN uses the router MAC for all external ARP replies. - // Setting the macvlan MAC to the router MAC ensures inbound traffic reaches OVS. - // Direct bridge mode doesn't need this — OVS sees all traffic on the wire. - if bridgeMode == BridgeModeMacvlan && cfg.Bootstrap != nil && cfg.Bootstrap.VpcId != "" && cfg.ExternalInterface != "" { - gwMAC := generateMAC("gw-" + cfg.Bootstrap.VpcId) - macvlanName := "spx-ext-" + cfg.ExternalInterface - if err := setMacvlanMAC(macvlanName, gwMAC); err != nil { - slog.Warn("vpcd: failed to set macvlan MAC (inbound traffic may not work)", "iface", macvlanName, "mac", gwMAC, "err", err) - } else { - slog.Info("vpcd: macvlan MAC aligned with gateway", "iface", macvlanName, "mac", gwMAC) - } - } - subs, err := topo.Subscribe(nc) if err != nil { slog.Error("Failed to subscribe to VPC topics", "err", err) @@ -554,21 +538,12 @@ func resolveBridgeConfig(cfgBridgeMode, externalIface, cfgDhcpBindBridge string) return bridgeMode, dhcpBindBridge } -// ifaceIsMacvlan returns true when the given interface exists and is a -// macvlan sub-interface. Injected as a var so detectBridgeMode tests can stub -// it without launching ip(8). -var ifaceIsMacvlan = func(name string) bool { - out, err := exec.Command("ip", "-d", "link", "show", name).CombinedOutput() - return err == nil && strings.Contains(string(out), "macvlan") -} - // ifaceExists returns true when the kernel reports the named link. var ifaceExists = func(name string) bool { return exec.Command("ip", "link", "show", name).Run() == nil } // detectBridgeMode checks how the WAN bridge is wired: -// - macvlan: spx-ext-{iface} macvlan sub-interface exists // - veth: veth-wan-ovs interface exists (Linux bridge linked to OVS via veth pair) // - direct: physical NIC is added directly to the OVS bridge // @@ -577,17 +552,12 @@ var ifaceExists = func(name string) bool { // silent Debug fall-through is what let the veth-persistence bug hide for // weeks (mulga-998.b Fix 2). func detectBridgeMode(externalIface string) string { - macvlanName := "spx-ext-" + externalIface - if ifaceIsMacvlan(macvlanName) { - slog.Info("vpcd: detected macvlan interface on WAN bridge", "iface", macvlanName, "mode", BridgeModeMacvlan) - return BridgeModeMacvlan - } if ifaceExists("veth-wan-ovs") { slog.Info("vpcd: detected veth pair linking Linux bridge to OVS", "mode", BridgeModeVeth) return BridgeModeVeth } - slog.Warn("vpcd: no macvlan or veth interface found, assuming direct bridge mode", - "external_iface", externalIface, "checked_macvlan", macvlanName, "checked_veth", "veth-wan-ovs", + slog.Warn("vpcd: no veth interface found, assuming direct bridge mode", + "external_iface", externalIface, "checked_veth", "veth-wan-ovs", "mode", BridgeModeDirect) return BridgeModeDirect } @@ -678,19 +648,3 @@ func verifyBridgeMode(mode, externalIface, dhcpBindBridge string) error { mode, BridgeModeDirect, BridgeModeVeth) } } - -// setMacvlanMAC sets the MAC address on a macvlan interface. The interface is -// brought down, MAC changed, and brought back up. Requires sudo/NET_ADMIN. -func setMacvlanMAC(iface, mac string) error { - cmds := [][]string{ - {"sudo", "ip", "link", "set", iface, "down"}, - {"sudo", "ip", "link", "set", iface, "address", mac}, - {"sudo", "ip", "link", "set", iface, "up"}, - } - for _, args := range cmds { - if out, err := exec.Command(args[0], args[1:]...).CombinedOutput(); err != nil { - return fmt.Errorf("%s: %w (%s)", args[3], err, string(out)) - } - } - return nil -} diff --git a/spinifex/services/vpcd/vpcd_test.go b/spinifex/services/vpcd/vpcd_test.go index 51fe7b329..194760f59 100644 --- a/spinifex/services/vpcd/vpcd_test.go +++ b/spinifex/services/vpcd/vpcd_test.go @@ -230,7 +230,7 @@ func TestVerifyBridgeMode_VethWrongLinuxMaster(t *testing.T) { } func TestVerifyBridgeMode_UnknownModeLists(t *testing.T) { - err := verifyBridgeMode("macvlan", "enp0s3", "br-wan") + err := verifyBridgeMode("bogus", "enp0s3", "br-wan") if err == nil { t.Fatal("expected error for unsupported mode") } @@ -279,49 +279,35 @@ func TestVerifyBridgeMode_VethLinuxBrMissing(t *testing.T) { // detectBridgeMode — mulga-998.b Fix 2. -func stubDetectProbes(t *testing.T, macvlans []string, links []string) { +func stubDetectProbes(t *testing.T, links []string) { t.Helper() - origMac := ifaceIsMacvlan origExists := ifaceExists t.Cleanup(func() { - ifaceIsMacvlan = origMac ifaceExists = origExists }) - macSet := map[string]bool{} - for _, m := range macvlans { - macSet[m] = true - } linkSet := map[string]bool{} for _, l := range links { linkSet[l] = true } - ifaceIsMacvlan = func(name string) bool { return macSet[name] } ifaceExists = func(name string) bool { return linkSet[name] } } -func TestDetectBridgeMode_MacvlanWins(t *testing.T) { - stubDetectProbes(t, []string{"spx-ext-enp0s3"}, []string{"veth-wan-ovs"}) - if got := detectBridgeMode("enp0s3"); got != BridgeModeMacvlan { - t.Errorf("want %q, got %q", BridgeModeMacvlan, got) - } -} - -func TestDetectBridgeMode_VethWhenNoMacvlan(t *testing.T) { - stubDetectProbes(t, nil, []string{"veth-wan-ovs"}) +func TestDetectBridgeMode_VethWins(t *testing.T) { + stubDetectProbes(t, []string{"veth-wan-ovs"}) if got := detectBridgeMode("enp0s3"); got != BridgeModeVeth { t.Errorf("want %q, got %q", BridgeModeVeth, got) } } func TestDetectBridgeMode_FallthroughDirect(t *testing.T) { - stubDetectProbes(t, nil, nil) + stubDetectProbes(t, nil) if got := detectBridgeMode("enp0s3"); got != BridgeModeDirect { t.Errorf("want %q, got %q", BridgeModeDirect, got) } } func TestResolveBridgeConfig_UsesExplicitMode(t *testing.T) { - stubDetectProbes(t, nil, nil) + stubDetectProbes(t, nil) mode, br := resolveBridgeConfig(BridgeModeVeth, "enp0s3", "br-wan") if mode != BridgeModeVeth || br != "br-wan" { t.Errorf("got (%q,%q), want (%q,br-wan)", mode, br, BridgeModeVeth) @@ -329,7 +315,7 @@ func TestResolveBridgeConfig_UsesExplicitMode(t *testing.T) { } func TestResolveBridgeConfig_AutoDetects(t *testing.T) { - stubDetectProbes(t, nil, []string{"veth-wan-ovs"}) + stubDetectProbes(t, []string{"veth-wan-ovs"}) mode, _ := resolveBridgeConfig("", "enp0s3", "br-wan") if mode != BridgeModeVeth { t.Errorf("want auto-detect veth, got %q", mode) diff --git a/tests/e2e/TEST_COVERAGE.md b/tests/e2e/TEST_COVERAGE.md index 7be72808d..54e2dc3e9 100644 --- a/tests/e2e/TEST_COVERAGE.md +++ b/tests/e2e/TEST_COVERAGE.md @@ -301,7 +301,7 @@ Gated on `external_mode = "pool"` (skipped when only `dev_networking` is configu - `run-instances` into default subnet → auto-assigned `PublicIpAddress` - `ovn-nbctl lr-nat-list` shows per-instance `dnat_and_snat` rule for `PUB_PRIVATE_IP` - Asserts NO blanket VPC CIDR SNAT (mulga-754 AWS-parity regression) - - SSH via public IP succeeds (when bridge/macvlan reachable) + - SSH via public IP succeeds (when bridge reachable) - Outbound internet from inside the VM (via OVN SNAT) - Step 2b: In-guest TLS trust — cloud-init injects Spinifex CA into the guest, so `curl https://awsgw` and `https://predastore-s3` work without `--insecure` - Step 3: Private subnet isolation diff --git a/tests/e2e/run-e2e.sh b/tests/e2e/run-e2e.sh index 0c65000e4..817bc3ff7 100755 --- a/tests/e2e/run-e2e.sh +++ b/tests/e2e/run-e2e.sh @@ -2931,7 +2931,7 @@ if [ "$PUB_SSH_READY" = true ]; then exit 1 fi else - echo "WARN: SSH via public IP $PUB_IP not reachable (macvlan isolation or bridge not ready)" + echo "WARN: SSH via public IP $PUB_IP not reachable (bridge not ready)" echo "Falling back to API-only verification (OVN state already validated above)" fi diff --git a/tests/e2e/run-reboot-e2e.sh b/tests/e2e/run-reboot-e2e.sh index 3b55ada88..dc4362947 100755 --- a/tests/e2e/run-reboot-e2e.sh +++ b/tests/e2e/run-reboot-e2e.sh @@ -544,7 +544,7 @@ if [ "$IPS_PRESERVED" = true ]; then fi # 8.3: guest VMs relaunched post-reboot (LaunchTime > reboot time via describe-instances) -# Two-hop SSH to VPC private IPs is not possible in single-node macvlan mode — +# Two-hop SSH to VPC private IPs is not possible in single-node mode — # the host has no route to VPC subnets. LaunchTime from the daemon's recovery # path (reset to time.Now() on relaunch) is a reliable reboot signal. log "Verifying guest VMs relaunched post-reboot (LaunchTime check)..." diff --git a/tests/e2e/single/lifecycle_test.go b/tests/e2e/single/lifecycle_test.go index 8a1bd8f5f..bd4ef1e26 100644 --- a/tests/e2e/single/lifecycle_test.go +++ b/tests/e2e/single/lifecycle_test.go @@ -418,8 +418,8 @@ func waitForSSHHandshake(t *testing.T, host string, port int, keyPath string) { // trySSHReady mirrors waitForSSHReady but returns reachability as a bool // instead of t.Fatal-ing on timeout. Matches run-e2e.sh ~2850–2860, which -// treats SSH-via-public-IP timeout as a WARN ("macvlan isolation or bridge -// not ready") rather than a hard test failure — new-subnet IGW SNAT/DNAT +// treats SSH-via-public-IP timeout as a WARN ("bridge not ready") rather +// than a hard test failure — new-subnet IGW SNAT/DNAT // flow installation is timing-sensitive on shared CI runners and not part // of the EC2 surface contract this test exists to validate. func trySSHReady(host string, port int, keyPath string, timeout time.Duration) bool {