Skip to content

feat: register Trino cells and select initial warehouse placement - #1169

Merged
benben merged 8 commits into
mainfrom
feat/trino-multicell-runtime
Sep 11, 2026
Merged

feat: register Trino cells and select initial warehouse placement#1169
benben merged 8 commits into
mainfrom
feat/trino-multicell-runtime

Conversation

@benben

@benben benben commented Sep 11, 2026

Copy link
Copy Markdown
Member

Summary

Add opt-in Trino cell registration and initial warehouse placement while preserving the existing legacy deployment and assignments.

  • Load additional logical cells from a mounted JSON registry; validate unique ownership, namespaces, endpoints, and blue/green configuration before bootstrap.
  • Reconcile cell-local authentication, OPA bundles, tenant credentials, and independent catalogs on running backends. Stopped backends retain supporting projections without blocking readiness.
  • Add admin-only initial cell selection, cell-aware console views, and durable assignment that survives disable/re-enable. No schema migration is required.
  • Fix a pre-existing assignment race: a losing conditional claim must not project the tenant into the losing cell.
  • Keep per-cell bundle credentials isolated and collect usage from each running backend using the observer identity.

Safety and scope

Existing stored cell IDs and default legacy placement are unchanged. Registered logical IDs use a separate reserved storage prefix. Already assigned warehouses cannot be moved through the initial-selection endpoint, including while disabled.

This does not implement Gateway assignment lookup, deployment cutovers, or existing-warehouse migration. A maintenance move requires an enforced source admission barrier and verified drain; destination readiness alone is not a safe assignment switch. See docs/trino-cells.md for configuration, recovery, and rollout dependencies.

The tests verify application authentication and OPA authorization isolation, not network isolation.

Validation

  • Required TDD: backend and registry tests failed before implementation; the lost-claim regression reproduced the incorrect tenant projection before the fix.
  • just test-controlplane-k8s passed.
  • just test-trino and additional cell-isolation / stopped-to-running hydration tests passed.
  • just test-trino-admin passed, including real PostgreSQL concurrent selection versus enablement/claim tests.
  • Admin UI: 192 tests, TypeScript check, and production build passed.
  • just lint passed with zero issues.
  • Separate adversarial review found and verified fixes for assignment, namespace isolation, readiness aggregation, and timeout boundaries; independent focused tests passed.

Real-stack acceptance

Validated commit 054d64fa92b9cedb917a5d0bdad6787dd5d709a7 with all required checks green, including the Trino acceptance tests. The tests verify initial assignment, blue queries with green stopped, cross-cell authentication and OPA rejection, and independent green catalog hydration while blue and legacy remain queryable. Both colors return the expected [[2,18]] fixture result. See tests/mw-dev/README.md for the test scope and limitations.

Legacy compatibility, neutral, DuckDB, and reshard tests passed. All test cleanup jobs passed. Independent adversarial review and local fixture tests also passed. No production deployment or existing-warehouse migration is included.

@benben
benben requested a review from a team September 11, 2026 12:28
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown

Test Impact Plan

Deterministic summary of how this PR changes tests, CI runners, and coverage-risk signals.

Summary

Area Added Changed Deleted
Test files 9 7 0
E2E/journey files 0 0 0
Workflow files 0 0 0

Signals

  • Test cases: +26 / -0
  • Assertions: +174 / -2
  • Skips or known failures added: 0
  • Workflow continue-on-error added: 0
  • Workflow path filters added: 0
  • Test commands removed from justfile: 0
  • E2E/journey retry lines added: 0

Coverage risk: neutral or increased

No coverage-reduction warnings detected.

@bill-ph bill-ph left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the current draft head. The cell registry validation, immutable initial assignment, lost-claim handling, per-backend reconciliation timeouts, and admin/UI cell scoping are covered by focused tests and the documented safety constraints. I found no P0 blocker requiring a code change. The PR description appropriately keeps the draft gate until the isolated real-stack additional-cell/blue-green hydration test is complete; remaining CI and that validation are still required before merge.

— Robo Bill

@benben
benben requested a review from a team September 11, 2026 12:41

@bill-ph bill-ph left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the updated head, including the typed-nil Trino provisioner guard, the reconciler wiring split, sibling-cell cancellation coverage, and the expanded isolated multicell E2E harness/cleanup paths. The changes remain scoped to the managed-warehouse cell rollout and add safeguards around numeric PR identities, namespace ownership, bounded HTTP calls, and independent blue/green hydration. I found no P0 blocker requiring a code change. The PR is still correctly kept as draft pending the isolated real-stack validation; remaining CI and that validation are still required before merge.\n\n— Robo Bill

@benben
benben requested a review from a team September 11, 2026 13:15

@bill-ph bill-ph left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the latest head, including the per-run config-store credential generation/Secret wiring, credential-leak regression coverage, and the explicit preservation checks for baseline network-policy and cluster-privilege posture. I found no P0 blocker requiring a code change. The PR remains correctly kept as draft pending the isolated real-stack validation; current CI is still in progress.\n\n— Robo Bill

@benben
benben marked this pull request as ready for review September 11, 2026 13:30
@benben
benben requested a review from a team September 11, 2026 13:38
@benben
benben merged commit efc8686 into main Sep 11, 2026
36 checks passed
@benben
benben deleted the feat/trino-multicell-runtime branch September 11, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants