Merge branch 'fix/iso-public-ip-boot-race' of github.com:mulgadc/spinifex into fix/iso-public-ip-boot-race

This commit is contained in:
tomnewton-mulga
2026-04-30 16:42:48 +10:00
9 changed files with 452 additions and 144 deletions
+11 -9
View File
@@ -734,15 +734,17 @@ var vpcdStartCmd = &cobra.Command{
var extPools []vpcd.ExternalPoolConfig
for _, p := range clusterConfig.Network.ExternalPools {
extPools = append(extPools, vpcd.ExternalPoolConfig{
Name: p.Name,
RangeStart: p.RangeStart,
RangeEnd: p.RangeEnd,
Gateway: p.Gateway,
GatewayIP: p.GatewayIP,
PrefixLen: p.PrefixLen,
DNSServers: p.DNSServers,
Region: p.Region,
AZ: p.AZ,
Name: p.Name,
RangeStart: p.RangeStart,
RangeEnd: p.RangeEnd,
Gateway: p.Gateway,
GatewayIP: p.GatewayIP,
PrefixLen: p.PrefixLen,
DNSServers: p.DNSServers,
Region: p.Region,
AZ: p.AZ,
GwLrpRangeStart: p.GwLrpRangeStart,
GwLrpRangeEnd: p.GwLrpRangeEnd,
})
}
+20
View File
@@ -290,6 +290,26 @@ sudo ovs-vsctl set Bridge br-int other-config:disable-in-band=true
sudo ip link set br-int up
echo " br-int: created, fail-mode=secure, up"
# Mark OVS internal netdevs Unmanaged for systemd-networkd. Without this,
# Trixie's networkd takes ownership of any unconfigured iface and may bring
# br-int/br-ext admin-down after setup-ovn.sh's `ip link set up` (no Match
# rule => default management => no carrier => link down). OVS dataplane
# still forwards, but ovn-controller flow programming and any tooling that
# probes link state misbehave. Unmanaged=yes keeps OVS in sole control.
# (mulga-siv-37)
OVS_INTERNAL_NET=/etc/systemd/network/05-spinifex-ovs-internal.network
if [ ! -f "$OVS_INTERNAL_NET" ]; then
sudo tee "$OVS_INTERNAL_NET" >/dev/null <<'NETWORK'
[Match]
Name=br-int br-ext
[Link]
Unmanaged=yes
NETWORK
sudo networkctl reload 2>/dev/null || true
echo " wrote $OVS_INTERNAL_NET (Unmanaged=yes for br-int br-ext)"
fi
# --- Step 3b: Configure WAN bridge for public subnet uplink ---
if [ -n "$WAN_BRIDGE" ]; then
echo ""
+8
View File
@@ -31,6 +31,14 @@ type ExternalPool struct {
DNSServers []string `mapstructure:"dns_servers"` // DNS servers for VM DHCP (auto-detected from host; fallback: 8.8.8.8, 1.1.1.1)
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
// 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
// and vpcd will fight over the same IP.
GwLrpRangeStart string `mapstructure:"gw_lrp_range_start"`
GwLrpRangeEnd string `mapstructure:"gw_lrp_range_end"`
}
// NetworkConfig holds cluster-wide external network settings.
+12 -11
View File
@@ -700,17 +700,18 @@ func (d *Daemon) Start() error {
}
for _, p := range d.clusterConfig.Network.ExternalPools {
pools = append(pools, handlers_ec2_vpc.ExternalPoolConfig{
Name: p.Name,
Source: p.Source,
RangeStart: p.RangeStart,
RangeEnd: p.RangeEnd,
Gateway: p.Gateway,
GatewayIP: p.GatewayIP,
PrefixLen: p.PrefixLen,
Region: p.Region,
AZ: p.AZ,
DhcpBindBridge: dhcpBindBridge,
GatewayMAC: gwMAC,
Name: p.Name,
Source: p.Source,
RangeStart: p.RangeStart,
RangeEnd: p.RangeEnd,
Gateway: p.Gateway,
GatewayIP: p.GatewayIP,
PrefixLen: p.PrefixLen,
Region: p.Region,
AZ: p.AZ,
DhcpBindBridge: dhcpBindBridge,
GwLrpRangeStart: p.GwLrpRangeStart,
GwLrpRangeEnd: p.GwLrpRangeEnd,
})
}
d.externalIPAM, err = handlers_ec2_vpc.NewExternalIPAM(d.natsConn, js, pools)
+79 -28
View File
@@ -38,16 +38,21 @@ type ExternalIPAllocation struct {
// ExternalIPAMRecord tracks allocated external IPs for a single pool.
type ExternalIPAMRecord struct {
PoolName string `json:"pool_name"`
Source string `json:"source"` // "static" (default) or "dhcp"
RangeStart string `json:"range_start"`
RangeEnd string `json:"range_end"`
Gateway string `json:"gateway"`
GatewayIP string `json:"gateway_ip"`
PrefixLen int `json:"prefix_len"`
Region string `json:"region,omitempty"`
AZ string `json:"az,omitempty"`
Allocated map[string]ExternalIPAllocation `json:"allocated"`
PoolName string `json:"pool_name"`
Source string `json:"source"` // "static" (default) or "dhcp"
RangeStart string `json:"range_start"`
RangeEnd string `json:"range_end"`
Gateway string `json:"gateway"`
GatewayIP string `json:"gateway_ip"`
PrefixLen int `json:"prefix_len"`
Region string `json:"region,omitempty"`
AZ string `json:"az,omitempty"`
// GwLrpRangeStart/End mirrors ExternalPoolConfig — IPAM skips this
// sub-range so it doesn't collide with vpcd's gateway LRP IPs in
// centralized NAT (mulga-siv-36).
GwLrpRangeStart string `json:"gw_lrp_range_start,omitempty"`
GwLrpRangeEnd string `json:"gw_lrp_range_end,omitempty"`
Allocated map[string]ExternalIPAllocation `json:"allocated"`
}
// ExternalPoolConfig is the admin-defined pool from spinifex.toml.
@@ -62,13 +67,11 @@ type ExternalPoolConfig struct {
Region string
AZ string
DhcpBindBridge string // Bridge where the DHCP AF_PACKET socket binds (e.g. "br-wan"). Linux bridge in veth mode; OVS bridge in direct mode. Never "br-ext".
// GatewayMAC, when set, is used as the chaddr in per-VM DHCP DORAs so
// that the upstream DHCP binding matches the MAC that OVN GARPs use (the
// gateway LRP MAC). Without this, the DHCP server binds the public IP to
// the generated ENI-derived MAC while OVN announces a different MAC via
// GARP, causing home routers that use their DHCP lease table as ARP to
// forward traffic to a non-existent interface.
GatewayMAC string
// GwLrpRangeStart/End reserves a sub-range of the LAN for OVN gateway
// LRP IPs in centralized NAT mode (mulga-siv-36). IPAM must skip these
// addresses or the per-VM EIP allocator and vpcd will fight over them.
GwLrpRangeStart string
GwLrpRangeEnd string
}
// IsDHCP returns true if this pool obtains IPs from router DHCP.
@@ -129,15 +132,20 @@ func (m *ExternalIPAM) initPool(pool ExternalPoolConfig) error {
}
if err == nil {
if chk.RangeStart != pool.RangeStart || chk.RangeEnd != pool.RangeEnd || chk.Source != pool.Source {
if chk.RangeStart != pool.RangeStart || chk.RangeEnd != pool.RangeEnd || chk.Source != pool.Source ||
chk.GwLrpRangeStart != pool.GwLrpRangeStart || chk.GwLrpRangeEnd != pool.GwLrpRangeEnd {
slog.Info("external IPAM pool config drift, reconciling KV",
"pool", pool.Name,
"old_range", chk.RangeStart+"-"+chk.RangeEnd, "new_range", pool.RangeStart+"-"+pool.RangeEnd,
"old_source", chk.Source, "new_source", pool.Source)
"old_source", chk.Source, "new_source", pool.Source,
"old_gw_lrp_range", chk.GwLrpRangeStart+"-"+chk.GwLrpRangeEnd,
"new_gw_lrp_range", pool.GwLrpRangeStart+"-"+pool.GwLrpRangeEnd)
chk.RangeStart = pool.RangeStart
chk.RangeEnd = pool.RangeEnd
chk.Source = pool.Source
chk.GwLrpRangeStart = pool.GwLrpRangeStart
chk.GwLrpRangeEnd = pool.GwLrpRangeEnd
data, err := json.Marshal(chk)
if err != nil {
@@ -190,14 +198,16 @@ func (m *ExternalIPAM) initPool(pool ExternalPoolConfig) error {
}
record := &ExternalIPAMRecord{
PoolName: pool.Name,
RangeStart: pool.RangeStart,
RangeEnd: pool.RangeEnd,
Gateway: pool.Gateway,
GatewayIP: gwIP,
PrefixLen: pool.PrefixLen,
Region: pool.Region,
AZ: pool.AZ,
PoolName: pool.Name,
RangeStart: pool.RangeStart,
RangeEnd: pool.RangeEnd,
Gateway: pool.Gateway,
GatewayIP: gwIP,
PrefixLen: pool.PrefixLen,
Region: pool.Region,
AZ: pool.AZ,
GwLrpRangeStart: pool.GwLrpRangeStart,
GwLrpRangeEnd: pool.GwLrpRangeEnd,
Allocated: map[string]ExternalIPAllocation{
gwIP: gatewayAlloc,
},
@@ -461,7 +471,9 @@ func (m *ExternalIPAM) getRecord(poolName string) (*ExternalIPAMRecord, uint64,
return &record, entry.Revision(), nil
}
// nextAvailableExternalIP finds the next unallocated IP in the pool's range.
// nextAvailableExternalIP finds the next unallocated IP in the pool's
// range. Addresses inside [GwLrpRangeStart, GwLrpRangeEnd] are skipped —
// vpcd reserves them for OVN gateway LRPs (mulga-siv-36).
func nextAvailableExternalIP(record *ExternalIPAMRecord) (string, error) {
startIP := net.ParseIP(record.RangeStart).To4()
endIP := net.ParseIP(record.RangeEnd).To4()
@@ -469,10 +481,23 @@ func nextAvailableExternalIP(record *ExternalIPAMRecord) (string, error) {
return "", fmt.Errorf("invalid IP range: %s - %s", record.RangeStart, record.RangeEnd)
}
var gwLrpStart, gwLrpEnd int64 = -1, -1
if record.GwLrpRangeStart != "" && record.GwLrpRangeEnd != "" {
s := net.ParseIP(record.GwLrpRangeStart).To4()
e := net.ParseIP(record.GwLrpRangeEnd).To4()
if s != nil && e != nil {
gwLrpStart = ipToInt(s).Int64()
gwLrpEnd = ipToInt(e).Int64()
}
}
startInt := ipToInt(startIP)
endInt := ipToInt(endIP)
for i := startInt.Int64(); i <= endInt.Int64(); i++ {
if gwLrpStart >= 0 && i >= gwLrpStart && i <= gwLrpEnd {
continue
}
candidate := intToIP(intFromInt64(i)).String()
if _, taken := record.Allocated[candidate]; !taken {
return candidate, nil
@@ -509,6 +534,32 @@ func ValidatePoolConfig(pool ExternalPoolConfig) error {
if ipToInt(startIP.To4()).Cmp(ipToInt(endIP.To4())) > 0 {
return fmt.Errorf("range_start %s is greater than range_end %s", pool.RangeStart, pool.RangeEnd)
}
// gw_lrp_range must be valid IPs and must NOT overlap range_start/end
// — otherwise vpcd's gateway LRP allocator and per-VM EIP allocator
// would fight over the same address (mulga-siv-36).
if pool.GwLrpRangeStart != "" || pool.GwLrpRangeEnd != "" {
gwS := net.ParseIP(pool.GwLrpRangeStart)
gwE := net.ParseIP(pool.GwLrpRangeEnd)
if gwS == nil {
return fmt.Errorf("invalid gw_lrp_range_start: %q", pool.GwLrpRangeStart)
}
if gwE == nil {
return fmt.Errorf("invalid gw_lrp_range_end: %q", pool.GwLrpRangeEnd)
}
gwSi := ipToInt(gwS.To4())
gwEi := ipToInt(gwE.To4())
if gwSi.Cmp(gwEi) > 0 {
return fmt.Errorf("gw_lrp_range_start %s is greater than gw_lrp_range_end %s",
pool.GwLrpRangeStart, pool.GwLrpRangeEnd)
}
rangeSi := ipToInt(startIP.To4())
rangeEi := ipToInt(endIP.To4())
// Overlap test: !(gwE < rangeS || gwS > rangeE)
if gwEi.Cmp(rangeSi) >= 0 && gwSi.Cmp(rangeEi) <= 0 {
return fmt.Errorf("gw_lrp_range %s-%s overlaps range %s-%s",
pool.GwLrpRangeStart, pool.GwLrpRangeEnd, pool.RangeStart, pool.RangeEnd)
}
}
}
return nil
}
+3 -52
View File
@@ -3,46 +3,12 @@ package dhcp
import (
"context"
"fmt"
"net"
"time"
"github.com/insomniacslk/dhcp/dhcpv4"
"github.com/insomniacslk/dhcp/dhcpv4/nclient4"
"github.com/mdlayher/packet"
"golang.org/x/sys/unix"
)
// dhcpClientPort is the IANA-assigned UDP port for DHCP clients.
const dhcpClientPort = 68
// openPromiscBridgeConn opens an AF_PACKET DGRAM socket on iface filtered
// to IPv4 and enables PACKET_MR_PROMISC on it. Equivalent to nclient4's
// own NewRawUDPConn but with promisc on.
//
// Why promisc: vpcd uses a synthetic per-tenant chaddr (e.g. 02:f3:...)
// that never matches the bridge's MAC. Many real-world DHCP servers
// ignore the BOOTP BROADCAST flag and reply with a unicast OFFER addressed
// to that chaddr. A Linux bridge floods unknown unicast to slave ports
// but does NOT pass it up to the bridge netdev unless IFF_PROMISC is set,
// so the AF_PACKET socket misses the OFFER and DORA times out. Promisc
// flips IFF_PROMISC for the lifetime of the socket (per-DORA), and the
// kernel refcounts it so it auto-clears on Close.
func openPromiscBridgeConn(iface string) (net.PacketConn, net.HardwareAddr, error) {
ifc, err := net.InterfaceByName(iface)
if err != nil {
return nil, nil, fmt.Errorf("lookup iface %s: %w", iface, err)
}
raw, err := packet.Listen(ifc, packet.Datagram, unix.ETH_P_IP, nil)
if err != nil {
return nil, nil, fmt.Errorf("packet.Listen on %s: %w", iface, err)
}
if err := raw.SetPromiscuous(true); err != nil {
_ = raw.Close()
return nil, nil, fmt.Errorf("set promisc on %s: %w", iface, err)
}
return nclient4.NewBroadcastUDPConn(raw, &net.UDPAddr{Port: dhcpClientPort}), ifc.HardwareAddr, nil
}
// NClient4Client is the production DHCP client backed by
// github.com/insomniacslk/dhcp/dhcpv4/nclient4. Each Acquire/Renew/Release
// opens an AF_PACKET socket on the target bridge for the duration of the
@@ -73,17 +39,12 @@ func (c *NClient4Client) Acquire(ctx context.Context, req AcquireRequest) (*Leas
return nil, fmt.Errorf("dhcp acquire: hw_addr is required")
}
conn, ifaceMAC, err := openPromiscBridgeConn(req.Bridge)
if err != nil {
return nil, fmt.Errorf("open promisc conn on %s: %w", req.Bridge, err)
}
client, err := nclient4.NewWithConn(conn, ifaceMAC,
client, err := nclient4.New(req.Bridge,
nclient4.WithHWAddr(req.HWAddr),
nclient4.WithTimeout(c.timeout),
nclient4.WithRetry(c.retry),
)
if err != nil {
_ = conn.Close()
return nil, fmt.Errorf("open nclient4 on %s: %w", req.Bridge, err)
}
defer func() { _ = client.Close() }()
@@ -110,17 +71,12 @@ func (c *NClient4Client) Renew(ctx context.Context, lease *Lease) (*Lease, error
return nil, fmt.Errorf("dhcp renew: %w", err)
}
conn, ifaceMAC, err := openPromiscBridgeConn(lease.Bridge)
if err != nil {
return nil, fmt.Errorf("open promisc conn on %s for renew: %w", lease.Bridge, err)
}
client, err := nclient4.NewWithConn(conn, ifaceMAC,
client, err := nclient4.New(lease.Bridge,
nclient4.WithHWAddr(lease.HWAddr),
nclient4.WithTimeout(c.timeout),
nclient4.WithRetry(c.retry),
)
if err != nil {
_ = conn.Close()
return nil, fmt.Errorf("open nclient4 on %s for renew: %w", lease.Bridge, err)
}
defer func() { _ = client.Close() }()
@@ -149,16 +105,11 @@ func (c *NClient4Client) Release(ctx context.Context, lease *Lease) error {
return fmt.Errorf("dhcp release: %w", err)
}
conn, ifaceMAC, err := openPromiscBridgeConn(lease.Bridge)
if err != nil {
return fmt.Errorf("open promisc conn on %s for release: %w", lease.Bridge, err)
}
client, err := nclient4.NewWithConn(conn, ifaceMAC,
client, err := nclient4.New(lease.Bridge,
nclient4.WithHWAddr(lease.HWAddr),
nclient4.WithTimeout(c.timeout),
)
if err != nil {
_ = conn.Close()
return fmt.Errorf("open nclient4 on %s for release: %w", lease.Bridge, err)
}
defer func() { _ = client.Close() }()
+217 -35
View File
@@ -32,13 +32,23 @@ const (
TopicDeleteNATGateway = "vpc.delete-nat-gateway"
)
// gatewayPortNetwork is the link-local CIDR every IGW gateway-LRP carries.
// Pool IPs are reserved for per-VM dnat_and_snat external addresses; reusing
// them on the LRP causes intra-cluster ARP collisions when multiple VPCs
// share a br-ext localnet (mulga-siv-26). The default route's OutputPort is
// set explicitly so the WAN nexthop need not be on this subnet.
// gatewayPortNetwork is the link-local CIDR every IGW gateway-LRP carries
// in distributed-NAT (direct) mode. Per-VM dnat_and_snat handles ARP per
// 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
// 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
// (mulga-siv-36).
const gatewayPortNetwork = "169.254.0.1/30"
// gatewayIPExtID is the LRP external_ids key holding the gw IP allocated
// from the pool's gw_lrp_range. Persisted so reconcile/retrofit can recover
// the assignment without re-allocating, and so siblings see it as "used".
const gatewayIPExtID = "spinifex:gateway_ip"
// VPCEvent is published on vpc.create after a VPC is persisted.
type VPCEvent struct {
VpcId string `json:"vpc_id"`
@@ -202,22 +212,73 @@ func (h *TopologyHandler) ensureLocalnetOptions(ctx context.Context, extPortName
return nil
}
// expectedGatewayPortNetwork returns the Networks CIDR a gateway LRP for
// vpcId should carry. In direct mode, or in centralized mode without a
// configured gw_lrp_range, it is the link-local fallback. In centralized
// mode with a range, it is the existing spinifex:gateway_ip on the LRP if
// any (idempotent), otherwise a fresh allocation. Returns the new gw IP
// (empty when link-local) so callers can persist it to external_ids.
func (h *TopologyHandler) expectedGatewayPortNetwork(ctx context.Context, vpcId string) (network, gwIP string, err error) {
if !h.useCentralizedNAT() {
return gatewayPortNetwork, "", nil
}
pool := h.findExternalPool("", "")
if pool == nil {
return gatewayPortNetwork, "", nil
}
ip, prefix, ok, allocErr := h.allocateGatewayLRPIP(ctx, vpcId, pool)
if allocErr != nil {
return "", "", allocErr
}
if !ok {
return gatewayPortNetwork, "", nil
}
return fmt.Sprintf("%s/%d", ip, prefix), ip, nil
}
// ensureGatewayPortNetworks rewrites the gateway LRP's Networks column in
// place when it drifts from the link-local CIDR. CreateLogicalRouterPort is
// a no-op when the row exists, so reconcile of an upgraded cluster otherwise
// keeps the old pool-IP networks (mulga-siv-26 D8). Idempotent — no UPDATE
// is issued when already correct.
// place when it drifts from the mode's expected value. CreateLogicalRouterPort
// is a no-op when the row exists, so reconcile of an upgraded cluster
// otherwise keeps the stale pool-IP networks (mulga-siv-26 D8) or, in
// centralized NAT, the off-subnet link-local that breaks ARP upstream
// (mulga-siv-36). Idempotent — no UPDATE is issued when already correct.
func (h *TopologyHandler) ensureGatewayPortNetworks(ctx context.Context, gwPortName string) error {
lrp, err := h.ovn.GetLogicalRouterPort(ctx, gwPortName)
if err != nil {
return fmt.Errorf("get gateway router port %s: %w", gwPortName, err)
}
if len(lrp.Networks) == 1 && lrp.Networks[0] == gatewayPortNetwork {
vpcId := lrp.ExternalIDs["spinifex:vpc_id"]
if vpcId == "" {
// No VPC tag — this is from the legacy "gw-vpc-..." naming or a
// hand-edited row; fall back to link-local to keep behavior safe.
if len(lrp.Networks) == 1 && lrp.Networks[0] == gatewayPortNetwork {
return nil
}
slog.Info("vpcd: rewriting stale gateway port networks (no vpc_id tag)",
"port", gwPortName, "old", lrp.Networks, "new", gatewayPortNetwork)
lrp.Networks = []string{gatewayPortNetwork}
return h.ovn.UpdateLogicalRouterPort(ctx, lrp)
}
wantNetwork, wantGwIP, err := h.expectedGatewayPortNetwork(ctx, vpcId)
if err != nil {
return fmt.Errorf("compute expected network for %s: %w", gwPortName, err)
}
currentGwIP := lrp.ExternalIDs[gatewayIPExtID]
if len(lrp.Networks) == 1 && lrp.Networks[0] == wantNetwork && currentGwIP == wantGwIP {
return nil
}
slog.Info("vpcd: rewriting stale gateway port networks",
"port", gwPortName, "old", lrp.Networks, "new", gatewayPortNetwork)
lrp.Networks = []string{gatewayPortNetwork}
"port", gwPortName, "old", lrp.Networks, "new", wantNetwork,
"old_gw_ip", currentGwIP, "new_gw_ip", wantGwIP)
lrp.Networks = []string{wantNetwork}
if lrp.ExternalIDs == nil {
lrp.ExternalIDs = map[string]string{}
}
if wantGwIP == "" {
delete(lrp.ExternalIDs, gatewayIPExtID)
} else {
lrp.ExternalIDs[gatewayIPExtID] = wantGwIP
}
if err := h.ovn.UpdateLogicalRouterPort(ctx, lrp); err != nil {
return fmt.Errorf("update gateway router port %s: %w", gwPortName, err)
}
@@ -225,10 +286,10 @@ func (h *TopologyHandler) ensureGatewayPortNetworks(ctx context.Context, gwPortN
}
// RetrofitAllGatewayPortNetworks walks every LRP tagged
// spinifex:role=gateway and ensures Networks is the link-local CIDR. Needed
// spinifex:role=gateway and ensures Networks matches the current mode. Needed
// because Reconcile/ReconcileFromKV early-return when the external switch
// already exists, so reconcileIGW never runs against pre-existing topologies
// shipped by the buggy pool-IP build (mulga-siv-26 D8). Idempotent — the
// shipped by older builds (mulga-siv-26 D8 / mulga-siv-36). Idempotent — the
// underlying ensureGatewayPortNetworks no-ops when already correct.
func (h *TopologyHandler) RetrofitAllGatewayPortNetworks(ctx context.Context) {
lrps, err := h.ovn.ListLogicalRouterPorts(ctx)
@@ -804,17 +865,42 @@ func (h *TopologyHandler) handleIGWAttach(msg *nats.Msg) {
return
}
// Resolve external pool for this VPC's WAN nexthop. The router port
// itself always takes link-local 169.254.0.1/30 — pool IPs are reserved
// for per-VM dnat_and_snat external addresses (mulga-siv-26). Sharing a
// pool IP across VPCs causes intra-cluster ARP collisions on br-ext.
// Resolve external pool for this VPC's WAN nexthop and gw LRP IP.
//
// Direct mode: link-local LRP is fine — distributed NAT puts a per-VM
// 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
// must hold a WAN-subnet IP. Allocate one from pool.GwLrpRange so each
// VPC gets a distinct sender IP (mulga-siv-36). Without this the
// upstream router silently drops ARP for the WAN nexthop (RFC 826) and
// the default route never resolves.
// TODO: use VPC's region/AZ once we track it; for now use first matching pool.
pool := h.findExternalPool("", "")
gatewayNetwork := gatewayPortNetwork
wanGateway := "169.254.0.2"
gwLrpIP := ""
if pool != nil {
wanGateway = pool.Gateway
if h.useCentralizedNAT() {
ip, prefix, allocOK, allocErr := h.allocateGatewayLRPIP(ctx, evt.VpcId, pool)
if allocErr != nil {
slog.Error("vpcd: failed to allocate gw LRP IP",
"vpc_id", evt.VpcId, "pool", pool.Name, "err", allocErr)
_ = h.ovn.DeleteLogicalSwitch(ctx, extSwitchName)
respond(msg, allocErr)
return
}
if allocOK {
gwLrpIP = ip
gatewayNetwork = fmt.Sprintf("%s/%d", ip, prefix)
} else {
slog.Warn("vpcd: centralized NAT requires pool.gw_lrp_range; LRP will use link-local and external traffic will not resolve ARP upstream",
"vpc_id", evt.VpcId, "pool", pool.Name)
}
}
slog.Info("vpcd: using external pool for IGW",
"pool", pool.Name,
"lrp_network", gatewayNetwork,
@@ -826,15 +912,19 @@ func (h *TopologyHandler) handleIGWAttach(msg *nats.Msg) {
// 3. Create gateway router port on the VPC router connecting to external switch
gwMAC := generateMAC("gw-" + evt.VpcId)
lrpExtIDs := map[string]string{
"spinifex:vpc_id": evt.VpcId,
"spinifex:igw_id": evt.InternetGatewayId,
"spinifex:role": "gateway",
}
if gwLrpIP != "" {
lrpExtIDs[gatewayIPExtID] = gwLrpIP
}
lrp := &nbdb.LogicalRouterPort{
Name: gwPortName,
MAC: gwMAC,
Networks: []string{gatewayNetwork},
ExternalIDs: map[string]string{
"spinifex:vpc_id": evt.VpcId,
"spinifex:igw_id": evt.InternetGatewayId,
"spinifex:role": "gateway",
},
Name: gwPortName,
MAC: gwMAC,
Networks: []string{gatewayNetwork},
ExternalIDs: lrpExtIDs,
}
if err := h.ovn.CreateLogicalRouterPort(ctx, routerName, lrp); err != nil {
slog.Error("vpcd: failed to create gateway router port", "port", gwPortName, "err", err)
@@ -1194,13 +1284,24 @@ func (h *TopologyHandler) reconcileIGW(ctx context.Context, vpcId, igwId string)
gwPortName := "gw-" + vpcId
switchGWPortName := "gw-port-" + vpcId
// Router port always link-local; pool IPs reserved for per-VM
// dnat_and_snat external addresses (mulga-siv-26).
// Direct mode: link-local LRP. Centralized mode: allocate from
// pool.GwLrpRange so the LRP can ARP on the WAN subnet (mulga-siv-36).
pool := h.findExternalPool("", "")
gatewayNetwork := gatewayPortNetwork
wanGateway := "169.254.0.2"
gwLrpIP := ""
if pool != nil {
wanGateway = pool.Gateway
if h.useCentralizedNAT() {
ip, prefix, allocOK, allocErr := h.allocateGatewayLRPIP(ctx, vpcId, pool)
if allocErr != nil {
return fmt.Errorf("allocate gw LRP IP for %s: %w", vpcId, allocErr)
}
if allocOK {
gwLrpIP = ip
gatewayNetwork = fmt.Sprintf("%s/%d", ip, prefix)
}
}
}
// Build external IDs with optional IGW ID
@@ -1252,14 +1353,18 @@ func (h *TopologyHandler) reconcileIGW(ctx context.Context, vpcId, igwId string)
// 3. Create gateway router port
gwMAC := generateMAC("gw-" + vpcId)
reconcileLrpExtIDs := map[string]string{
"spinifex:vpc_id": vpcId,
"spinifex:role": "gateway",
}
if gwLrpIP != "" {
reconcileLrpExtIDs[gatewayIPExtID] = gwLrpIP
}
lrp := &nbdb.LogicalRouterPort{
Name: gwPortName,
MAC: gwMAC,
Networks: []string{gatewayNetwork},
ExternalIDs: map[string]string{
"spinifex:vpc_id": vpcId,
"spinifex:role": "gateway",
},
Name: gwPortName,
MAC: gwMAC,
Networks: []string{gatewayNetwork},
ExternalIDs: reconcileLrpExtIDs,
}
if err := h.ovn.CreateLogicalRouterPort(ctx, routerName, lrp); err != nil {
_ = h.ovn.DeleteLogicalSwitch(ctx, extSwitchName)
@@ -1387,6 +1492,83 @@ func (h *TopologyHandler) reconcileGatewayChassis(ctx context.Context, validName
// --- Helpers ---
// gwLrpRange returns the gw LRP IP range configured on the pool.
// Returns ok=false when GwLrpRangeStart/End are not configured — caller
// must fall back to link-local in that case.
func gwLrpRange(pool *ExternalPoolConfig) (start, end net.IP, prefix int, ok bool) {
if pool == nil || pool.GwLrpRangeStart == "" || pool.GwLrpRangeEnd == "" {
return nil, nil, 0, false
}
s := net.ParseIP(pool.GwLrpRangeStart).To4()
e := net.ParseIP(pool.GwLrpRangeEnd).To4()
if s == nil || e == nil {
slog.Warn("vpcd: invalid gw_lrp_range, falling back to link-local",
"pool", pool.Name, "start", pool.GwLrpRangeStart, "end", pool.GwLrpRangeEnd)
return nil, nil, 0, false
}
if ipv4ToUint32(s) > ipv4ToUint32(e) {
slog.Warn("vpcd: gw_lrp_range start > end, falling back to link-local",
"pool", pool.Name, "start", pool.GwLrpRangeStart, "end", pool.GwLrpRangeEnd)
return nil, nil, 0, false
}
prefix = pool.PrefixLen
if prefix <= 0 || prefix > 32 {
prefix = 24
}
return s, e, prefix, true
}
// allocateGatewayLRPIP picks the next free IP in pool.GwLrpRange for the
// given VPC, scanning every existing LRP's spinifex:gateway_ip external_id
// to compute the used set. If gw-<vpcId> already has an allocation the
// existing IP is returned (idempotent reconcile). Returns ok=false when
// the pool has no gw_lrp_range — caller falls back to link-local.
func (h *TopologyHandler) allocateGatewayLRPIP(ctx context.Context, vpcId string, pool *ExternalPoolConfig) (ip string, prefix int, ok bool, err error) {
start, end, prefix, ok := gwLrpRange(pool)
if !ok {
return "", 0, false, nil
}
gwPortName := "gw-" + vpcId
lrps, err := h.ovn.ListLogicalRouterPorts(ctx)
if err != nil {
return "", 0, false, fmt.Errorf("list LRPs for gw IP allocation: %w", err)
}
used := make(map[uint32]struct{}, len(lrps))
for _, lrp := range lrps {
existing := lrp.ExternalIDs[gatewayIPExtID]
if existing == "" {
continue
}
if lrp.Name == gwPortName {
// Idempotent: already allocated to this VPC.
return existing, prefix, true, nil
}
if v := net.ParseIP(existing).To4(); v != nil {
used[ipv4ToUint32(v)] = struct{}{}
}
}
startU := ipv4ToUint32(start)
endU := ipv4ToUint32(end)
for n := startU; n <= endU; n++ {
if _, taken := used[n]; taken {
continue
}
return uint32ToIPv4(n).String(), prefix, true, nil
}
return "", 0, false, fmt.Errorf("gw_lrp_range exhausted for pool %q (%s-%s)", pool.Name, pool.GwLrpRangeStart, pool.GwLrpRangeEnd)
}
func ipv4ToUint32(ip net.IP) uint32 {
v := ip.To4()
return uint32(v[0])<<24 | uint32(v[1])<<16 | uint32(v[2])<<8 | uint32(v[3])
}
func uint32ToIPv4(n uint32) net.IP {
return net.IPv4(byte(n>>24&0xff), byte(n>>16&0xff), byte(n>>8&0xff), byte(n&0xff)).To4()
}
// subnetGateway computes the gateway IP (.1) from a CIDR string.
// Returns the gateway IP string and the prefix length.
func subnetGateway(cidr string) (string, int, error) {
+91
View File
@@ -1099,6 +1099,97 @@ func TestTopologyHandler_IGWAttach_MultiVPC_NoIPCollision(t *testing.T) {
}
}
// TestTopologyHandler_IGWAttach_CentralizedNATAllocatesGwLrpIP guards
// mulga-siv-36: in centralized NAT (veth/macvlan) 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.
func TestTopologyHandler_IGWAttach_CentralizedNATAllocatesGwLrpIP(t *testing.T) {
_, nc := startTestNATS(t)
mock := NewMockOVNClient()
_ = mock.Connect(context.Background())
ctx := context.Background()
pools := []ExternalPoolConfig{
{
Name: "lan",
RangeStart: "192.168.3.100",
RangeEnd: "192.168.3.150",
Gateway: "192.168.3.1",
PrefixLen: 24,
GwLrpRangeStart: "192.168.3.20",
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),
)
subs, err := topo.Subscribe(nc)
if err != nil {
t.Fatalf("subscribe: %v", err)
}
defer func() {
for _, s := range subs {
_ = s.Unsubscribe()
}
}()
vpcIds := []string{"vpc-cn-a", "vpc-cn-b", "vpc-cn-c"}
for _, vpcId := range vpcIds {
_ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{
Name: "vpc-" + vpcId,
ExternalIDs: map[string]string{
"spinifex:vpc_id": vpcId,
"spinifex:cidr": "10.0.0.0/16",
},
})
evt := types.IGWEvent{InternetGatewayId: "igw-" + vpcId, VpcId: vpcId}
data, _ := json.Marshal(evt)
resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000)
if err != nil {
t.Fatalf("request vpc.igw-attach %s: %v", vpcId, err)
}
assertSuccess(t, resp, "attach IGW "+vpcId)
}
seen := make(map[string]string, len(vpcIds))
for _, vpcId := range vpcIds {
gwPortName := "gw-" + vpcId
gwPort, err := mock.GetLogicalRouterPort(ctx, gwPortName)
if err != nil {
t.Fatalf("get %s: %v", gwPortName, err)
}
if len(gwPort.Networks) != 1 {
t.Fatalf("%s: expected 1 Network, got %v", gwPortName, gwPort.Networks)
}
net := gwPort.Networks[0]
if !strings.HasPrefix(net, "192.168.3.") || !strings.HasSuffix(net, "/24") {
t.Errorf("%s: expected 192.168.3.X/24 from gw_lrp_range, got %s", gwPortName, net)
}
gwIP := gwPort.ExternalIDs["spinifex:gateway_ip"]
if gwIP == "" {
t.Errorf("%s: missing spinifex:gateway_ip external_id", gwPortName)
}
if other, dup := seen[gwIP]; dup {
t.Errorf("gw IP collision: %s and %s share %s", gwPortName, other, gwIP)
}
seen[gwIP] = gwPortName
// Idempotent: re-attach must not change the assignment.
evt := types.IGWEvent{InternetGatewayId: "igw-" + vpcId, VpcId: vpcId}
data, _ := json.Marshal(evt)
_, _ = nc.Request(TopicIGWAttach, data, 5_000_000_000)
gwPort2, _ := mock.GetLogicalRouterPort(ctx, gwPortName)
if gwPort2.ExternalIDs["spinifex:gateway_ip"] != gwIP {
t.Errorf("%s: re-attach changed gateway_ip from %s to %s",
gwPortName, gwIP, gwPort2.ExternalIDs["spinifex:gateway_ip"])
}
}
}
func TestTopologyHandler_FindExternalPool(t *testing.T) {
pools := []ExternalPoolConfig{
{Name: "az-a", Region: "us-east-1", AZ: "us-east-1a", RangeStart: "1.1.1.1"},
+11 -9
View File
@@ -80,15 +80,17 @@ type Config struct {
// ExternalPoolConfig mirrors config.ExternalPool for vpcd's internal use.
type ExternalPoolConfig struct {
Name string
RangeStart string
RangeEnd string
Gateway string
GatewayIP string
PrefixLen int
DNSServers []string
Region string
AZ string
Name string
RangeStart string
RangeEnd string
Gateway string
GatewayIP string
PrefixLen int
DNSServers []string
Region string
AZ string
GwLrpRangeStart string // Sub-range for OVN gateway LRP IPs in centralized NAT mode (mulga-siv-36).
GwLrpRangeEnd string
}
// Service implements the Spinifex service interface for vpcd.