diff --git a/controlplane/trino_rollout_config.go b/controlplane/trino_rollout_config.go index de9a6c9a..366d7806 100644 --- a/controlplane/trino_rollout_config.go +++ b/controlplane/trino_rollout_config.go @@ -27,6 +27,24 @@ func buildTrinoRolloutReadiness(fleet trinoFleet, store rolloutCanaryStore) (*tr if tokenPath == "" && canaryPath == "" { return nil, nil } + // Pool Gateway authentication reuses the token without enabling fixed-slot readiness. + // An explicit canary file still requests validation, even for a pool-only fleet. + var fixed trinoFleet + hasPool := false + for _, wire := range fleet { + if wire.Cell.PublicID == "" { + continue + } + if wire.Cell.Mode == trinoPoolModeShared { + hasPool = true + continue + } + fixed = append(fixed, wire) + } + if hasPool && len(fixed) == 0 && canaryPath == "" { + return nil, nil + } + fleet = fixed if tokenPath == "" || canaryPath == "" || len(fleet) == 0 { return nil, errors.New("rollout readiness requires token, canaries, and registered cells") } diff --git a/controlplane/trino_rollout_config_test.go b/controlplane/trino_rollout_config_test.go index 6f68af3d..8acef326 100644 --- a/controlplane/trino_rollout_config_test.go +++ b/controlplane/trino_rollout_config_test.go @@ -72,3 +72,73 @@ func TestTrinoRolloutReadinessConfigurationExplicitAndPrivate(t *testing.T) { } } } + +func TestTrinoRolloutReadinessScopesFixedCells(t *testing.T) { + dir := t.TempDir() + tokenPath := filepath.Join(dir, "token") + if err := os.WriteFile(tokenPath, []byte(strings.Repeat("x", 48)), 0600); err != nil { + t.Fatal(err) + } + legacy := &trinoWiring{Cell: trinoCell{ID: "legacy"}} + pool := &trinoWiring{Cell: trinoCell{PublicID: "pool-test", Mode: trinoPoolModeShared}} + fixed := &trinoWiring{ + Cell: trinoCell{PublicID: "fixed-test", RoutingGroup: "group-test", Namespace: "test", + Backends: []trinoRegisteredBackend{{ID: "blue", CoordinatorURL: "https://blue.example.test"}, {ID: "green", CoordinatorURL: "https://green.example.test"}}}, + Kubernetes: fake.NewSimpleClientset(), + } + validCanary := `{"canaries":[{"cell":"fixed-test","orgID":"canary-org","principal":"canary-test","password":"test-password"}]}` + for _, tc := range []struct { + name string + fleet trinoFleet + canaries string + noToken bool + wantHandler bool + wantError bool + }{ + {name: "pool token does not enable fixed readiness", fleet: trinoFleet{pool}}, + {name: "pool with legacy single", fleet: trinoFleet{legacy, pool}}, + {name: "pool without either readiness setting", fleet: trinoFleet{pool}, noToken: true}, + {name: "token without any registered cells", wantError: true}, + {name: "legacy single token is incomplete", fleet: trinoFleet{legacy}, wantError: true}, + {name: "fixed token is incomplete", fleet: trinoFleet{fixed}, wantError: true}, + {name: "mixed token is incomplete", fleet: trinoFleet{pool, fixed}, wantError: true}, + {name: "mixed fixed without backends rejected", fleet: trinoFleet{pool, &trinoWiring{Cell: trinoCell{PublicID: "empty-test", Mode: "fixed"}}}, wantError: true}, + {name: "mixed unknown mode rejected", fleet: trinoFleet{pool, &trinoWiring{Cell: trinoCell{PublicID: "unknown-test", Mode: "unknown"}}}, wantError: true}, + {name: "fixed valid", fleet: trinoFleet{fixed}, canaries: validCanary, wantHandler: true}, + {name: "mixed valid", fleet: trinoFleet{legacy, pool, fixed}, canaries: validCanary, wantHandler: true}, + {name: "explicit pool canary rejected", fleet: trinoFleet{pool}, canaries: `{"canaries":[]}`, wantError: true}, + {name: "mixed malformed canary rejected", fleet: trinoFleet{pool, fixed}, canaries: `{`, wantError: true}, + {name: "mixed canary without token rejected", fleet: trinoFleet{pool, fixed}, canaries: validCanary, noToken: true, wantError: true}, + {name: "mixed extra pool canary rejected", fleet: trinoFleet{pool, fixed}, + canaries: `{"canaries":[{"cell":"fixed-test","orgID":"canary-org","principal":"canary-test","password":"test-password"},{"cell":"pool-test","orgID":"pool-org","principal":"pool-canary","password":"test-password"}]}`, wantError: true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("DUCKGRES_TRINO_ROLLOUT_TOKEN_FILE", tokenPath) + if tc.noToken { + t.Setenv("DUCKGRES_TRINO_ROLLOUT_TOKEN_FILE", "") + } + t.Setenv("DUCKGRES_TRINO_ROLLOUT_CANARIES_FILE", "") + if tc.canaries != "" { + path := filepath.Join(t.TempDir(), "canaries.json") + if err := os.WriteFile(path, []byte(tc.canaries), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("DUCKGRES_TRINO_ROLLOUT_CANARIES_FILE", path) + } + handler, err := buildTrinoRolloutReadiness(tc.fleet, rolloutEligibilityStub{}) + if (err != nil) != tc.wantError || (handler != nil) != tc.wantHandler { + t.Fatalf("handler=%v error=%v; want handler=%v error=%v", handler != nil, err, tc.wantHandler, tc.wantError) + } + if handler != nil { + if len(handler.slots) != 2 { + t.Fatalf("fixed readiness includes unexpected slots: %d", len(handler.slots)) + } + for _, slot := range handler.slots { + if slot.cell != fixed.Cell.PublicID { + t.Fatalf("fixed readiness includes cell %q", slot.cell) + } + } + } + }) + } +} diff --git a/docs/runbooks/trino-rollout-readiness.md b/docs/runbooks/trino-rollout-readiness.md index 3fe89505..d1b85e7f 100644 --- a/docs/runbooks/trino-rollout-readiness.md +++ b/docs/runbooks/trino-rollout-readiness.md @@ -13,7 +13,15 @@ provisioning state, or Gateway routes. Both variables are empty by default; the endpoint is then not registered. Supplying only one variable, invalid content, or incomplete cell configuration -fails startup. +fails startup for fixed-cell readiness. + +Shared-pool Gateway authentication reuses `DUCKGRES_TRINO_ROLLOUT_TOKEN_FILE`, +but this does not enable the fixed-slot endpoint. When all registered cells use +`mode: shared-pool` and no canary file is configured, the endpoint stays disabled; +the legacy single coordinator can coexist with those pools. The pool Gateway +client still validates its token independently. A mixed registry requires canaries +only for its fixed cells. Explicit canary configuration remains fail-closed, and +a canary entry for a shared-pool cell is rejected. | Variable | Content | | --- | --- | diff --git a/tests/mw-dev/README.md b/tests/mw-dev/README.md index db00c263..ed5450eb 100644 --- a/tests/mw-dev/README.md +++ b/tests/mw-dev/README.md @@ -884,6 +884,24 @@ IDs, TLS service names, auth projections, and cache-manager configuration. Run namespace before retrying. Namespace teardown removes both clusters, cached catalog rows in the throwaway database, and all ephemeral cache volumes. +### Shared-pool startup acceptance + +The existing `trino_shared_pool_active` assertion in `e2e/harness.sh` requires +`E2E_TRINO_POOL=1` and a separately configured shared-pool deployment. Its +structure stage waits for ready, independent coordinator instances; subsequent +opt-in stages check warehouse admission and a real query with an existing login. +Run it after rolling a candidate control-plane image with a shared-pool registry, +the Gateway token file, and no `DUCKGRES_TRINO_ROLLOUT_CANARIES_FILE`. Reusing +the token must not activate the obsolete fixed-slot canary endpoint or crash +control-plane startup. + +The default in-Job fixture does not configure a shared pool, and its harness runs +only after the control plane starts. It cannot reproduce this startup failure by +changing its own environment: the control plane reads these settings at process +startup. `TestTrinoRolloutReadinessScopesFixedCells` covers the startup selection +and malformed fixed/mixed configurations locally. The active-pool harness remains +the real-cluster acceptance check; passing unit tests alone does not prove it ran. + ### Optional shared catalog rollout lane `TRINO_SHARED_CATALOGS_ENABLED=true` adds an isolated, real Gateway to the