test(e2e): schema-upgrade coverage — auto, rollback, HA, unpullable version - #443
Conversation
Covers two of the gaps tracked in documentdb#439 that PR documentdb#426 deferred: - schemaVersion: "auto" single-step path — a single spec.documentDBVersion bump migrates both the binary and the extension schema in one step (operator runs ALTER EXTENSION automatically, no finalize patch). - Rollback within the safe window — in two-phase mode the installed schema stays at the old version, so reverting spec.documentDBVersion before finalize is admitted by the image-rollback webhook; asserts data and schema integrity across the down-hop. Adds a reusable schema_version mixin so specs can set spec.schemaVersion at creation. Both specs reuse the existing upgrade helpers and the E2E_UPGRADE_OLD/NEW_DOCUMENTDB_VERSION env pair. 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>
|
🤖 Auto-triaged by documentdb-triage-tool. Applied: Reasoningcomponent from path globs (test); effort from diff stats (373+0 LOC, 3 files); LLM: Adds new e2e test specs covering schema-upgrade auto-mode and safe-window rollback paths tracked in an existing coverage gap issue, with a reusable mixin; purely test-layer changes across a couple of files. If a label is wrong, remove it manually and ping |
Covers two more of the gaps tracked in documentdb#439: - Multi-instance / HA rollout — new upgrade_schema_ha_test.go runs the two-phase schema upgrade on a 3-instance cluster, asserts the cluster returns to 3 ready instances after both the binary roll and the schema finalize, and execs psql on a replica pod to confirm the migrated extension version propagated via WAL streaming replication (the operator computes status.schemaVersion from the primary only, so the replica is verified directly). - Invalid / unpullable documentDBVersion — added as a second ordered spec in upgrade_schema_test.go that reuses the existing cluster (no extra DocumentDB). It patches to a version above the installed schema (0.999.0, so the image-rollback webhook admits it but no image exists), asserts the schema does not advance and seeded data stays readable while pods can't pull, then rolls back to the valid version and asserts recovery. Adds a replicaInstalledSchemaVersion helper (reads pg_extension.extversion on a replica via CNPG EventuallyExecCommand) and hoists the shared cluster creation into BeforeAll so both ordered specs share one cluster. 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>
The HA schema-upgrade spec's replica check only exec'd psql on the first replica pod, so a lagging or not-yet-rolled second replica could pass unnoticed. replicaInstalledSchemaVersion now takes an expected replica count, requires exactly that many replica pods to be present, reads pg_extension.extversion on every one, and only returns a version when all replicas agree (erroring otherwise so Eventually keeps polling until the whole cluster converges). 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>
There was a problem hiding this comment.
Pull request overview
Expands the e2e upgrade test suite to cover additional DocumentDB schema-upgrade scenarios (auto mode, rollback window, HA behavior, and an unpullable version failure path), and adds supporting helpers/manifests to drive spec.schemaVersion at creation time and to verify replica schema convergence via psql.
Changes:
- Add new upgrade e2e specs covering:
schemaVersion: auto, rollback before finalize, HA (3 instances) migration, and unpullabledocumentDBVersionrecovery. - Add a
schema_versionmanifest mixin to setspec.schemaVersionduring initial resource creation. - Add a helper to read
pg_extension.extversionon replica pods to assert schema propagation via WAL streaming replication.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/tests/upgrade/upgrade_schema_test.go | Shares a single cluster across ordered specs and adds an unpullable-version recovery test. |
| test/e2e/tests/upgrade/upgrade_schema_auto_test.go | New spec for single-step schema migration when schemaVersion is "auto". |
| test/e2e/tests/upgrade/upgrade_schema_rollback_test.go | New spec validating rollback-safe window before schema finalize. |
| test/e2e/tests/upgrade/upgrade_schema_ha_test.go | New HA spec validating 3-instance rollout and replica schema convergence. |
| test/e2e/tests/upgrade/helpers_test.go | Adds replica schema version helper using CNPG exec to query pg_extension. |
| test/e2e/manifests/mixins/schema_version.yaml.template | Adds mixin to set spec.schemaVersion via templated manifests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| BeforeAll(func() { | ||
| skipUnlessUpgradeEnabled() |
xgerman
left a comment
There was a problem hiding this comment.
Additional test coverage concerns
The new scenarios cover the primary happy paths well, but these gaps could leave false confidence:
-
Validate all installed DocumentDB extensions. The HA helper checks only
documentdb. A dependent extension could remain stale or broken while that check passes. Verify every installed DocumentDB-related extension on the primary and each replica. -
Add version-skew and patch-version cases. The tests cover a clean adjacent-version transition only. Please cover an installed schema newer than the requested target, patch versions without an exact migration path, and targets below the installed schema. The expected behavior should be explicit: safely skip/reject the change or recover without corrupting state.
-
Test migration failure and recovery. The PR currently defers
ALTER EXTENSIONfailure coverage. Add a failure-path test that verifies the error is surfaced in status/events, the cluster does not falsely report Ready, the recorded schema version remains truthful, and retry/rollback recovers safely. -
Test multi-version jumps. Verify that skipping multiple minor versions either follows a supported chain or fails fast without partially changing the binary/schema state.
-
Avoid masking status-read failures.
schemaVersionGetterreturns the last successful value when a status fetch fails. Consequently,Consistentlycan pass during a prolonged API/status outage. The assertion should surface fetch errors or track them separately. -
Strengthen the unpullable-image test. It currently verifies that data remains readable, but does not prove that the requested image was attempted and failed. Assert that the desired version is persisted, the workload targets it, pods expose the expected image-pull failure, and status reports the failure while the existing primary remains available.
-
Validate migrated objects, not only
extversion. Checking the catalog version does not prove that migration-created or altered functions, tables, indexes, permissions, triggers, and other objects work. Add representative post-migration operations for objects affected by the release. -
Exercise failover around schema migration. After migration, promote a replica and verify the promoted primary has the complete extension/object state and continues serving traffic. This would cover rollout/failover ordering that the current end-state checks do not exercise.
What
Adds e2e specs to the upgrade area covering the gaps tracked in #439 (deferred from #426).
Included in this PR
schemaVersion: "auto"single-step path (upgrade_schema_auto_test.go) — a singlespec.documentDBVersionbump migrates both the binary and the extension schema in one step (operator runsALTER EXTENSION documentdb UPDATEautomatically, no finalize patch). Assertsstatus.schemaVersionadvances on its own and data is retained.upgrade_schema_rollback_test.go) — in two-phase mode the installed schema stays at the old version, so revertingspec.documentDBVersionbefore finalize is admitted by the image-rollback webhook. Asserts the cluster returns to Ready on the old binary, the schema is still old, and data survives the down-hop.upgrade_schema_ha_test.go) — runs the two-phase upgrade on a 3-instance cluster, asserts the cluster returns to 3 ready instances after the binary roll and the schema finalize, and execs psql on a replica pod to confirmpg_extension.extversionconverged to the new version via WAL streaming replication (the operator computesstatus.schemaVersionfrom the primary only, so the replica is verified directly).documentDBVersion(2nd ordered spec inupgrade_schema_test.go) — reuses the existing cluster (no extra DocumentDB). Patches to a version above the installed schema (0.999.0, so the rollback webhook admits it but no image exists); asserts the schema does not advance and seeded data stays readable while pods can't pull, then rolls back and asserts recovery.Supporting changes:
schema_versionmixin so specs can setspec.schemaVersionat creation.replicaInstalledSchemaVersionhelper (reads the extension version on a replica via CNPGEventuallyExecCommand).upgrade_schema_test.gointoBeforeAllso the happy-path and unpullable-version specs share one cluster.Deferred to a follow-up PR (needs operator change)
Per the design discussion on #439, these pair with a proposed fail-fast update-path preflight in the operator and will land together:
ALTER EXTENSION UPDATEfails) — realistically triggered by a broken update-path, which the preflight introducesVerification
go build ./...,go vet,gofmt, and Ginkgo--dry-runall clean; all new specs discovered.Coverage tracked in #439
spec.schemaVersion: "auto"single-step pathdocumentDBVersionALTER EXTENSION UPDATEfails) — follow-up with preflightRefs #439.