From 5c002abca21f5f4d53d74ef2afd512e1bfb71fa7 Mon Sep 17 00:00:00 2001
From: joshsiv-mulga <mrk@mulgadc.com>
Date: Sun, 24 May 2026 20:58:54 +1000
Subject: [PATCH] fix(reconcile): call AttachIGW when IGW topology missing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 spinifex/network/reconcile/apply.go      | 31 +++++--------
 spinifex/network/reconcile/apply_test.go | 56 ++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/spinifex/network/reconcile/apply.go b/spinifex/network/reconcile/apply.go
index 138c72e50..c7e20c623 100644
--- a/spinifex/network/reconcile/apply.go
+++ b/spinifex/network/reconcile/apply.go
@@ -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)
 	}
diff --git a/spinifex/network/reconcile/apply_test.go b/spinifex/network/reconcile/apply_test.go
index 29565e7b8..69bbf5d9a 100644
--- a/spinifex/network/reconcile/apply_test.go
+++ b/spinifex/network/reconcile/apply_test.go
@@ -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()