From 7f0dd9d4259675e129bfcae12f39db76d59810c8 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Thu, 23 Jul 2026 22:20:16 +0200 Subject: [PATCH 1/8] refactor: move the UpgradePhase type into the shared commonv1 package The expand-migrate-contract database upgrade flow is being lifted out of the keystone operator into internal/common so glance becomes its second consumer. Its phase enum must live where the shared flow can reference it, so move UpgradePhase and its four constants into internal/common/types beside the other shared database vocabulary. Keystone keeps a type alias (UpgradePhase = commonv1.UpgradePhase) and a re-exported constant block, mirroring the GatewaySpec alias pattern, so every existing keystonev1alpha1.UpgradePhase reference and bare UpgradePhase* constant keeps compiling unchanged. The generated CRD is byte-identical: the schema description is driven by the status field godoc, not the moved type godoc, and the enum marker travels with the type, so make generate/manifests/sync-crds produces no diff. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- internal/common/types/types.go | 11 +++++++++++ .../keystone/api/v1alpha1/keystone_types.go | 18 +++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) 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/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. From a6f372580b911e76447e4199e1a11992a8f0aa97 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Thu, 23 Jul 2026 22:33:06 +0200 Subject: [PATCH 2/8] feat: add the shared expand-migrate-contract upgrade flow Port keystone's private four-phase database upgrade machine (Expanding -> Migrating -> RollingUpdate -> Contracting) into internal/common/database as a parameterized flow so glance becomes its second consumer. The flow generalizes over a single seam, the spec-side release string (keystone's spec.image.tag, glance's spec.openStackRelease), supplied through UpgradeFlowParams alongside the owner CR, the condition vocabulary, the phase Job builder, and the terminal-metric callback. The exported surface mirrors keystone's semantics exactly: IsUpgrade, InitiateUpgrade, ReconcileUpgrade, AbortUpgrade, and CompleteRollingUpdate emit the same condition reasons, event reasons, and message wording as today, so the eventual keystone rewiring is behavior-preserving. Two service-specific phrasings are neutralized: "Image tag changed" becomes "Spec release changed" and "spec.image.tag reverted" becomes "spec release reverted"; the release-value substrings and the reasons keystone asserts on are unchanged. The expand-migrate-contract condition reasons move out of their keystone-private home into this package next to the steady-state Reason* set, and the package doc gains a bullet for the upgrade flow. Keystone's private machine stays in place and compiling; a later commit deletes it and rewires keystone onto this flow. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- internal/common/database/doc.go | 3 + internal/common/database/flow.go | 4 +- internal/common/database/upgrade.go | 495 +++++++++++++++++++++ internal/common/database/upgrade_test.go | 538 +++++++++++++++++++++++ 4 files changed, 1038 insertions(+), 2 deletions(-) create mode 100644 internal/common/database/upgrade.go create mode 100644 internal/common/database/upgrade_test.go 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}, + } +} From d0ac12d034bff074329e8d5384d3f297748e63da Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Thu, 23 Jul 2026 23:10:07 +0200 Subject: [PATCH 3/8] refactor: rewire keystone onto the shared database upgrade flow Delete keystone's private expand-migrate-contract machine and route the DatabaseReady upgrade path through internal/common/database, now that the shared flow added in the previous commit carries the same logic. reconcileDatabase delegates upgrade initiation, the active-phase dispatch, and the abort to database.InitiateUpgrade and database.ReconcileUpgrade; reconcileDeployment drives the RollingUpdate-to-Contracting transition through database.CompleteRollingUpdate. A new upgradeFlowParams binds the keystone-specific inputs: the owner CR, the DatabaseReady vocabulary, spec.image.tag as the requested release, the per-phase Job builders, and the terminal-metric callback. Behavior is frozen. The condition reasons, lifecycle events, db_sync metric suffixes, and the #468 abort semantics are unchanged, and the keystone unit, envtest, and e2e suites pass with no test edits. Two seams stay keystone-local to keep that behavior frozen: - The target-changed guard remains in reconcileDatabase so its error keeps the "image tag changed during active upgrade" prose the unit test pins; the shared flow phrases that guard generically as "spec release". - Only the seven upgrade reason constants keystone still emits locally or the tests reference by name are aliased to the shared database.Reason* set; aliasing the rest would leave unused constants the linter rejects. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- .../internal/controller/database_jobs.go | 9 +- .../internal/controller/reconcile_database.go | 182 +++++----- .../controller/reconcile_deployment.go | 56 +++- .../controller/reconcile_deployment_test.go | 55 +++ .../internal/controller/reconcile_upgrade.go | 316 ------------------ 5 files changed, 183 insertions(+), 435 deletions(-) delete mode 100644 operators/keystone/internal/controller/reconcile_upgrade.go 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 -} From a623a08566ba58244001f0f9d1aa19ba495717d6 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Thu, 23 Jul 2026 23:17:31 +0200 Subject: [PATCH 4/8] feat: add status.upgradePhase to the Glance CRD Glance is about to adopt the shared expand-migrate-contract database upgrade flow, which tracks the in-flight phase on the owning CR via a status pointer. Add the status.upgradePhase field to GlanceStatus so the shared flow can persist Expanding -> Migrating -> RollingUpdate -> Contracting and clear it when no upgrade is in flight, mirroring KeystoneStatus.UpgradePhase. The field reuses the shared commonv1.UpgradePhase type, so the CRD gains the same four-value enum keystone already carries. Regenerate the two Glance CRD copies (config/crd/bases and the Helm chart) accordingly; no other operator's CRD or deepcopy output changes. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- operators/glance/api/v1alpha1/glance_types.go | 5 +++++ .../crd/bases/glance.openstack.c5c3.io_glances.yaml | 11 +++++++++++ .../crds/glance.openstack.c5c3.io_glances.yaml | 11 +++++++++++ 3 files changed, 27 insertions(+) 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 From f9cd46f581391651f17f03532aad5ea1d5be4e93 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Thu, 23 Jul 2026 23:28:15 +0200 Subject: [PATCH 5/8] feat: adopt the shared upgrade flow for glance release transitions Glance becomes the second consumer of the shared expand-migrate-contract database-upgrade flow (internal/common/database), replacing its bespoke single-pass release handling. A release transition (a non-patch change from the installed release, e.g. 2025.2 -> 2026.1) now walks Expanding -> Migrating -> RollingUpdate -> Contracting with glance-manage db expand|migrate|contract phase Jobs, driven identically to keystone; status.upgradePhase tracks the active phase and TargetRelease is owned by the flow (set on initiate, cleared on completion or abort) instead of being stamped every pass. Fresh installs and patch bumps stay on the single-pass glance-manage db sync path. The accepted set of transitions is unchanged (patch-only or a single sequential step, no downgrade); only the failure reporting changes. The bespoke InvalidReleaseTransition reason and its non-zero-requeue short-circuit are gone: an invalid transition now surfaces through the shared vocabulary (VersionParseError, DowngradeNotSupported, UpgradePathInvalid) as a returned error, so the controller backs off with exponential retry exactly like keystone. The Deployment reconcile flips RollingUpdate -> Contracting once the rollout is ready, without stamping the endpoint on the flip pass. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- .../internal/controller/reconcile_database.go | 219 +++++--- .../controller/reconcile_database_test.go | 481 +++++++++++++++++- .../controller/reconcile_deployment.go | 59 +++ .../controller/reconcile_deployment_test.go | 109 ++++ .../internal/controller/requeue_intervals.go | 6 + 5 files changed, 795 insertions(+), 79 deletions(-) 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 ) From 9dbafbd0f8edf33b10b8fbe96578482deee28835 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Thu, 23 Jul 2026 23:40:48 +0200 Subject: [PATCH 6/8] test: cover the glance upgrade phase walk in envtest Add an integration test that drives a full glance release upgrade (2025.2 -> 2026.1) through the shared expand-migrate-contract flow against envtest, mirroring keystone's upgrade-cycle test. envtest runs no Job or Deployment controllers, so every phase Job completion and Deployment readiness is simulated. The test 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, asserted with a short Consistently before simulating the rollout; - the eventlet -> uWSGI launch-command switch the Deployment template takes on the release bump; - the post-upgrade steady-state db-sync Job re-running with the new image on pod-spec-hash drift. Only the glance integration test file changes. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- .../internal/controller/integration_test.go | 228 ++++++++++++++++++ 1 file changed, 228 insertions(+) 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) +} From 195664ce8b614c6023dd52a5545a247576384b9f Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Thu, 23 Jul 2026 23:48:17 +0200 Subject: [PATCH 7/8] test: add the glance release-upgrade e2e suite Cover the 2025.2 -> 2026.1 cross-release upgrade end-to-end, making glance the second consumer of the shared expand-migrate-contract database upgrade flow exercised by a chainsaw e2e suite. The suite deploys glance-release-upgrade at 2025.2 with a default S3 backend, asserts the pre-upgrade steady state (Ready/AllReady, installedRelease 2025.2, and the eventlet glance-api launch command), then patches spec.openStackRelease and spec.image.tag to 2026.1. It then asserts the upgrade drains: Ready/AllReady with installedRelease promoted to 2026.1, status.upgradePhase cleared, the db-expand, db-migrate and db-contract phase Jobs left in place, the Deployment flipped to the uWSGI launch command on the :2026.1 image with the rollout converged, and the steady-state db-sync Job re-run on the :2026.1 image (its db load_metadefs pass loads the 2026.1 metadata definitions). The suite is auto-discovered by the per-operator chainsaw run, so no CI wiring changes. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- .../glance/release-upgrade/00-glance-cr.yaml | 43 ++++ .../release-upgrade/01-glance-backend.yaml | 26 ++ .../release-upgrade/02-patch-upgrade.yaml | 16 ++ .../glance/release-upgrade/chainsaw-test.yaml | 225 ++++++++++++++++++ 4 files changed, 310 insertions(+) create mode 100644 tests/e2e/glance/release-upgrade/00-glance-cr.yaml create mode 100644 tests/e2e/glance/release-upgrade/01-glance-backend.yaml create mode 100644 tests/e2e/glance/release-upgrade/02-patch-upgrade.yaml create mode 100644 tests/e2e/glance/release-upgrade/chainsaw-test.yaml 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 From 7056e7274bf9410b23b86f2661ffbb7239fe6b3c Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Thu, 23 Jul 2026 23:59:53 +0200 Subject: [PATCH 8/8] docs: document the glance upgrade flow and fix the onboarding skill Glance now shares Keystone's expand-migrate-contract upgrade machine (internal/common/database), so a spec.openStackRelease bump with the image in lockstep walks Expanding -> Migrating -> RollingUpdate -> Contracting instead of a single-pass db sync. The reference docs and the prepare-new-service skill still described the old single-pass design, which no longer matches the code. Add a Glance Upgrade Flow reference page mirroring the Keystone one: trigger, version format, status fields, the phase table with the exact glance-manage commands and Job names, the eventlet -> uWSGI launch-mode flip during RollingUpdate, the DatabaseReady condition reasons and events, abort semantics, the image/release lockstep contract with its digest-pinning nuance, and the post-upgrade metadefs reload. Update the glance CRD, reconciler, events, and index pages: drop the "no upgradePhase field" and "reverse window is not closed" claims and every InvalidReleaseTransition mention, add the upgradePhase status row and the new condition reasons and events, and link the new page from the sidebar and reference lists. Note the shared machinery on the Keystone upgrade-flow page. In prepare-new-service, extend the Database profiling bullet to ask which zero-downtime upgrade mechanism the manage tool supports and to adopt the shared database.ReconcileUpgrade flow, and drop expand-migrate-contract from the keystone-specific classification now that it is shared. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt --- .claude/skills/prepare-new-service/SKILL.md | 13 +- docs/.vitepress/config.ts | 1 + docs/reference/glance/glance-crd.md | 16 +- docs/reference/glance/glance-events.md | 57 +++- docs/reference/glance/glance-reconciler.md | 45 ++- docs/reference/glance/glance-upgrade-flow.md | 316 ++++++++++++++++++ docs/reference/glance/index.md | 12 +- .../keystone/keystone-upgrade-flow.md | 4 +- 8 files changed, 408 insertions(+), 56 deletions(-) create mode 100644 docs/reference/glance/glance-upgrade-flow.md 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. ---