diff --git a/.claude/skills/prepare-new-service/SKILL.md b/.claude/skills/prepare-new-service/SKILL.md index ee629a312..b69b0cfe5 100644 --- a/.claude/skills/prepare-new-service/SKILL.md +++ b/.claude/skills/prepare-new-service/SKILL.md @@ -48,7 +48,14 @@ applies and which decisions need a Phase-0 spike: - **Database?** Which migration tool (alembic `db_sync` vs Django `migrate` vs none)? No DB ⇒ drop the database/db-sync/upgrade - sub-reconcilers entirely. A DB service also inherits the **dynamic + sub-reconcilers entirely. Which zero-downtime upgrade mechanism does + the manage tool support (expand-migrate-contract, single-pass + `db sync`, online data migrations), and which phases must straddle the + workload rollout? A database-backed service adopts the shared + `database.ReconcileUpgrade` flow (`internal/common/database/upgrade.go`), + keying it off the image tag (keystone) or `spec.openStackRelease` + (glance), or documents in its reference set why single-pass is + acceptable. A DB service also inherits the **dynamic credential chain**: the shared `credentialsMode` on `DatabaseSpec`, a per-service `databaseCredentialsMode` override on its c5c3 service spec, a `provision_service_tenant` leg in @@ -151,8 +158,8 @@ a second (or third) time?** Classify keystone internals into: 1. thin wrappers over `internal/common` — copy as pattern, fine; 2. generic logic living in keystone (pipeline/status machinery, workload builders, watch mappers, webhook validators) — **extraction candidates**; -3. genuinely keystone-specific (fernet, bootstrap, trust-flush, - expand-migrate-contract) — leave alone, rule of three. +3. genuinely keystone-specific (fernet, bootstrap, trust-flush) — leave + alone, rule of three. If category 2 is non-empty, file (or update) a **separate refactor issue** listing the candidates with file:line references, S/M/L effort, and a diff --git a/docs/.vitepress/config.ts b/docs/.vitepress/config.ts index 80de405dc..7a6c643e8 100644 --- a/docs/.vitepress/config.ts +++ b/docs/.vitepress/config.ts @@ -114,6 +114,7 @@ export default defineConfig({ { text: 'Backend CRD', link: '/reference/glance/glance-backend-crd' }, { text: 'Controller Events', link: '/reference/glance/glance-events' }, { text: 'Reconciler Architecture', link: '/reference/glance/glance-reconciler' }, + { text: 'Upgrade Flow', link: '/reference/glance/glance-upgrade-flow' }, ], }, { diff --git a/docs/reference/glance/glance-crd.md b/docs/reference/glance/glance-crd.md index cafd5984e..552c37047 100644 --- a/docs/reference/glance/glance-crd.md +++ b/docs/reference/glance/glance-crd.md @@ -115,16 +115,16 @@ owns via `spec.serviceUser.secretRef`. | `conditions` | List-map keyed by `type`; see the [reconciler reference](./glance-reconciler.md#conditions) for the vocabulary | | `observedGeneration` | The `.metadata.generation` last reconciled | | `endpoint` | The Glance API URL: `https://{gateway.hostname}/` when a gateway is set, otherwise the cluster-local Service URL | -| `installedRelease` | The OpenStack release whose schema is currently installed, promoted to `spec.openStackRelease` after a successful `db sync` | -| `targetRelease` | The `spec.openStackRelease` being converged to during an active release transition | +| `installedRelease` | The OpenStack release whose schema is currently installed, promoted to `spec.openStackRelease` after the upgrade completes (or after the first `db sync` on a fresh install) | +| `targetRelease` | The `spec.openStackRelease` being upgraded to during an active release transition. Set when the upgrade initiates, cleared on completion or abort; empty in steady state | +| `upgradePhase` | The current expand-migrate-contract phase during an active release upgrade (`Expanding`, `Migrating`, `RollingUpdate`, `Contracting`); empty when no upgrade is in flight | ::: info -Unlike Keystone, Glance carries **no `upgradePhase` field**: its migrations run -in a single `glance-manage db sync`, so a release transition is tracked only -through `installedRelease`/`targetRelease`, with no expand-migrate-contract -phase state. That is a trade-off, not only simpler status: without the phase -split, the outgoing release's pods serve against the already-migrated schema for -the duration of the roll — see [Database](./glance-reconciler.md#database). +A release transition (a `spec.openStackRelease` bump with the image in lockstep) +walks the shared expand-migrate-contract phase machine, tracked through +`upgradePhase`/`targetRelease`. See the +[Glance Upgrade Flow](./glance-upgrade-flow.md) for the phases, condition +reasons, events, and abort semantics. ::: ## Sub-Resource Naming Convention diff --git a/docs/reference/glance/glance-events.md b/docs/reference/glance/glance-events.md index 76af24e7e..4f4c0ec9b 100644 --- a/docs/reference/glance/glance-events.md +++ b/docs/reference/glance/glance-events.md @@ -84,13 +84,31 @@ All events follow these conventions: > schema-check Job, so — unlike Keystone — **`SchemaDriftDetected` never fires > for Glance**. -### Release Validation +### Upgrade + +A release transition (a `spec.openStackRelease` bump with the image in lockstep) +walks the shared expand-migrate-contract flow, which emits these events on the +Glance CR. For the phase machine see the +[Glance Upgrade Flow](./glance-upgrade-flow.md). | Reason | Type | Trigger Condition | Example Message | | --- | --- | --- | --- | -| `InvalidReleaseTransition` | Warning | The requested `spec.openStackRelease` is a downgrade, or a jump that is neither patch-only nor a single sequential step, relative to `status.installedRelease` | `downgrade from 2026.1 to 2025.2 is not supported` | - -**Source:** `rejectReleaseTransition` in `reconcile_database.go` +| `UpgradeInitiated` | Normal | An accepted release bump starts the upgrade | `Upgrade initiated: 2025.2 → 2026.1` | +| `ExpandComplete` | Normal | The expand phase Job succeeded | `Expand phase complete: 2025.2 → 2026.1` | +| `MigrateComplete` | Normal | The migrate phase Job succeeded | `Migrate phase complete: 2025.2 → 2026.1` | +| `DeploymentRolloutComplete` | Normal | The Deployment rolled out; the phase flips to Contracting | `Deployment rollout complete during upgrade 2025.2 → 2026.1` | +| `UpgradeComplete` | Normal | The contract phase Job succeeded; the upgrade finished | `Upgrade complete: 2025.2 → 2026.1` | +| `UpgradeAborted` | Normal | `spec.openStackRelease` reverted to the installed release, cancelling the upgrade | `Upgrade 2025.2 → 2026.1 aborted: spec release reverted to installed release 2025.2` | +| `VersionParseError` | Warning | The installed or target release is not a valid `YYYY.N` string | `Failed to parse target release "latest": ...` | +| `DowngradeNotSupported` | Warning | The target release is older than the installed release | `Downgrade from 2026.1 to 2025.2 is not supported` | +| `UpgradePathInvalid` | Warning | The requested jump is not a single sequential step | `Upgrade from 2024.2 to 2026.1 is not sequential` | +| `UpgradeTargetChanged` | Warning | `spec.openStackRelease` changed to a third value during an active upgrade | `Spec release changed to 2026.2 during active upgrade 2025.2 → 2026.1` | +| `ExpandFailed` | Warning | The expand phase Job failed permanently | `Expand job glance-db-expand failed: ...` | +| `MigrateFailed` | Warning | The migrate phase Job failed permanently | `Migrate job glance-db-migrate failed: ...` | +| `ContractFailed` | Warning | The contract phase Job failed permanently | `Contract job glance-db-contract failed: ...` | + +**Source:** the shared expand-migrate-contract flow in +`internal/common/database/upgrade.go` ### Finalization @@ -116,8 +134,8 @@ Use `kubectl get events --field-selector` to filter by reason: # Watch for db-sync failures kubectl get events --field-selector reason=DBSyncFailed -w -# Watch for a rejected release transition -kubectl get events --field-selector reason=InvalidReleaseTransition -w +# Watch for a rejected release upgrade path +kubectl get events --field-selector reason=UpgradePathInvalid -w # Watch for a skipped image-store backend kubectl get events --field-selector reason=GlanceBackendSkipped -w @@ -148,18 +166,18 @@ groups: summary: "Glance db-sync failed" description: "The Glance db-sync Job has failed. Check the Job logs for details." - - alert: GlanceInvalidReleaseTransition + - alert: GlanceUpgradePhaseFailed expr: | increase(kube_event_count{ - reason="InvalidReleaseTransition", + reason=~"ExpandFailed|MigrateFailed|ContractFailed", involved_object_kind="Glance" }[5m]) > 0 for: 0m labels: - severity: warning + severity: critical annotations: - summary: "Glance release transition rejected" - description: "spec.openStackRelease requests an unsupported transition from the installed release." + summary: "Glance database upgrade phase failed" + description: "An expand, migrate, or contract Job failed during a Glance release upgrade. Check the phase Job logs and consider aborting the upgrade." ``` --- @@ -180,9 +198,16 @@ GlanceReconciler.Reconcile() │ └─ spec.extraConfig overrides operator-owned keys → Warning ExtraConfigOwnedKeyOverride │ (gated on transition into ExtraConfigHealthy=False, Reason=OwnedKeysOverridden) │ - └── reconcileDatabase() - ├─ Invalid release transition → Warning InvalidReleaseTransition - ├─ db_sync fails → Warning DBSyncFailed - ├─ db_sync succeeds → Normal DatabaseSynced - └─ Job-UID patch fails → Warning DBSyncMetricEmissionDeferred + ├── reconcileDatabase() + │ ├─ db_sync fails → Warning DBSyncFailed + │ ├─ db_sync succeeds → Normal DatabaseSynced + │ ├─ Job-UID patch fails → Warning DBSyncMetricEmissionDeferred + │ └─ release upgrade → Normal UpgradeInitiated / ExpandComplete / + │ MigrateComplete / UpgradeComplete / UpgradeAborted + │ Warning VersionParseError / DowngradeNotSupported / + │ UpgradePathInvalid / UpgradeTargetChanged / + │ ExpandFailed / MigrateFailed / ContractFailed + │ + └── reconcileDeployment() + └─ rollout ready mid-upgrade → Normal DeploymentRolloutComplete ``` diff --git a/docs/reference/glance/glance-reconciler.md b/docs/reference/glance/glance-reconciler.md index f6e3d87ea..1feaad6de 100644 --- a/docs/reference/glance/glance-reconciler.md +++ b/docs/reference/glance/glance-reconciler.md @@ -33,8 +33,8 @@ Secrets ──► DBConnectionSecret ──► Backends ──► Config ── | DBConnectionSecret | Materializes the pymysql DSN into the derived `{name}-db-connection` Secret and digests it; **reports through `SecretsReady`** | `SecretsReady` | | Backends | Aggregates the attached, credential-ready `GlanceBackend`s into the content-hashed backends Secret. Waiting states never short-circuit the pipeline (first-install proceeds; a backend status flip re-enqueues via the watch) | `BackendsReady` | | Config | Renders `glance-api.conf` / `glance-api-paste.ini` (plus `policy.yaml` / `logging.conf` when applicable) into an immutable content-addressed ConfigMap. An invalid projection keeps the live Deployment's last-good names instead of re-rendering. **Reports through `SecretsReady`** (failure reason `ConfigError`) | `SecretsReady` | -| Database | Provisions/migrates the schema (MariaDB gate, `Database`/`User`/`Grant`, one `glance-manage db sync` Job); promotes `installedRelease` | `DatabaseReady` | -| Deployment | Ensures the API Deployment (both launch modes), Service (port 9292), and PDB; stamps `status.endpoint` | `DeploymentReady` | +| Database | Provisions/migrates the schema (MariaDB gate, `Database`/`User`/`Grant`, one `glance-manage db sync` Job); a release bump instead runs the shared expand-migrate-contract flow; promotes `installedRelease` | `DatabaseReady` | +| Deployment | Ensures the API Deployment (both launch modes), Service (port 9292), and PDB; stamps `status.endpoint`. Mid-upgrade it flips the `RollingUpdate` phase to `Contracting` once the rolled-out Deployment reports ready | `DeploymentReady` | | HTTPRoute | Full `spec.gateway` lifecycle; reflects the Gateway's Accepted condition | `HTTPRouteReady` | | HealthCheck | HTTP GET of the cluster-local `/healthcheck` through the shared TTL probe cache | `GlanceAPIReady` | | HPA | Creates/deletes the HorizontalPodAutoscaler | `HPAReady` | @@ -55,7 +55,7 @@ eight sub-conditions are `True`; otherwise `False` (`NotAllReady`). | --- | --- | --- | | `SecretsReady` | `SecretsAvailable` | `SecretStoreNotReady`, `WaitingForDBCredentials`, `WaitingForServiceUserCredentials`, `ConfigError` | | `BackendsReady` | `AllBackendsProjected` | `WaitingForBackends`, `NoDefaultBackend` | -| `DatabaseReady` | `DatabaseSynced` | `ClusterNotReady`, `WaitingForDatabase`, `DBSyncFailed`, `DBSyncInProgress`, `InvalidReleaseTransition`, `WaitingForBackends` | +| `DatabaseReady` | `DatabaseSynced` | `ClusterNotReady`, `WaitingForDatabase`, `DBSyncFailed`, `DBSyncInProgress`, `WaitingForBackends`, `ImageReleaseMismatch`, `VersionParseError`, `DowngradeNotSupported`, `UpgradePathInvalid`, `UpgradeTargetChanged`, `ExpandInProgress`, `MigrateInProgress`, `UpgradeRollingUpdate`, `ContractInProgress`, `ExpandFailed`, `MigrateFailed`, `ContractFailed` | | `DeploymentReady` | `DeploymentReady` | `WaitingForDeployment`, `WaitingForBackends` | | `GlanceAPIReady` | `APIHealthy` | `APIUnhealthy`, `EndpointNotReady`, `HealthCheckTimeout`, `ConnectionFailed`, `HealthCheckFailed` | | `HPAReady` | `HPAReady`, `HPANotRequired` | — (errors propagate) | @@ -116,27 +116,24 @@ The Database step runs the shared provisioning + sync flow: a MariaDB cluster gate and `Database`/`User`/`Grant` in managed mode (a no-op in brownfield), then a single `{name}-db-sync` Job running `glance-manage db sync` followed by `db load_metadefs`. Because Glance's migrations apply in one idempotent pass, -there is **no schema-check Job** — so, unlike Keystone, `SchemaDriftDetected` -never fires for Glance. On Job success `installedRelease` is promoted to -`spec.openStackRelease`. - -A release transition is validated against `installedRelease`: a downgrade, or a -jump that is neither patch-only nor a single sequential step, is rejected with -`DatabaseReady=False / InvalidReleaseTransition` and an `InvalidReleaseTransition` -Warning event. The rejection short-circuits the pipeline (non-zero requeue, no -error) so the Deployment is never rolled to new code against an un-migrated -schema; it stays rejected until the spec changes. - -**The reverse window is not closed.** `db sync` is a single pass with no -expand/migrate/contract split, and the pipeline runs `Database ──► Deployment`, -so on an accepted release bump the schema advances to the new release while the -outgoing release's pods keep serving — for the whole rolling update, and -`deployment.replicas` defaults to 3. Surviving pods therefore serve *old code -against the migrated schema*. A release transition is consequently **not a -zero-downtime operation**: drain or scale down before bumping -`spec.openStackRelease` if request failures during the roll are unacceptable. -There is no downgrade path to fall back to — `InvalidReleaseTransition` rejects -it. +there is **no schema-check Job**, so, unlike Keystone, `SchemaDriftDetected` +never fires for Glance. On fresh installs and patch bumps `installedRelease` is +promoted to `spec.openStackRelease` on Job success. + +A release transition (a `spec.openStackRelease` bump with the image in lockstep) +instead dispatches to the shared expand-migrate-contract flow +(`internal/common/database`), the same phase machine Keystone runs. The step +validates the path against `installedRelease`, rejecting a downgrade or a +non-sequential jump with `VersionParseError`, `DowngradeNotSupported`, or +`UpgradePathInvalid`, then walks `Expanding → Migrating → RollingUpdate → +Contracting` with `glance-manage db expand|migrate|contract` phase Jobs on the +new image. The outgoing pods keep serving the expanded schema across the +rollout, so the API stays available, and `installedRelease` is promoted only +after contract completes. The Deployment step owns the `RollingUpdate → +Contracting` flip: once `reconcileDeployment` sees the rolled-out Deployment +ready, it advances the phase to `Contracting` and requeues so the contract Job +runs. See the [Glance Upgrade Flow](./glance-upgrade-flow.md) for the phase +table, condition reasons, events, and abort semantics. ## Requeue semantics diff --git a/docs/reference/glance/glance-upgrade-flow.md b/docs/reference/glance/glance-upgrade-flow.md new file mode 100644 index 000000000..789af8204 --- /dev/null +++ b/docs/reference/glance/glance-upgrade-flow.md @@ -0,0 +1,316 @@ +--- +title: Glance Upgrade Flow +quadrant: operator +--- + +# Glance Upgrade Flow + +Reference documentation for the Glance expand-migrate-contract database upgrade +flow. When `spec.openStackRelease` advances to a new OpenStack release, the +operator runs phased database migrations while the Glance API keeps serving. + +For CRD type definitions (including the upgrade status fields), see +[Glance CRD](./glance-crd.md). For the sub-reconciler pipeline and condition +vocabulary, see [Glance Reconciler Architecture](./glance-reconciler.md). + +--- + +## Overview + +OpenStack services use the expand-migrate-contract pattern to upgrade a database +schema without taking the API down. The three phases let the old and new code +run against the same database while the schema moves forward: + +1. **Expand.** Add the new columns, tables, and triggers so the installed + release can still read and write while the new elements populate. +2. **Migrate.** Backfill and transform data into the new schema elements. +3. **Contract.** Drop the old columns, tables, and triggers the new release no + longer needs. + +Glance was the second operator to adopt this machine. The phase choreography +lives in `internal/common/database` (`upgrade.go`) and is shared with Keystone; +the Glance controller supplies the service-specific parts: the +`spec.openStackRelease` seam, the `glance-manage` phase commands, and the +`DatabaseReady` condition it reports on. Keystone's own behaviour is documented +in [Keystone Upgrade Flow](../keystone/keystone-upgrade-flow.md). + +The flow is driven by two sub-reconcilers. `reconcileDatabase` runs the expand, +migrate, and contract phases; `reconcileDeployment` owns the rolling update +between migrate and contract. + +--- + +## Trigger + +An upgrade starts when `spec.openStackRelease` moves one release forward, for +example `2025.2` to `2026.1`. Glance keys the upgrade off this field rather than +the image tag, so the image reference must be bumped in the same edit: the phase +Jobs run `spec.image`, and the new release's migration tree owns the schema +deltas. See [Image and Release Lockstep](#image-and-release-lockstep) for the +contract and its digest-pinning nuance. + +Bumping the image alone, with `spec.openStackRelease` unchanged, is not an +upgrade. It re-runs the single-pass sync; see +[Fresh Installs and Patch Bumps](#fresh-installs-and-patch-bumps). + +--- + +## Version Format + +`spec.openStackRelease` follows the OpenStack date-based scheme, two releases per +year: + +| Component | Format | Examples | +| --- | --- | --- | +| Release | `YYYY.N` where N is 1 or 2 | `2025.1`, `2025.2`, `2026.1` | + +The CRD pattern `^\d{4}\.[12]$`, the validating webhook, and +`release.ParseRelease` agree on this shape, so a non-cadence minor such as +`2025.9` is rejected at admission. + +### Accepted Transitions + +The operator accepts a same-release reconcile (a no-op for the schema) and a +single sequential step forward. Everything else is refused: + +| From | To | Accepted | Reason | +| --- | --- | --- | --- | +| `2025.1` | `2025.2` | Yes | Same year, minor +1 | +| `2025.2` | `2026.1` | Yes | Year +1, minor 2 to minor 1 | +| `2024.2` | `2026.1` | No | Skip-level (skips `2025.x`) | +| `2025.2` | `2026.2` | No | Skip-level (skips `2026.1`) | +| `2026.1` | `2025.2` | No | Downgrade | + +--- + +## Status Fields + +Three status fields track the upgrade, all written through the status +subresource. + +| Field | Type | During an upgrade | Steady state | +| --- | --- | --- | --- | +| `status.installedRelease` | `string` | The release installed **before** the upgrade began | The currently installed release | +| `status.targetRelease` | `string` | The release being upgraded **to** | Empty (`""`) | +| `status.upgradePhase` | `UpgradePhase` | The current phase (see below) | Empty (`""`) | + +`installedRelease` is promoted to `targetRelease` only after the contract phase +completes; it is the `Release` printer column in `kubectl get glances`. +`targetRelease` is set when the upgrade initiates and cleared on completion or +abort, and is not stamped on every steady-state pass. `upgradePhase` takes one +of four values while an upgrade is active: + +| Value | Meaning | +| --- | --- | +| `Expanding` | `glance-manage db expand` running on the new image | +| `Migrating` | `glance-manage db migrate` running on the new image | +| `RollingUpdate` | Waiting for the Deployment to roll out on the new image | +| `Contracting` | `glance-manage db contract` running on the new image | + +--- + +## Phases + +The upgrade walks a fixed sequence. Each transition is driven by a phase Job +completing or by the Deployment reporting ready. + +```text +spec.openStackRelease bumped (e.g. 2025.2 -> 2026.1, image in lockstep) + | + v + Expanding --- -db-expand: glance-manage db expand --- Job complete + | + v + Migrating --- -db-migrate: glance-manage db migrate --- Job complete + | + v + RollingUpdate --- Deployment rolls to the new image, waits for readiness + | + v + Contracting --- -db-contract: glance-manage db contract --- Job complete + | + v + installedRelease = "2026.1", targetRelease = "", upgradePhase = "" +``` + +Each job-running phase creates one distinctly named Job on `spec.image` (the new +release image) with `backoffLimit: 4`: + +| Phase | Job name | Command | +| --- | --- | --- | +| Expanding | `-db-expand` | `glance-manage --config-dir /etc/glance/glance-api.conf.d/ db expand` | +| Migrating | `-db-migrate` | `glance-manage --config-dir /etc/glance/glance-api.conf.d/ db migrate` | +| Contracting | `-db-contract` | `glance-manage --config-dir /etc/glance/glance-api.conf.d/ db contract` | + +Expand and migrate run with the new image because the target release's migration +tree owns the schema deltas: running expand with the old binary would leave the +contract step ahead of expand and fail the service's upgrade-order check. + +### Rolling Update and the Launch-Mode Flip + +`RollingUpdate` builds no Job. `reconcileDatabase` returns an empty result, the +pipeline proceeds to `reconcileDeployment`, and Kubernetes rolls the pods onto +the new image. Old pods keep serving the expanded schema until the new pods pass +their readiness checks, so the API stays available. + +The launch mode derives from `spec.openStackRelease`: the eventlet `glance-api` +server below `2026.1`, uWSGI from `2026.1` onward. A `2025.2` to `2026.1` +upgrade therefore switches the container command from eventlet to uWSGI during +this rollout. Both modes load the same two `--config-dir` roots; the reconciler +reference covers the [launch modes](./glance-reconciler.md#launch-modes) in +detail. + +Once the Deployment reports ready, `reconcileDeployment` flips the phase from +`RollingUpdate` to `Contracting`, emits `DeploymentRolloutComplete`, and +requeues so `reconcileDatabase` runs the contract Job on the next pass. The +status endpoint is not stamped on that flip pass. + +When the contract Job completes, `installedRelease` is promoted to +`targetRelease`, `targetRelease` and `upgradePhase` are cleared, and +`DatabaseReady` is set `True` with reason `DatabaseSynced`. + +--- + +## Condition Reasons + +Every phase reports through `DatabaseReady`; the upgrade adds no new condition +types. The message carries the source and target release strings, for example +`Migrate phase running: 2025.2 → 2026.1`. + +### In progress + +| Reason | Phase | +| --- | --- | +| `ExpandInProgress` | Expanding | +| `MigrateInProgress` | Migrating | +| `UpgradeRollingUpdate` | RollingUpdate | +| `ContractInProgress` | Contracting | + +### Failure + +| Reason | Cause | +| --- | --- | +| `VersionParseError` | The installed or target release is not a valid `YYYY.N` string | +| `DowngradeNotSupported` | The target release is older than the installed release | +| `UpgradePathInvalid` | The jump is not a single sequential step (skip-level) | +| `UpgradeTargetChanged` | `spec.openStackRelease` changed to a third value mid-upgrade | +| `ExpandFailed` | The expand Job failed past its backoff limit | +| `MigrateFailed` | The migrate Job failed past its backoff limit | +| `ContractFailed` | The contract Job failed past its backoff limit | + +A validation failure and a phase-Job failure both set `DatabaseReady=False`, +emit a Warning event, and return an error, so the controller backs off and +retries. The `UpgradeTargetChanged` guard means an upgrade that is already +running neither advances nor restarts when the spec target moves again: revert +`spec.openStackRelease` to `targetRelease` to continue, or to `installedRelease` +to abort. + +--- + +## Events + +The upgrade path emits these events on the Glance CR. The reasons come from +`internal/common/database` and are shared with Keystone. + +| Type | Reason | Trigger | +| --- | --- | --- | +| Normal | `UpgradeInitiated` | An accepted release bump starts the flow | +| Normal | `ExpandComplete` | The expand Job succeeded | +| Normal | `MigrateComplete` | The migrate Job succeeded | +| Normal | `DeploymentRolloutComplete` | The Deployment rolled out; phase flips to Contracting | +| Normal | `UpgradeComplete` | The contract Job succeeded; the upgrade finished | +| Normal | `UpgradeAborted` | The upgrade was aborted (see below) | +| Warning | `VersionParseError` | Unparseable installed or target release | +| Warning | `DowngradeNotSupported` | Target older than installed | +| Warning | `UpgradePathInvalid` | Non-sequential jump | +| Warning | `UpgradeTargetChanged` | Spec target changed mid-upgrade | +| Warning | `ExpandFailed` | The expand Job failed permanently | +| Warning | `MigrateFailed` | The migrate Job failed permanently | +| Warning | `ContractFailed` | The contract Job failed permanently | + +No events fire for the in-progress polling states, so a requeue loop does not +flood the event stream. + +--- + +## Aborting an Upgrade + +Revert `spec.openStackRelease` to the value in `status.installedRelease` while an +upgrade is active. The operator then: + +1. Deletes the `-db-expand`, `-db-migrate`, and `-db-contract` + Jobs (background propagation removes their Pods too). +2. Clears `status.upgradePhase` and `status.targetRelease`. +3. Emits a Normal `UpgradeAborted` event. +4. Requeues, so the next reconcile takes the steady-state `db sync` path and + restores `DatabaseReady` against the installed release. + +```bash +# Abort an in-flight upgrade by reverting the release to the installed one. +kubectl patch glance --type=merge \ + -p '{"spec":{"openStackRelease":""}}' +``` + +This is the escape hatch for an upgrade wedged on an expand or migrate Job that +cannot make progress. + +::: warning Abort is only safe before the contract phase +Expand and migrate are additive: they add columns and tables and backfill data +without dropping anything the installed release still reads, so the pre-contract +schema is a superset both releases run against. Aborting during `Expanding`, +`Migrating`, or `RollingUpdate` is therefore safe. Contract drops the columns the +new release no longer needs; aborting during `Contracting` can leave the +installed release pointed at a schema missing fields it expects. The operator +clears the upgrade state from any phase, so validate the database before relying +on a Contracting-phase abort. Once contract completes, reverting the release is a +downgrade, which the operator rejects. +::: + +--- + +## Image and Release Lockstep + +`spec.image` and `spec.openStackRelease` are separate fields — the release +drives tracking, the launch mode, and upgrade detection, while the phase Jobs and +the Deployment run `spec.image` — so digest pinning stays possible. The +operator's contract is that the two are bumped together, and for a tag-pinned +image the reconciler enforces it: when `spec.image.tag` parses as an OpenStack +release that differs from `spec.openStackRelease` (the patch suffix is ignored, +so `2026.1-p1` still matches `2026.1`), `DatabaseReady` goes `False` with reason +`ImageReleaseMismatch` and neither the upgrade nor the steady-state sync advances +until the image is bumped in lockstep. This closes the gap where bumping the +release alone would run the wrong `glance-manage` binary as a no-op and falsely +promote `installedRelease`. A digest-pinned image, or a tag that does not parse +as a release, carries no comparable release string and is trusted to match the +declared `spec.openStackRelease`. + +Keystone keys its upgrade off the image tag and skips release detection for a +digest-pinned image. Glance keys off `spec.openStackRelease`, which is always +set, so a digest-pinned Glance still upgrades: on a release bump the phase Jobs +run by digest. `spec.openStackRelease` also selects the launch mode, so a +digest-pinned image resolves both a schema target and a launch command. + +--- + +## Fresh Installs and Patch Bumps + +A fresh install (empty `status.installedRelease`) runs a single `-db-sync` +Job (`glance-manage db sync` followed by `db load_metadefs`) and sets +`installedRelease` to `spec.openStackRelease` on success. No expand, migrate, or +contract Jobs are created. + +A change that keeps `spec.openStackRelease` the same, such as bumping the image +to a patch build, is not an upgrade either. It stays on the single-pass sync: the +pod-spec-hash gate re-runs the `db-sync` Job on the new image, and +`glance-manage db sync` applies any pending migrations in one idempotent pass. + +--- + +## Post-Upgrade Metadata Definitions + +The phase Jobs do not touch Glance's metadata definitions. After contract +completes and `installedRelease` advances, the steady-state `-db-sync` Job +is rebuilt with the new image. Its pod-spec-hash gate re-runs it, and its +idempotent `db sync && db load_metadefs` pass inserts the new release's metadata +definitions. Namespaces already present are skipped, so the load only adds what +the new release ships. diff --git a/docs/reference/glance/index.md b/docs/reference/glance/index.md index 1c7873d96..95123957e 100644 --- a/docs/reference/glance/index.md +++ b/docs/reference/glance/index.md @@ -56,10 +56,12 @@ The v1 operator resolves the onboarding decisions as follows: - **`/healthcheck` probes.** Readiness and liveness both GET `/healthcheck`, served by the oslo healthcheck middleware without touching the database or Keystone, identical in both launch modes. -- **Plain `db sync`, no upgrade phases.** Glance's schema migrations run in a - single `glance-manage db sync` pass, so — unlike Keystone — there is no - expand-migrate-contract phase machine, no `spec`/`status` upgrade-phase field, - and no separate schema-check Job. +- **Expand-migrate-contract upgrades.** When `spec.openStackRelease` advances + to a new OpenStack release (with the image in lockstep), the operator drives + phased database migrations while the API keeps serving. Sequential-only + upgrade paths; a fresh install or a same-release image bump stays on the + single-pass `glance-manage db sync`. See + [Upgrade Flow](./glance-upgrade-flow.md). - **No live S3 probing by the operator.** The operator never connects to an S3 endpoint to validate a backend; it only resolves the credentials Secret and renders the store section. Bucket reachability is a runtime concern of the @@ -90,3 +92,5 @@ For a Glance CR named `{name}` the operator manages: controller emits - [Reconciler Architecture](./glance-reconciler.md) — the sub-reconciler pipeline, conditions, and requeue semantics +- [Upgrade Flow](./glance-upgrade-flow.md) — the expand-migrate-contract + release-upgrade machine diff --git a/docs/reference/keystone/keystone-upgrade-flow.md b/docs/reference/keystone/keystone-upgrade-flow.md index d37858352..582fa3b3d 100644 --- a/docs/reference/keystone/keystone-upgrade-flow.md +++ b/docs/reference/keystone/keystone-upgrade-flow.md @@ -30,7 +30,9 @@ coexist during the transition: The Keystone operator implements this as a state machine within the `reconcileDatabase` sub-reconciler, coordinated with `reconcileDeployment` for the rolling update phase -between migrate and contract. +between migrate and contract. The phase machine itself lives in +`internal/common/database` and is shared with the Glance operator; Keystone +supplies the image-tag seam and the `keystone-manage` phase commands. --- diff --git a/internal/common/database/doc.go b/internal/common/database/doc.go index cccfddce4..9b642e4df 100644 --- a/internal/common/database/doc.go +++ b/internal/common/database/doc.go @@ -16,6 +16,9 @@ // - ReconcileSyncJobs sequences the db-sync and schema-check migration Jobs // (built from the parameterized JobSetParams table) and promotes the // installed-release marker. +// - ReconcileUpgrade drives the expand-migrate-contract release upgrades +// shared by keystone and glance: it detects the upgrade, runs the phased +// migration Jobs, and aborts a reverted upgrade. // - ReconcileConnectionSecret reads the ESO-synced credentials Secret, // assembles the DSN, materialises the derived -db-connection Secret, // and returns the digest that rolls the pods on a credential rotation. diff --git a/internal/common/database/flow.go b/internal/common/database/flow.go index a39055c20..b0d4849a2 100644 --- a/internal/common/database/flow.go +++ b/internal/common/database/flow.go @@ -27,8 +27,8 @@ import ( // Condition reason constants for the steady-state database-readiness condition, // shared so every database-backed operator's condition uses the same -// vocabulary. The expand-migrate-contract upgrade reasons stay operator-private -// because they are parameterised by the upgrade phase name. +// vocabulary. The expand-migrate-contract upgrade reasons are shared too and +// live alongside the upgrade flow in upgrade.go. const ( ReasonClusterNotReady = "ClusterNotReady" ReasonWaitingForDatabase = "WaitingForDatabase" diff --git a/internal/common/database/upgrade.go b/internal/common/database/upgrade.go new file mode 100644 index 000000000..8c19d29c0 --- /dev/null +++ b/internal/common/database/upgrade.go @@ -0,0 +1,495 @@ +// SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company +// +// SPDX-License-Identifier: Apache-2.0 + +package database + +import ( + "context" + "fmt" + "strings" + "time" + + batchv1 "k8s.io/api/batch/v1" + 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/runtime" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/c5c3/forge/internal/common/conditions" + "github.com/c5c3/forge/internal/common/job" + "github.com/c5c3/forge/internal/common/release" + commonv1 "github.com/c5c3/forge/internal/common/types" +) + +// Condition and event reason constants for the expand-migrate-contract upgrade +// reasons shared by the upgrade flow. Unlike the steady-state Reason* set above, +// these drive the phased release-upgrade branch every database-backed operator +// runs through ReconcileUpgrade. The dynamic "{phase}InProgress"/"{phase}Failed" +// reasons runUpgradePhase computes from the phase name coincide with the +// matching constants here. +const ( + ReasonUpgradeTargetChanged = "UpgradeTargetChanged" + ReasonVersionParseError = "VersionParseError" + ReasonDowngradeNotSupported = "DowngradeNotSupported" + ReasonUpgradePathInvalid = "UpgradePathInvalid" + ReasonExpandInProgress = "ExpandInProgress" + ReasonExpandFailed = "ExpandFailed" + ReasonExpandComplete = "ExpandComplete" + ReasonMigrateInProgress = "MigrateInProgress" + ReasonMigrateFailed = "MigrateFailed" + ReasonMigrateComplete = "MigrateComplete" + ReasonContractInProgress = "ContractInProgress" + ReasonContractFailed = "ContractFailed" + ReasonUpgradeRollingUpdate = "UpgradeRollingUpdate" + ReasonUpgradeInitiated = "UpgradeInitiated" + ReasonUpgradeComplete = "UpgradeComplete" + ReasonUpgradeAborted = "UpgradeAborted" + ReasonDeploymentRolloutComplete = "DeploymentRolloutComplete" +) + +// upgradeJobSuffixes lists the BuildJob nameSuffixes for the Jobs created during +// the expand-migrate-contract upgrade flow. AbortUpgrade deletes exactly these +// (and not db-sync or schema-check, which belong to the steady-state path) so +// reverting an in-flight upgrade leaves no stale phase Jobs behind (#468). +var upgradeJobSuffixes = []string{"db-expand", "db-migrate", "db-contract"} + +// UpgradeFlowParams carries everything the expand-migrate-contract upgrade flow +// needs. The service-specific parts — the owner CR, the condition vocabulary, +// the spec-side release string, the phase Job builder, and the terminal-metric +// callback — are supplied by the caller; the phase choreography itself is +// identical across operators. The three status pointers (Phase, InstalledRelease, +// TargetRelease) are mutated in place so the caller persists them on the CR after +// the flow returns. +type UpgradeFlowParams struct { + Client client.Client + Scheme *runtime.Scheme + // Recorder records the upgrade lifecycle events on Owner. + Recorder record.EventRecorder + // Owner is the CR that owns the phase Jobs and receives the events. + Owner client.Object + // Conditions is the CR's condition slice, mutated in place. + Conditions *[]metav1.Condition + // Generation is stamped onto every condition the flow writes. + Generation int64 + // ConditionType is the readiness condition the flow reports on (for example + // "DatabaseReady"). + ConditionType string + // RequeueAfter is the requeue interval while a phase Job is in progress. + RequeueAfter time.Duration + // Phase is the CR's current upgrade phase, mutated in place as the flow + // advances (Expanding -> Migrating -> RollingUpdate -> Contracting -> ""). + Phase *commonv1.UpgradePhase + // InstalledRelease is the CR's installed-release marker, promoted to + // TargetRelease when the contract phase completes. + InstalledRelease *string + // TargetRelease is the CR's in-flight upgrade target, set on initiation and + // cleared on completion or abort. + TargetRelease *string + // SpecRelease is the release the spec currently requests (keystone: + // spec.image.tag; glance: spec.openStackRelease). It is the single seam the + // flow generalizes over. + SpecRelease string + // BuildPhaseJob builds the migration Job for the given phase. The caller pins + // the correct release image and manage command per phase; the flow only runs + // and observes the returned Job. + BuildPhaseJob func(phase commonv1.UpgradePhase) *batchv1.Job + // RecordTerminal, when non-nil, is invoked with the phase Job suffix + // ("db-expand"/"db-migrate"/"db-contract") and the Job observed this pass so + // the caller can emit its service-specific terminal metric. + RecordTerminal func(jobSuffix string, observed *batchv1.Job) +} + +// IsUpgrade returns true when specRelease differs from the installed release and +// the change requires the expand-migrate-contract upgrade flow. It returns false +// for fresh deployments (empty installed release), an empty spec release, the +// same release, and patch-only changes. On a parse error of either side it +// returns true so InitiateUpgrade surfaces the error with a proper condition. +func IsUpgrade(specRelease, installedRelease string) bool { + // A digest-pinned image carries no release string, so the release/upgrade + // machine has nothing to compare — skip release detection entirely. + if specRelease == "" { + return false + } + if installedRelease == "" { + return false + } + if specRelease == installedRelease { + return false + } + + from, err := release.ParseRelease(installedRelease) + if err != nil { + // Let InitiateUpgrade handle the error with proper conditions. + return true + } + to, err := release.ParseRelease(specRelease) + if err != nil { + return true + } + + return !release.IsPatchOnly(from, to) +} + +// InitiateUpgrade validates the upgrade path and sets the initial upgrade state. +// It parses the installed then the target release (reporting VersionParseError), +// rejects downgrades and non-sequential jumps, and on success stamps +// TargetRelease, sets Phase to Expanding, and requeues so ReconcileUpgrade runs +// the first phase on the next pass. +func InitiateUpgrade(ctx context.Context, p UpgradeFlowParams) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + from, err := release.ParseRelease(*p.InstalledRelease) + if err != nil { + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonVersionParseError, + Message: fmt.Sprintf("failed to parse installed release %q: %v", *p.InstalledRelease, err), + }) + p.Recorder.Eventf(p.Owner, corev1.EventTypeWarning, ReasonVersionParseError, "Failed to parse installed release %q: %v", *p.InstalledRelease, err) + return ctrl.Result{}, fmt.Errorf("parse installed release %q: %w", *p.InstalledRelease, err) + } + + to, err := release.ParseRelease(p.SpecRelease) + if err != nil { + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonVersionParseError, + Message: fmt.Sprintf("failed to parse target release %q: %v", p.SpecRelease, err), + }) + p.Recorder.Eventf(p.Owner, corev1.EventTypeWarning, ReasonVersionParseError, "Failed to parse target release %q: %v", p.SpecRelease, err) + return ctrl.Result{}, fmt.Errorf("parse target release %q: %w", p.SpecRelease, err) + } + + if release.IsDowngrade(from, to) { + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonDowngradeNotSupported, + Message: fmt.Sprintf("downgrade from %s to %s is not supported", from.Raw, to.Raw), + }) + p.Recorder.Eventf(p.Owner, corev1.EventTypeWarning, ReasonDowngradeNotSupported, "Downgrade from %s to %s is not supported", from.Raw, to.Raw) + return ctrl.Result{}, fmt.Errorf("downgrade from %s to %s is not supported", from.Raw, to.Raw) + } + + if !release.IsSequentialUpgrade(from, to) { + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonUpgradePathInvalid, + Message: fmt.Sprintf("upgrade from %s to %s is not sequential; only sequential upgrades are supported", from.Raw, to.Raw), + }) + p.Recorder.Eventf(p.Owner, corev1.EventTypeWarning, ReasonUpgradePathInvalid, "Upgrade from %s to %s is not sequential", from.Raw, to.Raw) + return ctrl.Result{}, fmt.Errorf("upgrade from %s to %s is not sequential; only sequential upgrades are supported", from.Raw, to.Raw) + } + + *p.TargetRelease = p.SpecRelease + *p.Phase = commonv1.UpgradePhaseExpanding + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonExpandInProgress, + Message: fmt.Sprintf("Upgrade detected: %s → %s (phase: Expanding)", from.Raw, to.Raw), + }) + + logger.Info("Upgrade detected", "from", from.Raw, "to", to.Raw, "phase", commonv1.UpgradePhaseExpanding) + p.Recorder.Eventf(p.Owner, corev1.EventTypeNormal, ReasonUpgradeInitiated, "Upgrade initiated: %s → %s", from.Raw, to.Raw) + return ctrl.Result{Requeue: true}, nil +} + +// ReconcileUpgrade drives an active upgrade. It first handles an abort (the spec +// release reverted to the installed release), then guards against the spec +// release changing to a third value mid-upgrade, then dispatches to the handler +// for the current phase. Expand, migrate, and contract run a Job via +// runUpgradePhase; RollingUpdate is a pass-through that lets the caller proceed +// to its Deployment reconcile. +func ReconcileUpgrade(ctx context.Context, p UpgradeFlowParams) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + // Abort path: reverting the spec release back to the installed release cancels + // the in-flight upgrade and unblocks a stuck or permanently failed + // expand/migrate phase (#468). Checked before the target-changed guard below + // because a revert also satisfies SpecRelease != TargetRelease and would + // otherwise return a hard error. + if p.SpecRelease == *p.InstalledRelease { + return AbortUpgrade(ctx, p) + } + + // Detect a spec-release change to a third value during an active upgrade. + if p.SpecRelease != *p.TargetRelease { + logger.Info("Spec release changed during active upgrade", + "targetRelease", *p.TargetRelease, + "newRelease", p.SpecRelease, + "phase", *p.Phase) + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonUpgradeTargetChanged, + Message: fmt.Sprintf("Spec release changed to %s during active upgrade %s → %s; complete or roll back the current upgrade first", + p.SpecRelease, *p.InstalledRelease, *p.TargetRelease), + }) + p.Recorder.Eventf(p.Owner, corev1.EventTypeWarning, ReasonUpgradeTargetChanged, "Spec release changed to %s during active upgrade %s → %s", p.SpecRelease, *p.InstalledRelease, *p.TargetRelease) + return ctrl.Result{}, fmt.Errorf("spec release changed during active upgrade: current upgrade targets %s but spec release is %s", + *p.TargetRelease, p.SpecRelease) + } + + switch *p.Phase { + case commonv1.UpgradePhaseExpanding: + return runUpgradePhase(ctx, p, upgradePhaseStep{ + name: "Expand", + jobSuffix: "db-expand", + phase: commonv1.UpgradePhaseExpanding, + failReason: ReasonExpandFailed, + onComplete: completeExpand, + }) + case commonv1.UpgradePhaseMigrating: + return runUpgradePhase(ctx, p, upgradePhaseStep{ + name: "Migrate", + jobSuffix: "db-migrate", + phase: commonv1.UpgradePhaseMigrating, + failReason: ReasonMigrateFailed, + onComplete: completeMigrate, + }) + case commonv1.UpgradePhaseRollingUpdate: + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonUpgradeRollingUpdate, + Message: fmt.Sprintf("Waiting for Deployment rollout: %s → %s", *p.InstalledRelease, *p.TargetRelease), + }) + return ctrl.Result{}, nil + case commonv1.UpgradePhaseContracting: + return runUpgradePhase(ctx, p, upgradePhaseStep{ + name: "Contract", + jobSuffix: "db-contract", + phase: commonv1.UpgradePhaseContracting, + failReason: ReasonContractFailed, + onComplete: completeContract, + }) + default: + return ctrl.Result{}, fmt.Errorf("unknown upgrade phase %q", *p.Phase) + } +} + +// upgradePhaseStep describes one job-running upgrade phase. The expand, migrate, +// and contract phases share an identical build/run/record/fail/requeue skeleton +// (runUpgradePhase) and differ only in the phase name, the phase constant the +// Job builder keys off, the failure event reason, and what completion does. +type upgradePhaseStep struct { + // name is the phase name ("Expand"); it derives the "{name}InProgress" and + // "{name}Failed" condition reasons and the failure event/error wording. + name string + // jobSuffix is the RecordTerminal key ("db-expand"). + jobSuffix string + // phase is the UpgradePhase constant passed to BuildPhaseJob so the caller + // builds the matching Job. + phase commonv1.UpgradePhase + // failReason is the Warning event reason emitted when the Job fails. + failReason string + // onComplete runs the phase-specific transition once the Job succeeds. + onComplete func(ctx context.Context, p UpgradeFlowParams) (ctrl.Result, error) +} + +// runUpgradePhase executes the shared build/run/record/fail/requeue skeleton for +// one job-running upgrade phase, delegating the phase-specific transition to +// step.onComplete. +// +// The phase Job is built by the caller's BuildPhaseJob, which runs it with the +// target release: per the OpenStack rolling-upgrade procedure the N+1 (target) +// migration tree owns the schema deltas, so expand and migrate are executed with +// the new binary. Running expand with the old binary only advances the expand +// head to the old release's HEAD, and a later contract run with the new binary +// then fails the service's upgrade-order validation. +func runUpgradePhase(ctx context.Context, p UpgradeFlowParams, step upgradePhaseStep) (ctrl.Result, error) { + phaseJob := p.BuildPhaseJob(step.phase) + done, observed, err := job.RunJob(ctx, p.Client, p.Scheme, p.Owner, phaseJob) + // Emit db_sync metrics for the phase Job so the dashboard panel and + // failure-rate alerts continue to observe activity during upgrades. The Job + // RunJob already read is threaded in to avoid a re-Get. + if p.RecordTerminal != nil { + p.RecordTerminal(step.jobSuffix, observed) + } + if err != nil { + setUpgradeJobFailed(p, step.name, phaseJob.Name, err) + p.Recorder.Eventf(p.Owner, corev1.EventTypeWarning, step.failReason, "%s job %s failed: %v", step.name, phaseJob.Name, err) + return ctrl.Result{}, fmt.Errorf("running %s job: %w", strings.ToLower(step.name), err) + } + if !done { + setUpgradePhaseRunning(p, step.name) + return ctrl.Result{RequeueAfter: p.RequeueAfter}, nil + } + return step.onComplete(ctx, p) +} + +// setUpgradePhaseRunning sets the readiness condition to False with reason +// "{phase}InProgress" for an upgrade phase that is currently executing. +func setUpgradePhaseRunning(p UpgradeFlowParams, phase string) { + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: phase + "InProgress", + Message: fmt.Sprintf("%s phase running: %s → %s", phase, *p.InstalledRelease, *p.TargetRelease), + }) +} + +// setUpgradeJobFailed sets the readiness condition to False with reason +// "{phase}Failed" when an upgrade job fails. +func setUpgradeJobFailed(p UpgradeFlowParams, phase, jobName string, err error) { + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: phase + "Failed", + Message: fmt.Sprintf("%s job %s failed: %v", phase, jobName, err), + }) +} + +// completeExpand transitions the upgrade from Expanding to Migrating after the +// expand Job succeeds. +func completeExpand(_ context.Context, p UpgradeFlowParams) (ctrl.Result, error) { + *p.Phase = commonv1.UpgradePhaseMigrating + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonMigrateInProgress, + Message: fmt.Sprintf("Expand complete, starting migrate: %s → %s", *p.InstalledRelease, *p.TargetRelease), + }) + p.Recorder.Eventf(p.Owner, corev1.EventTypeNormal, ReasonExpandComplete, "Expand phase complete: %s → %s", *p.InstalledRelease, *p.TargetRelease) + return ctrl.Result{Requeue: true}, nil +} + +// completeMigrate transitions the upgrade from Migrating to RollingUpdate after +// the migrate Job succeeds. +func completeMigrate(_ context.Context, p UpgradeFlowParams) (ctrl.Result, error) { + *p.Phase = commonv1.UpgradePhaseRollingUpdate + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: p.Generation, + Reason: ReasonUpgradeRollingUpdate, + Message: fmt.Sprintf("Migrate complete, waiting for Deployment rollout: %s → %s", *p.InstalledRelease, *p.TargetRelease), + }) + p.Recorder.Eventf(p.Owner, corev1.EventTypeNormal, ReasonMigrateComplete, "Migrate phase complete: %s → %s", *p.InstalledRelease, *p.TargetRelease) + return ctrl.Result{Requeue: true}, nil +} + +// completeContract finalizes the upgrade after the contract Job succeeds: it +// promotes TargetRelease to InstalledRelease, clears the upgrade state, and +// reports the readiness condition True. +func completeContract(ctx context.Context, p UpgradeFlowParams) (ctrl.Result, error) { + from := *p.InstalledRelease + to := *p.TargetRelease + *p.InstalledRelease = to + *p.TargetRelease = "" + *p.Phase = "" + conditions.SetCondition(p.Conditions, metav1.Condition{ + Type: p.ConditionType, + Status: metav1.ConditionTrue, + ObservedGeneration: p.Generation, + Reason: ReasonDatabaseSynced, + Message: fmt.Sprintf("Database schema is up to date (upgraded %s → %s)", from, to), + }) + p.Recorder.Eventf(p.Owner, corev1.EventTypeNormal, ReasonUpgradeComplete, "Upgrade complete: %s → %s", from, to) + log.FromContext(ctx).Info("Upgrade complete", "from", from, "to", to) + return ctrl.Result{}, nil +} + +// AbortUpgrade cancels an in-flight upgrade when the spec release is reverted to +// the installed release. It deletes the expand/migrate/contract Jobs, clears the +// upgrade phase and target release, emits an UpgradeAborted event, and requeues +// so the steady-state sync path takes over and re-derives readiness on the next +// pass. This is the only escape from an upgrade wedged on a stuck or permanently +// failed expand/migrate Job (#468). +// +// Aborting is only safe before the contract phase has run. Expand and migrate +// are additive by design: they create the new columns/tables and backfill data +// without dropping anything the installed release still reads, so the +// pre-contract schema is a superset both releases run against. Contract drops +// the now-unused columns; once it has started, the installed release would be +// pointed at a schema missing fields it expects, so a post-contract revert is +// not a safe rollback. The reverted release is the operator's responsibility to +// validate. The flow does not gate the abort on the current phase. +func AbortUpgrade(ctx context.Context, p UpgradeFlowParams) (ctrl.Result, error) { + logger := log.FromContext(ctx) + abortedTarget := *p.TargetRelease + abortedPhase := *p.Phase + + // Delete the phase Jobs before clearing status so a transient delete error + // leaves the upgrade state intact and the next reconcile retries the abort + // rather than dropping into the steady-state path with orphaned Jobs (#468). + if err := deleteUpgradeJobs(ctx, p); err != nil { + return ctrl.Result{}, fmt.Errorf("deleting upgrade jobs during abort: %w", err) + } + + *p.Phase = "" + *p.TargetRelease = "" + + logger.Info("Upgrade aborted: spec release reverted to installed release", + "installedRelease", *p.InstalledRelease, + "abortedTarget", abortedTarget, + "abortedPhase", abortedPhase) + p.Recorder.Eventf(p.Owner, corev1.EventTypeNormal, ReasonUpgradeAborted, + "Upgrade %s → %s aborted: spec release reverted to installed release %s", + *p.InstalledRelease, abortedTarget, *p.InstalledRelease) + return ctrl.Result{Requeue: true}, nil +} + +// deleteUpgradeJobs deletes the expand/migrate/contract Jobs owned by the CR. +// NotFound is tolerated so the call is idempotent across requeues, and Background +// propagation removes each Job's owned Pods too rather than orphaning them +// (#468). +func deleteUpgradeJobs(ctx context.Context, p UpgradeFlowParams) error { + logger := log.FromContext(ctx) + for _, suffix := range upgradeJobSuffixes { + jobName := fmt.Sprintf("%s-%s", p.Owner.GetName(), suffix) + j := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: jobName, + Namespace: p.Owner.GetNamespace(), + }, + } + err := p.Client.Delete(ctx, j, client.PropagationPolicy(metav1.DeletePropagationBackground)) + switch { + case apierrors.IsNotFound(err): + logger.V(1).Info("upgrade phase job already absent during abort", "job", jobName) + case err != nil: + return fmt.Errorf("deleting %s: %w", jobName, err) + default: + logger.V(1).Info("deleted upgrade phase job during abort", "job", jobName) + } + } + return nil +} + +// CompleteRollingUpdate transitions a RollingUpdate-phase upgrade to Contracting +// once the caller's Deployment rollout has completed. It is a no-op returning +// false for every other phase (including the empty steady-state phase). On the +// RollingUpdate phase it advances Phase to Contracting, emits the +// DeploymentRolloutComplete event, and returns true; the caller stamps its own +// DeploymentReady condition and requeues so ReconcileUpgrade runs the contract +// phase on the next pass. +func CompleteRollingUpdate(ctx context.Context, p UpgradeFlowParams) bool { + if *p.Phase != commonv1.UpgradePhaseRollingUpdate { + return false + } + *p.Phase = commonv1.UpgradePhaseContracting + p.Recorder.Eventf(p.Owner, corev1.EventTypeNormal, ReasonDeploymentRolloutComplete, "Deployment rollout complete during upgrade %s → %s", *p.InstalledRelease, *p.TargetRelease) + log.FromContext(ctx).Info("Deployment rollout complete, transitioning to contract phase", + "from", *p.InstalledRelease, "to", *p.TargetRelease) + return true +} diff --git a/internal/common/database/upgrade_test.go b/internal/common/database/upgrade_test.go new file mode 100644 index 000000000..a0f81c8f4 --- /dev/null +++ b/internal/common/database/upgrade_test.go @@ -0,0 +1,538 @@ +// SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company +// +// SPDX-License-Identifier: Apache-2.0 + +package database + +import ( + "context" + "errors" + "testing" + "time" + + . "github.com/onsi/gomega" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + "github.com/c5c3/forge/internal/common/job" + commonv1 "github.com/c5c3/forge/internal/common/types" +) + +// upgradeOwner is an owner CR named after flowInstance so the phase Jobs the +// flow builds ("-db-expand") and the Jobs AbortUpgrade deletes +// ("-db-expand") share one name, matching the real controller where +// the owner CR name IS the JobSetParams instance name. +func upgradeOwner() *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: flowInstance, Namespace: flowNamespace, UID: "upgrade-uid"}, + } +} + +// phaseJobBuilder returns a BuildPhaseJob closure that builds the expand/migrate/ +// contract Jobs for the keystone-shaped JobSetParams, mirroring how a caller +// pins the phase command and target-release image per phase. +func phaseJobBuilder(p JobSetParams) func(commonv1.UpgradePhase) *batchv1.Job { + return func(phase commonv1.UpgradePhase) *batchv1.Job { + switch phase { + case commonv1.UpgradePhaseExpanding: + return BuildJob(p, p.Image, "db-expand", []string{"keystone-manage", "db_sync", "--expand"}, 4) + case commonv1.UpgradePhaseMigrating: + return BuildJob(p, p.Image, "db-migrate", []string{"keystone-manage", "db_sync", "--migrate"}, 4) + case commonv1.UpgradePhaseContracting: + return BuildJob(p, p.Image, "db-contract", []string{"keystone-manage", "db_sync", "--contract"}, 4) + case commonv1.UpgradePhaseRollingUpdate: + // RollingUpdate runs no Job; the flow never calls BuildPhaseJob for it. + return nil + default: + return nil + } + } +} + +// completedPhaseJob seeds a finished phase Job whose stored re-run key matches +// the desired template, so RunJob observes it as done rather than re-creating it. +func completedPhaseJob(builder func(commonv1.UpgradePhase) *batchv1.Job, phase commonv1.UpgradePhase) *batchv1.Job { + pj := builder(phase) + return completedJob(pj.Name, job.PodSpecHash(&pj.Spec.Template)) +} + +// runningPhaseJob seeds a phase Job that exists but carries no terminal +// condition, so RunJob reports it as still in progress. +func runningPhaseJob(builder func(commonv1.UpgradePhase) *batchv1.Job, phase commonv1.UpgradePhase) *batchv1.Job { + pj := builder(phase) + return &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: pj.Name, Namespace: flowNamespace, + Annotations: map[string]string{job.PodSpecHashAnnotation: job.PodSpecHash(&pj.Spec.Template)}, + }, + } +} + +// failedPhaseJob seeds a permanently failed phase Job whose re-run key matches +// the desired template, so RunJob returns ErrJobFailed rather than re-creating it. +func failedPhaseJob(builder func(commonv1.UpgradePhase) *batchv1.Job, phase commonv1.UpgradePhase) *batchv1.Job { + pj := builder(phase) + return failedJob(pj.Name, job.PodSpecHash(&pj.Spec.Template)) +} + +// upgradeState holds the three mutable status fields the flow advances behind +// pointers, so a test can seed them and assert their final values. +type upgradeState struct { + phase commonv1.UpgradePhase + installed string + target string +} + +// upgradeParams assembles a keystone-shaped UpgradeFlowParams over st. +func upgradeParams(c client.Client, s *runtime.Scheme, owner client.Object, rec record.EventRecorder, conds *[]metav1.Condition, st *upgradeState, spec string) UpgradeFlowParams { + return UpgradeFlowParams{ + Client: c, + Scheme: s, + Recorder: rec, + Owner: owner, + Conditions: conds, + Generation: 1, + ConditionType: "DatabaseReady", + RequeueAfter: 30 * time.Second, + Phase: &st.phase, + InstalledRelease: &st.installed, + TargetRelease: &st.target, + SpecRelease: spec, + BuildPhaseJob: phaseJobBuilder(keystoneJobSet()), + } +} + +// --- IsUpgrade --- + +func TestIsUpgrade(t *testing.T) { + g := NewWithT(t) + cases := []struct { + name string + installed string + spec string + want bool + }{ + {"empty installed release is a fresh deploy", "", "2026.1", false}, + {"same release is no upgrade", "2025.2", "2025.2", false}, + {"empty spec release (digest-pinned) is no upgrade", "2025.2", "", false}, + {"patch-only bump is no upgrade", "2025.2", "2025.2-p1", false}, + {"sequential bump is an upgrade", "2025.2", "2026.1", true}, + {"skip-level bump is an upgrade", "2025.1", "2026.1", true}, + {"unparseable installed release defers to InitiateUpgrade", "latest", "2026.1", true}, + {"unparseable spec release defers to InitiateUpgrade", "2025.2", "latest", true}, + } + for _, tc := range cases { + g.Expect(IsUpgrade(tc.spec, tc.installed)).To(Equal(tc.want), tc.name) + } +} + +// --- InitiateUpgrade --- + +func TestInitiateUpgrade_HappyPath(t *testing.T) { + g := NewWithT(t) + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + st := upgradeState{phase: "", installed: "2025.2", target: ""} + p := upgradeParams(nil, nil, upgradeOwner(), rec, &conds, &st, "2026.1") + + res, err := InitiateUpgrade(context.Background(), p) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + // Target and phase are stamped for ReconcileUpgrade to pick up next pass. + g.Expect(st.target).To(Equal("2026.1")) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhaseExpanding)) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(ReasonExpandInProgress)) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonUpgradeInitiated))) +} + +func TestInitiateUpgrade_VersionParseErrorInstalled(t *testing.T) { + g := NewWithT(t) + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + st := upgradeState{phase: "", installed: "latest", target: ""} + p := upgradeParams(nil, nil, upgradeOwner(), rec, &conds, &st, "2026.1") + + res, err := InitiateUpgrade(context.Background(), p) + g.Expect(err).To(HaveOccurred()) + g.Expect(res.IsZero()).To(BeTrue()) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(ReasonVersionParseError)) + g.Expect(cond.Message).To(ContainSubstring("installed release")) + // The upgrade state is untouched on a validation failure. + g.Expect(st.phase).To(Equal(commonv1.UpgradePhase(""))) + g.Expect(st.target).To(BeEmpty()) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonVersionParseError))) +} + +func TestInitiateUpgrade_VersionParseErrorSpec(t *testing.T) { + g := NewWithT(t) + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + st := upgradeState{phase: "", installed: "2025.2", target: ""} + p := upgradeParams(nil, nil, upgradeOwner(), rec, &conds, &st, "latest") + + res, err := InitiateUpgrade(context.Background(), p) + g.Expect(err).To(HaveOccurred()) + g.Expect(res.IsZero()).To(BeTrue()) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(ReasonVersionParseError)) + g.Expect(cond.Message).To(ContainSubstring("target release")) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhase(""))) + g.Expect(st.target).To(BeEmpty()) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonVersionParseError))) +} + +func TestInitiateUpgrade_Downgrade(t *testing.T) { + g := NewWithT(t) + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + st := upgradeState{phase: "", installed: "2026.1", target: ""} + p := upgradeParams(nil, nil, upgradeOwner(), rec, &conds, &st, "2025.2") + + res, err := InitiateUpgrade(context.Background(), p) + g.Expect(err).To(HaveOccurred()) + g.Expect(res.IsZero()).To(BeTrue()) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(ReasonDowngradeNotSupported)) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhase(""))) + g.Expect(st.target).To(BeEmpty()) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonDowngradeNotSupported))) +} + +func TestInitiateUpgrade_NonSequential(t *testing.T) { + g := NewWithT(t) + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + // 2025.1 -> 2026.1 skips 2025.2, so it is a forward but non-sequential jump. + st := upgradeState{phase: "", installed: "2025.1", target: ""} + p := upgradeParams(nil, nil, upgradeOwner(), rec, &conds, &st, "2026.1") + + res, err := InitiateUpgrade(context.Background(), p) + g.Expect(err).To(HaveOccurred()) + g.Expect(res.IsZero()).To(BeTrue()) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(ReasonUpgradePathInvalid)) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhase(""))) + g.Expect(st.target).To(BeEmpty()) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonUpgradePathInvalid))) +} + +// --- ReconcileUpgrade phase transitions --- + +func TestReconcileUpgrade_ExpandCompleteTransitionsToMigrating(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + builder := phaseJobBuilder(keystoneJobSet()) + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(owner, completedPhaseJob(builder, commonv1.UpgradePhaseExpanding)). + Build() + + st := upgradeState{phase: commonv1.UpgradePhaseExpanding, installed: "2025.2", target: "2026.1"} + res, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2026.1")) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhaseMigrating)) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(ReasonMigrateInProgress)) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonExpandComplete))) +} + +func TestReconcileUpgrade_MigrateCompleteTransitionsToRollingUpdate(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + builder := phaseJobBuilder(keystoneJobSet()) + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(owner, completedPhaseJob(builder, commonv1.UpgradePhaseMigrating)). + Build() + + st := upgradeState{phase: commonv1.UpgradePhaseMigrating, installed: "2025.2", target: "2026.1"} + res, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2026.1")) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhaseRollingUpdate)) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(ReasonUpgradeRollingUpdate)) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonMigrateComplete))) +} + +func TestReconcileUpgrade_RollingUpdatePassThrough(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + c := fake.NewClientBuilder().WithScheme(s).WithObjects(owner).Build() + + st := upgradeState{phase: commonv1.UpgradePhaseRollingUpdate, installed: "2025.2", target: "2026.1"} + res, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2026.1")) + g.Expect(err).NotTo(HaveOccurred()) + // RollingUpdate is a pass-through: zero result, phase unchanged, no Job run. + g.Expect(res.IsZero()).To(BeTrue()) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhaseRollingUpdate)) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(ReasonUpgradeRollingUpdate)) +} + +func TestReconcileUpgrade_ContractCompleteFinalizes(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + builder := phaseJobBuilder(keystoneJobSet()) + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(owner, completedPhaseJob(builder, commonv1.UpgradePhaseContracting)). + Build() + + st := upgradeState{phase: commonv1.UpgradePhaseContracting, installed: "2025.2", target: "2026.1"} + res, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2026.1")) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.IsZero()).To(BeTrue()) + // Target is promoted to installed and the upgrade state is cleared. + g.Expect(st.installed).To(Equal("2026.1")) + g.Expect(st.target).To(BeEmpty()) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhase(""))) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(ReasonDatabaseSynced)) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonUpgradeComplete))) +} + +func TestReconcileUpgrade_InProgressRequeues(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + builder := phaseJobBuilder(keystoneJobSet()) + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(owner, runningPhaseJob(builder, commonv1.UpgradePhaseExpanding)). + Build() + + st := upgradeState{phase: commonv1.UpgradePhaseExpanding, installed: "2025.2", target: "2026.1"} + res, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2026.1")) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.RequeueAfter).To(Equal(30 * time.Second)) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhaseExpanding)) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal("ExpandInProgress")) +} + +func TestReconcileUpgrade_ExpandJobFailed(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + builder := phaseJobBuilder(keystoneJobSet()) + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(owner, failedPhaseJob(builder, commonv1.UpgradePhaseExpanding)). + Build() + + st := upgradeState{phase: commonv1.UpgradePhaseExpanding, installed: "2025.2", target: "2026.1"} + res, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2026.1")) + g.Expect(err).To(HaveOccurred()) + g.Expect(res.IsZero()).To(BeTrue()) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal("ExpandFailed")) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonExpandFailed))) +} + +func TestReconcileUpgrade_RecordTerminalSuffixes(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + builder := phaseJobBuilder(keystoneJobSet()) + cases := []struct { + phase commonv1.UpgradePhase + suffix string + }{ + {commonv1.UpgradePhaseExpanding, "db-expand"}, + {commonv1.UpgradePhaseMigrating, "db-migrate"}, + {commonv1.UpgradePhaseContracting, "db-contract"}, + } + for _, tc := range cases { + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + var calls []string + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(owner, completedPhaseJob(builder, tc.phase)). + Build() + + st := upgradeState{phase: tc.phase, installed: "2025.2", target: "2026.1"} + p := upgradeParams(c, s, owner, rec, &conds, &st, "2026.1") + p.RecordTerminal = func(jobSuffix string, _ *batchv1.Job) { + calls = append(calls, jobSuffix) + } + _, err := ReconcileUpgrade(context.Background(), p) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(calls).To(Equal([]string{tc.suffix}), string(tc.phase)) + } + + // A nil RecordTerminal must not panic on the same completed-Job path. + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(owner, completedPhaseJob(builder, commonv1.UpgradePhaseExpanding)). + Build() + st := upgradeState{phase: commonv1.UpgradePhaseExpanding, installed: "2025.2", target: "2026.1"} + p := upgradeParams(c, s, owner, rec, &conds, &st, "2026.1") + p.RecordTerminal = nil + g.Expect(func() { + _, _ = ReconcileUpgrade(context.Background(), p) + }).NotTo(Panic()) +} + +// --- Abort --- + +func TestReconcileUpgrade_AbortOnSpecRevert(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + // Seed all three phase Jobs so the abort has something to delete. + c := fake.NewClientBuilder().WithScheme(s). + WithObjects( + owner, + phaseJobStub("db-expand"), + phaseJobStub("db-migrate"), + phaseJobStub("db-contract"), + ).Build() + + // SpecRelease reverted to the installed release triggers the abort. + st := upgradeState{phase: commonv1.UpgradePhaseExpanding, installed: "2025.2", target: "2026.1"} + res, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2025.2")) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhase(""))) + g.Expect(st.target).To(BeEmpty()) + + // All three phase Jobs were deleted. + for _, suffix := range []string{"db-expand", "db-migrate", "db-contract"} { + j := &batchv1.Job{} + err := c.Get(context.Background(), client.ObjectKey{Name: flowInstance + "-" + suffix, Namespace: flowNamespace}, j) + g.Expect(err).To(HaveOccurred(), suffix) + } + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonUpgradeAborted))) +} + +func TestAbortUpgrade_DeleteErrorKeepsState(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + boom := errors.New("apiserver unavailable") + c := fake.NewClientBuilder().WithScheme(s).WithObjects(owner). + WithInterceptorFuncs(interceptor.Funcs{ + Delete: func(_ context.Context, _ client.WithWatch, _ client.Object, _ ...client.DeleteOption) error { + return boom + }, + }).Build() + + st := upgradeState{phase: commonv1.UpgradePhaseExpanding, installed: "2025.2", target: "2026.1"} + res, err := AbortUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2025.2")) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("deleting upgrade jobs during abort")) + g.Expect(res.IsZero()).To(BeTrue()) + // The delete error leaves the upgrade state intact so the next pass retries. + g.Expect(st.phase).To(Equal(commonv1.UpgradePhaseExpanding)) + g.Expect(st.target).To(Equal("2026.1")) +} + +// --- Guards --- + +func TestReconcileUpgrade_TargetChanged(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + c := fake.NewClientBuilder().WithScheme(s).WithObjects(owner).Build() + + // A third spec release, distinct from both installed and the active target. + st := upgradeState{phase: commonv1.UpgradePhaseExpanding, installed: "2025.2", target: "2026.1"} + res, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2026.2")) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("spec release changed during active upgrade")) + g.Expect(res.IsZero()).To(BeTrue()) + cond := meta.FindStatusCondition(conds, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(ReasonUpgradeTargetChanged)) + // The guard is inert: it changes no upgrade state. + g.Expect(st.phase).To(Equal(commonv1.UpgradePhaseExpanding)) + g.Expect(st.target).To(Equal("2026.1")) + g.Expect(st.installed).To(Equal("2025.2")) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonUpgradeTargetChanged))) +} + +func TestReconcileUpgrade_UnknownPhase(t *testing.T) { + g := NewWithT(t) + s := flowScheme() + owner := upgradeOwner() + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + c := fake.NewClientBuilder().WithScheme(s).WithObjects(owner).Build() + + st := upgradeState{phase: commonv1.UpgradePhase("Bogus"), installed: "2025.2", target: "2026.1"} + _, err := ReconcileUpgrade(context.Background(), upgradeParams(c, s, owner, rec, &conds, &st, "2026.1")) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(`unknown upgrade phase "Bogus"`)) +} + +// --- CompleteRollingUpdate --- + +func TestCompleteRollingUpdate(t *testing.T) { + g := NewWithT(t) + + // Every phase but RollingUpdate is a no-op that emits no event. + for _, phase := range []commonv1.UpgradePhase{ + "", + commonv1.UpgradePhaseExpanding, + commonv1.UpgradePhaseMigrating, + commonv1.UpgradePhaseContracting, + } { + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + st := upgradeState{phase: phase, installed: "2025.2", target: "2026.1"} + p := upgradeParams(nil, nil, upgradeOwner(), rec, &conds, &st, "2026.1") + g.Expect(CompleteRollingUpdate(context.Background(), p)).To(BeFalse(), string(phase)) + g.Expect(st.phase).To(Equal(phase), string(phase)) + g.Expect(rec.Events).NotTo(Receive()) + } + + // RollingUpdate advances to Contracting and emits DeploymentRolloutComplete. + var conds []metav1.Condition + rec := record.NewFakeRecorder(10) + st := upgradeState{phase: commonv1.UpgradePhaseRollingUpdate, installed: "2025.2", target: "2026.1"} + p := upgradeParams(nil, nil, upgradeOwner(), rec, &conds, &st, "2026.1") + g.Expect(CompleteRollingUpdate(context.Background(), p)).To(BeTrue()) + g.Expect(st.phase).To(Equal(commonv1.UpgradePhaseContracting)) + g.Expect(rec.Events).To(Receive(ContainSubstring(ReasonDeploymentRolloutComplete))) +} + +// phaseJobStub builds a bare phase Job named "-" for the abort +// tests, which only need the Jobs to exist so the delete has a target. +func phaseJobStub(suffix string) *batchv1.Job { + return &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Name: flowInstance + "-" + suffix, Namespace: flowNamespace}, + } +} diff --git a/internal/common/types/types.go b/internal/common/types/types.go index 75b452a87..b5ad6abb6 100644 --- a/internal/common/types/types.go +++ b/internal/common/types/types.go @@ -210,6 +210,17 @@ func (t *DatabaseTLSSpec) IsEnabled() bool { return t != nil && t.Mode != "" && t.Mode != "disabled" } +// UpgradePhase represents the current phase of a database upgrade. +// +kubebuilder:validation:Enum=Expanding;Migrating;RollingUpdate;Contracting +type UpgradePhase string + +const ( + UpgradePhaseExpanding UpgradePhase = "Expanding" + UpgradePhaseMigrating UpgradePhase = "Migrating" + UpgradePhaseRollingUpdate UpgradePhase = "RollingUpdate" + UpgradePhaseContracting UpgradePhase = "Contracting" +) + // CacheSpec supports managed (ClusterRef) and brownfield (explicit) modes. // Exactly one of ClusterRef or Servers must be set; the XValidation rule below // enforces that invariant at the schema layer for every operator that embeds a diff --git a/operators/glance/api/v1alpha1/glance_types.go b/operators/glance/api/v1alpha1/glance_types.go index ed662f139..a5619a521 100644 --- a/operators/glance/api/v1alpha1/glance_types.go +++ b/operators/glance/api/v1alpha1/glance_types.go @@ -342,6 +342,11 @@ type GlanceStatus struct { // TargetRelease is the spec.openStackRelease being converged to during an // active upgrade. TargetRelease string `json:"targetRelease,omitempty"` + + // UpgradePhase is the current expand-migrate-contract phase during an active + // release upgrade (Expanding, Migrating, RollingUpdate, Contracting); empty + // when no upgrade is in flight. + UpgradePhase commonv1.UpgradePhase `json:"upgradePhase,omitempty"` } // EffectiveKeystonePublicEndpoint resolves the [keystone_authtoken] diff --git a/operators/glance/config/crd/bases/glance.openstack.c5c3.io_glances.yaml b/operators/glance/config/crd/bases/glance.openstack.c5c3.io_glances.yaml index 207fd65c3..2b6d874a7 100644 --- a/operators/glance/config/crd/bases/glance.openstack.c5c3.io_glances.yaml +++ b/operators/glance/config/crd/bases/glance.openstack.c5c3.io_glances.yaml @@ -1570,6 +1570,17 @@ spec: TargetRelease is the spec.openStackRelease being converged to during an active upgrade. type: string + upgradePhase: + description: |- + UpgradePhase is the current expand-migrate-contract phase during an active + release upgrade (Expanding, Migrating, RollingUpdate, Contracting); empty + when no upgrade is in flight. + enum: + - Expanding + - Migrating + - RollingUpdate + - Contracting + type: string type: object type: object served: true diff --git a/operators/glance/helm/glance-operator/crds/glance.openstack.c5c3.io_glances.yaml b/operators/glance/helm/glance-operator/crds/glance.openstack.c5c3.io_glances.yaml index c9e7ae5b5..9166f69ef 100644 --- a/operators/glance/helm/glance-operator/crds/glance.openstack.c5c3.io_glances.yaml +++ b/operators/glance/helm/glance-operator/crds/glance.openstack.c5c3.io_glances.yaml @@ -1573,6 +1573,17 @@ spec: TargetRelease is the spec.openStackRelease being converged to during an active upgrade. type: string + upgradePhase: + description: |- + UpgradePhase is the current expand-migrate-contract phase during an active + release upgrade (Expanding, Migrating, RollingUpdate, Contracting); empty + when no upgrade is in flight. + enum: + - Expanding + - Migrating + - RollingUpdate + - Contracting + type: string type: object type: object served: true diff --git a/operators/glance/internal/controller/integration_test.go b/operators/glance/internal/controller/integration_test.go index e96f4f658..917fea355 100644 --- a/operators/glance/internal/controller/integration_test.go +++ b/operators/glance/internal/controller/integration_test.go @@ -451,3 +451,231 @@ func TestIntegrationGlance_DeleteReleasesFinalizer(t *testing.T) { return apierrors.IsNotFound(err) }, eventuallyTimeout, pollInterval).Should(BeTrue(), "Glance should be fully removed after finalizer release") } + +// TestIntegrationGlance_UpgradeCycle_ExpandMigrateContract drives a full glance +// release upgrade (2025.2 → 2026.1) through the shared expand-migrate-contract +// flow end to end against envtest, mirroring keystone's upgrade-cycle test. It +// locks four properties no unit test observes together: +// +// - the Expanding → Migrating → RollingUpdate → Contracting phase walk, with +// each glance-manage db expand|migrate|contract phase Job carrying the +// target-release image; +// - the rollout-gated contract flip: the phase stays RollingUpdate until the +// re-imaged Deployment reports ready, then advances to Contracting; +// - the eventlet → uWSGI launch-command switch the Deployment template takes on +// the release bump (glanceUsesUWSGI derives the mode from openStackRelease); +// - the post-upgrade steady-state db-sync Job re-running with the new image on +// pod-spec-hash drift, so its db load_metadefs step loads the new release's +// definitions. +// +// envtest runs no Job or Deployment controllers, so every Job completion and +// Deployment readiness is simulated; the phase machine cannot advance on its own. +func TestIntegrationGlance_UpgradeCycle_ExpandMigrateContract(t *testing.T) { + testutil.SkipIfEnvTestUnavailable(t) + g := NewGomegaWithT(t) + + c, ctx, _ := setupEnvTestWithController(t) + ns := createTestNamespace(t, ctx, c) + createGlancePrerequisites(t, ctx, c, ns) + + // Brownfield glance at release 2025.2 with spec.openStackRelease and + // spec.image.tag in lockstep (the operator's existing bump contract). + glance := integrationGlance("glance", ns) + g.Expect(c.Create(ctx, glance)).To(Succeed(), "create Glance CR") + glanceKey := types.NamespacedName{Name: "glance", Namespace: ns} + + // A default backend plus its credentials must exist before any config is + // rendered and the schema/Deployment can converge. + g.Expect(c.Create(ctx, integrationS3CredentialsSecret("store", ns))).To(Succeed(), "create S3 credentials Secret") + g.Expect(c.Create(ctx, integrationBackend("store", ns, "glance", true))).To(Succeed(), "create GlanceBackend CR") + waitForGlanceCondition(t, ctx, c, glanceKey, "BackendsReady", metav1.ConditionTrue, eventuallyTimeout) + + deployKey := client.ObjectKey{Namespace: ns, Name: "glance"} + dbSyncKey := client.ObjectKey{Namespace: ns, Name: "glance-db-sync"} + + // Drive the initial 2025.2 install to Ready: simulate the db-sync Job complete + // and the Deployment ready, exactly as the file's other tests do. + g.Eventually(func() error { + return c.Get(ctx, dbSyncKey, &batchv1.Job{}) + }, eventuallyTimeout, pollInterval).Should(Succeed(), "db-sync Job should appear") + g.Expect(simulators.SimulateJobComplete(ctx, c, dbSyncKey)).To(Succeed(), "simulate db-sync Job completion") + + var deploy appsv1.Deployment + g.Eventually(func() error { + return c.Get(ctx, deployKey, &deploy) + }, eventuallyLongTimeout, pollInterval).Should(Succeed(), "Glance Deployment should appear") + g.Expect(simulators.SimulateDeploymentReady(ctx, c, deployKey, ptr.Deref(deploy.Spec.Replicas, 1))). + To(Succeed(), "simulate initial Deployment readiness") + + waitForGlanceCondition(t, ctx, c, glanceKey, "DatabaseReady", metav1.ConditionTrue, eventuallyLongTimeout) + waitForGlanceCondition(t, ctx, c, glanceKey, "Ready", metav1.ConditionTrue, eventuallyLongTimeout) + + // The initial release is tracked, no upgrade is in flight, and the Deployment + // runs the pre-2026.1 eventlet launch (glance-api server, no uWSGI). + initial := &glancev1alpha1.Glance{} + g.Expect(c.Get(ctx, glanceKey, initial)).To(Succeed()) + g.Expect(initial.Status.InstalledRelease).To(Equal("2025.2"), + "installedRelease should be 2025.2 after the initial install") + g.Expect(string(initial.Status.UpgradePhase)).To(Equal(""), + "no upgrade should be in flight after a fresh install") + g.Expect(c.Get(ctx, deployKey, &deploy)).To(Succeed()) + initialCmd := deploy.Spec.Template.Spec.Containers[0].Command + g.Expect(initialCmd).To(ContainElement("glance-api"), + "2025.2 Deployment should run the eventlet glance-api launch") + g.Expect(initialCmd).NotTo(ContainElement("uwsgi"), + "2025.2 Deployment must not run the uWSGI launch") + + // --- Trigger the upgrade: bump spec.openStackRelease and spec.image.tag in + // lockstep. The Get→Update loop retries on the conflict a concurrent status + // write races in. --- + g.Eventually(func() error { + cur := &glancev1alpha1.Glance{} + if err := c.Get(ctx, glanceKey, cur); err != nil { + return err + } + cur.Spec.OpenStackRelease = "2026.1" + cur.Spec.Image.Tag = "2026.1" + return c.Update(ctx, cur) + }, eventuallyTimeout, pollInterval).Should(Succeed(), "bump glance to release 2026.1") + + // Phase 1: Expanding — the db-expand Job carries the target-release image and + // the glance-manage db expand command. + g.Eventually(func() commonv1.UpgradePhase { + cur := &glancev1alpha1.Glance{} + if err := c.Get(ctx, glanceKey, cur); err != nil { + return "" + } + return cur.Status.UpgradePhase + }, eventuallyTimeout, pollInterval).Should(Equal(commonv1.UpgradePhaseExpanding), + "upgradePhase should transition to Expanding") + + upgrading := &glancev1alpha1.Glance{} + g.Expect(c.Get(ctx, glanceKey, upgrading)).To(Succeed()) + g.Expect(upgrading.Status.TargetRelease).To(Equal("2026.1"), + "targetRelease should record the in-flight upgrade") + + expandKey := client.ObjectKey{Namespace: ns, Name: "glance-db-expand"} + g.Eventually(func() error { + return c.Get(ctx, expandKey, &batchv1.Job{}) + }, eventuallyTimeout, pollInterval).Should(Succeed(), "db-expand Job should appear") + expandJob := &batchv1.Job{} + g.Expect(c.Get(ctx, expandKey, expandJob)).To(Succeed()) + g.Expect(expandJob.Spec.Template.Spec.Containers[0].Image).To(HaveSuffix(":2026.1"), + "db-expand Job should run the target-release image") + g.Expect(expandJob.Spec.Template.Spec.Containers[0].Command).To(ContainElements("glance-manage", "db", "expand"), + "db-expand Job should run glance-manage db expand") + g.Expect(simulators.SimulateJobComplete(ctx, c, expandKey)).To(Succeed(), "simulate db-expand Job completion") + + // Phase 2: Migrating — the db-migrate Job runs glance-manage db migrate. + g.Eventually(func() commonv1.UpgradePhase { + cur := &glancev1alpha1.Glance{} + if err := c.Get(ctx, glanceKey, cur); err != nil { + return "" + } + return cur.Status.UpgradePhase + }, eventuallyTimeout, pollInterval).Should(Equal(commonv1.UpgradePhaseMigrating), + "upgradePhase should transition to Migrating") + + migrateKey := client.ObjectKey{Namespace: ns, Name: "glance-db-migrate"} + g.Eventually(func() error { + return c.Get(ctx, migrateKey, &batchv1.Job{}) + }, eventuallyTimeout, pollInterval).Should(Succeed(), "db-migrate Job should appear") + migrateJob := &batchv1.Job{} + g.Expect(c.Get(ctx, migrateKey, migrateJob)).To(Succeed()) + g.Expect(migrateJob.Spec.Template.Spec.Containers[0].Command).To(ContainElements("glance-manage", "db", "migrate"), + "db-migrate Job should run glance-manage db migrate") + g.Expect(simulators.SimulateJobComplete(ctx, c, migrateKey)).To(Succeed(), "simulate db-migrate Job completion") + + // Phase 3: RollingUpdate — the Deployment template flips to the 2026.1 uWSGI + // launch, which resets its simulated readiness. + g.Eventually(func() commonv1.UpgradePhase { + cur := &glancev1alpha1.Glance{} + if err := c.Get(ctx, glanceKey, cur); err != nil { + return "" + } + return cur.Status.UpgradePhase + }, eventuallyTimeout, pollInterval).Should(Equal(commonv1.UpgradePhaseRollingUpdate), + "upgradePhase should transition to RollingUpdate") + + g.Eventually(func(ig Gomega) { + d := &appsv1.Deployment{} + ig.Expect(c.Get(ctx, deployKey, d)).To(Succeed()) + ig.Expect(d.Spec.Template.Spec.Containers[0].Image).To(HaveSuffix(":2026.1"), + "Deployment should carry the 2026.1 image during RollingUpdate") + ig.Expect(d.Spec.Template.Spec.Containers[0].Command).To(ContainElement("uwsgi"), + "Deployment should switch to the uWSGI launch on the release bump") + }, eventuallyTimeout, pollInterval).Should(Succeed(), + "Deployment should flip to the 2026.1 uWSGI launch during RollingUpdate") + + // Rollout-gated contract (Acceptance Criterion): while the re-imaged Deployment + // is not ready (the template bump reset its simulated readiness), the phase + // MUST NOT advance past RollingUpdate. + g.Consistently(func(ig Gomega) { + cur := &glancev1alpha1.Glance{} + ig.Expect(c.Get(ctx, glanceKey, cur)).To(Succeed()) + ig.Expect(cur.Status.UpgradePhase).To(Equal(commonv1.UpgradePhaseRollingUpdate), + "upgradePhase must stay RollingUpdate until the Deployment reports ready") + }, 2*time.Second, pollInterval).Should(Succeed()) + + // Simulate the re-imaged rollout completing; the flow advances to Contracting. + var rollout appsv1.Deployment + g.Expect(c.Get(ctx, deployKey, &rollout)).To(Succeed()) + g.Expect(simulators.SimulateDeploymentReady(ctx, c, deployKey, ptr.Deref(rollout.Spec.Replicas, 1))). + To(Succeed(), "simulate the re-imaged Deployment rollout completing") + + // Phase 4: Contracting — the db-contract Job runs glance-manage db contract. + g.Eventually(func() commonv1.UpgradePhase { + cur := &glancev1alpha1.Glance{} + if err := c.Get(ctx, glanceKey, cur); err != nil { + return "" + } + return cur.Status.UpgradePhase + }, eventuallyTimeout, pollInterval).Should(Equal(commonv1.UpgradePhaseContracting), + "upgradePhase should transition to Contracting once the Deployment is ready") + + contractKey := client.ObjectKey{Namespace: ns, Name: "glance-db-contract"} + g.Eventually(func() error { + return c.Get(ctx, contractKey, &batchv1.Job{}) + }, eventuallyTimeout, pollInterval).Should(Succeed(), "db-contract Job should appear") + contractJob := &batchv1.Job{} + g.Expect(c.Get(ctx, contractKey, contractJob)).To(Succeed()) + g.Expect(contractJob.Spec.Template.Spec.Containers[0].Command).To(ContainElements("glance-manage", "db", "contract"), + "db-contract Job should run glance-manage db contract") + g.Expect(simulators.SimulateJobComplete(ctx, c, contractKey)).To(Succeed(), "simulate db-contract Job completion") + + // Upgrade completes: installedRelease promoted, phase and target cleared. + g.Eventually(func() string { + cur := &glancev1alpha1.Glance{} + if err := c.Get(ctx, glanceKey, cur); err != nil { + return "" + } + return cur.Status.InstalledRelease + }, eventuallyTimeout, pollInterval).Should(Equal("2026.1"), + "installedRelease should advance to 2026.1 after the contract phase") + + completed := &glancev1alpha1.Glance{} + g.Expect(c.Get(ctx, glanceKey, completed)).To(Succeed()) + g.Expect(string(completed.Status.UpgradePhase)).To(Equal(""), + "upgradePhase should be cleared once the upgrade completes") + g.Expect(completed.Status.TargetRelease).To(Equal(""), + "targetRelease should be cleared once the upgrade completes") + + // Post-upgrade steady state: the db-sync Job re-runs with the new image on + // pod-spec-hash drift (its db load_metadefs step then loads the 2026.1 + // definitions). Its recreation is a delete-and-create, so the Get may miss it + // transiently — Eventually rides that out. + g.Eventually(func(ig Gomega) { + j := &batchv1.Job{} + ig.Expect(c.Get(ctx, dbSyncKey, j)).To(Succeed()) + ig.Expect(j.Spec.Template.Spec.Containers[0].Image).To(HaveSuffix(":2026.1"), + "steady-state db-sync Job should be re-created with the 2026.1 image") + }, eventuallyLongTimeout, pollInterval).Should(Succeed(), + "steady-state db-sync Job should re-run with the new image") + g.Expect(simulators.SimulateJobComplete(ctx, c, dbSyncKey)).To(Succeed(), "simulate post-upgrade db-sync completion") + + // The system returns to DatabaseReady/Ready after the full upgrade cycle. The + // re-imaged Deployment was already simulated ready during the contract flip, so + // no further Deployment simulation is required for the aggregate to converge. + waitForGlanceCondition(t, ctx, c, glanceKey, "DatabaseReady", metav1.ConditionTrue, eventuallyLongTimeout) + waitForGlanceCondition(t, ctx, c, glanceKey, "Ready", metav1.ConditionTrue, eventuallyLongTimeout) +} diff --git a/operators/glance/internal/controller/reconcile_database.go b/operators/glance/internal/controller/reconcile_database.go index 769dad0b1..3ccc25e11 100644 --- a/operators/glance/internal/controller/reconcile_database.go +++ b/operators/glance/internal/controller/reconcile_database.go @@ -16,31 +16,37 @@ import ( "github.com/c5c3/forge/internal/common/conditions" "github.com/c5c3/forge/internal/common/database" "github.com/c5c3/forge/internal/common/release" + commonv1 "github.com/c5c3/forge/internal/common/types" glancev1alpha1 "github.com/c5c3/forge/operators/glance/api/v1alpha1" ) -// Condition reason constants for DatabaseReady set on the release-transition -// path. The steady-state reasons live in internal/common/database as -// database.Reason*. +// Condition reason constants for DatabaseReady set on the Glance-specific paths. +// The steady-state and expand-migrate-contract upgrade reasons live in +// internal/common/database as database.Reason*. const ( - // conditionReasonInvalidReleaseTransition is set when the requested - // spec.openStackRelease is an unsupported transition from the installed - // release (a downgrade, or a jump that is neither patch-only nor a single - // sequential step). It is terminal until the spec changes. - conditionReasonInvalidReleaseTransition = "InvalidReleaseTransition" // conditionReasonWaitingForBackends mirrors the backends step's reason: the // schema cannot be provisioned until a ready default backend has produced a // rendered config to db-sync against. conditionReasonDatabaseWaitingForBackends = "WaitingForBackends" + // conditionReasonImageReleaseMismatch flags the operator error where the + // tag-pinned spec.image names a different OpenStack release than + // spec.openStackRelease. Release tracking, the API launch mode, and the + // expand-migrate-contract upgrade detection all key on spec.openStackRelease + // while every migration Job and the Deployment run spec.image; the reconcile + // refuses to advance until the two agree rather than promoting an installed- + // release marker for a release the image is not. + conditionReasonImageReleaseMismatch = "ImageReleaseMismatch" ) // reconcileDatabase provisions and migrates the Glance database schema and // tracks the installed OpenStack release. It always runs the shared provisioning // flow (MariaDB cluster gate + Database/User/Grant in managed mode, no-op in -// brownfield), validates any release transition against the installed release -// (deliberately without keystone's expand-migrate-contract phase machine — -// Glance's schema migrations run in a single glance-manage db sync), and runs -// the db-sync Job once a rendered config exists. +// brownfield). A release transition (a non-patch change from the installed +// release) runs the shared expand-migrate-contract upgrade flow +// (internal/common/database), walking Expanding → Migrating → RollingUpdate → +// Contracting with glance-manage db expand|migrate|contract phase Jobs; fresh +// installs and patch bumps stay on the single-pass glance-manage db sync path. +// Both run only once a rendered config exists. func (r *GlanceReconciler) reconcileDatabase(ctx context.Context, glance *glancev1alpha1.Glance, configMapName string) (ctrl.Result, error) { // Managed/brownfield provisioning: MariaDB cluster gate, Database/User/Grant // ensure, Dynamic-credentials skip of the User/Grant. A non-zero result means @@ -61,38 +67,13 @@ func (r *GlanceReconciler) reconcileDatabase(ctx context.Context, glance *glance return res, err } - // Track the release being converged to. Stamped every pass so status reflects - // the current spec even before the db-sync promotes InstalledRelease. - glance.Status.TargetRelease = glance.Spec.OpenStackRelease - - // Validate a release transition against the installed release (skipped on a - // fresh install or a same-release reconcile). A rejection short-circuits the - // pipeline (non-zero requeue, no error) so the workload is NOT rolled to the - // new code against an un-migrated schema; it stays rejected until the spec - // changes and re-triggers the reconcile. - if installed := glance.Status.InstalledRelease; installed != "" && installed != glance.Spec.OpenStackRelease { - from, err := release.ParseRelease(installed) - if err != nil { - return r.rejectReleaseTransition(glance, fmt.Sprintf("cannot parse installed release %q: %v", installed, err)) - } - to, err := release.ParseRelease(glance.Spec.OpenStackRelease) - if err != nil { - return r.rejectReleaseTransition(glance, fmt.Sprintf("cannot parse target release %q: %v", glance.Spec.OpenStackRelease, err)) - } - if release.IsDowngrade(from, to) { - return r.rejectReleaseTransition(glance, fmt.Sprintf("downgrade from %s to %s is not supported", from.Raw, to.Raw)) - } - if !release.IsPatchOnly(from, to) && !release.IsSequentialUpgrade(from, to) { - return r.rejectReleaseTransition(glance, fmt.Sprintf( - "release transition from %s to %s is not supported: only a patch-level change or a single sequential upgrade is allowed", - from.Raw, to.Raw, - )) - } - } - // No config yet means no ready default backend has produced a glance-api.conf - // to migrate against — glance-manage db sync needs [glance_store] and the - // enabled_backends set. Wait rather than db-sync an incomplete config. + // to migrate against — glance-manage db sync (and the expand/migrate/contract + // phase Jobs) needs [glance_store] and the enabled_backends set, so a phase + // Job must never build against an empty config mount. Wait rather than migrate + // an incomplete config. The gate runs before the upgrade branches below; + // mid-upgrade an empty configMapName is unreachable anyway because the config + // step recovers the last-good names off the live Deployment. if configMapName == "" { conditions.SetCondition(&glance.Status.Conditions, metav1.Condition{ Type: "DatabaseReady", @@ -104,6 +85,40 @@ func (r *GlanceReconciler) reconcileDatabase(ctx context.Context, glance *glance return ctrl.Result{RequeueAfter: RequeueDatabaseWait}, nil } + // Active upgrade: the shared flow handles the abort (spec.openStackRelease + // reverted to the installed release), the target-changed guard, and the + // phase dispatch. + if glance.Status.UpgradePhase != "" { + // Enforce the decoupled-field contract mid-upgrade too. The shared flow's + // target-changed guard only watches spec.openStackRelease, so a lone + // spec.image.tag edit to an inconsistent release would otherwise slip + // through and dispatch phase Jobs built from the wrong image (every phase + // Job runs spec.image) while re-rendering the RollingUpdate Deployment in + // the target release's launch mode against an image that cannot launch that + // way. Skip the check when spec.openStackRelease has been reverted to the + // installed release: that is the shared flow's abort trigger (SpecRelease == + // InstalledRelease in ReconcileUpgrade), which must stay reachable even while + // the two fields disagree so a wedged upgrade can always be unstuck. + if glance.Spec.OpenStackRelease != glance.Status.InstalledRelease { + if res, blocked := checkImageReleaseMismatch(glance); blocked { + return res, nil + } + } + return database.ReconcileUpgrade(ctx, r.upgradeFlowParams(ctx, glance, configMapName)) + } + + // Enforce the decoupled-field contract before either the upgrade or the + // steady-state path advances release tracking (see checkImageReleaseMismatch). + if res, blocked := checkImageReleaseMismatch(glance); blocked { + return res, nil + } + + // Detect a release upgrade (patch-only and same-release changes stay on the + // steady-state path). + if database.IsUpgrade(glance.Spec.OpenStackRelease, glance.Status.InstalledRelease) { + return database.InitiateUpgrade(ctx, r.upgradeFlowParams(ctx, glance, configMapName)) + } + // Steady-state db-sync. glance-manage db sync applies every pending migration // in one pass, so there is no schema-check step (SchemaCheckCommand nil). // InstalledRelease is promoted to spec.openStackRelease on Job success. @@ -125,6 +140,49 @@ func (r *GlanceReconciler) reconcileDatabase(ctx context.Context, glance *glance }) } +// checkImageReleaseMismatch enforces the decoupled-field contract between +// spec.openStackRelease and spec.image. spec.openStackRelease drives release +// tracking, the API launch mode, and the upgrade detection, but every migration +// Job and the Deployment run spec.image — the two fields are deliberately separate +// (digest pinning, launch-mode selection) and nothing else enforces they agree. A +// tag-pinned image whose tag names a different OpenStack release would run the +// wrong glance-manage binary against a schema already at its own HEAD: the +// expand/migrate/contract Jobs exit 0 as no-ops while the flow promotes +// status.installedRelease to a release the pods neither run nor migrated to, and +// the RollingUpdate phase re-renders the Deployment in the target release's launch +// mode against an image that cannot launch that way. +// +// When the two disagree it sets DatabaseReady=False/ImageReleaseMismatch and +// returns (requeue, true) so the caller refuses to advance until the image is +// bumped in lockstep. A digest-pinned image (Tag == "") or a tag that does not +// parse as a release carries no comparable release string and is left to the +// operator's explicit spec.openStackRelease declaration, matching the +// digest-disables-tracking contract; the patch suffix is ignored so a patched +// image build (e.g. tag 2026.1-p1) still matches release 2026.1. +func checkImageReleaseMismatch(glance *glancev1alpha1.Glance) (ctrl.Result, bool) { + if glance.Spec.Image.Tag == "" { + return ctrl.Result{}, false + } + tagRel, tagErr := release.ParseRelease(glance.Spec.Image.Tag) + specRel, specErr := release.ParseRelease(glance.Spec.OpenStackRelease) + if tagErr != nil || specErr != nil { + return ctrl.Result{}, false + } + if tagRel.Year == specRel.Year && tagRel.Minor == specRel.Minor { + return ctrl.Result{}, false + } + conditions.SetCondition(&glance.Status.Conditions, metav1.Condition{ + Type: "DatabaseReady", + Status: metav1.ConditionFalse, + ObservedGeneration: glance.Generation, + Reason: conditionReasonImageReleaseMismatch, + Message: fmt.Sprintf("spec.image.tag %q names OpenStack release %d.%d but spec.openStackRelease is %q; "+ + "bump spec.image in lockstep with spec.openStackRelease", + glance.Spec.Image.Tag, tagRel.Year, tagRel.Minor, glance.Spec.OpenStackRelease), + }) + return ctrl.Result{RequeueAfter: RequeueDatabaseWait}, true +} + // glanceMetadefsSourcePath is the directory the glance image ships the default // metadata-definition JSON files at. Glance's setup.cfg installs etc/metadefs/* // to /etc/glance/metadefs, and the image installs glance with @@ -135,8 +193,9 @@ const glanceMetadefsSourcePath = "/var/lib/openstack/etc/glance/metadefs" // glanceJobSetParams derives the shared migration-Job inputs from the Glance CR: // the config mount, the DB-connection env override, and the glance-manage db -// sync command. The steady-state sync flow (database.ReconcileSyncJobs) consumes -// it; centralising it here lets tests build the identical Job. +// sync command. The steady-state sync flow (database.ReconcileSyncJobs) and the +// upgrade-phase builders (upgradeFlowParams.BuildPhaseJob) both consume it; +// centralising it here lets tests build the identical Job. func glanceJobSetParams(glance *glancev1alpha1.Glance, configMapName string) database.JobSetParams { return database.JobSetParams{ InstanceName: glance.Name, @@ -159,27 +218,59 @@ func glanceJobSetParams(glance *glancev1alpha1.Glance, configMapName string) dat "glance-manage --config-dir " + glanceConfigDir + " db load_metadefs --path " + glanceMetadefsSourcePath, }, // No schema-check: glance-manage db sync is idempotent and applies all - // pending migrations in one pass, unlike keystone's expand/migrate split. + // pending migrations in one pass for fresh installs and patch bumps. + // Release upgrades instead run the shared expand-migrate-contract phase + // machine (see upgradeFlowParams), so there is no schema-check step here. SchemaCheckCommand: nil, } } -// rejectReleaseTransition sets DatabaseReady=False with the -// InvalidReleaseTransition reason, emits a Warning event, and returns a non-zero -// requeue (no error) so the pipeline short-circuits before the Deployment step. -// A zero result would NOT short-circuit RunPipeline, letting reconcileDeployment -// roll the workload to the new code against an un-migrated schema. The transition -// stays rejected until the spec changes and re-triggers the reconcile; the -// periodic re-confirm requeue is harmless and mirrors the sibling holding paths -// (WaitingForBackends, DBSyncInProgress), which also return RequeueDatabaseWait. -func (r *GlanceReconciler) rejectReleaseTransition(glance *glancev1alpha1.Glance, msg string) (ctrl.Result, error) { - conditions.SetCondition(&glance.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: glance.Generation, - Reason: conditionReasonInvalidReleaseTransition, - Message: msg, - }) - r.Recorder.Event(glance, corev1.EventTypeWarning, conditionReasonInvalidReleaseTransition, msg) - return ctrl.Result{RequeueAfter: RequeueDatabaseWait}, nil +// upgradeFlowParams assembles the shared expand-migrate-contract upgrade-flow +// inputs for this Glance CR. The service-specific parts — the owner CR, the +// "DatabaseReady" condition vocabulary, spec.openStackRelease as the requested +// release, the per-phase Job builder, and the terminal-metric callback — are +// bound here; the phase choreography itself lives in internal/common/database. +// The three status pointers (UpgradePhase, InstalledRelease, TargetRelease) are +// mutated in place by the flow and persisted by the caller after it returns. +func (r *GlanceReconciler) upgradeFlowParams(ctx context.Context, glance *glancev1alpha1.Glance, configMapName string) database.UpgradeFlowParams { + return database.UpgradeFlowParams{ + Client: r.Client, + Scheme: r.Scheme, + Recorder: r.Recorder, + Owner: glance, + Conditions: &glance.Status.Conditions, + Generation: glance.Generation, + ConditionType: "DatabaseReady", + RequeueAfter: RequeueUpgradeWait, + Phase: &glance.Status.UpgradePhase, + InstalledRelease: &glance.Status.InstalledRelease, + TargetRelease: &glance.Status.TargetRelease, + SpecRelease: glance.Spec.OpenStackRelease, + // Each phase Job runs spec.image (glance.Spec.Image.Reference()): the + // operator's existing contract is that the image tag is bumped together + // with spec.openStackRelease, so the new release's migration tree owns the + // expand/migrate/contract schema deltas. backoffLimit 4 is keystone parity. + BuildPhaseJob: func(phase commonv1.UpgradePhase) *batchv1.Job { + switch phase { + case commonv1.UpgradePhaseExpanding: + return database.BuildJob(glanceJobSetParams(glance, configMapName), glance.Spec.Image.Reference(), "db-expand", + []string{"glance-manage", "--config-dir", glanceConfigDir, "db", "expand"}, 4) + case commonv1.UpgradePhaseMigrating: + return database.BuildJob(glanceJobSetParams(glance, configMapName), glance.Spec.Image.Reference(), "db-migrate", + []string{"glance-manage", "--config-dir", glanceConfigDir, "db", "migrate"}, 4) + case commonv1.UpgradePhaseContracting: + return database.BuildJob(glanceJobSetParams(glance, configMapName), glance.Spec.Image.Reference(), "db-contract", + []string{"glance-manage", "--config-dir", glanceConfigDir, "db", "contract"}, 4) + case commonv1.UpgradePhaseRollingUpdate: + // RollingUpdate builds no Job; the Deployment rollout drives it. + return nil + default: + // The empty steady-state phase never reaches BuildPhaseJob. + return nil + } + }, + RecordTerminal: func(jobSuffix string, observed *batchv1.Job) { + r.recordDBJobTerminalState(ctx, glance, jobSuffix, observed) + }, + } } diff --git a/operators/glance/internal/controller/reconcile_database_test.go b/operators/glance/internal/controller/reconcile_database_test.go index 277f368bc..60e67757b 100644 --- a/operators/glance/internal/controller/reconcile_database_test.go +++ b/operators/glance/internal/controller/reconcile_database_test.go @@ -10,11 +10,15 @@ import ( mariadbv1alpha1 "github.com/mariadb-operator/mariadb-operator/api/v1alpha1" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" 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/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -55,6 +59,58 @@ func managedGlance() *glancev1alpha1.Glance { return glance } +// upgradingGlance returns a Glance mid-upgrade: the installed release is 2025.2, +// the spec requests 2026.1 (both the OpenStack release and the image tag, per the +// operator's bump-in-lockstep contract), and the given phase is active. +func upgradingGlance(phase commonv1.UpgradePhase) *glancev1alpha1.Glance { + glance := testGlance() + glance.Spec.OpenStackRelease = "2026.1" + glance.Spec.Image.Tag = "2026.1" + glance.Status.InstalledRelease = "2025.2" + glance.Status.TargetRelease = "2026.1" + glance.Status.UpgradePhase = phase + return glance +} + +// glanceUpgradeJob builds the expand-migrate-contract phase Job the operator's +// upgradeFlowParams.BuildPhaseJob produces for the given db verb ("expand", +// "migrate", "contract"), so seeded Jobs share the pod-spec hash the runner +// compares against. +func glanceUpgradeJob(glance *glancev1alpha1.Glance, configMapName, verb string) *batchv1.Job { + cmd := []string{"glance-manage", "--config-dir", glanceConfigDir, "db", verb} + return database.BuildJob(glanceJobSetParams(glance, configMapName), glance.Spec.Image.Reference(), "db-"+verb, cmd, 4) +} + +// completedGlanceUpgradeJob returns a phase Job marked complete with the matching +// pod-spec hash and a stable UID for terminal-metric dedupe. +func completedGlanceUpgradeJob(glance *glancev1alpha1.Glance, configMapName, verb string) *batchv1.Job { + desired := glanceUpgradeJob(glance, configMapName, verb) + now := metav1.Now() + j := desired.DeepCopy() + j.UID = types.UID(glance.Name + "-db-" + verb + "-complete-uid") + j.Annotations = map[string]string{job.PodSpecHashAnnotation: job.PodSpecHash(&desired.Spec.Template)} + j.Status.Succeeded = 1 + j.Status.CompletionTime = &now + j.Status.Conditions = []batchv1.JobCondition{ + {Type: batchv1.JobComplete, Status: corev1.ConditionTrue}, + } + return j +} + +// failedGlanceUpgradeJob returns a permanently-failed phase Job with the matching +// pod-spec hash and a stable UID for terminal-metric dedupe. +func failedGlanceUpgradeJob(glance *glancev1alpha1.Glance, configMapName, verb string) *batchv1.Job { + desired := glanceUpgradeJob(glance, configMapName, verb) + j := desired.DeepCopy() + j.UID = types.UID(glance.Name + "-db-" + verb + "-failed-uid") + j.Annotations = map[string]string{job.PodSpecHashAnnotation: job.PodSpecHash(&desired.Spec.Template)} + j.Status.Failed = 5 + j.Status.Conditions = []batchv1.JobCondition{ + {Type: batchv1.JobFailed, Status: corev1.ConditionTrue}, + } + return j +} + func TestReconcileDatabase_ProvisionGatesOnClusterReady(t *testing.T) { g := NewGomegaWithT(t) glance := managedGlance() @@ -80,7 +136,9 @@ func TestReconcileDatabase_NoConfigWaitsForBackends(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(res.RequeueAfter).To(Equal(RequeueDatabaseWait)) - g.Expect(glance.Status.TargetRelease).To(Equal("2026.1"), "TargetRelease is stamped") + // TargetRelease is owned by the shared upgrade flow now (set on initiate, + // cleared on completion/abort); the steady-state path no longer stamps it. + g.Expect(glance.Status.TargetRelease).To(BeEmpty()) cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) g.Expect(cond.Reason).To(Equal(conditionReasonDatabaseWaitingForBackends)) @@ -89,21 +147,27 @@ func TestReconcileDatabase_NoConfigWaitsForBackends(t *testing.T) { func TestReconcileDatabase_DowngradeRejected(t *testing.T) { g := NewGomegaWithT(t) glance := testGlance() + // Both the release and the image tag name 2025.2 (bump-in-lockstep contract) + // so the image/release-mismatch guard passes and the downgrade path is what is + // exercised. glance.Spec.OpenStackRelease = "2025.2" + glance.Spec.Image.Tag = "2025.2" glance.Status.InstalledRelease = "2026.1" r := newGlanceTestReconciler(glance) - res, err := r.reconcileDatabase(context.Background(), glance, "test-glance-config-abc") + _, err := r.reconcileDatabase(context.Background(), glance, "test-glance-config-abc") - g.Expect(err).NotTo(HaveOccurred()) - // A rejection must return a non-zero requeue so RunPipeline short-circuits - // before the Deployment step; a zero result would let the workload roll to - // the new code against the un-migrated (downgraded) schema. - g.Expect(res.RequeueAfter).To(Equal(RequeueDatabaseWait), "an invalid transition short-circuits the pipeline, it is not a zero result") - g.Expect(glance.Status.TargetRelease).To(Equal("2025.2")) + // A downgrade now surfaces through the shared vocabulary as a returned error + // (controller backoff), exactly like keystone; the rejection precedes target + // stamping so TargetRelease stays empty. + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("downgrade")) + g.Expect(glance.Status.TargetRelease).To(BeEmpty()) cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - g.Expect(cond.Reason).To(Equal(conditionReasonInvalidReleaseTransition)) + g.Expect(cond.Reason).To(Equal(database.ReasonDowngradeNotSupported)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Warning DowngradeNotSupported"))) } func TestReconcileDatabase_NonSequentialJumpRejected(t *testing.T) { @@ -113,12 +177,15 @@ func TestReconcileDatabase_NonSequentialJumpRejected(t *testing.T) { glance.Status.InstalledRelease = "2025.1" // skips 2025.2 r := newGlanceTestReconciler(glance) - res, err := r.reconcileDatabase(context.Background(), glance, "test-glance-config-abc") + _, err := r.reconcileDatabase(context.Background(), glance, "test-glance-config-abc") - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(res.RequeueAfter).To(Equal(RequeueDatabaseWait), "a non-sequential jump short-circuits the pipeline, it is not a zero result") + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("sequential")) + g.Expect(glance.Status.TargetRelease).To(BeEmpty()) cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") - g.Expect(cond.Reason).To(Equal(conditionReasonInvalidReleaseTransition)) + g.Expect(cond.Reason).To(Equal(database.ReasonUpgradePathInvalid)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Warning UpgradePathInvalid"))) } func TestReconcileDatabase_PatchOnlyAccepted(t *testing.T) { @@ -131,9 +198,10 @@ func TestReconcileDatabase_PatchOnlyAccepted(t *testing.T) { res, err := r.reconcileDatabase(context.Background(), glance, "test-glance-config-abc") g.Expect(err).NotTo(HaveOccurred()) - // Accepted: the step proceeds to db-sync (a fresh Job is in progress) rather - // than rejecting the transition. + // Accepted: a patch-only change stays on the steady-state path (a fresh + // db-sync Job is in progress) rather than entering the upgrade flow. g.Expect(res.RequeueAfter).To(Equal(RequeueDatabaseWait)) + g.Expect(glance.Status.UpgradePhase).To(BeEmpty()) cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") g.Expect(cond.Reason).To(Equal(database.ReasonDBSyncInProgress)) } @@ -148,7 +216,71 @@ func TestReconcileDatabase_SequentialUpgradeAccepted(t *testing.T) { res, err := r.reconcileDatabase(context.Background(), glance, "test-glance-config-abc") g.Expect(err).NotTo(HaveOccurred()) + // A sequential upgrade initiates the shared flow: TargetRelease and the + // Expanding phase are stamped and the reconcile requeues immediately. + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + g.Expect(glance.Status.UpgradePhase).To(Equal(commonv1.UpgradePhaseExpanding)) + g.Expect(glance.Status.TargetRelease).To(Equal("2026.1")) + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(database.ReasonExpandInProgress)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Normal UpgradeInitiated"))) +} + +// TestReconcileDatabase_ImageReleaseMismatchBlocks verifies that a tag-pinned +// image whose tag names a different OpenStack release than spec.openStackRelease +// blocks the database pass instead of initiating a no-op upgrade that would +// falsely promote the installed-release marker. Regression guard for the +// decoupled spec.image / spec.openStackRelease contract: the migration Jobs and +// the Deployment run spec.image, but release tracking and the launch mode key on +// spec.openStackRelease, so the two must name the same release. +func TestReconcileDatabase_ImageReleaseMismatchBlocks(t *testing.T) { + g := NewGomegaWithT(t) + glance := testGlance() + // The operator bumped the release but left the image tag on the old release. + glance.Spec.OpenStackRelease = "2026.1" + glance.Spec.Image.Tag = "2025.2" + glance.Status.InstalledRelease = "2025.2" + r := newGlanceTestReconciler(glance) + + res, err := r.reconcileDatabase(context.Background(), glance, "test-glance-config-abc") + + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.RequeueAfter).To(Equal(RequeueDatabaseWait)) + // The upgrade flow is never entered: no phase is stamped, no target is + // recorded, and the installed-release marker stays at the installed release. + g.Expect(glance.Status.UpgradePhase).To(BeEmpty()) + g.Expect(glance.Status.TargetRelease).To(BeEmpty()) + g.Expect(glance.Status.InstalledRelease).To(Equal("2025.2")) + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(conditionReasonImageReleaseMismatch)) + + // No phase Job (nor a db-sync Job) is created while the fields disagree. + var expandJob batchv1.Job + getErr := r.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "test-glance-db-expand"}, &expandJob) + g.Expect(apierrors.IsNotFound(getErr)).To(BeTrue()) +} + +// TestReconcileDatabase_ImagePatchBuildAccepted verifies that a patched image +// build (tag 2026.1-p1) still matches the base release 2026.1 and passes the +// mismatch guard: the patch suffix is ignored, so the reconcile advances to the +// steady-state db-sync path rather than blocking. +func TestReconcileDatabase_ImagePatchBuildAccepted(t *testing.T) { + g := NewGomegaWithT(t) + glance := testGlance() // OpenStackRelease 2026.1, InstalledRelease empty (fresh) + glance.Spec.Image.Tag = "2026.1-p1" + r := newGlanceTestReconciler(glance) + + res, err := r.reconcileDatabase(context.Background(), glance, "test-glance-config-abc") + + g.Expect(err).NotTo(HaveOccurred()) + // Not blocked: the fresh install runs the steady-state db-sync (a Job is in + // progress), and the DatabaseReady reason is the sync reason, not the mismatch. g.Expect(res.RequeueAfter).To(Equal(RequeueDatabaseWait)) + g.Expect(glance.Status.UpgradePhase).To(BeEmpty()) cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") g.Expect(cond.Reason).To(Equal(database.ReasonDBSyncInProgress)) } @@ -216,3 +348,322 @@ func TestReconcileDatabase_InstalledReleasePromotedOnSuccess(t *testing.T) { // metric emits at most once per Job UID. g.Expect(glance.Annotations).To(HaveKey(dbJobUIDAnnotationKey("db-sync"))) } + +// --- Upgrade phase walk --- + +// TestReconcileDatabase_UpgradeExpand_CreatesJob verifies that the first pass of +// the Expanding phase creates the db-expand Job with the spec image, the +// glance-manage db expand command, and the backoffLimit-4 parity with keystone. +func TestReconcileDatabase_UpgradeExpand_CreatesJob(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(commonv1.UpgradePhaseExpanding) + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance) + + res, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.RequeueAfter).To(Equal(RequeueUpgradeWait)) + + var expandJob batchv1.Job + key := client.ObjectKey{Namespace: "default", Name: "test-glance-db-expand"} + g.Expect(r.Get(context.Background(), key, &expandJob)).To(Succeed()) + + container := expandJob.Spec.Template.Spec.Containers[0] + g.Expect(container.Image).To(Equal(glance.Spec.Image.Reference())) + g.Expect(container.Command).To(Equal([]string{ + "glance-manage", "--config-dir", glanceConfigDir, "db", "expand", + })) + g.Expect(expandJob.Spec.BackoffLimit).NotTo(BeNil()) + g.Expect(*expandJob.Spec.BackoffLimit).To(Equal(int32(4))) + + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(database.ReasonExpandInProgress)) +} + +// TestReconcileDatabase_UpgradeExpandComplete_TransitionsToMigrating verifies the +// Expanding → Migrating transition once the seeded expand Job is complete. +func TestReconcileDatabase_UpgradeExpandComplete_TransitionsToMigrating(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(commonv1.UpgradePhaseExpanding) + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance, completedGlanceUpgradeJob(glance, configMapName, "expand")) + + res, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + g.Expect(glance.Status.UpgradePhase).To(Equal(commonv1.UpgradePhaseMigrating)) + + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(database.ReasonMigrateInProgress)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Normal ExpandComplete"))) +} + +// TestReconcileDatabase_UpgradeMigrateComplete_TransitionsToRollingUpdate verifies +// the Migrating → RollingUpdate transition once the seeded migrate Job is +// complete. +func TestReconcileDatabase_UpgradeMigrateComplete_TransitionsToRollingUpdate(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(commonv1.UpgradePhaseMigrating) + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance, completedGlanceUpgradeJob(glance, configMapName, "migrate")) + + res, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + g.Expect(glance.Status.UpgradePhase).To(Equal(commonv1.UpgradePhaseRollingUpdate)) + + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(database.ReasonUpgradeRollingUpdate)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Normal MigrateComplete"))) +} + +// TestReconcileDatabase_UpgradeContractComplete_CompletesUpgrade verifies the +// contract Job's completion promotes InstalledRelease, clears the upgrade state, +// and reports DatabaseReady=True. +func TestReconcileDatabase_UpgradeContractComplete_CompletesUpgrade(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(commonv1.UpgradePhaseContracting) + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance, completedGlanceUpgradeJob(glance, configMapName, "contract")) + + res, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{})) + + g.Expect(glance.Status.InstalledRelease).To(Equal("2026.1")) + g.Expect(glance.Status.TargetRelease).To(BeEmpty()) + g.Expect(glance.Status.UpgradePhase).To(BeEmpty()) + + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(database.ReasonDatabaseSynced)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Normal UpgradeComplete"))) +} + +// TestReconcileDatabase_UpgradeExpandFailed_ReturnsError verifies a permanently +// failed expand Job surfaces as a reconcile error with ExpandFailed and a Warning +// event. +func TestReconcileDatabase_UpgradeExpandFailed_ReturnsError(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(commonv1.UpgradePhaseExpanding) + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance, failedGlanceUpgradeJob(glance, configMapName, "expand")) + + _, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).To(HaveOccurred()) + + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(database.ReasonExpandFailed)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Warning ExpandFailed"))) +} + +// TestReconcileDatabase_UpgradeAbort_RevertToInstalled verifies that reverting +// spec.openStackRelease (and the image tag) to the installed release aborts the +// upgrade: the phase Jobs are deleted, phase/target are cleared, an UpgradeAborted +// event fires, and a follow-up pass drops into the steady-state db-sync path. +func TestReconcileDatabase_UpgradeAbort_RevertToInstalled(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(commonv1.UpgradePhaseExpanding) + // Revert both the release and the image tag to the installed release. + glance.Spec.OpenStackRelease = "2025.2" + glance.Spec.Image.Tag = "2025.2" + configMapName := "test-glance-config-abc" + + // Seed all three phase Jobs so the abort has something to delete. + r := newGlanceTestReconciler( + glance, + glanceUpgradeJob(glance, configMapName, "expand"), + glanceUpgradeJob(glance, configMapName, "migrate"), + glanceUpgradeJob(glance, configMapName, "contract"), + ) + + res, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + + g.Expect(glance.Status.UpgradePhase).To(BeEmpty()) + g.Expect(glance.Status.TargetRelease).To(BeEmpty()) + g.Expect(glance.Status.InstalledRelease).To(Equal("2025.2")) + + for _, suffix := range []string{"db-expand", "db-migrate", "db-contract"} { + var jb batchv1.Job + getErr := r.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "test-glance-" + suffix}, &jb) + g.Expect(apierrors.IsNotFound(getErr)).To(BeTrue(), "%s Job must be absent after abort", suffix) + } + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Normal UpgradeAborted"))) + + // The next pass runs the steady-state sync: the db-sync Job is created and + // DatabaseReady flips to DBSyncInProgress. + res, err = r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.RequeueAfter).To(Equal(RequeueDatabaseWait)) + var syncJob batchv1.Job + g.Expect(r.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "test-glance-db-sync"}, &syncJob)).To(Succeed()) + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond.Reason).To(Equal(database.ReasonDBSyncInProgress)) +} + +// TestReconcileDatabase_UpgradeTargetChanged_Blocks verifies that changing +// spec.openStackRelease to a third value mid-upgrade blocks with +// UpgradeTargetChanged and leaves the upgrade state untouched. +func TestReconcileDatabase_UpgradeTargetChanged_Blocks(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(commonv1.UpgradePhaseExpanding) // target 2026.1 + // Someone flips the release to a third value mid-upgrade. + glance.Spec.OpenStackRelease = "2026.2" + glance.Spec.Image.Tag = "2026.2" + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance) + + _, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("spec release changed during active upgrade")) + + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(database.ReasonUpgradeTargetChanged)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Warning UpgradeTargetChanged"))) + + // Upgrade state is untouched. + g.Expect(glance.Status.UpgradePhase).To(Equal(commonv1.UpgradePhaseExpanding)) + g.Expect(glance.Status.TargetRelease).To(Equal("2026.1")) + g.Expect(glance.Status.InstalledRelease).To(Equal("2025.2")) +} + +// TestReconcileDatabase_MidUpgradeImageDriftBlocks verifies that editing +// spec.image.tag alone to an inconsistent release DURING an active upgrade blocks +// with ImageReleaseMismatch and dispatches no phase Job. The shared flow's +// target-changed guard only watches spec.openStackRelease, so without the +// mid-upgrade consistency check a lone image-tag drift would build phase Jobs from +// the wrong image and re-render the Deployment in a launch mode the image cannot +// run. Regression guard for the decoupled spec.image / spec.openStackRelease +// contract while a phase is in flight (keystone is immune: its release IS the tag). +func TestReconcileDatabase_MidUpgradeImageDriftBlocks(t *testing.T) { + g := NewGomegaWithT(t) + // Mid-upgrade to 2026.1 (openStackRelease and targetRelease agree), but the + // image tag is drifted to a third release — an edit the shared flow's + // target-changed guard (which keys on spec.openStackRelease) would not catch. + glance := upgradingGlance(commonv1.UpgradePhaseExpanding) + glance.Spec.Image.Tag = "2027.1" + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance) + + res, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.RequeueAfter).To(Equal(RequeueDatabaseWait)) + + // Blocked on the mismatch; the upgrade state is untouched and no phase Job runs. + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(conditionReasonImageReleaseMismatch)) + g.Expect(glance.Status.UpgradePhase).To(Equal(commonv1.UpgradePhaseExpanding)) + g.Expect(glance.Status.TargetRelease).To(Equal("2026.1")) + + var expandJob batchv1.Job + getErr := r.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "test-glance-db-expand"}, &expandJob) + g.Expect(apierrors.IsNotFound(getErr)).To(BeTrue(), "no phase Job may be dispatched while the image drifts") +} + +// TestReconcileDatabase_AbortReachableDuringImageDrift verifies the abort escape +// hatch stays reachable mid-upgrade even while spec.image.tag disagrees with the +// installed release: reverting spec.openStackRelease to the installed release must +// abort the upgrade (clearing the phase) rather than wedging on the mismatch guard. +func TestReconcileDatabase_AbortReachableDuringImageDrift(t *testing.T) { + g := NewGomegaWithT(t) + // Revert openStackRelease to the installed 2025.2 to abort, but the image tag + // still lags on the aborted target — the mismatch guard must not block the abort. + glance := upgradingGlance(commonv1.UpgradePhaseExpanding) + glance.Spec.OpenStackRelease = "2025.2" + glance.Spec.Image.Tag = "2026.1" + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance) + + res, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + + // The upgrade was aborted, not blocked on the mismatch. + g.Expect(glance.Status.UpgradePhase).To(BeEmpty()) + g.Expect(glance.Status.TargetRelease).To(BeEmpty()) + g.Expect(glance.Status.InstalledRelease).To(Equal("2025.2")) + cond := conditions.GetCondition(glance.Status.Conditions, "DatabaseReady") + if cond != nil { + g.Expect(cond.Reason).NotTo(Equal(conditionReasonImageReleaseMismatch), + "a revert-to-installed abort must not be blocked by the image mismatch guard") + } +} + +// TestUpgradePhaseFailureRecordsDBSyncMetric verifies that each +// expand-migrate-contract phase Job's terminal metric flows through +// recordDBJobTerminalState under its own suffix, stamping the per-phase +// dbJobUIDAnnotationKey dedupe annotation on the Glance CR while surfacing the +// failure as a reconcile error. +func TestUpgradePhaseFailureRecordsDBSyncMetric(t *testing.T) { + cases := []struct { + phase commonv1.UpgradePhase + verb string + suffix string + }{ + {commonv1.UpgradePhaseExpanding, "expand", "db-expand"}, + {commonv1.UpgradePhaseMigrating, "migrate", "db-migrate"}, + {commonv1.UpgradePhaseContracting, "contract", "db-contract"}, + } + for _, tc := range cases { + t.Run(tc.suffix, func(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(tc.phase) + configMapName := "test-glance-config-abc" + r := newGlanceTestReconciler(glance, failedGlanceUpgradeJob(glance, configMapName, tc.verb)) + + _, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).To(HaveOccurred(), + "the failing upgrade-phase Job MUST surface as a reconcile error so callers retry") + + g.Expect(glance.Annotations).To(HaveKey(dbJobUIDAnnotationKey(tc.suffix)), + "%s terminal metric MUST stamp the per-phase dedupe annotation on the CR", tc.suffix) + }) + } +} + +// TestReconcileDatabase_UpgradeInProgress_ShortCircuitsBeforeDeployment verifies +// that an in-progress expand Job returns a non-zero RequeueUpgradeWait so the +// pipeline short-circuits before the deployment step; a live Deployment carrying +// the old pod template is left untouched by the database pass. +func TestReconcileDatabase_UpgradeInProgress_ShortCircuitsBeforeDeployment(t *testing.T) { + g := NewGomegaWithT(t) + glance := upgradingGlance(commonv1.UpgradePhaseExpanding) + configMapName := "test-glance-config-abc" + + // In-progress expand Job (exists, no terminal condition). + inProgress := glanceUpgradeJob(glance, configMapName, "expand") + inProgress.Annotations = map[string]string{job.PodSpecHashAnnotation: job.PodSpecHash(&inProgress.Spec.Template)} + + // A live Deployment still running the old release's pod template. + oldGlance := testGlance() + oldGlance.Spec.OpenStackRelease = "2025.2" + oldGlance.Spec.Image.Tag = "2025.2" + oldDeploy := buildGlanceDeployment(oldGlance, testArtifacts(), "", "") + + r := newGlanceTestReconciler(glance, inProgress, oldDeploy) + + res, err := r.reconcileDatabase(context.Background(), glance, configMapName) + g.Expect(err).NotTo(HaveOccurred()) + // Non-zero requeue: the pipeline stops here and never rolls the Deployment. + g.Expect(res.RequeueAfter).To(Equal(RequeueUpgradeWait)) + + // The Deployment's old pod template is unchanged by the database pass. + var fetched appsv1.Deployment + g.Expect(r.Get(context.Background(), client.ObjectKeyFromObject(oldDeploy), &fetched)).To(Succeed()) + g.Expect(fetched.Spec.Template.Spec.Containers[0].Image).To(Equal("ghcr.io/c5c3/glance:2025.2")) +} diff --git a/operators/glance/internal/controller/reconcile_deployment.go b/operators/glance/internal/controller/reconcile_deployment.go index 7223fa035..b20a11923 100644 --- a/operators/glance/internal/controller/reconcile_deployment.go +++ b/operators/glance/internal/controller/reconcile_deployment.go @@ -24,6 +24,7 @@ import ( "github.com/c5c3/forge/internal/common/keystoneauth" "github.com/c5c3/forge/internal/common/naming" "github.com/c5c3/forge/internal/common/release" + commonv1 "github.com/c5c3/forge/internal/common/types" glancev1alpha1 "github.com/c5c3/forge/operators/glance/api/v1alpha1" ) @@ -185,6 +186,43 @@ func (r *GlanceReconciler) reconcileDeployment(ctx context.Context, glance *glan return ctrl.Result{RequeueAfter: RequeueDeploymentPolling}, nil } + // Hold the RollingUpdate → Contracting flip until the Deployment has FULLY + // converged onto the target-release image. The `ready` signal above comes from + // deployment.IsDeploymentReady, which is surge-tolerant: under the default + // MaxSurge=1/MaxUnavailable=0 strategy a new pod is added before an old one is + // removed, so it turns true as soon as the first new-image pod is Ready while + // old-image pods still serve. The contract phase then drops the now-unused + // columns those old pods still read — so gate the flip on every replica being + // updated, ready, and counted (glanceDeploymentRolledOut), and requeue to wait + // otherwise. Only the RollingUpdate upgrade phase is stricter here; steady-state + // rollouts keep the surge-tolerant readiness above. + if glance.Status.UpgradePhase == commonv1.UpgradePhaseRollingUpdate && !glanceDeploymentRolledOut(deploy) { + conditions.SetCondition(&glance.Status.Conditions, metav1.Condition{ + Type: "DeploymentReady", + Status: metav1.ConditionFalse, + ObservedGeneration: glance.Generation, + Reason: conditionReasonWaitingForDeployment, + Message: "Waiting for the upgraded image to finish rolling out before contracting the database schema", + }) + return ctrl.Result{RequeueAfter: RequeueDeploymentPolling}, nil + } + + // Transition from RollingUpdate to Contracting when the Deployment is ready. + // The shared flow advances the phase, emits the DeploymentRolloutComplete + // event, and logs; glance stamps its own DeploymentReady condition and + // requeues so ReconcileUpgrade runs the contract phase on the next pass. The + // endpoint is deliberately NOT stamped on this flip pass (keystone parity). + if database.CompleteRollingUpdate(ctx, r.upgradeFlowParams(ctx, glance, art.configMapName)) { + conditions.SetCondition(&glance.Status.Conditions, metav1.Condition{ + Type: "DeploymentReady", + Status: metav1.ConditionTrue, + ObservedGeneration: glance.Generation, + Reason: conditionReasonDeploymentReady, + Message: "Glance API deployment is available", + }) + return ctrl.Result{Requeue: true}, nil + } + // Status.Endpoint derivation is delegated to glanceStatusEndpoint so the // gateway-aware public URL is used when spec.gateway is set, and the // cluster-local URL otherwise. @@ -199,6 +237,27 @@ func (r *GlanceReconciler) reconcileDeployment(ctx context.Context, glance *glan return ctrl.Result{}, nil } +// glanceDeploymentRolledOut reports whether the Glance API Deployment has fully +// converged onto its current pod template: the deployment controller has observed +// the latest generation and every replica is updated, ready, and counted, with no +// surge or old-template pod still present. It is stricter than the surge-tolerant +// readiness deployment.EnsureDeployment reports (which turns true as soon as the +// first new-image pod is Ready while old-image pods still serve), so the shared +// upgrade flow's RollingUpdate → Contracting flip waits for the old image to drain +// before the schema is contracted. +func glanceDeploymentRolledOut(deploy *appsv1.Deployment) bool { + if deploy.Status.ObservedGeneration < deploy.Generation { + return false + } + desired := int32(1) + if deploy.Spec.Replicas != nil { + desired = *deploy.Spec.Replicas + } + return deploy.Status.UpdatedReplicas == desired && + deploy.Status.ReadyReplicas == desired && + deploy.Status.Replicas == desired +} + // buildGlanceDeployment constructs the desired Glance API Deployment. The // rendered config ConfigMap mounts at glanceConfigDir and the backends Secret at // glanceBackendsConfigDir (both oslo.config --config-dir roots). Reserved diff --git a/operators/glance/internal/controller/reconcile_deployment_test.go b/operators/glance/internal/controller/reconcile_deployment_test.go index 537c46896..3d2351b80 100644 --- a/operators/glance/internal/controller/reconcile_deployment_test.go +++ b/operators/glance/internal/controller/reconcile_deployment_test.go @@ -14,7 +14,9 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" "github.com/c5c3/forge/internal/common/database" "github.com/c5c3/forge/internal/common/keystoneauth" @@ -46,6 +48,11 @@ func readyGlanceDeployment(glance *glancev1alpha1.Glance, art configArtifacts) * deploy.Generation = 1 deploy.Status.ObservedGeneration = 1 deploy.Status.ReadyReplicas = replicas + // A ready Deployment in these tests is also fully rolled out — every replica + // updated and counted, no surge pod left — so the RollingUpdate → Contracting + // flip (which gates on full convergence, not surge-tolerant readiness) fires. + deploy.Status.UpdatedReplicas = replicas + deploy.Status.Replicas = replicas deploy.Status.Conditions = []appsv1.DeploymentCondition{ {Type: appsv1.DeploymentAvailable, Status: corev1.ConditionTrue}, } @@ -390,3 +397,105 @@ func TestReconcileDeployment_InvalidProjectionLiveDeploymentReAppliesLastGood(t g.Expect(configVol.ConfigMap.Name).To(Equal("glance-config-lastgood"), "an invalid projection with a live Deployment keeps the last-good config pinned") } + +// TestReconcileDeployment_RollingUpdateFlipsToContracting verifies that when the +// Deployment becomes ready during the RollingUpdate upgrade phase, +// reconcileDeployment advances the phase to Contracting, emits +// DeploymentRolloutComplete, sets DeploymentReady=True, and requeues immediately +// without stamping the endpoint (keystone parity). +func TestReconcileDeployment_RollingUpdateFlipsToContracting(t *testing.T) { + g := NewGomegaWithT(t) + + glance := upgradingGlance(commonv1.UpgradePhaseRollingUpdate) + glance.Spec.Deployment.Replicas = 2 + art := testArtifacts() + r := newGlanceTestReconciler(glance, readyGlanceDeployment(glance, art)) + + res, err := r.reconcileDeployment(context.Background(), glance, art, "", "") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(ctrl.Result{Requeue: true})) + + g.Expect(glance.Status.UpgradePhase).To(Equal(commonv1.UpgradePhaseContracting)) + // The endpoint is deliberately NOT stamped on the flip pass. + g.Expect(glance.Status.Endpoint).To(BeEmpty()) + + cond := meta.FindStatusCondition(glance.Status.Conditions, "DeploymentReady") + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(conditionReasonDeploymentReady)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + To(ContainElement(ContainSubstring("Normal DeploymentRolloutComplete"))) +} + +// TestReconcileDeployment_RollingUpdateSurgeReadyHoldsContract verifies the +// rollout-convergence gate: during the RollingUpdate upgrade phase a merely +// surge-ready Deployment — Available with ReadyReplicas at the desired count but +// not every replica updated (an old-image pod still serving under the default +// MaxSurge=1/MaxUnavailable=0 strategy) — MUST NOT advance to Contracting, because +// the contract phase would drop columns that old pod still reads. The phase stays +// RollingUpdate and the reconcile requeues to wait for full convergence. Regression +// guard: with the surge-tolerant IsDeploymentReady gate this flipped prematurely. +func TestReconcileDeployment_RollingUpdateSurgeReadyHoldsContract(t *testing.T) { + g := NewGomegaWithT(t) + + glance := upgradingGlance(commonv1.UpgradePhaseRollingUpdate) + glance.Spec.Deployment.Replicas = 2 + art := testArtifacts() + + // Surge-ready but not converged: Available=True and ReadyReplicas at the + // desired count (so IsDeploymentReady is true) while UpdatedReplicas lags and a + // surge pod is still counted — the exact window IsDeploymentReady tolerates. + deploy := buildGlanceDeployment(glance, art, "", "") + replicas := int32(glance.Spec.Deployment.Replicas) + deploy.Spec.Replicas = &replicas + deploy.Generation = 1 + deploy.Status.ObservedGeneration = 1 + deploy.Status.ReadyReplicas = replicas + deploy.Status.UpdatedReplicas = replicas - 1 + deploy.Status.Replicas = replicas + 1 + deploy.Status.Conditions = []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentAvailable, Status: corev1.ConditionTrue}, + } + r := newGlanceTestReconciler(glance, deploy) + + res, err := r.reconcileDeployment(context.Background(), glance, art, "", "") + g.Expect(err).NotTo(HaveOccurred()) + // Holds in RollingUpdate and requeues to wait for the old image to drain. + g.Expect(res.RequeueAfter).To(Equal(RequeueDeploymentPolling)) + g.Expect(glance.Status.UpgradePhase).To(Equal(commonv1.UpgradePhaseRollingUpdate), + "a surge-ready-but-not-converged Deployment must not flip to Contracting") + + cond := meta.FindStatusCondition(glance.Status.Conditions, "DeploymentReady") + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(conditionReasonWaitingForDeployment)) + + // No premature rollout-complete event while an old-image pod still serves. + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + NotTo(ContainElement(ContainSubstring("DeploymentRolloutComplete"))) +} + +// TestReconcileDeployment_EmptyPhaseNoFlipStampsEndpoint verifies that with no +// active upgrade (empty phase) the ready path stamps the endpoint and does NOT +// fire the RollingUpdate flip (no DeploymentRolloutComplete event). +func TestReconcileDeployment_EmptyPhaseNoFlipStampsEndpoint(t *testing.T) { + g := NewGomegaWithT(t) + + glance := deployGlance("2026.1") // empty UpgradePhase + art := testArtifacts() + r := newGlanceTestReconciler(glance, readyGlanceDeployment(glance, art)) + + res, err := r.reconcileDeployment(context.Background(), glance, art, "", "") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res.IsZero()).To(BeTrue()) + + g.Expect(glance.Status.UpgradePhase).To(BeEmpty()) + g.Expect(glance.Status.Endpoint).To(Equal("http://test-glance.default.svc.cluster.local:9292/")) + + cond := meta.FindStatusCondition(glance.Status.Conditions, "DeploymentReady") + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(conditionReasonDeploymentReady)) + g.Expect(collectEvents(r.Recorder.(*record.FakeRecorder))). + NotTo(ContainElement(ContainSubstring("DeploymentRolloutComplete"))) +} diff --git a/operators/glance/internal/controller/requeue_intervals.go b/operators/glance/internal/controller/requeue_intervals.go index 58291ef83..920e7fca0 100644 --- a/operators/glance/internal/controller/requeue_intervals.go +++ b/operators/glance/internal/controller/requeue_intervals.go @@ -44,4 +44,10 @@ const ( // longer interval avoids unnecessary API churn. It mirrors keystone's // RequeueDatabaseWait. RequeueDatabaseWait = 30 * time.Second + + // RequeueUpgradeWait is the interval for polling upgrade Job completion. + // Upgrade Jobs (expand, migrate, contract) may take several minutes depending + // on database size. A moderate interval balances responsiveness with API load. + // It mirrors keystone's RequeueUpgradeWait. + RequeueUpgradeWait = 30 * time.Second ) diff --git a/operators/keystone/api/v1alpha1/keystone_types.go b/operators/keystone/api/v1alpha1/keystone_types.go index 6de6d0781..4ce9e7d25 100644 --- a/operators/keystone/api/v1alpha1/keystone_types.go +++ b/operators/keystone/api/v1alpha1/keystone_types.go @@ -486,15 +486,19 @@ type ( GatewayParentRefSpec = commonv1.GatewayParentRefSpec ) -// UpgradePhase represents the current phase of a database upgrade. -// +kubebuilder:validation:Enum=Expanding;Migrating;RollingUpdate;Contracting -type UpgradePhase string +// UpgradePhase is aliased to the shared commonv1 definition. The database +// upgrade phase type was consolidated into internal/common/types so every +// database-backed operator shares one source of truth; commonv1 carries the +// canonical godoc and validation marker. This alias and the re-exported +// constants keep existing references — keystonev1alpha1.UpgradePhase and the +// bare UpgradePhase* constants alike — compiling unchanged. +type UpgradePhase = commonv1.UpgradePhase const ( - UpgradePhaseExpanding UpgradePhase = "Expanding" - UpgradePhaseMigrating UpgradePhase = "Migrating" - UpgradePhaseRollingUpdate UpgradePhase = "RollingUpdate" - UpgradePhaseContracting UpgradePhase = "Contracting" + UpgradePhaseExpanding UpgradePhase = commonv1.UpgradePhaseExpanding + UpgradePhaseMigrating UpgradePhase = commonv1.UpgradePhaseMigrating + UpgradePhaseRollingUpdate UpgradePhase = commonv1.UpgradePhaseRollingUpdate + UpgradePhaseContracting UpgradePhase = commonv1.UpgradePhaseContracting ) // KeystoneStatus defines the observed state of Keystone. diff --git a/operators/keystone/internal/controller/database_jobs.go b/operators/keystone/internal/controller/database_jobs.go index ba89d69c2..b0356583d 100644 --- a/operators/keystone/internal/controller/database_jobs.go +++ b/operators/keystone/internal/controller/database_jobs.go @@ -116,10 +116,11 @@ func buildDBSyncJob(keystone *keystonev1alpha1.Keystone, configMapName, domainsS } // buildUpgradeJob creates a db_sync Job for one of the expand-migrate-contract -// upgrade phases. The imageTag parameter allows callers to pin the -// image independently of spec.Image.Tag (expand/migrate use the old release, -// contract uses the new release). Upgrades only run in tag mode, so the "repo:tag" -// reference is always well-formed here. +// upgrade phases. The imageTag parameter pins the image independently of +// spec.Image.Tag; all three phases run the NEW release image (spec.image.tag) +// per the OpenStack rolling-upgrade procedure, because the N+1 migration tree +// owns the schema deltas for the upgrade. Upgrades only run in tag mode, so the +// "repo:tag" reference is always well-formed here. func buildUpgradeJob(keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName, imageTag, phase, flag string) *batchv1.Job { image := keystone.Spec.Image.Repository + ":" + imageTag return buildDBJob(keystone, configMapName, domainsSecretName, image, fmt.Sprintf("db-%s", phase), diff --git a/operators/keystone/internal/controller/reconcile_database.go b/operators/keystone/internal/controller/reconcile_database.go index 8289ec5d8..9a9ee9193 100644 --- a/operators/keystone/internal/controller/reconcile_database.go +++ b/operators/keystone/internal/controller/reconcile_database.go @@ -10,7 +10,6 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -31,43 +30,31 @@ func mariaDBResourceKey(keystone *keystonev1alpha1.Keystone) client.ObjectKey { return client.ObjectKey{Name: keystone.Name, Namespace: keystone.Namespace} } -// Condition reason constants for DatabaseReady set on the expand-migrate- -// contract upgrade path (kept keystone-private; the steady-state reasons live in -// internal/common/database as database.Reason*). Dynamic reasons generated by -// setUpgradePhaseRunning (phase+"InProgress") and setUpgradeJobFailed -// (phase+"Failed") are intentionally not extracted here because they are -// parameterised by the upgrade phase name. +// Condition and event reason constants for the expand-migrate-contract upgrade +// path, aliased to the shared vocabulary in internal/common/database so the +// keystone code and unit tests keep asserting the local names while the values +// stay in lockstep with the shared upgrade flow. Only the reasons keystone still +// emits locally (the target-changed guard in reconcileDatabase) or the tests +// reference by name are aliased here; every other upgrade reason is emitted by +// the shared flow directly. const ( - conditionReasonUpgradeTargetChanged = "UpgradeTargetChanged" - conditionReasonVersionParseError = "VersionParseError" - conditionReasonDowngradeNotSupported = "DowngradeNotSupported" - conditionReasonUpgradePathInvalid = "UpgradePathInvalid" - conditionReasonExpandInProgress = "ExpandInProgress" - conditionReasonExpandFailed = "ExpandFailed" - conditionReasonMigrateInProgress = "MigrateInProgress" - conditionReasonMigrateFailed = "MigrateFailed" - conditionReasonUpgradeRollingUpdate = "UpgradeRollingUpdate" - conditionReasonContractFailed = "ContractFailed" - conditionReasonUpgradeInitiated = "UpgradeInitiated" - conditionReasonExpandComplete = "ExpandComplete" - conditionReasonMigrateComplete = "MigrateComplete" - conditionReasonUpgradeComplete = "UpgradeComplete" - conditionReasonUpgradeAborted = "UpgradeAborted" + conditionReasonUpgradeTargetChanged = database.ReasonUpgradeTargetChanged + conditionReasonVersionParseError = database.ReasonVersionParseError + conditionReasonDowngradeNotSupported = database.ReasonDowngradeNotSupported + conditionReasonUpgradePathInvalid = database.ReasonUpgradePathInvalid + conditionReasonExpandInProgress = database.ReasonExpandInProgress + conditionReasonMigrateInProgress = database.ReasonMigrateInProgress + conditionReasonUpgradeRollingUpdate = database.ReasonUpgradeRollingUpdate ) -// upgradeJobSuffixes lists the buildDBJob nameSuffixes for the Jobs created -// during the expand-migrate-contract upgrade flow. abortUpgrade deletes exactly -// these (and not db-sync or schema-check, which belong to the steady-state path) -// so reverting an in-flight upgrade leaves no stale phase Jobs behind (#468). -var upgradeJobSuffixes = []string{"db-expand", "db-migrate", "db-contract"} - // reconcileDatabase ensures the Keystone database schema is provisioned and // migrated. In managed mode (ClusterRef set) it creates MariaDB Database, User, // and Grant CRs and waits for them to become Ready before running the db_sync // Job. In brownfield mode (Host set) it skips the MariaDB CRs and runs db_sync -// directly. The provisioning and steady-state sync flows are shared -// (internal/common/database); the expand-migrate-contract upgrade branch between -// them is keystone-private. +// directly. The provisioning, steady-state sync, and expand-migrate-contract +// upgrade flows are all shared (internal/common/database); only the +// keystone-specific inputs (assembled by upgradeFlowParams) and the +// target-changed guard below stay local. func (r *KeystoneReconciler) reconcileDatabase(ctx context.Context, keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName string) (ctrl.Result, error) { logger := log.FromContext(ctx) @@ -90,18 +77,16 @@ func (r *KeystoneReconciler) reconcileDatabase(ctx context.Context, keystone *ke return res, err } - // Active upgrade: delegate to phase-specific handling. + // Active upgrade: the shared flow handles the abort (#468) and the phase + // dispatch. The target-changed guard stays keystone-side so the error keeps + // the "image tag" prose the unit test pins (the shared flow phrases it + // generically as "spec release"). The abort case — spec.image.tag reverted + // to the installed release — bypasses the guard so the shared AbortUpgrade + // runs; it is checked first there because a revert also satisfies + // tag != targetRelease and would otherwise trip this guard. if keystone.Status.UpgradePhase != "" { - // Abort path: reverting spec.image.tag back to the installed release - // cancels the in-flight upgrade and unblocks a stuck or permanently - // failed expand/migrate phase (#468). Checked before the - // target-changed guard below because a revert also satisfies - // tag != targetRelease and would otherwise return a hard error. - if keystone.Spec.Image.Tag == keystone.Status.InstalledRelease { - return r.abortUpgrade(ctx, keystone) - } - // Detect tag change during an active upgrade. - if keystone.Spec.Image.Tag != keystone.Status.TargetRelease { + if keystone.Spec.Image.Tag != keystone.Status.InstalledRelease && + keystone.Spec.Image.Tag != keystone.Status.TargetRelease { logger.Info("Image tag changed during active upgrade", "targetRelease", keystone.Status.TargetRelease, "newTag", keystone.Spec.Image.Tag, @@ -118,12 +103,12 @@ func (r *KeystoneReconciler) reconcileDatabase(ctx context.Context, keystone *ke return ctrl.Result{}, fmt.Errorf("image tag changed during active upgrade: current upgrade targets %s but spec.image.tag is %s", keystone.Status.TargetRelease, keystone.Spec.Image.Tag) } - return r.reconcileUpgrade(ctx, keystone, configMapName, domainsSecretName) + return database.ReconcileUpgrade(ctx, r.upgradeFlowParams(ctx, keystone, configMapName, domainsSecretName)) } // Detect upgrade. if isUpgrade(keystone) { - return r.initiateUpgrade(ctx, keystone) + return database.InitiateUpgrade(ctx, r.upgradeFlowParams(ctx, keystone, configMapName, domainsSecretName)) } // Non-upgrade path: db_sync then schema-check, with terminal metrics emitted @@ -146,71 +131,56 @@ func (r *KeystoneReconciler) reconcileDatabase(ctx context.Context, keystone *ke }) } -// abortUpgrade cancels an in-flight upgrade when spec.image.tag is reverted to -// status.installedRelease. It deletes the expand/migrate/contract Jobs, clears -// status.upgradePhase/targetRelease, emits an UpgradeAborted event, and requeues -// so the steady-state db_sync path takes over and re-derives DatabaseReady on -// the next pass. This is the only escape from an upgrade wedged on a stuck or -// permanently failed expand/migrate Job (#468). -// -// Aborting is only safe before the contract phase has run. Expand and migrate -// are additive by design: they create the new columns/tables and backfill data -// without dropping anything the installed release still reads, so the -// pre-contract schema is a superset both releases run against. Contract drops -// the now-unused columns; once it has started, the installed release would be -// pointed at a schema missing fields it expects, so a post-contract revert is -// not a safe rollback. The reverted release is the operator's responsibility to -// validate. The controller does not gate the abort on the current phase. -func (r *KeystoneReconciler) abortUpgrade(ctx context.Context, keystone *keystonev1alpha1.Keystone) (ctrl.Result, error) { - logger := log.FromContext(ctx) - abortedTarget := keystone.Status.TargetRelease - abortedPhase := keystone.Status.UpgradePhase - - // Delete the phase Jobs before clearing status so a transient delete error - // leaves the upgrade state intact and the next reconcile retries the abort - // rather than dropping into the steady-state path with orphaned Jobs (#468). - if err := r.deleteUpgradeJobs(ctx, keystone); err != nil { - return ctrl.Result{}, fmt.Errorf("deleting upgrade jobs during abort: %w", err) - } - - keystone.Status.UpgradePhase = "" - keystone.Status.TargetRelease = "" - - logger.Info("Upgrade aborted: spec.image.tag reverted to installed release", - "installedRelease", keystone.Status.InstalledRelease, - "abortedTarget", abortedTarget, - "abortedPhase", abortedPhase) - r.Recorder.Eventf(keystone, corev1.EventTypeNormal, conditionReasonUpgradeAborted, - "Upgrade %s → %s aborted: spec.image.tag reverted to installed release %s", - keystone.Status.InstalledRelease, abortedTarget, keystone.Status.InstalledRelease) - return ctrl.Result{Requeue: true}, nil +// isUpgrade reports whether spec.image.tag requires the shared +// expand-migrate-contract flow for this Keystone CR. It is a thin package-private +// wrapper over database.IsUpgrade (kept because the unit test calls it directly) +// that passes the two values database.IsUpgrade reads: the spec release and the +// installed release. +func isUpgrade(keystone *keystonev1alpha1.Keystone) bool { + return database.IsUpgrade(keystone.Spec.Image.Tag, keystone.Status.InstalledRelease) } -// deleteUpgradeJobs deletes the expand/migrate/contract Jobs owned by keystone. -// NotFound is tolerated so the call is idempotent across requeues, and -// Background propagation removes each Job's owned Pods too rather than orphaning -// them (#468). -func (r *KeystoneReconciler) deleteUpgradeJobs(ctx context.Context, keystone *keystonev1alpha1.Keystone) error { - logger := log.FromContext(ctx) - for _, suffix := range upgradeJobSuffixes { - jobName := fmt.Sprintf("%s-%s", keystone.Name, suffix) - j := &batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: keystone.Namespace, - }, - } - err := r.Delete(ctx, j, client.PropagationPolicy(metav1.DeletePropagationBackground)) - switch { - case apierrors.IsNotFound(err): - logger.V(1).Info("upgrade phase job already absent during abort", "job", jobName) - case err != nil: - return fmt.Errorf("deleting %s: %w", jobName, err) - default: - logger.V(1).Info("deleted upgrade phase job during abort", "job", jobName) - } +// upgradeFlowParams assembles the shared expand-migrate-contract upgrade-flow +// inputs for this Keystone CR. The service-specific parts — the owner CR, the +// "DatabaseReady" condition vocabulary, spec.image.tag as the requested release, +// the per-phase Job builder, and the terminal-metric callback — are bound here; +// the phase choreography itself lives in internal/common/database. The three +// status pointers (UpgradePhase, InstalledRelease, TargetRelease) are mutated in +// place by the flow and persisted by the caller after it returns. +func (r *KeystoneReconciler) upgradeFlowParams(ctx context.Context, keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName string) database.UpgradeFlowParams { + return database.UpgradeFlowParams{ + Client: r.Client, + Scheme: r.Scheme, + Recorder: r.Recorder, + Owner: keystone, + Conditions: &keystone.Status.Conditions, + Generation: keystone.Generation, + ConditionType: "DatabaseReady", + RequeueAfter: RequeueUpgradeWait, + Phase: &keystone.Status.UpgradePhase, + InstalledRelease: &keystone.Status.InstalledRelease, + TargetRelease: &keystone.Status.TargetRelease, + SpecRelease: keystone.Spec.Image.Tag, + BuildPhaseJob: func(phase keystonev1alpha1.UpgradePhase) *batchv1.Job { + switch phase { + case keystonev1alpha1.UpgradePhaseExpanding: + return buildExpandJob(keystone, configMapName, domainsSecretName, keystone.Spec.Image.Tag) + case keystonev1alpha1.UpgradePhaseMigrating: + return buildMigrateJob(keystone, configMapName, domainsSecretName, keystone.Spec.Image.Tag) + case keystonev1alpha1.UpgradePhaseContracting: + return buildContractJob(keystone, configMapName, domainsSecretName, keystone.Spec.Image.Tag) + case keystonev1alpha1.UpgradePhaseRollingUpdate: + // RollingUpdate builds no Job; the Deployment rollout drives it. + return nil + default: + // The empty steady-state phase never reaches BuildPhaseJob. + return nil + } + }, + RecordTerminal: func(jobSuffix string, observed *batchv1.Job) { + r.recordDBJobTerminalState(ctx, keystone, jobSuffix, observed) + }, } - return nil } // finalizeDatabaseResources issues Delete for the MariaDB Database, User, and diff --git a/operators/keystone/internal/controller/reconcile_deployment.go b/operators/keystone/internal/controller/reconcile_deployment.go index d82820e10..9556e8d31 100644 --- a/operators/keystone/internal/controller/reconcile_deployment.go +++ b/operators/keystone/internal/controller/reconcile_deployment.go @@ -27,9 +27,6 @@ import ( keystonev1alpha1 "github.com/c5c3/forge/operators/keystone/api/v1alpha1" ) -// Condition reason constants for DeploymentReady. -const conditionReasonDeploymentRolloutComplete = "DeploymentRolloutComplete" - // buildDBConnectionEnvVar returns the EnvVar that overrides // [database].connection in keystone.conf by sourcing the URL from the derived // -db-connection Secret produced by reconcileDBConnectionSecret. @@ -130,10 +127,32 @@ func (r *KeystoneReconciler) reconcileDeployment(ctx context.Context, keystone * return ctrl.Result{RequeueAfter: RequeueDeploymentPolling}, nil } - // Transition from RollingUpdate to Contracting when Deployment is ready. - if keystone.Status.UpgradePhase == keystonev1alpha1.UpgradePhaseRollingUpdate { - keystone.Status.UpgradePhase = keystonev1alpha1.UpgradePhaseContracting - r.Recorder.Eventf(keystone, corev1.EventTypeNormal, conditionReasonDeploymentRolloutComplete, "Deployment rollout complete during upgrade %s \u2192 %s", keystone.Status.InstalledRelease, keystone.Status.TargetRelease) + // Hold the RollingUpdate → Contracting flip until the Deployment has FULLY + // converged onto the target-release image. The `ready` signal above comes from + // deployment.IsDeploymentReady, which is surge-tolerant: under the default + // MaxSurge=1/MaxUnavailable=0 strategy a new pod is added before an old one is + // removed, so it turns true as soon as the first new-image pod is Ready while + // old-image pods still serve. The contract phase then drops the now-unused + // columns those old pods still read — so gate the flip on every replica being + // updated, ready, and counted (keystoneDeploymentRolledOut), and requeue to wait + // otherwise. Only the RollingUpdate upgrade phase is stricter here; steady-state + // rollouts keep the surge-tolerant readiness above. + if keystone.Status.UpgradePhase == keystonev1alpha1.UpgradePhaseRollingUpdate && !keystoneDeploymentRolledOut(deploy) { + conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ + Type: "DeploymentReady", + Status: metav1.ConditionFalse, + ObservedGeneration: keystone.Generation, + Reason: "WaitingForDeployment", + Message: "Waiting for the upgraded image to finish rolling out before contracting the database schema", + }) + return ctrl.Result{RequeueAfter: RequeueDeploymentPolling}, nil + } + + // Transition from RollingUpdate to Contracting when the Deployment is ready. + // The shared flow advances the phase, emits the DeploymentRolloutComplete + // event, and logs; keystone stamps its own DeploymentReady condition and + // requeues so ReconcileUpgrade runs the contract phase on the next pass. + if database.CompleteRollingUpdate(ctx, r.upgradeFlowParams(ctx, keystone, configMapName, domainsSecretName)) { conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ Type: "DeploymentReady", Status: metav1.ConditionTrue, @@ -141,8 +160,6 @@ func (r *KeystoneReconciler) reconcileDeployment(ctx context.Context, keystone * Reason: "DeploymentReady", Message: "Keystone API deployment is available", }) - log.FromContext(ctx).Info("Deployment rollout complete, transitioning to contract phase", - "from", keystone.Status.InstalledRelease, "to", keystone.Status.TargetRelease) return ctrl.Result{Requeue: true}, nil } @@ -160,6 +177,27 @@ func (r *KeystoneReconciler) reconcileDeployment(ctx context.Context, keystone * return ctrl.Result{}, nil } +// keystoneDeploymentRolledOut reports whether the Keystone API Deployment has +// fully converged onto its current pod template: the deployment controller has +// observed the latest generation and every replica is updated, ready, and +// counted, with no surge or old-template pod still present. It is stricter than +// the surge-tolerant readiness deployment.EnsureDeployment reports (which turns +// true as soon as the first new-image pod is Ready while old-image pods still +// serve), so the shared upgrade flow's RollingUpdate → Contracting flip waits +// for the old image to drain before the schema is contracted. +func keystoneDeploymentRolledOut(deploy *appsv1.Deployment) bool { + if deploy.Status.ObservedGeneration < deploy.Generation { + return false + } + desired := int32(1) + if deploy.Spec.Replicas != nil { + desired = *deploy.Spec.Replicas + } + return deploy.Status.UpdatedReplicas == desired && + deploy.Status.ReadyReplicas == desired && + deploy.Status.Replicas == desired +} + // commonLabels returns the standard Kubernetes labels applied to all resources // owned by this Keystone instance. It delegates to the shared naming package // (internal/common/naming), the single source of truth for the label diff --git a/operators/keystone/internal/controller/reconcile_deployment_test.go b/operators/keystone/internal/controller/reconcile_deployment_test.go index a07c09f47..22cdb0f18 100644 --- a/operators/keystone/internal/controller/reconcile_deployment_test.go +++ b/operators/keystone/internal/controller/reconcile_deployment_test.go @@ -101,6 +101,11 @@ func readyDeployment(ks *keystonev1alpha1.Keystone, configMapName string) *appsv deploy.Generation = 1 deploy.Status.ObservedGeneration = 1 deploy.Status.ReadyReplicas = replicas + // A ready Deployment in these tests is also fully rolled out — every replica + // updated and counted, no surge pod left — so the RollingUpdate → Contracting + // flip (which gates on full convergence, not surge-tolerant readiness) fires. + deploy.Status.UpdatedReplicas = replicas + deploy.Status.Replicas = replicas deploy.Status.Conditions = []appsv1.DeploymentCondition{ { Type: appsv1.DeploymentAvailable, @@ -1270,6 +1275,56 @@ func TestReconcileDeployment_RollingUpdate_NotReady_Requeues(t *testing.T) { g.Expect(cond.Reason).To(Equal("WaitingForDeployment")) } +// TestReconcileDeployment_RollingUpdate_SurgeReady_HoldsContract verifies the +// rollout-convergence gate: during the RollingUpdate upgrade phase a merely +// surge-ready Deployment — Available with ReadyReplicas at the desired count but +// not every replica updated (an old-image pod still serving under the default +// MaxSurge=1/MaxUnavailable=0 strategy) — MUST NOT advance to Contracting, because +// the contract phase would drop columns that old pod still reads. The phase stays +// RollingUpdate and the reconcile requeues to wait for full convergence. Regression +// guard: with the surge-tolerant IsDeploymentReady gate this flipped prematurely. +func TestReconcileDeployment_RollingUpdate_SurgeReady_HoldsContract(t *testing.T) { + g := NewGomegaWithT(t) + s := deployTestScheme() + ks := deployTestKeystone() + ks.Status.InstalledRelease = "2025.2" + ks.Status.TargetRelease = "2026.1" + ks.Status.UpgradePhase = keystonev1alpha1.UpgradePhaseRollingUpdate + + // Surge-ready but not converged: Available=True and ReadyReplicas at the + // desired count (so IsDeploymentReady is true) while UpdatedReplicas lags and a + // surge pod is still counted — the exact window IsDeploymentReady tolerates. + deploy := buildKeystoneDeployment(ks, "keystone-config-abc123", "", "", nil) + replicas := int32(ks.Spec.Deployment.Replicas) + deploy.Spec.Replicas = &replicas + deploy.Generation = 1 + deploy.Status.ObservedGeneration = 1 + deploy.Status.ReadyReplicas = replicas + deploy.Status.UpdatedReplicas = replicas - 1 + deploy.Status.Replicas = replicas + 1 + deploy.Status.Conditions = []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentAvailable, Status: corev1.ConditionTrue}, + } + r := newDeployTestReconciler(s, ks, deploy) + + result, err := r.reconcileDeployment(context.Background(), ks, "keystone-config-abc123", "", "", nil) + g.Expect(err).NotTo(HaveOccurred()) + + // Holds in RollingUpdate and requeues to wait for the old image to drain. + g.Expect(result.RequeueAfter).To(Equal(RequeueDeploymentPolling)) + g.Expect(ks.Status.UpgradePhase).To(Equal(keystonev1alpha1.UpgradePhaseRollingUpdate), + "a surge-ready-but-not-converged Deployment must not flip to Contracting") + + // DeploymentReady condition must be False. + cond := meta.FindStatusCondition(ks.Status.Conditions, "DeploymentReady") + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal("WaitingForDeployment")) + + // No premature rollout-complete event while an old-image pod still serves. + expectNoEvent(g, r) +} + // TestReconcileDeployment_NoUpgrade_Ready_SetsEndpoint verifies that when there is no active // upgrade (empty UpgradePhase), the normal ready path sets the endpoint and DeploymentReady=True // without any phase transition (regression guard). diff --git a/operators/keystone/internal/controller/reconcile_upgrade.go b/operators/keystone/internal/controller/reconcile_upgrade.go deleted file mode 100644 index 43b6e89ee..000000000 --- a/operators/keystone/internal/controller/reconcile_upgrade.go +++ /dev/null @@ -1,316 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company -// -// SPDX-License-Identifier: Apache-2.0 - -package controller - -import ( - "context" - "fmt" - "strings" - - batchv1 "k8s.io/api/batch/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/log" - - "github.com/c5c3/forge/internal/common/conditions" - "github.com/c5c3/forge/internal/common/database" - "github.com/c5c3/forge/internal/common/job" - "github.com/c5c3/forge/internal/common/release" - keystonev1alpha1 "github.com/c5c3/forge/operators/keystone/api/v1alpha1" -) - -// isUpgrade returns true when spec.image.tag differs from status.installedRelease -// and the change requires the expand-migrate-contract upgrade flow. -// Returns false for fresh deployments (empty installedRelease), same version, -// and patch-only changes. -func isUpgrade(keystone *keystonev1alpha1.Keystone) bool { - // Digest-pinned images carry no tag, so the tag-keyed release/upgrade machine - // has nothing to compare — skip release detection entirely (Decision E). - if keystone.Spec.Image.Tag == "" { - return false - } - if keystone.Status.InstalledRelease == "" { - return false - } - if keystone.Spec.Image.Tag == keystone.Status.InstalledRelease { - return false - } - - from, err := release.ParseRelease(keystone.Status.InstalledRelease) - if err != nil { - // Let initiateUpgrade handle the error with proper conditions. - return true - } - to, err := release.ParseRelease(keystone.Spec.Image.Tag) - if err != nil { - return true - } - - return !release.IsPatchOnly(from, to) -} - -// initiateUpgrade validates the upgrade path and sets the initial upgrade state. -func (r *KeystoneReconciler) initiateUpgrade(ctx context.Context, keystone *keystonev1alpha1.Keystone) (ctrl.Result, error) { - logger := log.FromContext(ctx) - - from, err := release.ParseRelease(keystone.Status.InstalledRelease) - if err != nil { - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: conditionReasonVersionParseError, - Message: fmt.Sprintf("failed to parse installed release %q: %v", keystone.Status.InstalledRelease, err), - }) - r.Recorder.Eventf(keystone, corev1.EventTypeWarning, conditionReasonVersionParseError, "Failed to parse installed release %q: %v", keystone.Status.InstalledRelease, err) - return ctrl.Result{}, fmt.Errorf("parse installed release %q: %w", keystone.Status.InstalledRelease, err) - } - - to, err := release.ParseRelease(keystone.Spec.Image.Tag) - if err != nil { - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: conditionReasonVersionParseError, - Message: fmt.Sprintf("failed to parse target release %q: %v", keystone.Spec.Image.Tag, err), - }) - r.Recorder.Eventf(keystone, corev1.EventTypeWarning, conditionReasonVersionParseError, "Failed to parse target release %q: %v", keystone.Spec.Image.Tag, err) - return ctrl.Result{}, fmt.Errorf("parse target release %q: %w", keystone.Spec.Image.Tag, err) - } - - if release.IsDowngrade(from, to) { - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: conditionReasonDowngradeNotSupported, - Message: fmt.Sprintf("downgrade from %s to %s is not supported", from.Raw, to.Raw), - }) - r.Recorder.Eventf(keystone, corev1.EventTypeWarning, conditionReasonDowngradeNotSupported, "Downgrade from %s to %s is not supported", from.Raw, to.Raw) - return ctrl.Result{}, fmt.Errorf("downgrade from %s to %s is not supported", from.Raw, to.Raw) - } - - if !release.IsSequentialUpgrade(from, to) { - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: conditionReasonUpgradePathInvalid, - Message: fmt.Sprintf("upgrade from %s to %s is not sequential; only sequential upgrades are supported", from.Raw, to.Raw), - }) - r.Recorder.Eventf(keystone, corev1.EventTypeWarning, conditionReasonUpgradePathInvalid, "Upgrade from %s to %s is not sequential", from.Raw, to.Raw) - return ctrl.Result{}, fmt.Errorf("upgrade from %s to %s is not sequential; only sequential upgrades are supported", from.Raw, to.Raw) - } - - keystone.Status.TargetRelease = keystone.Spec.Image.Tag - keystone.Status.UpgradePhase = keystonev1alpha1.UpgradePhaseExpanding - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: conditionReasonExpandInProgress, - Message: fmt.Sprintf("Upgrade detected: %s \u2192 %s (phase: Expanding)", from.Raw, to.Raw), - }) - - logger.Info("Upgrade detected", "from", from.Raw, "to", to.Raw, "phase", keystonev1alpha1.UpgradePhaseExpanding) - r.Recorder.Eventf(keystone, corev1.EventTypeNormal, conditionReasonUpgradeInitiated, "Upgrade initiated: %s \u2192 %s", from.Raw, to.Raw) - return ctrl.Result{Requeue: true}, nil -} - -// setUpgradePhaseRunning sets DatabaseReady=False with reason "{phase}InProgress" -// for an upgrade phase that is currently executing. -func setUpgradePhaseRunning(keystone *keystonev1alpha1.Keystone, phase string) { - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: phase + "InProgress", - Message: fmt.Sprintf("%s phase running: %s \u2192 %s", phase, keystone.Status.InstalledRelease, keystone.Status.TargetRelease), - }) -} - -// setUpgradeJobFailed sets DatabaseReady=False with reason "{phase}Failed" -// when an upgrade job fails. -func setUpgradeJobFailed(keystone *keystonev1alpha1.Keystone, phase, jobName string, err error) { - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: phase + "Failed", - Message: fmt.Sprintf("%s job %s failed: %v", phase, jobName, err), - }) -} - -// reconcileUpgrade handles phased database migration during an active upgrade. -// It dispatches to the handler for the current UpgradePhase. -func (r *KeystoneReconciler) reconcileUpgrade(ctx context.Context, keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName string) (ctrl.Result, error) { - switch keystone.Status.UpgradePhase { - case keystonev1alpha1.UpgradePhaseExpanding: - return r.reconcileExpand(ctx, keystone, configMapName, domainsSecretName) - case keystonev1alpha1.UpgradePhaseMigrating: - return r.reconcileMigrate(ctx, keystone, configMapName, domainsSecretName) - case keystonev1alpha1.UpgradePhaseRollingUpdate: - return r.reconcileRollingUpdate(ctx, keystone) - case keystonev1alpha1.UpgradePhaseContracting: - return r.reconcileContract(ctx, keystone, configMapName, domainsSecretName) - default: - return ctrl.Result{}, fmt.Errorf("unknown upgrade phase %q", keystone.Status.UpgradePhase) - } -} - -// upgradePhaseStep describes one job-running upgrade phase. The expand, migrate, -// and contract phases share an identical build/run/record/fail/requeue skeleton -// (runUpgradePhase) and differ only in the phase name, the Job builder, the -// failure event reason, and what completion does. -type upgradePhaseStep struct { - // name is the phase name ("Expand"); it derives the "{name}InProgress" and - // "{name}Failed" condition reasons and the failure event/error wording. - name string - // jobSuffix is the recordDBJobTerminalState key ("db-expand"). - jobSuffix string - // buildJob builds the phase Job for the given image tag. - buildJob func(keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName, imageTag string) *batchv1.Job - // failReason is the Warning event reason emitted when the Job fails. - failReason string - // onComplete runs the phase-specific transition once the Job succeeds. - onComplete func(r *KeystoneReconciler, ctx context.Context, keystone *keystonev1alpha1.Keystone) (ctrl.Result, error) -} - -// runUpgradePhase executes the shared build/run/record/fail/requeue skeleton for -// one job-running upgrade phase, delegating the phase-specific transition to -// step.onComplete. The Job runs with the target image (spec.image.tag); -// see reconcileExpand for why expand and migrate also use the new release. -func (r *KeystoneReconciler) runUpgradePhase(ctx context.Context, keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName string, step upgradePhaseStep) (ctrl.Result, error) { - phaseJob := step.buildJob(keystone, configMapName, domainsSecretName, keystone.Spec.Image.Tag) - done, observed, err := job.RunJob(ctx, r.Client, r.Scheme, keystone, phaseJob) - // Emit db_sync metrics for the phase Job so the dashboard panel and - // failure-rate alerts continue to observe activity during upgrades. The Job - // RunJob already read is threaded in to avoid a re-Get. - r.recordDBJobTerminalState(ctx, keystone, step.jobSuffix, observed) - if err != nil { - setUpgradeJobFailed(keystone, step.name, phaseJob.Name, err) - r.Recorder.Eventf(keystone, corev1.EventTypeWarning, step.failReason, "%s job %s failed: %v", step.name, phaseJob.Name, err) - return ctrl.Result{}, fmt.Errorf("running %s job: %w", strings.ToLower(step.name), err) - } - if !done { - setUpgradePhaseRunning(keystone, step.name) - return ctrl.Result{RequeueAfter: RequeueUpgradeWait}, nil - } - return step.onComplete(r, ctx, keystone) -} - -// reconcileExpand runs the db_sync --expand Job using the NEW image (spec.image.tag). -// -// Per the OpenStack Keystone rolling upgrade procedure, expand migrations are -// executed with the N+1 (target) code: the N+1 alembic tree owns the schema -// deltas for the upgrade, and running expand with the N (old) binary only -// advances the expand head to the old release's HEAD. A subsequent contract -// run with the new binary then fails keystone's _validate_upgrade_order check -// with "upgrade contract ahead of expand". -func (r *KeystoneReconciler) reconcileExpand(ctx context.Context, keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName string) (ctrl.Result, error) { - return r.runUpgradePhase(ctx, keystone, configMapName, domainsSecretName, upgradePhaseStep{ - name: "Expand", - jobSuffix: "db-expand", - buildJob: buildExpandJob, - failReason: conditionReasonExpandFailed, - onComplete: (*KeystoneReconciler).completeExpand, - }) -} - -// completeExpand transitions the upgrade from Expanding to Migrating after the -// expand Job succeeds. -func (r *KeystoneReconciler) completeExpand(ctx context.Context, keystone *keystonev1alpha1.Keystone) (ctrl.Result, error) { - // Expand complete \u2014 transition to Migrating. - keystone.Status.UpgradePhase = keystonev1alpha1.UpgradePhaseMigrating - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: conditionReasonMigrateInProgress, - Message: fmt.Sprintf("Expand complete, starting migrate: %s \u2192 %s", keystone.Status.InstalledRelease, keystone.Status.TargetRelease), - }) - r.Recorder.Eventf(keystone, corev1.EventTypeNormal, conditionReasonExpandComplete, "Expand phase complete: %s \u2192 %s", keystone.Status.InstalledRelease, keystone.Status.TargetRelease) - return ctrl.Result{Requeue: true}, nil -} - -// reconcileMigrate runs the db_sync --migrate Job using the NEW image (spec.image.tag). -// -// Like expand, migrate is executed with the N+1 (target) code so that the -// alembic data-migration steps defined in the new release are applied (see -// reconcileExpand for the full rationale). -func (r *KeystoneReconciler) reconcileMigrate(ctx context.Context, keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName string) (ctrl.Result, error) { - return r.runUpgradePhase(ctx, keystone, configMapName, domainsSecretName, upgradePhaseStep{ - name: "Migrate", - jobSuffix: "db-migrate", - buildJob: buildMigrateJob, - failReason: conditionReasonMigrateFailed, - onComplete: (*KeystoneReconciler).completeMigrate, - }) -} - -// completeMigrate transitions the upgrade from Migrating to RollingUpdate after -// the migrate Job succeeds. -func (r *KeystoneReconciler) completeMigrate(ctx context.Context, keystone *keystonev1alpha1.Keystone) (ctrl.Result, error) { - // Migrate complete \u2014 transition to RollingUpdate. - keystone.Status.UpgradePhase = keystonev1alpha1.UpgradePhaseRollingUpdate - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: conditionReasonUpgradeRollingUpdate, - Message: fmt.Sprintf("Migrate complete, waiting for Deployment rollout: %s \u2192 %s", keystone.Status.InstalledRelease, keystone.Status.TargetRelease), - }) - r.Recorder.Eventf(keystone, corev1.EventTypeNormal, conditionReasonMigrateComplete, "Migrate phase complete: %s \u2192 %s", keystone.Status.InstalledRelease, keystone.Status.TargetRelease) - return ctrl.Result{Requeue: true}, nil -} - -// reconcileRollingUpdate is a pass-through that sets a condition and returns an empty -// result so the main reconcile loop proceeds to reconcileDeployment. -func (r *KeystoneReconciler) reconcileRollingUpdate(ctx context.Context, keystone *keystonev1alpha1.Keystone) (ctrl.Result, error) { - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionFalse, - ObservedGeneration: keystone.Generation, - Reason: conditionReasonUpgradeRollingUpdate, - Message: fmt.Sprintf("Waiting for Deployment rollout: %s \u2192 %s", keystone.Status.InstalledRelease, keystone.Status.TargetRelease), - }) - return ctrl.Result{}, nil -} - -// reconcileContract runs the db_sync --contract Job using the NEW image (spec.image.tag) -// and finalizes the upgrade on completion. -func (r *KeystoneReconciler) reconcileContract(ctx context.Context, keystone *keystonev1alpha1.Keystone, configMapName, domainsSecretName string) (ctrl.Result, error) { - return r.runUpgradePhase(ctx, keystone, configMapName, domainsSecretName, upgradePhaseStep{ - name: "Contract", - jobSuffix: "db-contract", - buildJob: buildContractJob, - failReason: conditionReasonContractFailed, - onComplete: (*KeystoneReconciler).completeContract, - }) -} - -// completeContract finalizes the upgrade after the contract Job succeeds: it -// promotes TargetRelease to InstalledRelease, clears the upgrade state, and -// reports DatabaseReady=True. -func (r *KeystoneReconciler) completeContract(ctx context.Context, keystone *keystonev1alpha1.Keystone) (ctrl.Result, error) { - // Contract complete \u2014 upgrade finished. - from := keystone.Status.InstalledRelease - to := keystone.Status.TargetRelease - keystone.Status.InstalledRelease = to - keystone.Status.TargetRelease = "" - keystone.Status.UpgradePhase = "" - conditions.SetCondition(&keystone.Status.Conditions, metav1.Condition{ - Type: "DatabaseReady", - Status: metav1.ConditionTrue, - ObservedGeneration: keystone.Generation, - Reason: database.ReasonDatabaseSynced, - Message: fmt.Sprintf("Database schema is up to date (upgraded %s \u2192 %s)", from, to), - }) - r.Recorder.Eventf(keystone, corev1.EventTypeNormal, conditionReasonUpgradeComplete, "Upgrade complete: %s \u2192 %s", from, to) - log.FromContext(ctx).Info("Upgrade complete", "from", from, "to", to) - return ctrl.Result{}, nil -} diff --git a/tests/e2e/glance/release-upgrade/00-glance-cr.yaml b/tests/e2e/glance/release-upgrade/00-glance-cr.yaml new file mode 100644 index 000000000..c5ca5b6c3 --- /dev/null +++ b/tests/e2e/glance/release-upgrade/00-glance-cr.yaml @@ -0,0 +1,43 @@ +# SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company +# +# SPDX-License-Identifier: Apache-2.0 + +# Glance CR fixture for the release-upgrade E2E test. +# Managed mode at the initial 2025.2 release: database via clusterRef to +# openstack-db (schema glance_release_upgrade), cache via clusterRef to +# openstack-memcached. The database credentials come from the ESO-synced +# glance-db Secret and the service-user password from the pre-seeded +# keystone-admin Secret (both deploy/kind/infrastructure). glance-api cannot +# start without a default image store, so 01-glance-backend.yaml attaches one +# before Ready can settle. A single replica keeps the rollout observable when +# 02-patch-upgrade.yaml bumps the release to 2026.1. +apiVersion: glance.openstack.c5c3.io/v1alpha1 +kind: Glance +metadata: + name: glance-release-upgrade + namespace: openstack +spec: + openStackRelease: "2025.2" + deployment: + replicas: 1 + image: + repository: ghcr.io/c5c3/glance + tag: "2025.2" + database: + clusterRef: + name: openstack-db + database: glance_release_upgrade + secretRef: + name: glance-db + cache: + clusterRef: + name: openstack-memcached + keystoneEndpoint: http://keystone.openstack.svc.cluster.local:5000/v3 + serviceUser: + username: admin + projectName: admin + userDomainName: Default + projectDomainName: Default + secretRef: + name: keystone-admin + key: password diff --git a/tests/e2e/glance/release-upgrade/01-glance-backend.yaml b/tests/e2e/glance/release-upgrade/01-glance-backend.yaml new file mode 100644 index 000000000..59b61bfe5 --- /dev/null +++ b/tests/e2e/glance/release-upgrade/01-glance-backend.yaml @@ -0,0 +1,26 @@ +# SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company +# +# SPDX-License-Identifier: Apache-2.0 + +# Default S3 image store for the glance-release-upgrade CR. It points at the +# in-cluster Garage S3 endpoint (deploy/flux-system/infrastructure/garage.yaml): +# the GarageCluster "garage" listens on port 3900 with SigV4 region "garage", +# the glance-images bucket is pre-created, and the garage-s3-credentials Secret +# is ESO-synced (deploy/kind/infrastructure). glance-api registers no store +# without it, so it must go Ready before the Glance CR can. +apiVersion: glance.openstack.c5c3.io/v1alpha1 +kind: GlanceBackend +metadata: + name: glance-release-upgrade-s3 + namespace: openstack +spec: + glanceRef: + name: glance-release-upgrade + type: S3 + isDefault: true + s3: + host: http://garage.openstack.svc.cluster.local:3900 + bucket: glance-images + region: garage + credentialsSecretRef: + name: garage-s3-credentials diff --git a/tests/e2e/glance/release-upgrade/02-patch-upgrade.yaml b/tests/e2e/glance/release-upgrade/02-patch-upgrade.yaml new file mode 100644 index 000000000..c4f4f95d6 --- /dev/null +++ b/tests/e2e/glance/release-upgrade/02-patch-upgrade.yaml @@ -0,0 +1,16 @@ +# SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company +# +# SPDX-License-Identifier: Apache-2.0 + +# Patch to trigger a cross-release upgrade from 2025.2 to 2026.1. Glance keys the +# expand-migrate-contract flow off spec.openStackRelease, and the phase Jobs run +# the release image, so both the release and the image tag are bumped together. +apiVersion: glance.openstack.c5c3.io/v1alpha1 +kind: Glance +metadata: + name: glance-release-upgrade + namespace: openstack +spec: + openStackRelease: "2026.1" + image: + tag: "2026.1" diff --git a/tests/e2e/glance/release-upgrade/chainsaw-test.yaml b/tests/e2e/glance/release-upgrade/chainsaw-test.yaml new file mode 100644 index 000000000..4cf64ed0f --- /dev/null +++ b/tests/e2e/glance/release-upgrade/chainsaw-test.yaml @@ -0,0 +1,225 @@ +# SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company +# +# SPDX-License-Identifier: Apache-2.0 + +# Chainsaw E2E test for a cross-release Glance upgrade from 2025.2 to 2026.1. +# Glance is the second consumer of the shared expand-migrate-contract database +# upgrade flow. Verifies the full upgrade lifecycle: +# (1) Fresh deployment at 2025.2 reaches Ready=True/AllReady with +# installedRelease=2025.2 and the eventlet glance-api launch command +# (2) Patching spec.openStackRelease and spec.image.tag to 2026.1 walks +# status.upgradePhase Expanding -> Migrating -> RollingUpdate -> +# Contracting -> "" via the db-expand/-db-migrate/-db-contract phase Jobs +# (3) installedRelease is promoted to 2026.1 only after the contract phase, and +# status.upgradePhase is cleared +# (4) The three phase Jobs are left in place after the upgrade succeeds +# (5) The Deployment flips to the uWSGI launch command on the :2026.1 image and +# the rollout completes +# (6) The steady-state db-sync Job re-runs on the :2026.1 image (its +# db load_metadefs pass loads the 2026.1 metadata definitions) +# +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: glance-release-upgrade +spec: + namespace: openstack + timeouts: + assert: 5m + steps: + # ── Step 1: Apply the Glance CR and its default S3 backend at 2025.2 ───── + - try: + - apply: + file: 00-glance-cr.yaml + - apply: + file: 01-glance-backend.yaml + # ── Step 2: Assert the pre-upgrade steady state and eventlet command ───── + - try: + - assert: + resource: + apiVersion: glance.openstack.c5c3.io/v1alpha1 + kind: Glance + metadata: + name: glance-release-upgrade + namespace: openstack + status: + installedRelease: "2025.2" + (conditions[?type == 'Ready']): + - status: "True" + reason: AllReady + - assert: + resource: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: glance-release-upgrade + namespace: openstack + spec: + template: + spec: + containers: + - name: glance-api + command: + - glance-api + - --config-dir + - /etc/glance/glance-api.conf.d/ + - --config-dir + - /etc/glance/backends.conf.d/ + status: + (availableReplicas > `0`): true + catch: + - script: + content: | + echo "=== Glance CR status ===" + kubectl get glance glance-release-upgrade -n $NAMESPACE -o yaml 2>&1 | tail -60 || true + echo "=== GlanceBackend status ===" + kubectl get glancebackend glance-release-upgrade-s3 -n $NAMESPACE -o yaml 2>&1 | tail -40 || true + echo "=== Operator pod logs ===" + kubectl logs -n glance-system -l app.kubernetes.io/name=glance-operator --all-containers --tail=60 2>&1 || true + kubectl logs -n glance-system -l app.kubernetes.io/name=glance-operator --all-containers --tail=60 --previous 2>&1 || true + echo "=== Glance API pod logs ===" + for pod in $(kubectl get pods -n $NAMESPACE -l app.kubernetes.io/instance=glance-release-upgrade -o name 2>/dev/null); do + echo "--- $pod ---" + kubectl logs -n $NAMESPACE "$pod" --all-containers --tail=60 2>&1 || true + kubectl logs -n $NAMESPACE "$pod" --all-containers --tail=60 --previous 2>&1 || true + done + echo "=== Events ===" + kubectl get events -n $NAMESPACE --sort-by='.lastTimestamp' 2>&1 | tail -30 || true + # ── Step 3: Patch release + image tag to trigger the cross-release upgrade ─ + - try: + - patch: + file: 02-patch-upgrade.yaml + # ── Step 4: Assert the upgrade completes via expand-migrate-contract ───── + - try: + # Gate on the settled end state first: installedRelease is promoted to + # 2026.1 only after the contract phase, and the fresh post-upgrade db-sync + # flips DatabaseReady (and so Ready) back False until it completes, so a + # stable Ready=True/AllReady here means the whole flow has drained. + - assert: + resource: + apiVersion: glance.openstack.c5c3.io/v1alpha1 + kind: Glance + metadata: + name: glance-release-upgrade + namespace: openstack + status: + installedRelease: "2026.1" + (conditions[?type == 'Ready']): + - status: "True" + reason: AllReady + - script: + content: | + set -eu + echo "=== Verifying status.upgradePhase is cleared ===" + PHASE=$(kubectl get glance glance-release-upgrade -n openstack \ + -o jsonpath='{.status.upgradePhase}') + if [ -z "$PHASE" ]; then + echo "OK: status.upgradePhase is empty" + else + echo "ERROR: status.upgradePhase still set to '$PHASE'" + exit 1 + fi + - script: + content: | + set -eu + echo "=== Verifying the expand-migrate-contract phase Jobs remain ===" + for phase in expand migrate contract; do + JOB="glance-release-upgrade-db-${phase}" + if kubectl get job "$JOB" -n openstack >/dev/null 2>&1; then + echo "OK: Job $JOB exists" + else + echo "ERROR: Job $JOB not found" + exit 1 + fi + done + # The Deployment now launches under uWSGI on the :2026.1 image and the + # single-replica rollout has fully converged (updatedReplicas == replicas). + - assert: + resource: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: glance-release-upgrade + namespace: openstack + spec: + template: + spec: + containers: + - name: glance-api + image: ghcr.io/c5c3/glance:2026.1 + command: + - uwsgi + - "--http" + - ":9292" + - "--http-auto-chunked" + - "--http-chunked-input" + - "--http-keepalive" + - "--log-master" + - "--log-format" + - "%(method) %(uri) => generated %(rsize) bytes in %(msecs) msecs (%(proto) %(status))" + - "--wsgi-file" + - "/var/lib/openstack/bin/glance-wsgi-api" + - "--master" + - "--lazy-apps" + - "--need-app" + - "--processes" + - "2" + - "--threads" + - "1" + - "--pyargv" + - "--config-dir /etc/glance/glance-api.conf.d/ --config-dir /etc/glance/backends.conf.d/" + status: + (availableReplicas > `0`): true + (updatedReplicas == replicas): true + - script: + timeout: 3m + content: | + set -eu + JOB=glance-release-upgrade-db-sync + echo "=== Verifying the steady-state db-sync Job re-ran on :2026.1 ===" + # job.RunJob deletes and re-creates a completed Job in place when its + # pod-spec-hash changes, so the same-named db-sync Job is replaced on + # the :2026.1 image after the contract phase promotes installedRelease. + # Its db load_metadefs pass loads the 2026.1 metadata definitions. Poll + # to ride out the brief delete/re-create window right after the upgrade + # settles, then assert the current Job carries :2026.1 and succeeded. + for _ in $(seq 1 30); do + IMG=$(kubectl get job "$JOB" -n openstack \ + -o jsonpath='{.spec.template.spec.containers[0].image}' 2>/dev/null || true) + SUCC=$(kubectl get job "$JOB" -n openstack \ + -o jsonpath='{.status.succeeded}' 2>/dev/null || true) + case "$IMG" in + *:2026.1) + if [ "$SUCC" = "1" ]; then + echo "OK: $JOB at $IMG succeeded=$SUCC" + exit 0 + fi + ;; + esac + sleep 5 + done + echo "ERROR: $JOB did not reach :2026.1 succeeded=1 (image=$IMG succeeded=$SUCC)" + exit 1 + catch: + - script: + content: | + echo "=== Glance CR status ===" + kubectl get glance glance-release-upgrade -n $NAMESPACE -o yaml 2>&1 | tail -60 || true + echo "=== Phase and db-sync Jobs ===" + kubectl get jobs -n $NAMESPACE -o wide 2>&1 || true + echo "=== Operator pod logs ===" + kubectl logs -n glance-system -l app.kubernetes.io/name=glance-operator --all-containers --tail=80 2>&1 || true + kubectl logs -n glance-system -l app.kubernetes.io/name=glance-operator --all-containers --tail=80 --previous 2>&1 || true + echo "=== Failed Job logs ===" + for job in $(kubectl get jobs -n $NAMESPACE -o name 2>/dev/null); do + echo "--- $job ---" + kubectl logs -n $NAMESPACE "$job" --all-containers --tail=100 2>&1 || true + done + echo "=== Glance API pod logs ===" + for pod in $(kubectl get pods -n $NAMESPACE -l app.kubernetes.io/instance=glance-release-upgrade -o name 2>/dev/null); do + echo "--- $pod ---" + kubectl logs -n $NAMESPACE "$pod" --all-containers --tail=60 2>&1 || true + kubectl logs -n $NAMESPACE "$pod" --all-containers --tail=60 --previous 2>&1 || true + done + echo "=== Events ===" + kubectl get events -n $NAMESPACE --sort-by='.lastTimestamp' 2>&1 | tail -30 || true