From 9ffca01c85f9c9629e8cd88664d1be8690d1c40f Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 27 Aug 2026 09:39:49 -0400 Subject: [PATCH 1/3] feat: fail-fast preflight for extension update paths The operator upgrades the documentdb extension with a blanket `ALTER EXTENSION documentdb UPDATE`, with no check that PostgreSQL can actually resolve an update path. When the extension image is missing a `documentdb----.sql` script anywhere in the requested range, the ALTER fails at execution time with a raw PostgreSQL error that re-fires on every reconcile, leaving the user with no actionable signal. Add a read-only preflight against `pg_extension_update_paths` that runs immediately before the ALTER. When the absence of a path is positively proven, the operator skips the ALTER, emits a Warning event, and reports `SchemaUpgradeBlocked=True` / `NoUpdatePath` with a message naming both versions and the remediation. Any error, unparseable output, or unexpected version format fails open, preserving the pre-existing behavior of letting PostgreSQL decide. The query wraps the lookup in `COALESCE(..., 'NO_UPDATE_PATH')` so that both "no rows" (version not advertised) and "NULL path" (known but unreachable) collapse into a single, always-present row, which makes the psql output unambiguous to parse. Also adds `status.conditions` to the DocumentDB CRD, reconciled across every migration-planning path so a stale block cannot survive a user correcting their spec. Test coverage: - 18 unit specs covering the SQL shape, parser, fail-open paths, the blocked/unblocked transitions, and recovery. - Three e2e specs closing the gaps deferred from #439: jumps of more than one minor, a sequential chain through every published version, and a deterministic migration failure built from a never-released patch version (reachable with stock images now that the preflight turns it into a clean, observable stop). Closes #448 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- CHANGELOG.md | 1 + .../preview/api-reference.md | 31 +- .../preview/operations/upgrades.md | 20 + .../crds/documentdb.io_dbs.yaml | 61 ++ operator/src/api/preview/documentdb_types.go | 36 ++ .../src/api/preview/zz_generated.deepcopy.go | 8 + .../config/crd/bases/documentdb.io_dbs.yaml | 61 ++ .../controller/documentdb_controller.go | 221 ++++++- .../controller/documentdb_controller_test.go | 604 ++++++++++++++++-- test/e2e/README.md | 1 + test/e2e/tests/upgrade/helpers_test.go | 130 ++++ .../upgrade_schema_multiversion_test.go | 235 +++++++ .../upgrade/upgrade_schema_preflight_test.go | 294 +++++++++ 13 files changed, 1645 insertions(+), 58 deletions(-) create mode 100644 test/e2e/tests/upgrade/upgrade_schema_multiversion_test.go create mode 100644 test/e2e/tests/upgrade/upgrade_schema_preflight_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 424044906..72dd98a51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Major Features +- **Fail-fast preflight for extension update paths**: Before running `ALTER EXTENSION documentdb UPDATE`, the operator now queries `pg_extension_update_paths` to confirm PostgreSQL can actually resolve a migration path from the installed schema to the requested `spec.schemaVersion`. When no path exists — for example when `schemaVersion` names a version that was never released — the operator **skips the migration entirely** rather than firing an `ALTER EXTENSION` that fails with a raw PostgreSQL error on every reconcile. It surfaces a `SchemaUpgradeBlocked` status condition (reason `NoUpdatePath`) naming both versions, plus a matching warning event, and stops cleanly; correcting `spec.schemaVersion` clears the condition and completes the migration. Multi-minor jumps (e.g. `0.110.0` → `0.113.0`) remain a fully supported single-step path — the preflight only blocks paths PostgreSQL genuinely cannot resolve. `DocumentDB.status` gains a standard `conditions` array to carry this. See the [upgrade guide](docs/operator-public-documentation/preview/operations/upgrades.md#skipping-versions). - **Fail-fast ImageVolume capability check**: The operator now depends on the Kubernetes [ImageVolume](https://kubernetes.io/docs/concepts/storage/volumes/#image) feature to mount the DocumentDB extension into PostgreSQL pods. Instead of gating on a Kubernetes version number, the validating webhook performs a capability probe (a server-side dry-run) when a `DocumentDB` is created and **rejects the resource with an actionable error if ImageVolume is unavailable**, so you find out immediately instead of waiting for pods that never become ready. ImageVolume is GA (on by default) in Kubernetes **1.35+**; on **1.33/1.34** it is beta and must be enabled via the `ImageVolume` feature gate on a containerd/CRI-O runtime. The Helm chart's `kubeVersion` floor is relaxed to `>= 1.33.0-0` accordingly. See [Before you start](docs/operator-public-documentation/preview/getting-started/before-you-start.md). ## [0.3.0] - 2026-07-15 diff --git a/docs/operator-public-documentation/preview/api-reference.md b/docs/operator-public-documentation/preview/api-reference.md index ef63617df..ec7f7ffab 100644 --- a/docs/operator-public-documentation/preview/api-reference.md +++ b/docs/operator-public-documentation/preview/api-reference.md @@ -117,6 +117,28 @@ _Appears in:_ | `primary` _string_ | Primary is the name of the primary cluster for replication. | | | | `clusterList` _[MemberCluster](#membercluster) array_ | ClusterList is the list of clusters participating in replication. | | | | `highAvailability` _boolean_ | Whether or not to have replicas on the primary cluster. | | | +| `disableTLS` _boolean_ | Disables TLS for replication traffic between clusters.
Only for use when an existing mesh is already providing TLS. | false | | + + +#### ComponentResources + + + +ComponentResources overrides the CPU and/or memory allocated to an individual +container in the DocumentDB pod (PostgreSQL, the gateway, or the OTel +collector). Each field is a Kubernetes quantity string; when set it is applied +as both the request and the limit for that container (Guaranteed-class) and +overrides the automatic carve-out derived from spec.resource.memory. + + + +_Appears in:_ +- [Resource](#resource) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `memory` _string_ | Memory is the memory request=limit for the container (e.g. "512Mi", "2Gi"). | | Pattern: `^([0-9]+(\.[0-9]+)?(m\|Ki\|Mi\|Gi\|Ti\|Pi\|Ei\|k\|M\|G\|T\|P\|E)?)?$`
Optional: \{\}
| +| `cpu` _string_ | CPU is the CPU request=limit for the container (e.g. "500m", "2"). | | Pattern: `^([0-9]+(\.[0-9]+)?(m\|Ki\|Mi\|Gi\|Ti\|Pi\|Ei\|k\|M\|G\|T\|P\|E)?)?$`
Optional: \{\}
| #### DocumentDB @@ -254,7 +276,7 @@ _Appears in:_ | --- | --- | --- | --- | | `documentDB` _string_ | DocumentDB is the container image for the DocumentDB extension layer.
This image is mounted into the PostgreSQL container via CNPG's
ImageVolumeSource so that the extension files are available alongside
an upstream PostgreSQL image. | | Optional: \{\}
| | `gateway` _string_ | Gateway is the container image for the DocumentDB Gateway sidecar. | | Optional: \{\}
| -| `postgres` _string_ | Postgres is the container image for the PostgreSQL server.
Must be an upstream CNPG-compatible PostgreSQL image (the operator
adds the DocumentDB extension via an ImageVolume mount), and must
use trixie (Debian 13) base to match the extension's GLIBC
requirements. | ghcr.io/cloudnative-pg/postgresql:18-minimal-trixie | Optional: \{\}
| +| `postgres` _string_ | Postgres is the container image for the PostgreSQL server.
Must be an upstream CNPG-compatible PostgreSQL image (the operator
adds the DocumentDB extension via an ImageVolume mount), and must
use trixie (Debian 13) base to match the extension's GLIBC
requirements.
Pinned to the 18.4 minor tag instead of the floating
"18-minimal-trixie" tag, which rolled 18.4 -> 18.6 on 2026-08-13 and
crashed the DocumentDB 0.113.0 extension on insert. Staying on 18.4
avoids that regression while still receiving CNPG's Debian/PGDG
security rebuilds; revert to the floating "18-minimal-trixie" tag once
a DocumentDB release carrying the PG 18.6 fix ships. | ghcr.io/cloudnative-pg/postgresql:18.4-minimal-trixie | Optional: \{\}
| #### IssuerRef @@ -443,7 +465,10 @@ _Appears in:_ | --- | --- | --- | --- | | `storage` _[StorageConfiguration](#storageconfiguration)_ | Storage configuration for DocumentDB persistent volumes. | | | | `memory` _string_ | Memory specifies the memory limit for each DocumentDB instance pod.
This value is passed to the CNPG Cluster's spec.resources.limits.memory
and spec.resources.requests.memory (Guaranteed QoS).
Memory-aware PostgreSQL parameters (shared_buffers, effective_cache_size, etc.)
are auto-computed from this value.
If not specified or set to "0", no memory limit is applied and static
defaults are used for memory-aware parameters.
Examples: "2Gi", "4Gi", "8Gi" | | Optional: \{\}
| -| `cpu` _string_ | CPU specifies the CPU limit for each DocumentDB instance pod.
This value is passed to the CNPG Cluster's spec.resources.limits.cpu
and spec.resources.requests.cpu (Guaranteed QoS).
If not specified or set to "0", no CPU limit is applied.
Examples: "2", "4", "500m" | | Optional: \{\}
| +| `cpu` _string_ | CPU specifies the total CPU envelope for each DocumentDB instance pod.
The operator divides this envelope across PostgreSQL, the documentdb-gateway
sidecar, and, when monitoring is enabled, the OTel collector sidecar.
PostgreSQL receives the remainder after gateway and OTel CPU reservations;
an explicit per-container CPU override wins over the automatic carve-out.
If not specified or set to "0", no CPU envelope is applied.
Examples: "2", "4", "500m" | | Optional: \{\}
| +| `gateway` _[ComponentResources](#componentresources)_ | Gateway optionally overrides the resources allocated to the
documentdb-gateway sidecar container. When unset, the operator derives the
gateway's memory as min(gatewayMemoryFraction × memory, gatewayMemoryCap)
and carves it out of the pod memory envelope. The value is applied as both
the request and the limit (Guaranteed-class) so a gateway leak is
OOM-isolated and cannot crowd out PostgreSQL. | | Optional: \{\}
| +| `database` _[ComponentResources](#componentresources)_ | Database optionally overrides the resources allocated to the PostgreSQL
container. When unset, PostgreSQL receives the pod memory and CPU envelopes
minus the gateway and (when monitoring is enabled) OTel collector carve-outs. | | Optional: \{\}
| +| `otel` _[ComponentResources](#componentresources)_ | OTel optionally overrides the resources allocated to the otel-collector
sidecar container (only present when spec.monitoring.enabled is true).
When unset, the operator applies built-in defaults: memory request 48Mi /
limit 128Mi and CPU request 50m / limit 200m (Burstable — the requests are
the reserved floor and the limits cap a telemetry burst). Setting otel.cpu
or otel.memory pins that dimension to request == limit (Guaranteed). | | Optional: \{\}
| #### ScheduledBackup @@ -514,7 +539,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `gateway` _[GatewayTLS](#gatewaytls)_ | Gateway configures TLS for the gateway sidecar (Phase 1: certificate provisioning only). | | | -| `postgres` _[CertificatesConfiguration](https://pkg.go.dev/github.com/cloudnative-pg/cloudnative-pg/api/v1#CertificatesConfiguration)_ | Postgres configures TLS for the Postgres server. | | | +| `postgres` _[CertificatesConfiguration](https://pkg.go.dev/github.com/cloudnative-pg/cloudnative-pg/api/v1#CertificatesConfiguration)_ | Postgres configures TLS for the Postgres server.
If server side certs are provided alone, the operator will use sslMode=require for cross-regional replication connections.
If replication certs are also provided, the operator will use verify-full, which requires the hostname to be correctly set.
See the multi-region-deployment docs for how to do that. | | | | `globalEndpoints` _[GlobalEndpointsTLS](#globalendpointstls)_ | GlobalEndpoints configures TLS for global endpoints (placeholder for future phases). | | | diff --git a/docs/operator-public-documentation/preview/operations/upgrades.md b/docs/operator-public-documentation/preview/operations/upgrades.md index a151d4acd..0164e9dbb 100644 --- a/docs/operator-public-documentation/preview/operations/upgrades.md +++ b/docs/operator-public-documentation/preview/operations/upgrades.md @@ -253,6 +253,22 @@ Choose the approach that matches your use case: !!! warning With `schemaVersion: "auto"`, the schema migration is irreversible once applied. You cannot roll back to the previous version — only restore from backup. +### Skipping Versions + +You do **not** have to upgrade one minor at a time. DocumentDB ships a continuous chain of extension migration scripts, so a single upgrade can span several minors — for example `0.110.0` → `0.113.0`. PostgreSQL resolves the intermediate steps internally and the operator applies them in one `ALTER EXTENSION UPDATE`. + +Before running the migration the operator **preflights** the update path by querying `pg_extension_update_paths`. If no path exists between the installed schema and the version you requested, the operator does **not** run `ALTER EXTENSION` at all. Instead it stops cleanly and reports: + +- a `SchemaUpgradeBlocked` condition with status `True` and reason `NoUpdatePath` on `status.conditions`, and +- a `SchemaUpgradeBlocked` warning event on the DocumentDB resource. + +```bash +kubectl get documentdb my-cluster -n default \ + -o jsonpath='{.status.conditions[?(@.type=="SchemaUpgradeBlocked")]}' +``` + +The most common cause is requesting a `schemaVersion` that was never released (for example a patch version that does not exist). Set `spec.schemaVersion` to a real released version — or to `"auto"` to target whatever the binary provides — and the condition clears on the next reconcile. Your data is untouched while the upgrade is blocked: the migration never started. + ### Monitoring the Upgrade ```bash @@ -264,6 +280,10 @@ kubectl get documentdb my-cluster -n default # Check the current schema version kubectl get documentdb my-cluster -n default -o jsonpath='{.status.schemaVersion}' + +# Check whether a schema migration was blocked +kubectl get documentdb my-cluster -n default \ + -o jsonpath='{.status.conditions[?(@.type=="SchemaUpgradeBlocked")].reason}' ``` ### Rollback and Recovery diff --git a/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml b/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml index 5d5832abc..90433441f 100644 --- a/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml +++ b/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml @@ -1666,6 +1666,67 @@ spec: status: description: DocumentDBStatus defines the observed state of DocumentDB. properties: + conditions: + description: Conditions represent the latest available observations + of the DocumentDB state. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map connectionString: type: string documentDBImage: diff --git a/operator/src/api/preview/documentdb_types.go b/operator/src/api/preview/documentdb_types.go index 27c6ddb66..1ff6d2238 100644 --- a/operator/src/api/preview/documentdb_types.go +++ b/operator/src/api/preview/documentdb_types.go @@ -535,8 +535,44 @@ type DocumentDBStatus struct { // TLS reports gateway TLS provisioning status (Phase 1). TLS *TLSStatus `json:"tls,omitempty"` + + // Conditions represent the latest available observations of the DocumentDB state. + // +optional + // +listType=map + // +listMapKey=type + // +patchStrategy=merge + // +patchMergeKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` } +const ( + // ConditionSchemaUpgradeBlocked is set when the operator has determined that the + // requested extension schema migration cannot be performed and has deliberately + // not run ALTER EXTENSION UPDATE. Status "True" means the upgrade is blocked. + ConditionSchemaUpgradeBlocked = "SchemaUpgradeBlocked" + + // ReasonNoUpdatePath indicates PostgreSQL exposes no chain of extension update + // scripts between the installed schema version and the requested target, so + // ALTER EXTENSION UPDATE would fail. Used with ConditionSchemaUpgradeBlocked=True. + ReasonNoUpdatePath = "NoUpdatePath" + + // ReasonUpdatePathAvailable indicates a resolvable update path exists between the + // installed schema version and the requested target. Used with + // ConditionSchemaUpgradeBlocked=False. + ReasonUpdatePathAvailable = "UpdatePathAvailable" + + // ReasonSchemaUpToDate indicates no schema migration is pending. Used with + // ConditionSchemaUpgradeBlocked=False. + ReasonSchemaUpToDate = "SchemaUpToDate" + + // ReasonNoMigrationPlanned indicates the operator is not attempting a schema + // migration on this reconcile — for example two-phase mode where the user has + // not set spec.schemaVersion, or a detected extension rollback. Used with + // ConditionSchemaUpgradeBlocked=False so a previously blocked upgrade does not + // leave a stale True condition after the user changes the spec. + ReasonNoMigrationPlanned = "NoMigrationPlanned" +) + // TLSStatus captures readiness and secret information. type TLSStatus struct { Ready bool `json:"ready,omitempty"` diff --git a/operator/src/api/preview/zz_generated.deepcopy.go b/operator/src/api/preview/zz_generated.deepcopy.go index 17f792db2..576c025ae 100644 --- a/operator/src/api/preview/zz_generated.deepcopy.go +++ b/operator/src/api/preview/zz_generated.deepcopy.go @@ -10,6 +10,7 @@ package preview import ( apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -349,6 +350,13 @@ func (in *DocumentDBStatus) DeepCopyInto(out *DocumentDBStatus) { *out = new(TLSStatus) **out = **in } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DocumentDBStatus. diff --git a/operator/src/config/crd/bases/documentdb.io_dbs.yaml b/operator/src/config/crd/bases/documentdb.io_dbs.yaml index 5d5832abc..90433441f 100644 --- a/operator/src/config/crd/bases/documentdb.io_dbs.yaml +++ b/operator/src/config/crd/bases/documentdb.io_dbs.yaml @@ -1666,6 +1666,67 @@ spec: status: description: DocumentDBStatus defines the observed state of DocumentDB. properties: + conditions: + description: Conditions represent the latest available observations + of the DocumentDB state. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map connectionString: type: string documentDBImage: diff --git a/operator/src/internal/controller/documentdb_controller.go b/operator/src/internal/controller/documentdb_controller.go index 9dd8ed856..eae678ce0 100644 --- a/operator/src/internal/controller/documentdb_controller.go +++ b/operator/src/internal/controller/documentdb_controller.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "fmt" + "regexp" "slices" "strings" "sync" @@ -18,6 +19,8 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" @@ -25,6 +28,7 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" "k8s.io/client-go/tools/remotecommand" + "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -775,6 +779,159 @@ func parseExtensionVersionsFromOutput(output string) (defaultVersion, installedV return defaultVersion, installedVersion, true } +// pgExtensionUpdatePathsSQL builds the preflight query that asks PostgreSQL whether a +// chain of `documentdb----.sql` update scripts exists between two extension +// versions. pg_extension_update_paths returns one row per (source, target) pair the +// extension advertises, with a NULL path when the pair is known but unreachable; the +// pair is absent entirely when either version is not advertised at all. The COALESCE +// wrapper collapses both "no rows" and "NULL path" into a single sentinel so the result +// is always exactly one non-empty row and the psql output is unambiguous to parse. +func pgExtensionUpdatePathsSQL(fromVersion, toVersion string) string { + return fmt.Sprintf( + "SELECT COALESCE((SELECT path FROM pg_extension_update_paths('documentdb') "+ + "WHERE source = '%s' AND target = '%s'), '%s') AS update_path", + fromVersion, toVersion, noUpdatePathSentinel) +} + +// noUpdatePathSentinel is returned by the preflight query when PostgreSQL cannot resolve +// an update path. It is deliberately not a valid extension version, so it can never +// collide with a real path value. +const noUpdatePathSentinel = "NO_UPDATE_PATH" + +// extensionVersionPattern matches the PostgreSQL extension version format the operator +// works with (Major.Minor-Patch, e.g. "0.109-0"). It gates the values interpolated into +// the preflight query. +var extensionVersionPattern = regexp.MustCompile(`^[0-9]+\.[0-9]+-[0-9]+$`) + +// parseUpdatePathFromOutput parses the single-row output of pgExtensionUpdatePathsSQL. +// Expected format: +// +// update_path +// ---------------------- +// 0.109-0--0.110-0 +// (1 row) +// +// Returns the resolved path and whether the result could be interpreted at all. When ok +// is true and path equals noUpdatePathSentinel, PostgreSQL has no update path. +func parseUpdatePathFromOutput(output string) (path string, ok bool) { + lines := strings.Split(strings.TrimSpace(output), "\n") + if len(lines) < 3 { + return "", false + } + + path = strings.TrimSpace(lines[2]) + // A row count footer such as "(0 rows)" means the COALESCE guarantee did not hold and + // the output is not what this parser expects; treat it as uninterpretable. + if path == "" || strings.HasPrefix(path, "(") { + return "", false + } + return path, true +} + +// checkExtensionUpdatePath is a fail-fast preflight for ALTER EXTENSION UPDATE. +// +// PostgreSQL resolves an extension upgrade by walking the graph of update scripts the +// extension ships. If any release in the range omits its script, the graph has a gap and +// ALTER EXTENSION UPDATE fails at execution time with a raw "extension has no update path +// from X to Y" error that re-fires on every reconcile. pg_extension_update_paths exposes +// that same graph read-only, so the operator can detect the gap *before* touching the +// schema and surface an actionable status condition instead. +// +// Returns blocked=true only when the absence of a path is positively proven. Any error or +// unparseable result fails open (blocked=false): the preflight is an advisory improvement, +// so an inconclusive check must preserve the pre-existing behavior of letting PostgreSQL +// decide rather than wedging an upgrade that would otherwise succeed. +func (r *DocumentDBReconciler) checkExtensionUpdatePath( + ctx context.Context, + cluster *cnpgv1.Cluster, + fromVersion string, + toVersion string, +) (blocked bool, path string) { + logger := log.FromContext(ctx) + + // toVersion is constrained by the CRD pattern and the validating webhook, but + // fromVersion is read out of pg_available_extensions, i.e. it ultimately derives from + // the extension image's script filenames rather than from a validated schema. Since + // both are interpolated into the query, require the expected Major.Minor-Patch shape + // and fail open otherwise instead of trusting the image. + if !extensionVersionPattern.MatchString(fromVersion) || !extensionVersionPattern.MatchString(toVersion) { + logger.Info("Unexpected extension version format; skipping update path preflight", + "fromVersion", fromVersion, + "toVersion", toVersion) + return false, "" + } + + output, err := r.SQLExecutor(ctx, cluster, pgExtensionUpdatePathsSQL(fromVersion, toVersion)) + if err != nil { + logger.Error(err, "Extension update path preflight failed; proceeding with ALTER EXTENSION", + "fromVersion", fromVersion, + "toVersion", toVersion) + return false, "" + } + + resolved, ok := parseUpdatePathFromOutput(output) + if !ok { + logger.Info("Could not parse extension update path preflight output; proceeding with ALTER EXTENSION", + "fromVersion", fromVersion, + "toVersion", toVersion, + "output", output) + return false, "" + } + + if resolved == noUpdatePathSentinel { + return true, "" + } + + logger.V(1).Info("Extension update path resolved", + "fromVersion", fromVersion, + "toVersion", toVersion, + "path", resolved) + return false, resolved +} + +// setSchemaUpgradeBlockedCondition writes the SchemaUpgradeBlocked condition onto the +// DocumentDB status. Condition churn is avoided by meta.SetStatusCondition, which reports +// whether anything actually changed, so a no-op reconcile issues no API write. +// +// The read comes from the informer cache, which can lag a status update this same +// reconcile just made, so the write is retried on conflict rather than relying on the +// resulting watch event to re-drive reconciliation. +// +// NOTE: this refetches into documentdb, replacing the caller's copy. +func (r *DocumentDBReconciler) setSchemaUpgradeBlockedCondition( + ctx context.Context, + documentdb *dbpreview.DocumentDB, + status metav1.ConditionStatus, + reason string, + message string, +) error { + name := types.NamespacedName{Name: documentdb.Name, Namespace: documentdb.Namespace} + + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := r.Get(ctx, name, documentdb); err != nil { + return err + } + + condition := metav1.Condition{ + Type: dbpreview.ConditionSchemaUpgradeBlocked, + Status: status, + Reason: reason, + Message: message, + ObservedGeneration: documentdb.Generation, + } + + if !meta.SetStatusCondition(&documentdb.Status.Conditions, condition) { + return nil + } + + return r.Status().Update(ctx, documentdb) + }) + if err != nil { + return fmt.Errorf("failed to update DocumentDB status conditions: %w", err) + } + return nil +} + // handleExtensionUpgrade handles the ALTER EXTENSION lifecycle after images have been synced // by SyncCnpgCluster. It: // 1. Updates DocumentDB status with the current images from the CNPG cluster @@ -836,6 +993,12 @@ func (r *DocumentDBReconciler) handleExtensionUpgrade(ctx context.Context, curre // If versions match, no upgrade needed if defaultVersion == installedVersion { logger.V(1).Info("DocumentDB extension is up to date", "version", installedVersion) + if err := r.setSchemaUpgradeBlockedCondition(ctx, documentdb, metav1.ConditionFalse, + dbpreview.ReasonSchemaUpToDate, + fmt.Sprintf("Extension schema is at %s; no migration pending.", util.ExtensionVersionToSemver(installedVersion)), + ); err != nil { + logger.Error(err, "Failed to clear SchemaUpgradeBlocked condition") + } return nil } @@ -862,20 +1025,74 @@ func (r *DocumentDBReconciler) handleExtensionUpgrade(ctx context.Context, curre if r.Recorder != nil { r.Recorder.Event(documentdb, corev1.EventTypeWarning, "ExtensionRollback", msg) } + // No schema migration is being attempted, so any previous block is moot. + if err := r.setSchemaUpgradeBlockedCondition(ctx, documentdb, metav1.ConditionFalse, + dbpreview.ReasonNoMigrationPlanned, + "Extension rollback detected; no schema migration is being attempted.", + ); err != nil { + logger.Error(err, "Failed to clear SchemaUpgradeBlocked condition") + } return nil } // Determine schema target based on spec.schemaVersion (two-phase upgrade logic) schemaTarget, updateSQL := r.determineSchemaTarget(ctx, documentdb, defaultVersion, installedVersion) if schemaTarget == "" { - // Two-phase mode or validation failure — do not run ALTER EXTENSION + // Two-phase mode or validation failure — do not run ALTER EXTENSION. + // Clear any prior block so reverting spec.schemaVersion actually resolves + // a SchemaUpgradeBlocked condition instead of leaving it stuck at True. + if err := r.setSchemaUpgradeBlockedCondition(ctx, documentdb, metav1.ConditionFalse, + dbpreview.ReasonNoMigrationPlanned, + "No schema migration is currently requested; set spec.schemaVersion to finalize an upgrade.", + ); err != nil { + logger.Error(err, "Failed to clear SchemaUpgradeBlocked condition") + } + return nil + } + + // Preflight: refuse to fire ALTER EXTENSION when PostgreSQL cannot resolve an update + // path. Without this, the ALTER fails at execution time with a raw PG error that + // re-fires every reconcile; with it, the user gets an actionable status condition and + // the reconcile stops cleanly. + blocked, resolvedPath := r.checkExtensionUpdatePath(ctx, currentCluster, installedVersion, schemaTarget) + if blocked { + msg := fmt.Sprintf( + "Schema upgrade blocked: the documentdb extension provides no update path from %s to %s. "+ + "ALTER EXTENSION UPDATE was not run because it would fail. "+ + "This means the extension image is missing one or more documentdb----.sql "+ + "migration scripts in that range. To resolve, upgrade to a version that has a "+ + "continuous update path from %s, or restore from a backup taken before the image change.", + util.ExtensionVersionToSemver(installedVersion), + util.ExtensionVersionToSemver(schemaTarget), + util.ExtensionVersionToSemver(installedVersion)) + logger.Info(msg) + if r.Recorder != nil { + r.Recorder.Event(documentdb, corev1.EventTypeWarning, dbpreview.ConditionSchemaUpgradeBlocked, msg) + } + if err := r.setSchemaUpgradeBlockedCondition(ctx, documentdb, metav1.ConditionTrue, + dbpreview.ReasonNoUpdatePath, msg); err != nil { + // The condition is the only actionable signal for this branch, so a failure to + // publish it must requeue rather than return cleanly. + return fmt.Errorf("failed to publish SchemaUpgradeBlocked condition: %w", err) + } + // Return cleanly: retrying cannot help until the user changes the spec. return nil } + if err := r.setSchemaUpgradeBlockedCondition(ctx, documentdb, metav1.ConditionFalse, + dbpreview.ReasonUpdatePathAvailable, + fmt.Sprintf("Update path from %s to %s is resolvable.", + util.ExtensionVersionToSemver(installedVersion), + util.ExtensionVersionToSemver(schemaTarget)), + ); err != nil { + logger.Error(err, "Failed to clear SchemaUpgradeBlocked condition") + } + // Run ALTER EXTENSION to upgrade logger.Info("Upgrading DocumentDB extension", "fromVersion", installedVersion, - "toVersion", schemaTarget) + "toVersion", schemaTarget, + "updatePath", resolvedPath) if _, err := r.SQLExecutor(ctx, currentCluster, updateSQL); err != nil { return fmt.Errorf("failed to run ALTER EXTENSION documentdb UPDATE: %w", err) diff --git a/operator/src/internal/controller/documentdb_controller_test.go b/operator/src/internal/controller/documentdb_controller_test.go index ee46ac21a..f5c5484e7 100644 --- a/operator/src/internal/controller/documentdb_controller_test.go +++ b/operator/src/internal/controller/documentdb_controller_test.go @@ -37,6 +37,41 @@ func parseExtensionVersions(output string) (defaultVersion, installedVersion str return parseExtensionVersionsFromOutput(output) } +// versionCheckOutput renders psql aligned output for the pg_available_extensions query +// that handleExtensionUpgrade issues first. +func versionCheckOutput(defaultVersion, installedVersion string) string { + return fmt.Sprintf( + " default_version | installed_version \n-----------------+-------------------\n %s | %s \n", + defaultVersion, installedVersion) +} + +// updatePathOutput renders psql aligned output for the pg_extension_update_paths preflight. +// Pass noUpdatePathSentinel to simulate a missing update path. +func updatePathOutput(path string) string { + return fmt.Sprintf(" update_path \n----------------------\n %s \n(1 row)\n", path) +} + +// extensionSQLResponder builds a SQLExecutor stub that answers both read-only queries +// handleExtensionUpgrade issues (the version check and the update-path preflight), +// records every statement it receives into calls, and returns a canned success for +// anything else (i.e. the ALTER EXTENSION itself). +func extensionSQLResponder( + calls *[]string, + defaultVersion, installedVersion, updatePath string, +) func(context.Context, *cnpgv1.Cluster, string) (string, error) { + return func(_ context.Context, _ *cnpgv1.Cluster, sql string) (string, error) { + *calls = append(*calls, sql) + switch { + case strings.Contains(sql, "pg_available_extensions"): + return versionCheckOutput(defaultVersion, installedVersion), nil + case strings.Contains(sql, "pg_extension_update_paths"): + return updatePathOutput(updatePath), nil + default: + return "ALTER EXTENSION", nil + } + } +} + var _ = Describe("DocumentDB Controller", func() { const ( clusterName = "test-cluster" @@ -834,27 +869,20 @@ var _ = Describe("DocumentDB Controller", func() { sqlCalls := []string{} reconciler := &DocumentDBReconciler{ - Client: fakeClient, - Scheme: scheme, - Recorder: recorder, - SQLExecutor: func(_ context.Context, _ *cnpgv1.Cluster, sql string) (string, error) { - sqlCalls = append(sqlCalls, sql) - if len(sqlCalls) == 1 { - // First call: version check — installed 0.109-0, default 0.110-0 - return " default_version | installed_version \n-----------------+-------------------\n 0.110-0 | 0.109-0 \n", nil - } - // Second call: ALTER EXTENSION - return "ALTER EXTENSION", nil - }, + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.110-0", "0.109-0", "0.109-0--0.110-0"), } err := reconciler.handleExtensionUpgrade(ctx, cluster, documentdb) Expect(err).ToNot(HaveOccurred()) - // Verify both SQL calls were made - Expect(sqlCalls).To(HaveLen(2)) + // Verify the version check, the update-path preflight, and the ALTER all ran + Expect(sqlCalls).To(HaveLen(3)) Expect(sqlCalls[0]).To(ContainSubstring("pg_available_extensions")) - Expect(sqlCalls[1]).To(Equal("ALTER EXTENSION documentdb UPDATE")) + Expect(sqlCalls[1]).To(ContainSubstring("pg_extension_update_paths")) + Expect(sqlCalls[2]).To(Equal("ALTER EXTENSION documentdb UPDATE")) // Status should reflect the upgraded version (default version as semver) updatedDB := &dbpreview.DocumentDB{} @@ -1208,23 +1236,16 @@ var _ = Describe("DocumentDB Controller", func() { sqlCalls := []string{} reconciler := &DocumentDBReconciler{ - Client: fakeClient, - Scheme: scheme, - Recorder: recorder, - SQLExecutor: func(_ context.Context, _ *cnpgv1.Cluster, sql string) (string, error) { - sqlCalls = append(sqlCalls, sql) - if len(sqlCalls) == 1 { - // default > installed → triggers ALTER EXTENSION - return " default_version | installed_version \n-----------------+-------------------\n 0.110-0 | 0.109-0 \n", nil - } - return "ALTER EXTENSION", nil - }, + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.110-0", "0.109-0", "0.109-0--0.110-0"), } err := reconciler.handleExtensionUpgrade(ctx, cluster, documentdb) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to update DocumentDB status after schema upgrade")) - Expect(sqlCalls).To(HaveLen(2)) + Expect(sqlCalls).To(HaveLen(3)) }) }) @@ -1343,25 +1364,20 @@ var _ = Describe("DocumentDB Controller", func() { sqlCalls := []string{} reconciler := &DocumentDBReconciler{ - Client: fakeClient, - Scheme: scheme, - Recorder: recorder, - SQLExecutor: func(_ context.Context, _ *cnpgv1.Cluster, sql string) (string, error) { - sqlCalls = append(sqlCalls, sql) - if len(sqlCalls) == 1 { - return " default_version | installed_version \n-----------------+-------------------\n 0.110-0 | 0.109-0 \n", nil - } - return "ALTER EXTENSION", nil - }, + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.110-0", "0.109-0", "0.109-0--0.110-0"), } err := reconciler.handleExtensionUpgrade(ctx, cluster, documentdb) Expect(err).ToNot(HaveOccurred()) - // Both version-check and ALTER EXTENSION should have been called - Expect(sqlCalls).To(HaveLen(2)) + // Version check, update-path preflight and ALTER EXTENSION should have been called + Expect(sqlCalls).To(HaveLen(3)) Expect(sqlCalls[0]).To(ContainSubstring("pg_available_extensions")) - Expect(sqlCalls[1]).To(Equal("ALTER EXTENSION documentdb UPDATE")) + Expect(sqlCalls[1]).To(ContainSubstring("pg_extension_update_paths")) + Expect(sqlCalls[2]).To(Equal("ALTER EXTENSION documentdb UPDATE")) // Status should reflect the upgraded version updatedDB := &dbpreview.DocumentDB{} @@ -1414,26 +1430,20 @@ var _ = Describe("DocumentDB Controller", func() { sqlCalls := []string{} reconciler := &DocumentDBReconciler{ - Client: fakeClient, - Scheme: scheme, - Recorder: recorder, - SQLExecutor: func(_ context.Context, _ *cnpgv1.Cluster, sql string) (string, error) { - sqlCalls = append(sqlCalls, sql) - if len(sqlCalls) == 1 { - // Binary is 0.110-0, installed is 0.109-0 - return " default_version | installed_version \n-----------------+-------------------\n 0.110-0 | 0.109-0 \n", nil - } - return "ALTER EXTENSION", nil - }, + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.110-0", "0.109-0", "0.109-0--0.110-0"), } err := reconciler.handleExtensionUpgrade(ctx, cluster, documentdb) Expect(err).ToNot(HaveOccurred()) // Should run ALTER EXTENSION UPDATE TO specific version - Expect(sqlCalls).To(HaveLen(2)) + Expect(sqlCalls).To(HaveLen(3)) Expect(sqlCalls[0]).To(ContainSubstring("pg_available_extensions")) - Expect(sqlCalls[1]).To(Equal("ALTER EXTENSION documentdb UPDATE TO '0.110-0'")) + Expect(sqlCalls[1]).To(ContainSubstring("pg_extension_update_paths")) + Expect(sqlCalls[2]).To(Equal("ALTER EXTENSION documentdb UPDATE TO '0.110-0'")) // Status should reflect the explicit version updatedDB := &dbpreview.DocumentDB{} @@ -1638,6 +1648,494 @@ var _ = Describe("DocumentDB Controller", func() { // admission time. The controller no longer needs to check for this case. }) + Describe("extension update path preflight", func() { + // upgradeCluster builds a healthy single-instance CNPG cluster suitable for + // driving handleExtensionUpgrade. + upgradeCluster := func() *cnpgv1.Cluster { + return &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: clusterNamespace, + }, + Spec: cnpgv1.ClusterSpec{ + PostgresConfiguration: cnpgv1.PostgresConfiguration{ + Extensions: []cnpgv1.ExtensionConfiguration{ + { + Name: "documentdb", + ImageVolumeSource: corev1.ImageVolumeSource{ + Reference: "documentdb/documentdb:v1.0.0", + }, + }, + }, + }, + }, + Status: cnpgv1.ClusterStatus{ + CurrentPrimary: "test-cluster-1", + InstancesStatus: map[cnpgv1.PodStatus][]string{ + cnpgv1.PodHealthy: {"test-cluster-1"}, + }, + }, + } + } + + upgradeDB := func(schemaVersion string) *dbpreview.DocumentDB { + return &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: clusterNamespace, + }, + Spec: dbpreview.DocumentDBSpec{ + SchemaVersion: schemaVersion, + }, + } + } + + newClient := func(objs ...client.Object) client.Client { + return fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + WithStatusSubresource(&dbpreview.DocumentDB{}). + Build() + } + + blockedCondition := func(c client.Client) *metav1.Condition { + updated := &dbpreview.DocumentDB{} + ExpectWithOffset(1, c.Get(ctx, types.NamespacedName{ + Name: documentDBName, Namespace: clusterNamespace, + }, updated)).To(Succeed()) + for i := range updated.Status.Conditions { + if updated.Status.Conditions[i].Type == dbpreview.ConditionSchemaUpgradeBlocked { + return &updated.Status.Conditions[i] + } + } + return nil + } + + Describe("pgExtensionUpdatePathsSQL", func() { + It("should build a single-row query with the sentinel fallback", func() { + sql := pgExtensionUpdatePathsSQL("0.109-0", "0.113-0") + Expect(sql).To(ContainSubstring("pg_extension_update_paths('documentdb')")) + Expect(sql).To(ContainSubstring("source = '0.109-0'")) + Expect(sql).To(ContainSubstring("target = '0.113-0'")) + Expect(sql).To(ContainSubstring(noUpdatePathSentinel)) + }) + }) + + Describe("parseUpdatePathFromOutput", func() { + It("should parse a resolvable single-hop path", func() { + path, ok := parseUpdatePathFromOutput(updatePathOutput("0.109-0--0.110-0")) + Expect(ok).To(BeTrue()) + Expect(path).To(Equal("0.109-0--0.110-0")) + }) + + It("should parse a resolvable multi-hop chain", func() { + chain := "0.109-0--0.110-0--0.111-0--0.112-0" + path, ok := parseUpdatePathFromOutput(updatePathOutput(chain)) + Expect(ok).To(BeTrue()) + Expect(path).To(Equal(chain)) + }) + + It("should surface the sentinel when no path exists", func() { + path, ok := parseUpdatePathFromOutput(updatePathOutput(noUpdatePathSentinel)) + Expect(ok).To(BeTrue()) + Expect(path).To(Equal(noUpdatePathSentinel)) + }) + + It("should report not-ok for truncated output", func() { + _, ok := parseUpdatePathFromOutput(" update_path \n-------------\n") + Expect(ok).To(BeFalse()) + }) + + It("should report not-ok for empty output", func() { + _, ok := parseUpdatePathFromOutput("") + Expect(ok).To(BeFalse()) + }) + + It("should report not-ok when a row-count footer lands in the data position", func() { + _, ok := parseUpdatePathFromOutput(" update_path \n-------------\n(0 rows)\n") + Expect(ok).To(BeFalse()) + }) + }) + + It("should skip the preflight and fail open on an unexpected version format", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("auto") + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", noUpdatePathSentinel), + } + + // A version that does not match Major.Minor-Patch must never reach the query. + blocked, path := reconciler.checkExtensionUpdatePath( + ctx, cluster, "0.109-0'; DROP SCHEMA public CASCADE; --", "0.113-0") + + Expect(blocked).To(BeFalse(), "an unverifiable version must fail open") + Expect(path).To(BeEmpty()) + Expect(sqlCalls).To(BeEmpty(), "no SQL may be generated from an unexpected version string") + }) + + Describe("setSchemaUpgradeBlockedCondition", func() { + It("should not issue a status write when the condition is unchanged", func() { + documentdb := upgradeDB("auto") + fakeClient := newClient(documentdb) + reconciler := &DocumentDBReconciler{Client: fakeClient, Scheme: scheme, Recorder: recorder} + + Expect(reconciler.setSchemaUpgradeBlockedCondition(ctx, documentdb, + metav1.ConditionTrue, dbpreview.ReasonNoUpdatePath, "blocked")).To(Succeed()) + + first := blockedCondition(fakeClient) + Expect(first).ToNot(BeNil()) + + // Re-applying an identical condition must be a no-op so the controller + // does not rewrite status on every reconcile. + Expect(reconciler.setSchemaUpgradeBlockedCondition(ctx, documentdb, + metav1.ConditionTrue, dbpreview.ReasonNoUpdatePath, "blocked")).To(Succeed()) + + second := blockedCondition(fakeClient) + Expect(second.LastTransitionTime).To(Equal(first.LastTransitionTime)) + }) + + It("should return an error when the DocumentDB cannot be read", func() { + documentdb := upgradeDB("auto") + fakeClient := newClient() // DocumentDB deliberately absent + reconciler := &DocumentDBReconciler{Client: fakeClient, Scheme: scheme, Recorder: recorder} + + err := reconciler.setSchemaUpgradeBlockedCondition(ctx, documentdb, + metav1.ConditionTrue, dbpreview.ReasonNoUpdatePath, "blocked") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("status conditions")) + }) + }) + + It("should requeue when the blocked condition cannot be published", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("auto") + // Pre-set so the earlier status.schemaVersion write is skipped and the + // interceptor only trips on the condition write under test. + documentdb.Status.SchemaVersion = "0.109.0" + + // Status writes fail, so the blocked condition never reaches the API server. + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(cluster, documentdb). + WithStatusSubresource(&dbpreview.DocumentDB{}). + WithInterceptorFuncs(interceptor.Funcs{ + SubResourceUpdate: func( + _ context.Context, _ client.Client, _ string, + obj client.Object, _ ...client.SubResourceUpdateOption, + ) error { + if _, ok := obj.(*dbpreview.DocumentDB); ok { + return fmt.Errorf("simulated status update failure") + } + return nil + }, + }). + Build() + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", noUpdatePathSentinel), + } + + err := reconciler.handleExtensionUpgrade(ctx, cluster, documentdb) + Expect(err).To(HaveOccurred(), + "a blocked upgrade whose condition cannot be published must requeue, "+ + "not return cleanly and leave the user with no signal") + Expect(err.Error()).To(ContainSubstring("SchemaUpgradeBlocked")) + + for _, sql := range sqlCalls { + Expect(sql).ToNot(ContainSubstring("ALTER EXTENSION")) + } + }) + + It("should skip ALTER EXTENSION and set the blocked condition when no update path exists", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("auto") + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", noUpdatePathSentinel), + } + + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + + // Version check + preflight only — the ALTER must never be issued. + Expect(sqlCalls).To(HaveLen(2)) + Expect(sqlCalls[1]).To(ContainSubstring("pg_extension_update_paths")) + for _, sql := range sqlCalls { + Expect(sql).ToNot(ContainSubstring("ALTER EXTENSION")) + } + + cond := blockedCondition(fakeClient) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(dbpreview.ReasonNoUpdatePath)) + Expect(cond.Message).To(ContainSubstring("0.109.0")) + Expect(cond.Message).To(ContainSubstring("0.113.0")) + + // The schema must stay where it was — a blocked upgrade is not a completed one. + updated := &dbpreview.DocumentDB{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: documentDBName, Namespace: clusterNamespace, + }, updated)).To(Succeed()) + Expect(updated.Status.SchemaVersion).To(Equal("0.109.0")) + + Eventually(recorder.Events).Should(Receive(And( + ContainSubstring("Warning"), + ContainSubstring(dbpreview.ConditionSchemaUpgradeBlocked), + ))) + }) + + It("should run ALTER EXTENSION and clear the condition when a path exists", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("auto") + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", "0.109-0--0.113-0"), + } + + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + + Expect(sqlCalls).To(HaveLen(3)) + Expect(sqlCalls[2]).To(Equal("ALTER EXTENSION documentdb UPDATE")) + + cond := blockedCondition(fakeClient) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(dbpreview.ReasonUpdatePathAvailable)) + + updated := &dbpreview.DocumentDB{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: documentDBName, Namespace: clusterNamespace, + }, updated)).To(Succeed()) + Expect(updated.Status.SchemaVersion).To(Equal("0.113.0")) + }) + + It("should not run the preflight at all when the schema is already current", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("auto") + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.113-0", noUpdatePathSentinel), + } + + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + + // Same-version no-op: only the version check runs. + Expect(sqlCalls).To(HaveLen(1)) + Expect(sqlCalls[0]).To(ContainSubstring("pg_available_extensions")) + + cond := blockedCondition(fakeClient) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(dbpreview.ReasonSchemaUpToDate)) + }) + + It("should not run the preflight in two-phase mode where no ALTER is planned", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("") // two-phase: schema stays put + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", noUpdatePathSentinel), + } + + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + + Expect(sqlCalls).To(HaveLen(1)) + cond := blockedCondition(fakeClient) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(dbpreview.ReasonNoMigrationPlanned)) + }) + + It("should clear a stale blocked condition when the user reverts to two-phase mode", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("0.111.0") // unreachable target → blocked + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", noUpdatePathSentinel), + } + + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + Expect(blockedCondition(fakeClient).Status).To(Equal(metav1.ConditionTrue)) + + By("reverting spec.schemaVersion to two-phase mode") + reverted := &dbpreview.DocumentDB{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: documentDBName, Namespace: clusterNamespace, + }, reverted)).To(Succeed()) + reverted.Spec.SchemaVersion = "" + Expect(fakeClient.Update(ctx, reverted)).To(Succeed()) + + sqlCalls = nil + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, reverted)).To(Succeed()) + + cond := blockedCondition(fakeClient) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse), + "a stale blocked condition must not survive reverting the spec") + Expect(cond.Reason).To(Equal(dbpreview.ReasonNoMigrationPlanned)) + }) + + It("should run the migration once the user retargets to a reachable version", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("0.111.0") // unreachable target → blocked + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + blockedReconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", noUpdatePathSentinel), + } + + Expect(blockedReconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + Expect(blockedCondition(fakeClient).Status).To(Equal(metav1.ConditionTrue)) + + By("retargeting spec.schemaVersion to a version with an update path") + retargeted := &dbpreview.DocumentDB{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: documentDBName, Namespace: clusterNamespace, + }, retargeted)).To(Succeed()) + retargeted.Spec.SchemaVersion = "0.113.0" + Expect(fakeClient.Update(ctx, retargeted)).To(Succeed()) + + sqlCalls = nil + okReconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", "0.109-0--0.113-0"), + } + Expect(okReconciler.handleExtensionUpgrade(ctx, cluster, retargeted)).To(Succeed()) + + Expect(sqlCalls).To(HaveLen(3)) + Expect(sqlCalls[2]).To(ContainSubstring("ALTER EXTENSION documentdb UPDATE TO '0.113-0'")) + + cond := blockedCondition(fakeClient) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(dbpreview.ReasonUpdatePathAvailable)) + }) + + It("should fail open and still run ALTER EXTENSION when the preflight query errors", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("auto") + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: func(_ context.Context, _ *cnpgv1.Cluster, sql string) (string, error) { + sqlCalls = append(sqlCalls, sql) + switch { + case strings.Contains(sql, "pg_available_extensions"): + return versionCheckOutput("0.113-0", "0.109-0"), nil + case strings.Contains(sql, "pg_extension_update_paths"): + return "", fmt.Errorf("function pg_extension_update_paths does not exist") + default: + return "ALTER EXTENSION", nil + } + }, + } + + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + + // An inconclusive preflight must not wedge an upgrade that would otherwise work. + Expect(sqlCalls).To(HaveLen(3)) + Expect(sqlCalls[2]).To(Equal("ALTER EXTENSION documentdb UPDATE")) + }) + + It("should fail open when the preflight output cannot be parsed", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("auto") + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: func(_ context.Context, _ *cnpgv1.Cluster, sql string) (string, error) { + sqlCalls = append(sqlCalls, sql) + switch { + case strings.Contains(sql, "pg_available_extensions"): + return versionCheckOutput("0.113-0", "0.109-0"), nil + case strings.Contains(sql, "pg_extension_update_paths"): + return "garbage", nil + default: + return "ALTER EXTENSION", nil + } + }, + } + + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + Expect(sqlCalls).To(HaveLen(3)) + Expect(sqlCalls[2]).To(Equal("ALTER EXTENSION documentdb UPDATE")) + }) + + It("should preflight against the explicit target when schemaVersion is pinned", func() { + cluster := upgradeCluster() + documentdb := upgradeDB("0.111.0") + fakeClient := newClient(cluster, documentdb) + + sqlCalls := []string{} + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + SQLExecutor: extensionSQLResponder(&sqlCalls, "0.113-0", "0.109-0", noUpdatePathSentinel), + } + + Expect(reconciler.handleExtensionUpgrade(ctx, cluster, documentdb)).To(Succeed()) + + // The preflight must ask about installed → pinned target, not installed → binary. + Expect(sqlCalls).To(HaveLen(2)) + Expect(sqlCalls[1]).To(ContainSubstring("source = '0.109-0'")) + Expect(sqlCalls[1]).To(ContainSubstring("target = '0.111-0'")) + + cond := blockedCondition(fakeClient) + Expect(cond).ToNot(BeNil()) + Expect(cond.Reason).To(Equal(dbpreview.ReasonNoUpdatePath)) + Expect(cond.Message).To(ContainSubstring("0.111.0")) + }) + }) + Describe("updateImageStatus", func() { It("should set DocumentDBImage and GatewayImage from cluster spec", func() { cluster := &cnpgv1.Cluster{ diff --git a/test/e2e/README.md b/test/e2e/README.md index 93519be2a..dc4710d6f 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -142,6 +142,7 @@ E2E_UPGRADE=1 E2E_UPGRADE_PREVIOUS_CHART=… \ | `E2E_UPGRADE_NEW_DOCUMENTDB_IMAGE` | Extension image used after upgrade | | `E2E_UPGRADE_OLD_DOCUMENTDB_VERSION` | Schema-upgrade spec: `spec.documentDBVersion` to start from (default `0.109.0`) | | `E2E_UPGRADE_NEW_DOCUMENTDB_VERSION` | Schema-upgrade spec: `spec.documentDBVersion` to upgrade to (default `0.110.0`) | +| `E2E_UPGRADE_DOCUMENTDB_VERSION_CHAIN` | Multi-version upgrade specs: ascending comma-separated list of **published** DocumentDB versions (default `0.109.0,0.110.0,0.113.0,0.114.0`). Drives the multi-minor-jump and sequential-chain specs; every entry must be a real released tag or the operator's update-path preflight will block the migration | > A note on `E2E_KEEP_CLUSTERS`: the design doc discusses a flag for keeping > DocumentDB-cluster fixtures around after a failed spec, but no such knob is diff --git a/test/e2e/tests/upgrade/helpers_test.go b/test/e2e/tests/upgrade/helpers_test.go index c399adcc5..1b944703f 100644 --- a/test/e2e/tests/upgrade/helpers_test.go +++ b/test/e2e/tests/upgrade/helpers_test.go @@ -3,10 +3,12 @@ package upgrade import ( "context" "fmt" + "math" "os" "os/exec" "path/filepath" "runtime" + "strconv" "strings" "time" @@ -16,7 +18,11 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + + previewv1 "github.com/documentdb/documentdb-operator/api/preview" + shareddb "github.com/documentdb/documentdb-operator/test/shared/documentdb" ) // Environment variables that gate and parameterize the upgrade area. @@ -42,6 +48,14 @@ const ( envOldDocumentDBVersion = "E2E_UPGRADE_OLD_DOCUMENTDB_VERSION" envNewDocumentDBVersion = "E2E_UPGRADE_NEW_DOCUMENTDB_VERSION" + // Ascending, comma-separated list of published DocumentDB versions used by + // the multi-version upgrade specs (multi-minor jump and sequential chain). + // The extension only ships documentdb----.sql scripts between + // released versions, so every entry must be a real published tag on both + // the documentdb and gateway GHCR repos — an invented version has no + // update path and would be blocked by the operator's preflight. + envDocumentDBVersionChain = "E2E_UPGRADE_DOCUMENTDB_VERSION_CHAIN" + // Default old/new versions for the schema-upgrade spec, applied when // the env vars above are unset. Chosen as the last released pair so // the spec exercises a real two-phase migration on every e2e PR @@ -57,6 +71,15 @@ const ( defaultOldDocumentDBVersion = "0.109.0" defaultNewDocumentDBVersion = "0.110.0" + // Default version chain for the multi-version upgrade specs. These are the + // published DocumentDB releases on ghcr.io/documentdb/documentdb-kubernetes-operator + // (documentdb + gateway). The gaps are deliberate and load-bearing: the chain + // omits 0.111.x and 0.112.x entirely, so the single-step jump spec (first entry + // → last entry, 0.109.0 → 0.114.0) crosses several unpublished minors — exactly + // the ">1 minor jump" case these specs exist to cover. Keep this list ascending + // and keep every entry a real published tag; add newly released versions to the end. + defaultDocumentDBVersionChain = "0.109.0,0.110.0,0.113.0,0.114.0" + // Optional gateway image overrides for the image-upgrade spec. // When unset the spec patches only spec.image.documentDB and leaves // spec.image.gateway as-is (operator uses its default gateway). The @@ -190,6 +213,113 @@ func createCredentialSecret(ctx context.Context, c client.Client, ns string) { } } +// documentDBVersionChain returns the ascending list of published DocumentDB +// versions used by the multi-version upgrade specs, read from +// envDocumentDBVersionChain (comma-separated) or falling back to +// defaultDocumentDBVersionChain. Specs that need more entries than are +// configured should Skip rather than fabricate versions. +func documentDBVersionChain() []string { + raw := envOr(envDocumentDBVersionChain, defaultDocumentDBVersionChain) + var out []string + for _, part := range strings.Split(raw, ",") { + if v := strings.TrimSpace(part); v != "" { + out = append(out, v) + } + } + return out +} + +// majorMinor splits a "Major.Minor.Patch" version string into its numeric major +// and minor components, reporting ok=false when either cannot be parsed. +func majorMinor(version string) (major, minor int, ok bool) { + parts := strings.Split(version, ".") + if len(parts) < 2 { + return 0, 0, false + } + major, err := strconv.Atoi(parts[0]) + if err != nil { + return 0, 0, false + } + minor, err = strconv.Atoi(parts[1]) + if err != nil { + return 0, 0, false + } + return major, minor, true +} + +// compareMajorMinor compares two "Major.Minor.Patch" versions on their major and +// minor components only, returning -1, 0 or 1. Unparseable input yields 0 so +// callers gate conservatively rather than acting on a bogus ordering. +func compareMajorMinor(a, b string) int { + aMajor, aMinor, aOK := majorMinor(a) + bMajor, bMinor, bOK := majorMinor(b) + if !aOK || !bOK { + return 0 + } + switch { + case aMajor != bMajor: + if aMajor < bMajor { + return -1 + } + return 1 + case aMinor != bMinor: + if aMinor < bMinor { + return -1 + } + return 1 + default: + return 0 + } +} + +// minorDistance reports how many minors separate two versions, counting a major +// bump as unbounded distance so a chain crossing a major is never mistaken for a +// narrow jump. Returns -1 when either version cannot be parsed. +func minorDistance(from, to string) int { + fromMajor, fromMinor, fromOK := majorMinor(from) + toMajor, toMinor, toOK := majorMinor(to) + if !fromOK || !toOK { + return -1 + } + if toMajor != fromMajor { + return math.MaxInt32 + } + return toMinor - fromMinor +} + +// majorMinorOf returns the "Major.Minor" prefix of a "Major.Minor.Patch" +// version string (e.g. "0.109.0" → "0.109"). Returns the input unchanged when +// it has fewer than two components. +func majorMinorOf(version string) string { + parts := strings.Split(version, ".") + if len(parts) < 2 { + return version + } + return parts[0] + "." + parts[1] +} + +// schemaUpgradeBlockedGetter returns a poll function reporting the DocumentDB's +// SchemaUpgradeBlocked condition, or nil when the condition is absent. A fetch +// error yields nil so Eventually keeps polling rather than failing outright. +func schemaUpgradeBlockedGetter( + ctx context.Context, + c client.Client, + key types.NamespacedName, +) func() *metav1.Condition { + return func() *metav1.Condition { + dd, err := shareddb.Get(ctx, c, key) + if err != nil { + return nil + } + for i := range dd.Status.Conditions { + if dd.Status.Conditions[i].Type == previewv1.ConditionSchemaUpgradeBlocked { + return &dd.Status.Conditions[i] + } + } + return nil + } +} + // replicaInstalledSchemaVersion execs psql on every replica pod of the // CNPG cluster backing the DocumentDB and returns their agreed installed // documentdb extension version, normalized to semver (e.g. "0.110.0"). diff --git a/test/e2e/tests/upgrade/upgrade_schema_multiversion_test.go b/test/e2e/tests/upgrade/upgrade_schema_multiversion_test.go new file mode 100644 index 000000000..c4ca2c5db --- /dev/null +++ b/test/e2e/tests/upgrade/upgrade_schema_multiversion_test.go @@ -0,0 +1,235 @@ +package upgrade + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "go.mongodb.org/mongo-driver/v2/bson" + "k8s.io/apimachinery/pkg/types" + + previewv1 "github.com/documentdb/documentdb-operator/api/preview" + "github.com/documentdb/documentdb-operator/test/e2e" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/assertions" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/documentdb" + e2emongo "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/mongo" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/namespaces" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/seed" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/timeouts" + shareddb "github.com/documentdb/documentdb-operator/test/shared/documentdb" + sharedmongo "github.com/documentdb/documentdb-operator/test/shared/mongo" +) + +// DocumentDB upgrade — schema across MORE THAN ONE minor version. +// +// upgrade_schema_test.go covers exactly one old→new hop. These specs cover the +// two gaps deferred from #439, both of which only became meaningful once the +// operator gained an update-path preflight (#448): +// +// 1. "multi-minor jump" — a single upgrade that skips intermediate minors +// (e.g. 0.110.0 → 0.113.0). The DocumentDB extension releases minors +// roughly monthly, so a user upgrading quarterly is routinely 3+ minors +// behind. This spec ratifies that such a jump is a SUPPORTED path: the +// operator resolves the chained update scripts in one ALTER EXTENSION +// UPDATE, the schema lands on the target, and data survives. If the +// extension ever ships a release without its update script, the preflight +// blocks the upgrade and this spec fails loudly — which is the point. +// +// 2. "sequential chain" — stepping through every version in the chain one at +// a time (0.109.0 → 0.110.0 → 0.113.0 → …), asserting the schema advances +// at each hop and the seeded data survives all of them cumulatively. This +// is the conservative upgrade style, and it exercises repeated +// rolling-restart + migrate cycles against a single volume. +// +// Both drive the upgrade through spec.documentDBVersion + spec.schemaVersion — +// the user-facing knobs — and read the versions from +// E2E_UPGRADE_DOCUMENTDB_VERSION_CHAIN (see documentDBVersionChain). +// +// These are the most expensive specs in the upgrade area (one cluster each, +// plus N rolling restarts), so they gate at the Lowest depth tier: they run in +// the full sweep (TEST_DEPTH=4) and are excluded from ordinary PR runs. +var _ = Describe("DocumentDB upgrade — schema across multiple minors", + Label(e2e.UpgradeLabel, e2e.DisruptiveLabel, e2e.SlowLabel), + e2e.LowestLevelLabel, + Serial, func() { + const ( + dbName = "upgrade_multiversion" + collName = "seed" + ) + + var ( + chain []string + ctx context.Context + cancel context.CancelFunc + ) + + BeforeEach(func() { + e2e.SkipUnlessLevel(e2e.Lowest) + skipUnlessUpgradeEnabled() + chain = documentDBVersionChain() + ctx, cancel = context.WithTimeout(context.Background(), imageRolloutTimeout) + DeferCleanup(func() { cancel() }) + }) + + // createAt provisions a fresh DocumentDB pinned to version, waits for + // Ready, and returns its namespaced key. Cleanup is registered for the + // caller. schemaVersion is left unset at creation (two-phase); each spec + // sets it explicitly when it wants a migration. + createAt := func(name, version string) types.NamespacedName { + env := e2e.SuiteEnv() + Expect(env).NotTo(BeNil(), "SuiteEnv must be initialized by SetupSuite") + c := env.Client + + ns := namespaces.NamespaceForSpec(e2e.UpgradeLabel) + createNamespace(ctx, c, ns) + createCredentialSecret(ctx, c, ns) + + vars := baseVars(name, ns, "2Gi") + // Drive the version via documentDBVersion, so the raw image fields + // must stay empty for the mixin to take effect. + vars["DOCUMENTDB_IMAGE"] = "" + vars["GATEWAY_IMAGE"] = "" + vars["DOCUMENTDB_VERSION"] = version + + dd, err := documentdb.Create(ctx, c, ns, name, documentdb.CreateOptions{ + Base: "documentdb", + Mixins: []string{"documentdb_version"}, + Vars: vars, + ManifestsRoot: manifestsRoot(), + }) + Expect(err).NotTo(HaveOccurred(), "create DocumentDB %s/%s at %s", ns, name, version) + DeferCleanup(func(ctx SpecContext) { + _ = shareddb.Delete(ctx, c, dd, 3*time.Minute) + }) + + key := types.NamespacedName{Namespace: ns, Name: name} + Eventually(assertions.AssertDocumentDBReady(ctx, c, key), + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Succeed(), "DocumentDB did not reach Ready on %s", version) + + Eventually(schemaVersionGetter(ctx, c, key), + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Equal(version), "initial schema version should be %s", version) + + return key + } + + // seedData writes the small dataset and returns once it is committed. + seedData := func(ns, name string) { + env := e2e.SuiteEnv() + handle, err := e2emongo.NewFromDocumentDB(ctx, env, ns, name) + Expect(err).NotTo(HaveOccurred(), "connect to DocumentDB gateway to seed") + defer func() { _ = handle.Close(ctx) }() + + inserted, err := sharedmongo.Seed(ctx, handle.Client(), dbName, collName, seed.SmallDataset()) + Expect(err).NotTo(HaveOccurred(), "seed %s.%s", dbName, collName) + Expect(inserted).To(Equal(seed.SmallDatasetSize)) + } + + // expectSeedIntact reconnects and asserts the seeded document count is + // unchanged — the data-survival half of every assertion below. + expectSeedIntact := func(ns, name, afterWhat string) { + env := e2e.SuiteEnv() + handle, err := e2emongo.NewFromDocumentDB(ctx, env, ns, name) + Expect(err).NotTo(HaveOccurred(), "reconnect to DocumentDB gateway after %s", afterWhat) + defer func() { _ = handle.Close(ctx) }() + + n, err := sharedmongo.Count(ctx, handle.Client(), dbName, collName, bson.M{}) + Expect(err).NotTo(HaveOccurred(), "count %s.%s after %s", dbName, collName, afterWhat) + Expect(n).To(Equal(int64(seed.SmallDatasetSize)), + "seeded document count changed across %s", afterWhat) + } + + // upgradeTo patches both knobs in one step and waits for the binary + // rollout, the schema migration, and Ready. + upgradeTo := func(key types.NamespacedName, version string) { + env := e2e.SuiteEnv() + c := env.Client + + fresh, err := shareddb.Get(ctx, c, key) + Expect(err).NotTo(HaveOccurred(), "re-fetch DocumentDB before upgrade to %s", version) + Expect(shareddb.PatchSpec(ctx, c, fresh, func(s *previewv1.DocumentDBSpec) { + s.DocumentDBVersion = version + s.SchemaVersion = version + })).To(Succeed(), "patch DocumentDB to version %s", version) + + Eventually(statusDocumentDBImageGetter(ctx, c, key), + timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(ContainSubstring(version), "status.documentDBImage did not advance to %s", version) + + Eventually(schemaVersionGetter(ctx, c, key), + timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(Equal(version), "schema did not migrate to %s", version) + + Eventually(assertions.AssertDocumentDBReady(ctx, c, key), + timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(Succeed(), "DocumentDB not Ready after upgrade to %s", version) + } + + It("migrates in a single step when the upgrade skips more than one minor", func() { + const ddName = "upgrade-multiminor" + + // Span the whole chain so the jump is as aggressive as the configured + // versions allow. + Expect(len(chain)).To(BeNumerically(">=", 2), + "%s must list at least two versions", envDocumentDBVersionChain) + from, to := chain[0], chain[len(chain)-1] + if minorDistance(from, to) < 2 { + Skip("configured version chain " + envOr(envDocumentDBVersionChain, defaultDocumentDBVersionChain) + + " does not span more than one minor; nothing to prove") + } + + env := e2e.SuiteEnv() + Expect(env).NotTo(BeNil(), "SuiteEnv must be initialized by SetupSuite") + c := env.Client + + By("creating a DocumentDB pinned to " + from + " and seeding data") + key := createAt(ddName, from) + seedData(key.Namespace, ddName) + + By("jumping straight to " + to + ", skipping the intermediate minors") + upgradeTo(key, to) + + By("verifying the operator did not report the jump as blocked") + // A missing update script anywhere in the from→to range would leave + // SchemaUpgradeBlocked=True and the schema behind; assert the + // condition explicitly so a regression names the cause rather than + // just timing out above. + cond := schemaUpgradeBlockedGetter(ctx, c, key)() + if cond != nil { + Expect(string(cond.Status)).To(Equal("False"), + "multi-minor jump %s → %s was blocked: %s", from, to, cond.Message) + } + + By("verifying seeded data survived the multi-minor migration") + expectSeedIntact(key.Namespace, ddName, "multi-minor jump "+from+" → "+to) + }) + + It("migrates step by step through every version in the chain", func() { + const ddName = "upgrade-chain" + + // The single-hop case is already covered by upgrade_schema_test.go; + // this spec only earns its cost with three or more versions. + if len(chain) < 3 { + Skip("configured version chain has fewer than 3 versions; " + + "the single-hop case is covered by upgrade_schema_test.go") + } + + By("creating a DocumentDB pinned to " + chain[0] + " and seeding data") + key := createAt(ddName, chain[0]) + seedData(key.Namespace, ddName) + + for _, version := range chain[1:] { + By("upgrading to " + version) + upgradeTo(key, version) + expectSeedIntact(key.Namespace, ddName, "upgrade to "+version) + } + }) + }) diff --git a/test/e2e/tests/upgrade/upgrade_schema_preflight_test.go b/test/e2e/tests/upgrade/upgrade_schema_preflight_test.go new file mode 100644 index 000000000..9cd7f87dc --- /dev/null +++ b/test/e2e/tests/upgrade/upgrade_schema_preflight_test.go @@ -0,0 +1,294 @@ +package upgrade + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "go.mongodb.org/mongo-driver/v2/bson" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + previewv1 "github.com/documentdb/documentdb-operator/api/preview" + "github.com/documentdb/documentdb-operator/test/e2e" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/assertions" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/documentdb" + e2emongo "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/mongo" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/namespaces" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/seed" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/timeouts" + shareddb "github.com/documentdb/documentdb-operator/test/shared/documentdb" + sharedmongo "github.com/documentdb/documentdb-operator/test/shared/mongo" +) + +// DocumentDB upgrade — schema migration failure (no update path). +// +// This is the third gap deferred from #439: "ALTER EXTENSION UPDATE fails". +// It was deferred because, before the preflight landed (#448), the failure was +// only reachable by shipping a doctored extension image with a hole in its +// update-script chain — and the resulting behavior was an un-ratified raw +// PostgreSQL error that re-fired on every reconcile. +// +// With the preflight in place the failure is reachable deterministically with +// stock images: request a schema version that is <= the binary version (so the +// validating webhook admits it) and > the installed version (so the operator +// actually plans a migration), but which the extension never released and +// therefore has no documentdb----.sql script for. A patch-level +// version such as 0.109.999 satisfies all three at once. +// +// The contract being pinned: +// +// - The operator does NOT fire ALTER EXTENSION UPDATE. +// - status.conditions gains SchemaUpgradeBlocked=True with reason +// NoUpdatePath, naming both versions. +// - status.schemaVersion does not move — no partial migration. +// - The cluster stays Ready and serving; data is intact. In particular the +// operator does not crash-loop the reconcile: we hold the assertions over +// a window rather than sampling once. +// - Correcting spec.schemaVersion to a real version clears the condition and +// completes the migration, so the block is recoverable, not terminal. +var _ = Describe("DocumentDB upgrade — schema migration blocked", + Label(e2e.UpgradeLabel, e2e.DisruptiveLabel, e2e.SlowLabel), + e2e.LowLevelLabel, + Serial, Ordered, func() { + const ( + ddName = "upgrade-blocked" + dbName = "upgrade_blocked" + collName = "seed" + + // A patch release the documentdb extension has never shipped. It is + // deliberately derived from oldVersion's minor so it sorts above the + // installed schema but below the new binary — see the file comment. + // .999 rather than a low number so the spec cannot silently invert if + // upstream ever ships more patch releases on an old minor. + unreachableSchemaSuffix = ".999" + ) + + var ( + oldVersion string + newVersion string + ctx context.Context + cancel context.CancelFunc + ns string + key types.NamespacedName + ) + + BeforeAll(func() { + // Gate before any cluster work: BeforeAll runs ahead of BeforeEach. + e2e.SkipUnlessLevel(e2e.Low) + skipUnlessUpgradeEnabled() + oldVersion = envOr(envOldDocumentDBVersion, defaultOldDocumentDBVersion) + newVersion = envOr(envNewDocumentDBVersion, defaultNewDocumentDBVersion) + if oldVersion == newVersion { + Skip(envOldDocumentDBVersion + " and " + envNewDocumentDBVersion + + " are identical; there is no upgrade to block") + } + // The unreachable target is built from oldVersion's major.minor plus a high + // patch, so it only stays below the binary version when the two versions + // differ by at least one minor. On a same-minor pair the validating webhook + // would reject the patch and the spec would fail for the wrong reason. + if compareMajorMinor(newVersion, oldVersion) <= 0 { + Skip(envOldDocumentDBVersion + " and " + envNewDocumentDBVersion + + " share a major.minor; cannot construct an unreachable patch version " + + "that the webhook will still admit") + } + + env := e2e.SuiteEnv() + Expect(env).NotTo(BeNil(), "SuiteEnv must be initialized by SetupSuite") + c := env.Client + + setupCtx, setupCancel := context.WithTimeout(context.Background(), imageRolloutTimeout) + DeferCleanup(func() { setupCancel() }) + + By("creating a DocumentDB pinned to the old version (schemaVersion unset → two-phase)") + ns = namespaces.NamespaceForSpec(e2e.UpgradeLabel) + createNamespace(setupCtx, c, ns) + createCredentialSecret(setupCtx, c, ns) + + vars := baseVars(ddName, ns, "2Gi") + vars["DOCUMENTDB_IMAGE"] = "" + vars["GATEWAY_IMAGE"] = "" + vars["DOCUMENTDB_VERSION"] = oldVersion + + dd, err := documentdb.Create(setupCtx, c, ns, ddName, documentdb.CreateOptions{ + Base: "documentdb", + Mixins: []string{"documentdb_version"}, + Vars: vars, + ManifestsRoot: manifestsRoot(), + }) + Expect(err).NotTo(HaveOccurred(), "create DocumentDB %s/%s", ns, ddName) + DeferCleanup(func(ctx SpecContext) { + _ = shareddb.Delete(ctx, c, dd, 3*time.Minute) + }) + + key = types.NamespacedName{Namespace: ns, Name: ddName} + Eventually(assertions.AssertDocumentDBReady(setupCtx, c, key), + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Succeed(), "DocumentDB did not reach Ready on oldVersion=%s", oldVersion) + }) + + BeforeEach(func() { + e2e.SkipUnlessLevel(e2e.Low) + ctx, cancel = context.WithTimeout(context.Background(), imageRolloutTimeout) + DeferCleanup(func() { cancel() }) + }) + + It("blocks the migration with an actionable condition instead of failing the ALTER", func() { + env := e2e.SuiteEnv() + Expect(env).NotTo(BeNil(), "SuiteEnv must be initialized by SetupSuite") + Expect(ctx).NotTo(BeNil(), "BeforeEach must have populated the spec context") + c := env.Client + + schemaVersion := schemaVersionGetter(ctx, c, key) + blocked := schemaUpgradeBlockedGetter(ctx, c, key) + + By("waiting for status.schemaVersion to settle on the old version") + Eventually(schemaVersion, + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Equal(oldVersion), "initial schema version should be %s", oldVersion) + + By("seeding data on the old schema") + handle, err := e2emongo.NewFromDocumentDB(ctx, env, ns, ddName) + Expect(err).NotTo(HaveOccurred(), "connect to DocumentDB gateway on oldVersion") + inserted, err := sharedmongo.Seed(ctx, handle.Client(), dbName, collName, seed.SmallDataset()) + Expect(err).NotTo(HaveOccurred(), "seed %s.%s", dbName, collName) + Expect(inserted).To(Equal(seed.SmallDatasetSize)) + Expect(handle.Close(ctx)).To(Succeed()) + + By("upgrading the binary to the new version, leaving the schema in two-phase") + fresh, err := shareddb.Get(ctx, c, key) + Expect(err).NotTo(HaveOccurred(), "re-fetch DocumentDB before binary upgrade") + Expect(shareddb.PatchSpec(ctx, c, fresh, func(s *previewv1.DocumentDBSpec) { + s.DocumentDBVersion = newVersion + })).To(Succeed(), "patch DocumentDBVersion from %s to %s", oldVersion, newVersion) + + Eventually(statusDocumentDBImageGetter(ctx, c, key), + timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(ContainSubstring(newVersion), "status.documentDBImage did not advance to %s", newVersion) + + Eventually(assertions.AssertDocumentDBReady(ctx, c, key), + timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(Succeed(), "DocumentDB did not reach Ready on newVersion=%s", newVersion) + + // e.g. old "0.109.0" → unreachable "0.109.7": above the installed + // schema, below the new binary, and never released — so no + // documentdb--0.109-0--0.109-7.sql exists. + unreachable := majorMinorOf(oldVersion) + unreachableSchemaSuffix + + By("requesting an unreachable schema version " + unreachable) + fresh2, err := shareddb.Get(ctx, c, key) + Expect(err).NotTo(HaveOccurred(), "re-fetch DocumentDB before unreachable schema patch") + Expect(shareddb.PatchSpec(ctx, c, fresh2, func(s *previewv1.DocumentDBSpec) { + s.SchemaVersion = unreachable + })).To(Succeed(), "patch schemaVersion to unreachable %s", unreachable) + + By("waiting for the operator to report SchemaUpgradeBlocked/NoUpdatePath") + Eventually(blocked, + timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).ShouldNot(BeNil(), "operator never set the %s condition", previewv1.ConditionSchemaUpgradeBlocked) + + Eventually(func() metav1.ConditionStatus { + cond := blocked() + if cond == nil { + return metav1.ConditionUnknown + } + return cond.Status + }, timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(Equal(metav1.ConditionTrue), "schema upgrade should be reported as blocked") + + cond := blocked() + Expect(cond).NotTo(BeNil()) + Expect(cond.Reason).To(Equal(previewv1.ReasonNoUpdatePath)) + Expect(cond.Message).To(ContainSubstring(oldVersion), + "blocked message should name the installed schema version") + Expect(cond.Message).To(ContainSubstring(unreachable), + "blocked message should name the requested target version") + + By("verifying the schema did not move and the block is stable (no crash-loop)") + // Holding the window matters: the pre-preflight behavior was to fire + // ALTER EXTENSION on every reconcile and error out each time. A + // stable schema across the window is the observable proof that the + // operator stopped cleanly instead of retrying. + Consistently(schemaVersion, + 60*time.Second, 5*time.Second, + ).Should(Equal(oldVersion), + "schema must stay at %s while the upgrade is blocked", oldVersion) + + Consistently(func() metav1.ConditionStatus { + c := blocked() + if c == nil { + return metav1.ConditionUnknown + } + return c.Status + }, 60*time.Second, 5*time.Second, + ).Should(Equal(metav1.ConditionTrue), "blocked condition should not flap") + + By("verifying the cluster stayed Ready and the data is intact") + Expect(assertions.AssertDocumentDBReady(ctx, c, key)()).To(Succeed(), + "DocumentDB should remain Ready while a schema upgrade is blocked") + + handle2, err := e2emongo.NewFromDocumentDB(ctx, env, ns, ddName) + Expect(err).NotTo(HaveOccurred(), "reconnect to DocumentDB gateway while blocked") + n, err := sharedmongo.Count(ctx, handle2.Client(), dbName, collName, bson.M{}) + Expect(err).NotTo(HaveOccurred(), "count %s.%s while blocked", dbName, collName) + Expect(n).To(Equal(int64(seed.SmallDatasetSize)), + "seeded document count changed while the upgrade was blocked") + Expect(handle2.Close(ctx)).To(Succeed()) + }) + + It("recovers and completes the migration once a reachable version is requested", func() { + env := e2e.SuiteEnv() + Expect(env).NotTo(BeNil(), "SuiteEnv must be initialized by SetupSuite") + Expect(ctx).NotTo(BeNil(), "BeforeEach must have populated the spec context") + c := env.Client + + schemaVersion := schemaVersionGetter(ctx, c, key) + blocked := schemaUpgradeBlockedGetter(ctx, c, key) + + By("correcting spec.schemaVersion to the real new version") + fresh, err := shareddb.Get(ctx, c, key) + Expect(err).NotTo(HaveOccurred(), "re-fetch DocumentDB before recovery patch") + Expect(shareddb.PatchSpec(ctx, c, fresh, func(s *previewv1.DocumentDBSpec) { + s.SchemaVersion = newVersion + })).To(Succeed(), "patch schemaVersion to %s", newVersion) + + By("waiting for the migration to complete") + Eventually(schemaVersion, + timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(Equal(newVersion), "schema did not migrate to %s after correcting the target", newVersion) + + By("verifying the blocked condition cleared") + Eventually(func() metav1.ConditionStatus { + cond := blocked() + if cond == nil { + return metav1.ConditionUnknown + } + return cond.Status + }, timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(Equal(metav1.ConditionFalse), "blocked condition should clear after a successful migration") + + Eventually(assertions.AssertDocumentDBReady(ctx, c, key), + timeouts.For(timeouts.DocumentDBUpgrade), + timeouts.PollInterval(timeouts.DocumentDBUpgrade), + ).Should(Succeed(), "DocumentDB not Ready after recovering the schema migration") + + By("verifying seeded data survived the blocked-then-recovered cycle") + handle, err := e2emongo.NewFromDocumentDB(ctx, env, ns, ddName) + Expect(err).NotTo(HaveOccurred(), "reconnect to DocumentDB gateway after recovery") + DeferCleanup(func(ctx SpecContext) { _ = handle.Close(ctx) }) + n, err := sharedmongo.Count(ctx, handle.Client(), dbName, collName, bson.M{}) + Expect(err).NotTo(HaveOccurred(), "count %s.%s after recovery", dbName, collName) + Expect(n).To(Equal(int64(seed.SmallDatasetSize)), + "seeded document count changed across the blocked-then-recovered migration") + }) + }) From 96997da8866fcbd5dbbbb7b34478c2bc1bb8d48f Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 27 Aug 2026 10:20:46 -0400 Subject: [PATCH 2/3] fix(ci): stop pipefail turning a found image into a setup failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `setup-test-environment` verified the loaded images with `docker images ... | grep -q "$IMAGE"`. Under the step's `set -o pipefail`, `grep -q` exits as soon as it matches, docker takes SIGPIPE, and the pipeline reports 141 — so a *successful* match fails the check. The race depends on how early the match appears in docker's output and how much output remains, which is why it fires intermittently and then fails every shard at once. Capture the image list into a variable and match against that, which removes the pipe entirely. Also switch to `grep -qxF` so the comparison is an exact literal line rather than a substring regex. The cert-manager `helm list | grep -q` check had the same shape and is fixed the same way. Repro of the old behavior: $ set -o pipefail $ seq 1 10000 | grep -q 1; echo $? 141 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- .../actions/setup-test-environment/action.yml | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/actions/setup-test-environment/action.yml b/.github/actions/setup-test-environment/action.yml index c084b01a2..205765fed 100644 --- a/.github/actions/setup-test-environment/action.yml +++ b/.github/actions/setup-test-environment/action.yml @@ -151,12 +151,18 @@ runs: echo " Operator: $OPERATOR_IMAGE" echo " Sidecar: $SIDECAR_IMAGE" - if ! docker images --format "table {{.Repository}}:{{.Tag}}" | grep -q "$OPERATOR_IMAGE"; then + # Capture the list first rather than piping into grep. Under + # `set -o pipefail`, `grep -q` exits as soon as it matches, which + # SIGPIPEs docker and makes the pipeline report failure even though the + # image was found. Matching against a variable removes that race. + LOADED_IMAGES="$(docker images --format '{{.Repository}}:{{.Tag}}')" + + if ! grep -qxF "$OPERATOR_IMAGE" <<< "$LOADED_IMAGES"; then echo "❌ Required operator image not found: $OPERATOR_IMAGE" exit 1 fi - if ! docker images --format "table {{.Repository}}:{{.Tag}}" | grep -q "$SIDECAR_IMAGE"; then + if ! grep -qxF "$SIDECAR_IMAGE" <<< "$LOADED_IMAGES"; then echo "❌ Required sidecar image not found: $SIDECAR_IMAGE" exit 1 fi @@ -428,8 +434,11 @@ runs: fi done - # Check if cert-manager is already installed - if helm list -n ${{ inputs.cert-manager-namespace }} 2>/dev/null | grep -q cert-manager; then + # Check if cert-manager is already installed. Capture first: piping into + # `grep -q` under `set -o pipefail` can SIGPIPE helm and misreport a match + # as a failure. + HELM_RELEASES="$(helm list -n ${{ inputs.cert-manager-namespace }} 2>/dev/null || true)" + if grep -q cert-manager <<< "$HELM_RELEASES"; then echo "cert-manager is already installed, skipping installation" echo "CERT_MANAGER_READY=true" >> $GITHUB_ENV else From 7fb8e0468ff5a3ca008f1598dc205f997dbbc2bd Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 27 Aug 2026 11:20:12 -0400 Subject: [PATCH 3/3] ci: expose the full depth tier set on the E2E workflow_dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TEST_DEPTH` accepts Highest|High|Medium|Low|Lowest (see test/e2e/levels.go), but the workflow_dispatch `depth` choice only offered Low|Medium|High. Specs declared at `level:lowest` — including the multi-version schema-upgrade specs added in this branch — were therefore unreachable from CI: they compile and are label-selected, but the runtime depth gate skips them at every tier the dispatch could request. List all five tiers. The default stays Medium, so scheduled and PR runs are unchanged; this only widens what a manual dispatch can ask for. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- .github/workflows/test-e2e.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index 757051026..6d4a76a9a 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -33,13 +33,15 @@ on: type: string default: '' depth: - description: 'Test depth tier (mapped to TEST_DEPTH; accepts Low|Medium|High)' + description: 'Test depth tier (mapped to TEST_DEPTH). Lowest is the deepest/full sweep; Highest is the shallowest.' required: false type: choice options: - - Low - - Medium + - Highest - High + - Medium + - Low + - Lowest default: Medium keep_clusters: description: 'Keep Kind clusters running after tests (for debugging)'