gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-15 17:11:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(vpcd): delete dhcp_manager + DHCP gateway-LRP path (Slice C)

Browse Source 
Removes upstream-DHCP-client gateway-LRP allocation in favor of static
WAN-pool allocation (StaticRangeAllocator). Part of bead mulga-siv-125.3.3.

- delete services/vpcd/dhcp_manager.go + dhcp/ subpackage (~865 lines)
- delete allocateGatewayLRPIPViaDHCP + releaseGatewayLRPLease paths
- drop ExternalPoolConfig.Source + DhcpBindBridge + IsDHCP() fields
- drop ExternalIPAllocation lease fields (Expiry/ServerID/HWAddr) and
  ExternalIPAMRecord.Source
- drop NewExternalIPAM nc *nats.Conn argument; update all 4 call sites
- rename verifyBridgeMode local dhcpBindBridge -> wanBridge; update error
  strings and tests

Slice D (delete attachIGWLegacy + dispatcher) intentionally deferred.
This commit is contained in:
joshsiv-mulga
2026-05-22 13:10:44 +10:00
parent 3d2444ee00
commit 7ff013a120
24 changed files with 122 additions and 3384 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/dhcptest/main.go
+2 -2
View File
@@ -89,7 +89,7 @@ func main() {
}
}
// Build nclient4 options — identical to spinifex/services/vpcd/dhcp/nclient4.go.
// Build nclient4 options — standalone DHCP probe for upstream-router debugging.
opts := []nclient4.ClientOpt{
nclient4.WithHWAddr(hwAddr),
nclient4.WithTimeout(*timeout),
@@ -105,7 +105,7 @@ func main() {
}
defer client.Close()
// Identity modifiers — identical to identityModifiers() in vpcd dhcp package.
// Identity modifiers — client-id / hostname / vendor-class for upstream lease tagging.
var mods []dhcpv4.Modifier
if cid != "" {
mods = append(mods, dhcpv4.WithOption(dhcpv4.OptClientIdentifier([]byte(cid))))
cmd/spinifex/cmd/service.go
-1
View File
@@ -728,7 +728,6 @@ var vpcdStartCmd = &cobra.Command{
for _, p := range clusterConfig.Network.ExternalPools {
extPools = append(extPools, vpcd.ExternalPoolConfig{
Name: p.Name,
Source: p.Source,
RangeStart: p.RangeStart,
RangeEnd: p.RangeEnd,
Gateway: p.Gateway,
spinifex/daemon/daemon.go
+1 -7
View File
@@ -1097,14 +1097,9 @@ func (d *Daemon) startCluster() error {
slog.Warn("Failed to get JetStream for external IPAM", "err", jsErr)
} else {
var pools []handlers_ec2_vpc.ExternalPoolConfig
gwMAC := ""
if d.clusterConfig.Bootstrap.VpcId != "" {
gwMAC = utils.HashMAC("gw-" + d.clusterConfig.Bootstrap.VpcId)
}
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,
@@ -1112,12 +1107,11 @@ func (d *Daemon) startCluster() error {
PrefixLen: p.PrefixLen,
Region: p.Region,
AZ: p.AZ,
GatewayMAC: gwMAC,
GwLrpRangeStart: p.GwLrpRangeStart,
GwLrpRangeEnd: p.GwLrpRangeEnd,
})
}
d.externalIPAM, err = handlers_ec2_vpc.NewExternalIPAM(d.natsConn, js, pools)
d.externalIPAM, err = handlers_ec2_vpc.NewExternalIPAM(js, pools)
if err != nil {
slog.Warn("Failed to initialize external IPAM", "err", err)
} else {
spinifex/daemon/daemon_handlers_test.go
+1 -1
View File
@@ -3548,7 +3548,7 @@ func TestDelegateHandlers_EIP(t *testing.T) {
js, err := nc.JetStream()
require.NoError(t, err)
ipam, err := handlers_ec2_vpc.NewExternalIPAM(nc, js, []handlers_ec2_vpc.ExternalPoolConfig{
ipam, err := handlers_ec2_vpc.NewExternalIPAM(js, []handlers_ec2_vpc.ExternalPoolConfig{
{Name: "test-pool", RangeStart: "192.168.100.2", RangeEnd: "192.168.100.254", Gateway: "192.168.100.1", PrefixLen: 24},
})
require.NoError(t, err)
spinifex/handlers/ec2/eip/service_impl_test.go
+1 -1
View File
@@ -30,7 +30,7 @@ func setupTestEIP(t *testing.T) (*EIPServiceImpl, *handlers_ec2_vpc.ExternalIPAM
_, nc, js := testutil.StartTestJetStream(t)
pool := testPool()
ipam, err := handlers_ec2_vpc.NewExternalIPAM(nc, js, []handlers_ec2_vpc.ExternalPoolConfig{pool})
ipam, err := handlers_ec2_vpc.NewExternalIPAM(js, []handlers_ec2_vpc.ExternalPoolConfig{pool})
require.NoError(t, err)
svc, err := NewEIPServiceImpl(nc, ipam, nil)
spinifex/handlers/ec2/vpc/external_ipam.go
+54 -221
View File
@@ -9,7 +9,6 @@ import (
"net"
"github.com/mulgadc/spinifex/spinifex/migrate"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go"
)
@@ -27,20 +26,11 @@ type ExternalIPAllocation struct {
ENIId string `json:"eni_id,omitempty"` // ENI owning this IP
InstanceId string `json:"instance_id,omitempty"` // Instance owning this IP
Note string `json:"note,omitempty"` // Human-readable note
// DHCP-sourced metadata — set only for allocations whose pool has
// source="dhcp" (see dhcp.RequestAcquire). These fields let describe
// surfaces and operators trace a given IP back to the upstream lease
// without re-querying vpcd.
LeaseExpiresUnix int64 `json:"lease_expires_unix,omitempty"` // Absolute expiry from the server's ACK
DHCPServerID string `json:"dhcp_server_id,omitempty"` // Option 54 of the ACK
HWAddr string `json:"hw_addr,omitempty"` // chaddr placed in the DHCP payload
}
// 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"`
@@ -58,23 +48,14 @@ 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
GatewayIP string
PrefixLen int
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
Name string
RangeStart string
RangeEnd string
Gateway string
GatewayIP string
PrefixLen int
Region string
AZ 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.
@@ -82,25 +63,14 @@ type ExternalPoolConfig struct {
GwLrpRangeEnd string
}
// 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.
type ExternalIPAM struct {
kv nats.KeyValue
pools []ExternalPoolConfig
// nc is used to talk to spinifex-vpcd for DHCP-sourced pools via
// vpc.dhcp.acquire / vpc.dhcp.release. May be nil when no pool has
// Source="dhcp" (callers should still pass it if available).
nc *nats.Conn
}
// NewExternalIPAM creates a new ExternalIPAM backed by NATS JetStream KV.
// nc is required for DHCP-sourced pools and may be nil for static-only
// deployments.
func NewExternalIPAM(nc *nats.Conn, js nats.JetStreamContext, pools []ExternalPoolConfig) (*ExternalIPAM, error) {
func NewExternalIPAM(js nats.JetStreamContext, pools []ExternalPoolConfig) (*ExternalIPAM, error) {
kv, err := utils.GetOrCreateKVBucket(js, KVBucketExternalIPAM, 5)
if err != nil {
return nil, fmt.Errorf("create external IPAM KV bucket: %w", err)
@@ -108,17 +78,16 @@ func NewExternalIPAM(nc *nats.Conn, js nats.JetStreamContext, pools []ExternalPo
if err := migrate.DefaultRegistry.RunKV(KVBucketExternalIPAM, kv, KVBucketExternalIPAMVersion); err != nil {
return nil, fmt.Errorf("migrate %s: %w", KVBucketExternalIPAM, err)
}
ipam := &ExternalIPAM{kv: kv, pools: pools, nc: nc}
ipam := &ExternalIPAM{kv: kv, pools: pools}
if err := ipam.initPools(); err != nil {
return nil, fmt.Errorf("init external IPAM pools: %w", err)
}
return ipam, nil
}
// NewExternalIPAMWithKV creates an ExternalIPAM with an existing KV bucket
// (for testing). nc may be nil when the test exercises only static pools.
func NewExternalIPAMWithKV(nc *nats.Conn, kv nats.KeyValue, pools []ExternalPoolConfig) *ExternalIPAM {
return &ExternalIPAM{kv: kv, pools: pools, nc: nc}
// NewExternalIPAMWithKV creates an ExternalIPAM with an existing KV bucket (for testing).
func NewExternalIPAMWithKV(kv nats.KeyValue, pools []ExternalPoolConfig) *ExternalIPAM {
return &ExternalIPAM{kv: kv, pools: pools}
}
// initPools ensures each configured pool has a KV record. Idempotent — safe to call
@@ -140,18 +109,16 @@ 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.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_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
@@ -172,39 +139,11 @@ func (m *ExternalIPAM) initPool(pool ExternalPoolConfig) error {
slog.Info("external IPAM pool not found, creating", "pool", pool.Name)
// TODO: Confirm gateway IP defined, should error, given we route traffic to the upstream DHCP gateway
gwIP := pool.GatewayIP
if gwIP == "" && !pool.IsDHCP() {
if gwIP == "" {
gwIP = pool.RangeStart
}
// For DHCP pools, obtain the gateway IP from router DHCP if not set.
// The gateway lease is long-lived: vpcd's renewal goroutine keeps it
// refreshed until the pool is torn down.
var gatewayLease *dhcp.LeaseResult
if gwIP == "" && pool.IsDHCP() {
lease, dhcpErr := dhcp.RequestAcquire(m.nc, pool.DhcpBindBridge,
"gateway-"+pool.Name,
"gateway-"+pool.Name,
"mulga-spinifex-gw",
pool.Name,
"",
)
if dhcpErr != nil {
return fmt.Errorf("DHCP gateway lease for pool %q: %w", pool.Name, dhcpErr)
}
gatewayLease = &lease
gwIP = lease.IP
slog.Info("external IPAM obtained gateway IP via DHCP", "pool", pool.Name, "gateway_ip", gwIP)
}
gatewayAlloc := ExternalIPAllocation{Type: "gateway", Note: "OVN router SNAT address"}
if gatewayLease != nil {
gatewayAlloc.LeaseExpiresUnix = gatewayLease.ExpiresUnix
gatewayAlloc.DHCPServerID = gatewayLease.ServerID
gatewayAlloc.HWAddr = gatewayLease.HWAddr
}
record := &ExternalIPAMRecord{
PoolName: pool.Name,
RangeStart: pool.RangeStart,
@@ -217,7 +156,7 @@ func (m *ExternalIPAM) initPool(pool ExternalPoolConfig) error {
GwLrpRangeStart: pool.GwLrpRangeStart,
GwLrpRangeEnd: pool.GwLrpRangeEnd,
Allocated: map[string]ExternalIPAllocation{
gwIP: gatewayAlloc,
gwIP: {Type: "gateway", Note: "OVN router SNAT address"},
},
}
@@ -234,7 +173,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, "source", pool.Source)
slog.Info("external IPAM pool initialized", "pool", pool.Name, "gateway_ip", gwIP)
return nil
}
@@ -259,71 +198,26 @@ func (m *ExternalIPAM) AllocateFromPool(poolName, allocType, allocID, eniID, ins
}
func (m *ExternalIPAM) allocateFromPool(poolName, allocType, allocID, eniID, instanceID string) (string, error) {
pool := m.findPoolByName(poolName)
// DHCP client-id: prefer allocID (EIPs), then eniID (ENIs), then instanceID.
// At least one must be non-empty for DHCP pools — the upstream DHCP server
// uses this to keep per-client leases distinct.
clientID := allocID
if clientID == "" {
clientID = eniID
}
if clientID == "" {
clientID = instanceID
}
// For DHCP-sourced pools: acquire the lease once, up front. vpcd's
// Manager is idempotent on ClientID, so CAS retries below will not
// trigger a second DORA — we only release at the end if no CAS
// attempt succeeded.
var dhcpLease *dhcp.LeaseResult
if pool != nil && pool.IsDHCP() {
hostname, vendorClass := dhcpIdentityOptions(eniID, instanceID, poolName)
lease, err := dhcp.RequestAcquire(m.nc, pool.DhcpBindBridge, clientID, hostname, vendorClass, poolName, pool.GatewayMAC)
if err != nil {
return "", fmt.Errorf("DHCP lease for %s: %w", clientID, err)
}
dhcpLease = &lease
}
for attempt := range 5 {
record, revision, err := m.getRecord(poolName)
if err != nil {
if dhcpLease != nil {
_ = dhcp.RequestRelease(m.nc, clientID)
}
return "", fmt.Errorf("get external IPAM record: %w", err)
}
var ip string
if dhcpLease != nil {
ip = dhcpLease.IP
} else {
// Static source: pick next IP from range
ip, err = nextAvailableExternalIP(record)
if err != nil {
return "", err
}
ip, err := nextAvailableExternalIP(record)
if err != nil {
return "", err
}
alloc := ExternalIPAllocation{
record.Allocated[ip] = ExternalIPAllocation{
Type: allocType,
AllocationID: allocID,
ENIId: eniID,
InstanceId: instanceID,
}
if dhcpLease != nil {
alloc.LeaseExpiresUnix = dhcpLease.ExpiresUnix
alloc.DHCPServerID = dhcpLease.ServerID
alloc.HWAddr = dhcpLease.HWAddr
}
record.Allocated[ip] = alloc
data, err := json.Marshal(record)
if err != nil {
if dhcpLease != nil {
_ = dhcp.RequestRelease(m.nc, clientID)
}
return "", fmt.Errorf("marshal external IPAM record: %w", err)
}
@@ -332,47 +226,15 @@ func (m *ExternalIPAM) allocateFromPool(poolName, allocType, allocID, eniID, ins
continue
}
source := "static"
if pool != nil {
source = pool.Source
}
slog.Info("external IPAM allocated IP", "pool", poolName, "ip", ip, "type", allocType, "source", source)
slog.Info("external IPAM allocated IP", "pool", poolName, "ip", ip, "type", allocType)
return ip, nil
}
// All CAS attempts exhausted — release any DHCP lease we acquired so
// vpcd doesn't hold a binding for an IP we won't use.
if dhcpLease != nil {
_ = dhcp.RequestRelease(m.nc, clientID)
}
return "", fmt.Errorf("external IPAM allocation failed after CAS retries for pool %s", poolName)
}
// dhcpIdentityOptions composes the option 12 (hostname) and option 60
// (vendor class) values for a per-allocation lease. Empty eniID /
// instanceID fall through to identifiers that still group usefully on
// the upstream server's lease table. Instance and ENI identifiers are
// used as-is (they already carry their "i-" / "eni-" prefixes).
func dhcpIdentityOptions(eniID, instanceID, poolName string) (hostname, vendorClass string) {
hostname = eniID
if hostname == "" {
if instanceID != "" {
hostname = "spinifex-" + instanceID
} else {
hostname = "spinifex-" + poolName
}
}
vendorClass = "mulga-spinifex"
if instanceID != "" {
vendorClass = instanceID
}
return hostname, vendorClass
}
// 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 {
@@ -387,22 +249,6 @@ 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. clientID
// selection mirrors allocateFromPool so vpcd's Manager can find the
// lease it originally acquired.
if pool != nil && pool.IsDHCP() {
clientID := alloc.AllocationID
if clientID == "" {
clientID = alloc.ENIId
}
if clientID == "" {
clientID = alloc.InstanceId
}
if releaseErr := dhcp.RequestRelease(m.nc, 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)
@@ -455,16 +301,6 @@ 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 {
@@ -529,44 +365,41 @@ 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)
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)
}
// 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)
}
endIP := net.ParseIP(pool.RangeEnd)
if endIP == nil {
return fmt.Errorf("invalid range_end: %q", pool.RangeEnd)
if gwE == nil {
return fmt.Errorf("invalid gw_lrp_range_end: %q", pool.GwLrpRangeEnd)
}
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)
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)
}
// 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)
}
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
spinifex/handlers/ec2/vpc/external_ipam_test.go
+8 -246
View File
@@ -1,24 +1,19 @@
package handlers_ec2_vpc
import (
"encoding/json"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
"github.com/mulgadc/spinifex/spinifex/testutil"
"github.com/nats-io/nats.go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func setupTestExternalIPAM(t *testing.T, pools []ExternalPoolConfig) *ExternalIPAM {
t.Helper()
_, nc, js := testutil.StartTestJetStream(t)
_, _, js := testutil.StartTestJetStream(t)
ipam, err := NewExternalIPAM(nc, js, pools)
ipam, err := NewExternalIPAM(js, pools)
require.NoError(t, err)
return ipam
}
@@ -339,10 +334,10 @@ func TestExternalIPAM_RangeValidation(t *testing.T) {
func TestExternalIPAM_InitFromConfig(t *testing.T) {
pool := testPool()
// Create IPAM twice — second init should be idempotent
_, nc, js := testutil.StartTestJetStream(t)
_, _, js := testutil.StartTestJetStream(t)
// First init
ipam1, err := NewExternalIPAM(nc, js, []ExternalPoolConfig{pool})
ipam1, err := NewExternalIPAM(js, []ExternalPoolConfig{pool})
require.NoError(t, err)
// Allocate an IP
@@ -351,7 +346,7 @@ func TestExternalIPAM_InitFromConfig(t *testing.T) {
assert.Equal(t, "192.168.1.151", ip)
// Second init (simulating restart) — should not lose allocation
ipam2, err := NewExternalIPAM(nc, js, []ExternalPoolConfig{pool})
ipam2, err := NewExternalIPAM(js, []ExternalPoolConfig{pool})
require.NoError(t, err)
record, err := ipam2.GetPoolRecord("wan")
@@ -389,247 +384,14 @@ func TestExternalIPAM_FindPoolByName_NotFound(t *testing.T) {
pool := testPool()
ipam := setupTestExternalIPAM(t, []ExternalPoolConfig{pool})
// findPoolByName is private, but exercised via AllocateFromPool with unknown pool.
// The function returns nil when no pool matches, which means static allocation path.
// We verify by using NewExternalIPAMWithKV to directly check findPoolByName returns nil.
// AllocateFromPool with a non-existent pool name: the pool "nonexistent"
// has no KV record, so getRecord fails.
kv := ipam.kv
ipam2 := NewExternalIPAMWithKV(nil, kv, []ExternalPoolConfig{pool})
// AllocateFromPool with a non-existent pool name: findPoolByName returns nil,
// so pool.IsDHCP() is skipped and static allocation is used.
// The pool "nonexistent" has no KV record, so getRecord fails.
ipam2 := NewExternalIPAMWithKV(kv, []ExternalPoolConfig{pool})
_, err := ipam2.AllocateFromPool("nonexistent", "auto_assign", "", "eni-1", "i-1")
assert.Error(t, err)
}
// dhcpStub captures acquire/release requests and replies with a
// deterministic lease so the handler-side IPAM code can be exercised
// without running vpcd.
type dhcpStub struct {
mu sync.Mutex
acquired []dhcp.AcquireRequestMsg
released []dhcp.ReleaseRequestMsg
nextIP string
acquireErr string
acquireCalls atomic.Int32
releaseCalls atomic.Int32
serverID string
leaseSeconds int64
hwAddrOverride string
releaseErrOnce atomic.Bool
}
func newDHCPStub(t *testing.T, nc *nats.Conn) *dhcpStub {
t.Helper()
s := &dhcpStub{
nextIP: "192.168.1.151",
serverID: "192.168.1.1",
leaseSeconds: 3600,
}
acquire, err := nc.Subscribe(dhcp.TopicAcquire, func(msg *nats.Msg) {
s.acquireCalls.Add(1)
var req dhcp.AcquireRequestMsg
if err := json.Unmarshal(msg.Data, &req); err != nil {
respondTestJSON(t, msg, dhcp.AcquireReplyMsg{Error: "malformed request"})
return
}
s.mu.Lock()
s.acquired = append(s.acquired, req)
ip := s.nextIP
errMsg := s.acquireErr
serverID := s.serverID
expires := time.Now().Add(time.Duration(s.leaseSeconds) * time.Second).Unix()
hw := s.hwAddrOverride
s.mu.Unlock()
if errMsg != "" {
respondTestJSON(t, msg, dhcp.AcquireReplyMsg{Error: errMsg})
return
}
if hw == "" {
hw = "02:00:00:aa:bb:cc"
}
respondTestJSON(t, msg, dhcp.AcquireReplyMsg{
IP: ip,
SubnetMask: "255.255.255.0",
Routers: []string{"192.168.1.1"},
DNS: []string{"192.168.1.1"},
ServerID: serverID,
HWAddr: hw,
ExpiresUnix: expires,
})
})
require.NoError(t, err)
t.Cleanup(func() { _ = acquire.Unsubscribe() })
release, err := nc.Subscribe(dhcp.TopicRelease, func(msg *nats.Msg) {
s.releaseCalls.Add(1)
var req dhcp.ReleaseRequestMsg
_ = json.Unmarshal(msg.Data, &req)
s.mu.Lock()
s.released = append(s.released, req)
s.mu.Unlock()
reply := dhcp.ReleaseReplyMsg{}
if s.releaseErrOnce.CompareAndSwap(true, false) {
reply.Error = "first release fails"
}
respondTestJSON(t, msg, reply)
})
require.NoError(t, err)
t.Cleanup(func() { _ = release.Unsubscribe() })
return s
}
func respondTestJSON(t *testing.T, msg *nats.Msg, v any) {
t.Helper()
data, err := json.Marshal(v)
require.NoError(t, err)
require.NoError(t, msg.Respond(data))
}
func TestExternalIPAM_DHCPAllocateAndRelease(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
stub := newDHCPStub(t, nc)
stub.nextIP = "192.168.1.200"
pool := ExternalPoolConfig{
Name: "wan-dhcp",
Source: "dhcp",
Gateway: "192.168.1.1",
GatewayIP: "192.168.1.1", // pre-set so initPool skips gateway DORA
PrefixLen: 24,
DhcpBindBridge: "br-wan",
}
ipam, err := NewExternalIPAM(nc, js, []ExternalPoolConfig{pool})
require.NoError(t, err)
ip, err := ipam.AllocateFromPool("wan-dhcp", "elastic_ip", "eipalloc-123", "eni-abc", "i-xyz")
require.NoError(t, err)
assert.Equal(t, "192.168.1.200", ip)
require.EqualValues(t, 1, stub.acquireCalls.Load())
stub.mu.Lock()
got := stub.acquired[0]
stub.mu.Unlock()
assert.Equal(t, "eipalloc-123", got.ClientID, "clientID precedence: allocID first")
assert.Equal(t, "eni-abc", got.Hostname, "hostname defaults to eniID")
assert.Equal(t, "i-xyz", got.VendorClass, "vendor class i-<instanceID>")
assert.Equal(t, "wan-dhcp", got.PoolName)
assert.Equal(t, "br-wan", got.Bridge)
record, err := ipam.GetPoolRecord("wan-dhcp")
require.NoError(t, err)
alloc, ok := record.Allocated["192.168.1.200"]
require.True(t, ok)
assert.Equal(t, "192.168.1.1", alloc.DHCPServerID)
assert.NotZero(t, alloc.LeaseExpiresUnix)
assert.Equal(t, "02:00:00:aa:bb:cc", alloc.HWAddr)
require.NoError(t, ipam.ReleaseIP("wan-dhcp", "192.168.1.200"))
require.EqualValues(t, 1, stub.releaseCalls.Load())
stub.mu.Lock()
assert.Equal(t, "eipalloc-123", stub.released[0].ClientID)
stub.mu.Unlock()
}
func TestExternalIPAM_DHCPAcquireErrorPropagates(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
stub := newDHCPStub(t, nc)
stub.acquireErr = "server unreachable"
pool := ExternalPoolConfig{
Name: "wan-dhcp-err", Source: "dhcp", Gateway: "192.168.1.1",
GatewayIP: "192.168.1.1", PrefixLen: 24, DhcpBindBridge: "br-wan",
}
ipam, err := NewExternalIPAM(nc, js, []ExternalPoolConfig{pool})
require.NoError(t, err)
_, err = ipam.AllocateFromPool("wan-dhcp-err", "elastic_ip", "eipalloc-err", "", "")
require.Error(t, err)
assert.Contains(t, err.Error(), "server unreachable")
}
func TestExternalIPAM_DHCPGatewayFromLease(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
stub := newDHCPStub(t, nc)
stub.nextIP = "192.168.3.247"
pool := ExternalPoolConfig{
Name: "wan-dhcp-gw", Source: "dhcp", Gateway: "192.168.3.1",
PrefixLen: 24, DhcpBindBridge: "br-wan",
// GatewayIP intentionally empty — initPool must pull it from DHCP.
}
_, err := NewExternalIPAM(nc, js, []ExternalPoolConfig{pool})
require.NoError(t, err)
assert.EqualValues(t, 1, stub.acquireCalls.Load())
stub.mu.Lock()
got := stub.acquired[0]
stub.mu.Unlock()
assert.Equal(t, "gateway-wan-dhcp-gw", got.ClientID)
assert.Equal(t, "mulga-spinifex-gw", got.VendorClass)
}
func TestExternalIPAM_DHCPGatewayErrorPropagates(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
stub := newDHCPStub(t, nc)
stub.acquireErr = "no response from upstream"
pool := ExternalPoolConfig{
Name: "wan-gw-err", Source: "dhcp", Gateway: "192.168.1.1",
PrefixLen: 24, DhcpBindBridge: "br-wan",
}
_, err := NewExternalIPAM(nc, js, []ExternalPoolConfig{pool})
require.Error(t, err)
assert.Contains(t, err.Error(), "no response from upstream")
}
func TestExternalIPAM_DHCPReleaseErrorIsNonFatal(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
stub := newDHCPStub(t, nc)
stub.nextIP = "192.168.1.155"
stub.releaseErrOnce.Store(true)
pool := ExternalPoolConfig{
Name: "wan-rel-err", Source: "dhcp", Gateway: "192.168.1.1",
GatewayIP: "192.168.1.1", PrefixLen: 24, DhcpBindBridge: "br-wan",
}
ipam, err := NewExternalIPAM(nc, js, []ExternalPoolConfig{pool})
require.NoError(t, err)
ip, err := ipam.AllocateFromPool("wan-rel-err", "elastic_ip", "eipalloc-rel", "", "")
require.NoError(t, err)
// Release reports vpcd failure via slog.Warn but ReleaseIP itself
// still succeeds — the allocation is removed from KV.
require.NoError(t, ipam.ReleaseIP("wan-rel-err", ip))
rec, err := ipam.GetPoolRecord("wan-rel-err")
require.NoError(t, err)
_, stillThere := rec.Allocated[ip]
assert.False(t, stillThere)
}
func TestDHCPIdentityOptions(t *testing.T) {
tests := []struct {
name string
eniID string
instanceID string
pool string
wantHost string
wantVendor string
}{
{"eni+instance", "eni-1", "i-abcd", "wan", "eni-1", "i-abcd"},
{"instance only", "", "i-xyz", "wan", "spinifex-i-xyz", "i-xyz"},
{"neither", "", "", "wan", "spinifex-wan", "mulga-spinifex"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
host, vendor := dhcpIdentityOptions(tc.eniID, tc.instanceID, tc.pool)
assert.Equal(t, tc.wantHost, host)
assert.Equal(t, tc.wantVendor, vendor)
})
}
}
// TestNextAvailableExternalIP_SkipsGwLrpRange verifies the per-VM EIP allocator
// honors gw_lrp_range by skipping IPs reserved for vpcd's gateway LRPs (siv-36).
func TestNextAvailableExternalIP_SkipsGwLrpRange(t *testing.T) {
spinifex/network/external/external.go
Vendored
+4 -6
View File
@@ -11,12 +11,10 @@
// grows those methods, IGWManager calls L1 directly for those L2 objects.
// This is documented and tracked for cleanup in a follow-on bead.
//
// - DHCP-sourced pools (source="dhcp") use upstream router DHCP to acquire
// the gateway LRP IP. The DHCP client and JetStream lease store live in
// services/vpcd/dhcp; this package keeps the dependency out by accepting
// a GatewayIPAllocator interface. StaticRangeAllocator is provided here;
// the DHCP-backed allocator stays in vpcd until bead mulga-siv-125.3.3
// (DHCP removal) is unblocked.
// - Gateway LRP IP allocation goes through a GatewayIPAllocator interface.
// StaticRangeAllocator (this package) picks from pool.gw_lrp_range;
// LinkLocalAllocator returns ok=false for distributed-NAT deployments
// where the gateway LRP never goes on the wire.
//
// See docs/development/feature/spinifex-network-redesign.md §10 and
// docs/development/feature/spinifex-network-redesign-phase2.md §2.4.
spinifex/network/external/pool.go
Vendored
-4
View File
@@ -64,10 +64,6 @@ func FindPool(pools []ExternalPoolConfig, region, az string) *ExternalPoolConfig
// pool.gw_lrp_range, persisting the assignment via the LRP's
// external_ids so reconciles see it on retry.
//
// - DHCP-backed allocator (lives in services/vpcd until bead
// mulga-siv-125.3.3 lands): requests an upstream DHCP lease and reuses
// it on idempotent re-attach.
//
// - LinkLocalAllocator (this package): always returns ok=false; callers
// fall back to linkLocalGatewayNetwork. Useful for distributed-NAT
// deployments where the gateway LRP never goes on the wire.
spinifex/network/external/types.go
Vendored
+4 -17
View File
@@ -1,23 +1,16 @@
package external
// ExternalPoolConfig describes one external IP pool wired to a VPC. Mirrors
// the [external_pools] entry in spinifex.toml. Two pool kinds are supported:
//
// - Source="static": IPs come from RangeStart..RangeEnd, gateway from
// Gateway, and the gateway LRP IP from GwLrpRangeStart..GwLrpRangeEnd
// (auto-derived from the WAN subnet when unset).
//
// - Source="dhcp": IPs come from upstream router DHCP. The gateway LRP IP
// is acquired via upstream DHCP using the pool's DhcpBindBridge. This
// package does not include the DHCP client; callers inject a
// GatewayIPAllocator that knows how to talk to the DHCP store.
// the [external_pools] entry in spinifex.toml. IPs come from
// RangeStart..RangeEnd, the gateway from Gateway, and the gateway LRP IP
// from GwLrpRangeStart..GwLrpRangeEnd (auto-derived from the WAN subnet
// when unset). The upstream-DHCP source model is gone (mulga-siv-125.3.3).
//
// This type duplicates services/vpcd/vpcd.go's ExternalPoolConfig. Dedup is
// out of scope for this bead; the two will merge when L2 (topology) grows
// the L5-driving methods and vpcd's local copy disappears.
type ExternalPoolConfig struct {
Name string
Source string
RangeStart string
RangeEnd string
Gateway string
@@ -26,16 +19,10 @@ type ExternalPoolConfig struct {
DNSServers []string
Region string
AZ string
DhcpBindBridge string
GwLrpRangeStart string
GwLrpRangeEnd string
}
// IsDHCP reports whether the pool acquires IPs from an upstream DHCP server.
func (p *ExternalPoolConfig) IsDHCP() bool {
return p.Source == "dhcp"
}
// IGWSpec is the L5 input for IGWManager.AttachIGW / DetachIGW. VPCID is
// the only required field; InternetGatewayID is propagated into OVN
// external_ids so reconcile paths can correlate state.
spinifex/services/vpcd/dhcp/client.go
-88
View File
@@ -1,88 +0,0 @@
// Package dhcp provides an in-process DHCPv4 client used by spinifex-vpcd
// to obtain, renew and release real server-bound leases on behalf of the
// external IPAM pool. It replaces the historical "shell out to sudo dhcpcd"
// path which had no renewal, phantom leases under -T, and a release channel
// that collided with a host-side dhcpcd daemon managing the same bridge.
//
// The real implementation (NewNClient4) opens an AF_PACKET socket per DORA
// on the target bridge via github.com/insomniacslk/dhcp/dhcpv4/nclient4.
// Tests inject Fake instead.
package dhcp
import (
"context"
"net"
"time"
)
// Client is the per-handshake DHCP surface. Backed by NewNClient4 in
// production; Fake satisfies this for unit tests.
type Client interface {
// Acquire performs a full DORA on req.Bridge and returns the
// server-bound lease. Blocks until ACK or ctx cancellation.
Acquire(ctx context.Context, req AcquireRequest) (*Lease, error)
// Renew refreshes an existing lease with its recorded server-id.
// Returns a new Lease reflecting the refreshed timers.
Renew(ctx context.Context, lease *Lease) (*Lease, error)
// Release sends DHCPRELEASE to the server-id in the lease.
Release(ctx context.Context, lease *Lease) error
}
// AcquireRequest identifies a new DHCP lease. Fields map 1:1 to DHCP
// options and the payload chaddr.
type AcquireRequest struct {
Bridge string // OVS bridge carrying the AF_PACKET socket
ClientID string // option 61 — client-identifier
Hostname string // option 12 — host-name
VendorClass string // option 60 — vendor-class-identifier
HWAddr net.HardwareAddr // chaddr in the DHCP payload
}
// Lease is a server-bound DHCPv4 lease. Carries enough state for Renew
// and Release, plus the raw Offer/ACK packets so that the Manager can
// resume after a daemon restart.
type Lease struct {
// Request context — echoed for audit; also used to rebuild the
// underlying nclient4 client on Renew/Release.
Bridge string
ClientID string
Hostname string
VendorClass string
HWAddr net.HardwareAddr
// Wire state extracted from the server's ACK.
IP net.IP
SubnetMask net.IPMask
Routers []net.IP
DNS []net.IP
ServerID net.IP
AcquiredAt time.Time
LeaseDuration time.Duration
T1 time.Duration // renew timer (option 58, or lease/2)
T2 time.Duration // rebind timer (option 59, or lease*7/8)
// Raw wire packets. Opaque to callers; nclient4 needs them to build
// RENEW / RELEASE messages that hit the same server binding. Persisted
// in KV for crash recovery.
RawOffer []byte
RawACK []byte
}
// ExpiresAt returns the absolute deadline at which the upstream server is
// free to reassign this IP.
func (l *Lease) ExpiresAt() time.Time {
return l.AcquiredAt.Add(l.LeaseDuration)
}
// RenewAt returns the absolute time the Manager should attempt a RENEW
// (option 58 / T1).
func (l *Lease) RenewAt() time.Time {
return l.AcquiredAt.Add(l.T1)
}
// RebindAt returns the absolute time the Manager should attempt a REBIND
// (option 59 / T2).
func (l *Lease) RebindAt() time.Time {
return l.AcquiredAt.Add(l.T2)
}
spinifex/services/vpcd/dhcp/client_test.go
-174
View File
@@ -1,174 +0,0 @@
package dhcp_test
import (
"context"
"errors"
"net"
"testing"
"time"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
)
func TestLeaseTimers(t *testing.T) {
acq := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC)
l := &dhcp.Lease{
AcquiredAt: acq,
LeaseDuration: time.Hour,
T1: 30 * time.Minute,
T2: 52*time.Minute + 30*time.Second,
}
if got := l.RenewAt(); !got.Equal(acq.Add(30 * time.Minute)) {
t.Errorf("RenewAt: got %v, want %v", got, acq.Add(30*time.Minute))
}
if got := l.RebindAt(); !got.Equal(acq.Add(52*time.Minute + 30*time.Second)) {
t.Errorf("RebindAt: got %v, want %v", got, acq.Add(52*time.Minute+30*time.Second))
}
if got := l.ExpiresAt(); !got.Equal(acq.Add(time.Hour)) {
t.Errorf("ExpiresAt: got %v, want %v", got, acq.Add(time.Hour))
}
}
func TestFakeAcquireReturnsLeaseAndTracksCount(t *testing.T) {
f := dhcp.NewFake()
mac, _ := net.ParseMAC("02:00:00:aa:bb:cc")
lease, err := f.Acquire(context.Background(), dhcp.AcquireRequest{
Bridge: "br-wan", ClientID: "eni-1", HWAddr: mac,
})
if err != nil {
t.Fatalf("Acquire: %v", err)
}
if lease == nil || lease.IP == nil {
t.Fatalf("expected lease with IP, got %+v", lease)
}
if f.AcquireCount() != 1 {
t.Errorf("acquire count = %d, want 1", f.AcquireCount())
}
if held, ok := f.HeldLease("eni-1"); !ok || held.IP.String() != lease.IP.String() {
t.Errorf("held lease = %v/%v, want match", held, ok)
}
}
func TestFakeAcquireHookOverridesDefault(t *testing.T) {
f := dhcp.NewFake()
f.AcquireHook = func(req dhcp.AcquireRequest) (*dhcp.Lease, error) {
return nil, errors.New("injected")
}
_, err := f.Acquire(context.Background(), dhcp.AcquireRequest{Bridge: "br-wan", ClientID: "x"})
if err == nil || err.Error() != "injected" {
t.Fatalf("expected injected error, got %v", err)
}
}
func TestFakeRenewRefreshesAcquiredAt(t *testing.T) {
f := dhcp.NewFake()
lease, err := f.Acquire(context.Background(), dhcp.AcquireRequest{
Bridge: "br-wan", ClientID: "eni-2",
})
if err != nil {
t.Fatalf("Acquire: %v", err)
}
firstAcq := lease.AcquiredAt
time.Sleep(10 * time.Millisecond)
renewed, err := f.Renew(context.Background(), lease)
if err != nil {
t.Fatalf("Renew: %v", err)
}
if !renewed.AcquiredAt.After(firstAcq) {
t.Errorf("renewed AcquiredAt %v not after original %v", renewed.AcquiredAt, firstAcq)
}
if f.RenewCount() != 1 {
t.Errorf("renew count = %d, want 1", f.RenewCount())
}
}
func TestFakeReleaseClearsTrackedLease(t *testing.T) {
f := dhcp.NewFake()
lease, _ := f.Acquire(context.Background(), dhcp.AcquireRequest{
Bridge: "br-wan", ClientID: "eni-3",
})
if err := f.Release(context.Background(), lease); err != nil {
t.Fatalf("Release: %v", err)
}
if _, ok := f.HeldLease("eni-3"); ok {
t.Errorf("expected lease removed after Release")
}
if f.ReleaseCount() != 1 {
t.Errorf("release count = %d, want 1", f.ReleaseCount())
}
}
func TestFakeRenewHookSurfacesError(t *testing.T) {
f := dhcp.NewFake()
f.RenewHook = func(l *dhcp.Lease) (*dhcp.Lease, error) {
return nil, errors.New("server NAK")
}
lease, _ := f.Acquire(context.Background(), dhcp.AcquireRequest{Bridge: "br-wan", ClientID: "eni-4"})
if _, err := f.Renew(context.Background(), lease); err == nil {
t.Fatal("expected hook error")
}
}
func TestFakeReleaseHookSurfacesError(t *testing.T) {
f := dhcp.NewFake()
f.ReleaseHook = func(*dhcp.Lease) error { return errors.New("server unreachable") }
lease, _ := f.Acquire(context.Background(), dhcp.AcquireRequest{Bridge: "br-wan", ClientID: "eni-rel"})
if err := f.Release(context.Background(), lease); err == nil {
t.Fatal("expected hook error")
}
}
func TestFakeRenewNilLease(t *testing.T) {
f := dhcp.NewFake()
if _, err := f.Renew(context.Background(), nil); err == nil {
t.Fatal("expected error for nil lease")
}
}
func TestFakeReleaseNilIsNoop(t *testing.T) {
f := dhcp.NewFake()
if err := f.Release(context.Background(), nil); err != nil {
t.Fatalf("expected nil, got %v", err)
}
}
func TestNClient4ValidatesInputs(t *testing.T) {
c := dhcp.NewNClient4(0, 0) // defaults applied
if _, err := c.Acquire(context.Background(), dhcp.AcquireRequest{}); err == nil {
t.Fatal("expected bridge-required error")
}
mac, _ := net.ParseMAC("02:00:00:aa:bb:cc")
if _, err := c.Acquire(context.Background(), dhcp.AcquireRequest{Bridge: "br-wan"}); err == nil {
t.Fatal("expected hw_addr-required error")
}
_ = mac
}
func TestNClient4RenewRejectsNilAndMissingRaw(t *testing.T) {
c := dhcp.NewNClient4(0, 0)
if _, err := c.Renew(context.Background(), nil); err == nil {
t.Fatal("expected nil-lease error")
}
lease := &dhcp.Lease{Bridge: "br-wan", ClientID: "x"} // no Raw bytes
if _, err := c.Renew(context.Background(), lease); err == nil {
t.Fatal("expected missing-raw-bytes error")
}
}
func TestNClient4ReleaseNilIsNoop(t *testing.T) {
c := dhcp.NewNClient4(0, 0)
if err := c.Release(context.Background(), nil); err != nil {
t.Fatalf("expected nil, got %v", err)
}
}
func TestNClient4ReleaseRejectsMissingRaw(t *testing.T) {
c := dhcp.NewNClient4(0, 0)
lease := &dhcp.Lease{Bridge: "br-wan", ClientID: "x"}
if err := c.Release(context.Background(), lease); err == nil {
t.Fatal("expected missing-raw-bytes error")
}
}
spinifex/services/vpcd/dhcp/fake.go
-179
View File
@@ -1,179 +0,0 @@
package dhcp
import (
"context"
"fmt"
"net"
"sync"
"time"
)
// Fake is a configurable in-memory Client for unit tests. It mimics the
// DORA / RENEW / RELEASE lifecycle well enough to exercise the Manager
// without opening real sockets. Hooks let tests inject errors or custom
// responses at each step.
type Fake struct {
mu sync.Mutex
// Default template for synthesised leases. Tests can override via
// SetDefaultLease.
defaultLease LeaseTemplate
// Optional hooks — if non-nil the fake calls the hook instead of
// synthesising a response. Useful for fault injection.
AcquireHook func(AcquireRequest) (*Lease, error)
RenewHook func(*Lease) (*Lease, error)
ReleaseHook func(*Lease) error
// Tracked state.
leases map[string]*Lease // keyed by ClientID
acquireCount int
renewCount int
releaseCount int
}
var _ Client = (*Fake)(nil)
// LeaseTemplate configures the synthetic lease Fake returns from Acquire
// when no AcquireHook is set.
type LeaseTemplate struct {
IP net.IP
SubnetMask net.IPMask
Routers []net.IP
DNS []net.IP
ServerID net.IP
LeaseDuration time.Duration
}
// DefaultLeaseTemplate returns a reasonable template for tests (RFC 5737
// TEST-NET-1, 1 h lease).
func DefaultLeaseTemplate() LeaseTemplate {
return LeaseTemplate{
IP: net.IPv4(192, 0, 2, 100),
SubnetMask: net.CIDRMask(24, 32),
Routers: []net.IP{net.IPv4(192, 0, 2, 1)},
DNS: []net.IP{net.IPv4(192, 0, 2, 1)},
ServerID: net.IPv4(192, 0, 2, 1),
LeaseDuration: time.Hour,
}
}
// NewFake creates a Fake pre-loaded with DefaultLeaseTemplate.
func NewFake() *Fake {
return &Fake{
defaultLease: DefaultLeaseTemplate(),
leases: map[string]*Lease{},
}
}
// SetDefaultLease overrides the template used for subsequent Acquire calls.
func (f *Fake) SetDefaultLease(tmpl LeaseTemplate) {
f.mu.Lock()
defer f.mu.Unlock()
f.defaultLease = tmpl
}
// AcquireCount returns the number of Acquire calls observed.
func (f *Fake) AcquireCount() int { f.mu.Lock(); defer f.mu.Unlock(); return f.acquireCount }
// RenewCount returns the number of Renew calls observed.
func (f *Fake) RenewCount() int { f.mu.Lock(); defer f.mu.Unlock(); return f.renewCount }
// ReleaseCount returns the number of Release calls observed.
func (f *Fake) ReleaseCount() int { f.mu.Lock(); defer f.mu.Unlock(); return f.releaseCount }
// HeldLease returns the last lease Fake synthesised for the given ClientID.
func (f *Fake) HeldLease(clientID string) (*Lease, bool) {
f.mu.Lock()
defer f.mu.Unlock()
l, ok := f.leases[clientID]
return l, ok
}
func (f *Fake) Acquire(_ context.Context, req AcquireRequest) (*Lease, error) {
f.mu.Lock()
f.acquireCount++
hook := f.AcquireHook
tmpl := f.defaultLease
f.mu.Unlock()
if hook != nil {
lease, err := hook(req)
if err == nil && lease != nil {
f.mu.Lock()
f.leases[req.ClientID] = lease
f.mu.Unlock()
}
return lease, err
}
lease := &Lease{
Bridge: req.Bridge,
ClientID: req.ClientID,
Hostname: req.Hostname,
VendorClass: req.VendorClass,
HWAddr: append(net.HardwareAddr(nil), req.HWAddr...),
IP: tmpl.IP,
SubnetMask: tmpl.SubnetMask,
Routers: tmpl.Routers,
DNS: tmpl.DNS,
ServerID: tmpl.ServerID,
AcquiredAt: time.Now(),
LeaseDuration: tmpl.LeaseDuration,
T1: tmpl.LeaseDuration / 2,
T2: tmpl.LeaseDuration * 7 / 8,
// Synthetic raw packets — opaque marker bytes so Renew/Release
// distinguish a fake lease from a nil-bytes one.
RawOffer: []byte("fake-offer-" + req.ClientID),
RawACK: []byte("fake-ack-" + req.ClientID),
}
f.mu.Lock()
f.leases[req.ClientID] = lease
f.mu.Unlock()
return lease, nil
}
func (f *Fake) Renew(_ context.Context, lease *Lease) (*Lease, error) {
if lease == nil {
return nil, fmt.Errorf("fake renew: lease is nil")
}
f.mu.Lock()
f.renewCount++
hook := f.RenewHook
f.mu.Unlock()
if hook != nil {
renewed, err := hook(lease)
if err == nil && renewed != nil {
f.mu.Lock()
f.leases[lease.ClientID] = renewed
f.mu.Unlock()
}
return renewed, err
}
renewed := *lease
renewed.AcquiredAt = time.Now()
f.mu.Lock()
f.leases[lease.ClientID] = &renewed
f.mu.Unlock()
return &renewed, nil
}
func (f *Fake) Release(_ context.Context, lease *Lease) error {
if lease == nil {
return nil
}
f.mu.Lock()
f.releaseCount++
hook := f.ReleaseHook
f.mu.Unlock()
if hook != nil {
return hook(lease)
}
f.mu.Lock()
delete(f.leases, lease.ClientID)
f.mu.Unlock()
return nil
}
spinifex/services/vpcd/dhcp/nats.go
-73
View File
@@ -1,73 +0,0 @@
package dhcp
// NATS subjects that spinifex-vpcd's DHCP manager subscribes to and
// publishes on. The daemon-side handlers in handlers/ec2/vpc send
// requests against the request/reply subjects.
const (
// TopicAcquire is the request/reply subject for obtaining a lease.
// Request: AcquireRequestMsg. Reply: AcquireReplyMsg.
TopicAcquire = "vpc.dhcp.acquire"
// TopicRelease is the request/reply subject for releasing a lease.
// Request: ReleaseRequestMsg. Reply: ReleaseReplyMsg.
TopicRelease = "vpc.dhcp.release"
// TopicLeaseExpired is a fan-out event published by vpcd when a
// renewal permanently fails and the lease has passed its expiry.
// Consumers (EIP / instance / VPC reconcilers) decide whether to
// re-request a lease, reschedule, or fail the pending API call.
TopicLeaseExpired = "vpc.dhcp.lease-expired"
)
// KV bucket for persisted lease state. Keyed by ClientID. Separate from
// the external IPAM allocation ledger by design: IPAM records who owns
// which IP, this bucket records when a lease expires and how to renew it.
const (
KVBucketDHCPLeases = "spinifex-dhcp-leases"
KVBucketDHCPLeasesVersion = 1
)
// AcquireRequestMsg is the NATS payload for TopicAcquire.
type AcquireRequestMsg struct {
Bridge string `json:"bridge"`
ClientID string `json:"client_id"`
Hostname string `json:"hostname"`
VendorClass string `json:"vendor_class"`
// HWAddr is the MAC to put in the DHCP payload chaddr, formatted as
// 02:aa:bb:cc:dd:ee. Empty means the vpcd Manager derives a synthetic
// MAC from ClientID.
HWAddr string `json:"hw_addr,omitempty"`
PoolName string `json:"pool_name,omitempty"`
}
// AcquireReplyMsg is the NATS reply for TopicAcquire. Error is the only
// field set on failure.
type AcquireReplyMsg struct {
IP string `json:"ip,omitempty"`
SubnetMask string `json:"subnet_mask,omitempty"`
Routers []string `json:"routers,omitempty"`
DNS []string `json:"dns,omitempty"`
ServerID string `json:"server_id,omitempty"`
HWAddr string `json:"hw_addr,omitempty"`
ExpiresUnix int64 `json:"expires_unix,omitempty"`
Error string `json:"error,omitempty"`
}
// ReleaseRequestMsg is the NATS payload for TopicRelease. ClientID must
// match the identifier the lease was acquired with.
type ReleaseRequestMsg struct {
ClientID string `json:"client_id"`
}
// ReleaseReplyMsg is the NATS reply for TopicRelease.
type ReleaseReplyMsg struct {
Error string `json:"error,omitempty"`
}
// LeaseExpiredEvent is the payload fan-out on TopicLeaseExpired.
type LeaseExpiredEvent struct {
ClientID string `json:"client_id"`
PoolName string `json:"pool_name,omitempty"`
IP string `json:"ip,omitempty"`
// Reason is a short machine-readable code (e.g. "renew_failed",
// "nak", "server_unreachable").
Reason string `json:"reason"`
}
spinifex/services/vpcd/dhcp/nats_client.go
-151
View File
@@ -1,151 +0,0 @@
package dhcp
import (
"encoding/json"
"fmt"
"log/slog"
"time"
"github.com/nats-io/nats.go"
)
// NATSTimeout bounds the request/reply round-trip to vpcd's DHCPManager.
// Picked to comfortably exceed the DORA handshake (15 s default + server
// jitter + ~10 s of slack for NAK + fresh DISCOVER fallbacks). A var rather
// than a const so tests can drive the timeout error path without waiting
// 20 s per table entry.
var NATSTimeout = 30 * time.Second
// AcquireMaxAttempts caps retry of vpc.dhcp.acquire. 60 attempts × 1 s sleep
// covers a cold-boot vpcd start window where NATS is up but the dhcp.Manager
// hasn't yet subscribed (typical: 5–30 s, pathological: 60 s during full
// reconcile). Tests override to keep timeout-path coverage fast.
var AcquireMaxAttempts = 60
// AcquireRetryDelay is the wait between retries. Var for the same reason as
// AcquireMaxAttempts.
var AcquireRetryDelay = 1 * time.Second
// LeaseResult is the data callers need from a successful acquire. Mirrors
// AcquireReplyMsg but exposed as a typed result so callers don't depend on
// the wire struct.
type LeaseResult struct {
IP string
SubnetMask string
Routers []string
DNS []string
ServerID string
HWAddr string
ExpiresUnix int64
}
// RequestAcquire asks vpcd's DHCPManager (over NATS) to acquire a DHCP lease
// on the given bridge, identifying the caller with option 61 (client-id),
// option 12 (hostname) and option 60 (vendor class). Blocks until vpcd
// replies or NATSTimeout expires. The Manager-side handler is idempotent on
// clientID: a second call with the same clientID while a live lease exists
// returns the same lease without a fresh DORA, so CAS retry loops on the
// caller are safe.
func RequestAcquire(nc *nats.Conn, bridge, clientID, hostname, vendorClass, poolName, hwAddr string) (LeaseResult, error) {
if nc == nil {
return LeaseResult{}, fmt.Errorf("DHCP lease: NATS connection is required")
}
if bridge == "" {
return LeaseResult{}, fmt.Errorf("DHCP lease: bridge name is required")
}
if clientID == "" {
return LeaseResult{}, fmt.Errorf("DHCP lease: client ID is required")
}
req := AcquireRequestMsg{
Bridge: bridge,
ClientID: clientID,
Hostname: hostname,
VendorClass: vendorClass,
PoolName: poolName,
HWAddr: hwAddr,
}
data, err := json.Marshal(req)
if err != nil {
return LeaseResult{}, fmt.Errorf("marshal dhcp acquire request: %w", err)
}
var reply AcquireReplyMsg
maxAttempts := AcquireMaxAttempts
for attempt := 1; attempt <= maxAttempts; attempt++ {
msg, err := nc.Request(TopicAcquire, data, NATSTimeout)
if err != nil {
if attempt == maxAttempts {
return LeaseResult{}, fmt.Errorf(
"dhcp acquire NATS request failed after %d attempts (client %s): %w",
maxAttempts, clientID, err,
)
}
slog.Warn("dhcp acquire NATS request failed, retrying",
"clientID", clientID,
"attempt", attempt,
"maxAttempts", maxAttempts,
"err", err,
)
time.Sleep(AcquireRetryDelay)
continue
}
if err := json.Unmarshal(msg.Data, &reply); err != nil {
return LeaseResult{}, fmt.Errorf("unmarshal dhcp acquire reply: %w", err)
}
if reply.Error != "" {
return LeaseResult{}, fmt.Errorf("dhcp acquire client %s: %s", clientID, reply.Error)
}
break
}
slog.Info("DHCP lease obtained",
"bridge", bridge,
"client_id", clientID,
"ip", reply.IP,
"server_id", reply.ServerID,
"expires_unix", reply.ExpiresUnix,
)
return LeaseResult{
IP: reply.IP,
SubnetMask: reply.SubnetMask,
Routers: reply.Routers,
DNS: reply.DNS,
ServerID: reply.ServerID,
HWAddr: reply.HWAddr,
ExpiresUnix: reply.ExpiresUnix,
}, nil
}
// RequestRelease asks vpcd to release the lease identified by clientID.
// Returns nil silently when nc or clientID is empty so callers in clean-up
// paths can invoke it unconditionally.
func RequestRelease(nc *nats.Conn, clientID string) error {
if nc == nil || clientID == "" {
return nil
}
data, err := json.Marshal(ReleaseRequestMsg{ClientID: clientID})
if err != nil {
return fmt.Errorf("marshal dhcp release request: %w", err)
}
msg, err := nc.Request(TopicRelease, data, NATSTimeout)
if err != nil {
return fmt.Errorf("dhcp release NATS request (client %s): %w", clientID, err)
}
var reply ReleaseReplyMsg
if err := json.Unmarshal(msg.Data, &reply); err != nil {
return fmt.Errorf("unmarshal dhcp release reply: %w", err)
}
if reply.Error != "" {
return fmt.Errorf("dhcp release (client %s): %s", clientID, reply.Error)
}
slog.Info("DHCP lease released", "client_id", clientID)
return nil
}
spinifex/services/vpcd/dhcp/nats_client_test.go
-104
View File
@@ -1,104 +0,0 @@
package dhcp
import (
"encoding/json"
"testing"
"time"
"github.com/mulgadc/spinifex/spinifex/testutil"
"github.com/nats-io/nats.go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestRequestAcquire_RequestTimeout(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
sub, err := nc.Subscribe(TopicAcquire, func(*nats.Msg) {})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
prev := NATSTimeout
NATSTimeout = 150 * time.Millisecond
t.Cleanup(func() { NATSTimeout = prev })
prevMax := AcquireMaxAttempts
prevDelay := AcquireRetryDelay
AcquireMaxAttempts = 2
AcquireRetryDelay = 10 * time.Millisecond
t.Cleanup(func() {
AcquireMaxAttempts = prevMax
AcquireRetryDelay = prevDelay
})
_, err = RequestAcquire(nc, "br-wan", "eni-timeout", "eni-timeout", "mulga-spinifex", "wan", "")
assert.ErrorContains(t, err, "dhcp acquire NATS request")
}
func TestRequestRelease_RequestTimeout(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
sub, err := nc.Subscribe(TopicRelease, func(*nats.Msg) {})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
prev := NATSTimeout
NATSTimeout = 150 * time.Millisecond
t.Cleanup(func() { NATSTimeout = prev })
err = RequestRelease(nc, "eni-timeout")
assert.ErrorContains(t, err, "dhcp release NATS request")
}
func TestRequestAcquire_MalformedReply(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
sub, err := nc.Subscribe(TopicAcquire, func(msg *nats.Msg) {
_ = msg.Respond([]byte("not json"))
})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
_, err = RequestAcquire(nc, "br-wan", "eni-malformed", "eni-malformed", "mulga-spinifex", "wan", "")
assert.ErrorContains(t, err, "unmarshal dhcp acquire reply")
}
func TestRequestRelease_MalformedReply(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
sub, err := nc.Subscribe(TopicRelease, func(msg *nats.Msg) {
_ = msg.Respond([]byte("not json"))
})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
err = RequestRelease(nc, "eni-malformed")
assert.ErrorContains(t, err, "unmarshal dhcp release reply")
}
func TestRequestRelease_ReplyError(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
sub, err := nc.Subscribe(TopicRelease, func(msg *nats.Msg) {
data, _ := json.Marshal(ReleaseReplyMsg{Error: "vpcd rejected release"})
_ = msg.Respond(data)
})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
err = RequestRelease(nc, "eni-rejected")
assert.ErrorContains(t, err, "vpcd rejected release")
}
func TestRequestAcquire_Guards(t *testing.T) {
_, err := RequestAcquire(nil, "br-wan", "eni-1", "eni-1", "mulga-spinifex", "wan", "")
assert.ErrorContains(t, err, "NATS connection is required")
_, nc := testutil.StartTestNATS(t)
_, err = RequestAcquire(nc, "", "eni-1", "eni-1", "mulga-spinifex", "wan", "")
assert.ErrorContains(t, err, "bridge name is required")
_, err = RequestAcquire(nc, "br-wan", "", "eni-1", "mulga-spinifex", "wan", "")
assert.ErrorContains(t, err, "client ID is required")
}
func TestRequestRelease_NoopWhenNilOrEmpty(t *testing.T) {
assert.NoError(t, RequestRelease(nil, "eni-1"))
_, nc := testutil.StartTestNATS(t)
assert.NoError(t, RequestRelease(nc, ""))
}
spinifex/services/vpcd/dhcp/nclient4.go
-188
View File
@@ -1,188 +0,0 @@
package dhcp
import (
"context"
"fmt"
"time"
"github.com/insomniacslk/dhcp/dhcpv4"
"github.com/insomniacslk/dhcp/dhcpv4/nclient4"
)
// 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
// handshake and closes it when done — no long-lived per-lease process.
//
// Retransmission lives one layer up in DHCPManager.acquireWithBackoff
// (RFC 2131 §4.1). This client always does exactly one DISCOVER attempt;
// the per-attempt window is whatever the caller's context allows.
type NClient4Client struct {
timeout time.Duration
}
var _ Client = (*NClient4Client)(nil)
// NewNClient4 creates an NClient4Client. timeout is the nclient4-internal
// socket read deadline used as a safety net when the caller supplies no
// context deadline; in production DHCPManager always wraps each call in a
// per-attempt ctx so this is rarely hit. The retry argument is accepted
// for backwards compat and ignored — DHCPManager.acquireWithBackoff owns
// retransmission.
func NewNClient4(timeout time.Duration, _ int) *NClient4Client {
if timeout <= 0 {
timeout = 60 * time.Second
}
return &NClient4Client{timeout: timeout}
}
func (c *NClient4Client) Acquire(ctx context.Context, req AcquireRequest) (*Lease, error) {
if req.Bridge == "" {
return nil, fmt.Errorf("dhcp acquire: bridge is required")
}
if len(req.HWAddr) == 0 {
return nil, fmt.Errorf("dhcp acquire: hw_addr is required")
}
client, err := nclient4.New(req.Bridge,
nclient4.WithHWAddr(req.HWAddr),
nclient4.WithTimeout(c.timeout),
// Single attempt per call — DHCPManager.acquireWithBackoff drives
// retransmission per RFC 2131 §4.1.
nclient4.WithRetry(1),
)
if err != nil {
return nil, fmt.Errorf("open nclient4 on %s: %w", req.Bridge, err)
}
defer func() { _ = client.Close() }()
// Without the broadcast flag, the server sends a unicast OFFER to the
// generated chaddr MAC. The physical NIC drops it in hardware (not its MAC),
// so the AF_PACKET socket on the bridge never sees the frame. Setting the
// broadcast flag forces the server to respond with ff:ff:ff:ff:ff:ff, which
// all NICs accept unconditionally.
mods := append(identityModifiers(req.ClientID, req.Hostname, req.VendorClass), dhcpv4.WithBroadcast(true))
lease, err := client.Request(ctx, mods...)
if err != nil {
return nil, fmt.Errorf("dhcp DORA on %s (client=%s): %w", req.Bridge, req.ClientID, err)
}
return leaseFromNClient4(req, lease), nil
}
func (c *NClient4Client) Renew(ctx context.Context, lease *Lease) (*Lease, error) {
if lease == nil {
return nil, fmt.Errorf("dhcp renew: lease is nil")
}
nclient4Lease, err := reconstructNClient4Lease(lease)
if err != nil {
return nil, fmt.Errorf("dhcp renew: %w", err)
}
client, err := nclient4.New(lease.Bridge,
nclient4.WithHWAddr(lease.HWAddr),
nclient4.WithTimeout(c.timeout),
nclient4.WithRetry(1),
)
if err != nil {
return nil, fmt.Errorf("open nclient4 on %s for renew: %w", lease.Bridge, err)
}
defer func() { _ = client.Close() }()
renewed, err := client.Renew(ctx, nclient4Lease,
identityModifiers(lease.ClientID, lease.Hostname, lease.VendorClass)...)
if err != nil {
return nil, fmt.Errorf("dhcp renew on %s (client=%s): %w", lease.Bridge, lease.ClientID, err)
}
return leaseFromNClient4(AcquireRequest{
Bridge: lease.Bridge,
ClientID: lease.ClientID,
Hostname: lease.Hostname,
VendorClass: lease.VendorClass,
HWAddr: lease.HWAddr,
}, renewed), nil
}
func (c *NClient4Client) Release(ctx context.Context, lease *Lease) error {
if lease == nil {
return nil
}
nclient4Lease, err := reconstructNClient4Lease(lease)
if err != nil {
return fmt.Errorf("dhcp release: %w", err)
}
client, err := nclient4.New(lease.Bridge,
nclient4.WithHWAddr(lease.HWAddr),
nclient4.WithTimeout(c.timeout),
)
if err != nil {
return fmt.Errorf("open nclient4 on %s for release: %w", lease.Bridge, err)
}
defer func() { _ = client.Close() }()
if err := client.Release(nclient4Lease,
dhcpv4.WithOption(dhcpv4.OptClientIdentifier([]byte(lease.ClientID)))); err != nil {
return fmt.Errorf("dhcp release on %s (client=%s): %w", lease.Bridge, lease.ClientID, err)
}
return nil
}
// identityModifiers builds the three identifying DHCP options we set on
// every outbound message: option 61 (client-id), option 12 (hostname),
// option 60 (vendor class).
func identityModifiers(clientID, hostname, vendorClass string) []dhcpv4.Modifier {
var mods []dhcpv4.Modifier
if clientID != "" {
mods = append(mods, dhcpv4.WithOption(dhcpv4.OptClientIdentifier([]byte(clientID))))
}
if hostname != "" {
mods = append(mods, dhcpv4.WithOption(dhcpv4.OptHostName(hostname)))
}
if vendorClass != "" {
mods = append(mods, dhcpv4.WithOption(dhcpv4.OptClassIdentifier(vendorClass)))
}
return mods
}
func leaseFromNClient4(req AcquireRequest, in *nclient4.Lease) *Lease {
ack := in.ACK
leaseTime := ack.IPAddressLeaseTime(24 * time.Hour)
return &Lease{
Bridge: req.Bridge,
ClientID: req.ClientID,
Hostname: req.Hostname,
VendorClass: req.VendorClass,
HWAddr: req.HWAddr,
IP: ack.YourIPAddr,
SubnetMask: ack.SubnetMask(),
Routers: ack.Router(),
DNS: ack.DNS(),
ServerID: ack.ServerIdentifier(),
AcquiredAt: in.CreationTime,
LeaseDuration: leaseTime,
T1: ack.IPAddressRenewalTime(leaseTime / 2),
T2: ack.IPAddressRebindingTime(leaseTime * 7 / 8),
RawOffer: in.Offer.ToBytes(),
RawACK: ack.ToBytes(),
}
}
func reconstructNClient4Lease(l *Lease) (*nclient4.Lease, error) {
if len(l.RawOffer) == 0 || len(l.RawACK) == 0 {
return nil, fmt.Errorf("lease is missing raw offer/ack bytes; renewal/release not possible")
}
offer, err := dhcpv4.FromBytes(l.RawOffer)
if err != nil {
return nil, fmt.Errorf("parse stored offer: %w", err)
}
ack, err := dhcpv4.FromBytes(l.RawACK)
if err != nil {
return nil, fmt.Errorf("parse stored ack: %w", err)
}
return &nclient4.Lease{
Offer: offer,
ACK: ack,
CreationTime: l.AcquiredAt,
}, nil
}
spinifex/services/vpcd/dhcp_manager.go
-865
View File
@@ -1,865 +0,0 @@
package vpcd
import (
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
"math/rand/v2"
"net"
"strings"
"sync"
"time"
"github.com/mulgadc/spinifex/spinifex/migrate"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go"
)
// DHCPManager owns the active DHCP leases held by spinifex-vpcd. It
// services vpc.dhcp.acquire / vpc.dhcp.release NATS requests from the
// daemon-side ExternalIPAM handlers, runs a per-lease renewal goroutine,
// and persists each lease in the spinifex-dhcp-leases KV bucket so that
// a vpcd restart does not forget what has already been allocated upstream.
type DHCPManager struct {
client dhcp.Client
kv nats.KeyValue
nc *nats.Conn
// macForClientID produces the chaddr for a request that came in with
// HWAddr unset. Defaults to generateMAC; tests override.
macForClientID func(clientID string) net.HardwareAddr
// acquireTimeout is the wallclock budget for a full Acquire — sum of
// every retransmit attempt in acquireBackoff plus slack. Renew/Release
// reuse this as their single-attempt deadline.
acquireTimeout time.Duration
// acquireBackoff drives the per-DISCOVER timeouts used by
// acquireWithBackoff (RFC 2131 §4.1 retransmission). Each entry is
// the wait-for-OFFER window for one attempt; the next entry doubles.
// Default: 4s, 8s, 16s, 32s. ±1s jitter applied per attempt.
acquireBackoff []time.Duration
// jitterFraction is the ± range applied to renewal sleeps, expressed
// as a fraction of T1 (0.1 = ±10%).
jitterFraction float64
rng *rand.Rand
// promiscOn/Off are called to toggle IFF_PROMISC on the DHCP bridge
// around each Acquire. nil = no-op (non-veth modes, or tests).
// promiscCount tracks in-flight acquires so the bridge is not cleared
// while a concurrent DORA is still waiting for a reply.
promiscOn func()
promiscOff func()
promiscMu sync.Mutex
promiscCount int
mu sync.Mutex
leases map[string]*managedLease // keyed by ClientID
// inFlightMu guards inFlight. Tracks concurrent Acquire calls per
// bridge so handleAcquire can surface contention as a diagnostic
// signal (mulga-siv-32).
inFlightMu sync.Mutex
inFlight map[string]int
stopCtx context.Context
stopCancel context.CancelFunc
wg sync.WaitGroup
}
type managedLease struct {
lease *dhcp.Lease
renewStop context.CancelFunc
renewDone chan struct{}
}
// DHCPManagerOption configures DHCPManager at construction.
type DHCPManagerOption func(*DHCPManager)
// WithDHCPMACFunc overrides the default MAC derivation (generateMAC).
func WithDHCPMACFunc(fn func(clientID string) net.HardwareAddr) DHCPManagerOption {
return func(m *DHCPManager) { m.macForClientID = fn }
}
// WithDHCPAcquireTimeout overrides the wallclock budget for a full Acquire
// (sum of every retransmit attempt + slack). Renew/Release reuse it.
func WithDHCPAcquireTimeout(d time.Duration) DHCPManagerOption {
return func(m *DHCPManager) { m.acquireTimeout = d }
}
// WithDHCPAcquireBackoff overrides the per-DISCOVER timeout schedule used
// by acquireWithBackoff. Tests use a tight schedule; production keeps the
// RFC 2131 §4.1 default (4s, 8s, 16s, 32s with ±1s jitter).
func WithDHCPAcquireBackoff(b []time.Duration) DHCPManagerOption {
return func(m *DHCPManager) { m.acquireBackoff = b }
}
// WithDHCPJitterFraction overrides the ±fraction applied to renewal sleeps.
func WithDHCPJitterFraction(f float64) DHCPManagerOption {
return func(m *DHCPManager) { m.jitterFraction = f }
}
// WithDHCPRand overrides the random source used for jitter (tests only).
func WithDHCPRand(r *rand.Rand) DHCPManagerOption {
return func(m *DHCPManager) { m.rng = r }
}
// WithDHCPPromiscFuncs wires IFF_PROMISC toggling around each Acquire. on is
// called before the DORA and off is called after (even on error). Both are
// reference-counted so concurrent acquires keep the bridge promisc until the
// last one completes. Pass nil to disable (default).
func WithDHCPPromiscFuncs(on, off func()) DHCPManagerOption {
return func(m *DHCPManager) {
m.promiscOn = on
m.promiscOff = off
}
}
// NewDHCPManager creates a Manager wired against js for KV persistence and
// nc for NATS request/reply. The caller still needs to call Bootstrap (to
// rehydrate persisted leases) and Subscribe (to start servicing requests).
func NewDHCPManager(nc *nats.Conn, js nats.JetStreamContext, client dhcp.Client, opts ...DHCPManagerOption) (*DHCPManager, error) {
if nc == nil {
return nil, fmt.Errorf("DHCPManager: nats.Conn is required")
}
if js == nil {
return nil, fmt.Errorf("DHCPManager: JetStreamContext is required")
}
if client == nil {
return nil, fmt.Errorf("DHCPManager: dhcp.Client is required")
}
kv, err := utils.GetOrCreateKVBucket(js, dhcp.KVBucketDHCPLeases, 3)
if err != nil {
return nil, fmt.Errorf("create DHCP lease KV bucket: %w", err)
}
if err := migrate.DefaultRegistry.RunKV(dhcp.KVBucketDHCPLeases, kv, dhcp.KVBucketDHCPLeasesVersion); err != nil {
return nil, fmt.Errorf("migrate %s: %w", dhcp.KVBucketDHCPLeases, err)
}
ctx, cancel := context.WithCancel(context.Background())
m := &DHCPManager{
client: client,
kv: kv,
nc: nc,
// 90s = sum of default acquireBackoff (4+8+16+32 = 60s) + slack for
// socket open/close on each attempt and the final ACK round-trip.
acquireTimeout: 90 * time.Second,
acquireBackoff: []time.Duration{
4 * time.Second,
8 * time.Second,
16 * time.Second,
32 * time.Second,
},
jitterFraction: 0.1,
rng: rand.New(rand.NewPCG(uint64(time.Now().UnixNano()), 0xdeadbeef)), //nolint:gosec // non-cryptographic jitter
leases: map[string]*managedLease{},
inFlight: map[string]int{},
stopCtx: ctx,
stopCancel: cancel,
macForClientID: func(id string) net.HardwareAddr {
mac, err := net.ParseMAC(generateMAC(id))
if err != nil {
return nil
}
return mac
},
}
for _, opt := range opts {
opt(m)
}
return m, nil
}
// Bootstrap reads every persisted lease from KV and schedules the
// renewal goroutine. Leases already past their expiry are removed and
// a TopicLeaseExpired event is published so the reconcilers upstream
// know they need to re-request.
func (m *DHCPManager) Bootstrap(ctx context.Context) error {
keys, err := m.kv.Keys()
if err != nil {
if errors.Is(err, nats.ErrNoKeysFound) {
slog.Info("DHCP Manager bootstrapped", "leases", 0)
return nil
}
return fmt.Errorf("list DHCP lease keys: %w", err)
}
now := time.Now()
resumed, expired := 0, 0
for _, key := range keys {
if key == "_version" {
continue
}
entry, err := m.kv.Get(key)
if err != nil {
slog.Warn("DHCP Manager: failed to load lease", "client_id", key, "err", err)
continue
}
lease, err := decodeLease(entry.Value())
if err != nil {
slog.Warn("DHCP Manager: failed to decode lease", "client_id", key, "err", err)
continue
}
if now.After(lease.ExpiresAt()) {
_ = m.kv.Delete(key)
m.publishExpired(lease, "", "bootstrap_past_expiry")
expired++
continue
}
m.trackAndRenew(lease)
resumed++
}
slog.Info("DHCP Manager bootstrapped", "leases", resumed, "expired", expired)
return nil
}
// Subscribe registers queue-subscribed handlers on vpc.dhcp.acquire and
// vpc.dhcp.release. Queue group "vpcd-dhcp-workers" matches the existing
// vpcd-workers convention.
func (m *DHCPManager) Subscribe(nc *nats.Conn) ([]*nats.Subscription, error) {
var subs []*nats.Subscription
acquire, err := nc.QueueSubscribe(dhcp.TopicAcquire, "vpcd-dhcp-workers", m.handleAcquire)
if err != nil {
return nil, fmt.Errorf("subscribe %s: %w", dhcp.TopicAcquire, err)
}
subs = append(subs, acquire)
slog.Info("DHCP Manager subscribed", "topic", dhcp.TopicAcquire)
release, err := nc.QueueSubscribe(dhcp.TopicRelease, "vpcd-dhcp-workers", m.handleRelease)
if err != nil {
_ = acquire.Unsubscribe()
return nil, fmt.Errorf("subscribe %s: %w", dhcp.TopicRelease, err)
}
subs = append(subs, release)
slog.Info("DHCP Manager subscribed", "topic", dhcp.TopicRelease)
return subs, nil
}
// Close stops all renewal goroutines and waits for them to exit.
func (m *DHCPManager) Close() {
m.stopCancel()
m.wg.Wait()
}
// handleAcquire processes a vpc.dhcp.acquire request. Idempotent: if we
// already hold a valid lease for the requested ClientID we return it
// without a fresh DORA. Lets the daemon-side CAS retry loop be safe.
func (m *DHCPManager) handleAcquire(msg *nats.Msg) {
var req dhcp.AcquireRequestMsg
if err := json.Unmarshal(msg.Data, &req); err != nil {
m.replyAcquireError(msg, fmt.Sprintf("invalid acquire request: %v", err))
return
}
if req.Bridge == "" || req.ClientID == "" {
m.replyAcquireError(msg, "acquire: bridge and client_id are required")
return
}
if existing, ok := m.get(req.ClientID); ok {
if time.Now().Before(existing.ExpiresAt()) {
m.replyAcquire(msg, existing)
return
}
m.forget(req.ClientID, "stale_existing")
}
hwAddr, err := resolveHWAddr(req.HWAddr, req.ClientID, m.macForClientID)
if err != nil {
m.replyAcquireError(msg, err.Error())
return
}
m.enablePromisc()
defer m.disablePromisc()
ctx, cancel := context.WithTimeout(m.stopCtx, m.acquireTimeout)
defer cancel()
// Diagnostic for mulga-siv-32: track concurrent Acquire calls per
// bridge and the wallclock cost of each DORA. Concurrent DISCOVERs on
// the same bridge are a suspected contributor to nightly DORA timeouts.
inFlight := m.beginAcquire(req.Bridge)
defer m.endAcquire(req.Bridge)
start := time.Now()
lease, err := m.acquireWithBackoff(ctx, dhcp.AcquireRequest{
Bridge: req.Bridge,
ClientID: req.ClientID,
Hostname: req.Hostname,
VendorClass: req.VendorClass,
HWAddr: hwAddr,
})
if err != nil {
slog.Warn("DHCP Manager: acquire failed",
"client_id", req.ClientID, "bridge", req.Bridge,
"in_flight_at_start", inFlight, "duration", time.Since(start),
"err", err)
m.replyAcquireError(msg, err.Error())
return
}
slog.Debug("DHCP Manager: acquire ok",
"client_id", req.ClientID, "bridge", req.Bridge,
"in_flight_at_start", inFlight, "duration", time.Since(start))
if err := m.persistLease(lease); err != nil {
slog.Warn("DHCP Manager: persist lease failed; lease will not survive restart", "client_id", req.ClientID, "err", err)
}
m.trackAndRenew(lease)
slog.Info("DHCP Manager acquired lease",
"client_id", lease.ClientID, "ip", lease.IP.String(),
"bridge", lease.Bridge, "server_id", lease.ServerID.String(),
"expires_in", time.Until(lease.ExpiresAt()))
m.replyAcquire(msg, lease)
}
// handleRelease processes a vpc.dhcp.release request.
func (m *DHCPManager) handleRelease(msg *nats.Msg) {
var req dhcp.ReleaseRequestMsg
if err := json.Unmarshal(msg.Data, &req); err != nil {
m.replyRelease(msg, fmt.Sprintf("invalid release request: %v", err))
return
}
if req.ClientID == "" {
m.replyRelease(msg, "release: client_id is required")
return
}
lease, ok := m.get(req.ClientID)
if !ok {
// No active lease; treat as a no-op so the daemon can call release
// in clean-up paths without worrying about order.
m.replyRelease(msg, "")
return
}
ctx, cancel := context.WithTimeout(m.stopCtx, m.acquireTimeout)
defer cancel()
if err := m.client.Release(ctx, lease); err != nil {
slog.Warn("DHCP Manager: release failed", "client_id", req.ClientID, "err", err)
// Still forget locally — upstream server may or may not have seen
// the RELEASE, but holding on to a lease we've been told to drop
// serves no purpose.
}
m.forget(req.ClientID, "released")
slog.Info("DHCP Manager released lease", "client_id", req.ClientID)
m.replyRelease(msg, "")
}
// enablePromisc increments the in-flight acquire count and enables IFF_PROMISC
// on the first call.
func (m *DHCPManager) enablePromisc() {
if m.promiscOn == nil {
return
}
m.promiscMu.Lock()
defer m.promiscMu.Unlock()
if m.promiscCount == 0 {
m.promiscOn()
}
m.promiscCount++
}
// disablePromisc decrements the count and disables IFF_PROMISC when it reaches
// zero (i.e. no more in-flight acquires need it).
func (m *DHCPManager) disablePromisc() {
if m.promiscOff == nil {
return
}
m.promiscMu.Lock()
defer m.promiscMu.Unlock()
m.promiscCount--
if m.promiscCount == 0 {
m.promiscOff()
}
}
// trackAndRenew registers the lease in the in-memory map and starts the
// per-lease renewal goroutine. Replaces any prior tracker for the same
// ClientID.
func (m *DHCPManager) trackAndRenew(lease *dhcp.Lease) {
m.mu.Lock()
if prev, ok := m.leases[lease.ClientID]; ok && prev.renewStop != nil {
prev.renewStop()
m.mu.Unlock()
<-prev.renewDone
m.mu.Lock()
}
// cancel is stored in tracker.renewStop and fired from forget() or Close().
ctx, cancel := context.WithCancel(m.stopCtx)
tracker := &managedLease{
lease: lease,
renewStop: cancel,
renewDone: make(chan struct{}),
}
m.leases[lease.ClientID] = tracker
m.mu.Unlock()
m.wg.Add(1)
go m.renewLoop(ctx, tracker)
}
// renewLoop drives one lease's renewal lifecycle. Sleeps until T1 (with
// ±jitter), attempts Renew; on failure retries at T2; on terminal failure
// past the expiry, publishes TopicLeaseExpired and forgets the lease.
func (m *DHCPManager) renewLoop(ctx context.Context, tracker *managedLease) {
defer m.wg.Done()
defer close(tracker.renewDone)
for {
lease := m.snapshotLease(tracker)
if lease == nil {
return
}
// Phase 1 — wait for T1 (± jitter) and try RENEW.
wait := m.jitter(time.Until(lease.RenewAt()))
if !sleep(ctx, wait) {
return
}
renewed, err := m.tryRenew(ctx, lease)
if err == nil {
tracker.lease = renewed
if perr := m.persistLease(renewed); perr != nil {
slog.Warn("DHCP Manager: persist renewed lease failed", "client_id", renewed.ClientID, "err", perr)
}
continue
}
slog.Warn("DHCP Manager: renew failed, will retry at T2", "client_id", lease.ClientID, "err", err)
// Phase 2 — retry until T2.
wait = m.jitter(time.Until(lease.RebindAt()))
if !sleep(ctx, wait) {
return
}
renewed, err = m.tryRenew(ctx, lease)
if err == nil {
tracker.lease = renewed
if perr := m.persistLease(renewed); perr != nil {
slog.Warn("DHCP Manager: persist rebound lease failed", "client_id", renewed.ClientID, "err", perr)
}
continue
}
// Phase 3 — wait out the rest of the lease; on expiry, emit
// lease-expired and drop.
wait = time.Until(lease.ExpiresAt())
if !sleep(ctx, wait) {
return
}
slog.Warn("DHCP Manager: lease expired without renewal", "client_id", lease.ClientID, "ip", lease.IP.String(), "err", err)
m.publishExpired(lease, "", classifyRenewErr(err))
m.forget(lease.ClientID, "expired")
return
}
}
// acquireWithBackoff drives RFC 2131 §4.1 DISCOVER retransmission. Each
// entry in acquireBackoff sets one attempt's per-OFFER wait window with
// ±1s jitter; on timeout the loop tries the next entry. Returns on first
// success, on parent ctx cancellation, or after the schedule is exhausted.
//
// Real DHCP servers (consumer routers, ISC dhcpd, dnsmasq) silently drop
// OFFERs under load — without retransmission a single lost packet kills
// the whole DORA. mulga-siv-39.
func (m *DHCPManager) acquireWithBackoff(parent context.Context, req dhcp.AcquireRequest) (*dhcp.Lease, error) {
schedule := m.acquireBackoff
if len(schedule) == 0 {
// Defensive: caller passed an empty schedule via WithDHCPAcquireBackoff.
// Fall through to a single attempt bounded by parent ctx.
schedule = []time.Duration{0}
}
var lastErr error
for i, base := range schedule {
if err := parent.Err(); err != nil {
if lastErr != nil {
return nil, fmt.Errorf("dhcp acquire on %s (client=%s): %w (last attempt err: %v)", req.Bridge, req.ClientID, err, lastErr)
}
return nil, fmt.Errorf("dhcp acquire on %s (client=%s): %w", req.Bridge, req.ClientID, err)
}
attemptTimeout := m.dhcpAttemptTimeout(base)
var (
ctx context.Context
cancel context.CancelFunc
)
if attemptTimeout > 0 {
ctx, cancel = context.WithTimeout(parent, attemptTimeout)
} else {
ctx, cancel = context.WithCancel(parent)
}
lease, err := m.client.Acquire(ctx, req)
cancel()
if err == nil {
return lease, nil
}
lastErr = err
if i < len(schedule)-1 {
slog.Debug("DHCP Manager: DORA retransmit",
"client_id", req.ClientID, "bridge", req.Bridge,
"attempt", i+1, "next_timeout", m.dhcpAttemptTimeout(schedule[i+1]),
"prev_err", err)
}
}
return nil, lastErr
}
// dhcpAttemptTimeout applies ±1s jitter to a base DISCOVER timeout.
// Pass 0 to disable timeout (used when schedule is empty fallback).
func (m *DHCPManager) dhcpAttemptTimeout(base time.Duration) time.Duration {
if base <= 0 {
return 0
}
jitter := time.Duration((m.rng.Float64()*2 - 1) * float64(time.Second))
out := base + jitter
if out <= 0 {
return base
}
return out
}
// tryRenew calls the backend Renew under a bounded context.
func (m *DHCPManager) tryRenew(parent context.Context, lease *dhcp.Lease) (*dhcp.Lease, error) {
ctx, cancel := context.WithTimeout(parent, m.acquireTimeout)
defer cancel()
return m.client.Renew(ctx, lease)
}
// beginAcquire increments the in-flight Acquire counter for bridge and
// returns the count after increment (>= 1). Diagnostic for mulga-siv-32.
func (m *DHCPManager) beginAcquire(bridge string) int {
m.inFlightMu.Lock()
defer m.inFlightMu.Unlock()
m.inFlight[bridge]++
return m.inFlight[bridge]
}
// endAcquire decrements the in-flight Acquire counter for bridge.
func (m *DHCPManager) endAcquire(bridge string) {
m.inFlightMu.Lock()
defer m.inFlightMu.Unlock()
if m.inFlight[bridge] > 0 {
m.inFlight[bridge]--
}
}
// snapshotLease returns the tracker's current lease under the lock.
func (m *DHCPManager) snapshotLease(tracker *managedLease) *dhcp.Lease {
m.mu.Lock()
defer m.mu.Unlock()
return tracker.lease
}
// get returns the current lease for a ClientID, if tracked.
func (m *DHCPManager) get(clientID string) (*dhcp.Lease, bool) {
m.mu.Lock()
defer m.mu.Unlock()
t, ok := m.leases[clientID]
if !ok {
return nil, false
}
return t.lease, true
}
// forget removes a lease from the tracking map, KV and stops its renewal
// goroutine.
func (m *DHCPManager) forget(clientID, reason string) {
m.mu.Lock()
t, ok := m.leases[clientID]
if ok {
delete(m.leases, clientID)
}
m.mu.Unlock()
if ok && t.renewStop != nil {
t.renewStop()
}
if err := m.kv.Delete(clientID); err != nil && !errors.Is(err, nats.ErrKeyNotFound) {
slog.Debug("DHCP Manager: delete KV key failed", "client_id", clientID, "reason", reason, "err", err)
}
}
// persistLease writes the lease state to the KV bucket under ClientID.
func (m *DHCPManager) persistLease(lease *dhcp.Lease) error {
data, err := encodeLease(lease)
if err != nil {
return err
}
if _, err := m.kv.Put(lease.ClientID, data); err != nil {
return fmt.Errorf("put lease %s: %w", lease.ClientID, err)
}
return nil
}
func (m *DHCPManager) publishExpired(lease *dhcp.Lease, poolName, reason string) {
evt := dhcp.LeaseExpiredEvent{
ClientID: lease.ClientID,
PoolName: poolName,
IP: lease.IP.String(),
Reason: reason,
}
data, err := json.Marshal(evt)
if err != nil {
slog.Warn("DHCP Manager: marshal lease-expired event failed", "client_id", lease.ClientID, "err", err)
return
}
if err := m.nc.Publish(dhcp.TopicLeaseExpired, data); err != nil {
slog.Warn("DHCP Manager: publish lease-expired failed", "client_id", lease.ClientID, "err", err)
}
}
func (m *DHCPManager) replyAcquire(msg *nats.Msg, lease *dhcp.Lease) {
reply := dhcp.AcquireReplyMsg{
IP: lease.IP.String(),
SubnetMask: net.IP(lease.SubnetMask).String(),
Routers: ipsToStrings(lease.Routers),
DNS: ipsToStrings(lease.DNS),
ServerID: lease.ServerID.String(),
HWAddr: lease.HWAddr.String(),
ExpiresUnix: lease.ExpiresAt().Unix(),
}
respondJSON(msg, reply)
}
func (m *DHCPManager) replyAcquireError(msg *nats.Msg, errMsg string) {
respondJSON(msg, dhcp.AcquireReplyMsg{Error: errMsg})
}
func (m *DHCPManager) replyRelease(msg *nats.Msg, errMsg string) {
respondJSON(msg, dhcp.ReleaseReplyMsg{Error: errMsg})
}
// jitter applies ±jitterFraction of d to produce an actual sleep duration.
// Negative d is clamped to 0.
func (m *DHCPManager) jitter(d time.Duration) time.Duration {
if d <= 0 {
return 0
}
if m.jitterFraction <= 0 {
return d
}
spread := float64(d) * m.jitterFraction
offset := (m.rng.Float64()*2 - 1) * spread
out := time.Duration(float64(d) + offset)
if out < 0 {
return 0
}
return out
}
// resolveHWAddr parses the hw_addr string supplied in the NATS request,
// falling back to a derived MAC when the request carries no value.
func resolveHWAddr(raw, clientID string, derive func(string) net.HardwareAddr) (net.HardwareAddr, error) {
if raw != "" {
mac, err := net.ParseMAC(raw)
if err != nil {
return nil, fmt.Errorf("parse hw_addr %q: %w", raw, err)
}
return mac, nil
}
if derive == nil {
return nil, fmt.Errorf("hw_addr is empty and no derivation function configured")
}
mac := derive(clientID)
if len(mac) == 0 {
return nil, fmt.Errorf("derive hw_addr for client %q: empty result", clientID)
}
return mac, nil
}
// classifyRenewErr maps a renew error to a short reason code on the
// lease-expired event. Kept coarse; callers that care about specifics
// can re-log from their side.
func classifyRenewErr(err error) string {
if err == nil {
return "unknown"
}
// nclient4 returns *nclient4.ErrNak on NAK. Rather than taking a
// dependency on its private type here, match the known message.
msg := err.Error()
if strings.Contains(msg, "NAK") || strings.Contains(msg, "Nak") {
return "nak"
}
if strings.Contains(msg, "timeout") || strings.Contains(msg, "deadline exceeded") || strings.Contains(msg, "i/o timeout") {
return "server_unreachable"
}
return "renew_failed"
}
func ipsToStrings(ips []net.IP) []string {
if len(ips) == 0 {
return nil
}
out := make([]string, 0, len(ips))
for _, ip := range ips {
out = append(out, ip.String())
}
return out
}
// respondJSON marshals v and sends it as the reply. If the message has
// no Reply subject (fire-and-forget), drops the reply silently.
func respondJSON(msg *nats.Msg, v any) {
if msg.Reply == "" {
return
}
data, err := json.Marshal(v)
if err != nil {
slog.Warn("DHCP Manager: marshal reply failed", "err", err)
return
}
if err := msg.Respond(data); err != nil {
slog.Warn("DHCP Manager: respond failed", "err", err)
}
}
// sleep blocks until d elapses or ctx is done. Returns false if ctx was
// cancelled (caller should exit).
func sleep(ctx context.Context, d time.Duration) bool {
if d <= 0 {
select {
case <-ctx.Done():
return false
default:
return true
}
}
t := time.NewTimer(d)
defer t.Stop()
select {
case <-ctx.Done():
return false
case <-t.C:
return true
}
}
// --- KV encoding ---
// leaseRecord is the JSON representation persisted in KV and used to
// rehydrate on vpcd startup. Explicit format rather than gob-encoding
// *dhcp.Lease so a future Go type change doesn't invalidate every
// in-flight lease.
type leaseRecord struct {
Version int `json:"version"`
Bridge string `json:"bridge"`
ClientID string `json:"client_id"`
Hostname string `json:"hostname,omitempty"`
VendorClass string `json:"vendor_class,omitempty"`
HWAddr string `json:"hw_addr"`
IP string `json:"ip"`
SubnetMask string `json:"subnet_mask,omitempty"`
Routers []string `json:"routers,omitempty"`
DNS []string `json:"dns,omitempty"`
ServerID string `json:"server_id,omitempty"`
AcquiredUnix int64 `json:"acquired_unix"`
LeaseSeconds int64 `json:"lease_seconds"`
T1Seconds int64 `json:"t1_seconds"`
T2Seconds int64 `json:"t2_seconds"`
RawOfferB64 []byte `json:"raw_offer,omitempty"`
RawACKB64 []byte `json:"raw_ack,omitempty"`
}
const leaseRecordVersion = 1
func encodeLease(l *dhcp.Lease) ([]byte, error) {
rec := leaseRecord{
Version: leaseRecordVersion,
Bridge: l.Bridge,
ClientID: l.ClientID,
Hostname: l.Hostname,
VendorClass: l.VendorClass,
HWAddr: l.HWAddr.String(),
IP: l.IP.String(),
SubnetMask: maskString(l.SubnetMask),
Routers: ipsToStrings(l.Routers),
DNS: ipsToStrings(l.DNS),
ServerID: l.ServerID.String(),
AcquiredUnix: l.AcquiredAt.Unix(),
LeaseSeconds: int64(l.LeaseDuration / time.Second),
T1Seconds: int64(l.T1 / time.Second),
T2Seconds: int64(l.T2 / time.Second),
RawOfferB64: l.RawOffer,
RawACKB64: l.RawACK,
}
return json.Marshal(rec)
}
func decodeLease(data []byte) (*dhcp.Lease, error) {
var rec leaseRecord
if err := json.Unmarshal(data, &rec); err != nil {
return nil, fmt.Errorf("unmarshal lease: %w", err)
}
if rec.Version != leaseRecordVersion {
return nil, fmt.Errorf("unsupported lease record version %d", rec.Version)
}
mac, err := net.ParseMAC(rec.HWAddr)
if err != nil {
return nil, fmt.Errorf("parse hw_addr %q: %w", rec.HWAddr, err)
}
return &dhcp.Lease{
Bridge: rec.Bridge,
ClientID: rec.ClientID,
Hostname: rec.Hostname,
VendorClass: rec.VendorClass,
HWAddr: mac,
IP: net.ParseIP(rec.IP),
SubnetMask: parseMask(rec.SubnetMask),
Routers: parseIPs(rec.Routers),
DNS: parseIPs(rec.DNS),
ServerID: net.ParseIP(rec.ServerID),
AcquiredAt: time.Unix(rec.AcquiredUnix, 0),
LeaseDuration: time.Duration(rec.LeaseSeconds) * time.Second,
T1: time.Duration(rec.T1Seconds) * time.Second,
T2: time.Duration(rec.T2Seconds) * time.Second,
RawOffer: rec.RawOfferB64,
RawACK: rec.RawACKB64,
}, nil
}
func maskString(m net.IPMask) string {
if len(m) == 0 {
return ""
}
return net.IP(m).String()
}
func parseMask(s string) net.IPMask {
ip := net.ParseIP(s)
if ip == nil {
return nil
}
ip4 := ip.To4()
if ip4 == nil {
return nil
}
return net.IPv4Mask(ip4[0], ip4[1], ip4[2], ip4[3])
}
func parseIPs(ss []string) []net.IP {
if len(ss) == 0 {
return nil
}
out := make([]net.IP, 0, len(ss))
for _, s := range ss {
if ip := net.ParseIP(s); ip != nil {
out = append(out, ip)
}
}
return out
}
spinifex/services/vpcd/dhcp_manager_test.go
-555
View File
@@ -1,555 +0,0 @@
package vpcd
import (
"context"
"encoding/json"
"errors"
"net"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
"github.com/mulgadc/spinifex/spinifex/testutil"
"github.com/nats-io/nats.go"
"github.com/stretchr/testify/require"
)
// newTestDHCPManager spins up an embedded JetStream, wires a Fake client,
// and returns the Manager along with the NATS connection. Cleanup of NATS
// is handled by testutil; the Manager is closed via t.Cleanup.
func newTestDHCPManager(t *testing.T, opts ...DHCPManagerOption) (*DHCPManager, *nats.Conn, *dhcp.Fake) {
t.Helper()
_, nc, js := testutil.StartTestJetStream(t)
fake := dhcp.NewFake()
m, err := NewDHCPManager(nc, js, fake, opts...)
require.NoError(t, err)
t.Cleanup(m.Close)
subs, err := m.Subscribe(nc)
require.NoError(t, err)
t.Cleanup(func() {
for _, s := range subs {
_ = s.Unsubscribe()
}
})
return m, nc, fake
}
func requestAcquire(t *testing.T, nc *nats.Conn, req dhcp.AcquireRequestMsg) dhcp.AcquireReplyMsg {
t.Helper()
data, err := json.Marshal(req)
require.NoError(t, err)
msg, err := nc.Request(dhcp.TopicAcquire, data, 5*time.Second)
require.NoError(t, err)
var reply dhcp.AcquireReplyMsg
require.NoError(t, json.Unmarshal(msg.Data, &reply))
return reply
}
func requestRelease(t *testing.T, nc *nats.Conn, clientID string) dhcp.ReleaseReplyMsg {
t.Helper()
data, err := json.Marshal(dhcp.ReleaseRequestMsg{ClientID: clientID})
require.NoError(t, err)
msg, err := nc.Request(dhcp.TopicRelease, data, 5*time.Second)
require.NoError(t, err)
var reply dhcp.ReleaseReplyMsg
require.NoError(t, json.Unmarshal(msg.Data, &reply))
return reply
}
func TestDHCPManager_AcquireReturnsLease(t *testing.T) {
_, nc, fake := newTestDHCPManager(t)
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{
Bridge: "br-wan", ClientID: "eni-1", Hostname: "eni-1", VendorClass: "i-abc",
})
require.Empty(t, reply.Error, "unexpected error: %s", reply.Error)
require.Equal(t, "192.0.2.100", reply.IP)
require.NotEmpty(t, reply.ServerID)
require.NotZero(t, reply.ExpiresUnix)
require.NotEmpty(t, reply.HWAddr)
require.Equal(t, 1, fake.AcquireCount())
}
func TestDHCPManager_AcquireIdempotentWhileLeaseActive(t *testing.T) {
_, nc, fake := newTestDHCPManager(t)
req := dhcp.AcquireRequestMsg{Bridge: "br-wan", ClientID: "eni-idem"}
first := requestAcquire(t, nc, req)
require.Empty(t, first.Error)
second := requestAcquire(t, nc, req)
require.Empty(t, second.Error)
require.Equal(t, first.IP, second.IP, "idempotent acquire should return the same IP")
require.Equal(t, 1, fake.AcquireCount(), "second request must not trigger a fresh DORA")
}
func TestDHCPManager_AcquireFailureSurfacesError(t *testing.T) {
_, nc, fake := newTestDHCPManager(t)
fake.AcquireHook = func(dhcp.AcquireRequest) (*dhcp.Lease, error) {
return nil, errors.New("server unreachable")
}
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{Bridge: "br-wan", ClientID: "eni-err"})
require.Contains(t, reply.Error, "server unreachable")
require.Empty(t, reply.IP)
}
func TestDHCPManager_ReleaseRemovesLease(t *testing.T) {
m, nc, fake := newTestDHCPManager(t)
req := dhcp.AcquireRequestMsg{Bridge: "br-wan", ClientID: "eni-release"}
require.Empty(t, requestAcquire(t, nc, req).Error)
reply := requestRelease(t, nc, req.ClientID)
require.Empty(t, reply.Error)
require.Equal(t, 1, fake.ReleaseCount())
_, ok := m.get("eni-release")
require.False(t, ok, "lease should be forgotten after Release")
}
func TestDHCPManager_ReleaseUnknownIsNoop(t *testing.T) {
_, nc, fake := newTestDHCPManager(t)
reply := requestRelease(t, nc, "nonexistent")
require.Empty(t, reply.Error)
require.Equal(t, 0, fake.ReleaseCount())
}
func TestDHCPManager_AcquireRejectsMissingFields(t *testing.T) {
_, nc, _ := newTestDHCPManager(t)
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{Bridge: "", ClientID: "eni-x"})
require.Contains(t, reply.Error, "bridge and client_id are required")
}
func TestDHCPManager_HWAddrDerivedFromClientID(t *testing.T) {
_, nc, fake := newTestDHCPManager(t)
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{Bridge: "br-wan", ClientID: "eni-mac"})
require.Empty(t, reply.Error)
lease, ok := fake.HeldLease("eni-mac")
require.True(t, ok)
// generateMAC produces a locally-administered unicast MAC. First octet
// is hash-derived; assert the IEEE 802 reserved bits.
require.Equal(t, byte(0x02), lease.HWAddr[0]&0x03)
}
func TestDHCPManager_HWAddrExplicitlySuppliedIsUsed(t *testing.T) {
_, nc, fake := newTestDHCPManager(t)
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{
Bridge: "br-wan", ClientID: "eni-mac-explicit", HWAddr: "02:aa:bb:cc:dd:ee",
})
require.Empty(t, reply.Error)
lease, _ := fake.HeldLease("eni-mac-explicit")
require.Equal(t, "02:aa:bb:cc:dd:ee", lease.HWAddr.String())
}
func TestDHCPManager_RenewalFiresAndUpdatesLease(t *testing.T) {
tmpl := dhcp.DefaultLeaseTemplate()
tmpl.LeaseDuration = 40 * time.Millisecond
fake := dhcp.NewFake()
fake.SetDefaultLease(tmpl)
_, nc, js := testutil.StartTestJetStream(t)
m, err := NewDHCPManager(nc, js, fake, WithDHCPJitterFraction(0))
require.NoError(t, err)
t.Cleanup(m.Close)
subs, err := m.Subscribe(nc)
require.NoError(t, err)
t.Cleanup(func() {
for _, s := range subs {
_ = s.Unsubscribe()
}
})
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{Bridge: "br-wan", ClientID: "eni-renew"})
require.Empty(t, reply.Error)
// T1 = lease/2 = 20 ms. Wait a few multiples to catch at least one renew.
require.Eventually(t, func() bool { return fake.RenewCount() >= 1 }, 2*time.Second, 10*time.Millisecond)
}
func TestDHCPManager_RenewFailureEmitsLeaseExpired(t *testing.T) {
tmpl := dhcp.DefaultLeaseTemplate()
tmpl.LeaseDuration = 60 * time.Millisecond
fake := dhcp.NewFake()
fake.SetDefaultLease(tmpl)
fake.RenewHook = func(*dhcp.Lease) (*dhcp.Lease, error) {
return nil, errors.New("server NAK")
}
_, nc, js := testutil.StartTestJetStream(t)
m, err := NewDHCPManager(nc, js, fake, WithDHCPJitterFraction(0))
require.NoError(t, err)
t.Cleanup(m.Close)
var events atomic.Int32
expiredMsg := make(chan dhcp.LeaseExpiredEvent, 1)
sub, err := nc.Subscribe(dhcp.TopicLeaseExpired, func(msg *nats.Msg) {
events.Add(1)
var evt dhcp.LeaseExpiredEvent
if err := json.Unmarshal(msg.Data, &evt); err == nil {
select {
case expiredMsg <- evt:
default:
}
}
})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
subs, err := m.Subscribe(nc)
require.NoError(t, err)
t.Cleanup(func() {
for _, s := range subs {
_ = s.Unsubscribe()
}
})
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{Bridge: "br-wan", ClientID: "eni-expire"})
require.Empty(t, reply.Error)
// T1=30ms → try, fail. T2=~52ms → try, fail. Expires at 60ms.
select {
case evt := <-expiredMsg:
require.Equal(t, "eni-expire", evt.ClientID)
require.Equal(t, "nak", evt.Reason)
case <-time.After(2 * time.Second):
t.Fatalf("expected lease-expired event (events=%d)", events.Load())
}
}
func TestDHCPManager_BootstrapRehydratesLease(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
fake := dhcp.NewFake()
m1, err := NewDHCPManager(nc, js, fake)
require.NoError(t, err)
subs, err := m1.Subscribe(nc)
require.NoError(t, err)
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{Bridge: "br-wan", ClientID: "eni-boot"})
require.Empty(t, reply.Error)
// Close first manager; KV entry should survive.
for _, s := range subs {
_ = s.Unsubscribe()
}
m1.Close()
// Second manager picks up the lease on Bootstrap.
fake2 := dhcp.NewFake()
m2, err := NewDHCPManager(nc, js, fake2)
require.NoError(t, err)
t.Cleanup(m2.Close)
require.NoError(t, m2.Bootstrap(context.Background()))
lease, ok := m2.get("eni-boot")
require.True(t, ok, "manager should rehydrate persisted lease")
require.Equal(t, "192.0.2.100", lease.IP.String())
require.Equal(t, 0, fake2.AcquireCount(), "no fresh DORA on bootstrap")
}
func TestDHCPManager_BootstrapDropsExpiredLeases(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
fake := dhcp.NewFake()
m1, err := NewDHCPManager(nc, js, fake)
require.NoError(t, err)
subs, err := m1.Subscribe(nc)
require.NoError(t, err)
reply := requestAcquire(t, nc, dhcp.AcquireRequestMsg{Bridge: "br-wan", ClientID: "eni-stale"})
require.Empty(t, reply.Error)
for _, s := range subs {
_ = s.Unsubscribe()
}
m1.Close()
// Rewrite the persisted record so AcquiredAt + LeaseDuration is in the past.
kv, err := js.KeyValue(dhcp.KVBucketDHCPLeases)
require.NoError(t, err)
entry, err := kv.Get("eni-stale")
require.NoError(t, err)
var rec map[string]any
require.NoError(t, json.Unmarshal(entry.Value(), &rec))
rec["acquired_unix"] = time.Now().Add(-2 * time.Hour).Unix()
rec["lease_seconds"] = int64(60)
raw, err := json.Marshal(rec)
require.NoError(t, err)
_, err = kv.Put("eni-stale", raw)
require.NoError(t, err)
fake2 := dhcp.NewFake()
m2, err := NewDHCPManager(nc, js, fake2)
require.NoError(t, err)
t.Cleanup(m2.Close)
var expired sync.WaitGroup
expired.Add(1)
sub, err := nc.Subscribe(dhcp.TopicLeaseExpired, func(msg *nats.Msg) {
var evt dhcp.LeaseExpiredEvent
if err := json.Unmarshal(msg.Data, &evt); err == nil && evt.ClientID == "eni-stale" {
expired.Done()
}
})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
require.NoError(t, m2.Bootstrap(context.Background()))
done := make(chan struct{})
go func() { expired.Wait(); close(done) }()
select {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal("expected lease-expired event for stale lease")
}
_, err = kv.Get("eni-stale")
require.ErrorIs(t, err, nats.ErrKeyNotFound)
}
func TestDHCPManager_JitterWithinBounds(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
m, err := NewDHCPManager(nc, js, dhcp.NewFake(), WithDHCPJitterFraction(0.1))
require.NoError(t, err)
t.Cleanup(m.Close)
base := 100 * time.Millisecond
minOK := time.Duration(float64(base) * 0.9)
maxOK := time.Duration(float64(base) * 1.1)
for i := range 100 {
got := m.jitter(base)
if got < minOK || got > maxOK {
t.Fatalf("iteration %d: jitter produced %v, outside [%v,%v]", i, got, minOK, maxOK)
}
}
}
func TestNewDHCPManager_RejectsNilDeps(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
_, err := NewDHCPManager(nil, js, dhcp.NewFake())
require.ErrorContains(t, err, "nats.Conn is required")
_, err = NewDHCPManager(nc, nil, dhcp.NewFake())
require.ErrorContains(t, err, "JetStreamContext is required")
_, err = NewDHCPManager(nc, js, nil)
require.ErrorContains(t, err, "dhcp.Client is required")
}
func TestDHCPManager_OptionsApply(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
customMAC := func(string) net.HardwareAddr {
mac, _ := net.ParseMAC("02:de:ad:be:ef:01")
return mac
}
m, err := NewDHCPManager(nc, js, dhcp.NewFake(),
WithDHCPMACFunc(customMAC),
WithDHCPAcquireTimeout(1*time.Second),
WithDHCPJitterFraction(0.05),
)
require.NoError(t, err)
t.Cleanup(m.Close)
require.Equal(t, time.Second, m.acquireTimeout)
require.InDelta(t, 0.05, m.jitterFraction, 0.0001)
require.Equal(t, "02:de:ad:be:ef:01", m.macForClientID("anything").String())
}
func TestDHCPManager_BootstrapEmptyBucket(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
m, err := NewDHCPManager(nc, js, dhcp.NewFake())
require.NoError(t, err)
t.Cleanup(m.Close)
// No leases in KV → Bootstrap is a clean no-op.
require.NoError(t, m.Bootstrap(context.Background()))
}
func TestDHCPManager_JitterWithZeroFractionReturnsInput(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
m, err := NewDHCPManager(nc, js, dhcp.NewFake(), WithDHCPJitterFraction(0))
require.NoError(t, err)
t.Cleanup(m.Close)
require.Equal(t, 100*time.Millisecond, m.jitter(100*time.Millisecond))
require.Equal(t, time.Duration(0), m.jitter(0))
}
func TestDHCPManager_ClassifyRenewErr(t *testing.T) {
require.Equal(t, "unknown", classifyRenewErr(nil))
require.Equal(t, "nak", classifyRenewErr(errors.New("got NAK from server")))
require.Equal(t, "server_unreachable", classifyRenewErr(errors.New("i/o timeout")))
require.Equal(t, "renew_failed", classifyRenewErr(errors.New("something else")))
}
func TestDHCPManager_EncodeDecodeLeaseRoundTrip(t *testing.T) {
mac, _ := net.ParseMAC("02:00:00:de:ad:be")
in := &dhcp.Lease{
Bridge: "br-wan", ClientID: "eni-rt", Hostname: "eni-rt", VendorClass: "i-xyz",
HWAddr: mac,
IP: net.ParseIP("192.0.2.50"),
SubnetMask: net.CIDRMask(24, 32),
Routers: []net.IP{net.ParseIP("192.0.2.1")},
DNS: []net.IP{net.ParseIP("1.1.1.1")},
ServerID: net.ParseIP("192.0.2.1"),
AcquiredAt: time.Unix(1_700_000_000, 0).UTC(),
LeaseDuration: time.Hour,
T1: 30 * time.Minute,
T2: 52 * time.Minute,
RawOffer: []byte{0x01, 0x02},
RawACK: []byte{0x03, 0x04},
}
data, err := encodeLease(in)
require.NoError(t, err)
out, err := decodeLease(data)
require.NoError(t, err)
require.Equal(t, in.ClientID, out.ClientID)
require.Equal(t, in.IP.String(), out.IP.String())
require.Equal(t, in.LeaseDuration, out.LeaseDuration)
require.Equal(t, in.HWAddr.String(), out.HWAddr.String())
require.Equal(t, in.RawACK, out.RawACK)
require.Equal(t, in.RawOffer, out.RawOffer)
}
// --- acquireWithBackoff (mulga-siv-39, RFC 2131 §4.1) ---
func newBackoffManager(t *testing.T, schedule []time.Duration, fake *dhcp.Fake) *DHCPManager {
t.Helper()
_, nc, js := testutil.StartTestJetStream(t)
m, err := NewDHCPManager(nc, js, fake, WithDHCPAcquireBackoff(schedule))
require.NoError(t, err)
t.Cleanup(m.Close)
return m
}
func TestAcquireWithBackoff_FirstAttemptSucceeds(t *testing.T) {
fake := dhcp.NewFake()
m := newBackoffManager(t, []time.Duration{10 * time.Millisecond, 10 * time.Millisecond}, fake)
lease, err := m.acquireWithBackoff(context.Background(), dhcp.AcquireRequest{
Bridge: "br-wan", ClientID: "eni-fast", HWAddr: net.HardwareAddr{0x02, 0, 0, 0, 0, 1},
})
require.NoError(t, err)
require.NotNil(t, lease)
require.Equal(t, 1, fake.AcquireCount(), "first-attempt success must not retransmit")
}
func TestAcquireWithBackoff_RetriesThenSucceeds(t *testing.T) {
fake := dhcp.NewFake()
var calls atomic.Int32
tmpl := dhcp.DefaultLeaseTemplate()
fake.AcquireHook = func(req dhcp.AcquireRequest) (*dhcp.Lease, error) {
n := calls.Add(1)
if n < 3 {
return nil, errors.New("simulated OFFER timeout")
}
return &dhcp.Lease{
Bridge: req.Bridge, ClientID: req.ClientID, HWAddr: req.HWAddr,
IP: tmpl.IP, ServerID: tmpl.ServerID, LeaseDuration: tmpl.LeaseDuration,
AcquiredAt: time.Now(),
}, nil
}
m := newBackoffManager(t, []time.Duration{
5 * time.Millisecond, 5 * time.Millisecond, 5 * time.Millisecond, 5 * time.Millisecond,
}, fake)
lease, err := m.acquireWithBackoff(context.Background(), dhcp.AcquireRequest{
Bridge: "br-wan", ClientID: "eni-retry", HWAddr: net.HardwareAddr{0x02, 0, 0, 0, 0, 2},
})
require.NoError(t, err)
require.NotNil(t, lease)
require.Equal(t, int32(3), calls.Load())
}
func TestAcquireWithBackoff_AllAttemptsFail(t *testing.T) {
fake := dhcp.NewFake()
fake.AcquireHook = func(dhcp.AcquireRequest) (*dhcp.Lease, error) {
return nil, errors.New("server silent")
}
schedule := []time.Duration{
2 * time.Millisecond, 2 * time.Millisecond, 2 * time.Millisecond,
}
m := newBackoffManager(t, schedule, fake)
lease, err := m.acquireWithBackoff(context.Background(), dhcp.AcquireRequest{
Bridge: "br-wan", ClientID: "eni-fail", HWAddr: net.HardwareAddr{0x02, 0, 0, 0, 0, 3},
})
require.Error(t, err)
require.Nil(t, lease)
require.ErrorContains(t, err, "server silent")
require.Equal(t, len(schedule), fake.AcquireCount(),
"every entry in the schedule must run before giving up")
}
func TestAcquireWithBackoff_ParentCtxCancel(t *testing.T) {
fake := dhcp.NewFake()
fake.AcquireHook = func(dhcp.AcquireRequest) (*dhcp.Lease, error) {
return nil, errors.New("should not be called")
}
m := newBackoffManager(t, []time.Duration{
time.Second, time.Second, time.Second,
}, fake)
ctx, cancel := context.WithCancel(context.Background())
cancel()
lease, err := m.acquireWithBackoff(ctx, dhcp.AcquireRequest{
Bridge: "br-wan", ClientID: "eni-cancel", HWAddr: net.HardwareAddr{0x02, 0, 0, 0, 0, 4},
})
require.Error(t, err)
require.Nil(t, lease)
require.ErrorIs(t, err, context.Canceled)
require.Equal(t, 0, fake.AcquireCount(), "cancelled parent must short-circuit before any attempt")
}
func TestDHCPAttemptTimeoutJitterBounds(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
m, err := NewDHCPManager(nc, js, dhcp.NewFake())
require.NoError(t, err)
t.Cleanup(m.Close)
base := 4 * time.Second
for i := range 200 {
got := m.dhcpAttemptTimeout(base)
if got < base-time.Second || got > base+time.Second {
t.Fatalf("iter %d: dhcpAttemptTimeout(%v) = %v, outside ±1s", i, base, got)
}
}
require.Zero(t, m.dhcpAttemptTimeout(0))
}
func TestNewDHCPManager_DefaultBackoffSchedule(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
m, err := NewDHCPManager(nc, js, dhcp.NewFake())
require.NoError(t, err)
t.Cleanup(m.Close)
require.Equal(t, []time.Duration{
4 * time.Second, 8 * time.Second, 16 * time.Second, 32 * time.Second,
}, m.acquireBackoff)
require.Equal(t, 90*time.Second, m.acquireTimeout)
}
func TestResolveHWAddr(t *testing.T) {
t.Run("InvalidRawMACReturnsParseError", func(t *testing.T) {
mac, err := resolveHWAddr("not-a-mac", "client-1", nil)
require.Error(t, err)
require.Nil(t, mac)
require.Contains(t, err.Error(), "parse hw_addr")
})
t.Run("EmptyRawWithNilDeriveErrors", func(t *testing.T) {
mac, err := resolveHWAddr("", "client-1", nil)
require.Error(t, err)
require.Nil(t, mac)
require.Contains(t, err.Error(), "no derivation function")
})
t.Run("EmptyRawWithDeriveReturningEmptyErrors", func(t *testing.T) {
mac, err := resolveHWAddr("", "client-1", func(string) net.HardwareAddr { return nil })
require.Error(t, err)
require.Nil(t, mac)
require.Contains(t, err.Error(), "empty result")
})
}
spinifex/services/vpcd/topology.go
+17 -117
View File
@@ -14,7 +14,6 @@ import (
"github.com/mulgadc/spinifex/spinifex/network/external"
"github.com/mulgadc/spinifex/spinifex/network/policy"
"github.com/mulgadc/spinifex/spinifex/network/topology"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/nbdb"
"github.com/mulgadc/spinifex/spinifex/types"
"github.com/mulgadc/spinifex/spinifex/utils"
@@ -160,7 +159,7 @@ const (
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
// setup and independent of dhcp_bind_bridge (which is Linux-side in veth
// setup and independent of the WAN bridge (which is Linux-side in veth
// mode). Named as a constant so the vpcd sanity check refers to the
// contract, not a hardcoded string (D18).
OvnExternalBridge = "br-ext"
@@ -173,11 +172,6 @@ type TopologyHandler struct {
externalPools []ExternalPoolConfig
chassisNames []string // OVN chassis names for gateway HA scheduling
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
// the test stack supplies a static-only mock.
nc *nats.Conn
// lm + lmOnce cache the topology.Manager produced from h.ovn so each
// adapter call doesn't re-construct (Phase 2.6, mulga-siv-129).
@@ -197,9 +191,8 @@ type TopologyHandler struct {
natmErr error
// igwm + igwmOnce cache the external.IGWManager. Handler routes the
// static-pool / distributed branches through it; the DHCP-coupled
// centralised path keeps using attachIGWLegacy until bead
// mulga-siv-125.3.3 lands and the DHCP manager goes away.
// static-pool / distributed branches through it; attachIGWLegacy is a
// dead branch retained for Slice D's deletion (mulga-siv-125.3.3).
igwmOnce sync.Once
igwm external.IGWManager
igwmErr error
@@ -240,15 +233,6 @@ func WithBridgeMode(mode string) TopologyOption {
}
}
// WithNATSConn wires the NATS connection used to talk to vpcd's DHCPManager
// for gateway-LRP DHCP-acquire (mulga-siv-38). Tests with mock OVN clients
// can omit this — the static / auto-derive paths still work.
func WithNATSConn(nc *nats.Conn) TopologyOption {
return func(h *TopologyHandler) {
h.nc = nc
}
}
// useCentralizedNAT returns true if the bridge mode requires centralized NAT.
// Veth mode needs centralized NAT because the Linux bridge intermediary breaks
// distributed NAT hairpin routing. Only direct bridge mode supports distributed NAT.
@@ -301,11 +285,10 @@ func (h *TopologyHandler) ensureLocalnetOptions(ctx context.Context, extPortName
// expectedGatewayPortNetwork returns the Networks CIDR a gateway LRP for
// vpcId should carry. In direct mode, or in centralized mode without a pool,
// it is the link-local fallback. In centralized mode with a DHCP pool the
// gateway IP is acquired via vpc.dhcp.acquire (mulga-siv-38). With a static
// pool the IP comes from gw_lrp_range or the auto-derived top-of-subnet
// block (mulga-siv-36). Returns the gw IP (empty when link-local) so
// callers can persist it to external_ids.
// it is the link-local fallback. In centralized mode with a static pool the
// IP comes from gw_lrp_range or the auto-derived top-of-subnet block
// (mulga-siv-36). Returns the 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
@@ -314,13 +297,6 @@ func (h *TopologyHandler) expectedGatewayPortNetwork(ctx context.Context, vpcId
if pool == nil {
return gatewayPortNetwork, "", nil
}
if pool.IsDHCP() {
ip, prefix, dhcpErr := h.allocateGatewayLRPIPViaDHCP(ctx, vpcId, pool)
if dhcpErr != nil {
return "", "", dhcpErr
}
return fmt.Sprintf("%s/%d", ip, prefix), ip, nil
}
ip, prefix, ok, allocErr := h.allocateGatewayLRPIP(ctx, vpcId, pool)
if allocErr != nil {
return "", "", allocErr
@@ -704,18 +680,10 @@ func (h *TopologyHandler) handleUpdatePortSGs(msg *nats.Msg) {
// --- Internet Gateway (external connectivity + NAT) ---
// handleIGWAttach decodes the event and chooses a path:
//
// - Centralised NAT + DHCP-sourced pool: stays on attachIGWLegacy
// because the gateway LRP IP is allocated via vpc.dhcp.acquire, and
// that DHCP client lives in services/vpcd. Bead mulga-siv-125.3.3
// removes the DHCP manager; the legacy branch goes away then.
//
// - All other cases (distributed NAT, static-range pool, no pool):
// delegate to external.IGWManager.AttachIGW. Behaviour is identical
// end-to-end — same external switch, same localnet port options,
// same gateway LRP networks, same default route, same chassis HA,
// same waitForFlowsHV tail barrier.
// handleIGWAttach decodes the event and delegates to
// external.IGWManager.AttachIGW. The legacy DHCP-coupled path is dead
// (pool.IsDHCP() now always returns false) and is removed in Slice D
// (mulga-siv-125.3.3).
func (h *TopologyHandler) handleIGWAttach(msg *nats.Msg) {
if h.ovn == nil {
respond(msg, fmt.Errorf("OVN client not connected"))
@@ -753,9 +721,9 @@ func (h *TopologyHandler) handleIGWAttach(msg *nats.Msg) {
respond(msg, nil)
}
// attachIGWLegacy is the DHCP-coupled centralised-NAT IGW attach path.
// Kept inline until the vpcd-local DHCP manager goes away in bead
// mulga-siv-125.3.3. The caller pre-resolved pool.
// attachIGWLegacy is the dead legacy centralised-NAT IGW attach path.
// Unreachable since pool.IsDHCP() always returns false; retained only
// for Slice D's deletion (mulga-siv-125.3.3).
func (h *TopologyHandler) attachIGWLegacy(ctx context.Context, msg *nats.Msg, evt types.IGWEvent, pool *ExternalPoolConfig) {
routerName := "vpc-" + evt.VpcId
extSwitchName := "ext-" + evt.VpcId
@@ -848,7 +816,6 @@ func (h *TopologyHandler) attachIGWLegacy(ctx context.Context, msg *nats.Msg, ev
}
slog.Info("vpcd: using external pool for IGW",
"pool", pool.Name,
"source", pool.Source,
"lrp_network", gatewayNetwork,
"wan_gateway", wanGateway,
)
@@ -962,12 +929,9 @@ func (h *TopologyHandler) attachIGWLegacy(ctx context.Context, msg *nats.Msg, ev
respond(msg, nil)
}
// handleIGWDetach decodes the event, delegates the OVN teardown to
// external.IGWManager.DetachIGW, and unconditionally calls
// releaseGatewayLRPLease afterwards. The DHCP lease release stays here
// because the upstream client lives in services/vpcd; both attach paths
// (legacy + manager-driven) tear down through the same exit so we don't
// need to branch.
// handleIGWDetach decodes the event and delegates the OVN teardown to
// external.IGWManager.DetachIGW. Static gateway LRP IPs live in OVN
// external_ids and are cleared when the LRP itself is deleted.
func (h *TopologyHandler) handleIGWDetach(msg *nats.Msg) {
if h.ovn == nil {
respond(msg, fmt.Errorf("OVN client not connected"))
@@ -992,7 +956,6 @@ func (h *TopologyHandler) handleIGWDetach(msg *nats.Msg) {
respond(msg, err)
return
}
h.releaseGatewayLRPLease(evt.VpcId)
slog.Info("vpcd: detached internet gateway from VPC",
"igw_id", evt.InternetGatewayId, "vpc_id", evt.VpcId)
respond(msg, nil)
@@ -1327,69 +1290,6 @@ func gwLrpRange(pool *ExternalPoolConfig) (start, end net.IP, prefix int, ok boo
return uint32ToIPv4(autoStartU), uint32ToIPv4(autoEndU), prefix, true
}
// gwLrpClientID is the DHCP client-id used for a VPC's gateway LRP lease.
// Stable across reboots — DHCPManager reuses the same lease on idempotent
// re-attach so the LRP IP doesn't change unless the upstream server
// reassigns it.
func gwLrpClientID(vpcId string) string { return "gw-lrp-" + vpcId }
// allocateGatewayLRPIPViaDHCP requests a DHCP lease from vpcd's DHCPManager
// for the gateway LRP of vpcId. The handler-side acquire is idempotent on
// client-id so retries return the same IP without a fresh DORA. Prefix
// comes from the lease's SubnetMask, falling back to pool.PrefixLen when
// the server omits option 1 (rare; should not happen in practice).
func (h *TopologyHandler) allocateGatewayLRPIPViaDHCP(ctx context.Context, vpcId string, pool *ExternalPoolConfig) (ip string, prefix int, err error) {
_ = ctx
if h.nc == nil {
return "", 0, fmt.Errorf("vpcd: no NATS conn for DHCP gw LRP allocation (vpc %s, pool %s)", vpcId, pool.Name)
}
clientID := gwLrpClientID(vpcId)
hostname := "spinifex-gw-" + vpcId
vendorClass := "mulga-spinifex-gw-lrp"
lease, dhcpErr := dhcp.RequestAcquire(h.nc, pool.DhcpBindBridge, clientID, hostname, vendorClass, pool.Name, "")
if dhcpErr != nil {
return "", 0, fmt.Errorf("dhcp acquire gw LRP IP for vpc %s: %w", vpcId, dhcpErr)
}
prefix = prefixFromMask(lease.SubnetMask)
if prefix == 0 {
prefix = pool.PrefixLen
}
if prefix <= 0 || prefix > 32 {
return "", 0, fmt.Errorf("dhcp gw LRP for vpc %s: cannot determine prefix (mask=%q pool prefix=%d)", vpcId, lease.SubnetMask, pool.PrefixLen)
}
return lease.IP, prefix, nil
}
// releaseGatewayLRPLease asks vpcd's DHCPManager to drop the lease held for
// vpcId's gateway LRP. Best-effort — log on failure but never block IGW
// detach, since the upstream server will eventually expire the lease on
// its own.
func (h *TopologyHandler) releaseGatewayLRPLease(vpcId string) {
if h.nc == nil {
return
}
if err := dhcp.RequestRelease(h.nc, gwLrpClientID(vpcId)); err != nil {
slog.Warn("vpcd: dhcp release for gw LRP failed", "vpc_id", vpcId, "err", err)
}
}
// prefixFromMask converts a dotted-decimal mask ("255.255.255.0") to a
// prefix length. Returns 0 when the mask is empty or unparseable.
func prefixFromMask(mask string) int {
if mask == "" {
return 0
}
ip := net.ParseIP(mask).To4()
if ip == nil {
return 0
}
ones, bits := net.IPMask(ip).Size()
if bits != 32 {
return 0
}
return ones
}
// 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
spinifex/services/vpcd/topology_manager.go
+3 -6
View File
@@ -120,10 +120,9 @@ func (h *TopologyHandler) natManager() (policy.NATManager, error) {
return h.natm, h.natmErr
}
// igwManager returns the lazily-constructed external.IGWManager. Only the
// static-pool / distributed-NAT IGW subscriber path goes through it; the
// DHCP-coupled centralised path is held in attachIGWLegacy until bead
// mulga-siv-125.3.3 removes the vpcd-local DHCP manager.
// igwManager returns the lazily-constructed external.IGWManager. The
// static-pool / distributed-NAT IGW subscriber path goes through it;
// attachIGWLegacy in topology.go is dead code awaiting Slice D deletion.
func (h *TopologyHandler) igwManager() (external.IGWManager, error) {
h.igwmOnce.Do(func() {
nm, err := h.natManager()
@@ -139,7 +138,6 @@ func (h *TopologyHandler) igwManager() (external.IGWManager, error) {
if p := h.findExternalPool("", ""); p != nil {
shared := external.ExternalPoolConfig{
Name: p.Name,
Source: p.Source,
RangeStart: p.RangeStart,
RangeEnd: p.RangeEnd,
Gateway: p.Gateway,
@@ -148,7 +146,6 @@ func (h *TopologyHandler) igwManager() (external.IGWManager, error) {
DNSServers: p.DNSServers,
Region: p.Region,
AZ: p.AZ,
DhcpBindBridge: p.DhcpBindBridge,
GwLrpRangeStart: p.GwLrpRangeStart,
GwLrpRangeEnd: p.GwLrpRangeEnd,
}
spinifex/services/vpcd/topology_test.go
-262
View File
@@ -11,7 +11,6 @@ import (
"time"
"github.com/mulgadc/spinifex/spinifex/network/topology"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/nbdb"
"github.com/mulgadc/spinifex/spinifex/testutil"
"github.com/mulgadc/spinifex/spinifex/types"
@@ -1373,267 +1372,6 @@ func TestTopologyHandler_IGWAttach_CentralizedNATAllocatesGwLrpIP(t *testing.T)
}
}
// dhcpStub is a minimal in-test responder for vpc.dhcp.acquire / release.
// Each acquire returns a distinct lease IP keyed off ClientID so multi-VPC
// tests get unique IPs without standing up a full DHCPManager.
type dhcpStub struct {
t *testing.T
leases map[string]string // client_id -> ip
released map[string]bool
nextOct int
}
func newDHCPStub(t *testing.T, nc *nats.Conn) *dhcpStub {
t.Helper()
s := &dhcpStub{
t: t,
leases: map[string]string{},
released: map[string]bool{},
nextOct: 101,
}
subA, err := nc.Subscribe(dhcp.TopicAcquire, func(msg *nats.Msg) {
var req dhcp.AcquireRequestMsg
if err := json.Unmarshal(msg.Data, &req); err != nil {
data, _ := json.Marshal(dhcp.AcquireReplyMsg{Error: err.Error()})
_ = msg.Respond(data)
return
}
ip, ok := s.leases[req.ClientID]
if !ok {
ip = fmt.Sprintf("192.168.3.%d", s.nextOct)
s.nextOct++
s.leases[req.ClientID] = ip
}
reply := dhcp.AcquireReplyMsg{
IP: ip,
SubnetMask: "255.255.255.0",
ServerID: "192.168.3.1",
Routers: []string{"192.168.3.1"},
HWAddr: "02:00:00:00:00:" + fmt.Sprintf("%02x", s.nextOct%256),
ExpiresUnix: 0,
}
data, _ := json.Marshal(reply)
_ = msg.Respond(data)
})
if err != nil {
t.Fatalf("subscribe acquire: %v", err)
}
subR, err := nc.Subscribe(dhcp.TopicRelease, func(msg *nats.Msg) {
var req dhcp.ReleaseRequestMsg
_ = json.Unmarshal(msg.Data, &req)
s.released[req.ClientID] = true
delete(s.leases, req.ClientID)
data, _ := json.Marshal(dhcp.ReleaseReplyMsg{})
_ = msg.Respond(data)
})
if err != nil {
t.Fatalf("subscribe release: %v", err)
}
t.Cleanup(func() {
_ = subA.Unsubscribe()
_ = subR.Unsubscribe()
})
return s
}
// TestTopologyHandler_IGWAttach_DHCPGwLrpIP guards mulga-siv-38: with
// pool.Source="dhcp" the gateway LRP IP must come from a DHCP lease
// (vpc.dhcp.acquire), not the static gw_lrp_range allocator. Three VPCs
// sharing one DHCP pool must each get a distinct lease IP and persist it
// on external_ids:spinifex:gateway_ip. IGW detach must release the lease.
func TestTopologyHandler_IGWAttach_DHCPGwLrpIP(t *testing.T) {
t.Skip("DHCP source no longer supported; Slice C deletes attachIGWLegacy + DHCP gateway-LRP path")
_, nc := startTestNATS(t)
stub := newDHCPStub(t, nc)
mock := NewMockOVNClient()
_ = mock.Connect(context.Background())
ctx := context.Background()
pools := []ExternalPoolConfig{
{
Name: "lan-dhcp",
Source: "dhcp",
Gateway: "192.168.3.1",
PrefixLen: 24,
DhcpBindBridge: "br-wan",
},
}
topo := NewTopologyHandler(mock,
WithExternalNetwork("pool", pools),
WithBridgeMode(BridgeModeVeth),
WithNATSConn(nc),
)
subs, err := topo.Subscribe(nc)
if err != nil {
t.Fatalf("subscribe: %v", err)
}
defer func() {
for _, s := range subs {
_ = s.Unsubscribe()
}
}()
vpcIds := []string{"vpc-dh-a", "vpc-dh-b", "vpc-dh-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)
}
ip, cidr, _ := strings.Cut(gwPort.Networks[0], "/")
if cidr != "24" {
t.Errorf("%s: expected /24 from DHCP lease mask, got /%s", gwPortName, cidr)
}
if !strings.HasPrefix(ip, "192.168.3.") {
t.Errorf("%s: expected 192.168.3.X from stub, got %s", gwPortName, ip)
}
gwIP := gwPort.ExternalIDs["spinifex:gateway_ip"]
if gwIP != ip {
t.Errorf("%s: external_ids gateway_ip=%q, expected %q", gwPortName, gwIP, ip)
}
if other, dup := seen[gwIP]; dup {
t.Errorf("gw IP collision: %s and %s share %s", gwPortName, other, gwIP)
}
seen[gwIP] = gwPortName
// stub recorded the lease keyed by gw-lrp-<vpcId>
clientID := "gw-lrp-" + vpcId
if stub.leases[clientID] != ip {
t.Errorf("%s: stub lease %q != lrp IP %q", clientID, stub.leases[clientID], ip)
}
}
// Detach one VPC, assert release reaches the stub.
detachVpc := vpcIds[0]
devt := types.IGWEvent{InternetGatewayId: "igw-" + detachVpc, VpcId: detachVpc}
ddata, _ := json.Marshal(devt)
resp, err := nc.Request(TopicIGWDetach, ddata, 5_000_000_000)
if err != nil {
t.Fatalf("request vpc.igw-detach: %v", err)
}
assertSuccess(t, resp, "detach IGW "+detachVpc)
if !stub.released["gw-lrp-"+detachVpc] {
t.Errorf("expected DHCP release for gw-lrp-%s, stub released=%v", detachVpc, stub.released)
}
}
// TestTopologyHandler_IGWAttach_DHCPAcquireFailureAborts ensures that when
// vpcd's DHCPManager replies with an error, IGW attach surfaces the failure
// loud instead of falling back to link-local. Otherwise we silently restore
// the broken pre-siv-36 behavior.
func TestTopologyHandler_IGWAttach_DHCPAcquireFailureAborts(t *testing.T) {
t.Skip("DHCP source no longer supported; Slice C deletes attachIGWLegacy + DHCP gateway-LRP path")
_, nc := startTestNATS(t)
sub, err := nc.Subscribe(dhcp.TopicAcquire, func(msg *nats.Msg) {
reply := dhcp.AcquireReplyMsg{Error: "no leases available"}
data, _ := json.Marshal(reply)
_ = msg.Respond(data)
})
if err != nil {
t.Fatalf("subscribe: %v", err)
}
defer func() { _ = sub.Unsubscribe() }()
mock := NewMockOVNClient()
_ = mock.Connect(context.Background())
ctx := context.Background()
pools := []ExternalPoolConfig{
{
Name: "lan-dhcp",
Source: "dhcp",
Gateway: "192.168.3.1",
PrefixLen: 24,
DhcpBindBridge: "br-wan",
},
}
topo := NewTopologyHandler(mock,
WithExternalNetwork("pool", pools),
WithBridgeMode(BridgeModeVeth),
WithNATSConn(nc),
)
subs, err := topo.Subscribe(nc)
if err != nil {
t.Fatalf("subscribe: %v", err)
}
defer func() {
for _, s := range subs {
_ = s.Unsubscribe()
}
}()
_ = mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{
Name: "vpc-vpc-dh-fail",
ExternalIDs: map[string]string{
"spinifex:vpc_id": "vpc-dh-fail",
"spinifex:cidr": "10.0.0.0/16",
},
})
evt := types.IGWEvent{InternetGatewayId: "igw-fail", VpcId: "vpc-dh-fail"}
data, _ := json.Marshal(evt)
resp, _ := nc.Request(TopicIGWAttach, data, 5_000_000_000)
var reply struct {
Error string `json:"error"`
}
_ = json.Unmarshal(resp.Data, &reply)
if reply.Error == "" {
t.Fatal("expected attach to fail when DHCP acquire errors, got success")
}
if !strings.Contains(reply.Error, "dhcp acquire") && !strings.Contains(reply.Error, "no leases available") {
t.Errorf("expected dhcp-related error, got %q", reply.Error)
}
// External switch must be rolled back.
if _, err := mock.GetLogicalSwitch(ctx, "ext-vpc-dh-fail"); err == nil {
t.Error("expected external switch rolled back after DHCP failure")
}
}
// TestPrefixFromMask checks dotted-mask → prefix conversion across the
// boundary cases gw LRP DHCP relies on (option 1 missing, /32, /0,
// non-contiguous masks, IPv6).
func TestPrefixFromMask(t *testing.T) {
cases := []struct {
mask string
want int
}{
{"", 0},
{"255.255.255.0", 24},
{"255.255.255.255", 32},
{"0.0.0.0", 0},
{"255.255.0.0", 16},
{"not-an-ip", 0},
}
for _, tc := range cases {
t.Run(tc.mask, func(t *testing.T) {
got := prefixFromMask(tc.mask)
if got != tc.want {
t.Errorf("prefixFromMask(%q)=%d, want %d", tc.mask, got, tc.want)
}
})
}
}
func TestTopologyHandler_FindExternalPool(t *testing.T) {
pools := []ExternalPoolConfig{
{Name: "az-a", Region: "us-east-1", AZ: "us-east-1a", RangeStart: "1.1.1.1"},
spinifex/services/vpcd/vpcd.go
+23 -112
View File
@@ -19,9 +19,7 @@ import (
"github.com/mulgadc/spinifex/spinifex/network/host"
"github.com/mulgadc/spinifex/spinifex/network/policy"
"github.com/mulgadc/spinifex/spinifex/network/reconcile"
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go"
)
// sudoCommand wraps exec.Command with sudo when running as non-root.
@@ -86,7 +84,6 @@ type Config struct {
// ExternalPoolConfig mirrors config.ExternalPool for vpcd's internal use.
type ExternalPoolConfig struct {
Name string
Source string // "static" (default) or "dhcp"
RangeStart string
RangeEnd string
Gateway string
@@ -95,13 +92,13 @@ type ExternalPoolConfig struct {
DNSServers []string
Region string
AZ string
DhcpBindBridge string // Bridge where the DHCP AF_PACKET socket binds (e.g. "br-wan"). Required for source="dhcp".
GwLrpRangeStart string // Sub-range for OVN gateway LRP IPs in centralized NAT mode (mulga-siv-36).
GwLrpRangeEnd string
}
// IsDHCP is retained until Slice C removes the DHCP-source code paths; loader
// rejects source="dhcp" so this always returns false in steady state.
// IsDHCP is a dead stub retained for the unreachable attachIGWLegacy
// dispatcher branch in topology.go; Slice D deletes both together
// (mulga-siv-125.3.3).
func (p *ExternalPoolConfig) IsDHCP() bool {
return false
}
@@ -413,16 +410,16 @@ func launchService(cfg *Config) error {
defer liveClient.Close()
slog.Info("Connected to OVN NB DB", "endpoint", cfg.OVNNBAddr)
bridgeMode, dhcpBindBridge := resolveBridgeConfig(cfg.BridgeMode, cfg.ExternalInterface)
slog.Info("External bridge mode", "mode", bridgeMode, "dhcp_bind_bridge", dhcpBindBridge)
if err := verifyBridgeMode(bridgeMode, cfg.ExternalInterface, dhcpBindBridge); err != nil {
bridgeMode, wanBridge := resolveBridgeConfig(cfg.BridgeMode, cfg.ExternalInterface)
slog.Info("External bridge mode", "mode", bridgeMode, "wan_bridge", wanBridge)
if err := verifyBridgeMode(bridgeMode, cfg.ExternalInterface, wanBridge); err != nil {
slog.Error("vpcd: bridge mode sanity check failed", "err", err)
return err
}
// Resolve external CIDR from the WAN bridge before any reconcile. Skipped
// when external networking is disabled (overlay-only deployments).
if err := ensureExternalCIDRReady(ctx, cfg.ExternalMode, dhcpBindBridge); err != nil {
if err := ensureExternalCIDRReady(ctx, cfg.ExternalMode, wanBridge); err != nil {
return err
}
@@ -449,7 +446,6 @@ func launchService(cfg *Config) error {
topoOpts = append(topoOpts, WithChassisNames(chassisNames))
slog.Info("vpcd: gateway chassis discovered", "chassis", chassisNames)
topoOpts = append(topoOpts, WithBridgeMode(bridgeMode))
topoOpts = append(topoOpts, WithNATSConn(nc))
topo := NewTopologyHandler(liveClient, topoOpts...)
// Elect a single vpcd to run startup reconcile. Without this, N vpcds in a
@@ -472,51 +468,11 @@ func launchService(cfg *Config) error {
}
}()
// DHCP manager: services vpc.dhcp.acquire/release requests from the
// daemon-side ExternalIPAM handlers and (mulga-siv-38) topology's gw-LRP
// allocator. MUST subscribe before any reconcile pass — bootstrap
// reconcile, ReconcileFromKV, and retrofit all call into reconcileIGW /
// expectedGatewayPortNetwork which fan out vpc.dhcp.acquire on
// source="dhcp" pools. Without a live subscription those requests hit
// "no responders" and burn the 60-attempt cold-boot retry budget per
// existing IGW. Started unconditionally — pools with source="static"
// never issue acquire requests, so the Manager sits idle until
// something actually wants DHCP.
js, err := nc.JetStream()
if err != nil {
slog.Error("Failed to get JetStream context for DHCP manager", "err", err)
slog.Error("Failed to get JetStream context", "err", err)
return err
}
// timeout/retry args are legacy no-ops (mulga-siv-39): DHCPManager
// owns DORA retransmission via acquireWithBackoff.
//
// Retry on JetStream-not-ready: NewDHCPManager creates the DHCP-leases
// KV bucket, which fails immediately if the JetStream cluster has not
// reached quorum. On multi-node bring-up vpcd's launchService runs
// before all peers are reachable, so we mirror the daemon's
// initJetStream backoff (500ms→10s, 5 min cap) instead of crashing the
// service and aborting the rest of the node bring-up.
dhcpManager, err := newDHCPManagerWithRetry(ctx, nc, js)
if err != nil {
slog.Error("Failed to create DHCP manager", "err", err)
return err
}
defer dhcpManager.Close()
if err := dhcpManager.Bootstrap(ctx); err != nil {
slog.Error("DHCP manager bootstrap failed", "err", err)
return err
}
dhcpSubs, err := dhcpManager.Subscribe(nc)
if err != nil {
slog.Error("DHCP manager subscribe failed", "err", err)
return err
}
defer func() {
for _, s := range dhcpSubs {
_ = s.Unsubscribe()
}
}()
// Construct the network/reconcile manager stack. The reconciler replaces
// the five-pass startup sequence (Reconcile + ReconcileFromKV +
@@ -598,7 +554,7 @@ func launchService(cfg *Config) error {
}()
slog.Info("vpcd service started, waiting for VPC lifecycle events",
"subscriptions", len(subs), "dhcp_subscriptions", len(dhcpSubs))
"subscriptions", len(subs))
// Wait for shutdown signal
sigChan := make(chan os.Signal, 1)
@@ -611,48 +567,6 @@ func launchService(cfg *Config) error {
return nil
}
// newDHCPManagerWithRetry constructs a DHCPManager, retrying on transient
// JetStream-not-ready errors with exponential backoff (500ms→10s, capped
// at 5 min). NewDHCPManager creates the spinifex-dhcp-leases KV bucket;
// on multi-node bring-up the JetStream cluster may not yet have quorum
// when vpcd starts, and a single-shot CreateKeyValue would otherwise crash
// the service before the rest of the node finishes coming up.
//
// Mirrors daemon.initJetStream's backoff so vpcd waits as patiently as the
// daemon does for the cluster to form.
func newDHCPManagerWithRetry(ctx context.Context, nc *nats.Conn, js nats.JetStreamContext) (*DHCPManager, error) {
const maxWait = 5 * time.Minute
retryDelay := 500 * time.Millisecond
start := time.Now()
attempt := 0
for {
attempt++
mgr, err := NewDHCPManager(nc, js, dhcp.NewNClient4(0, 0))
if err == nil {
if attempt > 1 {
slog.Info("vpcd: DHCP manager ready", "attempts", attempt, "elapsed", time.Since(start).Round(time.Second))
}
return mgr, nil
}
elapsed := time.Since(start)
if elapsed >= maxWait {
return nil, fmt.Errorf("DHCP manager init timed out after %s (%d attempts): %w", elapsed.Round(time.Second), attempt, err)
}
slog.Warn("vpcd: DHCP manager not ready (waiting for JetStream cluster quorum)",
"err", err, "attempt", attempt, "elapsed", elapsed.Round(time.Second), "retryIn", retryDelay)
select {
case <-ctx.Done():
return nil, fmt.Errorf("DHCP manager init cancelled: %w", ctx.Err())
case <-time.After(retryDelay):
}
retryDelay = min(retryDelay*2, 10*time.Second)
}
}
// externalPoolConfigToShared translates the vpcd-local ExternalPoolConfig
// into the network/external package's shared shape. The two will merge
// once topology.go is split (bead mulga-siv-125.3.4 — see external.go's
@@ -660,7 +574,6 @@ func newDHCPManagerWithRetry(ctx context.Context, nc *nats.Conn, js nats.JetStre
func externalPoolConfigToShared(p ExternalPoolConfig) external.ExternalPoolConfig {
return external.ExternalPoolConfig{
Name: p.Name,
Source: p.Source,
RangeStart: p.RangeStart,
RangeEnd: p.RangeEnd,
Gateway: p.Gateway,
@@ -669,7 +582,6 @@ func externalPoolConfigToShared(p ExternalPoolConfig) external.ExternalPoolConfi
DNSServers: p.DNSServers,
Region: p.Region,
AZ: p.AZ,
DhcpBindBridge: p.DhcpBindBridge,
GwLrpRangeStart: p.GwLrpRangeStart,
GwLrpRangeEnd: p.GwLrpRangeEnd,
}
@@ -744,37 +656,36 @@ var readLinkMaster = func(iface string) (string, error) {
// verifyBridgeMode is the post-detect sanity check. It refuses to start vpcd
// when the chosen bridge mode does not match the host plumbing (D4+D18):
//
// - direct: ExternalInterface must be an OVS port on DhcpBindBridge. That is
// the whole contract of direct mode.
// - direct: ExternalInterface must be an OVS port on the WAN bridge. That
// is the whole contract of direct mode.
// - veth: (a) veth-wan-ovs must be an OVS port on OvnExternalBridge — the
// OVN side, owned by setup-ovn.sh's ovn-bridge-mappings. (b) veth-wan-br
// must be enslaved to DhcpBindBridge — the Linux side, where the DHCP
// client sees LAN frames.
// must be enslaved to the WAN bridge — the Linux side.
// - empty / unknown: fail with the list of supported values.
//
// Fail-start, not soft-degrade — the distributed-NAT-on-veth-host footgun is
// exactly what this plan set out to kill.
func verifyBridgeMode(mode, externalIface, dhcpBindBridge string) error {
func verifyBridgeMode(mode, externalIface, wanBridge string) error {
switch mode {
case BridgeModeDirect:
if externalIface == "" {
return fmt.Errorf("vpcd: direct bridge mode requires external_interface (the WAN NIC name)")
}
if dhcpBindBridge == "" {
return fmt.Errorf("vpcd: direct bridge mode requires dhcp_bind_bridge (the OVS bridge holding the WAN NIC)")
if wanBridge == "" {
return fmt.Errorf("vpcd: direct bridge mode requires the WAN bridge (the OVS bridge holding the WAN NIC)")
}
br, err := portToBr(externalIface)
if err != nil {
return fmt.Errorf("vpcd: direct bridge mode: %w", err)
}
if br != dhcpBindBridge {
return fmt.Errorf("vpcd: direct bridge mode: %q is on OVS bridge %q, expected %q (dhcp_bind_bridge)",
externalIface, br, dhcpBindBridge)
if br != wanBridge {
return fmt.Errorf("vpcd: direct bridge mode: %q is on OVS bridge %q, expected %q",
externalIface, br, wanBridge)
}
return nil
case BridgeModeVeth:
if dhcpBindBridge == "" {
return fmt.Errorf("vpcd: veth bridge mode requires dhcp_bind_bridge (the Linux bridge holding the WAN NIC)")
if wanBridge == "" {
return fmt.Errorf("vpcd: veth bridge mode requires wan_bridge (the Linux bridge holding the WAN NIC)")
}
br, err := portToBr("veth-wan-ovs")
if err != nil {
@@ -788,9 +699,9 @@ func verifyBridgeMode(mode, externalIface, dhcpBindBridge string) error {
if err != nil {
return fmt.Errorf("vpcd: veth bridge mode: veth-wan-br missing or has no master — is spinifex-veth-wan.service running? %w", err)
}
if master != dhcpBindBridge {
return fmt.Errorf("vpcd: veth bridge mode: veth-wan-br master is %q, expected %q (dhcp_bind_bridge)",
master, dhcpBindBridge)
if master != wanBridge {
return fmt.Errorf("vpcd: veth bridge mode: veth-wan-br master is %q, expected %q (wan_bridge)",
master, wanBridge)
}
return nil
default:
spinifex/services/vpcd/vpcd_test.go
+4 -4
View File
@@ -258,15 +258,15 @@ func TestVerifyBridgeMode_DirectMissingExternalIface(t *testing.T) {
func TestVerifyBridgeMode_DirectMissingBindBridge(t *testing.T) {
err := verifyBridgeMode(BridgeModeDirect, "enp0s3", "")
if err == nil || !strings.Contains(err.Error(), "dhcp_bind_bridge") {
t.Fatalf("expected dhcp_bind_bridge error, got: %v", err)
if err == nil || !strings.Contains(err.Error(), "WAN bridge") {
t.Fatalf("expected WAN bridge error, got: %v", err)
}
}
func TestVerifyBridgeMode_VethMissingBindBridge(t *testing.T) {
err := verifyBridgeMode(BridgeModeVeth, "", "")
if err == nil || !strings.Contains(err.Error(), "dhcp_bind_bridge") {
t.Fatalf("expected dhcp_bind_bridge error, got: %v", err)
if err == nil || !strings.Contains(err.Error(), "wan_bridge") {
t.Fatalf("expected wan_bridge error, got: %v", err)
}
}