OPTI-3818: stop a transient DigitalOcean 422 from permanently stranding a Reserved IP

[craig-johnston]

Fixes the flipop defect behind OPTI-3818: a transient DigitalOcean 422 permanently stranded the turn pool's Reserved IPs, causing a 2h31m production TURN outage on rts-sgp-1-production-cluster.

Based on base-7db46fe, the exact commit we run in production (digitaloceanapps/flipop:7db46fe, digest sha256:e627e528…), so this diff is the precise production delta. Upstream main (02016d5) does not fix any of this.

Root cause — not "missing retry"

Retrying on 422 would not have prevented the outage on its own, because no retry was ever attempted. The work item is lost from its queue before the retry machinery matters.

1. orderedSet.Front() is a pop (ip_controller.go:847). reconcileAssignment pops the IP (:516) and the node (:527); the deferred requeue (:496-504) replays only the two early-continue guards. The AssignIP-error branch requeues neither, so both are dropped.
2. The assignment is recorded before the API call (:546, :549) and never unwound. EnableNodes then reads that phantom entry (:676), logs enabling node; already assigned to ip — the exact line in our logs at 03:27:15Z — deletes the IP from assignableIPs (:685) and continues (:686), never reaching assignableNodes.Add (:689).
3. reconcileIPStatus partially heals — re-adds the IP, clears nodeProviderID, sets state unassigned (our observed 8×unassigned) — but cannot requeue the node: the assignableNodes.Add at :470 is guarded by providerIDToNodeName[providerID], and providerID is "" by then.
4. EnableNodes can never help again (:665, "Already enabled").
5. assignableNodes is permanently empty, so the loop guard at :505 never passes again.

The 1-minute reconcile loop ran ~150 times and was structurally incapable of recovering. Only a restart rebuilds the queues — which is why rollout restart bound the IPs in 45 seconds.

Changes

floatingip: unwind assignment bookkeeping and requeue on AssignIP failure — the primary fix. Unwinds the two premature writes and requeues both the IP and the node. status.state / assignmentErrors are deliberately left set so the failure stays visible in the CRD status and the ip_assignment_errors metric.

provider/digitalocean: treat a 422 for an unowned IP as fast-retryable — secondary. When the 422 probe shows the IP is owned by nobody, return NewRetryError(err, RetryFast) rather than a bare error that ErrorToRetrySchedule maps to RetrySlow (1m/5m/10m, then 10m forever). The iErr == nil guard prevents misclassifying a failed IPToProviderID lookup. A 422 naming a different owner still yields RetrySlow — that is a real conflict.

floatingip: requeue a node whose IP is removed out of band — a second, independent stranding path, found by running the reproduction against the deployed image on dev. reconcileIPStatus guards the reassignment requeue on providerID (the IP's new owner) but requeues expectedProviderID (the node that lost it). When an IP is taken away entirely, providerID is "", the lookup fails, and the node is never returned to assignableNodes. No AssignIP call fails here, because none is ever made. Also stops writing providerIDToIP[""] = ip, which is read back as expectedIP on the next pass and fabricates a "node assignment mismatch" against an unrelated pool IP.

floatingip: defer assignment only on a retry deadline that has not elapsed — see below. Required by the first commit; without it the fix trades a stranded IP for a pegged CPU.

The retry gates: an earlier revision of this PR called these correct. That was wrong.

Both gates in reconcileAssignment deferred when the retry deadline had elapsed, then handed that already-past timestamp to i.retry():

if !status.nextRetry.IsZero() && !status.nextRetry.After(now) {
    retryIPs = append(retryIPs, ip)
    i.retry(status.nextRetry)   // already in the past
    continue
}

run() sleeps until i.nextRetry, so it re-enters reconcile() at or just after the deadline it was given — an elapsed deadline is the normal state on wake-up. retryTimerDuration() maps any past timestamp to 0, so the loop re-enters immediately, the gate skips again, and the controller busy-loops without ever retrying the bind.

Upstream never reached this, because the error branch dropped the IP and node from their queues entirely. Requeueing them — the first commit — makes the gates reachable. Deploying that commit alone to our dev cluster produced ~1,280 reconciles/sec, CPU pegged at the 250m limit, 173,963 log lines in 60 seconds, and zero assignment attempts.

Two things made this hard to see, and both are worth flagging to anyone else testing this code:

Naively flipping the IP gate to a plain After(now) is also wrong, and that is what the earlier revision tried. reconcileIPStatus reuses status.nextRetry for the routine healthyRetrySchedule (5m) refresh and sets it on every healthy IP immediately before reconcileAssignment runs. An unqualified check therefore defers every IP and nothing is ever assigned — which is exactly the TestFloatingIPPoolUpdateK8s failure across 6 subtests that the earlier revision saw and misread as "the guard is not a defect". The fix gates on IPStateError as well, which is what the comment always claimed the check meant. The node gate needs no qualifier: providerIDToRetry only holds nodes whose assignment failed, and the entry is deleted on success.

RetrySchedule.Next() builds its deadlines from time.Now(), not from the injectable i.now(). Several existing tests pin i.now to a 2021 constant, so every retry deadline lands years in the future, no retry gate is ever reached, and the tests pass without exercising the code under test. A first attempt at a spin regression test went green against the wedged build for exactly this reason. Those clocks are now seeded from time.Now().

TestIPControllerRunDoesNotSpinOnPersistentFailure drives the real run() loop against the real clock — the only level at which this bug is visible, since the spin is a disagreement between the deadlines reconcile() schedules and the timer run() derives from them. It counts reconcile passes via the clock rather than via the provider, because a wedged controller skips the provider calls on every pass and provider call counts stay flat:

	clock reads in 1.5s
buggy gate	966,295
fixed	a handful

TestIPControllerElapsedNodeRetryDoesNotSpin / TestIPControllerElapsedIPRetryDoesNotSpin cover each gate in isolation; TestIPControllerPendingIPRetryIsDeferred / TestIPControllerHealthyIPIsAssignable pin the converse so the fix cannot regress into "never defer" or "always defer".

Test plan

• Red/green proven both directions with the project's own runner (make test, Go 1.23, dockerised)
• Applied to pristine 7db46fe, the new tests fail with exactly the defect signature: assignableNodes.Len() == 0 ("node should have been requeued"), providerIDToIP retaining the providerID, and RetrySlow{1m,5m,10m} where RetryFast is required
• With the fix, go test ./... is green across all packages; gofmt and go vet clean
• TestFloatingIPPoolUpdateK8s and TestNodeDNSRecordSetController fail under -race at pristine upstream 7db46fe as well — pre-existing, not introduced here
• Deployed to do-dev-sgp-1 and validated against a real unassigned Reserved IP

One pre-existing expectation changed: the "assignment error" case in TestIPControllerReconcileAssignment asserted that providerIDToIP retains the mapping after a failed assignment — it encoded this defect as intended behaviour. It now asserts the mapping is unwound and both sides requeued.

Reproduction on dev, against the deployed image, with no API failure at all

05:45:46  doctl compute reserved-ip-action unassign 144.126.243.20
05:50:38  "ip address is available for assignment"   (both pool IPs)
05:50:38  "node assignment mismatch ... expected_ip=144.126.241.39 provider_id="   <- the junk "" key
05:50–05:56 "no IPs assigned; skipping DNS update"  every minute, indefinitely
          ZERO "assigning IP to node" attempts. No error. No retry.
05:56:36  kubectl -n infra rollout restart deploy/flipop
05:58:02  IP rebound, state active   (~85s)

That is the same end state as the production incident, reached without any API failure at all.

Generated with Devin