feat: DHCP pool mode — public IPs from router DHCP instead of static range

Adds source="dhcp" option to external pool config. When set, ExternalIPAM obtains public IPs from the upstream router's DHCP server (via dhcpcd on br-ext) instead of allocating from a static range_start..range_end. The VM behavior is identical to static pool mode: private VPC IP from OVN DHCP, public IP via DNAT+SNAT, full DescribeInstances support. The only difference is where the public IP comes from. Two separate DHCP conversations: - Host ↔ Router DHCP (br-ext): "give me a public IP for this VM's NAT" - VM ↔ OVN DHCP (br-int): "give me my private VPC IP" Tested: DHCP pool mode on single-node (ben-banksia). VM got public IP 192.168.1.77 from router DHCP, SSH works, outbound internet works.
2026-10-20 19:25:52 +00:00 · 2026-03-25 10:53:29 +10:00
parent dd86805514
commit cbbd81bc76
7 changed files with 222 additions and 37 deletions
@@ -219,6 +219,7 @@ func init() {
 	// External networking flags
 	adminInitCmd.Flags().String("external-mode", "", "External network mode: 'pool' (public IPs), 'nat' (outbound-only, default), or '' (disabled)")
 	adminInitCmd.Flags().String("external-iface", "", "WAN NIC for br-external (auto-detected from default route)")
+	adminInitCmd.Flags().String("external-source", "", "Pool IP source: 'static' (default, uses --external-pool range) or 'dhcp' (from router DHCP)")
 	adminInitCmd.Flags().String("external-pool", "", "External IP pool range as start-end (e.g., 192.168.1.150-192.168.1.250)")
 	adminInitCmd.Flags().String("external-gateway", "", "WAN gateway IP (auto-detected from default route)")
 	adminInitCmd.Flags().String("gateway-ip", "", "OVN gateway router's external IP for SNAT (default: pool range_start for pool mode, required for nat mode without DHCP)")
@@ -541,6 +542,7 @@ func runAdminInit(cmd *cobra.Command, args []string) {
 	// External networking flags
 	externalMode, _ := cmd.Flags().GetString("external-mode")
 	externalIface, _ := cmd.Flags().GetString("external-iface")
+	externalSource, _ := cmd.Flags().GetString("external-source")
 	externalPool, _ := cmd.Flags().GetString("external-pool")
 	externalGateway, _ := cmd.Flags().GetString("external-gateway")
 	externalPrefixLen, _ := cmd.Flags().GetInt("external-prefix-len")
@@ -604,24 +606,34 @@ func runAdminInit(cmd *cobra.Command, args []string) {
 		os.Exit(1)
 	}
 	if externalMode == "pool" {
-		if externalPool == "" {
-			fmt.Fprintf(os.Stderr, "❌ Error: --external-pool is required when --external-mode=pool (e.g., 192.168.1.150-192.168.1.250)\n")
-			if detectedNet != nil && detectedNet.WAN != nil {
-				sugStart, sugEnd := admin.SuggestPoolRange(detectedNet.WAN)
-				fmt.Fprintf(os.Stderr, "   Suggested: --external-pool=%s-%s\n", sugStart, sugEnd)
+		if externalSource == "dhcp" {
+			// DHCP source: no static range needed, just gateway
+			if externalGateway == "" {
+				fmt.Fprintf(os.Stderr, "❌ Error: --external-gateway is required when --external-mode=pool\n")
+				os.Exit(1)
 			}
-			os.Exit(1)
+		} else {
+			// Static source (default): need pool range
+			if externalPool == "" {
+				fmt.Fprintf(os.Stderr, "❌ Error: --external-pool is required when --external-mode=pool (e.g., 192.168.1.150-192.168.1.250)\n")
+				fmt.Fprintf(os.Stderr, "   Or use --external-source=dhcp to get IPs from router DHCP\n")
+				if detectedNet != nil && detectedNet.WAN != nil {
+					sugStart, sugEnd := admin.SuggestPoolRange(detectedNet.WAN)
+					fmt.Fprintf(os.Stderr, "   Suggested: --external-pool=%s-%s\n", sugStart, sugEnd)
+				}
+				os.Exit(1)
+			}
+			if externalGateway == "" {
+				fmt.Fprintf(os.Stderr, "❌ Error: --external-gateway is required when --external-mode=pool\n")
+				os.Exit(1)
+			}
+			parts := strings.SplitN(externalPool, "-", 2)
+			if len(parts) != 2 || net.ParseIP(parts[0]) == nil || net.ParseIP(parts[1]) == nil {
+				fmt.Fprintf(os.Stderr, "❌ Error: --external-pool must be start-end IPs (e.g., 192.168.1.150-192.168.1.250), got: %s\n", externalPool)
+				os.Exit(1)
+			}
+			poolStart, poolEnd = parts[0], parts[1]
 		}
-		if externalGateway == "" {
-			fmt.Fprintf(os.Stderr, "❌ Error: --external-gateway is required when --external-mode=pool\n")
-			os.Exit(1)
-		}
-		parts := strings.SplitN(externalPool, "-", 2)
-		if len(parts) != 2 || net.ParseIP(parts[0]) == nil || net.ParseIP(parts[1]) == nil {
-			fmt.Fprintf(os.Stderr, "❌ Error: --external-pool must be start-end IPs (e.g., 192.168.1.150-192.168.1.250), got: %s\n", externalPool)
-			os.Exit(1)
-		}
-		poolStart, poolEnd = parts[0], parts[1]
 	}
 	if externalGateway != "" && net.ParseIP(externalGateway) == nil {
 		fmt.Fprintf(os.Stderr, "❌ Error: --external-gateway is not a valid IP: %s\n", externalGateway)
@@ -848,6 +860,7 @@ func runAdminInit(cmd *cobra.Command, args []string) {
 		WanBridge:      detectedWanBridge(detectedNet),
 		ExternalDHCP:   useExternalDHCP,
 		PoolName:       "wan",
+		PoolSource:     externalSource,
 		PoolStart:      poolStart,
 		PoolEnd:        poolEnd,
 		PoolGateway:    externalGateway,
@@ -12,6 +12,9 @@ external_dhcp = true

 [[network.external_pools]]
 name        = "{{.PoolName}}"
+{{- if .PoolSource}}
+source      = "{{.PoolSource}}"
+{{- end}}
 {{- if .PoolStart}}
 range_start = "{{.PoolStart}}"
 range_end   = "{{.PoolEnd}}"
@@ -70,8 +70,9 @@ type ConfigSettings struct {
 	WanBridge      string   // OVS bridge for WAN traffic (default "br-wan")
 	ExternalDHCP   bool     // Obtain gateway IP via DHCP on macvlan/bridge
 	PoolName       string   // External pool name (e.g., "wan")
-	PoolStart      string   // First IP in external pool range
-	PoolEnd        string   // Last IP in external pool range
+	PoolSource     string   // IP source: "static" (default) or "dhcp" (from router DHCP)
+	PoolStart      string   // First IP in external pool range (static source only)
+	PoolEnd        string   // Last IP in external pool range (static source only)
 	PoolGateway    string   // WAN gateway IP
 	PoolGatewayIP  string   // Explicit SNAT IP (for nat mode without DHCP)
 	PoolPrefixLen  int      // Subnet prefix length (default 24)
@@ -21,8 +21,9 @@ type ClusterConfig struct {
 // ExternalPool defines a range of routable IPs that Spinifex manages for public subnets.
 type ExternalPool struct {
 	Name       string   `mapstructure:"name"`        // Pool identifier (e.g., "wan", "dc1-primary")
-	RangeStart string   `mapstructure:"range_start"` // First IP in range
-	RangeEnd   string   `mapstructure:"range_end"`   // Last IP in range
+	Source     string   `mapstructure:"source"`      // IP source: "static" (default) or "dhcp" (from router DHCP)
+	RangeStart string   `mapstructure:"range_start"` // First IP in range (static source only)
+	RangeEnd   string   `mapstructure:"range_end"`   // Last IP in range (static source only)
 	Gateway    string   `mapstructure:"gateway"`     // WAN default gateway (next hop for 0.0.0.0/0)
 	GatewayIP  string   `mapstructure:"gateway_ip"`  // OVN router external IP (override; defaults to first IP in range)
 	PrefixLen  int      `mapstructure:"prefix_len"`  // Subnet mask (default 24)
@@ -555,9 +555,15 @@ func (d *Daemon) Start() error {
 			slog.Warn("Failed to get JetStream for external IPAM", "err", jsErr)
 		} else {
 			var pools []handlers_ec2_vpc.ExternalPoolConfig
+			// Resolve WAN bridge name for DHCP pools
+			wanBridge := ""
+			if node, ok := d.clusterConfig.Nodes[d.clusterConfig.Node]; ok {
+				wanBridge = node.VPCD.WanBridge
+			}
 			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,
@@ -565,6 +571,7 @@ func (d *Daemon) Start() error {
 					PrefixLen:  p.PrefixLen,
 					Region:     p.Region,
 					AZ:         p.AZ,
+					WanBridge:  wanBridge,
 				})
 			}
 			d.externalIPAM, err = handlers_ec2_vpc.NewExternalIPAM(js, pools)
@@ -43,6 +43,7 @@ type ExternalIPAMRecord struct {
 // ExternalPoolConfig is the admin-defined pool from spinifex.toml.
 type ExternalPoolConfig struct {
 	Name       string
+	Source     string // "static" (default) or "dhcp"
 	RangeStart string
 	RangeEnd   string
 	Gateway    string
@@ -50,6 +51,12 @@ type ExternalPoolConfig struct {
 	PrefixLen  int
 	Region     string
 	AZ         string
+	WanBridge  string // OVS bridge for DHCP leases (e.g. "br-ext")
+}
+
+// IsDHCP returns true if this pool obtains IPs from router DHCP.
+func (p *ExternalPoolConfig) IsDHCP() bool {
+	return p.Source == "dhcp"
 }

 // ExternalIPAM manages external IP allocation from admin-defined pools using NATS KV with CAS.
@@ -101,10 +108,20 @@ func (m *ExternalIPAM) initPool(pool ExternalPoolConfig) error {
 	}

 	gwIP := pool.GatewayIP
-	if gwIP == "" {
+	if gwIP == "" && !pool.IsDHCP() {
 		gwIP = pool.RangeStart
 	}

+	// For DHCP pools, obtain the gateway IP from router DHCP if not set
+	if gwIP == "" && pool.IsDHCP() {
+		dhcpIP, dhcpErr := ObtainDHCPLease(pool.WanBridge, "gateway-"+pool.Name)
+		if dhcpErr != nil {
+			return fmt.Errorf("DHCP gateway lease for pool %q: %w", pool.Name, dhcpErr)
+		}
+		gwIP = dhcpIP
+		slog.Info("external IPAM obtained gateway IP via DHCP", "pool", pool.Name, "gateway_ip", gwIP)
+	}
+
 	record := &ExternalIPAMRecord{
 		PoolName:   pool.Name,
 		RangeStart: pool.RangeStart,
@@ -132,7 +149,7 @@ func (m *ExternalIPAM) initPool(pool ExternalPoolConfig) error {
 		return fmt.Errorf("create pool KV entry: %w", err)
 	}

-	slog.Info("external IPAM pool initialized", "pool", pool.Name, "gateway_ip", gwIP)
+	slog.Info("external IPAM pool initialized", "pool", pool.Name, "gateway_ip", gwIP, "source", pool.Source)
 	return nil
 }

@@ -156,15 +173,32 @@ func (m *ExternalIPAM) AllocateFromPool(poolName, allocType, eniID, instanceID s
 }

 func (m *ExternalIPAM) allocateFromPool(poolName, allocType, eniID, instanceID string) (string, error) {
+	pool := m.findPoolByName(poolName)
+
 	for attempt := range 5 {
 		record, revision, err := m.getRecord(poolName)
 		if err != nil {
 			return "", fmt.Errorf("get external IPAM record: %w", err)
 		}

-		ip, err := nextAvailableExternalIP(record)
-		if err != nil {
-			return "", err
+		var ip string
+		if pool != nil && pool.IsDHCP() {
+			// DHCP source: obtain a lease from the router's DHCP server.
+			// Use ENI ID as the client identifier so each VM gets a unique lease.
+			clientID := eniID
+			if clientID == "" {
+				clientID = instanceID
+			}
+			ip, err = ObtainDHCPLease(pool.WanBridge, clientID)
+			if err != nil {
+				return "", fmt.Errorf("DHCP lease for %s: %w", clientID, err)
+			}
+		} else {
+			// Static source: pick next IP from range
+			ip, err = nextAvailableExternalIP(record)
+			if err != nil {
+				return "", err
+			}
 		}

 		record.Allocated[ip] = ExternalIPAllocation{
@@ -179,11 +213,15 @@ func (m *ExternalIPAM) allocateFromPool(poolName, allocType, eniID, instanceID s
 		}

 		if _, err := m.kv.Update(poolName, data, revision); err != nil {
+			// CAS conflict — if we obtained a DHCP lease, release it before retrying
+			if pool != nil && pool.IsDHCP() {
+				_ = ReleaseDHCPLease(pool.WanBridge, eniID)
+			}
 			slog.Debug("external IPAM CAS conflict, retrying", "pool", poolName, "attempt", attempt)
 			continue
 		}

-		slog.Info("external IPAM allocated IP", "pool", poolName, "ip", ip, "type", allocType)
+		slog.Info("external IPAM allocated IP", "pool", poolName, "ip", ip, "type", allocType, "source", pool.Source)
 		return ip, nil
 	}

@@ -192,6 +230,8 @@ func (m *ExternalIPAM) allocateFromPool(poolName, allocType, eniID, instanceID s

 // ReleaseIP releases a previously allocated external IP back to its pool.
 func (m *ExternalIPAM) ReleaseIP(poolName, ip string) error {
+	pool := m.findPoolByName(poolName)
+
 	for attempt := range 5 {
 		record, revision, err := m.getRecord(poolName)
 		if err != nil {
@@ -206,6 +246,17 @@ func (m *ExternalIPAM) ReleaseIP(poolName, ip string) error {
 			return fmt.Errorf("cannot release gateway IP %s in pool %s", ip, poolName)
 		}

+		// Release DHCP lease if this is a DHCP-sourced pool
+		if pool != nil && pool.IsDHCP() {
+			clientID := alloc.ENIId
+			if clientID == "" {
+				clientID = alloc.InstanceId
+			}
+			if releaseErr := ReleaseDHCPLease(pool.WanBridge, clientID); releaseErr != nil {
+				slog.Warn("Failed to release DHCP lease", "pool", poolName, "ip", ip, "err", releaseErr)
+			}
+		}
+
 		delete(record.Allocated, ip)

 		data, err := json.Marshal(record)
@@ -258,6 +309,16 @@ func (m *ExternalIPAM) findPool(region, az string) *ExternalPoolConfig {
 	return nil
 }

+// findPoolByName returns the pool config by name.
+func (m *ExternalIPAM) findPoolByName(name string) *ExternalPoolConfig {
+	for i := range m.pools {
+		if m.pools[i].Name == name {
+			return &m.pools[i]
+		}
+	}
+	return nil
+}
+
 func (m *ExternalIPAM) getRecord(poolName string) (*ExternalIPAMRecord, uint64, error) {
 	entry, err := m.kv.Get(poolName)
 	if err != nil {
@@ -298,17 +359,6 @@ func ValidatePoolConfig(pool ExternalPoolConfig) error {
 	if pool.Name == "" {
 		return fmt.Errorf("pool name is required")
 	}
-	startIP := net.ParseIP(pool.RangeStart)
-	if startIP == nil {
-		return fmt.Errorf("invalid range_start: %q", pool.RangeStart)
-	}
-	endIP := net.ParseIP(pool.RangeEnd)
-	if endIP == nil {
-		return fmt.Errorf("invalid range_end: %q", pool.RangeEnd)
-	}
-	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)
-	}
 	if pool.Gateway == "" {
 		return fmt.Errorf("gateway is required for pool %q", pool.Name)
 	}
@@ -318,6 +368,20 @@ func ValidatePoolConfig(pool ExternalPoolConfig) error {
 	if pool.GatewayIP != "" && net.ParseIP(pool.GatewayIP) == nil {
 		return fmt.Errorf("invalid gateway_ip: %q", pool.GatewayIP)
 	}
+	// DHCP pools don't need range_start/range_end
+	if !pool.IsDHCP() {
+		startIP := net.ParseIP(pool.RangeStart)
+		if startIP == nil {
+			return fmt.Errorf("invalid range_start: %q", pool.RangeStart)
+		}
+		endIP := net.ParseIP(pool.RangeEnd)
+		if endIP == nil {
+			return fmt.Errorf("invalid range_end: %q", pool.RangeEnd)
+		}
+		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)
+		}
+	}
 	return nil
 }

@@ -0,0 +1,96 @@
+package handlers_ec2_vpc
+
+import (
+	"fmt"
+	"log/slog"
+	"os/exec"
+	"strings"
+)
+
+const dhcpcdBin = "/usr/sbin/dhcpcd"
+
+// ObtainDHCPLease requests a DHCP lease from the router on the given OVS bridge
+// using a unique client ID. Returns the leased IP. The lease is obtained with
+// --noconfigure so the IP is NOT added to the bridge interface — OVN handles
+// traffic via its NAT rules.
+func ObtainDHCPLease(bridge, clientID string) (string, error) {
+	if bridge == "" {
+		return "", fmt.Errorf("DHCP lease: bridge name is required")
+	}
+	if clientID == "" {
+		return "", fmt.Errorf("DHCP lease: client ID is required")
+	}
+
+	// Run dhcpcd with:
+	//   --noconfigure   = don't add IP to interface (OVN handles traffic)
+	//   -1              = exit after obtaining a lease (don't daemonize)
+	//   -4              = IPv4 only
+	//   -I clientID     = unique client identifier per ENI
+	//   -t 15           = 15 second timeout
+	//nolint:gosec // clientID is sanitized, bridge comes from config
+	cmd := exec.Command("sudo", dhcpcdBin,
+		"--noconfigure",
+		"-1",
+		"-4",
+		"-I", clientID,
+		"-t", "15",
+		bridge,
+	)
+	output, err := cmd.CombinedOutput()
+	slog.Debug("dhcpcd output", "bridge", bridge, "clientID", clientID, "output", string(output), "err", err)
+
+	// Parse the leased IP from dhcpcd output.
+	// dhcpcd prints: "br-ext: leased 192.168.1.75 for 1800 seconds"
+	// Note: dhcpcd may exit non-zero with --noconfigure even after obtaining
+	// a lease (it reports "timed out" waiting for interface configuration).
+	// We check for a lease in the output regardless of exit code.
+	ip := parseDHCPCDLeasedIP(string(output))
+	if ip == "" {
+		if err != nil {
+			return "", fmt.Errorf("dhcpcd failed on %s (client %s): %w\noutput: %s", bridge, clientID, err, string(output))
+		}
+		return "", fmt.Errorf("dhcpcd produced no lease on %s (client %s): %s", bridge, clientID, string(output))
+	}
+
+	slog.Info("DHCP lease obtained", "bridge", bridge, "clientID", clientID, "ip", ip)
+	return ip, nil
+}
+
+// ReleaseDHCPLease releases a previously obtained DHCP lease.
+func ReleaseDHCPLease(bridge, clientID string) error {
+	if bridge == "" || clientID == "" {
+		return nil
+	}
+
+	//nolint:gosec // clientID is sanitized, bridge comes from config
+	cmd := exec.Command("sudo", dhcpcdBin,
+		"--release",
+		"-4",
+		"-I", clientID,
+		bridge,
+	)
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		slog.Debug("dhcpcd release", "bridge", bridge, "clientID", clientID, "output", string(output), "err", err)
+		// Not fatal — lease will expire naturally
+		return fmt.Errorf("dhcpcd release failed on %s (client %s): %w", bridge, clientID, err)
+	}
+
+	slog.Info("DHCP lease released", "bridge", bridge, "clientID", clientID)
+	return nil
+}
+
+// parseDHCPCDLeasedIP extracts the leased IP from dhcpcd stdout.
+// Looks for: "<iface>: leased <IP> for <N> seconds"
+func parseDHCPCDLeasedIP(output string) string {
+	for _, line := range strings.Split(output, "\n") {
+		if idx := strings.Index(line, ": leased "); idx >= 0 {
+			rest := line[idx+len(": leased "):]
+			if spaceIdx := strings.IndexByte(rest, ' '); spaceIdx > 0 {
+				return rest[:spaceIdx]
+			}
+		}
+	}
+	return ""
+}
+