From a6a60b5ba65f5c08def27059da1d9840834f1cd0 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Wed, 10 Jun 2026 10:11:14 +1000 Subject: [PATCH] fix: persist EIP MAC so distributed NAT survives host reboot (#333) Associated Elastic IPs lost external connectivity after a host reboot: vpcd's reconcile re-applied the dnat_and_snat rule without the ENI MAC, collapsing it to the centralised shape (external_mac/logical_port unset), and the daemon's relaunch republish only fired for auto-assigned public IPs, never for store-backed EIPs. - EIPRecord gains MacAddress, captured at AssociateAddress and cleared at DisassociateAddress, so reconcile can re-apply the distributed shape without re-querying the ENI. - loadEIPs feeds rec.MacAddress into the reconcile spec; policy.AddEIP self-heals a stale centralised row when given a MAC. - AssociatedPublicIPForInstance lets the relaunch hook resolve an associated EIP from the store and republish vpc.add-nat with the MAC. --- spinifex/daemon/vm_adapters.go | 17 ++++++++++-- spinifex/handlers/ec2/eip/service_impl.go | 34 +++++++++++++++++++++++ spinifex/handlers/ec2/eip/types.go | 27 ++++++++++-------- spinifex/network/reconcile/intent.go | 4 +++ 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/spinifex/daemon/vm_adapters.go b/spinifex/daemon/vm_adapters.go index fb4e6f9f1..0203a800d 100644 --- a/spinifex/daemon/vm_adapters.go +++ b/spinifex/daemon/vm_adapters.go @@ -442,7 +442,18 @@ func (d *Daemon) onInstanceUpHook() func(*vm.VM) error { // Idempotent: vpcd's handler deletes-then-adds (topology.go:1297), // so this is a no-op on fresh launches where the initial publish // already fired. - if d.natsConn != nil && instance.PublicIP != "" && instance.ENIId != "" && instance.Instance != nil { + // + // instance.PublicIP only carries auto-assigned public IPs; an + // associated Elastic IP lives in the EIP store, so resolve it from + // there when the instance field is empty — otherwise a rebooted host + // never re-announces the EIP's NAT and the VM loses connectivity. + if d.natsConn != nil && instance.ENIId != "" && instance.Instance != nil { + publicIP := instance.PublicIP + if publicIP == "" && d.eipService != nil { + if eipIP, ok := d.eipService.AssociatedPublicIPForInstance(instance.AccountID, instance.ID); ok { + publicIP = eipIP + } + } vpcID := "" privateIP := "" if instance.Instance.VpcId != nil { @@ -451,9 +462,9 @@ func (d *Daemon) onInstanceUpHook() func(*vm.VM) error { if instance.Instance.PrivateIpAddress != nil { privateIP = *instance.Instance.PrivateIpAddress } - if vpcID != "" && privateIP != "" { + if publicIP != "" && vpcID != "" && privateIP != "" { portName := topology.Port(instance.ENIId) - utils.PublishNATEvent(d.natsConn, "vpc.add-nat", vpcID, instance.PublicIP, privateIP, portName, instance.ENIMac) + utils.PublishNATEvent(d.natsConn, "vpc.add-nat", vpcID, publicIP, privateIP, portName, instance.ENIMac) } } return nil diff --git a/spinifex/handlers/ec2/eip/service_impl.go b/spinifex/handlers/ec2/eip/service_impl.go index 02526a326..c52ccd0cb 100644 --- a/spinifex/handlers/ec2/eip/service_impl.go +++ b/spinifex/handlers/ec2/eip/service_impl.go @@ -213,6 +213,7 @@ func (s *EIPServiceImpl) AssociateAddress(input *ec2.AssociateAddressInput, acco record.InstanceId = instanceID record.PrivateIp = privateIP record.VpcId = vpcID + record.MacAddress = macAddr record.State = "associated" data, err := json.Marshal(record) @@ -270,6 +271,7 @@ func (s *EIPServiceImpl) DisassociateAddress(input *ec2.DisassociateAddressInput record.InstanceId = "" record.PrivateIp = "" record.VpcId = "" + record.MacAddress = "" record.State = "allocated" data, err := json.Marshal(record) @@ -568,6 +570,38 @@ func (s *EIPServiceImpl) findByAssociationID(accountID, associationID string) (* return nil, "", 0, errors.New(awserrors.ErrorInvalidAssociationIDNotFound) } +// AssociatedPublicIPForInstance returns the public IP of the Elastic IP +// associated with instanceID, if any. The daemon uses it to re-announce a +// post-launch EIP's dnat_and_snat on VM relaunch, where the instance's own +// PublicIP field is unset (only auto-assigned public IPs populate it). +func (s *EIPServiceImpl) AssociatedPublicIPForInstance(accountID, instanceID string) (string, bool) { + if instanceID == "" { + return "", false + } + prefix := accountID + "." + keys, err := s.eipKV.Keys() + if err != nil { + return "", false + } + for _, k := range keys { + if k == utils.VersionKey || !strings.HasPrefix(k, prefix) { + continue + } + entry, err := s.eipKV.Get(k) + if err != nil { + continue + } + var record EIPRecord + if err := json.Unmarshal(entry.Value(), &record); err != nil { + continue + } + if record.State == "associated" && record.InstanceId == instanceID && record.PublicIp != "" { + return record.PublicIp, true + } + } + return "", false +} + // publishNATEvent publishes a NAT lifecycle event to NATS for vpcd consumption. // This is fire-and-forget; errors are logged but do not fail the API response. // diff --git a/spinifex/handlers/ec2/eip/types.go b/spinifex/handlers/ec2/eip/types.go index 2fdfe0e3c..13a16f50a 100644 --- a/spinifex/handlers/ec2/eip/types.go +++ b/spinifex/handlers/ec2/eip/types.go @@ -9,15 +9,20 @@ const ( // EIPRecord represents a stored Elastic IP address allocation. type EIPRecord struct { - AllocationId string `json:"allocation_id"` - PublicIp string `json:"public_ip"` - PoolName string `json:"pool_name"` - AssociationId string `json:"association_id,omitempty"` - ENIId string `json:"eni_id,omitempty"` - InstanceId string `json:"instance_id,omitempty"` - PrivateIp string `json:"private_ip,omitempty"` - VpcId string `json:"vpc_id,omitempty"` - State string `json:"state"` // "allocated", "associated" - Tags map[string]string `json:"tags"` - CreatedAt time.Time `json:"created_at"` + AllocationId string `json:"allocation_id"` + PublicIp string `json:"public_ip"` + PoolName string `json:"pool_name"` + AssociationId string `json:"association_id,omitempty"` + ENIId string `json:"eni_id,omitempty"` + InstanceId string `json:"instance_id,omitempty"` + PrivateIp string `json:"private_ip,omitempty"` + // MacAddress is the associated ENI's MAC, captured at associate-time so + // the vpcd reconcile can re-apply the distributed-shape dnat_and_snat + // (external_mac/logical_port) after a host reboot without re-querying the + // ENI. Empty while the EIP is unassociated. + MacAddress string `json:"mac_address,omitempty"` + VpcId string `json:"vpc_id,omitempty"` + State string `json:"state"` // "allocated", "associated" + Tags map[string]string `json:"tags"` + CreatedAt time.Time `json:"created_at"` } diff --git a/spinifex/network/reconcile/intent.go b/spinifex/network/reconcile/intent.go index bfbccb3d3..d97ef29fe 100644 --- a/spinifex/network/reconcile/intent.go +++ b/spinifex/network/reconcile/intent.go @@ -491,6 +491,10 @@ func loadEIPs(js nats.JetStreamContext, localVPCs map[string]struct{}, out map[s if rec.ENIId != "" { spec.PortName = topology.Port(rec.ENIId) } + // MAC drives the distributed dnat_and_snat shape in policy.AddEIP; an + // empty MAC re-applies the rule centralised (external_mac/logical_port + // unset), which is the host-reboot connectivity regression this avoids. + spec.MAC = rec.MacAddress out[rec.PrivateIp] = spec } return nil