mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-19 19:01:42 +00:00
fix(vpcd): add flows-ready barrier after IGW attach (mulga-siv-105)
handleIGWAttach wrote the external switch + gateway port + LRP + default route to OVN NB and returned immediately, before ovn-northd had compiled NB -> SB and before ovn-controller on the gateway chassis had installed the resulting flows. A subsequent RunInstances -> vpc.add-nat could complete while the datapath was still dark; the launched VM was then unreachable on its public IP for whatever flow-install latency the chassis happened to incur. Customer-visible as 'I just created a VPC, launched a VM, and can't SSH in' for ~minutes. Reproduced deterministically in CI run 26037892507 Phase8b: a VM in a fresh VPC was unreachable at 192.168.0.202:22 for 3+ minutes, while a VM at the same public IP in the warm default VPC (Phase8d) was reachable in ~40 seconds. Block at the end of handleIGWAttach on `ovn-nbctl --wait=hv sync`, which bumps NB_Global.nb_cfg and waits for every connected chassis to acknowledge the new sequence number — i.e. flows installed. Bounded by --timeout=30; overruns log a WARN and continue (a 30s OVN stall is already a deeper problem than failing VPC create would surface). waitForFlowsHV is a package var so tests can stub it; in environments without ovn-nbctl on PATH the shell-out fails fast and the WARN path returns nil, so existing IGW tests stay green without a live OVN. Refs: mulga-siv-105
This commit is contained in:
@@ -8,6 +8,7 @@ import (
|
||||
"net"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/services/vpcd/dhcp"
|
||||
"github.com/mulgadc/spinifex/spinifex/services/vpcd/nbdb"
|
||||
@@ -16,6 +17,39 @@ import (
|
||||
"github.com/nats-io/nats.go"
|
||||
)
|
||||
|
||||
// waitForFlowsHV shells out to `ovn-nbctl --wait=hv sync`, which bumps
|
||||
// NB_Global.nb_cfg and blocks until every connected chassis has acknowledged
|
||||
// the new sequence number — i.e. ovn-northd has compiled NB -> SB and
|
||||
// ovn-controller has installed the resulting flows. Used after IGW attach
|
||||
// so newly-launched VMs aren't unreachable while their gateway chassis is
|
||||
// still catching up (mulga-siv-105).
|
||||
//
|
||||
// Bounded by ovn-nbctl --timeout=30 (seconds). On overrun we log a WARN
|
||||
// and return nil — the caller continues. In practice flows converge within
|
||||
// seconds; a 30s overrun means something OVN-side is wedged enough that
|
||||
// failing VPC create wouldn't improve things.
|
||||
//
|
||||
// Declared as a var so tests can stub it.
|
||||
var waitForFlowsHV = func() error {
|
||||
start := time.Now()
|
||||
cmd := sudoCommand("ovn-nbctl",
|
||||
"--no-leader-only",
|
||||
"--timeout=30",
|
||||
"--wait=hv",
|
||||
"sync",
|
||||
)
|
||||
if out, err := cmd.CombinedOutput(); err != nil {
|
||||
slog.Warn("vpcd: OVN flows-ready barrier overran; continuing without confirmation",
|
||||
"elapsed", time.Since(start),
|
||||
"err", err,
|
||||
"output", strings.TrimSpace(string(out)),
|
||||
)
|
||||
return nil
|
||||
}
|
||||
slog.Debug("vpcd: OVN flows-ready barrier complete", "elapsed", time.Since(start))
|
||||
return nil
|
||||
}
|
||||
|
||||
// NATS topics for VPC lifecycle events published by the daemon.
|
||||
const (
|
||||
TopicVPCCreate = "vpc.create"
|
||||
@@ -1174,6 +1208,14 @@ func (h *TopologyHandler) handleIGWAttach(msg *nats.Msg) {
|
||||
"wan_gateway", wanGateway,
|
||||
"chassis_count", len(h.chassisNames),
|
||||
)
|
||||
|
||||
// Block until ovn-northd compiles + the gateway chassis has installed
|
||||
// flows for this VPC. Without this, a subsequent RunInstances ->
|
||||
// vpc.add-nat can complete while the datapath is still dark, and the
|
||||
// new VM is unreachable on its public IP for the duration of the
|
||||
// flow-install latency (typically seconds; observed up to minutes when
|
||||
// the chassis was loaded during CI). mulga-siv-105.
|
||||
_ = waitForFlowsHV()
|
||||
respond(msg, nil)
|
||||
}
|
||||
|
||||
|
||||
@@ -877,6 +877,49 @@ func TestTopologyHandler_IGWAttach(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestTopologyHandler_IGWAttach_InvokesFlowBarrier asserts handleIGWAttach
|
||||
// calls waitForFlowsHV after the OVN writes complete (mulga-siv-105). The
|
||||
// barrier itself shells out to ovn-nbctl --wait=hv sync; stub it here so
|
||||
// the test doesn't depend on a live OVN.
|
||||
func TestTopologyHandler_IGWAttach_InvokesFlowBarrier(t *testing.T) {
|
||||
orig := waitForFlowsHV
|
||||
var called int
|
||||
waitForFlowsHV = func() error { called++; return nil }
|
||||
defer func() { waitForFlowsHV = orig }()
|
||||
|
||||
_, nc := startTestNATS(t)
|
||||
mock := NewMockOVNClient()
|
||||
_ = mock.Connect(context.Background())
|
||||
ctx := context.Background()
|
||||
|
||||
topo := NewTopologyHandler(mock)
|
||||
subs, err := topo.Subscribe(nc)
|
||||
require.NoError(t, err)
|
||||
defer func() {
|
||||
for _, s := range subs {
|
||||
_ = s.Unsubscribe()
|
||||
}
|
||||
}()
|
||||
|
||||
require.NoError(t, mock.CreateLogicalRouter(ctx, &nbdb.LogicalRouter{
|
||||
Name: "vpc-vpc-barrier1",
|
||||
ExternalIDs: map[string]string{
|
||||
"spinifex:vpc_id": "vpc-barrier1",
|
||||
"spinifex:cidr": "10.0.0.0/16",
|
||||
},
|
||||
}))
|
||||
|
||||
evt := types.IGWEvent{InternetGatewayId: "igw-barrier1", VpcId: "vpc-barrier1"}
|
||||
data, _ := json.Marshal(evt)
|
||||
resp, err := nc.Request(TopicIGWAttach, data, 5_000_000_000)
|
||||
require.NoError(t, err)
|
||||
assertSuccess(t, resp, "attach IGW")
|
||||
|
||||
if called != 1 {
|
||||
t.Errorf("expected waitForFlowsHV to run exactly once after IGW attach, got %d", called)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTopologyHandler_IGWAttach_WithExternalPool(t *testing.T) {
|
||||
_, nc := startTestNATS(t)
|
||||
mock := NewMockOVNClient()
|
||||
|
||||
Reference in New Issue
Block a user