mirror of
https://github.com/go-admin-team/go-admin.git
synced 2026-09-22 02:27:57 +00:00
/ready has failed from the moment shutdown begins since the readiness probe was added, and the order it does that in is right: reversed, the state would be reported after the connections were already cut. But order alone does not produce a window. Nothing waited between the flip and Shutdown, so the two were microseconds apart, and a poller on a multi-second interval never saw the 503 - it saw a refused connection, which is the thing the probe was supposed to avoid. Polling a container through a SIGTERM on the demo host recorded exactly that: 200, then connection refused, and no 503 in between. extend.shutdown.drain is that wait. The process keeps serving normally for it - answering requests, not refusing them, because refusing them would move the outage earlier rather than avoid it - and only then closes the listener. It is zero by default, so nothing changes for a deployment that does not ask for it. That is not timidity: the budgets are spent one after another, and a non-zero default would push every existing shutdown closer to the orchestrator's grace period, where being cut off part-way through the cleanup callbacks is worse than never draining at all. Keep-alive is switched off with the flip. The server keeps connections alive until Shutdown sets shuttingDown() itself, so without this the pooled connections a balancer holds would sit untouched for the whole window and be cut at the end of it anyway - the cost of the window without its benefit. This is the switch Shutdown flips, moved earlier by the window's length. The signal disposition is restored after the window rather than on the first signal. Before there was a window, the interval where a second signal killed the process outright was only reachable while a cleanup callback hung; putting a multi-second wait inside it would have made every ordinary shutdown interruptible for the length of the drain. A second signal during the window is taken by the channel and ends the window early instead - somebody sending another kill wants this over with sooner - and the escape hatch comes back the moment the window does. What the window is worth depends on who removes this instance. A balancer that polls /ready acts on the 503 and needs the window to cover its check interval times its failure threshold; a Kubernetes Service withdraws the endpoint when the Pod is deleted, concurrently with SIGTERM and regardless of what the probe returns, and there the window covers the delay in that removal reaching every node. The three comments that used to say a balancer "has a chance to" take the instance out said it without either qualification, which is how a claim comes to be repeated after a live test has refuted it. The subprocess test polls the real probes on a connection it opens after the signal - a reused one can be served after the listener is closed, which would let this pass against a shutdown that had already broken it - and asserts on the draining answer in the body, not on the status code. With no database the status is 503 from start-up, so a status-code assertion would hold even with BeginDraining deleted. Two window lengths, because one proves only that something takes that long.
69 lines
2.2 KiB
Go
69 lines
2.2 KiB
Go
package config
|
|
|
|
import (
|
|
"testing"
|
|
|
|
coreconfig "github.com/go-admin-team/go-admin-core/v2/config"
|
|
"github.com/go-admin-team/go-admin-core/v2/config/source/file"
|
|
)
|
|
|
|
// shippedSettings is the shape the loader fills in, cut down to the part under
|
|
// test. The reader is JSON-based, so the keys are matched against field names
|
|
// case-insensitively - which is exactly the matching that silently drops a
|
|
// section the struct has no field for.
|
|
type shippedSettings struct {
|
|
Settings struct {
|
|
Extend Extend
|
|
}
|
|
}
|
|
|
|
func (*shippedSettings) OnChange() {}
|
|
|
|
// The shutdown section has to arrive where it is read from, and with the
|
|
// values the documentation claims.
|
|
//
|
|
// This is the failure this batch exists to remove, one level up: the loader
|
|
// discards keys no field matches, without an error and without a log line, so
|
|
// a section put in the wrong place is written, accepted, and never applied.
|
|
// Nothing but loading the shipped file through the real loader can tell the
|
|
// two apart - the struct compiles either way.
|
|
//
|
|
// The values are asserted as well as the arrival. A settings.yml that shipped
|
|
// a different default from config.Default*Seconds would give two answers to
|
|
// "what does an unconfigured deployment spend", and the file is the one people
|
|
// read.
|
|
func TestTheShippedSettingsReachTheShutdownStruct(t *testing.T) {
|
|
for _, name := range []string{"settings.yml", "settings.full.yml"} {
|
|
t.Run(name, func(t *testing.T) {
|
|
var loaded shippedSettings
|
|
c, err := coreconfig.NewConfig(
|
|
coreconfig.WithSource(file.NewSource(file.WithPath(name))),
|
|
coreconfig.WithEntity(&loaded),
|
|
)
|
|
if err != nil {
|
|
t.Fatalf("load %s: %v", name, err)
|
|
}
|
|
t.Cleanup(func() { _ = c.Close() })
|
|
|
|
s := loaded.Settings.Extend.Shutdown
|
|
if s.Drain == nil || s.Server == nil || s.Cleanup == nil {
|
|
t.Fatalf("%s left extend.shutdown unfilled (%+v); the section is written but nothing reads it",
|
|
name, s)
|
|
}
|
|
|
|
want := ShutdownBudget{
|
|
Drain: DefaultDrainSeconds,
|
|
Server: DefaultServerSeconds,
|
|
Cleanup: DefaultCleanupSeconds,
|
|
}
|
|
got, err := s.Budget()
|
|
if err != nil {
|
|
t.Fatalf("%s does not resolve: %v", name, err)
|
|
}
|
|
if got != want {
|
|
t.Errorf("%s ships %+v, want the documented defaults %+v", name, got, want)
|
|
}
|
|
})
|
|
}
|
|
}
|