diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 00ac042..7aa9664 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -222,6 +222,25 @@ func (o *Orchestrator) SelfCheckResult(ctx context.Context, validatorID string, if err != nil { return err } + // A late report for an address this validator no longer owns (lease + // already reclaimed, address cancelled or deleted) must not touch + // the floating IP: it may be attached for another validator now. + if item.State != db.IPAwaitingSelfCheck || item.OwnerValidatorID == nil || *item.OwnerValidatorID != validatorID { + o.Log.Warn("ignoring failed self-check for an address the validator does not hold", + "validator", validatorID, "ip_id", ipID, "state", item.State) + return nil + } + // Detach the floating IP before the address goes back to the queue. + // requeueOrFail frees the validator in the database but knows nothing + // about the cloud: a floating IP left on the validator's port makes + // every later association on that port fail with 409 ("fixed IP + // already has a floating IP"). Best-effort, like the other release + // paths — the database state must be freed even if Neutron hiccups. + if item.FIPID != "" { + if err := o.OS.DisassociateFloatingIP(ctx, item.FIPID); err != nil { + o.Log.Error("disassociate fip after failed self-check", "ip_id", ipID, "fip_id", item.FIPID, "err", err) + } + } if item.RetryCount+1 > o.Cfg.MaxSelfCheckRetries { o.requeueOrFail(ctx, ipID, validatorID, "self-check failed: "+detail) return nil diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index e00ecd5..d230c17 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -1086,3 +1086,79 @@ func TestClearQueueFreesAllValidatorPorts(t *testing.T) { t.Fatalf("port-2: expected only the foreign 9.9.9.9 to remain, got %+v", got) } } + +// A failed self-check must detach the floating IP in the cloud before the +// address returns to the queue; otherwise the validator's port keeps it and +// every later association on that port fails with 409. +func TestFailedSelfCheckDetachesFloatingIP(t *testing.T) { + ctx := context.Background() + o, d, mock := newTestOrchestrator(t, 180) + + mock.Seed("fip-1", "1.2.3.4", "svc-project") + if err := d.RegisterValidator(ctx, "validator-1", "host-1", "port-1", "v0.1"); err != nil { + t.Fatal(err) + } + if err := d.SeedQueue(ctx, []string{"1.2.3.4"}); err != nil { + t.Fatal(err) + } + o.Tick(ctx) + if got := portFIPs(t, mock, "port-1"); len(got) != 1 { + t.Fatalf("setup: expected the floating ip on port-1, got %d", len(got)) + } + ip, err := d.GetIPByAddress(ctx, "1.2.3.4") + if err != nil { + t.Fatal(err) + } + + if err := o.SelfCheckResult(ctx, "validator-1", ip.ID, false, "ip echo timeout"); err != nil { + t.Fatalf("self-check result: %v", err) + } + if got := portFIPs(t, mock, "port-1"); len(got) != 0 { + t.Fatalf("port-1 still holds the floating ip after a failed self-check: %+v", got) + } + after, err := d.GetIPByAddress(ctx, "1.2.3.4") + if err != nil { + t.Fatal(err) + } + if after.State != db.IPQueued { + t.Fatalf("expected the address back in queued, got %s", after.State) + } + + // The validator must be able to take the next claim: a second Tick + // associates the same address again without a conflict. + o.Tick(ctx) + if got := portFIPs(t, mock, "port-1"); len(got) != 1 { + t.Fatalf("expected the retry to attach the floating ip again, got %d", len(got)) + } +} + +// A failed self-check for an address the validator no longer holds is a late +// report: it must not detach a floating IP that now belongs to someone else. +func TestLateFailedSelfCheckIsIgnored(t *testing.T) { + ctx := context.Background() + o, d, mock := newTestOrchestrator(t, 180) + + mock.Seed("fip-1", "1.2.3.4", "svc-project") + for i, id := range []string{"validator-1", "validator-2"} { + if err := d.RegisterValidator(ctx, id, "h", fmt.Sprintf("port-%d", i+1), "v"); err != nil { + t.Fatal(err) + } + } + if err := d.SeedQueue(ctx, []string{"1.2.3.4"}); err != nil { + t.Fatal(err) + } + o.Tick(ctx) // validator-1 (first by id) holds 1.2.3.4 + ip, _ := d.GetIPByAddress(ctx, "1.2.3.4") + + // validator-2 reports a failure for an address it does not own. + if err := o.SelfCheckResult(ctx, "validator-2", ip.ID, false, "late"); err != nil { + t.Fatalf("self-check result: %v", err) + } + if got := portFIPs(t, mock, "port-1"); len(got) != 1 { + t.Fatalf("a late report from another validator detached the floating ip: %+v", got) + } + cur, _ := d.GetIPByAddress(ctx, "1.2.3.4") + if cur.State != db.IPAwaitingSelfCheck { + t.Fatalf("a late report changed the address state to %s", cur.State) + } +}