fix(api): don't burn wake cooldown when refused at capacity

A wake refused by the §9.1 running-server cap returns 503, but the
per-server cooldown was recorded before the cap check ran. A player
held because the cluster was momentarily full would then also have to
wait out the wake cooldown once a slot freed, even though their refused
wake never actually flipped desiredState.

Split cooldownLimiter.allow into allowed (peek, no record) and record
(commit). Both wake paths now consult allowed for the 429, then call
record only after SetDesiredState succeeds — so neither a 503
at_capacity nor a SetDesiredState error consumes the cooldown. The
split is safe against the running cap, which counts CRD truth via
ListServers and is independent of the limiter.
This commit is contained in:
flyemoji committed 2026-06-30 20:03:55 +09:00
1 parent c14ed170cc
commit 2a4a81b9b2
4 files changed
+69 -7

No files matched your search

+20 -5
View File
@@ -377,21 +377,36 @@ type cooldownLimiter struct {
window time.Duration window time.Duration
} }
// allow reports whether name may wake now, recording the attempt when allowed. // allowed reports whether name may wake now WITHOUT recording the attempt. A
func (c *cooldownLimiter) allow(name string, window time.Duration) bool { // non-positive window disables the throttle. Splitting the check (allowed) from
// the commit (record) lets the wake path consult the cooldown for its 429 before
// a downstream gate — the §9.1 running-cap 503 — decides whether the wake will
// actually happen, so a wake refused at capacity never burns the per-server
// cooldown.
func (c *cooldownLimiter) allowed(name string, window time.Duration) bool {
if window <= 0 { if window <= 0 {
return true return true
} }
c.mu.Lock() c.mu.Lock()
defer c.mu.Unlock() defer c.mu.Unlock()
t := c.now() if last, ok := c.last[name]; ok && c.now().Sub(last) < window {
if last, ok := c.last[name]; ok && t.Sub(last) < window {
return false return false
} }
c.last[name] = t
return true return true
} }
// record starts name's cooldown at the current time. The wake path calls it only
// after the wake actually flips desiredState, so neither a 503 at_capacity nor a
// SetDesiredState error consumes the cooldown. allowed→record is deliberately not
// atomic: like the running-cap above, the cooldown is a soft throttle (a burst of
// truly concurrent wakes may each pass allowed before any records), which is
// harmless because SetDesiredState is idempotent.
func (c *cooldownLimiter) record(name string) {
c.mu.Lock()
defer c.mu.Unlock()
c.last[name] = c.now()
}
// ---- running-server cap ---- // ---- running-server cap ----
// withinRunningCap reports whether waking info's server is allowed under the // withinRunningCap reports whether waking info's server is allowed under the
+38
View File
@@ -977,6 +977,44 @@ func TestWakeRunningCap(t *testing.T) {
}) })
} }
// TestWakeCooldownNotBurnedAtCapacity is the §9.1 regression guard for the
// cooldown/cap ordering: a wake the running-cap refuses with 503 must NOT start
// the per-server cooldown. Otherwise a player held because the cluster was
// momentarily full would, once a slot frees, still be made to wait out a 30s
// cooldown their refused wake never earned. With the clock frozen, a 503 followed
// by the same server waking the instant capacity frees must return 202, not 429.
func TestWakeCooldownNotBurnedAtCapacity(t *testing.T) {
cl := newFakeCluster()
target := &ServerInfo{Name: "survival", AutostartPolicy: "public",
DesiredState: string(v1alpha1.DesiredStopped)}
cl.byName["survival"] = target
cl.list = []ServerInfo{*target, {Name: "other", DesiredState: string(v1alpha1.DesiredRunning)}}
api := newTestAPI(newFakeRepo(), cl)
api.WakeCooldown = time.Minute
api.MaxRunningServers = 1
api.External = staticExternal{p: &Principal{UserID: "u", Role: "user"}}
h := api.ExternalHandler()
// The cluster is full (1 running == cap): the wake is refused with 503 and must
// leave the cooldown unstarted.
if w := do(h, "POST", "/api/v1/servers/survival/wake", "", nil); w.Code != http.StatusServiceUnavailable {
t.Fatalf("at-capacity wake code = %d, want 503", w.Code)
}
if _, set := cl.desired["survival"]; set {
t.Fatal("desiredState must not change when refused at capacity")
}
// A slot frees (the other server is gone). The same server, same frozen clock,
// must now wake — a 429 here would prove the 503 had burned the cooldown.
cl.list = []ServerInfo{*target}
if w := do(h, "POST", "/api/v1/servers/survival/wake", "", nil); w.Code != http.StatusAccepted {
t.Fatalf("post-capacity wake code = %d, want 202 (the 503 must not burn the cooldown)", w.Code)
}
if cl.desired["survival"] != v1alpha1.DesiredRunning {
t.Fatalf("desired = %q, want Running", cl.desired["survival"])
}
}
// ---- Zero-Trust admin boundary (§14) ---- // ---- Zero-Trust admin boundary (§14) ----
func TestAdminBoundary(t *testing.T) { func TestAdminBoundary(t *testing.T) {
+5 -1
View File
@@ -148,7 +148,7 @@ func (a *API) handleInternalWake(w http.ResponseWriter, r *http.Request) {
writeError(w, r, err) writeError(w, r, err)
return return
} }
if !a.limiter().allow(name, a.WakeCooldown) { if !a.limiter().allowed(name, a.WakeCooldown) {
writeError(w, r, newError(http.StatusTooManyRequests, "cooldown", "wake is cooling down, retry shortly")) writeError(w, r, newError(http.StatusTooManyRequests, "cooldown", "wake is cooling down, retry shortly"))
return return
} }
@@ -170,6 +170,10 @@ func (a *API) handleInternalWake(w http.ResponseWriter, r *http.Request) {
writeError(w, r, err) writeError(w, r, err)
return return
} }
// Consume the shared per-server cooldown only after the wake flips, so a join
// the cap held with 503 (or a SetDesiredState error) leaves the cooldown
// untouched and the next join attempt is not also throttled.
a.limiter().record(name)
_ = a.Repo.Audit(r.Context(), AuditEntry{ _ = a.Repo.Audit(r.Context(), AuditEntry{
Actor: "velocity", Source: "internal", Action: "wake", ServerName: name, Actor: "velocity", Source: "internal", Action: "wake", ServerName: name,
RequestID: requestIDFromContext(r.Context()), RequestID: requestIDFromContext(r.Context()),
+6 -1
View File
@@ -39,7 +39,7 @@ func (a *API) handleWake(w http.ResponseWriter, r *http.Request) {
writeError(w, r, err) writeError(w, r, err)
return return
} }
if !a.limiter().allow(name, a.WakeCooldown) { if !a.limiter().allowed(name, a.WakeCooldown) {
writeError(w, r, newError(http.StatusTooManyRequests, "cooldown", "wake is cooling down, retry shortly")) writeError(w, r, newError(http.StatusTooManyRequests, "cooldown", "wake is cooling down, retry shortly"))
return return
} }
@@ -60,6 +60,11 @@ func (a *API) handleWake(w http.ResponseWriter, r *http.Request) {
writeError(w, r, err) writeError(w, r, err)
return return
} }
// The wake actually flipped, so consume the per-server cooldown only now: a 503
// at_capacity or the SetDesiredState failure above must not burn it (a player
// held at capacity should retry the instant a slot frees, not wait out a
// cooldown their refused wake never earned).
a.limiter().record(name)
a.audit(r, p.Email, "wake", name) a.audit(r, p.Email, "wake", name)
writeJSON(w, http.StatusAccepted, map[string]any{"name": name, "desiredState": "Running"}) writeJSON(w, http.StatusAccepted, map[string]any{"name": name, "desiredState": "Running"})
} }