diff --git a/.env.template b/.env.template index 1605c1fc..992c9f29 100644 --- a/.env.template +++ b/.env.template @@ -5,6 +5,14 @@ # Header used to read/write request user_path values (default: X-GoModel-User-Path) # USER_PATH_HEADER=X-GoModel-User-Path +# Where the running gateway records its process id so `gomodel --reload` can find it +# (default: data/gomodel.pid next to a ./data directory, otherwise the per-user data +# directory). Set it per instance when several gateways share a host. Leaving this +# empty means "unset" and keeps the default, as everywhere else here; to write no pid +# file at all (which also disables --reload), set `server.pid_file: ""` in config.yaml. +# Changing it takes effect on the next restart, not on `gomodel --reload`. +# PID_FILE=data/gomodel.pid + # Reject unknown keys in config.yaml and in the JSON env vars that declare the same # structures (VIRTUAL_MODELS, SET_RATE_LIMIT_*, SET_BUDGET_*). Default: true, so a # typo or a misindented section fails startup instead of silently dropping providers, diff --git a/CLAUDE.md b/CLAUDE.md index c1031f4a..0901a1cb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -111,6 +111,7 @@ Full reference: `.env.template` and `config/config.yaml` - `GOMODEL_MASTER_KEY` (empty = unsafe mode). Managed API keys (dashboard API Keys page / `POST /admin/auth-keys`) carry a per-key `dashboard_access` flag (default false, changeable via `PUT /admin/auth-keys/{id}/dashboard-access`): only the master key and flagged keys can call the admin REST API endpoints under `/admin/*` (others get 403 `dashboard_access_denied`); the dashboard UI shell and static assets (`/admin/dashboard`, `/admin/static/*`) skip auth entirely — only the admin data they load is gated; model endpoints and `GET /v1/usage` stay open to every key, and the no-master-key lockout-recovery path (auth skipped on `/admin/*`) is unaffected. - `BODY_SIZE_LIMIT` ("10M") - `USER_PATH_HEADER` (`X-GoModel-User-Path`: Header used to read/write request `user_path` values) + - `PID_FILE` / `server.pid_file` (`data/gomodel.pid` next to a `./data` directory, otherwise the OS per-user data dir — same resolution as `SQLITE_PATH`): where the running gateway records its process id. `gomodel --reload` reads it and signals that process (SIGHUP; `kill -HUP` works too) to reload configuration without a restart, like `nginx -s reload`. The reload re-reads `.env` (exported variables still win over the file; variables removed from the file are unset) and the whole config, then rebuilds the application — so every setting reloads, not a curated subset. The replacement is built before the running one is stopped, so a broken config keeps the current one serving; the listening socket is held across generations, so no connection is refused mid-reload. `PORT` and `PID_FILE` changes still need a restart (warned about), and in-memory state — rate limit counters, session affinity pins, live log buffers — resets as it would on restart. `server.pid_file: ""` in `config.yaml` disables the pid file and `--reload` (an empty `PID_FILE` env var reads as unset and keeps the default). Not available on Windows (POSIX signals). - `ENABLE_PASSTHROUGH_ROUTES` (true: Enable provider-native passthrough routes under /p/{provider}/...) - `ALLOW_PASSTHROUGH_V1_ALIAS` (true: Allow /p/{provider}/v1/... aliases while keeping /p/{provider}/... canonical) - `ENABLED_PASSTHROUGH_PROVIDERS` (openai,anthropic,openrouter,zai,vllm: Comma-separated list of enabled passthrough providers) diff --git a/config/config.example.yaml b/config/config.example.yaml index 585168c4..9805425a 100644 --- a/config/config.example.yaml +++ b/config/config.example.yaml @@ -15,6 +15,7 @@ server: user_path_header: "X-GoModel-User-Path" # env: USER_PATH_HEADER; inbound header used for user_path scoping enabled_passthrough_providers: ["openai", "anthropic", "cohere", "openrouter", "kilo", "zai", "vllm", "deepseek", "bailian"] # providers enabled on /p/{provider}/... realtime_enabled: true # env: REALTIME_ENABLED; expose /v1/realtime websocket and /p/{provider}/v1/realtime upgrades (OpenAI only) + pid_file: "data/gomodel.pid" # env: PID_FILE; where the running gateway records its process id so `gomodel --reload` can find it. Set per instance when several gateways share a host; empty writes no pid file and disables --reload; changing it needs a restart, not a reload models: enabled_by_default: true # env: MODELS_ENABLED_BY_DEFAULT; when false, models stay unavailable until an access override allows one or more user paths diff --git a/config/config.go b/config/config.go index 9225c768..865ebc9a 100644 --- a/config/config.go +++ b/config/config.go @@ -61,6 +61,7 @@ func buildDefaultConfig() *Config { UserPathHeader: "X-GoModel-User-Path", SwaggerEnabled: false, PprofEnabled: false, + PIDFile: DefaultPIDFilePath(), EnablePassthroughRoutes: true, AllowPassthroughV1Alias: true, RealtimeEnabled: true, diff --git a/config/server.go b/config/server.go index 39edad4a..cabf5370 100644 --- a/config/server.go +++ b/config/server.go @@ -7,6 +7,8 @@ import ( "regexp" "strconv" "strings" + + "github.com/enterpilot/gomodel/internal/platformdir" ) // Body size limit constants @@ -43,6 +45,26 @@ type ServerConfig struct { // at /v1/realtime and the /p/{provider}/v1/realtime passthrough upgrade. // Default: true. Only providers implementing realtime accept sessions. RealtimeEnabled bool `yaml:"realtime_enabled" env:"REALTIME_ENABLED"` + // PIDFile records the process id of the running gateway so `gomodel --reload` + // can find it. Default: DefaultPIDFilePath(). Set it per instance when + // several gateways share a host, or to "" in config.yaml to write no pid + // file at all, which also disables `--reload` (an empty PID_FILE reads as + // unset, like every other env var here, and keeps the default). Changing it + // needs a restart — it names the process that is already running — so a + // reload only warns about it. + PIDFile string `yaml:"pid_file" env:"PID_FILE"` +} + +// LegacyPIDFilePath is the pid file location used next to a project-local +// ./data directory, matching where the SQLite database lands in the same setup. +const LegacyPIDFilePath = "data/gomodel.pid" + +// DefaultPIDFilePath returns the pid file path used when none is configured: +// LegacyPIDFilePath when a ./data directory already exists (Docker images and +// existing deployments), otherwise the OS-conventional per-user data directory +// — the same resolution the database uses, so both land together. +func DefaultPIDFilePath() string { + return platformdir.DataFile("gomodel.pid") } var headerNameRegex = regexp.MustCompile(`^[!#$%&'*+\-.^_` + "`" + `|~0-9A-Za-z]+$`) diff --git a/config/server_test.go b/config/server_test.go new file mode 100644 index 00000000..262e1add --- /dev/null +++ b/config/server_test.go @@ -0,0 +1,102 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/enterpilot/gomodel/internal/platformdir" +) + +// The pid file follows the database instead of scattering GoModel's state +// across the filesystem: a Docker image with /app/data keeps both +// project-local, and a binary install started from an arbitrary working +// directory keeps both in the per-user data directory. +func TestDefaultPIDFilePath(t *testing.T) { + platformDataDir, err := platformdir.DataDir() + if err != nil { + t.Fatalf("platformdir.DataDir() error: %v", err) + } + + tests := []struct { + name string + setup func(t *testing.T, dir string) + want string + }{ + { + name: "data directory exists keeps the project-local path", + setup: func(t *testing.T, dir string) { + if err := os.Mkdir(filepath.Join(dir, "data"), 0o755); err != nil { + t.Fatal(err) + } + }, + want: LegacyPIDFilePath, + }, + { + name: "no data directory uses the platform path", + setup: func(t *testing.T, dir string) {}, + want: filepath.Join(platformDataDir, "gomodel.pid"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + tt.setup(t, dir) + t.Chdir(dir) + + if got := DefaultPIDFilePath(); got != tt.want { + t.Errorf("DefaultPIDFilePath() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestPIDFilePathResolution(t *testing.T) { + tests := []struct { + name string + env string + configYAML string + want string + }{ + { + name: "env var wins", + env: "/var/run/gomodel/custom.pid", + want: "/var/run/gomodel/custom.pid", + }, + { + // Empty env vars are "unset" everywhere in this config, so PID_FILE= + // keeps the default rather than disabling the pid file. Asserted so + // the documented way to disable it stays the config file. + name: "empty env var keeps the default", + env: "", + want: DefaultPIDFilePath(), + }, + { + name: "empty config value writes no pid file", + configYAML: "server:\n pid_file: \"\"\n", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + t.Setenv("PID_FILE", tt.env) + if tt.configYAML != "" { + if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(tt.configYAML), 0o600); err != nil { + t.Fatal(err) + } + } + + result, err := Load() + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if got := result.Config.Server.PIDFile; got != tt.want { + t.Errorf("Server.PIDFile = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/docs/advanced/cli.mdx b/docs/advanced/cli.mdx index 621d23c8..e3439cd6 100644 --- a/docs/advanced/cli.mdx +++ b/docs/advanced/cli.mdx @@ -1,8 +1,8 @@ --- title: "CLI Operations" -description: "Command-line flags for inspecting the GoModel binary and probing a running gateway's health." +description: "Command-line flags for inspecting the GoModel binary, probing a running gateway's health, and reloading its configuration." icon: "terminal" -keywords: ["CLI", "command-line flags", "health check", "version probe"] +keywords: ["CLI", "command-line flags", "health check", "version probe", "config reload", "SIGHUP"] --- ## Overview @@ -18,6 +18,7 @@ the examples below use the long form. | `--health-timeout` | Maximum time to wait for the `--health` probe | `2s` | | `--ready` | Probe the local `/health/ready` (readiness) endpoint and exit | — | | `--ready-timeout` | Maximum time to wait for the `--ready` probe | `4s` | +| `--reload` | Tell the running gateway to reload its configuration and exit | — | ## Version @@ -90,6 +91,96 @@ gomodel --ready { "status": "ready", "components": { "storage": "ok", "cache": "ok" } } ``` +## Configuration reload + +`--reload` applies configuration changes to a running gateway without +restarting it — the same operation as `nginx -s reload`: + +```bash +gomodel --reload +``` + +It loads the same configuration the gateway does to find the pid file, signals +that process, and exits. Loading it first means a `config.yaml` the binary +cannot parse fails the command — non-zero exit, no signal sent — before the +running gateway is ever asked to look at it. On success the gateway: + +1. Re-reads the `.env` file. New and edited values are applied; values removed + from the file are unset. Variables exported into the process environment + keep winning over the file, exactly as they do at startup — a container's + environment is not overridden by a file inside it. +2. Re-reads `config/config.yaml` (or `config.yaml`) and every environment + variable, then rebuilds itself from the result. Providers, virtual models, + budgets, rate limits, guardrails, MCP servers, caching, logging, admin + settings — all of it reloads, because the reload re-runs the same startup + path rather than a hand-picked subset. + +The replacement is built **before** the running configuration is stopped, so a +configuration that fails to load or initialize changes nothing: the gateway logs +`reload failed; keeping the running configuration` and keeps serving on what +already works. + +The listening socket is held for the lifetime of the process and handed to each +configuration in turn, so requests arriving mid-reload wait to be accepted +rather than being refused. In-flight requests get the same 10-second drain +window as a shutdown, and streamed responses that outlive it are cut, so a +reload during heavy streaming traffic is not free — but no connection is dropped +at the socket. + +That last guarantee depends on the operating system letting the gateway +duplicate a listening socket, which covers every platform that can be sent a +reload signal in the first place. Where duplication is unavailable, the next +configuration rebinds the address instead, and connections are refused for the +length of the swap. + +Sending the signal directly does the same thing, which is what a process manager +or a container without a shell can use: + +```bash +kill -HUP "$(cat data/gomodel.pid)" +``` + + + Reload is a POSIX signal feature and is not available on Windows. + + +### The pid file + +The gateway writes its process id to `PID_FILE` / `server.pid_file` at startup +and removes it on shutdown. The default is `data/gomodel.pid` when a `./data` +directory exists (Docker images and existing deployments) and the +OS-conventional per-user data directory otherwise — the same resolution the +SQLite database uses. + +Give each instance its own path when several gateways share a host, since +`--reload` signals whichever process the file names. To write no pid file at +all, set `server.pid_file: ""` in `config.yaml` — that disables `--reload`, +though `kill -HUP` still works. An empty `PID_FILE` env var reads as unset and +keeps the default, the same as every other setting. If the path is not writable +the gateway logs a warning and serves normally, only without `--reload` support. + +Both the gateway and `gomodel --reload` resolve the path from the same +configuration, so run the command from the same working directory (or with the +same `PID_FILE`) as the gateway: + +```bash +docker exec my-gateway /gomodel --reload +``` + +### What a reload does not change + +- **`PORT`** — the socket stays bound so no connection is refused; a port change + needs a restart. The gateway logs a warning naming both ports. +- **`PID_FILE`** — it names the process that is already running. +- **`GOMODEL_DEMO_MODE`** — the demo warnings are wired up once at startup. +- **In-memory state** — rate limit counters, virtual-model session affinity, and + live log buffers start fresh, exactly as they would after a restart. Budgets + and usage are stored in the database and are unaffected. + +For refreshing provider model catalogs and admin-managed data *without* re-reading +configuration, the dashboard's runtime refresh (`POST /admin/runtime/refresh`) is +the lighter option. + Liveness (`--health`) is the right signal for a Docker `HEALTHCHECK` (restart on crash). Readiness (`/health/ready`) is the right signal for a Kubernetes `readinessProbe` (gate traffic) — point it at the HTTP endpoint directly or run diff --git a/docs/advanced/config-yaml.mdx b/docs/advanced/config-yaml.mdx index 641511f9..9d77740b 100644 --- a/docs/advanced/config-yaml.mdx +++ b/docs/advanced/config-yaml.mdx @@ -110,6 +110,13 @@ For deployments mounted below a domain root, set `server.base_path` or To change the inbound user path header, set `server.user_path_header` or `USER_PATH_HEADER`. The default remains `X-GoModel-User-Path`. +Edits to this file are applied to a running gateway with `gomodel --reload`, +which re-reads the file and the environment and rebuilds the gateway in place — +see [CLI Operations](/advanced/cli#configuration-reload). Nothing changes if the +new file fails to load. `server.port` and `server.pid_file` are the exceptions: +both are fixed for the life of the process, so a reload logs a warning and keeps +the running values until the gateway is restarted. + ## Docker GoModel reads `config/config.yaml` first, then `config.yaml`. diff --git a/docs/advanced/configuration.mdx b/docs/advanced/configuration.mdx index 95518e16..2eb2e0b8 100644 --- a/docs/advanced/configuration.mdx +++ b/docs/advanced/configuration.mdx @@ -47,6 +47,7 @@ The most common way to configure GoModel. Set any of the variables below to over | `GOMODEL_DEMO_MODE` | Enable public demo warnings in logs and the dashboard | `false` | | `BODY_SIZE_LIMIT` | Max request body size (e.g., `10M`, `1024K`, `500KB`) | _(no limit)_ | | `USER_PATH_HEADER` | Header used to read/write request `user_path` values | `X-GoModel-User-Path` | +| `PID_FILE` | Where the running gateway records its process id for `gomodel --reload`. Changing it needs a restart | `data/gomodel.pid` next to a `./data` directory, otherwise the per-user data directory | Set `GOMODEL_DEMO_MODE=true` for a public or shared demonstration instance. GoModel logs a warning at startup and every five minutes, renders a persistent diff --git a/internal/platformdir/platformdir.go b/internal/platformdir/platformdir.go index a2a6c390..f94002ce 100644 --- a/internal/platformdir/platformdir.go +++ b/internal/platformdir/platformdir.go @@ -45,6 +45,32 @@ func DataDir() (string, error) { } } +// LocalDataDir is the project-local data directory. Deployments that already +// have one — the Docker image, anyone running from a checkout — keep their +// state there, so upgrades never move a database or a pid file out from under +// an operator. +const LocalDataDir = "data" + +// DataFile returns where a durable data file called name belongs: inside +// LocalDataDir when that directory exists next to the process, otherwise inside +// DataDir. Callers share this rule so a deployment's files stay together +// instead of some landing project-local and others in the per-user directory. +// +// The local form is spelled with a forward slash on every platform, which +// Windows accepts, so it stays comparable to the legacy path constants built +// the same way. +func DataFile(name string) string { + local := LocalDataDir + "/" + name + if info, err := os.Stat(LocalDataDir); err == nil && info.IsDir() { + return local + } + dir, err := DataDir() + if err != nil { + return local + } + return filepath.Join(dir, name) +} + // CacheDir returns the directory for re-creatable caches such as the model // catalog: // diff --git a/internal/server/http.go b/internal/server/http.go index 818427fd..0cd06b03 100644 --- a/internal/server/http.go +++ b/internal/server/http.go @@ -538,14 +538,16 @@ func (s *Server) Start(ctx context.Context, addr string) error { return newGatewayStartConfig(addr).Start(ctx, s.echo) } -// StartWithListener starts the HTTP server using a pre-bound listener. -// This is useful in tests that need an already-reserved loopback port. +// StartWithListener starts the HTTP server using a pre-bound listener. The +// gateway serves this way in production — the listening socket outlives each +// configuration a reload installs — and tests use it to reserve a loopback +// port up front, so it configures the server exactly like Start does: same +// inbound timeouts, same drain window. func (s *Server) StartWithListener(ctx context.Context, listener net.Listener) error { - sc := echo.StartConfig{ - HideBanner: true, - Listener: listener, + if listener == nil { + return errors.New("listener is required") } - return sc.Start(ctx, s.echo) + return newGatewayStartConfigForListener(listener).Start(ctx, s.echo) } // Shutdown releases server resources. The HTTP server itself is stopped by @@ -606,6 +608,14 @@ func newGatewayStartConfig(addr string) echo.StartConfig { } } +// newGatewayStartConfig with a pre-bound listener. Echo ignores Address once +// Listener is set; it is filled in anyway so the two describe the same server. +func newGatewayStartConfigForListener(listener net.Listener) echo.StartConfig { + sc := newGatewayStartConfig(listener.Addr().String()) + sc.Listener = listener + return sc +} + func configureGatewayHTTPServer(server *http.Server) error { if server == nil { return nil diff --git a/internal/server/http_start_test.go b/internal/server/http_start_test.go index f59cf79e..d6f34654 100644 --- a/internal/server/http_start_test.go +++ b/internal/server/http_start_test.go @@ -2,6 +2,7 @@ package server import ( "context" + "net" "net/http" "net/http/httptest" "testing" @@ -120,3 +121,58 @@ func (w *deadlineTrackingWriter) SetWriteDeadline(deadline time.Time) error { w.deadlines = append(w.deadlines, deadline) return nil } + +// The gateway serves on a pre-bound listener in production, because the +// listening socket has to outlive the configuration a reload replaces. That +// path went through a bare start config once, which silently dropped the +// inbound timeouts and the drain window from every request the gateway served. +func TestNewGatewayStartConfigForListener_KeepsTheServerConfiguration(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer listener.Close() + + cfg := newGatewayStartConfigForListener(listener) + + if cfg.Listener != listener { + t.Error("Listener = nil, want the pre-bound listener") + } + if cfg.GracefulTimeout != GracefulDrainTimeout { + t.Errorf("GracefulTimeout = %v, want %v", cfg.GracefulTimeout, GracefulDrainTimeout) + } + if cfg.OnShutdownError == nil { + t.Error("OnShutdownError = nil, want the drain cutoff reported by the gateway") + } + if cfg.BeforeServeFunc == nil { + t.Fatal("BeforeServeFunc = nil, want the inbound server timeouts") + } + + server := &http.Server{} + if err := cfg.BeforeServeFunc(server); err != nil { + t.Fatalf("BeforeServeFunc() error = %v", err) + } + for _, timeout := range []struct { + name string + got time.Duration + want time.Duration + }{ + {"ReadTimeout", server.ReadTimeout, inboundServerReadTimeout}, + {"ReadHeaderTimeout", server.ReadHeaderTimeout, inboundServerReadHeaderTimeout}, + {"WriteTimeout", server.WriteTimeout, inboundServerWriteTimeout}, + } { + if timeout.got != timeout.want { + t.Errorf("%s = %v, want %v", timeout.name, timeout.got, timeout.want) + } + } +} + +// A nil listener is a caller mistake, not something to hand to Echo: it would +// bind a fresh address from the empty start config and serve there instead. +func TestStartWithListenerRejectsANilListener(t *testing.T) { + srv := New(nil, &Config{}) + + if err := srv.StartWithListener(context.Background(), nil); err == nil { + t.Fatal("StartWithListener(nil) error = nil, want an error") + } +} diff --git a/internal/storage/storage.go b/internal/storage/storage.go index b23c9378..63c829c6 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -7,8 +7,6 @@ import ( "context" "database/sql" "fmt" - "os" - "path/filepath" "github.com/jackc/pgx/v5/pgxpool" "go.mongodb.org/mongo-driver/v2/mongo" @@ -31,16 +29,9 @@ const LegacySQLitePath = "data/gomodel.db" // DefaultSQLitePath returns the database path used when none is configured: // LegacySQLitePath when a ./data directory already exists, otherwise the -// OS-conventional per-user data directory (see platformdir.DataDir). +// OS-conventional per-user data directory (see platformdir.DataFile). func DefaultSQLitePath() string { - if info, err := os.Stat("data"); err == nil && info.IsDir() { - return LegacySQLitePath - } - dir, err := platformdir.DataDir() - if err != nil { - return LegacySQLitePath - } - return filepath.Join(dir, "gomodel.db") + return platformdir.DataFile("gomodel.db") } // Config holds storage configuration diff --git a/run/flags.go b/run/flags.go index 2d2f14b7..8aa07523 100644 --- a/run/flags.go +++ b/run/flags.go @@ -21,6 +21,7 @@ type cliOptions struct { HealthTimeout time.Duration Ready bool ReadyTimeout time.Duration + Reload bool } func parseCLI(productName string, args []string, output io.Writer) (cliOptions, error) { @@ -32,6 +33,7 @@ func parseCLI(productName string, args []string, output io.Writer) (cliOptions, flags.DurationVar(&opts.HealthTimeout, "health-timeout", defaultHealthTimeout, "Timeout for --health") flags.BoolVar(&opts.Ready, "ready", false, "Check the local GoModel readiness endpoint and exit") flags.DurationVar(&opts.ReadyTimeout, "ready-timeout", defaultReadyTimeout, "Timeout for --ready") + flags.BoolVar(&opts.Reload, "reload", false, "Tell the running GoModel to reload its configuration and exit") if err := flags.Parse(args); err != nil { return opts, err } diff --git a/run/lifecycle_test.go b/run/lifecycle_test.go index 56dc2820..a787faf6 100644 --- a/run/lifecycle_test.go +++ b/run/lifecycle_test.go @@ -3,6 +3,7 @@ package run import ( "context" "errors" + "net" "slices" "sync" "sync/atomic" @@ -24,7 +25,7 @@ type stubLifecycleApp struct { shutdownBlock <-chan struct{} } -func (s *stubLifecycleApp) Start(_ context.Context, _ string) error { +func (s *stubLifecycleApp) StartWithListener(_ context.Context, _ net.Listener) error { s.mu.Lock() s.startCalls++ s.mu.Unlock() @@ -63,11 +64,11 @@ func (s *stubLifecycleApp) capturedShutdownContext() context.Context { // A server that never came up still holds a database handle and whatever the // loggers buffered while it was being built, so it gets torn down — once, on // one shutdownTimeout budget, from the same place every other exit uses. -func TestServeUntilShutdown_TearsDownOnceAfterAFailedStart(t *testing.T) { +func TestServeGeneration_TearsDownOnceAfterAFailedStart(t *testing.T) { startErr := errors.New("listen tcp :8080: bind: address already in use") app := &stubLifecycleApp{startErr: startErr} - err := serveUntilShutdown(context.Background(), app, ":8080") + err := serveGeneration(context.Background(), app, nil) if !errors.Is(err, startErr) { t.Fatalf("error = %v, want start error %v", err, startErr) } @@ -92,11 +93,11 @@ func TestServeUntilShutdown_TearsDownOnceAfterAFailedStart(t *testing.T) { // The start error is what the operator needs to see and what sets the exit // code, so a teardown that also fails is logged rather than wrapped around it. -func TestServeUntilShutdown_ShutdownFailureDoesNotMaskTheStartError(t *testing.T) { +func TestServeGeneration_ShutdownFailureDoesNotMaskTheStartError(t *testing.T) { startErr := errors.New("listen failed") app := &stubLifecycleApp{startErr: startErr, shutdownErr: errors.New("close failed")} - err := serveUntilShutdown(context.Background(), app, ":8080") + err := serveGeneration(context.Background(), app, nil) if !errors.Is(err, startErr) { t.Fatalf("error = %v, want start error %v", err, startErr) } @@ -106,8 +107,8 @@ func TestServeUntilShutdown_ShutdownFailureDoesNotMaskTheStartError(t *testing.T } // A teardown that wedges must not wedge the process with it: the wait is -// bounded by shutdownTimeout and serveUntilShutdown returns regardless. -func TestServeUntilShutdown_StopsWaitingWhenShutdownTimesOut(t *testing.T) { +// bounded by shutdownTimeout and serveGeneration returns regardless. +func TestServeGeneration_StopsWaitingWhenShutdownTimesOut(t *testing.T) { previousTimeout := shutdownTimeout shutdownTimeout = 10 * time.Millisecond defer func() { @@ -122,7 +123,7 @@ func TestServeUntilShutdown_StopsWaitingWhenShutdownTimesOut(t *testing.T) { done := make(chan error, 1) go func() { - done <- serveUntilShutdown(context.Background(), app, ":8080") + done <- serveGeneration(context.Background(), app, nil) }() select { @@ -131,7 +132,7 @@ func TestServeUntilShutdown_StopsWaitingWhenShutdownTimesOut(t *testing.T) { t.Fatalf("error = %v, want start error %v", err, startErr) } case <-time.After(5 * time.Second): - t.Fatal("serveUntilShutdown blocked on a shutdown that never returned") + t.Fatal("serveGeneration blocked on a shutdown that never returned") } if calls := app.shutdownCallCount(); calls != 1 { t.Fatalf("shutdownCalls = %d, want 1", calls) @@ -151,12 +152,12 @@ func TestGracefulDrainFitsInsideTheShutdownBudget(t *testing.T) { } } -// servingApp mirrors the ordering that matters in the real App: Start blocks -// until Shutdown stops the server, and Shutdown keeps working afterwards — -// flushing buffered usage and audit records, closing the database — before it -// returns. +// servingApp mirrors the ordering that matters in the real App: +// StartWithListener blocks until Shutdown stops the server, and Shutdown keeps +// working afterwards — flushing buffered usage and audit records, closing the +// database — before it returns. type servingApp struct { - serverStopped chan struct{} // closed by Shutdown, releases Start + serverStopped chan struct{} // closed by Shutdown, releases StartWithListener flushing chan struct{} // closed by the test, releases Shutdown shutdownDone atomic.Bool } @@ -168,7 +169,7 @@ func newServingApp() *servingApp { } } -func (a *servingApp) Start(context.Context, string) error { +func (a *servingApp) StartWithListener(context.Context, net.Listener) error { <-a.serverStopped return nil } @@ -184,14 +185,14 @@ func (a *servingApp) Shutdown(context.Context) error { // flushing loses whatever it had not written yet. That is what happened on // every Ctrl+C: the server stopped, Start returned, the process left, and // "application shutdown complete" was never reached. -func TestServeUntilShutdown_WaitsForTeardownToFinish(t *testing.T) { +func TestServeGeneration_WaitsForTeardownToFinish(t *testing.T) { app := newServingApp() ctx, cancel := context.WithCancel(context.Background()) defer cancel() returned := make(chan error, 1) go func() { - returned <- serveUntilShutdown(ctx, app, ":0") + returned <- serveGeneration(ctx, app, nil) }() cancel() // the SIGINT equivalent @@ -199,7 +200,7 @@ func TestServeUntilShutdown_WaitsForTeardownToFinish(t *testing.T) { // Start has returned by now; Shutdown is still flushing. select { case err := <-returned: - t.Fatalf("serveUntilShutdown returned mid-teardown (error = %v)", err) + t.Fatalf("serveGeneration returned mid-teardown (error = %v)", err) case <-time.After(100 * time.Millisecond): } @@ -207,10 +208,10 @@ func TestServeUntilShutdown_WaitsForTeardownToFinish(t *testing.T) { select { case err := <-returned: if err != nil { - t.Fatalf("serveUntilShutdown() error = %v, want nil", err) + t.Fatalf("serveGeneration() error = %v, want nil", err) } case <-time.After(5 * time.Second): - t.Fatal("serveUntilShutdown did not return after teardown finished") + t.Fatal("serveGeneration did not return after teardown finished") } if !app.shutdownDone.Load() { t.Fatal("teardown did not run to completion") @@ -219,23 +220,23 @@ func TestServeUntilShutdown_WaitsForTeardownToFinish(t *testing.T) { // A server that stops without a signal still owns a database handle and // buffered records, so it gets the same teardown. -func TestServeUntilShutdown_TearsDownWhenServerStopsOnItsOwn(t *testing.T) { +func TestServeGeneration_TearsDownWhenServerStopsOnItsOwn(t *testing.T) { app := &stubLifecycleApp{} - if err := serveUntilShutdown(context.Background(), app, ":0"); err != nil { - t.Fatalf("serveUntilShutdown() error = %v, want nil", err) + if err := serveGeneration(context.Background(), app, nil); err != nil { + t.Fatalf("serveGeneration() error = %v, want nil", err) } if calls := app.shutdownCallCount(); calls != 1 { t.Fatalf("shutdownCalls = %d, want 1", calls) } } -func TestServeUntilShutdown_ReturnsStartFailure(t *testing.T) { +func TestServeGeneration_ReturnsStartFailure(t *testing.T) { startErr := errors.New("listen tcp :8080: bind: address already in use") app := &stubLifecycleApp{startErr: startErr} - if err := serveUntilShutdown(context.Background(), app, ":8080"); !errors.Is(err, startErr) { - t.Fatalf("serveUntilShutdown() error = %v, want start error %v", err, startErr) + if err := serveGeneration(context.Background(), app, nil); !errors.Is(err, startErr) { + t.Fatalf("serveGeneration() error = %v, want start error %v", err, startErr) } } diff --git a/run/reload.go b/run/reload.go new file mode 100644 index 00000000..826d00e8 --- /dev/null +++ b/run/reload.go @@ -0,0 +1,191 @@ +package run + +import ( + "errors" + "fmt" + "io" + "io/fs" + "log/slog" + "maps" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + + "github.com/joho/godotenv" + + "github.com/enterpilot/gomodel/config" +) + +// reloadSignal asks a running gateway to re-read its configuration, the same +// way SIGHUP does for nginx. `gomodel --reload` sends it; `kill -HUP ` +// works just as well. +const reloadSignal = syscall.SIGHUP + +// envFile is the environment file loaded at startup and re-read on reload. +const envFile = ".env" + +// dotenv applies envFile to the process environment and remembers which +// variables it set. A reload can then pick up edited values, and drop the ones +// removed from the file, without clobbering variables that came from the real +// environment: a value already exported wins over the file, exactly as it does +// at startup. +type dotenv struct { + applied map[string]string +} + +func newDotenv() *dotenv { + return &dotenv{applied: make(map[string]string)} +} + +// apply merges the current contents of envFile into the process environment. +// A missing file is normal — configuration may come entirely from the real +// environment — and clears whatever the file previously contributed. A file +// that cannot be read or parsed is not: it leaves the environment as it stands, +// so a half-typed edit does not strip the running configuration. +// +// The returned function undoes exactly the changes this call made. A reload +// reads the file before it knows whether the configuration built from it is +// usable, so rejecting that configuration has to put the environment back the +// way the still-running gateway expects it. +func (d *dotenv) apply() (undo func()) { + values, err := godotenv.Read(envFile) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + slog.Warn("failed to read env file; keeping the current environment", "file", envFile, "error", err) + return func() {} + } + values = map[string]string{} + } + + // A nil entry records a variable that was not set before this call. + previous := make(map[string]*string) + record := func(key string) { + if _, seen := previous[key]; seen { + return + } + if value, exported := os.LookupEnv(key); exported { + previous[key] = &value + return + } + previous[key] = nil + } + appliedBefore := maps.Clone(d.applied) + + for key, value := range values { + if _, owned := d.applied[key]; !owned { + if _, exported := os.LookupEnv(key); exported { + continue + } + } + record(key) + if err := os.Setenv(key, value); err != nil { + slog.Warn("failed to apply env file variable", "file", envFile, "variable", key, "error", err) + continue + } + d.applied[key] = value + } + + for key := range d.applied { + if _, present := values[key]; present { + continue + } + record(key) + if err := os.Unsetenv(key); err != nil { + slog.Warn("failed to unset removed env file variable", "file", envFile, "variable", key, "error", err) + continue + } + delete(d.applied, key) + } + + return func() { + for key, value := range previous { + if value == nil { + _ = os.Unsetenv(key) + continue + } + _ = os.Setenv(key, *value) + } + d.applied = appliedBefore + } +} + +// writePIDFile records the running process id so `gomodel --reload` can find +// the gateway to signal. The returned function removes the file again, unless +// another instance has claimed it in the meantime. +func writePIDFile(path string) (func(), error) { + remove := func() {} + path = strings.TrimSpace(path) + if path == "" { + return remove, nil + } + if dir := filepath.Dir(path); dir != "" && dir != "." { + if err := os.MkdirAll(dir, 0o755); err != nil { + return remove, fmt.Errorf("create pid file directory %s: %w", dir, err) + } + } + if err := os.WriteFile(path, []byte(strconv.Itoa(os.Getpid())+"\n"), 0o644); err != nil { + return remove, fmt.Errorf("write pid file %s: %w", path, err) + } + return func() { + // Leave the file alone if it is no longer ours: another instance has + // taken this path over, and removing it would leave that one + // unreachable from --reload. + if pid, err := readPIDFile(path); err == nil && pid != os.Getpid() { + return + } + _ = os.Remove(path) + }, nil +} + +// readPIDFile returns the process id recorded at path. A missing file and a +// file that holds anything else both report why, since both are what an +// operator sees when --reload cannot find the gateway. +func readPIDFile(path string) (int, error) { + data, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return 0, fmt.Errorf("no pid file at %s: is the gateway running?", path) + } + return 0, fmt.Errorf("read pid file %s: %w", path, err) + } + pid, err := strconv.Atoi(strings.TrimSpace(string(data))) + if err != nil || pid <= 0 { + return 0, fmt.Errorf("pid file %s does not contain a process id", path) + } + return pid, nil +} + +// sendReloadSignal implements --reload: it tells the gateway recorded in the +// pid file to re-read its configuration, the way `nginx -s reload` does. The +// running process keeps serving on the current configuration if the new one +// turns out to be invalid. +func sendReloadSignal(stdout io.Writer) error { + result, err := config.Load() + if err != nil { + return fmt.Errorf("load config: %w", err) + } + + path := strings.TrimSpace(result.Config.Server.PIDFile) + if path == "" { + return errors.New("no pid file configured: set server.pid_file (or PID_FILE) on the gateway and on this command") + } + pid, err := readPIDFile(path) + if err != nil { + return err + } + process, err := os.FindProcess(pid) + if err != nil { + return fmt.Errorf("find process %d from %s: %w", pid, path, err) + } + if err := process.Signal(reloadSignal); err != nil { + if errors.Is(err, os.ErrProcessDone) || errors.Is(err, syscall.ESRCH) { + return fmt.Errorf("process %d from %s is not running: remove the stale pid file", pid, path) + } + return fmt.Errorf("signal process %d from %s: %w", pid, path, err) + } + + fmt.Fprintf(stdout, "reload requested (pid %d)\n", pid) + return nil +} diff --git a/run/reload_test.go b/run/reload_test.go new file mode 100644 index 00000000..ad84b46b --- /dev/null +++ b/run/reload_test.go @@ -0,0 +1,411 @@ +package run + +import ( + "context" + "errors" + "net" + "os" + "os/signal" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" +) + +// A .env value applies only where the real environment has nothing to say, +// which is godotenv.Load's rule and therefore the rule a reload has to keep. +func TestDotenvLeavesExportedVariablesAlone(t *testing.T) { + t.Chdir(t.TempDir()) + t.Setenv("GOMODEL_TEST_EXPORTED", "from-environment") + writeEnvFile(t, "GOMODEL_TEST_EXPORTED=from-file\nGOMODEL_TEST_FILE_ONLY=from-file\n") + t.Cleanup(func() { os.Unsetenv("GOMODEL_TEST_FILE_ONLY") }) + + newDotenv().apply() + + if got := os.Getenv("GOMODEL_TEST_EXPORTED"); got != "from-environment" { + t.Errorf("exported variable = %q, want it untouched by the env file", got) + } + if got := os.Getenv("GOMODEL_TEST_FILE_ONLY"); got != "from-file" { + t.Errorf("file-only variable = %q, want %q", got, "from-file") + } +} + +// Reloading is worth little if it cannot see edited credentials and endpoints, +// so a second apply must pick up new values and forget deleted ones — without +// ever taking over a variable the process was started with. +func TestDotenvReappliesEditedFile(t *testing.T) { + t.Chdir(t.TempDir()) + t.Setenv("GOMODEL_TEST_EXPORTED", "from-environment") + writeEnvFile(t, "GOMODEL_TEST_EXPORTED=from-file\nGOMODEL_TEST_EDITED=before\nGOMODEL_TEST_REMOVED=present\n") + t.Cleanup(func() { + os.Unsetenv("GOMODEL_TEST_EDITED") + os.Unsetenv("GOMODEL_TEST_REMOVED") + }) + + env := newDotenv() + env.apply() + writeEnvFile(t, "GOMODEL_TEST_EXPORTED=from-file\nGOMODEL_TEST_EDITED=after\n") + env.apply() + + if got := os.Getenv("GOMODEL_TEST_EDITED"); got != "after" { + t.Errorf("edited variable = %q, want %q", got, "after") + } + if _, present := os.LookupEnv("GOMODEL_TEST_REMOVED"); present { + t.Error("variable dropped from the env file is still set") + } + if got := os.Getenv("GOMODEL_TEST_EXPORTED"); got != "from-environment" { + t.Errorf("exported variable = %q, want it untouched by the env file", got) + } +} + +// A missing .env file is the normal case for container deployments: it means +// "configuration comes from the environment", not "keep the last file I saw". +func TestDotenvClearsWhenTheFileDisappears(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + writeEnvFile(t, "GOMODEL_TEST_VANISHING=present\n") + t.Cleanup(func() { os.Unsetenv("GOMODEL_TEST_VANISHING") }) + + env := newDotenv() + env.apply() + if err := os.Remove(filepath.Join(dir, envFile)); err != nil { + t.Fatal(err) + } + env.apply() + + if _, present := os.LookupEnv("GOMODEL_TEST_VANISHING"); present { + t.Error("variable survived the removal of the env file") + } +} + +func TestPIDFileRoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "nested", "gomodel.pid") + + remove, err := writePIDFile(path) + if err != nil { + t.Fatalf("writePIDFile() error = %v", err) + } + pid, err := readPIDFile(path) + if err != nil { + t.Fatalf("readPIDFile() error = %v", err) + } + if pid != os.Getpid() { + t.Errorf("pid = %d, want %d", pid, os.Getpid()) + } + + remove() + if _, err := readPIDFile(path); err == nil { + t.Error("readPIDFile() after removal = nil error, want an error") + } +} + +func TestPIDFileEmptyPathIsANoop(t *testing.T) { + remove, err := writePIDFile(" ") + if err != nil { + t.Fatalf("writePIDFile(\"\") error = %v", err) + } + remove() +} + +func TestReadPIDFileRejectsGarbage(t *testing.T) { + path := filepath.Join(t.TempDir(), "gomodel.pid") + if err := os.WriteFile(path, []byte("not-a-pid"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := readPIDFile(path); err == nil { + t.Error("readPIDFile() on a garbage file = nil error, want an error") + } +} + +// The whole point of building the replacement before stopping what is running: +// a configuration that does not load must cost nothing but a log line. +func TestServeUntilShutdownKeepsServingWhenReloadFails(t *testing.T) { + socket := testSocket(t) + first := newFakeGeneration() + second := newFakeGeneration() + + attempts := make(chan struct{}, 2) + var count atomic.Int32 + rebuild := func() (lifecycleApp, error) { + defer func() { attempts <- struct{}{} }() + if count.Add(1) == 1 { + return nil, errors.New("invalid configuration") + } + return second, nil + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + reload := make(chan os.Signal, 1) + served := make(chan error, 1) + go func() { served <- serveUntilShutdown(ctx, reload, socket, first, rebuild) }() + + <-first.started + reload <- reloadSignal + <-attempts + if first.shutdowns.Load() != 0 { + t.Fatal("a failed reload stopped the running generation") + } + + reload <- reloadSignal + <-attempts + <-second.started + if got := first.shutdowns.Load(); got != 1 { + t.Fatalf("first generation shutdowns = %d, want 1", got) + } + + cancel() + select { + case err := <-served: + if err != nil { + t.Fatalf("serveUntilShutdown() error = %v, want nil", err) + } + case <-time.After(5 * time.Second): + t.Fatal("serveUntilShutdown did not return after cancellation") + } + if got := second.shutdowns.Load(); got != 1 { + t.Fatalf("second generation shutdowns = %d, want 1", got) + } +} + +// Reloading must not cost the port. This walks the window a reload opens: the +// generation that was serving has closed its listener and the next one has not +// started, and a client connecting right then must still be connected — waiting +// in the kernel's accept queue — rather than refused. +func TestBoundSocketSurvivesGenerations(t *testing.T) { + socket := testSocket(t) + if socket.file == nil { + t.Skip("this platform cannot duplicate the listening socket; generations rebind instead") + } + + first, err := socket.next() + if err != nil { + t.Fatalf("socket.next() error = %v", err) + } + address := first.Addr().String() + if err := first.Close(); err != nil { + t.Fatalf("close first listener: %v", err) + } + + // Nothing is accepting at this point. + conn, err := net.DialTimeout("tcp", address, 5*time.Second) + if err != nil { + t.Fatalf("connect while no generation is accepting: %v", err) + } + defer conn.Close() + + second, err := socket.next() + if err != nil { + t.Fatalf("socket.next() after a generation ended = %v", err) + } + defer second.Close() + if got := second.Addr().String(); got != address { + t.Fatalf("second generation address = %q, want %q", got, address) + } + if deadliner, ok := second.(interface{ SetDeadline(time.Time) error }); ok { + if err := deadliner.SetDeadline(time.Now().Add(5 * time.Second)); err != nil { + t.Fatalf("set accept deadline: %v", err) + } + } + + waiting, err := second.Accept() + if err != nil { + t.Fatalf("accept the connection that waited through the swap: %v", err) + } + _ = waiting.Close() +} + +func testSocket(t *testing.T) *boundSocket { + t.Helper() + socket, err := listenOn("127.0.0.1:0") + if err != nil { + t.Fatalf("listenOn() error = %v", err) + } + t.Cleanup(func() { _ = socket.Close() }) + return socket +} + +func writeEnvFile(t *testing.T, contents string) { + t.Helper() + if err := os.WriteFile(envFile, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } +} + +// fakeGeneration stands in for one built application: it serves until it is +// shut down, and records how often that happened. +type fakeGeneration struct { + started chan struct{} + stopped chan struct{} + stopOnce sync.Once + shutdowns atomic.Int32 +} + +func newFakeGeneration() *fakeGeneration { + return &fakeGeneration{started: make(chan struct{}), stopped: make(chan struct{})} +} + +func (g *fakeGeneration) StartWithListener(_ context.Context, listener net.Listener) error { + if listener != nil { + defer listener.Close() + } + close(g.started) + <-g.stopped + return nil +} + +func (g *fakeGeneration) Shutdown(context.Context) error { + g.shutdowns.Add(1) + g.stopOnce.Do(func() { close(g.stopped) }) + return nil +} + +// A second instance configured with the same pid file path owns it; the first +// one must not remove it on its way out, or --reload loses the survivor. +func TestPIDFileRemovalLeavesAnotherInstanceAlone(t *testing.T) { + path := filepath.Join(t.TempDir(), "gomodel.pid") + remove, err := writePIDFile(path) + if err != nil { + t.Fatalf("writePIDFile() error = %v", err) + } + if err := os.WriteFile(path, []byte("424242\n"), 0o644); err != nil { + t.Fatal(err) + } + + remove() + + pid, err := readPIDFile(path) + if err != nil { + t.Fatalf("readPIDFile() error = %v, want the other instance's pid file intact", err) + } + if pid != 424242 { + t.Errorf("pid = %d, want 424242", pid) + } +} + +// A reload reads the environment file before it can know whether the +// configuration built from it works. When it does not, the generation that +// keeps serving must keep the environment it was built with — the operator was +// told the new configuration was rejected. +func TestDotenvApplyUndoRestoresTheEnvironment(t *testing.T) { + t.Chdir(t.TempDir()) + t.Setenv("GOMODEL_TEST_EXPORTED", "from-environment") + writeEnvFile(t, "GOMODEL_TEST_KEPT=before\nGOMODEL_TEST_DROPPED=present\n") + t.Cleanup(func() { + os.Unsetenv("GOMODEL_TEST_KEPT") + os.Unsetenv("GOMODEL_TEST_DROPPED") + os.Unsetenv("GOMODEL_TEST_ADDED") + }) + + env := newDotenv() + env.apply() + + // The edit a failed reload would have read. + writeEnvFile(t, "GOMODEL_TEST_KEPT=after\nGOMODEL_TEST_ADDED=new\nGOMODEL_TEST_EXPORTED=from-file\n") + undo := env.apply() + if got := os.Getenv("GOMODEL_TEST_KEPT"); got != "after" { + t.Fatalf("edited variable before undo = %q, want %q", got, "after") + } + + undo() + + if got := os.Getenv("GOMODEL_TEST_KEPT"); got != "before" { + t.Errorf("edited variable after undo = %q, want %q", got, "before") + } + if got := os.Getenv("GOMODEL_TEST_DROPPED"); got != "present" { + t.Errorf("removed variable after undo = %q, want %q", got, "present") + } + if _, present := os.LookupEnv("GOMODEL_TEST_ADDED"); present { + t.Error("variable added by the rejected file is still set") + } + if got := os.Getenv("GOMODEL_TEST_EXPORTED"); got != "from-environment" { + t.Errorf("exported variable = %q, want it untouched throughout", got) + } + + // The bookkeeping has to be restored too, or the next reload treats the + // rolled-back variables as none of its business. + writeEnvFile(t, "GOMODEL_TEST_KEPT=third\n") + env.apply() + if got := os.Getenv("GOMODEL_TEST_KEPT"); got != "third" { + t.Errorf("variable after a later reload = %q, want %q", got, "third") + } + if _, present := os.LookupEnv("GOMODEL_TEST_DROPPED"); present { + t.Error("variable dropped from the env file survived the later reload") + } +} + +func TestSendReloadSignal(t *testing.T) { + tests := []struct { + name string + pidFile func(t *testing.T, dir string) string + wantError bool + }{ + { + name: "signals the process named by the pid file", + pidFile: func(t *testing.T, dir string) string { + path := filepath.Join(dir, "gomodel.pid") + remove, err := writePIDFile(path) + if err != nil { + t.Fatalf("writePIDFile() error = %v", err) + } + t.Cleanup(remove) + return path + }, + }, + { + name: "reports a missing pid file", + pidFile: func(t *testing.T, dir string) string { + return filepath.Join(dir, "absent.pid") + }, + wantError: true, + }, + { + name: "reports a pid file that names no process", + pidFile: func(t *testing.T, dir string) string { + path := filepath.Join(dir, "garbage.pid") + if err := os.WriteFile(path, []byte("not-a-pid"), 0o644); err != nil { + t.Fatal(err) + } + return path + }, + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) // no config.yaml here, so only PID_FILE decides the path + t.Setenv("PID_FILE", tt.pidFile(t, dir)) + + // Registered before signalling, exactly as the gateway does it, so a + // delivered SIGHUP is caught here instead of killing the test binary. + delivered := make(chan os.Signal, 1) + signal.Notify(delivered, reloadSignal) + defer signal.Stop(delivered) + + var out strings.Builder + err := sendReloadSignal(&out) + if tt.wantError { + if err == nil { + t.Fatal("sendReloadSignal() error = nil, want an error") + } + return + } + if err != nil { + t.Fatalf("sendReloadSignal() error = %v", err) + } + + select { + case <-delivered: + case <-time.After(5 * time.Second): + t.Fatal("the reload signal was never delivered") + } + if !strings.Contains(out.String(), "reload requested") { + t.Errorf("output = %q, want it to confirm the reload", out.String()) + } + }) + } +} diff --git a/run/run.go b/run/run.go index 275a2f14..69da7ca3 100644 --- a/run/run.go +++ b/run/run.go @@ -18,14 +18,13 @@ import ( "fmt" "io" "log/slog" + "net" "os" "os/signal" "runtime" "syscall" "time" - "github.com/joho/godotenv" - "github.com/enterpilot/gomodel/config" "github.com/enterpilot/gomodel/ext" "github.com/enterpilot/gomodel/internal/app" @@ -101,11 +100,16 @@ func ExitCode(err error) int { return 1 } -// Run executes the full gateway lifecycle: CLI parsing, --version and -// --health/--ready probe modes, dotenv loading, logging setup, config -// loading, provider registration, application construction (including -// registered extensions), signal handling, and start with graceful shutdown. -// Cancelling ctx triggers the same graceful shutdown as SIGINT/SIGTERM. +// Run executes the full gateway lifecycle: CLI parsing, --version, +// --health/--ready probe and --reload signalling modes, dotenv loading, +// logging setup, config loading, provider registration, application +// construction (including registered extensions), signal handling, and start +// with graceful shutdown. +// +// Cancelling ctx triggers the same graceful shutdown as SIGINT/SIGTERM. A +// reload signal (SIGHUP, what `gomodel --reload` sends) instead re-reads the +// environment file and the configuration and replaces the running application +// with one built from them, without giving up the listening socket. func Run(ctx context.Context, opts Options) error { opts = opts.withDefaults() @@ -122,7 +126,8 @@ func Run(ctx context.Context, opts Options) error { return nil } - _ = godotenv.Load() + env := newDotenv() + env.apply() // startup has nothing to roll back to if cliOpts.Health { if err := runHealthProbe(cliOpts.HealthTimeout); err != nil { @@ -140,6 +145,14 @@ func Run(ctx context.Context, opts Options) error { return nil } + if cliOpts.Reload { + if err := sendReloadSignal(opts.Stdout); err != nil { + fmt.Fprintf(opts.Stderr, "reload failed: %v\n", err) + return err + } + return nil + } + demoMode, err := demoModeFromEnv() if err != nil { fmt.Fprintln(opts.Stderr, err) @@ -169,29 +182,91 @@ func Run(ctx context.Context, opts Options) error { } } - result, err := config.Load() + // build produces one generation of the gateway from the configuration as it + // stands right now. It is called again for every reload, which is what lets + // a reload re-read every configuration value rather than a hand-picked + // subset — everything except what is fixed for the life of the process (see + // warnAboutStartupOnlySettings). + build := func() (*app.App, *config.Config, error) { + result, err := config.Load() + if err != nil { + return nil, nil, fmt.Errorf("failed to load config: %w", err) + } + opts.ConfigureSwaggerDocs(result.Config.Server.BasePath) + + application, err := app.New(ctx, app.Config{ + AppConfig: result, + Factory: defaultProviderFactory(result.Config), + Extensions: opts.Extensions, + DemoMode: demoMode, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to initialize application: %w", err) + } + return application, result.Config, nil + } + + application, appCfg, err := build() if err != nil { - slog.Error("failed to load config", "error", err) + slog.Error("startup failed", "error", err) return err } - opts.ConfigureSwaggerDocs(result.Config.Server.BasePath) - - application, err := app.New(ctx, app.Config{ - AppConfig: result, - Factory: defaultProviderFactory(result.Config), - Extensions: opts.Extensions, - DemoMode: demoMode, - }) + + socket, err := listenOn(":" + appCfg.Server.Port) if err != nil { - slog.Error("failed to initialize application", "error", err) + slog.Error("failed to bind the server address", "error", err) + _ = shutdownApplicationWithTimeout(application) return err } + defer func() { _ = socket.Close() }() + + // Claim the reload signal before the pid file exists. The pid file is what + // tells an operator or a process manager that this instance can be signalled, + // and until Notify runs, SIGHUP still carries its default disposition: it + // would kill the gateway instead of reloading it. + reload := make(chan os.Signal, 1) + signal.Notify(reload, reloadSignal) + defer signal.Stop(reload) + + removePIDFile, err := writePIDFile(appCfg.Server.PIDFile) + if err != nil { + slog.Warn("could not write the pid file; --reload will not find this instance", "error", err) + } + defer removePIDFile() signalCtx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) defer stop() - addr := ":" + result.Config.Server.Port - if err := serveUntilShutdown(signalCtx, application, addr); err != nil { + // rebuild prepares the next generation. Reading the environment file and + // installing the new logging configuration have to happen first — config.Load + // reads the process environment, and the new log level applies to the build + // itself — but both are process-wide, so a build that fails would otherwise + // leave the generation that keeps serving running under a configuration the + // operator was told had been rejected. Everything is put back instead. + rebuild := func() (lifecycleApp, error) { + // Variables exported into the process keep winning over the file. + undoEnv := env.apply() + previousLogger := slog.Default() + rollback := func() { + slog.SetDefault(previousLogger) + undoEnv() + } + + if err := configureLogging(opts.Stderr); err != nil { + rollback() + return nil, err + } + next, nextCfg, err := build() + if err != nil { + rollback() + return nil, err + } + warnAboutStartupOnlySettings(appCfg.Server, nextCfg.Server) + appCfg = nextCfg + return next, nil + } + + if err := serveUntilShutdown(signalCtx, reload, socket, application, rebuild); err != nil { slog.Error("application failed", "error", err) return err } @@ -204,26 +279,94 @@ func versionLine(productName string) string { } type lifecycleApp interface { - Start(ctx context.Context, addr string) error + StartWithListener(ctx context.Context, listener net.Listener) error Shutdown(ctx context.Context) error } -// serveUntilShutdown starts the application and returns only once the server -// has stopped *and* the teardown that stopped it has finished. +// serveUntilShutdown serves the gateway until it is asked to stop, replacing +// the running application with a freshly configured one every time a reload +// signal arrives. +// +// A reload builds its replacement before stopping the generation it replaces, +// so a configuration that fails to load or to initialize leaves the gateway +// serving on the one that already works — nginx's rule, and the reason the +// operator can reload without holding their breath. +func serveUntilShutdown(ctx context.Context, reload <-chan os.Signal, socket *boundSocket, application lifecycleApp, rebuild func() (lifecycleApp, error)) error { + for { + listener, err := socket.next() + if err != nil { + _ = shutdownApplicationWithTimeout(application) + return err + } + + generationCtx, endGeneration := context.WithCancel(ctx) + replacement := watchForReload(generationCtx, endGeneration, reload, rebuild) + + startErr := serveGeneration(generationCtx, application, listener) + endGeneration() + + next := <-replacement + switch { + case next == nil: + return startErr + case startErr != nil || ctx.Err() != nil: + // The gateway is on its way out anyway, so the replacement built + // alongside the shutdown never gets to serve. + _ = shutdownApplicationWithTimeout(next) + return startErr + } + application = next + slog.Info("configuration reloaded") + } +} + +// watchForReload turns reload signals into the next application generation. It +// builds the replacement first and ends the running generation only once that +// succeeded, so a failed build costs a log line rather than an outage. The +// returned channel yields the replacement, or nil when the generation ended for +// any other reason. +func watchForReload(ctx context.Context, endGeneration context.CancelFunc, reload <-chan os.Signal, rebuild func() (lifecycleApp, error)) <-chan lifecycleApp { + replacement := make(chan lifecycleApp, 1) + go func() { + defer close(replacement) + for { + select { + case <-ctx.Done(): + return + case <-reload: + slog.Info("reloading configuration") + next, err := rebuild() + if err != nil { + slog.Error("reload failed; keeping the running configuration", "error", err) + continue + } + replacement <- next + endGeneration() + return + } + } + }() + return replacement +} + +// serveGeneration starts one application generation and returns only once its +// server has stopped *and* the teardown that stopped it has finished. // -// The teardown has to run on its own goroutine because Start blocks until the -// server stops and Shutdown is what stops it. Waiting for that goroutine here -// is the load-bearing part: the process exits the moment Run returns, so -// anything Shutdown had not reached yet — the buffered usage and audit -// records, the database handle — would be dropped on every Ctrl+C. +// The teardown has to run on its own goroutine because StartWithListener +// blocks until the server stops and Shutdown is what stops it. Waiting for that +// goroutine here is the load-bearing part: the process exits the moment Run +// returns, so anything Shutdown had not reached yet — the buffered usage and +// audit records, the database handle — would be dropped on every Ctrl+C. // -// This is the only caller of shutdownApplication, so teardown runs exactly -// once per exit and on a single shutdownTimeout budget, whichever way the -// server ended: a signal, a stop of its own accord, or a Start that never got -// off the ground all converge here. Routing the failed-start path through the -// same place is what removes the second teardown that used to run alongside -// it, and with it any reliance on Shutdown being idempotent. -func serveUntilShutdown(ctx context.Context, application lifecycleApp, addr string) error { +// Every generation that serves is torn down here, exactly once and on a single +// shutdownTimeout budget, whichever way the server ended: a signal, a reload, a +// stop of its own accord, or a start that never got off the ground all converge +// here. Routing the failed-start path through the same place is what removes +// the second teardown that used to run alongside it, and with it any reliance +// on Shutdown being idempotent. An application built but never served — one +// that could not be given a listener, or a replacement overtaken by shutdown — +// is torn down by its owner in serveUntilShutdown, on the same budget. +func serveGeneration(ctx context.Context, application lifecycleApp, listener net.Listener) error { serverReturned := make(chan struct{}) shutdownDone := make(chan error, 1) go func() { @@ -231,12 +374,10 @@ func serveUntilShutdown(ctx context.Context, application lifecycleApp, addr stri case <-ctx.Done(): case <-serverReturned: } - shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) - defer cancel() - shutdownDone <- shutdownApplication(application, shutdownCtx) + shutdownDone <- shutdownApplicationWithTimeout(application) }() - startErr := application.Start(context.Background(), addr) + startErr := application.StartWithListener(context.Background(), listener) close(serverReturned) if err := <-shutdownDone; err != nil { @@ -245,6 +386,28 @@ func serveUntilShutdown(ctx context.Context, application lifecycleApp, addr stri return startErr } +// shutdownApplicationWithTimeout tears an application down on the one budget +// every teardown gets, whether or not it ever served. +func shutdownApplicationWithTimeout(application lifecycleApp) error { + shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) + defer cancel() + return shutdownApplication(application, shutdownCtx) +} + +// warnAboutStartupOnlySettings reports the settings a reload cannot apply: the +// listening socket is kept bound across generations precisely so no connection +// is refused, and the pid file names the process that is already running. +func warnAboutStartupOnlySettings(current, next config.ServerConfig) { + if next.Port != current.Port { + slog.Warn("server port change needs a restart; keeping the bound address", + "bound_port", current.Port, "configured_port", next.Port) + } + if next.PIDFile != current.PIDFile { + slog.Warn("pid file change needs a restart; keeping the current pid file", + "current_pid_file", current.PIDFile, "configured_pid_file", next.PIDFile) + } +} + func shutdownApplication(application lifecycleApp, ctx context.Context) error { done := make(chan error, 1) go func() { diff --git a/run/socket.go b/run/socket.go new file mode 100644 index 00000000..506ba61a --- /dev/null +++ b/run/socket.go @@ -0,0 +1,82 @@ +package run + +import ( + "fmt" + "log/slog" + "net" + "os" +) + +// boundSocket owns the gateway's listening socket for the lifetime of the +// process so that a configuration reload never gives the port up. +// +// Each generation of the application is handed its own listener and closes it +// when it stops; the socket itself survives because boundSocket holds a +// duplicate descriptor and is the last to let go. Connections that arrive +// while one generation is draining and the next is starting therefore wait in +// the kernel's accept queue instead of being refused. +// +// Duplicating the descriptor is what keeps the socket alive, and not every +// platform and listener type supports it. Where it fails, the listener is +// served directly and a later generation rebinds the address instead, which +// does leave a gap where connections are refused. Windows never reaches that +// gap for a different reason: Go delivers no SIGHUP there, so nothing asks for +// a second generation in the first place. +type boundSocket struct { + address string + file *os.File + listener net.Listener +} + +// listenOn binds address and keeps hold of it for the process's lifetime. +func listenOn(address string) (*boundSocket, error) { + listener, err := net.Listen("tcp", address) + if err != nil { + return nil, err + } + + tcp, ok := listener.(*net.TCPListener) + if !ok { + return &boundSocket{address: address, listener: listener}, nil + } + file, err := tcp.File() + if err != nil { + slog.Debug("listening socket cannot be duplicated; a reload will rebind the address", + "address", address, "error", err) + return &boundSocket{address: address, listener: listener}, nil + } + // The socket stays open through the duplicate held in file. + _ = listener.Close() + return &boundSocket{address: address, file: file}, nil +} + +// next returns the listener for the next application generation. +func (s *boundSocket) next() (net.Listener, error) { + if s.file != nil { + listener, err := net.FileListener(s.file) + if err != nil { + return nil, fmt.Errorf("reuse listening socket on %s: %w", s.address, err) + } + return listener, nil + } + if s.listener != nil { + listener := s.listener + s.listener = nil + return listener, nil + } + return net.Listen("tcp", s.address) +} + +// Close releases the socket for good, which is the process exiting rather than +// a generation ending. +func (s *boundSocket) Close() error { + if s.listener != nil { + defer func() { s.listener = nil }() + return s.listener.Close() + } + if s.file != nil { + defer func() { s.file = nil }() + return s.file.Close() + } + return nil +}