feat: fail-fast preflight for schema upgrade path (multi-minor jumps) - #444
Conversation
documentdb-triage-tool
Bot
added
enhancement
|
🤖 Auto-triaged by documentdb-triage-tool. Applied: Reasoningcomponent from path globs (controllers, test); effort from diff stats (245+17 LOC, 2 files); LLM: Adds a fail-fast preflight check in the reconcile loop to detect broken schema upgrade chains before executing ALTER EXTENSION, preventing repeated reconcile failures with no actionable signal — a meaningful functional improvement confined to the controller/extension upgrade logic. If a label is wrong, remove it manually and ping |
Before running ALTER EXTENSION documentdb UPDATE, query pg_extension_update_paths to confirm PostgreSQL can resolve a chain of update scripts from the installed schema to the target. The documentdb extension ships minors frequently, so a user may bump more than one minor in a single change; that only succeeds when the extension packages the intermediate documentdb--A--B.sql scripts. When the chain is broken, ALTER EXTENSION UPDATE would otherwise fail every reconcile with a raw "no update path" error. The preflight detects the missing path up front, skips the ALTER, and emits a SchemaUpgradePathMissing Warning event (mirroring the existing ExtensionRollback precedent) so the schema and data are left untouched and the operator surfaces an actionable message instead of looping. Adds unit coverage for the no-path skip case and for extensionUpdatePathExists (source==target short-circuit, has-path, no-path, and SQL error propagation). Existing ALTER-path mocks are converted from order-based to content-based branching to account for the extra preflight query. Refs: documentdb#439 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d53b8fec-0585-4888-89da-d9a6847597a5 Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
6302edf to
1bc67c1
Compare
There was a problem hiding this comment.
Pull request overview
Adds a controller-side preflight check to detect missing PostgreSQL extension update chains before attempting ALTER EXTENSION documentdb UPDATE, avoiding repeated reconcile failures and surfacing a targeted Warning event when the upgrade path is not resolvable.
Changes:
- Introduce
extensionUpdatePathExiststo querypg_extension_update_paths('documentdb')and determine whether an update-script chain exists for the installed → target schema versions. - Gate
ALTER EXTENSIONexecution on the preflight result; when missing, skip schema changes and emit aSchemaUpgradePathMissingWarning event. - Update existing unit tests to account for the new preflight query and add new unit coverage for both the helper and the “no update path” behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| operator/src/internal/controller/documentdb_controller.go | Adds the update-path preflight helper and uses it to fail-fast (skip ALTER + emit Warning event) when no extension upgrade chain exists. |
| operator/src/internal/controller/documentdb_controller_test.go | Updates mocks to be content-based (to tolerate the extra preflight query) and adds tests covering path/no-path + helper behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| updatedDB := &dbpreview.DocumentDB{} | ||
| Expect(fakeClient.Get(ctx, types.NamespacedName{Name: "test-documentdb", Namespace: clusterNamespace}, updatedDB)).To(Succeed()) | ||
| Expect(updatedDB.Status.SchemaVersion).ToNot(Equal("0.110.0")) | ||
| }) |
Summary
Adds an operator-side fail-fast update-path preflight before running
ALTER EXTENSION documentdb UPDATE, so a schema upgrade that PostgreSQL cannot resolve is detected up front instead of failing every reconcile.Follow-up to #426 / part of #439 (the "skip more than one minor" / multi-version-chain gap).
Motivation
The
documentdbextension ships minors frequently (roughly every couple of weeks), so users may bump more than one minor in a single change.ALTER EXTENSION ... UPDATEcan chain intermediatedocumentdb--A--B.sqlscripts, but only when the extension actually packages that continuous chain. When the chain is broken,ALTER EXTENSION UPDATEfails at execution time with a rawno update patherror on every reconcile, with no actionable signal.What this does
ALTER EXTENSION, querypg_extension_update_paths('documentdb')for a non-NULL path from the installed schema to the target (same version graphALTER EXTENSION UPDATEwalks internally).SchemaUpgradePathMissingWarning event (mirroring the existingExtensionRollbackprecedent).source == target) short-circuits to "path exists" without querying.auto/ explicit version); two-phase mode returns early before it, so those paths are unaffected.Tests
extensionUpdatePathExistsunit tests:source==targetshort-circuit, has-path, no-path, and SQL error propagation.go test ./internal/controller/...green (163 specs).Design note / open question
The status API has no
Conditionsfield today, so this surfaces the failure via a Warning event only (consistent withExtensionRollback). Adding a status condition would need a CRD/deepcopy regen — flagging as a possible follow-up. See the design comment on #439; pending maintainer sign-off on the fail-fast contract and webhook-vs-controller placement.Follow-ups (not in this PR)
Refs: #439