mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
fix: clean up stale NAT rules before adding new ones for reused IPs
When a public IP is returned to the external pool and quickly reallocated (e.g. instance terminated, then LB created), the fire-and-forget vpc.delete-nat may not have been processed yet. handleAddNAT now removes any existing dnat_and_snat rule for the same external IP before creating the new one, preventing duplicate/conflicting OVN NAT rules. This was causing intermittent NLB connectivity failures in multi-node E2E where NLB public IPs collided with recently-freed instance IPs.
This commit is contained in:
@@ -375,6 +375,34 @@ func (m *MockOVNClient) DeleteNAT(_ context.Context, routerName string, natType,
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (m *MockOVNClient) DeleteNATByExternalIP(_ context.Context, routerName string, natType, externalIP string) error {
|
||||||
|
m.mu.Lock()
|
||||||
|
defer m.mu.Unlock()
|
||||||
|
lr, exists := m.routers[routerName]
|
||||||
|
if !exists {
|
||||||
|
return fmt.Errorf("logical router %q not found", routerName)
|
||||||
|
}
|
||||||
|
var foundUUIDs []string
|
||||||
|
for uuid, n := range m.nats {
|
||||||
|
if n.Type == natType && n.ExternalIP == externalIP {
|
||||||
|
foundUUIDs = append(foundUUIDs, uuid)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(foundUUIDs) == 0 {
|
||||||
|
return fmt.Errorf("NAT %s external_ip=%s not found", natType, externalIP)
|
||||||
|
}
|
||||||
|
for _, foundUUID := range foundUUIDs {
|
||||||
|
for i, uuid := range lr.NAT {
|
||||||
|
if uuid == foundUUID {
|
||||||
|
lr.NAT = append(lr.NAT[:i], lr.NAT[i+1:]...)
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
delete(m.nats, foundUUID)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// Static Routes
|
// Static Routes
|
||||||
|
|
||||||
func (m *MockOVNClient) AddStaticRoute(_ context.Context, routerName string, route *nbdb.LogicalRouterStaticRoute) error {
|
func (m *MockOVNClient) AddStaticRoute(_ context.Context, routerName string, route *nbdb.LogicalRouterStaticRoute) error {
|
||||||
|
|||||||
@@ -75,6 +75,7 @@ type OVNClient interface {
|
|||||||
// NAT rules
|
// NAT rules
|
||||||
AddNAT(ctx context.Context, routerName string, nat *nbdb.NAT) error
|
AddNAT(ctx context.Context, routerName string, nat *nbdb.NAT) error
|
||||||
DeleteNAT(ctx context.Context, routerName string, natType, logicalIP string) error
|
DeleteNAT(ctx context.Context, routerName string, natType, logicalIP string) error
|
||||||
|
DeleteNATByExternalIP(ctx context.Context, routerName string, natType, externalIP string) error
|
||||||
|
|
||||||
// Static routes
|
// Static routes
|
||||||
AddStaticRoute(ctx context.Context, routerName string, route *nbdb.LogicalRouterStaticRoute) error
|
AddStaticRoute(ctx context.Context, routerName string, route *nbdb.LogicalRouterStaticRoute) error
|
||||||
@@ -631,6 +632,50 @@ func (c *LiveOVNClient) DeleteNAT(ctx context.Context, routerName string, natTyp
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DeleteNATByExternalIP removes a NAT rule matching the given external IP.
|
||||||
|
// Returns an error if no matching rule is found (callers can ignore this).
|
||||||
|
func (c *LiveOVNClient) DeleteNATByExternalIP(ctx context.Context, routerName string, natType, externalIP string) error {
|
||||||
|
var nats []nbdb.NAT
|
||||||
|
err := c.client.WhereCache(func(n *nbdb.NAT) bool {
|
||||||
|
return n.Type == natType && n.ExternalIP == externalIP
|
||||||
|
}).List(ctx, &nats)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("find NAT by external IP: %w", err)
|
||||||
|
}
|
||||||
|
if len(nats) == 0 {
|
||||||
|
return fmt.Errorf("NAT %s external_ip=%s not found", natType, externalIP)
|
||||||
|
}
|
||||||
|
|
||||||
|
lr, err := c.GetLogicalRouter(ctx, routerName)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("get logical router for NAT delete: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
var allOps []ovsdb.Operation
|
||||||
|
for i := range nats {
|
||||||
|
nat := &nats[i]
|
||||||
|
mutateOps, mErr := c.client.Where(lr).Mutate(lr, model.Mutation{
|
||||||
|
Field: &lr.NAT,
|
||||||
|
Mutator: "delete",
|
||||||
|
Value: []string{nat.UUID},
|
||||||
|
})
|
||||||
|
if mErr != nil {
|
||||||
|
return fmt.Errorf("mutate router NAT ops: %w", mErr)
|
||||||
|
}
|
||||||
|
deleteOps, dErr := c.client.Where(nat).Delete()
|
||||||
|
if dErr != nil {
|
||||||
|
return fmt.Errorf("delete NAT ops: %w", dErr)
|
||||||
|
}
|
||||||
|
allOps = append(allOps, mutateOps...)
|
||||||
|
allOps = append(allOps, deleteOps...)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := c.transactOps(ctx, allOps); err != nil {
|
||||||
|
return fmt.Errorf("delete NAT by external IP transact: %w", err)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func (c *LiveOVNClient) AddStaticRoute(ctx context.Context, routerName string, route *nbdb.LogicalRouterStaticRoute) error {
|
func (c *LiveOVNClient) AddStaticRoute(ctx context.Context, routerName string, route *nbdb.LogicalRouterStaticRoute) error {
|
||||||
// Set a named UUID so the route can be referenced in the same transaction
|
// Set a named UUID so the route can be referenced in the same transaction
|
||||||
if route.UUID == "" {
|
if route.UUID == "" {
|
||||||
|
|||||||
@@ -915,6 +915,14 @@ func (h *TopologyHandler) handleAddNAT(msg *nats.Msg) {
|
|||||||
"external_ip", evt.ExternalIP, "port", evt.PortName, "mac", evt.MAC)
|
"external_ip", evt.ExternalIP, "port", evt.PortName, "mac", evt.MAC)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Remove any stale NAT rule for the same external IP before adding the new
|
||||||
|
// one. This prevents duplicate DNAT rules when a public IP is reused quickly
|
||||||
|
// (e.g. instance terminated, IP returned to pool, new LB allocated same IP
|
||||||
|
// before the fire-and-forget vpc.delete-nat was processed).
|
||||||
|
if err := h.ovn.DeleteNATByExternalIP(ctx, routerName, "dnat_and_snat", evt.ExternalIP); err != nil {
|
||||||
|
slog.Debug("vpcd: no existing NAT rule to clean up for external IP", "external_ip", evt.ExternalIP, "err", err)
|
||||||
|
}
|
||||||
|
|
||||||
if err := h.ovn.AddNAT(ctx, routerName, natRule); err != nil {
|
if err := h.ovn.AddNAT(ctx, routerName, natRule); err != nil {
|
||||||
slog.Error("vpcd: failed to add dnat_and_snat rule", "router", routerName, "externalIP", evt.ExternalIP, "logicalIP", evt.LogicalIP, "err", err)
|
slog.Error("vpcd: failed to add dnat_and_snat rule", "router", routerName, "externalIP", evt.ExternalIP, "logicalIP", evt.LogicalIP, "err", err)
|
||||||
respond(msg, err)
|
respond(msg, err)
|
||||||
|
|||||||
Reference in New Issue
Block a user