diff --git a/spinifex/services/vpcd/mock_ovn.go b/spinifex/services/vpcd/mock_ovn.go index 535375699..166d7e024 100644 --- a/spinifex/services/vpcd/mock_ovn.go +++ b/spinifex/services/vpcd/mock_ovn.go @@ -375,6 +375,34 @@ func (m *MockOVNClient) DeleteNAT(_ context.Context, routerName string, natType, 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 func (m *MockOVNClient) AddStaticRoute(_ context.Context, routerName string, route *nbdb.LogicalRouterStaticRoute) error { diff --git a/spinifex/services/vpcd/ovn.go b/spinifex/services/vpcd/ovn.go index 8e9f60c29..873058ff5 100644 --- a/spinifex/services/vpcd/ovn.go +++ b/spinifex/services/vpcd/ovn.go @@ -75,6 +75,7 @@ type OVNClient interface { // NAT rules AddNAT(ctx context.Context, routerName string, nat *nbdb.NAT) error DeleteNAT(ctx context.Context, routerName string, natType, logicalIP string) error + DeleteNATByExternalIP(ctx context.Context, routerName string, natType, externalIP string) error // Static routes 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 } +// 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 { // Set a named UUID so the route can be referenced in the same transaction if route.UUID == "" { diff --git a/spinifex/services/vpcd/topology.go b/spinifex/services/vpcd/topology.go index cca3b3fc4..de55b96fa 100644 --- a/spinifex/services/vpcd/topology.go +++ b/spinifex/services/vpcd/topology.go @@ -915,6 +915,14 @@ func (h *TopologyHandler) handleAddNAT(msg *nats.Msg) { "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 { slog.Error("vpcd: failed to add dnat_and_snat rule", "router", routerName, "externalIP", evt.ExternalIP, "logicalIP", evt.LogicalIP, "err", err) respond(msg, err)