Skip the check cycle for a Floating IP already occupied by another port
The cloud is live: the address list submitted as "free" (bootstrap config
or POST /api/v1/admin/ips) can drift by the time the orchestrator claims
it, or an operator can queue an already-occupied address by mistake.
Neutron's floating-IP association is a blind "last write wins" PUT with
no conflict error to catch, so associateFIP now checks the FIP's PortID
(already fetched via GetFloatingIPByAddress) before associating, guarded
against the false-positive of the FIP already belonging to this same
validator's own port.
A match routes the address straight to a new terminal ip_queue.state
("occupied", distinct from failed/fail) via db.MarkFIPOccupied — no
retries, since Neutron won't free it on its own and requeuing would let
it be reclaimed again next tick, starving the rest of the queue — plus a
dedicated fip_occupied audit event. Resubmitting the address later (once
the conflict is resolved) resets it to queued via the existing
POST /api/v1/admin/ips resubmit path (CancelIP/ListExpiredLeases updated
to treat occupied as terminal too). admin-dashboard gets its own "занят"
badge, distinct from fail/partial/cancelled.
Rebuilt bin/{control-api,admin-dashboard,prober,validator-agent} and
bin/SHA256SUMS per docs/SETUP.md's documented build recipe, since
control-api and admin-dashboard source changed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeVbMVEiE7XQAkBd7HQgj6
This commit is contained in:
1 parent
2246369b64
commit
93c79b63ea
17 files changed
+326
-17
No files matched your search
@@ -106,6 +106,27 @@ func (o *Orchestrator) associateFIP(ctx context.Context, validatorID, osPortID s
|
||||
o.requeueOrFail(ctx, item.ID, validatorID, fmt.Sprintf("lookup floating ip: %v", err))
|
||||
return err
|
||||
}
|
||||
// The cloud is live: an address queued as "free" (from bootstrap config
|
||||
// or an admin POST) may have drifted onto another port by the time we
|
||||
// actually get here, or an operator may have queued an already-occupied
|
||||
// address by mistake. This is the one authoritative moment to catch it —
|
||||
// checked here rather than at enqueue time because enqueue-time state
|
||||
// could itself be stale by the time the claim happens. fip.PortID !=
|
||||
// osPortID guards against a false positive when the FIP is already
|
||||
// associated to this same validator's own port (e.g. control-api
|
||||
// restarted between associating and recording it) — that's a resume, not
|
||||
// a conflict.
|
||||
if fip.PortID != "" && fip.PortID != osPortID {
|
||||
if err := o.DB.MarkFIPOccupied(ctx, item.ID, validatorID); err != nil {
|
||||
o.Log.Error("mark fip occupied", "ip_id", item.ID, "err", err)
|
||||
return err
|
||||
}
|
||||
o.event(ctx, "control-api", "", &item.ID, "fip_occupied",
|
||||
fmt.Sprintf(`{"fip_id":%q,"port_id":%q}`, fip.ID, fip.PortID))
|
||||
o.Log.Info("fip already occupied by another port, skipping check cycle",
|
||||
"ip", item.IPAddress, "fip_port_id", fip.PortID)
|
||||
return nil
|
||||
}
|
||||
if err := o.OS.AssociateFloatingIP(ctx, fip.ID, osPortID); err != nil {
|
||||
o.requeueOrFail(ctx, item.ID, validatorID, fmt.Sprintf("associate floating ip: %v", err))
|
||||
return err
|
||||
@@ -608,7 +629,8 @@ func (o *Orchestrator) SweepStaleSiteHeartbeats(ctx context.Context) error {
|
||||
|
||||
// RecordEvent is the exported entry point httpapi uses to log
|
||||
// agent/prober-reported audit events (config_received, fip_changed,
|
||||
// error, etc.) through the same path as internally generated events.
|
||||
// error, etc.) through the same path as internally generated events
|
||||
// (fip_associated, fip_occupied, retry_or_fail, etc.).
|
||||
func (o *Orchestrator) RecordEvent(ctx context.Context, sourceType, sourceID string, ipID *int64, eventType, payload string) {
|
||||
o.event(ctx, sourceType, sourceID, ipID, eventType, payload)
|
||||
}
|
||||
|
||||
@@ -165,6 +165,100 @@ func TestHappyPath(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestFIPAlreadyOccupiedByAnotherPortSkipsCheckCycle covers the defense
|
||||
// against a Floating IP that turns out to already be attached to some other
|
||||
// VM's port at claim time — the cloud is live, so the "free" list supplied
|
||||
// at bootstrap/via the admin API can drift, or an operator can mistakenly
|
||||
// queue an already-occupied address. The address must be terminated as
|
||||
// `occupied` immediately, without ever entering the check cycle, without
|
||||
// stealing the port, and with the validator freed back to idle so the rest
|
||||
// of the queue isn't starved behind it.
|
||||
func TestFIPAlreadyOccupiedByAnotherPortSkipsCheckCycle(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
o, d, mock := newTestOrchestrator(t, 180)
|
||||
|
||||
mock.SeedWithPort("fip-1", "1.2.3.4", "svc-project", "someone-elses-port")
|
||||
if err := d.RegisterValidator(ctx, "validator-1", "host-1", "port-1", "v0.1"); err != nil {
|
||||
t.Fatalf("register validator: %v", err)
|
||||
}
|
||||
if err := d.SeedQueue(ctx, []string{"1.2.3.4"}); err != nil {
|
||||
t.Fatalf("seed queue: %v", err)
|
||||
}
|
||||
|
||||
o.Tick(ctx)
|
||||
|
||||
ip, err := d.GetIPByAddress(ctx, "1.2.3.4")
|
||||
if err != nil {
|
||||
t.Fatalf("get ip: %v", err)
|
||||
}
|
||||
if ip.State != db.IPOccupied {
|
||||
t.Fatalf("expected occupied, got %s", ip.State)
|
||||
}
|
||||
if ip.OverallResult != "" {
|
||||
t.Fatalf("expected empty overall_result, got %q", ip.OverallResult)
|
||||
}
|
||||
if ip.OwnerValidatorID != nil {
|
||||
t.Fatalf("expected no owning validator, got %v", *ip.OwnerValidatorID)
|
||||
}
|
||||
if ip.FIPID != "" {
|
||||
t.Fatalf("expected no fip_id recorded, got %q", ip.FIPID)
|
||||
}
|
||||
|
||||
v, err := d.GetValidator(ctx, "validator-1")
|
||||
if err != nil {
|
||||
t.Fatalf("get validator: %v", err)
|
||||
}
|
||||
if v.State != db.ValidatorIdle || v.CurrentIPID != nil {
|
||||
t.Fatalf("expected validator freed back to idle, got state=%s current_ip=%v", v.State, v.CurrentIPID)
|
||||
}
|
||||
|
||||
if fip, _ := mock.GetFloatingIPByAddress(ctx, "1.2.3.4"); fip.PortID != "someone-elses-port" {
|
||||
t.Fatalf("expected fip's port untouched (not stolen), got %q", fip.PortID)
|
||||
}
|
||||
|
||||
events, err := d.ListEventsForIP(ctx, ip.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("list events: %v", err)
|
||||
}
|
||||
found := false
|
||||
for _, e := range events {
|
||||
if e.EventType == "fip_occupied" {
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatalf("expected a fip_occupied event, got %+v", events)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFIPAssociatedToOwnValidatorPortIsNotOccupied guards against a false
|
||||
// positive: a Floating IP already attached to the very validator we're
|
||||
// about to associate it with (e.g. control-api restarted between the
|
||||
// OpenStack call and recording it in the DB) is a resume, not a conflict —
|
||||
// it must proceed through the normal happy path, not be flagged occupied.
|
||||
func TestFIPAssociatedToOwnValidatorPortIsNotOccupied(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
o, d, mock := newTestOrchestrator(t, 180)
|
||||
|
||||
mock.SeedWithPort("fip-1", "1.2.3.4", "svc-project", "port-1")
|
||||
if err := d.RegisterValidator(ctx, "validator-1", "host-1", "port-1", "v0.1"); err != nil {
|
||||
t.Fatalf("register validator: %v", err)
|
||||
}
|
||||
if err := d.SeedQueue(ctx, []string{"1.2.3.4"}); err != nil {
|
||||
t.Fatalf("seed queue: %v", err)
|
||||
}
|
||||
|
||||
o.Tick(ctx)
|
||||
|
||||
ip, err := d.GetIPByAddress(ctx, "1.2.3.4")
|
||||
if err != nil {
|
||||
t.Fatalf("get ip: %v", err)
|
||||
}
|
||||
if ip.State != db.IPAwaitingSelfCheck {
|
||||
t.Fatalf("expected awaiting_self_check (not occupied), got %s", ip.State)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPartialResult(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
o, d, mock := newTestOrchestrator(t, 180)
|
||||
|
||||
Reference in new issue
Block a user