diff --git a/.gitignore b/.gitignore index 2d3ef47..4061d3a 100644 --- a/.gitignore +++ b/.gitignore @@ -4,5 +4,8 @@ tools/yaegi-check/llmgw-yaegi-check dist/ # Rendered from integration/traefik/dynamic.yml.tmpl by `make integration-up` -# (INTEGRATION_REAL_BASEURL substitution) — generated, not source. -integration/traefik/dynamic.yml +# (INTEGRATION_REAL_BASEURL substitution) — generated, not source. A whole +# directory (not a single file): Traefik's directory-based file provider +# watches integration/traefik/dynamic/, matching the k8s-ConfigMap-mount +# pattern (see traefik.yml's own comment). +integration/traefik/dynamic/ diff --git a/Makefile b/Makefile index a35d876..a7e1050 100644 --- a/Makefile +++ b/Makefile @@ -28,12 +28,15 @@ INTEGRATION_ALICE_KEY := sk-int-alice # INTEGRATION_REAL=1) never hardcodes one operator's personal hostname as # the only option: `INTEGRATION_REAL_BASEURL=https://other.example make # integration` points it elsewhere. Rendered into -# integration/traefik/dynamic.yml (git-ignored) from the checked-in -# dynamic.yml.tmpl by integration-up, below. +# integration/traefik/dynamic/dynamic.yml (git-ignored directory) from the +# checked-in dynamic.yml.tmpl by integration-up, below. Traefik's file +# provider watches that whole directory (traefik.yml's providers.file.directory), +# not the single rendered file — see traefik.yml's own comment for why. INTEGRATION_REAL_BASEURL ?= https://llmgw.example.com integration-up: - sed 's#__INTEGRATION_REAL_BASEURL__#$(INTEGRATION_REAL_BASEURL)#' integration/traefik/dynamic.yml.tmpl > integration/traefik/dynamic.yml + mkdir -p integration/traefik/dynamic + sed 's#__INTEGRATION_REAL_BASEURL__#$(INTEGRATION_REAL_BASEURL)#' integration/traefik/dynamic.yml.tmpl > integration/traefik/dynamic/dynamic.yml docker compose -f $(INTEGRATION_COMPOSE) up -d --build integration-wait: diff --git a/integration/config_hot_reload_test.go b/integration/config_hot_reload_test.go new file mode 100644 index 0000000..7af8fb3 --- /dev/null +++ b/integration/config_hot_reload_test.go @@ -0,0 +1,267 @@ +//go:build integration + +package integration + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" +) + +// dynamicConfigPath returns the path (relative to this package's directory, +// also `go test`'s working directory) to the rendered dynamic config +// Traefik's directory-based file provider watches — the whole directory +// docker-compose.yml bind-mounts into both traefik1 and traefik2 at +// /etc/traefik/dynamic (traefik.yml's providers.file.directory), rendered +// from dynamic.yml.tmpl by `make integration-up`/`integration-keep`. +func dynamicConfigPath() string { + return filepath.Join("traefik", "dynamic", "dynamic.yml") +} + +// runChatLoop issues a POST /v1/chat/completions against baseURL for model +// every interval, from goroutine start until stop is closed, then closes +// done. Every response whose status is not 200, and every request-level +// error, is appended to *failures under mu's protection. +// +// This never calls any *testing.T method: it runs on its own goroutine, and +// testing.T.Fatal/FailNow "must be called from the goroutine running the +// test... not from other goroutines created during the test" (testing +// package doc) — a violation silently corrupts the test result rather than +// failing loudly. Every outcome is instead recorded into failures and +// asserted from the test's own goroutine after the loop stops. +func runChatLoop(baseURL, apiKey, model string, interval time.Duration, stop <-chan struct{}, done chan<- struct{}, mu *sync.Mutex, failures *[]string) { + defer close(done) + + reqBody, err := json.Marshal(map[string]any{ + "model": model, + "messages": []map[string]any{{"role": "user", "content": "hot-reload-probe"}}, + }) + if err != nil { + mu.Lock() + *failures = append(*failures, fmt.Sprintf("marshal probe request body: %v", err)) + mu.Unlock() + return + } + + client := &http.Client{Timeout: 5 * time.Second} + ticker := time.NewTicker(interval) + defer ticker.Stop() + + for { + select { + case <-stop: + return + case <-ticker.C: + req, err := http.NewRequest(http.MethodPost, baseURL+"/v1/chat/completions", bytes.NewReader(reqBody)) + if err != nil { + mu.Lock() + *failures = append(*failures, fmt.Sprintf("build probe request: %v", err)) + mu.Unlock() + continue + } + req.Header.Set("Authorization", "Bearer "+apiKey) + req.Header.Set("Content-Type", "application/json") + + resp, err := client.Do(req) + if err != nil { + mu.Lock() + *failures = append(*failures, fmt.Sprintf("probe request error: %v", err)) + mu.Unlock() + continue + } + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + mu.Lock() + *failures = append(*failures, fmt.Sprintf("probe request: status %d", resp.StatusCode)) + mu.Unlock() + } + } + } +} + +// waitForModelAliasBounded polls GET /v1/models on baseURL with apiKey +// until an entry with id wantID appears, returning true — or false once +// timeout elapses without ever seeing it. Unlike waitForModelAlias +// (integration_test.go), a timeout here is not itself a test failure: +// TestConfigHotReload uses the bool to choose between asserting success and +// t.Skipf-ing a known, host-dependent limitation (the directory watch not +// firing under this host's Docker Desktop VirtioFS). +func waitForModelAliasBounded(t *testing.T, baseURL, apiKey, wantID string, timeout time.Duration) bool { + t.Helper() + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + resp, body := doJSON(t, http.MethodGet, baseURL+"/v1/models", apiKey, nil) + if resp.StatusCode == http.StatusOK { + if data, ok := body["data"].([]any); ok { + for _, entry := range data { + if m, ok := entry.(map[string]any); ok && m["id"] == wantID { + return true + } + } + } + } + time.Sleep(500 * time.Millisecond) + } + return false +} + +// TestConfigHotReload is item A's proof-and-parity test: Traefik's +// directory-based file provider (traefik.yml's providers.file.directory, +// switched from a single-file .filename specifically because a single +// file's fsnotify watch never fires under Docker Desktop's VirtioFS bind +// mount) must pick up a config change with no blip in service, and — when +// it does — the newly added config must actually be live. +// +// Sequence: +// 1. Baseline: an existing model answers 200. +// 2. A sustained ~20rps request loop against that same model starts, +// recording every non-200 (runChatLoop above). +// 3. While the loop runs, the rendered dynamic config is rewritten to add +// a new model alias ("aliased/hot": "gpt-mock"), written to a temp +// file in the same directory then os.Rename'd over the target — an +// atomic swap mirroring a Kubernetes ConfigMap mount's symlink swap, +// never an in-place truncate+rewrite a watcher could observe +// half-written. +// 4. GET /v1/models is polled (bounded 30s) for "aliased/hot" to appear — +// this is the directory watch actually firing. +// 5. The loop stops. The no-blip property — zero non-200s across the +// entire rewrite/propagation window — is asserted unconditionally, +// whether or not the watch fired within 30s: a slow or absent watch is +// a config-propagation limitation, not a service disruption, and the +// two must never be conflated. +// 6. Only if the watch fired: a chat request through the newly live +// "aliased/hot" alias must succeed (200) — proving the config that +// actually landed is usable, not just present in the listing. +// +// t.Cleanup restores the original rendered config content unconditionally, +// including on a Skip or a failure partway through. +func TestConfigHotReload(t *testing.T) { + baseReq := map[string]any{ + "model": "openai/gpt-mock", + "messages": []map[string]any{{"role": "user", "content": "baseline"}}, + } + resp, body := doJSON(t, http.MethodPost, traefik1URL+"/v1/chat/completions", aliceKey, baseReq) + if resp.StatusCode != http.StatusOK { + t.Fatalf("baseline chat: status = %d, body=%#v", resp.StatusCode, body) + } + + configPath := dynamicConfigPath() + original, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("read rendered dynamic config %q: %v", configPath, err) + } + // The restore is written the same atomic way as the mutation below (temp + // file in the same directory, then os.Rename over the target), and + // t.Cleanup then WAITS (bounded 30s) for "aliased/hot" to actually + // disappear from a live /v1/models before returning. This matters + // because VirtioFS propagation latency is highly variable (this test's + // own poll below observed anywhere from under a second to over 30s) — + // without waiting, this cleanup's own restore can trigger a Traefik + // middleware rebuild that lands arbitrarily late, during a LATER test's + // execution. That was reproduced empirically: it raced + // TestUsersFileHotReload's non-atomic os.WriteFile to users.json + // (integration_test.go), the rebuild read a torn/empty file mid-write, + // and plugin construction failed outright — removing the router + // entirely (global 404s) until the next unrelated config change + // happened to trigger a successful rebuild. Waiting here for this + // test's OWN change to fully settle before it hands control back + // removes that window for every test that runs after it. + t.Cleanup(func() { + tmpPath := configPath + ".restore.tmp" + if err := os.WriteFile(tmpPath, original, 0o644); err != nil { + t.Errorf("write restore temp config %q: %v", tmpPath, err) + return + } + if err := os.Rename(tmpPath, configPath); err != nil { + t.Errorf("rename restore temp config over %q: %v", configPath, err) + return + } + deadline := time.Now().Add(30 * time.Second) + for time.Now().Before(deadline) { + resp, body := doJSON(t, http.MethodGet, traefik1URL+"/v1/models", aliceKey, nil) + if resp.StatusCode == http.StatusOK { + stillPresent := false + if data, ok := body["data"].([]any); ok { + for _, entry := range data { + if m, ok := entry.(map[string]any); ok && m["id"] == "aliased/hot" { + stillPresent = true + break + } + } + } + if !stillPresent { + return + } + } + time.Sleep(500 * time.Millisecond) + } + t.Logf("cleanup: %q still visible in /v1/models (or /v1/models still unhealthy) 30s after restoring the config — a later test may race a still-pending rebuild", "aliased/hot") + }) + + // Sustained request loop: started before the config mutation below and + // stopped only after the poll resolves, so it spans the entire + // rewrite/propagation window, not just the moment of the rename. + stop := make(chan struct{}) + done := make(chan struct{}) + var mu sync.Mutex + var failures []string + go runChatLoop(traefik1URL, aliceKey, "openai/gpt-mock", 50*time.Millisecond, stop, done, &mu, &failures) + + const anchor = `aliased/claude: "claude-mock"` + if !strings.Contains(string(original), anchor) { + close(stop) + <-done + t.Fatalf("rendered dynamic config does not contain the expected modelAliases anchor %q — dynamic.yml.tmpl may have changed; update this test's injection point", anchor) + } + updated := strings.Replace(string(original), anchor, anchor+"\n aliased/hot: \"gpt-mock\"", 1) + + tmpPath := configPath + ".tmp" + if err := os.WriteFile(tmpPath, []byte(updated), 0o644); err != nil { + close(stop) + <-done + t.Fatalf("write temp config %q: %v", tmpPath, err) + } + if err := os.Rename(tmpPath, configPath); err != nil { + close(stop) + <-done + t.Fatalf("rename %q over %q: %v", tmpPath, configPath, err) + } + + fired := waitForModelAliasBounded(t, traefik1URL, aliceKey, "aliased/hot", 30*time.Second) + + close(stop) + <-done + + mu.Lock() + failCount := len(failures) + sample := append([]string(nil), failures...) + mu.Unlock() + if failCount != 0 { + t.Errorf("sustained request loop saw %d non-200/error result(s) during the config rewrite window, want 0: %v", failCount, sample) + } + + if !fired { + t.Skipf("directory-based file provider watch did not surface the new alias within 30s on this host (a VirtioFS/Docker Desktop fsnotify propagation limitation, not a plugin defect) — the no-blip property above still held (0 failures) regardless") + } + + hotReq := map[string]any{ + "model": "aliased/hot", + "messages": []map[string]any{{"role": "user", "content": "hi"}}, + } + hotResp, hotBody := doJSON(t, http.MethodPost, traefik1URL+"/v1/chat/completions", aliceKey, hotReq) + if hotResp.StatusCode != http.StatusOK { + t.Errorf("chat via the newly hot-reloaded alias: status = %d, body=%#v", hotResp.StatusCode, hotBody) + } + if content := firstChoiceContent(t, hotBody); content != "mock openai response" { + t.Errorf("chat via the newly hot-reloaded alias: content = %q, want %q", content, "mock openai response") + } +} diff --git a/integration/docker-compose.yml b/integration/docker-compose.yml index d62226d..c64e650 100644 --- a/integration/docker-compose.yml +++ b/integration/docker-compose.yml @@ -69,7 +69,7 @@ services: volumes: - ../:/plugins-local/src/github.com/lukaszraczylo/traefik-llmgateway:ro - ./traefik/traefik.yml:/etc/traefik/traefik.yml:ro - - ./traefik/dynamic.yml:/etc/traefik/dynamic.yml:ro + - ./traefik/dynamic:/etc/traefik/dynamic:ro - ./users:/users:ro depends_on: redis: @@ -92,7 +92,7 @@ services: volumes: - ../:/plugins-local/src/github.com/lukaszraczylo/traefik-llmgateway:ro - ./traefik/traefik.yml:/etc/traefik/traefik.yml:ro - - ./traefik/dynamic.yml:/etc/traefik/dynamic.yml:ro + - ./traefik/dynamic:/etc/traefik/dynamic:ro - ./users:/users:ro depends_on: redis: diff --git a/integration/traefik/traefik.yml b/integration/traefik/traefik.yml index 483f6c2..9364bfd 100644 --- a/integration/traefik/traefik.yml +++ b/integration/traefik/traefik.yml @@ -7,9 +7,17 @@ experimental: llmgateway: moduleName: github.com/lukaszraczylo/traefik-llmgateway +# providers.file.directory, not .filename: a single-file watch's fsnotify +# never fires under Docker Desktop's VirtioFS bind mount (empirically +# confirmed against this exact stack) — Traefik's directory-based watch +# does, and it is also the correct pattern for a Kubernetes ConfigMap mount +# (a ConfigMap update swaps the mounted directory's symlink target, which a +# single-file watch on the file itself never observes either). See the +# integration/docker-compose.yml volumes and Makefile's integration-up +# render target, which mount/render into traefik/dynamic/ accordingly. providers: file: - filename: /etc/traefik/dynamic.yml + directory: /etc/traefik/dynamic watch: true log: