Files
go-admin/cmd/api/shutdown_test.go
T
zhangwenjian f3b67e9abc fix🐛: keep the rate limiter away from the health probes
The limiter is installed on the engine and the probes are routes like any
other, so above the threshold they are answered with 429 too. Point a liveness
probe at one and the failure mode writes itself: traffic crosses the threshold,
the probe collects three 429s, the kubelet restarts the container, the capacity
that was already short gets shorter, and the instances that are left are pushed
further past the threshold. The limiter working exactly as designed is what
kills the pod.

It is the argument common/health already makes about restarting a process whose
database is unreachable, applied to load: turning one outage into a crash loop
is not an improvement on the outage.

Nothing points a liveness probe at these routes yet. The manifest that will is
two commits away, and this has to land first, because that manifest without
this change would be actively harmful.

The exemption wraps the middleware rather than teaching the limiter about these
paths. common/ may not import app/ - the contract check enforces it - so the
limiter cannot name routes that are registered over there. Wrapping it in the
command package, which imports both, is what keeps the boundary.

Naming those routes needs them exported, so the group prefix and the two paths
become constants and the router function becomes RegisterMonitorRouter. That
also gives a test something real to mount: a probe asserted against a
re-implementation of itself is a test of the copy.

The check that the middleware never runs is separate from the check that the
answer is not 429, because a probe can produce a 429 on its own. What has to be
true is that the request never reached the limiter.
2026-09-06 21:49:16 +08:00

103 lines
3.4 KiB
Go

package api
import (
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/gin-gonic/gin"
otherrouter "go-admin/app/other/router"
ext "go-admin/config"
)
// The seconds in the configuration and the durations the sequence waits on are
// two spellings of one budget.
func TestBudgetFromSeconds(t *testing.T) {
got := budgetFrom(ext.ShutdownBudget{Server: 5, Cleanup: 3})
want := budget{
server: 5 * time.Second,
cleanup: 3 * time.Second,
}
if got != want {
t.Errorf("budgetFrom = %+v, want %+v", got, want)
}
}
// The package variables and config.Default*Seconds have to say the same thing.
// They are the same default written twice - once as durations for the shutdown
// and once as seconds for the fallback - and a deployment that configures
// nothing is entitled to one answer, not two.
func TestDefaultBudgetIsTheConfiguredFallback(t *testing.T) {
unconfigured, err := ext.Shutdown{}.Budget()
if err != nil {
t.Fatalf("the empty section did not resolve: %v", err)
}
if got, want := defaultBudget(), budgetFrom(unconfigured); got != want {
t.Errorf("defaultBudget = %+v, want the unconfigured budget %+v", got, want)
}
}
// The rate limiter must not answer for the probes.
//
// It is installed on the engine, so without this the probes are limited like
// any other route and answer 429 above the threshold. A liveness probe that
// collects 429s fails its threshold and the container is restarted - taking
// capacity out of a deployment that is already short of it and pushing the
// rest closer to the threshold. The limiter working exactly as designed is
// what would cause it.
//
// The stand-in rejects everything rather than being a real limiter: what is
// under test is which requests reach it, and a real one would need the traffic
// to cross a threshold before it said anything.
func TestTheProbesSkipTheRateLimiter(t *testing.T) {
gin.SetMode(gin.TestMode)
var reached []string
r := gin.New()
r.Use(exemptProbes(func(c *gin.Context) {
reached = append(reached, c.FullPath())
c.AbortWithStatus(http.StatusTooManyRequests)
}))
v1 := r.Group(otherrouter.APIPrefix)
otherrouter.RegisterMonitorRouter(v1)
v1.GET("/business", func(c *gin.Context) { c.Status(http.StatusOK) })
for _, tc := range []struct {
path string
limited bool
}{
{otherrouter.APIPrefix + otherrouter.HealthPath, false},
{otherrouter.APIPrefix + otherrouter.ReadyPath, false},
// Not a probe, and deliberately not exempt: the exemption is for the
// two routes an orchestrator acts on, not for everything under
// /api/v1 that happens to be unauthenticated.
{otherrouter.APIPrefix + "/metrics", true},
{otherrouter.APIPrefix + "/business", true},
} {
t.Run(tc.path, func(t *testing.T) {
w := httptest.NewRecorder()
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, tc.path, nil))
if tc.limited {
if w.Code != http.StatusTooManyRequests {
t.Errorf("answered %d, want the middleware's 429 - it was skipped for a route that is not a probe", w.Code)
}
return
}
if w.Code == http.StatusTooManyRequests {
t.Errorf("answered 429; a probe that can be rate-limited gets the container restarted under load")
}
})
}
// Said separately, because a probe could also answer 429 by itself: what
// has to be true is that the middleware never saw the request.
for _, p := range reached {
if probePaths[p] {
t.Errorf("the middleware ran for %s", p)
}
}
}