feat: register Trino cells and select initial warehouse placement - #1169
Conversation
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: neutral or increased No coverage-reduction warnings detected. |
bill-ph
left a comment
There was a problem hiding this comment.
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
bill-ph
left a comment
There was a problem hiding this comment.
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
bill-ph
left a comment
There was a problem hiding this comment.
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
Summary
Add opt-in Trino cell registration and initial warehouse placement while preserving the existing legacy deployment and assignments.
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.mdfor configuration, recovery, and rollout dependencies.The tests verify application authentication and OPA authorization isolation, not network isolation.
Validation
just test-controlplane-k8spassed.just test-trinoand additional cell-isolation / stopped-to-running hydration tests passed.just test-trino-adminpassed, including real PostgreSQL concurrent selection versus enablement/claim tests.just lintpassed with zero issues.Real-stack acceptance
Validated commit
054d64fa92b9cedb917a5d0bdad6787dd5d709a7with 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. Seetests/mw-dev/README.mdfor 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.