mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
fix(reconcile): call AttachIGW when IGW topology missing
applyIGWs previously deferred to the NATS handler when an intent IGW had no external switch in OVN. When the vpc.igw-attach event is lost (bootstrap-time default-VPC IGW published before vpcd subscriber is ready), deferral never resolves and VM public-IP traffic dies. Call IGWManager.AttachIGW directly — it is idempotent (short-circuits when the external switch already exists), so the reconciler converges default-VPC IGWs on first pass without duplicating work on subsequent passes. Closes mulga-siv-132.
This commit is contained in:
@@ -224,27 +224,20 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual
|
||||
}
|
||||
}
|
||||
|
||||
// applyIGWs rebinds Gateway_Chassis on every intent IGW whose OVN topology
|
||||
// already exists. Fresh-IGW creation flows through the NATS handler path
|
||||
// (handlers/ec2/igw → vpc.igw-attach event → TopologyHandler.reconcileIGW)
|
||||
// for the duration of Phase 2 — that path retains DHCP-uplink awareness
|
||||
// via dhcpManager, which network/external.NewIGWManager does not yet have
|
||||
// (bead mulga-siv-125.3.3 will remove the DHCP manager entirely; Phase 4
|
||||
// then migrates the handler onto external.IGWManager).
|
||||
//
|
||||
// Chassis-rebind drift handling stays in the reconciler because it's a
|
||||
// pure NB-DB-only fix-up: a system-id change across reboot leaves
|
||||
// Gateway_Chassis rows pointing at a chassis no ovn-controller owns
|
||||
// (mulga-999), and SetGatewayChassis is idempotent.
|
||||
//
|
||||
// The igw manager field stays wired (config requires it) so this stage
|
||||
// can grow into a full AttachIGW path once DHCP awareness lands in
|
||||
// network/external.
|
||||
// applyIGWs ensures every intent IGW has its OVN topology (external switch,
|
||||
// localnet LSP, gateway LRP, default route, Gateway_Chassis bindings) and
|
||||
// rebinds chassis on existing IGWs. AttachIGW is idempotent (short-circuits
|
||||
// when the external switch already exists), so calling it when actual state
|
||||
// is missing converges bootstrap-time default-VPC IGWs whose vpc.igw-attach
|
||||
// NATS event arrived before vpcd's subscriber was ready (mulga-siv-132).
|
||||
func (r *reconciler) applyIGWs(ctx context.Context, intent IntentState, actual ActualState) {
|
||||
for vpcID := range intent.IGWs {
|
||||
for vpcID, spec := range intent.IGWs {
|
||||
if _, ok := actual.ExternalSwch[vpcID]; !ok {
|
||||
slog.Debug("reconcile/apply: IGW topology missing in OVN, deferring to NATS handler path", "vpc_id", vpcID)
|
||||
continue
|
||||
if err := r.igw.AttachIGW(ctx, spec); err != nil {
|
||||
slog.Error("reconcile/apply: AttachIGW failed", "vpc_id", vpcID, "err", err)
|
||||
continue
|
||||
}
|
||||
actual.ExternalSwch[vpcID] = struct{}{}
|
||||
}
|
||||
r.rebindGatewayChassis(ctx, vpcID)
|
||||
}
|
||||
|
||||
@@ -225,6 +225,62 @@ func TestReconcile_ChassisRebindOnExistingIGW(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestReconcile_IGWAttachWhenTopologyMissing(t *testing.T) {
|
||||
m := mock.New()
|
||||
sg := policy.NewSecurityGroupManager(m)
|
||||
nat, _ := policy.NewNATManager(m, policy.NATModeDistributed)
|
||||
routes := policy.NewRouteManager(m)
|
||||
igw, _ := external.NewIGWManager(external.IGWManagerConfig{
|
||||
OVN: m, Routes: routes, NAT: nat,
|
||||
Allocator: external.LinkLocalAllocator{},
|
||||
NATMode: policy.NATModeDistributed,
|
||||
})
|
||||
topo := topology.NewLiveManager(m)
|
||||
rec, err := New(Config{
|
||||
OVN: m, SG: sg, NAT: nat, Routes: routes, IGW: igw, Topology: topo,
|
||||
LocalAZ: "us-east-1a", NodeHostname: "test-host",
|
||||
Chassis: []string{"chassis-1"},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
ctx := context.Background()
|
||||
|
||||
intent := IntentState{
|
||||
VPCs: map[string]topology.VPCSpec{
|
||||
"vpc-a": {VPCID: "vpc-a", CIDR: netip.MustParsePrefix("10.0.0.0/16"), VNI: 100},
|
||||
},
|
||||
Subnets: map[string]topology.SubnetSpec{},
|
||||
Ports: map[string]topology.PortSpec{},
|
||||
SGs: map[string]policy.SGSpec{},
|
||||
IGWs: map[string]external.IGWSpec{"vpc-a": {VPCID: "vpc-a", InternetGatewayID: "igw-a"}},
|
||||
EIPs: map[string]policy.EIPSpec{},
|
||||
NATGWs: map[string]policy.NATGWSpec{},
|
||||
}
|
||||
|
||||
if err := rec.Reconcile(ctx, intent); err != nil {
|
||||
t.Fatalf("Reconcile: %v", err)
|
||||
}
|
||||
|
||||
if _, ok := m.Switches[topology.ExternalSwitch("vpc-a")]; !ok {
|
||||
t.Errorf("external switch not created by reconciler AttachIGW path")
|
||||
}
|
||||
gwPort := topology.GatewayRouterPort("vpc-a")
|
||||
if _, ok := m.RouterPorts[gwPort]; !ok {
|
||||
t.Errorf("gateway LRP %s not created", gwPort)
|
||||
}
|
||||
|
||||
// Second pass must be idempotent — AttachIGW's first-line short-circuit
|
||||
// fires once the external switch exists, so total switch count stays put.
|
||||
switchCount := len(m.Switches)
|
||||
if err := rec.Reconcile(ctx, intent); err != nil {
|
||||
t.Fatalf("Reconcile #2: %v", err)
|
||||
}
|
||||
if len(m.Switches) != switchCount {
|
||||
t.Errorf("second reconcile created duplicate switches: %d → %d", switchCount, len(m.Switches))
|
||||
}
|
||||
}
|
||||
|
||||
func TestReconcile_PortMembershipDriftCorrected(t *testing.T) {
|
||||
rec, m := newTestReconciler(t)
|
||||
ctx := context.Background()
|
||||
|
||||
Reference in New Issue
Block a user