From 19ca281e058755d4644779a1321759facf4b1491 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Wed, 13 May 2026 15:00:05 +0530 Subject: [PATCH 01/13] SCF-830: Added state management for cluster hibernation --- manifests/postgresql.crd.yaml | 10 +++ pkg/apis/acid.zalan.do/v1/const.go | 2 + pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml | 10 +++ pkg/apis/acid.zalan.do/v1/postgresql_type.go | 9 ++- pkg/apis/acid.zalan.do/v1/util.go | 10 +++ .../acid.zalan.do/v1/zz_generated.deepcopy.go | 21 +++++ pkg/cluster/cluster.go | 81 +++++++++++++++++++ pkg/cluster/lifecycle.go | 78 ++++++++++++++++++ pkg/cluster/sync.go | 7 +- pkg/util/k8sutil/k8sutil.go | 10 +++ 10 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 pkg/cluster/lifecycle.go diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index 39811824e..1bba0c788 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -3246,6 +3246,13 @@ spec: - name type: object type: array + lifecycle: + description: LifecycleSpec describes the lifecycle state of a Postgres + cluster. + properties: + phase: + type: string + type: object logicalBackupRetention: type: string logicalBackupSchedule: @@ -4197,6 +4204,9 @@ spec: properties: PostgresClusterStatus: type: string + previousNumberOfInstances: + format: int32 + type: integer required: - PostgresClusterStatus type: object diff --git a/pkg/apis/acid.zalan.do/v1/const.go b/pkg/apis/acid.zalan.do/v1/const.go index 4102ea3d3..69012427a 100644 --- a/pkg/apis/acid.zalan.do/v1/const.go +++ b/pkg/apis/acid.zalan.do/v1/const.go @@ -9,6 +9,8 @@ const ( ClusterStatusSyncFailed = "SyncFailed" ClusterStatusAddFailed = "CreateFailed" ClusterStatusRunning = "Running" + ClusterStatusStopping = "Stopping" + ClusterStatusStopped = "Stopped" ClusterStatusInvalid = "Invalid" ) diff --git a/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml b/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml index 39811824e..1bba0c788 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml +++ b/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml @@ -3246,6 +3246,13 @@ spec: - name type: object type: array + lifecycle: + description: LifecycleSpec describes the lifecycle state of a Postgres + cluster. + properties: + phase: + type: string + type: object logicalBackupRetention: type: string logicalBackupSchedule: @@ -4197,6 +4204,9 @@ spec: properties: PostgresClusterStatus: type: string + previousNumberOfInstances: + format: int32 + type: integer required: - PostgresClusterStatus type: object diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 1dadfd06c..4b17cf67f 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -115,6 +115,7 @@ type PostgresSpec struct { TLS *TLSDescription `json:"tls,omitempty"` AdditionalVolumes []AdditionalVolume `json:"additionalVolumes,omitempty"` Streams []Stream `json:"streams,omitempty"` + Lifecycle *LifecycleSpec `json:"lifecycle,omitempty"` Env []v1.EnvVar `json:"env,omitempty"` // deprecated @@ -257,6 +258,11 @@ type StandbyDescription struct { StandbyPrimarySlotName string `json:"standby_primary_slot_name,omitempty"` } +// LifecycleSpec describes the lifecycle state of a Postgres cluster. +type LifecycleSpec struct { + Phase string `json:"phase,omitempty"` +} + // TLSDescription specs TLS properties type TLSDescription struct { // +required @@ -302,7 +308,8 @@ type UserFlags []string // PostgresStatus contains status of the PostgreSQL cluster (running, creation failed etc.) type PostgresStatus struct { - PostgresClusterStatus string `json:"PostgresClusterStatus"` + PostgresClusterStatus string `json:"PostgresClusterStatus"` + PreviousNumberOfInstances int32 `json:"previousNumberOfInstances,omitempty"` } // ConnectionPooler Options for connection pooler diff --git a/pkg/apis/acid.zalan.do/v1/util.go b/pkg/apis/acid.zalan.do/v1/util.go index 719defe93..7bbdc0bbf 100644 --- a/pkg/apis/acid.zalan.do/v1/util.go +++ b/pkg/apis/acid.zalan.do/v1/util.go @@ -101,6 +101,16 @@ func (postgresStatus PostgresStatus) Creating() bool { return postgresStatus.PostgresClusterStatus == ClusterStatusCreating } +// Stopping status of cluster +func (postgresStatus PostgresStatus) Stopping() bool { + return postgresStatus.PostgresClusterStatus == ClusterStatusStopping +} + +// Stopped status of cluster +func (postgresStatus PostgresStatus) Stopped() bool { + return postgresStatus.PostgresClusterStatus == ClusterStatusStopped +} + func (postgresStatus PostgresStatus) String() string { return postgresStatus.PostgresClusterStatus } diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index 159a87f35..692a6fe30 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -310,6 +310,22 @@ func (in *KubernetesMetaConfiguration) DeepCopy() *KubernetesMetaConfiguration { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LifecycleSpec) DeepCopyInto(out *LifecycleSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LifecycleSpec. +func (in *LifecycleSpec) DeepCopy() *LifecycleSpec { + if in == nil { + return nil + } + out := new(LifecycleSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadBalancerConfiguration) DeepCopyInto(out *LoadBalancerConfiguration) { *out = *in @@ -874,6 +890,11 @@ func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Lifecycle != nil { + in, out := &in.Lifecycle, &out.Lifecycle + *out = new(LifecycleSpec) + **out = **in + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make([]corev1.EnvVar, len(*in)) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 04c974f4c..bf1e24a09 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1008,6 +1008,22 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { c.mu.Lock() defer c.mu.Unlock() + // Block all spec changes when cluster is stopped or stopping + if c.Status.Stopped() || c.Status.Stopping() { + lifecyclePhase := "" + if newSpec.Spec.Lifecycle != nil { + lifecyclePhase = newSpec.Spec.Lifecycle.Phase + } + // During Stopping: block ALL spec changes (no cancellation allowed) + // During Stopped: only block if keeping lifecycle.phase="stopped" + if c.Status.Stopping() { + return fmt.Errorf("cannot update cluster while it is stopping. Wait for it to fully stop first") + } + if lifecyclePhase == "stopped" { + return fmt.Errorf("cannot update cluster while stopped. Remove lifecycle.phase to wake up the cluster") + } + } + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating newSpec, err := c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec) @@ -1015,6 +1031,71 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { return fmt.Errorf("could not set cluster status to updating: %w", err) } + // Check if user is initiating hibernate (Running -> Stopping) + if c.Status.Running() && newSpec.Spec.Lifecycle != nil && newSpec.Spec.Lifecycle.Phase == "stopped" { + c.logger.Infof("[lifecycle] initiating hibernate for cluster %s: current numberOfInstances=%d", c.Name, c.Spec.NumberOfInstances) + + // Store previousNumberOfInstances BEFORE setting numberOfInstances to 0 + newSpec.Status.PreviousNumberOfInstances = c.Spec.NumberOfInstances + newSpec.Spec.NumberOfInstances = 0 + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopping + + c.logger.Infof("[lifecycle] hibernate initiated: setting numberOfInstances=0, previousNumberOfInstances=%d", newSpec.Status.PreviousNumberOfInstances) + + // Update spec first (Update only updates spec when CR has status subresource) + pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) + if err != nil { + return fmt.Errorf("could not update spec during hibernate: %w", err) + } + c.logger.Infof("[lifecycle] hibernate: spec updated successfully") + + // Update status separately - we need to preserve the status values we set + // because UpdatePostgresCR returns object with status zeroed (subresource behavior) + pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances + pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus + + pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) + if err != nil { + return fmt.Errorf("could not update status during hibernate: %w", err) + } + c.logger.Infof("[lifecycle] hibernate: status updated successfully, previousNumberOfInstances=%d", pgUpdated.Status.PreviousNumberOfInstances) + + c.setSpec(pgUpdated) + return nil + } + + // Check if user is waking up from stopped state (Stopped -> Running) + // This is when user clears lifecycle.phase to wake up the cluster + if c.Status.Stopped() && (newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped") { + if newSpec.Status.PreviousNumberOfInstances > 0 { + c.logger.Infof("[lifecycle] waking up cluster %s: restoring numberOfInstances=%d", c.Name, newSpec.Status.PreviousNumberOfInstances) + + // Restore numberOfInstances from previousNumberOfInstances + newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating + + // Update spec first + pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) + if err != nil { + return fmt.Errorf("could not update spec during wake-up: %w", err) + } + c.logger.Infof("[lifecycle] wake-up: spec updated successfully") + + // Update status separately, and clear previousNumberOfInstances after restore + pgUpdated.Status.PreviousNumberOfInstances = 0 // Clear after successful restore + pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus + + pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) + if err != nil { + return fmt.Errorf("could not update status during wake-up: %w", err) + } + c.logger.Infof("[lifecycle] wake-up: status updated successfully, previousNumberOfInstances cleared") + + c.setSpec(pgUpdated) + return nil + } + } + if !c.isInMaintenanceWindow(newSpec.Spec.MaintenanceWindows) { // do not apply any major version related changes yet newSpec.Spec.PostgresqlParam.PgVersion = oldSpec.Spec.PostgresqlParam.PgVersion diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go new file mode 100644 index 000000000..83d58a2b9 --- /dev/null +++ b/pkg/cluster/lifecycle.go @@ -0,0 +1,78 @@ +package cluster + +import ( + acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" +) + +// manageHibernateState manages cluster hibernate/wake-up state transitions. +// Returns true if sync should continue, false if it should return early. +// +// This function handles the following state transitions: +// - Running -> Stopping: When user sets lifecycle.phase = "stopped" +// - Stopping -> Stopped: When StatefulSet replicas reach 0 +// - Stopped -> Updating: When user clears lifecycle.phase (wake-up) +// - Updating -> Running: Normal sync continues, defer sets final status +func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv1.Postgresql) bool { + + // FIX B: Detect wake-up by comparing oldSpec status vs newSpec status + // When Update() is called, it sets status=Updating before Sync() runs. + // So we need to check if oldSpec.Status was Stopped and newSpec is Updating + // with lifecycle cleared to properly detect wake-up. + isWakingUp := oldSpec.Status.Stopped() && + newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusUpdating && + (newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped") + + // FIX C: Additional wake-up detection with simpler condition + // If lifecycle was cleared and we have previousNumberOfInstances and numberOfInstances is 0 + isWakingUpSimple := newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped" + hasPreviousInstances := newSpec.Status.PreviousNumberOfInstances > 0 + needsRestore := newSpec.Spec.NumberOfInstances == 0 + + isWakingUp = isWakingUp || (isWakingUpSimple && hasPreviousInstances && needsRestore) + + // === INITIATE HIBERNATE: Running -> Stopping === + // Only initiate if not already stopping or stopped, and lifecycle.phase = "stopped" + if newSpec.Spec.Lifecycle != nil && + newSpec.Spec.Lifecycle.Phase == "stopped" && + !newSpec.Status.Stopping() && + !newSpec.Status.Stopped() { + + newSpec.Status.PreviousNumberOfInstances = newSpec.Spec.NumberOfInstances + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopping + newSpec.Spec.NumberOfInstances = 0 + c.logger.Infof("[lifecycle] cluster is going to hibernate, stored previous number of instances: %d", + newSpec.Status.PreviousNumberOfInstances) + return true + } + + // === STOPPING -> STOPPED: Check actual StatefulSet replicas === + // Only transition to Stopped when StatefulSet replicas have actually reached 0 + if newSpec.Status.Stopping() { + if c.Statefulset != nil && *c.Statefulset.Spec.Replicas == 0 { + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped + c.logger.Infof("[lifecycle] cluster has stopped, all pods are terminated") + } + return true + } + + // === WAKE-UP: Stopped/Updating -> Running === + // Restore numberOfInstances from previousNumberOfInstances when waking up + if newSpec.Status.Stopped() || isWakingUp { + // Check if lifecycle.phase was cleared (user wants to wake up) + if isWakingUp || newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped" { + if newSpec.Status.PreviousNumberOfInstances > 0 { + newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances + c.logger.Infof("[lifecycle] cluster is waking up, restoring number of instances: %d", + newSpec.Status.PreviousNumberOfInstances) + } else { + c.logger.Warningf("[lifecycle] cluster is waking up but previousNumberOfInstances is 0, cannot restore") + } + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating + return true + } + // Still stopped and lifecycle.phase = "stopped", skip further sync + return false + } + + return true +} \ No newline at end of file diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index ffebd306c..13982900c 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -47,7 +47,7 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { if err != nil { c.logger.Warningf("error while syncing cluster state: %v", err) newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusSyncFailed - } else if !c.Status.Running() { + } else if !c.Status.Running() && !c.Status.Stopping() && !c.Status.Stopped() { newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusRunning } @@ -65,6 +65,11 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { c.logger.Debugf("could not sync finalizers: %v", err) } + // Handle lifecycle hibernate/wake-up state transitions + if !c.manageHibernateState(oldSpec, newSpec) { + return nil + } + if err = c.initUsers(); err != nil { err = fmt.Errorf("could not init users: %v", err) return err diff --git a/pkg/util/k8sutil/k8sutil.go b/pkg/util/k8sutil/k8sutil.go index c34faddd4..0e31112ad 100644 --- a/pkg/util/k8sutil/k8sutil.go +++ b/pkg/util/k8sutil/k8sutil.go @@ -200,6 +200,16 @@ func (client *KubernetesClient) SetPostgresCRDStatus(clusterName spec.Namespaced return pg, nil } +// UpdatePostgresCR of Postgres cluster (updates full resource including spec) +func (client *KubernetesClient) UpdatePostgresCR(clusterName spec.NamespacedName, pg *apiacidv1.Postgresql) (*apiacidv1.Postgresql, error) { + pg, err := client.PostgresqlsGetter.Postgresqls(clusterName.Namespace).Update(context.TODO(), pg, metav1.UpdateOptions{}) + if err != nil { + return pg, fmt.Errorf("could not update PostgresCR: %v", err) + } + + return pg, nil +} + // SetFinalizer of Postgres cluster func (client *KubernetesClient) SetFinalizer(clusterName spec.NamespacedName, pg *apiacidv1.Postgresql, finalizers []string) (*apiacidv1.Postgresql, error) { var ( From 8d75f56abcbf990e04b625efc6cfcf7fb74827f8 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Wed, 13 May 2026 15:20:35 +0530 Subject: [PATCH 02/13] SCF-830: Steamlined cluster update function --- pkg/cluster/cluster.go | 189 +++++++++++++++++++++++---------------- pkg/cluster/lifecycle.go | 7 +- 2 files changed, 116 insertions(+), 80 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index bf1e24a09..e28fa46d0 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1009,91 +1009,28 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { defer c.mu.Unlock() // Block all spec changes when cluster is stopped or stopping - if c.Status.Stopped() || c.Status.Stopping() { - lifecyclePhase := "" - if newSpec.Spec.Lifecycle != nil { - lifecyclePhase = newSpec.Spec.Lifecycle.Phase - } - // During Stopping: block ALL spec changes (no cancellation allowed) - // During Stopped: only block if keeping lifecycle.phase="stopped" - if c.Status.Stopping() { - return fmt.Errorf("cannot update cluster while it is stopping. Wait for it to fully stop first") - } - if lifecyclePhase == "stopped" { - return fmt.Errorf("cannot update cluster while stopped. Remove lifecycle.phase to wake up the cluster") - } + blocked, err := c.blockLifecycleUpdate(newSpec) + if err != nil { + return err + } + if blocked { + return nil } newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating - newSpec, err := c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec) + newSpec, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec) if err != nil { return fmt.Errorf("could not set cluster status to updating: %w", err) } - // Check if user is initiating hibernate (Running -> Stopping) - if c.Status.Running() && newSpec.Spec.Lifecycle != nil && newSpec.Spec.Lifecycle.Phase == "stopped" { - c.logger.Infof("[lifecycle] initiating hibernate for cluster %s: current numberOfInstances=%d", c.Name, c.Spec.NumberOfInstances) - - // Store previousNumberOfInstances BEFORE setting numberOfInstances to 0 - newSpec.Status.PreviousNumberOfInstances = c.Spec.NumberOfInstances - newSpec.Spec.NumberOfInstances = 0 - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopping - - c.logger.Infof("[lifecycle] hibernate initiated: setting numberOfInstances=0, previousNumberOfInstances=%d", newSpec.Status.PreviousNumberOfInstances) - - // Update spec first (Update only updates spec when CR has status subresource) - pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) - if err != nil { - return fmt.Errorf("could not update spec during hibernate: %w", err) - } - c.logger.Infof("[lifecycle] hibernate: spec updated successfully") - - // Update status separately - we need to preserve the status values we set - // because UpdatePostgresCR returns object with status zeroed (subresource behavior) - pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances - pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus - - pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) - if err != nil { - return fmt.Errorf("could not update status during hibernate: %w", err) - } - c.logger.Infof("[lifecycle] hibernate: status updated successfully, previousNumberOfInstances=%d", pgUpdated.Status.PreviousNumberOfInstances) - - c.setSpec(pgUpdated) - return nil + // Handle lifecycle transitions (hibernate/wake-up) + handled, err := c.handleHibernateAndWakeUp(newSpec) + if err != nil { + return err } - - // Check if user is waking up from stopped state (Stopped -> Running) - // This is when user clears lifecycle.phase to wake up the cluster - if c.Status.Stopped() && (newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped") { - if newSpec.Status.PreviousNumberOfInstances > 0 { - c.logger.Infof("[lifecycle] waking up cluster %s: restoring numberOfInstances=%d", c.Name, newSpec.Status.PreviousNumberOfInstances) - - // Restore numberOfInstances from previousNumberOfInstances - newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating - - // Update spec first - pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) - if err != nil { - return fmt.Errorf("could not update spec during wake-up: %w", err) - } - c.logger.Infof("[lifecycle] wake-up: spec updated successfully") - - // Update status separately, and clear previousNumberOfInstances after restore - pgUpdated.Status.PreviousNumberOfInstances = 0 // Clear after successful restore - pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus - - pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) - if err != nil { - return fmt.Errorf("could not update status during wake-up: %w", err) - } - c.logger.Infof("[lifecycle] wake-up: status updated successfully, previousNumberOfInstances cleared") - - c.setSpec(pgUpdated) - return nil - } + if handled { + return nil } if !c.isInMaintenanceWindow(newSpec.Spec.MaintenanceWindows) { @@ -1301,6 +1238,106 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { return nil } +// blockLifecycleUpdate checks if an update should be blocked due to lifecycle state. +// Returns (blocked bool, err error): +// - (true, nil) if update is blocked and caller should return early +// - (false, nil) if update can proceed +// - (false, error) on error +func (c *Cluster) blockLifecycleUpdate(newSpec *acidv1.Postgresql) (bool, error) { + if !c.Status.Stopped() && !c.Status.Stopping() { + return false, nil + } + + lifecyclePhase := "" + if newSpec.Spec.Lifecycle != nil { + lifecyclePhase = newSpec.Spec.Lifecycle.Phase + } + + // During Stopping: block ALL spec changes (no cancellation allowed) + if c.Status.Stopping() { + return true, fmt.Errorf("cannot update cluster while it is stopping. Wait for it to fully stop first") + } + + // During Stopped: only block if keeping lifecycle.phase="stopped" + if lifecyclePhase == "stopped" { + return true, fmt.Errorf("cannot update cluster while stopped. Remove lifecycle.phase to wake up the cluster") + } + + return false, nil +} + +// handleHibernateAndWakeUp processes lifecycle hibernate/wake-up transitions. +// Returns (handled bool, err error): +// - (true, nil) if lifecycle transition was handled, Update() should return early +// - (false, nil) if no lifecycle transition, normal update continues +// - (false, error) on error +func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, error) { + // === INITIATE HIBERNATE: Running -> Stopping === + if c.Status.Running() && newSpec.Spec.Lifecycle != nil && newSpec.Spec.Lifecycle.Phase == "stopped" { + c.logger.Infof("[lifecycle] initiating hibernate for cluster %s: current numberOfInstances=%d", c.Name, c.Spec.NumberOfInstances) + + // Store previousNumberOfInstances BEFORE setting numberOfInstances to 0 + newSpec.Status.PreviousNumberOfInstances = c.Spec.NumberOfInstances + newSpec.Spec.NumberOfInstances = 0 + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopping + + c.logger.Infof("[lifecycle] hibernate initiated: setting numberOfInstances=0, previousNumberOfInstances=%d", newSpec.Status.PreviousNumberOfInstances) + + // Update spec first (Update only updates spec when CR has status subresource) + pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) + if err != nil { + return false, fmt.Errorf("could not update spec during hibernate: %w", err) + } + c.logger.Infof("[lifecycle] hibernate: spec updated successfully") + + // Update status separately - preserve status values since UpdatePostgresCR returns object with status zeroed + pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances + pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus + + pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) + if err != nil { + return false, fmt.Errorf("could not update status during hibernate: %w", err) + } + c.logger.Infof("[lifecycle] hibernate: status updated successfully, previousNumberOfInstances=%d", pgUpdated.Status.PreviousNumberOfInstances) + + c.setSpec(pgUpdated) + return true, nil + } + + // === WAKE-UP: Stopped -> Running === + if c.Status.Stopped() && (newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped") { + if newSpec.Status.PreviousNumberOfInstances > 0 { + c.logger.Infof("[lifecycle] waking up cluster %s: restoring numberOfInstances=%d", c.Name, newSpec.Status.PreviousNumberOfInstances) + + // Restore numberOfInstances from previousNumberOfInstances + newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating + + // Update spec first + pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) + if err != nil { + return false, fmt.Errorf("could not update spec during wake-up: %w", err) + } + c.logger.Infof("[lifecycle] wake-up: spec updated successfully") + + // Update status separately, and clear previousNumberOfInstances after restore + pgUpdated.Status.PreviousNumberOfInstances = 0 + pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus + + pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) + if err != nil { + return false, fmt.Errorf("could not update status during wake-up: %w", err) + } + c.logger.Infof("[lifecycle] wake-up: status updated successfully, previousNumberOfInstances cleared") + + c.setSpec(pgUpdated) + return true, nil + } + } + + return false, nil +} + func syncResources(a, b *v1.ResourceRequirements) bool { for _, res := range []v1.ResourceName{ v1.ResourceCPU, diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go index 83d58a2b9..bb27fd533 100644 --- a/pkg/cluster/lifecycle.go +++ b/pkg/cluster/lifecycle.go @@ -13,8 +13,6 @@ import ( // - Stopped -> Updating: When user clears lifecycle.phase (wake-up) // - Updating -> Running: Normal sync continues, defer sets final status func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv1.Postgresql) bool { - - // FIX B: Detect wake-up by comparing oldSpec status vs newSpec status // When Update() is called, it sets status=Updating before Sync() runs. // So we need to check if oldSpec.Status was Stopped and newSpec is Updating // with lifecycle cleared to properly detect wake-up. @@ -22,12 +20,12 @@ func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusUpdating && (newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped") - // FIX C: Additional wake-up detection with simpler condition // If lifecycle was cleared and we have previousNumberOfInstances and numberOfInstances is 0 isWakingUpSimple := newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped" hasPreviousInstances := newSpec.Status.PreviousNumberOfInstances > 0 needsRestore := newSpec.Spec.NumberOfInstances == 0 + // double verification of waking up isWakingUp = isWakingUp || (isWakingUpSimple && hasPreviousInstances && needsRestore) // === INITIATE HIBERNATE: Running -> Stopping === @@ -75,4 +73,5 @@ func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv } return true -} \ No newline at end of file +} + From 599a263be7b9c1204081a2535315690734520c4b Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Tue, 19 May 2026 13:25:28 +0530 Subject: [PATCH 03/13] SCF-830: Handled pooler hibernation while stopping/starting the cluster too --- manifests/postgresql.crd.yaml | 5 ++ pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml | 5 ++ pkg/apis/acid.zalan.do/v1/postgresql_type.go | 5 +- pkg/cluster/cluster.go | 14 ++- pkg/cluster/lifecycle.go | 86 +++++++++++++++++++ 5 files changed, 112 insertions(+), 3 deletions(-) diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index 1bba0c788..10e952cd2 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -4207,6 +4207,11 @@ spec: previousNumberOfInstances: format: int32 type: integer + previousPoolerInstances: + type: object + additionalProperties: + format: int32 + type: integer required: - PostgresClusterStatus type: object diff --git a/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml b/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml index 1bba0c788..10e952cd2 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml +++ b/pkg/apis/acid.zalan.do/v1/postgresql.crd.yaml @@ -4207,6 +4207,11 @@ spec: previousNumberOfInstances: format: int32 type: integer + previousPoolerInstances: + type: object + additionalProperties: + format: int32 + type: integer required: - PostgresClusterStatus type: object diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 4b17cf67f..23c8a286a 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -308,8 +308,9 @@ type UserFlags []string // PostgresStatus contains status of the PostgreSQL cluster (running, creation failed etc.) type PostgresStatus struct { - PostgresClusterStatus string `json:"PostgresClusterStatus"` - PreviousNumberOfInstances int32 `json:"previousNumberOfInstances,omitempty"` + PostgresClusterStatus string `json:"PostgresClusterStatus"` + PreviousNumberOfInstances int32 `json:"previousNumberOfInstances,omitempty"` + PreviousPoolerInstances map[string]int32 `json:"previousPoolerInstances,omitempty"` } // ConnectionPooler Options for connection pooler diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index e28fa46d0..e5b63daab 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1281,6 +1281,11 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er newSpec.Spec.NumberOfInstances = 0 newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopping + // Scale down pooler deployments and store current replica counts + if err := c.scalePoolerDown(newSpec); err != nil { + return false, fmt.Errorf("could not scale pooler during hibernate: %w", err) + } + c.logger.Infof("[lifecycle] hibernate initiated: setting numberOfInstances=0, previousNumberOfInstances=%d", newSpec.Status.PreviousNumberOfInstances) // Update spec first (Update only updates spec when CR has status subresource) @@ -1293,6 +1298,7 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er // Update status separately - preserve status values since UpdatePostgresCR returns object with status zeroed pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus + pgUpdated.Status.PreviousPoolerInstances = newSpec.Status.PreviousPoolerInstances pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) if err != nil { @@ -1309,6 +1315,11 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er if newSpec.Status.PreviousNumberOfInstances > 0 { c.logger.Infof("[lifecycle] waking up cluster %s: restoring numberOfInstances=%d", c.Name, newSpec.Status.PreviousNumberOfInstances) + // Restore pooler deployments to previous replica counts FIRST + if err := c.scalePoolerUp(newSpec); err != nil { + return false, fmt.Errorf("could not scale pooler during wake-up: %w", err) + } + // Restore numberOfInstances from previousNumberOfInstances newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating @@ -1320,9 +1331,10 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er } c.logger.Infof("[lifecycle] wake-up: spec updated successfully") - // Update status separately, and clear previousNumberOfInstances after restore + // Update status separately, and clear previousNumberOfInstances and previousPoolerInstances after restore pgUpdated.Status.PreviousNumberOfInstances = 0 pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus + pgUpdated.Status.PreviousPoolerInstances = nil pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) if err != nil { diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go index bb27fd533..ed20d7cac 100644 --- a/pkg/cluster/lifecycle.go +++ b/pkg/cluster/lifecycle.go @@ -1,6 +1,13 @@ package cluster import ( + "context" + "fmt" + + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" ) @@ -75,3 +82,82 @@ func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv return true } +// getPoolerReplicas returns the current replica count for a pooler role. +// Returns 0 if pooler doesn't exist or hasn't been synced yet. +func (c *Cluster) getPoolerReplicas(role PostgresRole) int32 { + if c.ConnectionPooler == nil || c.ConnectionPooler[role] == nil || + c.ConnectionPooler[role].Deployment == nil || + c.ConnectionPooler[role].Deployment.Spec.Replicas == nil { + return 0 + } + return *c.ConnectionPooler[role].Deployment.Spec.Replicas +} + +// scalePoolerDown scales pooler deployments to 0 and stores current replica count. +// Should be called during hibernate initiation. +func (c *Cluster) scalePoolerDown(newSpec *acidv1.Postgresql) error { + if c.ConnectionPooler == nil { + return nil + } + + for role := range c.ConnectionPooler { + replicas := c.getPoolerReplicas(role) + + // Store current replicas in status + if newSpec.Status.PreviousPoolerInstances == nil { + newSpec.Status.PreviousPoolerInstances = make(map[string]int32) + } + newSpec.Status.PreviousPoolerInstances[string(role)] = replicas + + // Scale to 0 if currently non-zero + if replicas > 0 { + if err := c.patchPoolerReplicas(role, 0); err != nil { + return err + } + c.logger.Infof("[lifecycle] pooler %s scaled to 0 (was %d)", role, replicas) + } + } + return nil +} + +// scalePoolerUp restores pooler deployments to previous replica counts. +// Should be called during wake-up. +func (c *Cluster) scalePoolerUp(newSpec *acidv1.Postgresql) error { + if newSpec.Status.PreviousPoolerInstances == nil { + return nil + } + + for roleStr, replicas := range newSpec.Status.PreviousPoolerInstances { + role := PostgresRole(roleStr) + + // Scale to stored value (could be 0) + if err := c.patchPoolerReplicas(role, replicas); err != nil { + return err + } + c.logger.Infof("[lifecycle] pooler %s scaled to %d", role, replicas) + } + return nil +} + +// patchPoolerReplicas patches the pooler deployment replica count. +func (c *Cluster) patchPoolerReplicas(role PostgresRole, replicas int32) error { + // Check if deployment exists first + _, err := c.KubeClient.Deployments(c.Namespace).Get( + context.TODO(), c.connectionPoolerName(role), metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil // Pooler doesn't exist, no-op + } + return fmt.Errorf("could not get pooler deployment for %s: %w", role, err) + } + + // Patch with merge patch + patchData := fmt.Sprintf(`{"spec":{"replicas":%d}}`, replicas) + _, err = c.KubeClient.Deployments(c.Namespace).Patch( + context.TODO(), c.connectionPoolerName(role), types.MergePatchType, []byte(patchData), metav1.PatchOptions{}) + if err != nil { + return fmt.Errorf("could not patch pooler deployment %s replicas: %w", role, err) + } + return nil +} + From ea8438f3c2a36a0103416bb42a8bc4d26a1e6c28 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Tue, 19 May 2026 13:27:37 +0530 Subject: [PATCH 04/13] SCF-830: Added feature documentation for cluster hibernation --- docs/reference/cluster_manifest.md | 37 +++++++++++++ docs/user.md | 89 ++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md index 63d400934..634320d82 100644 --- a/docs/reference/cluster_manifest.md +++ b/docs/reference/cluster_manifest.md @@ -486,6 +486,25 @@ Note that `s3_wal_path` and `gs_wal_path` are mutually exclusive. from a remote primary. See the Patroni documentation [here](https://patroni.readthedocs.io/en/latest/standby_cluster.html) for more details. Optional. +## Lifecycle configuration + +Parameters to control cluster hibernate/wake-up behavior. + +* **phase** + Set to `"stopped"` to hibernate the cluster. When this field is set on a + running cluster, the operator will: + * Store the current number of instances in the status + * Scale down the StatefulSet to 0 replicas + * Scale down the connection pooler to 0 replicas + * Set the cluster status to "Stopping", then "Stopped" + + When this field is removed from a stopped cluster, the operator will: + * Restore the number of instances from the stored value + * Scale up the StatefulSet and connection pooler + * Set the cluster status to "Updating", then "Running" + + This field is optional. When not set, the cluster operates normally. + ## Volume properties Those parameters are grouped under the `volume` top-level key and define the @@ -714,3 +733,21 @@ can have the following properties: * **memory** memory requests to be set as an annotation on the stream resource. Optional. + +## Status fields + +The operator reports the cluster state through the `status` sub-resource. These +fields are managed by the operator and should not be set manually. + +* **PostgresClusterStatus** + Current state of the cluster. One of: Creating, Updating, Running, + UpdateFailed, SyncFailed, CreateFailed, Invalid, Stopping, Stopped. + +* **previousNumberOfInstances** + The number of instances the cluster had before hibernation. Used to restore + the cluster to its previous size when waking up. Cleared after wake-up. + +* **previousPoolerInstances** + A map of connection pooler role to its replica count before hibernation. + The keys are "master" and "replica". Used to restore the pooler when waking + up. Cleared after wake-up. diff --git a/docs/user.md b/docs/user.md index 1c530a48c..54a847de9 100644 --- a/docs/user.md +++ b/docs/user.md @@ -930,6 +930,95 @@ When you apply this manifest, the operator will: The process is asynchronous. You can monitor the operator logs and the state of the `postgresql` resource to follow the progress. Once the new cluster is up and running, your applications can reconnect. +## Hibernate and Wake-up a Cluster + +The operator supports hibernating a PostgreSQL cluster to save resources when it's +not needed, and waking it up again when required. This feature: + +* Scales down the PostgreSQL StatefulSet to 0 replicas (stops all pods) +* Scales down the connection pooler to 0 replicas +* Preserves the cluster configuration and data (PVCs are retained) +* Stores the previous replica counts for automatic restoration + +### Initiating Hibernate + +To hibernate a running cluster, set the `lifecycle.phase` field to `"stopped"`: + +```yaml +apiVersion: "acid.zalan.do/v1" +kind: postgresql +metadata: + name: acid-test-cluster +spec: + teamId: "test-team" + # ... other cluster parameters + numberOfInstances: 3 + lifecycle: + phase: "stopped" +``` + +When you apply this manifest, the operator will: + +* Store the current `numberOfInstances` in `status.previousNumberOfInstances` +* Store the connection pooler replica counts in `status.previousPoolerInstances` +* Set `spec.numberOfInstances` to 0 +* Scale down the StatefulSet to 0 replicas +* Scale down the connection pooler deployments to 0 replicas +* Set `status.PostgresClusterStatus` to "Stopping", then "Stopped" + +### Waking up a Cluster + +To wake up a hibernated cluster, remove the `lifecycle.phase` field or set it to +an empty value: + +```yaml +apiVersion: "acid.zalan.do/v1" +kind: postgresql +metadata: + name: acid-test-cluster +spec: + teamId: "test-team" + # ... other cluster parameters + # lifecycle.phase is not set or is removed +``` + +When you apply this manifest, the operator will: + +* Restore `numberOfInstances` from `status.previousNumberOfInstances` +* Restore the connection pooler replica counts from `status.previousPoolerInstances` +* Scale up the StatefulSet to the previous replica count +* Scale up the connection pooler deployments to the previous replica counts +* Set `status.PostgresClusterStatus` to "Updating", then "Running" +* Clear `status.previousNumberOfInstances` and `status.previousPoolerInstances` + +### Cluster Status During Lifecycle Transitions + +| Status | Meaning | +|--------|---------| +| Running | Cluster is running normally | +| Stopping | Cluster is transitioning to stopped state (pods terminating) | +| Stopped | All pods have been terminated, cluster is hibernated | + +### Restrictions During Hibernate + +* **During Stopping**: All spec changes are blocked. You must wait for the cluster + to reach the Stopped state before making changes. + +* **During Stopped**: Spec changes are blocked unless you remove `lifecycle.phase` + to wake up the cluster. This prevents accidental modifications to a hibernated + cluster. + +### Connection Pooler Behavior + +The connection pooler is automatically scaled alongside the cluster: + +* When the cluster hibernates, the pooler is scaled to 0 replicas +* When the cluster wakes up, the pooler is restored to its previous replica count +* The previous replica counts are stored in `status.previousPoolerInstances` + +Note: If the connection pooler was already at 0 replicas before hibernate, it +will remain at 0 after wake-up. + ## Setting up a standby cluster Standby cluster is a [Patroni feature](https://github.com/zalando/patroni/blob/master/docs/replica_bootstrap.rst#standby-cluster) From 9424f7ccae83410cd031973fcbe22e2ce1d3d3da Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Wed, 20 May 2026 13:08:07 +0530 Subject: [PATCH 05/13] SCF-830: Added unit test from hibernate --- pkg/cluster/cluster_test.go | 289 ++++++++ pkg/cluster/lifecycle_test.go | 1235 +++++++++++++++++++++++++++++++++ 2 files changed, 1524 insertions(+) create mode 100644 pkg/cluster/lifecycle_test.go diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index e38540d3e..2d54bf49c 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -2354,3 +2354,292 @@ func TestUpdatePITRResources(t *testing.T) { }) } } + +func TestUpdate_LifecycleBlocksDuringStopping(t *testing.T) { + clientSet := fake.NewSimpleClientset() + acidClientSet := fakeacidv1.NewSimpleClientset() + + client := k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), + ConfigMapsGetter: clientSet.CoreV1(), + PodsGetter: clientSet.CoreV1(), + EndpointsGetter: clientSet.CoreV1(), + } + + pg := acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: 3, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Stopping", + }, + } + + cluster := New( + Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + }, + }, client, pg, logger, eventRecorder) + + cluster.Name = "test-cluster" + cluster.Namespace = "default" + + oldSpec := pg.DeepCopy() + newSpec := pg.DeepCopy() + newSpec.Spec.NumberOfInstances = 5 + + err := cluster.Update(oldSpec, newSpec) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot update cluster while it is stopping") +} + +func TestUpdate_LifecycleBlocksWhenStoppedWithPhase(t *testing.T) { + clientSet := fake.NewSimpleClientset() + acidClientSet := fakeacidv1.NewSimpleClientset() + + client := k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), + ConfigMapsGetter: clientSet.CoreV1(), + PodsGetter: clientSet.CoreV1(), + EndpointsGetter: clientSet.CoreV1(), + } + + pg := acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: 0, + Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Stopped", + PreviousNumberOfInstances: 3, + }, + } + + cluster := New( + Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + }, + }, client, pg, logger, eventRecorder) + + cluster.Name = "test-cluster" + cluster.Namespace = "default" + + oldSpec := pg.DeepCopy() + newSpec := pg.DeepCopy() + newSpec.Spec.NumberOfInstances = 5 + + err := cluster.Update(oldSpec, newSpec) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot update cluster while stopped") +} + +func TestUpdate_LifecycleAllowsWakeUp(t *testing.T) { + clientSet := fake.NewSimpleClientset() + acidClientSet := fakeacidv1.NewSimpleClientset() + + updateCalled := false + statusUpdateCalled := false + + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + updateAction := action.(k8stesting.UpdateAction) + pg := updateAction.GetObject().(*acidv1.Postgresql) + updateCalled = true + return true, pg, nil + }) + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + if action.GetSubresource() == "status" { + statusUpdateCalled = true + updateAction := action.(k8stesting.UpdateAction) + pg := updateAction.GetObject().(*acidv1.Postgresql) + return true, pg, nil + } + return false, nil, nil + }) + + client := k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), + ConfigMapsGetter: clientSet.CoreV1(), + PodsGetter: clientSet.CoreV1(), + EndpointsGetter: clientSet.CoreV1(), + } + + pg := acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: 0, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Stopped", + PreviousNumberOfInstances: 3, + }, + } + + cluster := New( + Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + }, + }, client, pg, logger, eventRecorder) + + cluster.Name = "test-cluster" + cluster.Namespace = "default" + + oldSpec := pg.DeepCopy() + newSpec := pg.DeepCopy() + + err := cluster.Update(oldSpec, newSpec) + + assert.NoError(t, err) + assert.True(t, updateCalled, "Update should have been called for wake-up") + assert.True(t, statusUpdateCalled, "Status update should have been called for wake-up") +} + +func TestUpdate_LifecycleInitiatesHibernate(t *testing.T) { + clientSet := fake.NewSimpleClientset() + acidClientSet := fakeacidv1.NewSimpleClientset() + + updateCalled := false + statusUpdateCalled := false + + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + updateAction := action.(k8stesting.UpdateAction) + pg := updateAction.GetObject().(*acidv1.Postgresql) + updateCalled = true + return true, pg, nil + }) + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + if action.GetSubresource() == "status" { + statusUpdateCalled = true + updateAction := action.(k8stesting.UpdateAction) + pg := updateAction.GetObject().(*acidv1.Postgresql) + return true, pg, nil + } + return false, nil, nil + }) + + client := k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), + ConfigMapsGetter: clientSet.CoreV1(), + PodsGetter: clientSet.CoreV1(), + EndpointsGetter: clientSet.CoreV1(), + } + + pg := acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: 3, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Running", + }, + } + + cluster := New( + Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + }, + }, client, pg, logger, eventRecorder) + + cluster.Name = "test-cluster" + cluster.Namespace = "default" + + oldSpec := pg.DeepCopy() + newSpec := pg.DeepCopy() + newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: "stopped"} + + err := cluster.Update(oldSpec, newSpec) + + assert.NoError(t, err) + assert.True(t, updateCalled, "Update should have been called for hibernate") + assert.True(t, statusUpdateCalled, "Status update should have been called for hibernate") +} + +func TestUpdate_LifecycleNormalUpdate(t *testing.T) { + clientSet := fake.NewSimpleClientset() + acidClientSet := fakeacidv1.NewSimpleClientset() + + client := k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), + ConfigMapsGetter: clientSet.CoreV1(), + PodsGetter: clientSet.CoreV1(), + EndpointsGetter: clientSet.CoreV1(), + } + + pg := acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: 3, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Running", + }, + } + + cluster := New( + Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + }, + }, client, pg, logger, eventRecorder) + + cluster.Name = "test-cluster" + cluster.Namespace = "default" + + newSpec := pg.DeepCopy() + blocked, err := cluster.blockLifecycleUpdate(newSpec) + + assert.False(t, blocked) + assert.NoError(t, err) +} diff --git a/pkg/cluster/lifecycle_test.go b/pkg/cluster/lifecycle_test.go new file mode 100644 index 000000000..b3dd90d0c --- /dev/null +++ b/pkg/cluster/lifecycle_test.go @@ -0,0 +1,1235 @@ +package cluster + +import ( + "context" + "fmt" + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" + fakeacidv1 "github.com/zalando/postgres-operator/pkg/generated/clientset/versioned/fake" + "github.com/zalando/postgres-operator/pkg/util/config" + "github.com/zalando/postgres-operator/pkg/util/k8sutil" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" + "k8s.io/client-go/tools/record" +) + +var lifecycleLogger = logrus.New().WithField("test", "lifecycle") +var lifecycleEventRecorder = record.NewFakeRecorder(10) + +func int32Ptr(i int32) *int32 { return &i } + +func newTestLifecycleCluster(status string, numberOfInstances int32, lifecyclePhase string) *Cluster { + pg := acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: numberOfInstances, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: status, + }, + } + + if lifecyclePhase != "" { + pg.Spec.Lifecycle = &acidv1.LifecycleSpec{ + Phase: lifecyclePhase, + } + } + + return &Cluster{ + Config: Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + }, + }, + Postgresql: pg, + logger: lifecycleLogger, + eventRecorder: lifecycleEventRecorder, + } +} + +func newTestPoolerObjects(role PostgresRole, replicas int32) *ConnectionPoolerObjects { + return &ConnectionPoolerObjects{ + Deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("test-%s-pooler", role), + Namespace: "default", + }, + Spec: appsv1.DeploymentSpec{ + Replicas: int32Ptr(replicas), + }, + }, + Name: fmt.Sprintf("test-%s-pooler", role), + ClusterName: "test-cluster", + Namespace: "default", + Role: role, + } +} + +func newFakeK8sClientForLifecycle() (*k8sutil.KubernetesClient, *fake.Clientset, *fakeacidv1.Clientset) { + clientSet := fake.NewSimpleClientset() + acidClientSet := fakeacidv1.NewSimpleClientset() + + client := &k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), + ConfigMapsGetter: clientSet.CoreV1(), + PodsGetter: clientSet.CoreV1(), + EndpointsGetter: clientSet.CoreV1(), + } + + return client, clientSet, acidClientSet +} + +func createTestPostgresqlInClient(client *k8sutil.KubernetesClient, name, namespace string, spec *acidv1.PostgresSpec, status *acidv1.PostgresStatus) *acidv1.Postgresql { + pg := &acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + if spec != nil { + pg.Spec = *spec + } + if status != nil { + pg.Status = *status + } + + created, err := client.Postgresqls(namespace).Create(context.TODO(), pg, metav1.CreateOptions{}) + if err == nil { + return created + } + return pg +} + +func updatePostgresqlInClient(client *k8sutil.KubernetesClient, pg *acidv1.Postgresql) (*acidv1.Postgresql, error) { + return client.Postgresqls(pg.Namespace).Update(context.TODO(), pg, metav1.UpdateOptions{}) +} + +func updatePostgresqlStatusInClient(client *k8sutil.KubernetesClient, pg *acidv1.Postgresql) (*acidv1.Postgresql, error) { + return client.Postgresqls(pg.Namespace).UpdateStatus(context.TODO(), pg, metav1.UpdateOptions{}) +} + +func TestGetPoolerReplicas(t *testing.T) { + tests := []struct { + name string + poolerObjs map[PostgresRole]*ConnectionPoolerObjects + role PostgresRole + want int32 + }{ + { + name: "nil ConnectionPooler map", + poolerObjs: nil, + role: Master, + want: 0, + }, + { + name: "ConnectionPooler for role is nil", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{Master: nil}, + role: Master, + want: 0, + }, + { + name: "Deployment is nil", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{Master: {Deployment: nil}}, + role: Master, + want: 0, + }, + { + name: "Replicas is nil", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{ + Master: {Deployment: &appsv1.Deployment{Spec: appsv1.DeploymentSpec{Replicas: nil}}}, + }, + role: Master, + want: 0, + }, + { + name: "Master with 2 replicas", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{ + Master: newTestPoolerObjects(Master, 2), + }, + role: Master, + want: 2, + }, + { + name: "Master with 0 replicas", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{ + Master: newTestPoolerObjects(Master, 0), + }, + role: Master, + want: 0, + }, + { + name: "Replica with 3 replicas", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{ + Replica: newTestPoolerObjects(Replica, 3), + }, + role: Replica, + want: 3, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Cluster{ + ConnectionPooler: tt.poolerObjs, + } + got := c.getPoolerReplicas(tt.role) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestPatchPoolerReplicas(t *testing.T) { + tests := []struct { + name string + replicas int32 + setupClient func(clientSet *fake.Clientset) + wantErr bool + errContains string + }{ + { + name: "deployment exists, patch succeeds", + replicas: 0, + setupClient: func(clientSet *fake.Clientset) { + // Pre-create the deployment in the fake clientset + clientSet.AppsV1().Deployments("default").Create(context.TODO(), &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-pooler"}, + Spec: appsv1.DeploymentSpec{Replicas: int32Ptr(2)}, + }, metav1.CreateOptions{}) + }, + wantErr: false, + }, + { + name: "deployment not found", + replicas: 2, + setupClient: func(clientSet *fake.Clientset) { + // Don't create anything - will trigger NotFound + }, + wantErr: false, // NotFound is handled gracefully + }, + { + name: "patch returns error", + replicas: 2, + setupClient: func(clientSet *fake.Clientset) { + // Pre-create deployment but make patch fail via reactor + clientSet.AppsV1().Deployments("default").Create(context.TODO(), &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-pooler"}, + Spec: appsv1.DeploymentSpec{Replicas: int32Ptr(2)}, + }, metav1.CreateOptions{}) + clientSet.PrependReactor("patch", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("network error") + }) + }, + wantErr: true, + errContains: "could not patch pooler deployment", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientSet := fake.NewSimpleClientset() + if tt.setupClient != nil { + tt.setupClient(clientSet) + } + + kubeClient := &k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + } + + c := &Cluster{ + KubeClient: *kubeClient, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + } + + err := c.patchPoolerReplicas(Master, tt.replicas) + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestBlockLifecycleUpdate(t *testing.T) { + tests := []struct { + name string + currentStatus string + lifecyclePhase string + wantBlocked bool + wantErr bool + errContains string + }{ + { + name: "Running cluster, allows update", + currentStatus: "Running", + wantBlocked: false, + wantErr: false, + }, + { + name: "Stopping state, blocks update", + currentStatus: "Stopping", + wantBlocked: true, + wantErr: true, + errContains: "cannot update cluster while it is stopping", + }, + { + name: "Stopped with lifecycle phase, blocks update", + currentStatus: "Stopped", + lifecyclePhase: "stopped", + wantBlocked: true, + wantErr: true, + errContains: "cannot update cluster while stopped", + }, + { + name: "Stopped without lifecycle phase, allows update (wake-up)", + currentStatus: "Stopped", + lifecyclePhase: "", + wantBlocked: false, + wantErr: false, + }, + { + name: "Stopped with nil lifecycle, allows update (wake-up)", + currentStatus: "Stopped", + lifecyclePhase: "", + wantBlocked: false, + wantErr: false, + }, + { + name: "Stopped with empty lifecycle phase, allows update (wake-up)", + currentStatus: "Stopped", + lifecyclePhase: "", + wantBlocked: false, + wantErr: false, + }, + { + name: "UpdateFailed state, allows update", + currentStatus: "UpdateFailed", + wantBlocked: false, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := newTestLifecycleCluster(tt.currentStatus, 3, tt.lifecyclePhase) + + newSpec := c.DeepCopy() + if tt.lifecyclePhase != "" && tt.currentStatus != "Stopped" { + newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: tt.lifecyclePhase} + } + + blocked, err := c.blockLifecycleUpdate(newSpec) + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.wantBlocked, blocked) + }) + } +} + +func TestManageHibernateState(t *testing.T) { + tests := []struct { + name string + oldSpecStatus string + newSpecStatus string + newSpecLifecyclePhase string + numberOfInstances int32 + previousNumberOfInst int32 + statefulsetReplicas *int32 + wantContinue bool + wantNumberOfInstances *int32 + wantStatus string + }{ + { + name: "Running to Stopping - initiates hibernate", + oldSpecStatus: "Running", + newSpecStatus: "Running", + newSpecLifecyclePhase: "stopped", + numberOfInstances: 3, + statefulsetReplicas: int32Ptr(3), + wantContinue: true, + wantNumberOfInstances: int32Ptr(0), + wantStatus: "Stopping", + }, + { + name: "Stopping to Stopped - when replicas reach 0", + oldSpecStatus: "Stopping", + newSpecStatus: "Stopping", + numberOfInstances: 0, + statefulsetReplicas: int32Ptr(0), + wantContinue: true, + wantNumberOfInstances: int32Ptr(0), + wantStatus: "Stopped", + }, + { + name: "Stopping - replicas not yet 0", + oldSpecStatus: "Stopping", + newSpecStatus: "Stopping", + numberOfInstances: 0, + statefulsetReplicas: int32Ptr(2), // Still terminating + wantContinue: true, + wantNumberOfInstances: nil, // Should NOT change + wantStatus: "Stopping", // Should NOT change + }, + { + name: "Stopped to wake-up - restores numberOfInstances", + oldSpecStatus: "Stopped", + newSpecStatus: "Stopped", + newSpecLifecyclePhase: "", // Cleared + numberOfInstances: 0, + previousNumberOfInst: 3, + statefulsetReplicas: int32Ptr(0), + wantContinue: true, + wantNumberOfInstances: int32Ptr(3), + wantStatus: "Updating", + }, + { + name: "Stopped but lifecycle still 'stopped' - skip sync", + oldSpecStatus: "Stopped", + newSpecStatus: "Stopped", + newSpecLifecyclePhase: "stopped", + numberOfInstances: 0, + statefulsetReplicas: int32Ptr(0), + wantContinue: false, // Should skip sync + wantNumberOfInstances: nil, + wantStatus: "Stopped", + }, + { + name: "Running without lifecycle - continue normal", + oldSpecStatus: "Running", + newSpecStatus: "Running", + newSpecLifecyclePhase: "", + numberOfInstances: 3, + statefulsetReplicas: int32Ptr(3), + wantContinue: true, + wantNumberOfInstances: int32Ptr(3), // Unchanged + wantStatus: "Running", // Unchanged + }, + { + name: "Running to Updating - normal update", + oldSpecStatus: "Running", + newSpecStatus: "Updating", + newSpecLifecyclePhase: "", + numberOfInstances: 3, + statefulsetReplicas: int32Ptr(3), + wantContinue: true, + wantNumberOfInstances: int32Ptr(3), + wantStatus: "Updating", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Cluster{ + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + } + + if tt.statefulsetReplicas != nil { + c.Statefulset = &appsv1.StatefulSet{ + Spec: appsv1.StatefulSetSpec{ + Replicas: tt.statefulsetReplicas, + }, + } + } + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: tt.oldSpecStatus, + }, + } + + newSpec := &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{ + NumberOfInstances: tt.numberOfInstances, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: tt.newSpecStatus, + PreviousNumberOfInstances: tt.previousNumberOfInst, + }, + } + + if tt.newSpecLifecyclePhase != "" { + newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{ + Phase: tt.newSpecLifecyclePhase, + } + } + + gotContinue := c.manageHibernateState(oldSpec, newSpec) + + assert.Equal(t, tt.wantContinue, gotContinue) + + if tt.wantNumberOfInstances != nil { + assert.Equal(t, *tt.wantNumberOfInstances, newSpec.Spec.NumberOfInstances) + } + assert.Equal(t, tt.wantStatus, newSpec.Status.PostgresClusterStatus) + }) + } +} + +func TestHandleHibernateAndWakeUp_Hibernate(t *testing.T) { + tests := []struct { + name string + currentStatus string + numberOfInstances int32 + lifecyclePhase string + poolerObjs map[PostgresRole]*ConnectionPoolerObjects + k8sUpdateSucceeds bool + k8sStatusSucceeds bool + poolerPatchSucceeds bool + wantHandled bool + wantErr bool + errContains string + }{ + { + name: "Running + lifecycle.phase=stopped - initiates hibernate", + currentStatus: "Running", + numberOfInstances: 3, + lifecyclePhase: "stopped", + k8sUpdateSucceeds: true, + k8sStatusSucceeds: true, + poolerPatchSucceeds: true, + wantHandled: true, + wantErr: false, + }, + { + name: "Running + lifecycle.phase=stopped - K8s update fails", + currentStatus: "Running", + numberOfInstances: 3, + lifecyclePhase: "stopped", + k8sUpdateSucceeds: false, + wantHandled: false, + wantErr: true, + errContains: "could not update spec during hibernate", + }, + { + name: "Running + lifecycle.phase=stopped - K8s status update fails", + currentStatus: "Running", + numberOfInstances: 3, + lifecyclePhase: "stopped", + k8sUpdateSucceeds: true, + k8sStatusSucceeds: false, + wantHandled: false, + wantErr: true, + errContains: "could not update status during hibernate", + }, + { + name: "Running + no lifecycle phase - no transition", + currentStatus: "Running", + numberOfInstances: 3, + lifecyclePhase: "", + wantHandled: false, + wantErr: false, + }, + { + name: "Stopped + lifecycle cleared - no-op (Update handles this)", + currentStatus: "Stopped", + numberOfInstances: 0, + lifecyclePhase: "", + wantHandled: false, // handleHibernateAndWakeUp doesn't handle wake-up, that's done via Update flow + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kubeClient, _, acidClientSet := newFakeK8sClientForLifecycle() + + if tt.k8sUpdateSucceeds { + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + updateAction := action.(k8stesting.UpdateAction) + pg := updateAction.GetObject().(*acidv1.Postgresql) + return true, pg, nil + }) + } + + if !tt.k8sStatusSucceeds { + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + if action.GetSubresource() == "status" { + return true, nil, fmt.Errorf("status update failed") + } + return false, nil, nil + }) + } + + c := &Cluster{ + Config: Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + }, + }, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: tt.numberOfInstances, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: tt.currentStatus, + }, + }, + KubeClient: *kubeClient, + ConnectionPooler: tt.poolerObjs, + logger: lifecycleLogger, + eventRecorder: lifecycleEventRecorder, + } + + newSpec := c.DeepCopy() + if tt.lifecyclePhase != "" { + newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: tt.lifecyclePhase} + } + + handled, err := c.handleHibernateAndWakeUp(newSpec) + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.wantHandled, handled) + }) + } +} + +func TestHandleHibernateAndWakeUp_WakeUp(t *testing.T) { + tests := []struct { + name string + currentStatus string + numberOfInstances int32 + previousNumberOfInst int32 + previousPoolerInstances map[string]int32 + lifecyclePhase string + k8sUpdateSucceeds bool + k8sStatusSucceeds bool + poolerPatchSucceeds bool + wantHandled bool + wantErr bool + errContains string + }{ + { + name: "Stopped + lifecycle cleared - wake-up", + currentStatus: "Stopped", + numberOfInstances: 0, + previousNumberOfInst: 3, + previousPoolerInstances: map[string]int32{"master": 2, "replica": 0}, + lifecyclePhase: "", + k8sUpdateSucceeds: true, + k8sStatusSucceeds: true, + wantHandled: true, + wantErr: false, + }, + { + name: "Stopped + previousNumberOfInstances is 0", + currentStatus: "Stopped", + numberOfInstances: 0, + previousNumberOfInst: 0, + lifecyclePhase: "", + wantHandled: false, // No restore possible + wantErr: false, + }, + { + name: "Stopped + lifecycle cleared + K8s update fails", + currentStatus: "Stopped", + numberOfInstances: 0, + previousNumberOfInst: 3, + lifecyclePhase: "", + k8sUpdateSucceeds: false, + wantHandled: false, + wantErr: true, + errContains: "could not update spec during wake-up", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kubeClient, _, acidClientSet := newFakeK8sClientForLifecycle() + + if tt.k8sUpdateSucceeds { + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + updateAction := action.(k8stesting.UpdateAction) + pg := updateAction.GetObject().(*acidv1.Postgresql) + return true, pg, nil + }) + } + + c := &Cluster{ + Config: Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + }, + }, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: tt.numberOfInstances, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: tt.currentStatus, + PreviousNumberOfInstances: tt.previousNumberOfInst, + PreviousPoolerInstances: tt.previousPoolerInstances, + }, + }, + KubeClient: *kubeClient, + logger: lifecycleLogger, + eventRecorder: lifecycleEventRecorder, + } + + newSpec := c.DeepCopy() + + handled, err := c.handleHibernateAndWakeUp(newSpec) + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.wantHandled, handled) + }) + } +} + +func TestScalePoolerDown(t *testing.T) { + tests := []struct { + name string + poolerObjs map[PostgresRole]*ConnectionPoolerObjects + wantStored map[string]int32 + }{ + { + name: "nil ConnectionPooler - no-op", + poolerObjs: nil, + wantStored: nil, + }, + { + name: "Master at 2 replicas", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{Master: newTestPoolerObjects(Master, 2)}, + wantStored: map[string]int32{"master": 2}, + }, + { + name: "Master already at 0 replicas", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{Master: newTestPoolerObjects(Master, 0)}, + wantStored: map[string]int32{"master": 0}, + }, + { + name: "Both Master and Replica", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{ + Master: newTestPoolerObjects(Master, 2), + Replica: newTestPoolerObjects(Replica, 1), + }, + wantStored: map[string]int32{"master": 2, "replica": 1}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientSet := fake.NewSimpleClientset() + kubeClient := &k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + } + + c := &Cluster{ + KubeClient: *kubeClient, + ConnectionPooler: tt.poolerObjs, + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + } + + newSpec := &acidv1.Postgresql{} + err := c.scalePoolerDown(newSpec) + + assert.NoError(t, err) + assert.Equal(t, tt.wantStored, newSpec.Status.PreviousPoolerInstances) + }) + } +} + +func TestScalePoolerUp(t *testing.T) { + tests := []struct { + name string + prevInst map[string]int32 + wantErr bool + }{ + { + name: "nil PreviousPoolerInstances - no-op", + prevInst: nil, + }, + { + name: "Restore master to 2", + prevInst: map[string]int32{"master": 2}, + }, + { + name: "Restore both roles", + prevInst: map[string]int32{"master": 2, "replica": 1}, + }, + { + name: "Restore master to 0 (keep at 0)", + prevInst: map[string]int32{"master": 0}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientSet := fake.NewSimpleClientset() + kubeClient := &k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + } + + c := &Cluster{ + KubeClient: *kubeClient, + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + } + + newSpec := &acidv1.Postgresql{ + Status: acidv1.PostgresStatus{ + PreviousPoolerInstances: tt.prevInst, + }, + } + err := c.scalePoolerUp(newSpec) + + assert.NoError(t, err) + }) + } +} + +func TestLifecycleStateTransitions(t *testing.T) { + t.Run("complete Hibernate flow: Running -> Stopping -> Stopped", func(t *testing.T) { + c := &Cluster{ + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + }, + } + + // Initial: Running + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, + } + newSpec := &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 3, + Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, + }, + Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, + } + + // Step 1: manageHibernateState should initiate hibernate + continueSync := c.manageHibernateState(oldSpec, newSpec) + assert.True(t, continueSync) + assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances) + assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) + assert.Equal(t, int32(3), newSpec.Status.PreviousNumberOfInstances) + }) + + t.Run("complete Wake-up flow: Stopped -> Updating -> Running", func(t *testing.T) { + c := &Cluster{ + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + }, + } + + // After hibernate: Stopped, replicas = 0 + c.Statefulset = &appsv1.StatefulSet{ + Spec: appsv1.StatefulSetSpec{Replicas: int32Ptr(0)}, + } + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: "Stopped"}, + } + newSpec := &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 0, + // Lifecycle cleared by user + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Stopped", + PreviousNumberOfInstances: 3, + }, + } + + // Step 1: manageHibernateState should restore + continueSync := c.manageHibernateState(oldSpec, newSpec) + assert.True(t, continueSync) + assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) + assert.Equal(t, "Updating", newSpec.Status.PostgresClusterStatus) + }) +} + +func TestLifecycleUpdateBlocksDuringStopping(t *testing.T) { + kubeClient, _, _ := newFakeK8sClientForLifecycle() + + c := &Cluster{ + Config: Config{ + OpConfig: config.Config{}, + }, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 0, + Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Stopping", + }, + }, + KubeClient: *kubeClient, + logger: lifecycleLogger, + } + + newSpec := c.DeepCopy() + blocked, err := c.blockLifecycleUpdate(newSpec) + + assert.True(t, blocked) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot update cluster while it is stopping") +} + +func TestLifecycleUpdateBlocksWhenStoppedWithPhase(t *testing.T) { + kubeClient, _, _ := newFakeK8sClientForLifecycle() + + c := &Cluster{ + Config: Config{ + OpConfig: config.Config{}, + }, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 0, + Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Stopped", + }, + }, + KubeClient: *kubeClient, + logger: lifecycleLogger, + } + + newSpec := c.DeepCopy() + blocked, err := c.blockLifecycleUpdate(newSpec) + + assert.True(t, blocked) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot update cluster while stopped") +} + +func TestLifecycleUpdateAllowsWakeUp(t *testing.T) { + kubeClient, _, _ := newFakeK8sClientForLifecycle() + + c := &Cluster{ + Config: Config{ + OpConfig: config.Config{}, + }, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 0, + // Lifecycle cleared by user + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Stopped", + PreviousNumberOfInstances: 3, + }, + }, + KubeClient: *kubeClient, + logger: lifecycleLogger, + } + + newSpec := c.DeepCopy() + blocked, err := c.blockLifecycleUpdate(newSpec) + + assert.False(t, blocked) + assert.NoError(t, err) +} + +func TestManageHibernateState_EdgeCases(t *testing.T) { + tests := []struct { + name string + statefulsetReplicas *int32 + currentStatus string + newSpecLifecyclePhase string + newSpecNumberOfInst int32 + previousNumberOfInst int32 + wantContinue bool + wantNumberOfInstances *int32 + wantStatus string + }{ + { + name: "Stopping state with nil statefulset - should not transition to Stopped", + statefulsetReplicas: nil, + currentStatus: "Stopping", + newSpecLifecyclePhase: "stopped", + newSpecNumberOfInst: 0, + wantContinue: true, + wantNumberOfInstances: nil, + wantStatus: "Stopping", + }, + { + name: "Stopping with nil Lifecycle spec", + statefulsetReplicas: int32Ptr(0), + currentStatus: "Stopping", + newSpecLifecyclePhase: "", + newSpecNumberOfInst: 0, + wantContinue: true, + wantNumberOfInstances: nil, + wantStatus: "Stopped", + }, + { + name: "Running to Stopping when numberOfInstances already 0", + statefulsetReplicas: int32Ptr(0), + currentStatus: "Running", + newSpecLifecyclePhase: "stopped", + newSpecNumberOfInst: 0, + wantContinue: true, + wantNumberOfInstances: int32Ptr(0), + wantStatus: "Stopping", + }, + { + name: "Stopped with previousNumberOfInstances=0 - sets Updating but warns", + statefulsetReplicas: int32Ptr(0), + currentStatus: "Stopped", + newSpecLifecyclePhase: "", + newSpecNumberOfInst: 0, + previousNumberOfInst: 0, + wantContinue: true, + wantNumberOfInstances: int32Ptr(0), + wantStatus: "Updating", + }, + { + name: "Stopped with nil Lifecycle - wake-up", + statefulsetReplicas: int32Ptr(0), + currentStatus: "Stopped", + newSpecLifecyclePhase: "", + newSpecNumberOfInst: 0, + previousNumberOfInst: 3, + wantContinue: true, + wantNumberOfInstances: int32Ptr(3), + wantStatus: "Updating", + }, + { + name: "Stopped with empty Lifecycle phase - wake-up", + statefulsetReplicas: int32Ptr(0), + currentStatus: "Stopped", + newSpecLifecyclePhase: "", + newSpecNumberOfInst: 0, + previousNumberOfInst: 2, + wantContinue: true, + wantNumberOfInstances: int32Ptr(2), + wantStatus: "Updating", + }, + { + name: "Stopped but lifecycle still 'stopped' - skip sync", + statefulsetReplicas: int32Ptr(0), + currentStatus: "Stopped", + newSpecLifecyclePhase: "stopped", + newSpecNumberOfInst: 0, + previousNumberOfInst: 3, + wantContinue: false, + wantNumberOfInstances: nil, + wantStatus: "Stopped", + }, + { + name: "Running without lifecycle change - normal update", + statefulsetReplicas: int32Ptr(3), + currentStatus: "Running", + newSpecLifecyclePhase: "", + newSpecNumberOfInst: 3, + previousNumberOfInst: 0, + wantContinue: true, + wantNumberOfInstances: int32Ptr(3), + wantStatus: "Running", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Cluster{ + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + } + + if tt.statefulsetReplicas != nil { + c.Statefulset = &appsv1.StatefulSet{ + Spec: appsv1.StatefulSetSpec{ + Replicas: tt.statefulsetReplicas, + }, + } + } + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: tt.currentStatus, + }, + } + + newSpec := &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{ + NumberOfInstances: tt.newSpecNumberOfInst, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: tt.currentStatus, + PreviousNumberOfInstances: tt.previousNumberOfInst, + }, + } + + if tt.newSpecLifecyclePhase != "" || tt.newSpecLifecyclePhase == "" && tt.currentStatus == "Stopped" { + newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{ + Phase: tt.newSpecLifecyclePhase, + } + } + + gotContinue := c.manageHibernateState(oldSpec, newSpec) + + assert.Equal(t, tt.wantContinue, gotContinue, "continue sync mismatch") + if tt.wantNumberOfInstances != nil { + assert.Equal(t, *tt.wantNumberOfInstances, newSpec.Spec.NumberOfInstances, "numberOfInstances mismatch") + } + assert.Equal(t, tt.wantStatus, newSpec.Status.PostgresClusterStatus, "status mismatch") + }) + } +} + +func TestManageHibernateState_StateTransitionSequence(t *testing.T) { + t.Run("Running -> Stopping -> Stopped sequence", func(t *testing.T) { + c := &Cluster{ + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + }, + } + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, + } + newSpec := &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 3, + Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, + }, + Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, + } + + continueSync := c.manageHibernateState(oldSpec, newSpec) + assert.True(t, continueSync) + assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances) + assert.Equal(t, int32(3), newSpec.Status.PreviousNumberOfInstances) + assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) + + c.Statefulset = &appsv1.StatefulSet{ + Spec: appsv1.StatefulSetSpec{Replicas: int32Ptr(2)}, + } + oldSpec = acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: "Stopping"}, + } + newSpec.Status.PostgresClusterStatus = "Stopping" + + continueSync = c.manageHibernateState(oldSpec, newSpec) + assert.True(t, continueSync) + assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) + + c.Statefulset.Spec.Replicas = int32Ptr(0) + continueSync = c.manageHibernateState(oldSpec, newSpec) + assert.True(t, continueSync) + assert.Equal(t, "Stopped", newSpec.Status.PostgresClusterStatus) + }) + + t.Run("Stopped -> Updating -> Running sequence", func(t *testing.T) { + c := &Cluster{ + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + }, + } + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: "Stopped"}, + } + newSpec := &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 0, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: "Stopped", + PreviousNumberOfInstances: 3, + }, + } + + continueSync := c.manageHibernateState(oldSpec, newSpec) + assert.True(t, continueSync) + assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) + assert.Equal(t, "Updating", newSpec.Status.PostgresClusterStatus) + + oldSpec = acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: "Updating"}, + } + newSpec.Status.PostgresClusterStatus = "Updating" + + continueSync = c.manageHibernateState(oldSpec, newSpec) + assert.True(t, continueSync) + assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) + }) +} \ No newline at end of file From 0f4eabe55ad8fe43ba3831cebdfc79b5e336f32b Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Wed, 20 May 2026 14:47:13 +0530 Subject: [PATCH 06/13] SCF-830: Added cronjob pause/resume while cluster hibernation --- docs/user.md | 12 +++ pkg/cluster/cluster.go | 22 +++- pkg/cluster/lifecycle.go | 2 +- pkg/cluster/lifecycle_test.go | 191 ++++++++++++++++++++++++++++++++++ pkg/cluster/resources.go | 52 +++++++++ 5 files changed, 275 insertions(+), 4 deletions(-) diff --git a/docs/user.md b/docs/user.md index 54a847de9..5cca700a3 100644 --- a/docs/user.md +++ b/docs/user.md @@ -964,6 +964,7 @@ When you apply this manifest, the operator will: * Set `spec.numberOfInstances` to 0 * Scale down the StatefulSet to 0 replicas * Scale down the connection pooler deployments to 0 replicas +* Suspend the logical backup CronJob (if enabled) * Set `status.PostgresClusterStatus` to "Stopping", then "Stopped" ### Waking up a Cluster @@ -986,6 +987,7 @@ When you apply this manifest, the operator will: * Restore `numberOfInstances` from `status.previousNumberOfInstances` * Restore the connection pooler replica counts from `status.previousPoolerInstances` +* Resume the logical backup CronJob (if enabled) * Scale up the StatefulSet to the previous replica count * Scale up the connection pooler deployments to the previous replica counts * Set `status.PostgresClusterStatus` to "Updating", then "Running" @@ -1019,6 +1021,16 @@ The connection pooler is automatically scaled alongside the cluster: Note: If the connection pooler was already at 0 replicas before hibernate, it will remain at 0 after wake-up. +### Logical Backup Behavior + +The logical backup CronJob is automatically suspended during hibernate: + +* When the cluster hibernates, the backup CronJob is suspended (`.spec.suspend: true`) +* When the cluster wakes up, the backup CronJob is automatically resumed +* The backup schedule is preserved and resumes from its normal schedule + +This prevents failed backup jobs from running when the database is unavailable. + ## Setting up a standby cluster Standby cluster is a [Patroni feature](https://github.com/zalando/patroni/blob/master/docs/replica_bootstrap.rst#standby-cluster) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index e5b63daab..dfe328821 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1286,6 +1286,14 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er return false, fmt.Errorf("could not scale pooler during hibernate: %w", err) } + // Suspend logical backup CronJob if enabled + if c.Spec.EnableLogicalBackup { + if err := c.suspendLogicalBackupJob(); err != nil { + return false, fmt.Errorf("could not suspend logical backup job during hibernate: %w", err) + } + c.logger.Info("[lifecycle] logical backup job suspended") + } + c.logger.Infof("[lifecycle] hibernate initiated: setting numberOfInstances=0, previousNumberOfInstances=%d", newSpec.Status.PreviousNumberOfInstances) // Update spec first (Update only updates spec when CR has status subresource) @@ -1293,7 +1301,7 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er if err != nil { return false, fmt.Errorf("could not update spec during hibernate: %w", err) } - c.logger.Infof("[lifecycle] hibernate: spec updated successfully") + c.logger.Info("[lifecycle] hibernate: spec updated successfully") // Update status separately - preserve status values since UpdatePostgresCR returns object with status zeroed pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances @@ -1320,6 +1328,14 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er return false, fmt.Errorf("could not scale pooler during wake-up: %w", err) } + // Resume logical backup CronJob if enabled + if c.Spec.EnableLogicalBackup { + if err := c.unsuspendLogicalBackupJob(); err != nil { + return false, fmt.Errorf("could not resume logical backup job during wake-up: %w", err) + } + c.logger.Info("[lifecycle] logical backup job resumed") + } + // Restore numberOfInstances from previousNumberOfInstances newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating @@ -1329,7 +1345,7 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er if err != nil { return false, fmt.Errorf("could not update spec during wake-up: %w", err) } - c.logger.Infof("[lifecycle] wake-up: spec updated successfully") + c.logger.Info("[lifecycle] wake-up: spec updated successfully") // Update status separately, and clear previousNumberOfInstances and previousPoolerInstances after restore pgUpdated.Status.PreviousNumberOfInstances = 0 @@ -1340,7 +1356,7 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er if err != nil { return false, fmt.Errorf("could not update status during wake-up: %w", err) } - c.logger.Infof("[lifecycle] wake-up: status updated successfully, previousNumberOfInstances cleared") + c.logger.Info("[lifecycle] wake-up: status updated successfully, previousNumberOfInstances cleared") c.setSpec(pgUpdated) return true, nil diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go index ed20d7cac..249e43ec3 100644 --- a/pkg/cluster/lifecycle.go +++ b/pkg/cluster/lifecycle.go @@ -55,7 +55,7 @@ func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv if newSpec.Status.Stopping() { if c.Statefulset != nil && *c.Statefulset.Spec.Replicas == 0 { newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped - c.logger.Infof("[lifecycle] cluster has stopped, all pods are terminated") + c.logger.Info("[lifecycle] cluster has stopped, all pods are terminated") } return true } diff --git a/pkg/cluster/lifecycle_test.go b/pkg/cluster/lifecycle_test.go index b3dd90d0c..b43f43cd0 100644 --- a/pkg/cluster/lifecycle_test.go +++ b/pkg/cluster/lifecycle_test.go @@ -12,6 +12,7 @@ import ( "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/k8sutil" appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" @@ -1232,4 +1233,194 @@ func TestManageHibernateState_StateTransitionSequence(t *testing.T) { assert.True(t, continueSync) assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) }) +} + +func TestSuspendLogicalBackupJob(t *testing.T) { + tests := []struct { + name string + jobExists bool + patchFails bool + wantErr bool + }{ + { + name: "job exists, suspend succeeds", + jobExists: true, + wantErr: false, + }, + { + name: "job does not exist - no-op", + jobExists: false, + wantErr: false, + }, + { + name: "job exists but patch fails", + jobExists: true, + patchFails: true, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientSet := fake.NewSimpleClientset() + jobName := "logical-backup-test-cluster" + + if tt.jobExists { + clientSet.BatchV1().CronJobs("default").Create(context.TODO(), &batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: jobName, + Namespace: "default", + }, + Spec: batchv1.CronJobSpec{ + Schedule: "30 00 * * *", + }, + }, metav1.CreateOptions{}) + } + + if tt.patchFails { + clientSet.PrependReactor("patch", "cronjobs", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("network error") + }) + } + + kubeClient := &k8sutil.KubernetesClient{ + CronJobsGetter: clientSet.BatchV1(), + } + + var job *batchv1.CronJob + if tt.jobExists { + job, _ = kubeClient.CronJobs("default").Get(context.TODO(), jobName, metav1.GetOptions{}) + } + + c := New( + Config{ + OpConfig: config.Config{ + LogicalBackup: config.LogicalBackup{ + LogicalBackupJobPrefix: "logical-backup-", + }, + }, + }, + *kubeClient, + acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + lifecycleLogger, + lifecycleEventRecorder, + ) + c.LogicalBackupJob = job + + err := c.suspendLogicalBackupJob() + + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + if tt.jobExists && !tt.patchFails { + updatedJob, _ := kubeClient.CronJobs("default").Get(context.TODO(), jobName, metav1.GetOptions{}) + if updatedJob != nil { + assert.True(t, *updatedJob.Spec.Suspend, "job should be suspended") + } + } + } + }) + } +} + +func TestUnsuspendLogicalBackupJob(t *testing.T) { + tests := []struct { + name string + jobExists bool + patchFails bool + wantErr bool + }{ + { + name: "job exists, unsuspend succeeds", + jobExists: true, + wantErr: false, + }, + { + name: "job does not exist - no-op", + jobExists: false, + wantErr: false, + }, + { + name: "job exists but patch fails", + jobExists: true, + patchFails: true, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientSet := fake.NewSimpleClientset() + jobName := "logical-backup-test-cluster" + + if tt.jobExists { + suspendTrue := true + clientSet.BatchV1().CronJobs("default").Create(context.TODO(), &batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: jobName, + Namespace: "default", + }, + Spec: batchv1.CronJobSpec{ + Schedule: "30 00 * * *", + Suspend: &suspendTrue, + }, + }, metav1.CreateOptions{}) + } + + if tt.patchFails { + clientSet.PrependReactor("patch", "cronjobs", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("network error") + }) + } + + kubeClient := &k8sutil.KubernetesClient{ + CronJobsGetter: clientSet.BatchV1(), + } + + var job *batchv1.CronJob + if tt.jobExists { + job, _ = kubeClient.CronJobs("default").Get(context.TODO(), jobName, metav1.GetOptions{}) + } + + c := New( + Config{ + OpConfig: config.Config{ + LogicalBackup: config.LogicalBackup{ + LogicalBackupJobPrefix: "logical-backup-", + }, + }, + }, + *kubeClient, + acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + lifecycleLogger, + lifecycleEventRecorder, + ) + c.LogicalBackupJob = job + + err := c.unsuspendLogicalBackupJob() + + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + if tt.jobExists && !tt.patchFails { + updatedJob, _ := kubeClient.CronJobs("default").Get(context.TODO(), jobName, metav1.GetOptions{}) + if updatedJob != nil { + assert.False(t, *updatedJob.Spec.Suspend, "job should be unsuspended") + } + } + } + }) + } } \ No newline at end of file diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 1925733de..07dfae0c1 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -825,6 +825,58 @@ func (c *Cluster) deleteLogicalBackupJob() error { return nil } +func (c *Cluster) suspendLogicalBackupJob() error { + if c.LogicalBackupJob == nil { + c.logger.Debug("logical backup job is not loaded, skipping suspend") + return nil + } + + c.setProcessName("suspending logical backup job") + + patchData := fmt.Sprintf(`{"spec":{"suspend":true}}`) + cronJob, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Patch( + context.TODO(), + c.getLogicalBackupJobName(), + types.MergePatchType, + []byte(patchData), + metav1.PatchOptions{}, + "", + ) + if err != nil { + return fmt.Errorf("could not suspend logical backup job: %w", err) + } + c.LogicalBackupJob = cronJob + c.logger.Info("logical backup job suspended") + + return nil +} + +func (c *Cluster) unsuspendLogicalBackupJob() error { + if c.LogicalBackupJob == nil { + c.logger.Debug("logical backup job is not loaded, skipping unsuspend") + return nil + } + + c.setProcessName("resuming logical backup job") + + patchData := fmt.Sprintf(`{"spec":{"suspend":false}}`) + cronJob, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Patch( + context.TODO(), + c.getLogicalBackupJobName(), + types.MergePatchType, + []byte(patchData), + metav1.PatchOptions{}, + "", + ) + if err != nil { + return fmt.Errorf("could not resume logical backup job: %w", err) + } + c.LogicalBackupJob = cronJob + c.logger.Info("logical backup job resumed") + + return nil +} + // GetServiceMaster returns cluster's kubernetes master Service func (c *Cluster) GetServiceMaster() *v1.Service { return c.Services[Master] From 8c8147a4d8a95ea4ab2b367bd0478e7c02868f10 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Thu, 21 May 2026 16:20:29 +0530 Subject: [PATCH 07/13] SCF-830: Refactored into a common function for spec update, pesisting in update --- pkg/cluster/cluster.go | 115 +++------- pkg/cluster/lifecycle.go | 387 ++++++++++++++++++++++++++++------ pkg/cluster/lifecycle_test.go | 14 +- pkg/cluster/resources.go | 50 ----- 4 files changed, 358 insertions(+), 208 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index dfe328821..1d22a6609 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1266,101 +1266,46 @@ func (c *Cluster) blockLifecycleUpdate(newSpec *acidv1.Postgresql) (bool, error) return false, nil } -// handleHibernateAndWakeUp processes lifecycle hibernate/wake-up transitions. +// handleHibernateAndWakeUp handles cluster hibernate/wake-up transitions during Update(). +// This is the Update path - it detects the transition, modifies the spec via manageHibernateState, +// and persists the changes to Kubernetes. +// // Returns (handled bool, err error): // - (true, nil) if lifecycle transition was handled, Update() should return early // - (false, nil) if no lifecycle transition, normal update continues -// - (false, error) on error +// - (false, error) on error during persistence +// +// Flow: +// 1. Detect action via detectLifecycleTransition() +// 2. Check for Stopping->Stopped (via detectStoppingCompleted) +// 3. Call manageHibernateState() to prepare the spec (same logic as Sync path) +// 4. Persist to Kubernetes via persistHibernateTransition/persistWakeUpTransition func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, error) { - // === INITIATE HIBERNATE: Running -> Stopping === - if c.Status.Running() && newSpec.Spec.Lifecycle != nil && newSpec.Spec.Lifecycle.Phase == "stopped" { - c.logger.Infof("[lifecycle] initiating hibernate for cluster %s: current numberOfInstances=%d", c.Name, c.Spec.NumberOfInstances) - - // Store previousNumberOfInstances BEFORE setting numberOfInstances to 0 - newSpec.Status.PreviousNumberOfInstances = c.Spec.NumberOfInstances - newSpec.Spec.NumberOfInstances = 0 - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopping - - // Scale down pooler deployments and store current replica counts - if err := c.scalePoolerDown(newSpec); err != nil { - return false, fmt.Errorf("could not scale pooler during hibernate: %w", err) - } - - // Suspend logical backup CronJob if enabled - if c.Spec.EnableLogicalBackup { - if err := c.suspendLogicalBackupJob(); err != nil { - return false, fmt.Errorf("could not suspend logical backup job during hibernate: %w", err) - } - c.logger.Info("[lifecycle] logical backup job suspended") - } - - c.logger.Infof("[lifecycle] hibernate initiated: setting numberOfInstances=0, previousNumberOfInstances=%d", newSpec.Status.PreviousNumberOfInstances) - - // Update spec first (Update only updates spec when CR has status subresource) - pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) - if err != nil { - return false, fmt.Errorf("could not update spec during hibernate: %w", err) - } - c.logger.Info("[lifecycle] hibernate: spec updated successfully") - - // Update status separately - preserve status values since UpdatePostgresCR returns object with status zeroed - pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances - pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus - pgUpdated.Status.PreviousPoolerInstances = newSpec.Status.PreviousPoolerInstances + action := detectLifecycleTransition( + &c.Status, + c.Spec.Lifecycle, + newSpec.Spec.Lifecycle, + newSpec.Spec.NumberOfInstances, + newSpec.Status.PreviousNumberOfInstances, + c.Status.Running(), + ) - pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) - if err != nil { - return false, fmt.Errorf("could not update status during hibernate: %w", err) + if action == LifecycleActionNone { + if detectStoppingCompleted(&c.Status, c.getStatefulsetReplicas()) { + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped + return c.persistStoppingCompletedTransition(newSpec) } - c.logger.Infof("[lifecycle] hibernate: status updated successfully, previousNumberOfInstances=%d", pgUpdated.Status.PreviousNumberOfInstances) - - c.setSpec(pgUpdated) - return true, nil + return false, nil } - // === WAKE-UP: Stopped -> Running === - if c.Status.Stopped() && (newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped") { - if newSpec.Status.PreviousNumberOfInstances > 0 { - c.logger.Infof("[lifecycle] waking up cluster %s: restoring numberOfInstances=%d", c.Name, newSpec.Status.PreviousNumberOfInstances) - - // Restore pooler deployments to previous replica counts FIRST - if err := c.scalePoolerUp(newSpec); err != nil { - return false, fmt.Errorf("could not scale pooler during wake-up: %w", err) - } - - // Resume logical backup CronJob if enabled - if c.Spec.EnableLogicalBackup { - if err := c.unsuspendLogicalBackupJob(); err != nil { - return false, fmt.Errorf("could not resume logical backup job during wake-up: %w", err) - } - c.logger.Info("[lifecycle] logical backup job resumed") - } - - // Restore numberOfInstances from previousNumberOfInstances - newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating + c.manageHibernateState(c.Postgresql, newSpec) - // Update spec first - pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) - if err != nil { - return false, fmt.Errorf("could not update spec during wake-up: %w", err) - } - c.logger.Info("[lifecycle] wake-up: spec updated successfully") + switch action { + case LifecycleActionHibernate: + return c.persistHibernateTransition(newSpec) - // Update status separately, and clear previousNumberOfInstances and previousPoolerInstances after restore - pgUpdated.Status.PreviousNumberOfInstances = 0 - pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus - pgUpdated.Status.PreviousPoolerInstances = nil - - pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) - if err != nil { - return false, fmt.Errorf("could not update status during wake-up: %w", err) - } - c.logger.Info("[lifecycle] wake-up: status updated successfully, previousNumberOfInstances cleared") - - c.setSpec(pgUpdated) - return true, nil - } + case LifecycleActionWakeUp: + return c.persistWakeUpTransition(newSpec) } return false, nil diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go index 249e43ec3..eb6a4ee03 100644 --- a/pkg/cluster/lifecycle.go +++ b/pkg/cluster/lifecycle.go @@ -11,79 +11,330 @@ import ( acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" ) -// manageHibernateState manages cluster hibernate/wake-up state transitions. -// Returns true if sync should continue, false if it should return early. +// LifecycleAction represents the detected lifecycle transition for a cluster. +// Used by both Sync (manageHibernateState) and Update (handleHibernateAndWakeUp) paths +// to determine what action to take regarding cluster hibernate/wake-up. +type LifecycleAction int + +const ( + LifecycleActionNone LifecycleAction = iota + LifecycleActionHibernate // Running -> Stopping (initiate hibernate) + LifecycleActionStoppingCompleted // Stopping -> Stopped (pods fully terminated) + LifecycleActionWakeUp // Stopped -> Updating (initiate wake-up) +) + +// detectLifecycleTransition is a pure function that examines the current and proposed specs +// and determines what lifecycle action (if any) should be taken. // -// This function handles the following state transitions: -// - Running -> Stopping: When user sets lifecycle.phase = "stopped" -// - Stopping -> Stopped: When StatefulSet replicas reach 0 -// - Stopped -> Updating: When user clears lifecycle.phase (wake-up) -// - Updating -> Running: Normal sync continues, defer sets final status -func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv1.Postgresql) bool { - // When Update() is called, it sets status=Updating before Sync() runs. - // So we need to check if oldSpec.Status was Stopped and newSpec is Updating - // with lifecycle cleared to properly detect wake-up. - isWakingUp := oldSpec.Status.Stopped() && - newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusUpdating && - (newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped") - - // If lifecycle was cleared and we have previousNumberOfInstances and numberOfInstances is 0 - isWakingUpSimple := newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped" - hasPreviousInstances := newSpec.Status.PreviousNumberOfInstances > 0 - needsRestore := newSpec.Spec.NumberOfInstances == 0 - - // double verification of waking up +// Detection logic: +// - Hibernate: Running status + lifecycle.phase="stopped" + not already stopping/stopped +// - Wake-up: Stopped status + lifecycle.phase cleared + (previousNumberOfInstances > 0 OR isWakingUp flag) +// - isWakingUp flag is set when old spec was Running, new status is Updating, and lifecycle is cleared +func detectLifecycleTransition( + currentStatus *acidv1.PostgresStatus, + oldSpecLifecycle *acidv1.LifecycleSpec, + newSpecLifecycle *acidv1.LifecycleSpec, + newSpecNumberOfInstances int32, + newSpecPreviousNumberOfInstances int32, + oldSpecStatusRunning bool, +) LifecycleAction { + isWakingUpSimple := newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped" + hasPreviousInstances := newSpecPreviousNumberOfInstances > 0 + needsRestore := newSpecNumberOfInstances == 0 + + // Detect wake-up by checking if Update() set status to Updating before Sync() runs + isWakingUp := oldSpecStatusRunning && + currentStatus.PostgresClusterStatus == acidv1.ClusterStatusUpdating && + (newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped") + + // Also detect wake-up by simple conditions: lifecycle cleared + has previous + needs restore isWakingUp = isWakingUp || (isWakingUpSimple && hasPreviousInstances && needsRestore) - // === INITIATE HIBERNATE: Running -> Stopping === - // Only initiate if not already stopping or stopped, and lifecycle.phase = "stopped" - if newSpec.Spec.Lifecycle != nil && - newSpec.Spec.Lifecycle.Phase == "stopped" && - !newSpec.Status.Stopping() && - !newSpec.Status.Stopped() { - - newSpec.Status.PreviousNumberOfInstances = newSpec.Spec.NumberOfInstances - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopping - newSpec.Spec.NumberOfInstances = 0 - c.logger.Infof("[lifecycle] cluster is going to hibernate, stored previous number of instances: %d", + if currentStatus.Stopped() || isWakingUp { + if isWakingUp || newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped" { + return LifecycleActionWakeUp + } + } + + if newSpecLifecycle != nil && + newSpecLifecycle.Phase == "stopped" && + !currentStatus.Stopping() && + !currentStatus.Stopped() { + return LifecycleActionHibernate + } + + return LifecycleActionNone +} + +// detectStoppingCompleted checks if the cluster should transition from Stopping to Stopped. +// This happens when the StatefulSet replicas have actually reached 0 (all pods terminated). +func detectStoppingCompleted(currentStatus *acidv1.PostgresStatus, statefulsetReplicas *int32) bool { + if !currentStatus.Stopping() { + return false + } + if statefulsetReplicas == nil { + return false + } + return *statefulsetReplicas == 0 +} + +// initiateHibernate prepares the cluster for hibernation by: +// - Storing current numberOfInstances in PreviousNumberOfInstances +// - Setting numberOfInstances to 0 +// - Setting status to Stopping +// - Scaling down connection pooler deployments +// - Suspending logical backup CronJob +// Errors during pooler/backup operations are logged but do not fail the transition. +func (c *Cluster) initiateHibernate(newSpec *acidv1.Postgresql) { + newSpec.Status.PreviousNumberOfInstances = newSpec.Spec.NumberOfInstances + newSpec.Spec.NumberOfInstances = 0 + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopping + + c.logger.Infof("[lifecycle] initiating hibernate: stored previousNumberOfInstances=%d", + newSpec.Status.PreviousNumberOfInstances) + + if err := c.scalePoolerDown(newSpec); err != nil { + c.logger.Warningf("[lifecycle] failed to scale pooler during hibernate: %v", err) + } + + if err := c.suspendLogicalBackupJob(); err != nil { + c.logger.Warningf("[lifecycle] failed to suspend logical backup job: %v", err) + } +} + +// initiateWakeUp prepares the cluster for wake-up by: +// - Restoring numberOfInstances from PreviousNumberOfInstances (if > 0) +// +// - Setting status to Updating +// - Scaling up connection pooler deployments +// - Resuming logical backup CronJob +// If PreviousNumberOfInstances is 0, logs a warning but still sets status to Updating. +// Errors during pooler/backup operations are logged but do not fail the transition. +func (c *Cluster) initiateWakeUp(newSpec *acidv1.Postgresql) { + if newSpec.Status.PreviousNumberOfInstances > 0 { + newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances + c.logger.Infof("[lifecycle] initiating wake-up: restoring numberOfInstances=%d", newSpec.Status.PreviousNumberOfInstances) + } else { + c.logger.Warningf("[lifecycle] cluster is waking up but previousNumberOfInstances is 0, cannot restore") + } + + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating + + if err := c.scalePoolerUp(newSpec); err != nil { + c.logger.Warningf("[lifecycle] failed to scale pooler during wake-up: %v", err) + } + + if err := c.unsuspendLogicalBackupJob(); err != nil { + c.logger.Warningf("[lifecycle] failed to resume logical backup job: %v", err) + } +} + +// persistHibernateTransition persists the hibernate transition to Kubernetes by: +// 1. Updating the PostgresCR spec (numberOfInstances=0, previousNumberOfInstances stored) +// 2. Updating the PostgresCR status (status=Stopping, previousPoolerInstances stored) +// 3. Updating the local cluster spec +// Returns (handled=true, nil) on success, (handled=false, error) on failure. +func (c *Cluster) persistHibernateTransition(newSpec *acidv1.Postgresql) (bool, error) { + pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) + if err != nil { + return false, fmt.Errorf("could not update spec during hibernate: %w", err) + } + + pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances + pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus + pgUpdated.Status.PreviousPoolerInstances = newSpec.Status.PreviousPoolerInstances + + pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) + if err != nil { + return false, fmt.Errorf("could not update status during hibernate: %w", err) + } + + c.setSpec(pgUpdated) + c.logger.Infof("[lifecycle] hibernate completed: cluster is stopping, numberOfInstances=0, previousNumberOfInstances=%d", + pgUpdated.Status.PreviousNumberOfInstances) + return true, nil +} + +// persistWakeUpTransition persists the wake-up transition to Kubernetes by: +// 1. Clearing PreviousNumberOfInstances and PreviousPoolerInstances +// 2. Updating the PostgresCR spec (numberOfInstances restored from previous) +// 3. Updating the PostgresCR status (status=Updating) +// 4. Updating the local cluster spec +// Returns (handled=true, nil) on success, (handled=false, error) on failure. +func (c *Cluster) persistWakeUpTransition(newSpec *acidv1.Postgresql) (bool, error) { + newSpec.Status.PreviousNumberOfInstances = 0 + newSpec.Status.PreviousPoolerInstances = nil + + pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) + if err != nil { + return false, fmt.Errorf("could not update spec during wake-up: %w", err) + } + + pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus + + pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) + if err != nil { + return false, fmt.Errorf("could not update status during wake-up: %w", err) + } + + c.setSpec(pgUpdated) + c.logger.Infof("[lifecycle] wake-up completed: cluster is updating, numberOfInstances=%d", + pgUpdated.Spec.NumberOfInstances) + return true, nil +} + +// persistStoppingCompletedTransition persists the Stopping->Stopped transition to Kubernetes +// by updating only the status (status=Stopped). This is called when StatefulSet replicas +// have actually reached 0. +// Returns (handled=true, nil) on success, (handled=false, error) on failure. +func (c *Cluster) persistStoppingCompletedTransition(newSpec *acidv1.Postgresql) (bool, error) { + pgUpdated, err := c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec) + if err != nil { + return false, fmt.Errorf("could not update status during stopping completed: %w", err) + } + + c.setSpec(pgUpdated) + c.logger.Info("[lifecycle] stopping completed: cluster is stopped") + return true, nil +} + +// manageHibernateState handles cluster hibernate/wake-up state transitions during Sync(). +// This is the Sync path - it only modifies the in-memory spec and returns a boolean +// indicating whether sync should continue or return early. +// +// State transitions handled: +// - Running -> Stopping: Via initiateHibernate() (when lifecycle.phase="stopped") +// - Stopping -> Stopped: When StatefulSet replicas reach 0 (detected here) +// - Stopped -> Updating: Via initiateWakeUp() (when lifecycle cleared) +// +// Returns true if sync should continue, false if it should return early. +func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv1.Postgresql) bool { + action := detectLifecycleTransition( + &newSpec.Status, + oldSpec.Spec.Lifecycle, + newSpec.Spec.Lifecycle, + newSpec.Spec.NumberOfInstances, + newSpec.Status.PreviousNumberOfInstances, + oldSpec.Status.Running(), + ) + + switch action { + case LifecycleActionHibernate: + c.initiateHibernate(newSpec) + return true + + case LifecycleActionWakeUp: + c.initiateWakeUp(newSpec) return true } - // === STOPPING -> STOPPED: Check actual StatefulSet replicas === - // Only transition to Stopped when StatefulSet replicas have actually reached 0 - if newSpec.Status.Stopping() { - if c.Statefulset != nil && *c.Statefulset.Spec.Replicas == 0 { - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped - c.logger.Info("[lifecycle] cluster has stopped, all pods are terminated") - } + // Check if Stopping -> Stopped transition is needed + if detectStoppingCompleted(&newSpec.Status, c.getStatefulsetReplicas()) { + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped + c.logger.Info("[lifecycle] cluster has stopped, all pods are terminated") return true } - // === WAKE-UP: Stopped/Updating -> Running === - // Restore numberOfInstances from previousNumberOfInstances when waking up - if newSpec.Status.Stopped() || isWakingUp { - // Check if lifecycle.phase was cleared (user wants to wake up) - if isWakingUp || newSpec.Spec.Lifecycle == nil || newSpec.Spec.Lifecycle.Phase != "stopped" { - if newSpec.Status.PreviousNumberOfInstances > 0 { - newSpec.Spec.NumberOfInstances = newSpec.Status.PreviousNumberOfInstances - c.logger.Infof("[lifecycle] cluster is waking up, restoring number of instances: %d", - newSpec.Status.PreviousNumberOfInstances) - } else { - c.logger.Warningf("[lifecycle] cluster is waking up but previousNumberOfInstances is 0, cannot restore") - } - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating - return true - } - // Still stopped and lifecycle.phase = "stopped", skip further sync + // Skip sync if cluster is stopped and lifecycle.phase="stopped" is set + if newSpec.Status.Stopped() && newSpec.Spec.Lifecycle != nil && newSpec.Spec.Lifecycle.Phase == "stopped" { return false } return true } -// getPoolerReplicas returns the current replica count for a pooler role. -// Returns 0 if pooler doesn't exist or hasn't been synced yet. +// getStatefulsetReplicas returns the current replica count from the StatefulSet. +// Returns nil if StatefulSet is nil or Replicas field is nil. +func (c *Cluster) getStatefulsetReplicas() *int32 { + if c.Statefulset == nil || c.Statefulset.Spec.Replicas == nil { + return nil + } + return c.Statefulset.Spec.Replicas +} + +// suspendLogicalBackupJob suspends the logical backup CronJob by setting spec.suspend=true. +// If the job was previously loaded but has been deleted externally, clears the cached reference. +// Returns nil if job is not loaded (no-op) or if job was not found (clears cache). +// Returns error only for actual failures (network errors, etc). +func (c *Cluster) suspendLogicalBackupJob() error { + if c.LogicalBackupJob == nil { + c.logger.Debug("logical backup job is not loaded, skipping suspend") + return nil + } + + // Check if job still exists (handles externally deleted jobs) + _, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Get( + context.TODO(), c.getLogicalBackupJobName(), metav1.GetOptions{}) + if k8serrors.IsNotFound(err) { + c.logger.Info("logical backup job not found during suspend, clearing cached reference") + c.LogicalBackupJob = nil + return nil + } + if err != nil { + return fmt.Errorf("could not get logical backup job: %w", err) + } + + patchData := fmt.Sprintf(`{"spec":{"suspend":true}}`) + cronJob, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Patch( + context.TODO(), + c.getLogicalBackupJobName(), + types.MergePatchType, + []byte(patchData), + metav1.PatchOptions{}, + "", + ) + if err != nil { + return fmt.Errorf("could not suspend logical backup job: %w", err) + } + c.LogicalBackupJob = cronJob + c.logger.Info("logical backup job suspended") + + return nil +} + +// unsuspendLogicalBackupJob resumes the logical backup CronJob by setting spec.suspend=false. +// If the job was previously loaded but has been deleted externally, clears the cached reference. +// Returns nil if job is not loaded (no-op) or if job was not found (clears cache). +// Returns error only for actual failures (network errors, etc). +func (c *Cluster) unsuspendLogicalBackupJob() error { + if c.LogicalBackupJob == nil { + c.logger.Debug("logical backup job is not loaded, skipping unsuspend") + return nil + } + + // Check if job still exists (handles externally deleted jobs) + _, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Get( + context.TODO(), c.getLogicalBackupJobName(), metav1.GetOptions{}) + if k8serrors.IsNotFound(err) { + c.logger.Info("logical backup job not found during unsuspend, clearing cached reference") + c.LogicalBackupJob = nil + return nil + } + if err != nil { + return fmt.Errorf("could not get logical backup job: %w", err) + } + + patchData := fmt.Sprintf(`{"spec":{"suspend":false}}`) + cronJob, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Patch( + context.TODO(), + c.getLogicalBackupJobName(), + types.MergePatchType, + []byte(patchData), + metav1.PatchOptions{}, + "", + ) + if err != nil { + return fmt.Errorf("could not resume logical backup job: %w", err) + } + c.LogicalBackupJob = cronJob + c.logger.Info("logical backup job resumed") + + return nil +} + +// getPoolerReplicas returns the current replica count for a pooler deployment. +// Returns 0 if pooler doesn't exist, hasn't been synced yet, or has nil Replicas. func (c *Cluster) getPoolerReplicas(role PostgresRole) int32 { if c.ConnectionPooler == nil || c.ConnectionPooler[role] == nil || c.ConnectionPooler[role].Deployment == nil || @@ -93,8 +344,10 @@ func (c *Cluster) getPoolerReplicas(role PostgresRole) int32 { return *c.ConnectionPooler[role].Deployment.Spec.Replicas } -// scalePoolerDown scales pooler deployments to 0 and stores current replica count. +// scalePoolerDown scales all connection pooler deployments to 0 and stores their current +// replica counts in newSpec.Status.PreviousPoolerInstances. // Should be called during hibernate initiation. +// Errors are returned immediately if a patch fails (partial state possible). func (c *Cluster) scalePoolerDown(newSpec *acidv1.Postgresql) error { if c.ConnectionPooler == nil { return nil @@ -103,13 +356,11 @@ func (c *Cluster) scalePoolerDown(newSpec *acidv1.Postgresql) error { for role := range c.ConnectionPooler { replicas := c.getPoolerReplicas(role) - // Store current replicas in status if newSpec.Status.PreviousPoolerInstances == nil { newSpec.Status.PreviousPoolerInstances = make(map[string]int32) } newSpec.Status.PreviousPoolerInstances[string(role)] = replicas - // Scale to 0 if currently non-zero if replicas > 0 { if err := c.patchPoolerReplicas(role, 0); err != nil { return err @@ -120,8 +371,10 @@ func (c *Cluster) scalePoolerDown(newSpec *acidv1.Postgresql) error { return nil } -// scalePoolerUp restores pooler deployments to previous replica counts. +// scalePoolerUp restores connection pooler deployments to their previous replica counts +// from newSpec.Status.PreviousPoolerInstances. // Should be called during wake-up. +// Errors are returned immediately if a patch fails (partial state possible). func (c *Cluster) scalePoolerUp(newSpec *acidv1.Postgresql) error { if newSpec.Status.PreviousPoolerInstances == nil { return nil @@ -130,7 +383,6 @@ func (c *Cluster) scalePoolerUp(newSpec *acidv1.Postgresql) error { for roleStr, replicas := range newSpec.Status.PreviousPoolerInstances { role := PostgresRole(roleStr) - // Scale to stored value (could be 0) if err := c.patchPoolerReplicas(role, replicas); err != nil { return err } @@ -139,19 +391,19 @@ func (c *Cluster) scalePoolerUp(newSpec *acidv1.Postgresql) error { return nil } -// patchPoolerReplicas patches the pooler deployment replica count. +// patchPoolerReplicas patches a pooler deployment's replica count. +// If the deployment doesn't exist (not found), returns nil (no-op). +// Returns error for other failures (network errors, etc). func (c *Cluster) patchPoolerReplicas(role PostgresRole, replicas int32) error { - // Check if deployment exists first _, err := c.KubeClient.Deployments(c.Namespace).Get( context.TODO(), c.connectionPoolerName(role), metav1.GetOptions{}) if err != nil { if k8serrors.IsNotFound(err) { - return nil // Pooler doesn't exist, no-op + return nil } return fmt.Errorf("could not get pooler deployment for %s: %w", role, err) } - // Patch with merge patch patchData := fmt.Sprintf(`{"spec":{"replicas":%d}}`, replicas) _, err = c.KubeClient.Deployments(c.Namespace).Patch( context.TODO(), c.connectionPoolerName(role), types.MergePatchType, []byte(patchData), metav1.PatchOptions{}) @@ -159,5 +411,4 @@ func (c *Cluster) patchPoolerReplicas(role PostgresRole, replicas int32) error { return fmt.Errorf("could not patch pooler deployment %s replicas: %w", role, err) } return nil -} - +} \ No newline at end of file diff --git a/pkg/cluster/lifecycle_test.go b/pkg/cluster/lifecycle_test.go index b43f43cd0..3dcd53380 100644 --- a/pkg/cluster/lifecycle_test.go +++ b/pkg/cluster/lifecycle_test.go @@ -557,12 +557,14 @@ func TestHandleHibernateAndWakeUp_Hibernate(t *testing.T) { wantErr: false, }, { - name: "Stopped + lifecycle cleared - no-op (Update handles this)", + name: "Stopped + lifecycle cleared - initiates wake-up", currentStatus: "Stopped", numberOfInstances: 0, lifecyclePhase: "", - wantHandled: false, // handleHibernateAndWakeUp doesn't handle wake-up, that's done via Update flow - wantErr: false, + k8sUpdateSucceeds: true, + k8sStatusSucceeds: true, + wantHandled: true, + wantErr: false, }, } @@ -661,12 +663,14 @@ func TestHandleHibernateAndWakeUp_WakeUp(t *testing.T) { wantErr: false, }, { - name: "Stopped + previousNumberOfInstances is 0", + name: "Stopped + previousNumberOfInstances is 0 - still initiates wake-up", currentStatus: "Stopped", numberOfInstances: 0, previousNumberOfInst: 0, lifecyclePhase: "", - wantHandled: false, // No restore possible + k8sUpdateSucceeds: true, + k8sStatusSucceeds: true, + wantHandled: true, wantErr: false, }, { diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 07dfae0c1..b75f6626d 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -825,57 +825,7 @@ func (c *Cluster) deleteLogicalBackupJob() error { return nil } -func (c *Cluster) suspendLogicalBackupJob() error { - if c.LogicalBackupJob == nil { - c.logger.Debug("logical backup job is not loaded, skipping suspend") - return nil - } - - c.setProcessName("suspending logical backup job") - - patchData := fmt.Sprintf(`{"spec":{"suspend":true}}`) - cronJob, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Patch( - context.TODO(), - c.getLogicalBackupJobName(), - types.MergePatchType, - []byte(patchData), - metav1.PatchOptions{}, - "", - ) - if err != nil { - return fmt.Errorf("could not suspend logical backup job: %w", err) - } - c.LogicalBackupJob = cronJob - c.logger.Info("logical backup job suspended") - - return nil -} -func (c *Cluster) unsuspendLogicalBackupJob() error { - if c.LogicalBackupJob == nil { - c.logger.Debug("logical backup job is not loaded, skipping unsuspend") - return nil - } - - c.setProcessName("resuming logical backup job") - - patchData := fmt.Sprintf(`{"spec":{"suspend":false}}`) - cronJob, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Patch( - context.TODO(), - c.getLogicalBackupJobName(), - types.MergePatchType, - []byte(patchData), - metav1.PatchOptions{}, - "", - ) - if err != nil { - return fmt.Errorf("could not resume logical backup job: %w", err) - } - c.LogicalBackupJob = cronJob - c.logger.Info("logical backup job resumed") - - return nil -} // GetServiceMaster returns cluster's kubernetes master Service func (c *Cluster) GetServiceMaster() *v1.Service { From 18a42b8f493cea821ef800cc4021da5c17013e8d Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Mon, 22 Jun 2026 15:22:57 +0530 Subject: [PATCH 08/13] Retriggering pipeline From 8e7018b55fc3679f5f0a1d09442474da94719ce0 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Tue, 14 Jul 2026 00:40:01 +0530 Subject: [PATCH 09/13] SCF-830: Refactor detect lifecycle, fix Update 409 follow-up, and persist lifecycle spec changes in Sync --- pkg/cluster/cluster.go | 33 ++++++++++++++++------- pkg/cluster/lifecycle.go | 50 +++++++++++++++-------------------- pkg/cluster/lifecycle_test.go | 18 ++++++------- pkg/cluster/sync.go | 18 ++++++++++++- 4 files changed, 72 insertions(+), 47 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 1d22a6609..618385eb7 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1017,13 +1017,6 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { return nil } - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating - - newSpec, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec) - if err != nil { - return fmt.Errorf("could not set cluster status to updating: %w", err) - } - // Handle lifecycle transitions (hibernate/wake-up) handled, err := c.handleHibernateAndWakeUp(newSpec) if err != nil { @@ -1033,6 +1026,24 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { return nil } + // If a previous Update already set a lifecycle status (e.g. persistWakeUpTransition + // wrote Updating), do not clobber it. The watch event that triggered this Update + // is a follow-up from the operator's own persistXxxTransition write, so the + // status subresource is already at the correct value, and writing again with + // this stale newSpec.resourceVersion would cause a 409 Conflict. + isLifecycleActive := newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusUpdating || + newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusStopping || + newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusStopped + + if !isLifecycleActive { + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating + + newSpec, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec) + if err != nil { + return fmt.Errorf("could not set cluster status to updating: %w", err) + } + } + if !c.isInMaintenanceWindow(newSpec.Spec.MaintenanceWindows) { // do not apply any major version related changes yet newSpec.Spec.PostgresqlParam.PgVersion = oldSpec.Spec.PostgresqlParam.PgVersion @@ -1040,6 +1051,12 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { c.setSpec(newSpec) defer func() { + if isLifecycleActive { + // Status was set by a previous Update (persistXxxTransition); do not + // overwrite Updating/Stopping/Stopped back to Running. Sync's defer + // will transition to Running once the cluster is ready. + return + } currentStatus := newSpec.Status.DeepCopy() newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusRunning @@ -1283,11 +1300,9 @@ func (c *Cluster) blockLifecycleUpdate(newSpec *acidv1.Postgresql) (bool, error) func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, error) { action := detectLifecycleTransition( &c.Status, - c.Spec.Lifecycle, newSpec.Spec.Lifecycle, newSpec.Spec.NumberOfInstances, newSpec.Status.PreviousNumberOfInstances, - c.Status.Running(), ) if action == LifecycleActionNone { diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go index eb6a4ee03..456be60eb 100644 --- a/pkg/cluster/lifecycle.go +++ b/pkg/cluster/lifecycle.go @@ -17,38 +17,30 @@ import ( type LifecycleAction int const ( - LifecycleActionNone LifecycleAction = iota - LifecycleActionHibernate // Running -> Stopping (initiate hibernate) - LifecycleActionStoppingCompleted // Stopping -> Stopped (pods fully terminated) - LifecycleActionWakeUp // Stopped -> Updating (initiate wake-up) + LifecycleActionNone LifecycleAction = iota + LifecycleActionHibernate // Running -> Stopping (initiate hibernate) + LifecycleActionStoppingCompleted // Stopping -> Stopped (pods fully terminated) + LifecycleActionWakeUp // Stopped -> Updating (initiate wake-up) ) // detectLifecycleTransition is a pure function that examines the current and proposed specs // and determines what lifecycle action (if any) should be taken. // // Detection logic: -// - Hibernate: Running status + lifecycle.phase="stopped" + not already stopping/stopped -// - Wake-up: Stopped status + lifecycle.phase cleared + (previousNumberOfInstances > 0 OR isWakingUp flag) -// - isWakingUp flag is set when old spec was Running, new status is Updating, and lifecycle is cleared +// - Hibernate: lifecycle.phase="stopped" + status not Stopping or Stopped +// - Wake-up: (status == Stopped OR lifecycle cleared + has previousNumberOfInstances +// - numberOfInstances == 0) AND new lifecycle is cleared (or nil) func detectLifecycleTransition( currentStatus *acidv1.PostgresStatus, - oldSpecLifecycle *acidv1.LifecycleSpec, newSpecLifecycle *acidv1.LifecycleSpec, newSpecNumberOfInstances int32, newSpecPreviousNumberOfInstances int32, - oldSpecStatusRunning bool, ) LifecycleAction { isWakingUpSimple := newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped" hasPreviousInstances := newSpecPreviousNumberOfInstances > 0 needsRestore := newSpecNumberOfInstances == 0 - // Detect wake-up by checking if Update() set status to Updating before Sync() runs - isWakingUp := oldSpecStatusRunning && - currentStatus.PostgresClusterStatus == acidv1.ClusterStatusUpdating && - (newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped") - - // Also detect wake-up by simple conditions: lifecycle cleared + has previous + needs restore - isWakingUp = isWakingUp || (isWakingUpSimple && hasPreviousInstances && needsRestore) + isWakingUp := isWakingUpSimple && hasPreviousInstances && needsRestore if currentStatus.Stopped() || isWakingUp { if isWakingUp || newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped" { @@ -171,6 +163,8 @@ func (c *Cluster) persistWakeUpTransition(newSpec *acidv1.Postgresql) (bool, err return false, fmt.Errorf("could not update spec during wake-up: %w", err) } + pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances + pgUpdated.Status.PreviousPoolerInstances = newSpec.Status.PreviousPoolerInstances pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) @@ -200,48 +194,47 @@ func (c *Cluster) persistStoppingCompletedTransition(newSpec *acidv1.Postgresql) } // manageHibernateState handles cluster hibernate/wake-up state transitions during Sync(). -// This is the Sync path - it only modifies the in-memory spec and returns a boolean -// indicating whether sync should continue or return early. +// This is the Sync path - it only modifies the in-memory spec. // // State transitions handled: // - Running -> Stopping: Via initiateHibernate() (when lifecycle.phase="stopped") // - Stopping -> Stopped: When StatefulSet replicas reach 0 (detected here) // - Stopped -> Updating: Via initiateWakeUp() (when lifecycle cleared) // -// Returns true if sync should continue, false if it should return early. -func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv1.Postgresql) bool { +// Returns the detected LifecycleAction and a boolean indicating whether sync +// should continue (true) or return early (false, only when cluster is Stopped +// with lifecycle.phase="stopped" set). +func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv1.Postgresql) (LifecycleAction, bool) { action := detectLifecycleTransition( &newSpec.Status, - oldSpec.Spec.Lifecycle, newSpec.Spec.Lifecycle, newSpec.Spec.NumberOfInstances, newSpec.Status.PreviousNumberOfInstances, - oldSpec.Status.Running(), ) switch action { case LifecycleActionHibernate: c.initiateHibernate(newSpec) - return true + return action, true case LifecycleActionWakeUp: c.initiateWakeUp(newSpec) - return true + return action, true } // Check if Stopping -> Stopped transition is needed if detectStoppingCompleted(&newSpec.Status, c.getStatefulsetReplicas()) { newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped c.logger.Info("[lifecycle] cluster has stopped, all pods are terminated") - return true + return LifecycleActionStoppingCompleted, true } // Skip sync if cluster is stopped and lifecycle.phase="stopped" is set if newSpec.Status.Stopped() && newSpec.Spec.Lifecycle != nil && newSpec.Spec.Lifecycle.Phase == "stopped" { - return false + return LifecycleActionNone, false } - return true + return LifecycleActionNone, true } // getStatefulsetReplicas returns the current replica count from the StatefulSet. @@ -411,4 +404,5 @@ func (c *Cluster) patchPoolerReplicas(role PostgresRole, replicas int32) error { return fmt.Errorf("could not patch pooler deployment %s replicas: %w", role, err) } return nil -} \ No newline at end of file +} + diff --git a/pkg/cluster/lifecycle_test.go b/pkg/cluster/lifecycle_test.go index 3dcd53380..0cdad48a0 100644 --- a/pkg/cluster/lifecycle_test.go +++ b/pkg/cluster/lifecycle_test.go @@ -490,7 +490,7 @@ func TestManageHibernateState(t *testing.T) { } } - gotContinue := c.manageHibernateState(oldSpec, newSpec) + _, gotContinue := c.manageHibernateState(oldSpec, newSpec) assert.Equal(t, tt.wantContinue, gotContinue) @@ -877,7 +877,7 @@ func TestLifecycleStateTransitions(t *testing.T) { } // Step 1: manageHibernateState should initiate hibernate - continueSync := c.manageHibernateState(oldSpec, newSpec) + _, continueSync := c.manageHibernateState(oldSpec, newSpec) assert.True(t, continueSync) assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances) assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) @@ -912,7 +912,7 @@ func TestLifecycleStateTransitions(t *testing.T) { } // Step 1: manageHibernateState should restore - continueSync := c.manageHibernateState(oldSpec, newSpec) + _, continueSync := c.manageHibernateState(oldSpec, newSpec) assert.True(t, continueSync) assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) assert.Equal(t, "Updating", newSpec.Status.PostgresClusterStatus) @@ -1147,7 +1147,7 @@ func TestManageHibernateState_EdgeCases(t *testing.T) { } } - gotContinue := c.manageHibernateState(oldSpec, newSpec) + _, gotContinue := c.manageHibernateState(oldSpec, newSpec) assert.Equal(t, tt.wantContinue, gotContinue, "continue sync mismatch") if tt.wantNumberOfInstances != nil { @@ -1178,7 +1178,7 @@ func TestManageHibernateState_StateTransitionSequence(t *testing.T) { Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, } - continueSync := c.manageHibernateState(oldSpec, newSpec) + _, continueSync := c.manageHibernateState(oldSpec, newSpec) assert.True(t, continueSync) assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances) assert.Equal(t, int32(3), newSpec.Status.PreviousNumberOfInstances) @@ -1192,12 +1192,12 @@ func TestManageHibernateState_StateTransitionSequence(t *testing.T) { } newSpec.Status.PostgresClusterStatus = "Stopping" - continueSync = c.manageHibernateState(oldSpec, newSpec) + _, continueSync = c.manageHibernateState(oldSpec, newSpec) assert.True(t, continueSync) assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) c.Statefulset.Spec.Replicas = int32Ptr(0) - continueSync = c.manageHibernateState(oldSpec, newSpec) + _, continueSync = c.manageHibernateState(oldSpec, newSpec) assert.True(t, continueSync) assert.Equal(t, "Stopped", newSpec.Status.PostgresClusterStatus) }) @@ -1223,7 +1223,7 @@ func TestManageHibernateState_StateTransitionSequence(t *testing.T) { }, } - continueSync := c.manageHibernateState(oldSpec, newSpec) + _, continueSync := c.manageHibernateState(oldSpec, newSpec) assert.True(t, continueSync) assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) assert.Equal(t, "Updating", newSpec.Status.PostgresClusterStatus) @@ -1233,7 +1233,7 @@ func TestManageHibernateState_StateTransitionSequence(t *testing.T) { } newSpec.Status.PostgresClusterStatus = "Updating" - continueSync = c.manageHibernateState(oldSpec, newSpec) + _, continueSync = c.manageHibernateState(oldSpec, newSpec) assert.True(t, continueSync) assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) }) diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 13982900c..8a0b68eaa 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -66,10 +66,26 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { } // Handle lifecycle hibernate/wake-up state transitions - if !c.manageHibernateState(oldSpec, newSpec) { + lifecycleAction, continueSync := c.manageHibernateState(oldSpec, newSpec) + if !continueSync { return nil } + // Hibernate and WakeUp transitions modify newSpec (e.g. numberOfInstances). + // Persist them here so the cluster actually scales. Without this, a Sync + // that runs before any Update (e.g. operator restart catching up via the + // informer cache) would leave the spec unchanged in the API and the cluster + // would stay stuck in Stopping/Updating because no scaling occurs. + if lifecycleAction == LifecycleActionHibernate || lifecycleAction == LifecycleActionWakeUp { + pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) + if err != nil { + c.logger.Errorf("could not update spec after lifecycle transition in sync: %v", err) + return err + } + c.setSpec(pgUpdated) + newSpec = pgUpdated + } + if err = c.initUsers(); err != nil { err = fmt.Errorf("could not init users: %v", err) return err From 356aa782b1f4f0bebdf57a04a49aa6fd2f768d06 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Tue, 14 Jul 2026 12:59:54 +0530 Subject: [PATCH 10/13] SCF-830: Added fresh replica status checking while stopping --- pkg/cluster/cluster.go | 6 +- pkg/cluster/lifecycle.go | 45 ++++++- pkg/cluster/lifecycle_test.go | 230 +++++++++++++++++++++++++++++++--- 3 files changed, 265 insertions(+), 16 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 618385eb7..468163a0b 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1306,7 +1306,11 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er ) if action == LifecycleActionNone { - if detectStoppingCompleted(&c.Status, c.getStatefulsetReplicas()) { + done, err := c.checkStoppingCompleted(&c.Status) + if err != nil { + return false, fmt.Errorf("could not check stopping completed: %w", err) + } + if done { newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped return c.persistStoppingCompletedTransition(newSpec) } diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go index 456be60eb..d352b30e0 100644 --- a/pkg/cluster/lifecycle.go +++ b/pkg/cluster/lifecycle.go @@ -9,6 +9,7 @@ import ( "k8s.io/apimachinery/pkg/types" acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" + "github.com/zalando/postgres-operator/pkg/util/k8sutil" ) // LifecycleAction represents the detected lifecycle transition for a cluster. @@ -70,6 +71,42 @@ func detectStoppingCompleted(currentStatus *acidv1.PostgresStatus, statefulsetRe return *statefulsetReplicas == 0 } +// refreshStatefulset fetches the latest StatefulSet from the API and updates c.Statefulset. +// The in-memory c.Statefulset cache is only populated by Create() and by Sync() inside +// syncStatefulSet(); after an operator restart the cache is nil, and Sync() calls +// manageHibernateState before reaching syncStatefulSet(). Without this refresh, +// detectStoppingCompleted would see stale (or nil) replica counts and never transition +// to Stopped. If the StatefulSet is not found in the API, the cache is cleared and +// nil is returned (the cluster may have been deleted). +func (c *Cluster) refreshStatefulset() error { + sset, err := c.KubeClient.StatefulSets(c.Namespace).Get( + context.TODO(), c.statefulSetName(), metav1.GetOptions{}) + if err != nil { + if k8sutil.ResourceNotFound(err) { + c.Statefulset = nil + return nil + } + return fmt.Errorf("could not refresh statefulset: %w", err) + } + c.Statefulset = sset + return nil +} + +// checkStoppingCompleted is an I/O wrapper around detectStoppingCompleted. It refreshes +// the StatefulSet cache before delegating so callers see fresh replica counts, especially +// after an operator restart where c.Statefulset is nil. To avoid an extra API call on the +// hot Update/Sync path, the refresh is skipped when status is not Stopping (the only +// state where detectStoppingCompleted can return true). +func (c *Cluster) checkStoppingCompleted(currentStatus *acidv1.PostgresStatus) (bool, error) { + if !currentStatus.Stopping() { + return false, nil + } + if err := c.refreshStatefulset(); err != nil { + return false, err + } + return detectStoppingCompleted(currentStatus, c.getStatefulsetReplicas()), nil +} + // initiateHibernate prepares the cluster for hibernation by: // - Storing current numberOfInstances in PreviousNumberOfInstances // - Setting numberOfInstances to 0 @@ -223,7 +260,13 @@ func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv } // Check if Stopping -> Stopped transition is needed - if detectStoppingCompleted(&newSpec.Status, c.getStatefulsetReplicas()) { + done, err := c.checkStoppingCompleted(&newSpec.Status) + if err != nil { + // Refresh failed; treat as not-completed and let the next Sync retry. + // Not returning the error so we don't mark the cluster SyncFailed on a + // transient API hiccup while the cluster is mid-stop. + c.logger.Warningf("[lifecycle] could not refresh statefulset for stopping check: %v", err) + } else if done { newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped c.logger.Info("[lifecycle] cluster has stopped, all pods are terminated") return LifecycleActionStoppingCompleted, true diff --git a/pkg/cluster/lifecycle_test.go b/pkg/cluster/lifecycle_test.go index 0cdad48a0..0f1cdfad0 100644 --- a/pkg/cluster/lifecycle_test.go +++ b/pkg/cluster/lifecycle_test.go @@ -450,6 +450,25 @@ func TestManageHibernateState(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() + + if tt.statefulsetReplicas != nil { + _, err := clientSet.AppsV1().StatefulSets("default").Create( + context.TODO(), + &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: appsv1.StatefulSetSpec{ + Replicas: tt.statefulsetReplicas, + }, + }, + metav1.CreateOptions{}, + ) + assert.NoError(t, err) + } + c := &Cluster{ logger: lifecycleLogger, Postgresql: acidv1.Postgresql{ @@ -458,14 +477,7 @@ func TestManageHibernateState(t *testing.T) { Namespace: "default", }, }, - } - - if tt.statefulsetReplicas != nil { - c.Statefulset = &appsv1.StatefulSet{ - Spec: appsv1.StatefulSetSpec{ - Replicas: tt.statefulsetReplicas, - }, - } + KubeClient: *kubeClient, } oldSpec := acidv1.Postgresql{ @@ -1107,8 +1119,28 @@ func TestManageHibernateState_EdgeCases(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() + + if tt.statefulsetReplicas != nil { + _, err := clientSet.AppsV1().StatefulSets("default").Create( + context.TODO(), + &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: appsv1.StatefulSetSpec{ + Replicas: tt.statefulsetReplicas, + }, + }, + metav1.CreateOptions{}, + ) + assert.NoError(t, err) + } + c := &Cluster{ - logger: lifecycleLogger, + logger: lifecycleLogger, + KubeClient: *kubeClient, Postgresql: acidv1.Postgresql{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -1160,8 +1192,20 @@ func TestManageHibernateState_EdgeCases(t *testing.T) { func TestManageHibernateState_StateTransitionSequence(t *testing.T) { t.Run("Running -> Stopping -> Stopped sequence", func(t *testing.T) { + kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() + _, err := clientSet.AppsV1().StatefulSets("default").Create( + context.TODO(), + &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: appsv1.StatefulSetSpec{Replicas: int32Ptr(3)}, + }, + metav1.CreateOptions{}, + ) + assert.NoError(t, err) + c := &Cluster{ - logger: lifecycleLogger, + logger: lifecycleLogger, + KubeClient: *kubeClient, Postgresql: acidv1.Postgresql{ ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, }, @@ -1184,9 +1228,13 @@ func TestManageHibernateState_StateTransitionSequence(t *testing.T) { assert.Equal(t, int32(3), newSpec.Status.PreviousNumberOfInstances) assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) - c.Statefulset = &appsv1.StatefulSet{ - Spec: appsv1.StatefulSetSpec{Replicas: int32Ptr(2)}, - } + // Simulate STS still terminating (replicas=2) + sset, err := clientSet.AppsV1().StatefulSets("default").Get(context.TODO(), "test", metav1.GetOptions{}) + assert.NoError(t, err) + sset.Spec.Replicas = int32Ptr(2) + _, err = clientSet.AppsV1().StatefulSets("default").Update(context.TODO(), sset, metav1.UpdateOptions{}) + assert.NoError(t, err) + oldSpec = acidv1.Postgresql{ Status: acidv1.PostgresStatus{PostgresClusterStatus: "Stopping"}, } @@ -1196,7 +1244,13 @@ func TestManageHibernateState_StateTransitionSequence(t *testing.T) { assert.True(t, continueSync) assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) - c.Statefulset.Spec.Replicas = int32Ptr(0) + // Simulate STS scaled to 0 + sset, err = clientSet.AppsV1().StatefulSets("default").Get(context.TODO(), "test", metav1.GetOptions{}) + assert.NoError(t, err) + sset.Spec.Replicas = int32Ptr(0) + _, err = clientSet.AppsV1().StatefulSets("default").Update(context.TODO(), sset, metav1.UpdateOptions{}) + assert.NoError(t, err) + _, continueSync = c.manageHibernateState(oldSpec, newSpec) assert.True(t, continueSync) assert.Equal(t, "Stopped", newSpec.Status.PostgresClusterStatus) @@ -1427,4 +1481,152 @@ func TestUnsuspendLogicalBackupJob(t *testing.T) { } }) } +} + +func TestRefreshStatefulset(t *testing.T) { + const stsName = "test-cluster" + + makeSTS := func(replicas *int32) *appsv1.StatefulSet { + return &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{Name: stsName, Namespace: "default"}, + Spec: appsv1.StatefulSetSpec{Replicas: replicas}, + } + } + + makeCluster := func(t *testing.T) (*Cluster, *fake.Clientset) { + t.Helper() + kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() + return &Cluster{ + logger: lifecycleLogger, + KubeClient: *kubeClient, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: stsName, Namespace: "default"}, + }, + }, clientSet + } + + t.Run("success: populates cache from API when c.Statefulset is nil", func(t *testing.T) { + c, clientSet := makeCluster(t) + _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(0)), metav1.CreateOptions{}) + assert.NoError(t, err) + assert.Nil(t, c.Statefulset) + + assert.NoError(t, c.refreshStatefulset()) + assert.NotNil(t, c.Statefulset) + assert.Equal(t, stsName, c.Statefulset.Name) + assert.Equal(t, int32(0), *c.Statefulset.Spec.Replicas) + }) + + t.Run("success: overwrites stale cache with fresh API value", func(t *testing.T) { + c, clientSet := makeCluster(t) + _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(0)), metav1.CreateOptions{}) + assert.NoError(t, err) + // Stale cache says replicas=3 + c.Statefulset = makeSTS(int32Ptr(3)) + + assert.NoError(t, c.refreshStatefulset()) + assert.Equal(t, int32(0), *c.Statefulset.Spec.Replicas, "should reflect API value, not cached value") + }) + + t.Run("not found: clears cache and returns nil", func(t *testing.T) { + c, _ := makeCluster(t) + c.Statefulset = makeSTS(int32Ptr(2)) // stale cache + assert.NotNil(t, c.Statefulset) + + assert.NoError(t, c.refreshStatefulset()) + assert.Nil(t, c.Statefulset, "cache should be cleared when STS is missing") + }) + + t.Run("not found from nil: leaves cache nil, returns nil", func(t *testing.T) { + c, _ := makeCluster(t) + assert.Nil(t, c.Statefulset) + + assert.NoError(t, c.refreshStatefulset()) + assert.Nil(t, c.Statefulset) + }) +} + +func TestCheckStoppingCompleted(t *testing.T) { + const stsName = "test-cluster" + + makeSTS := func(replicas *int32) *appsv1.StatefulSet { + return &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{Name: stsName, Namespace: "default"}, + Spec: appsv1.StatefulSetSpec{Replicas: replicas}, + } + } + + makeCluster := func(t *testing.T) (*Cluster, *fake.Clientset) { + t.Helper() + kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() + return &Cluster{ + logger: lifecycleLogger, + KubeClient: *kubeClient, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: stsName, Namespace: "default"}, + }, + }, clientSet + } + + stoppingStatus := acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusStopping} + runningStatus := acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusRunning} + + t.Run("fast path: status not Stopping returns false without touching cache", func(t *testing.T) { + c, _ := makeCluster(t) + // Pre-populate cache to prove it's not refreshed + c.Statefulset = makeSTS(int32Ptr(0)) + + done, err := c.checkStoppingCompleted(&runningStatus) + assert.NoError(t, err) + assert.False(t, done) + assert.NotNil(t, c.Statefulset, "fast path should not refresh the cache") + }) + + t.Run("Stopping with API replicas=0 returns true (operator-restart scenario)", func(t *testing.T) { + c, clientSet := makeCluster(t) + // Operator just restarted: c.Statefulset is nil. + // API has the truth: replicas=0. + _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(0)), metav1.CreateOptions{}) + assert.NoError(t, err) + assert.Nil(t, c.Statefulset) + + done, err := c.checkStoppingCompleted(&stoppingStatus) + assert.NoError(t, err) + assert.True(t, done, "should transition to Stopped based on fresh API value") + assert.NotNil(t, c.Statefulset, "cache should be populated after refresh") + }) + + t.Run("Stopping with API replicas=2 returns false", func(t *testing.T) { + c, clientSet := makeCluster(t) + _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(2)), metav1.CreateOptions{}) + assert.NoError(t, err) + + done, err := c.checkStoppingCompleted(&stoppingStatus) + assert.NoError(t, err) + assert.False(t, done) + }) + + t.Run("Stopping with stale cache (replicas=2) but API says 0 returns true", func(t *testing.T) { + c, clientSet := makeCluster(t) + _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(0)), metav1.CreateOptions{}) + assert.NoError(t, err) + // Stale cache says replicas=2 (e.g. another operator pod restarted and + // cached the pre-hibernate state). + c.Statefulset = makeSTS(int32Ptr(2)) + + done, err := c.checkStoppingCompleted(&stoppingStatus) + assert.NoError(t, err) + assert.True(t, done, "should use fresh API value, not stale cache") + assert.Equal(t, int32(0), *c.Statefulset.Spec.Replicas) + }) + + t.Run("Stopping with STS missing in API returns false", func(t *testing.T) { + c, _ := makeCluster(t) + c.Statefulset = makeSTS(int32Ptr(2)) + + done, err := c.checkStoppingCompleted(&stoppingStatus) + assert.NoError(t, err) + assert.False(t, done) + assert.Nil(t, c.Statefulset, "cache should be cleared when STS is missing") + }) } \ No newline at end of file From 293ebfc81b370c00f7ebd8a364ea79d27eadd99b Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Thu, 16 Jul 2026 01:58:56 +0530 Subject: [PATCH 11/13] SCF-830: Refactored and made hibernation sync cycle free --- pkg/cluster/cluster.go | 74 +- pkg/cluster/cluster_test.go | 287 ------ pkg/cluster/lifecycle.go | 267 +++-- pkg/cluster/lifecycle_test.go | 1820 +++++++++++---------------------- pkg/cluster/sync.go | 53 +- pkg/cluster/util.go | 23 + 6 files changed, 792 insertions(+), 1732 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 468163a0b..15ffe0761 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1026,11 +1026,10 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { return nil } - // If a previous Update already set a lifecycle status (e.g. persistWakeUpTransition - // wrote Updating), do not clobber it. The watch event that triggered this Update - // is a follow-up from the operator's own persistXxxTransition write, so the - // status subresource is already at the correct value, and writing again with - // this stale newSpec.resourceVersion would cause a 409 Conflict. + // If a previous Update already set a lifecycle status do not clobber it. + // The watch event that triggered this Update is a follow-up from the operator's + // write, so the status subresource is already at the correct value, and writing again + // would cause a 409 Conflict. isLifecycleActive := newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusUpdating || newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusStopping || newSpec.Status.PostgresClusterStatus == acidv1.ClusterStatusStopped @@ -1048,13 +1047,23 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { // do not apply any major version related changes yet newSpec.Spec.PostgresqlParam.PgVersion = oldSpec.Spec.PostgresqlParam.PgVersion } - c.setSpec(newSpec) + + // When isLifecycleActive, the watch delivered newSpec with a status from + // BEFORE our latest status write (e.g. status=Updating while c.Status is + // already Running). Clobbering c.Status with the stale value would break + // the next Update's blockLifecycleUpdate check — preserve the authoritative + // c.Status, only refresh the spec fields. + if !isLifecycleActive { + c.setSpec(newSpec) + } else { + cached, _ := c.GetSpec() + cached.Spec = newSpec.Spec + c.setSpec(cached) + } defer func() { if isLifecycleActive { - // Status was set by a previous Update (persistXxxTransition); do not - // overwrite Updating/Stopping/Stopped back to Running. Sync's defer - // will transition to Running once the cluster is ready. + // Status was set by a previous Update; sync's defer will take care return } currentStatus := newSpec.Status.DeepCopy() @@ -1283,53 +1292,6 @@ func (c *Cluster) blockLifecycleUpdate(newSpec *acidv1.Postgresql) (bool, error) return false, nil } -// handleHibernateAndWakeUp handles cluster hibernate/wake-up transitions during Update(). -// This is the Update path - it detects the transition, modifies the spec via manageHibernateState, -// and persists the changes to Kubernetes. -// -// Returns (handled bool, err error): -// - (true, nil) if lifecycle transition was handled, Update() should return early -// - (false, nil) if no lifecycle transition, normal update continues -// - (false, error) on error during persistence -// -// Flow: -// 1. Detect action via detectLifecycleTransition() -// 2. Check for Stopping->Stopped (via detectStoppingCompleted) -// 3. Call manageHibernateState() to prepare the spec (same logic as Sync path) -// 4. Persist to Kubernetes via persistHibernateTransition/persistWakeUpTransition -func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, error) { - action := detectLifecycleTransition( - &c.Status, - newSpec.Spec.Lifecycle, - newSpec.Spec.NumberOfInstances, - newSpec.Status.PreviousNumberOfInstances, - ) - - if action == LifecycleActionNone { - done, err := c.checkStoppingCompleted(&c.Status) - if err != nil { - return false, fmt.Errorf("could not check stopping completed: %w", err) - } - if done { - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped - return c.persistStoppingCompletedTransition(newSpec) - } - return false, nil - } - - c.manageHibernateState(c.Postgresql, newSpec) - - switch action { - case LifecycleActionHibernate: - return c.persistHibernateTransition(newSpec) - - case LifecycleActionWakeUp: - return c.persistWakeUpTransition(newSpec) - } - - return false, nil -} - func syncResources(a, b *v1.ResourceRequirements) bool { for _, res := range []v1.ResourceName{ v1.ResourceCPU, diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 2d54bf49c..a3d88f7b1 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -2355,291 +2355,4 @@ func TestUpdatePITRResources(t *testing.T) { } } -func TestUpdate_LifecycleBlocksDuringStopping(t *testing.T) { - clientSet := fake.NewSimpleClientset() - acidClientSet := fakeacidv1.NewSimpleClientset() - - client := k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - PostgresqlsGetter: acidClientSet.AcidV1(), - StatefulSetsGetter: clientSet.AppsV1(), - ServicesGetter: clientSet.CoreV1(), - SecretsGetter: clientSet.CoreV1(), - ConfigMapsGetter: clientSet.CoreV1(), - PodsGetter: clientSet.CoreV1(), - EndpointsGetter: clientSet.CoreV1(), - } - - pg := acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: acidv1.PostgresSpec{ - TeamID: "test-team", - NumberOfInstances: 3, - Volume: acidv1.Volume{Size: "1Gi"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Stopping", - }, - } - - cluster := New( - Config{ - OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", - }, - }, client, pg, logger, eventRecorder) - - cluster.Name = "test-cluster" - cluster.Namespace = "default" - - oldSpec := pg.DeepCopy() - newSpec := pg.DeepCopy() - newSpec.Spec.NumberOfInstances = 5 - - err := cluster.Update(oldSpec, newSpec) - - assert.Error(t, err) - assert.Contains(t, err.Error(), "cannot update cluster while it is stopping") -} - -func TestUpdate_LifecycleBlocksWhenStoppedWithPhase(t *testing.T) { - clientSet := fake.NewSimpleClientset() - acidClientSet := fakeacidv1.NewSimpleClientset() - - client := k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - PostgresqlsGetter: acidClientSet.AcidV1(), - StatefulSetsGetter: clientSet.AppsV1(), - ServicesGetter: clientSet.CoreV1(), - SecretsGetter: clientSet.CoreV1(), - ConfigMapsGetter: clientSet.CoreV1(), - PodsGetter: clientSet.CoreV1(), - EndpointsGetter: clientSet.CoreV1(), - } - - pg := acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: acidv1.PostgresSpec{ - TeamID: "test-team", - NumberOfInstances: 0, - Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, - Volume: acidv1.Volume{Size: "1Gi"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Stopped", - PreviousNumberOfInstances: 3, - }, - } - - cluster := New( - Config{ - OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", - }, - }, client, pg, logger, eventRecorder) - - cluster.Name = "test-cluster" - cluster.Namespace = "default" - - oldSpec := pg.DeepCopy() - newSpec := pg.DeepCopy() - newSpec.Spec.NumberOfInstances = 5 - - err := cluster.Update(oldSpec, newSpec) - - assert.Error(t, err) - assert.Contains(t, err.Error(), "cannot update cluster while stopped") -} - -func TestUpdate_LifecycleAllowsWakeUp(t *testing.T) { - clientSet := fake.NewSimpleClientset() - acidClientSet := fakeacidv1.NewSimpleClientset() - - updateCalled := false - statusUpdateCalled := false - - acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { - updateAction := action.(k8stesting.UpdateAction) - pg := updateAction.GetObject().(*acidv1.Postgresql) - updateCalled = true - return true, pg, nil - }) - acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { - if action.GetSubresource() == "status" { - statusUpdateCalled = true - updateAction := action.(k8stesting.UpdateAction) - pg := updateAction.GetObject().(*acidv1.Postgresql) - return true, pg, nil - } - return false, nil, nil - }) - - client := k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - PostgresqlsGetter: acidClientSet.AcidV1(), - StatefulSetsGetter: clientSet.AppsV1(), - ServicesGetter: clientSet.CoreV1(), - SecretsGetter: clientSet.CoreV1(), - ConfigMapsGetter: clientSet.CoreV1(), - PodsGetter: clientSet.CoreV1(), - EndpointsGetter: clientSet.CoreV1(), - } - - pg := acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: acidv1.PostgresSpec{ - TeamID: "test-team", - NumberOfInstances: 0, - Volume: acidv1.Volume{Size: "1Gi"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Stopped", - PreviousNumberOfInstances: 3, - }, - } - - cluster := New( - Config{ - OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", - }, - }, client, pg, logger, eventRecorder) - - cluster.Name = "test-cluster" - cluster.Namespace = "default" - - oldSpec := pg.DeepCopy() - newSpec := pg.DeepCopy() - - err := cluster.Update(oldSpec, newSpec) - - assert.NoError(t, err) - assert.True(t, updateCalled, "Update should have been called for wake-up") - assert.True(t, statusUpdateCalled, "Status update should have been called for wake-up") -} - -func TestUpdate_LifecycleInitiatesHibernate(t *testing.T) { - clientSet := fake.NewSimpleClientset() - acidClientSet := fakeacidv1.NewSimpleClientset() - - updateCalled := false - statusUpdateCalled := false - - acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { - updateAction := action.(k8stesting.UpdateAction) - pg := updateAction.GetObject().(*acidv1.Postgresql) - updateCalled = true - return true, pg, nil - }) - acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { - if action.GetSubresource() == "status" { - statusUpdateCalled = true - updateAction := action.(k8stesting.UpdateAction) - pg := updateAction.GetObject().(*acidv1.Postgresql) - return true, pg, nil - } - return false, nil, nil - }) - client := k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - PostgresqlsGetter: acidClientSet.AcidV1(), - StatefulSetsGetter: clientSet.AppsV1(), - ServicesGetter: clientSet.CoreV1(), - SecretsGetter: clientSet.CoreV1(), - ConfigMapsGetter: clientSet.CoreV1(), - PodsGetter: clientSet.CoreV1(), - EndpointsGetter: clientSet.CoreV1(), - } - - pg := acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: acidv1.PostgresSpec{ - TeamID: "test-team", - NumberOfInstances: 3, - Volume: acidv1.Volume{Size: "1Gi"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Running", - }, - } - - cluster := New( - Config{ - OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", - }, - }, client, pg, logger, eventRecorder) - - cluster.Name = "test-cluster" - cluster.Namespace = "default" - - oldSpec := pg.DeepCopy() - newSpec := pg.DeepCopy() - newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: "stopped"} - - err := cluster.Update(oldSpec, newSpec) - - assert.NoError(t, err) - assert.True(t, updateCalled, "Update should have been called for hibernate") - assert.True(t, statusUpdateCalled, "Status update should have been called for hibernate") -} - -func TestUpdate_LifecycleNormalUpdate(t *testing.T) { - clientSet := fake.NewSimpleClientset() - acidClientSet := fakeacidv1.NewSimpleClientset() - - client := k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - PostgresqlsGetter: acidClientSet.AcidV1(), - StatefulSetsGetter: clientSet.AppsV1(), - ServicesGetter: clientSet.CoreV1(), - SecretsGetter: clientSet.CoreV1(), - ConfigMapsGetter: clientSet.CoreV1(), - PodsGetter: clientSet.CoreV1(), - EndpointsGetter: clientSet.CoreV1(), - } - - pg := acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: acidv1.PostgresSpec{ - TeamID: "test-team", - NumberOfInstances: 3, - Volume: acidv1.Volume{Size: "1Gi"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Running", - }, - } - - cluster := New( - Config{ - OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", - }, - }, client, pg, logger, eventRecorder) - - cluster.Name = "test-cluster" - cluster.Namespace = "default" - - newSpec := pg.DeepCopy() - blocked, err := cluster.blockLifecycleUpdate(newSpec) - - assert.False(t, blocked) - assert.NoError(t, err) -} diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go index d352b30e0..8ee2c4086 100644 --- a/pkg/cluster/lifecycle.go +++ b/pkg/cluster/lifecycle.go @@ -9,7 +9,6 @@ import ( "k8s.io/apimachinery/pkg/types" acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" - "github.com/zalando/postgres-operator/pkg/util/k8sutil" ) // LifecycleAction represents the detected lifecycle transition for a cluster. @@ -59,52 +58,106 @@ func detectLifecycleTransition( return LifecycleActionNone } -// detectStoppingCompleted checks if the cluster should transition from Stopping to Stopped. -// This happens when the StatefulSet replicas have actually reached 0 (all pods terminated). -func detectStoppingCompleted(currentStatus *acidv1.PostgresStatus, statefulsetReplicas *int32) bool { - if !currentStatus.Stopping() { - return false +// handleHibernateAndWakeUp detects a hibernate/wake-up transition and asks +// syncStateLocked to mutate + persist the spec+status and run the full sync +// body. +// For hibernate it then waits inline for pods to terminate and persist Stopped. +// For wake-up the sync's existing defer transitions Updating → Running +// once resources exist. +// +// Returns (handled bool, err error): +// - (true, nil) lifecycle transition completed; Update() should return early +// - (false, nil) no lifecycle transition; Update() proceeds normally +// - (true, err) transition attempted but failed; caller returns the error +func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, error) { + action := detectLifecycleTransition( + &c.Status, + newSpec.Spec.Lifecycle, + newSpec.Spec.NumberOfInstances, + newSpec.Status.PreviousNumberOfInstances, + ) + + if action == LifecycleActionNone { + return false, nil } - if statefulsetReplicas == nil { - return false + + // syncStateLocked → prepareLifecycleTransition: detects the transition, + // mutates the spec via initiateHibernate/initiateWakeUp, and immediately + // persists spec+status. + if err := c.syncStateLocked(newSpec); err != nil { + return true, fmt.Errorf("could not sync after lifecycle transition: %w", err) } - return *statefulsetReplicas == 0 -} -// refreshStatefulset fetches the latest StatefulSet from the API and updates c.Statefulset. -// The in-memory c.Statefulset cache is only populated by Create() and by Sync() inside -// syncStatefulSet(); after an operator restart the cache is nil, and Sync() calls -// manageHibernateState before reaching syncStatefulSet(). Without this refresh, -// detectStoppingCompleted would see stale (or nil) replica counts and never transition -// to Stopped. If the StatefulSet is not found in the API, the cache is cleared and -// nil is returned (the cluster may have been deleted). -func (c *Cluster) refreshStatefulset() error { - sset, err := c.KubeClient.StatefulSets(c.Namespace).Get( - context.TODO(), c.statefulSetName(), metav1.GetOptions{}) - if err != nil { - if k8sutil.ResourceNotFound(err) { - c.Statefulset = nil - return nil + // Hibernate: wait inline for pods to actually terminate and persist Stopped. + // Doing this here (outside syncStateLocked) means a timeout returns an error + // without triggering the defer's SyncFailed status — the operator controller + // requeues and the next Sync (or this same Update on retry) resumes the wait. + if action == LifecycleActionHibernate { + if err := c.completeStoppingTransition(newSpec); err != nil { + return true, fmt.Errorf("hibernate failed: %w", err) } - return fmt.Errorf("could not refresh statefulset: %w", err) } - c.Statefulset = sset - return nil + + return true, nil } -// checkStoppingCompleted is an I/O wrapper around detectStoppingCompleted. It refreshes -// the StatefulSet cache before delegating so callers see fresh replica counts, especially -// after an operator restart where c.Statefulset is nil. To avoid an extra API call on the -// hot Update/Sync path, the refresh is skipped when status is not Stopping (the only -// state where detectStoppingCompleted can return true). -func (c *Cluster) checkStoppingCompleted(currentStatus *acidv1.PostgresStatus) (bool, error) { - if !currentStatus.Stopping() { +// prepareLifecycleTransition inspects newSpec for a lifecycle transition +// On detection it mutates the in-memory spec via initiateHibernate/initiateWakeUp +// and immediately persists BOTH spec and status to the K8s API so the new status +// (Stopping/Updating) is visible in same reconciliation pass — without waiting for +// syncStateLocked's defer. +// Uses oldSpec for detection so it works correctly when handleHibernateAndWakeUp +// has already mutated newSpec before calling syncStateLocked. +// +// Returns: +// - (true, nil): proceed with the rest of syncStateLocked (no transition, +// or transition was persisted) +// - (false, nil): skip sync — cluster is Stopped with no transition requested +// - (false, err): error persisting the spec or status; caller propagates to +// controller (next Update will retry the whole transition) +func (c *Cluster) prepareLifecycleTransition(newSpec **acidv1.Postgresql, oldSpec acidv1.Postgresql) (bool, error) { + spec := *newSpec + action := detectLifecycleTransition( + &oldSpec.Status, + spec.Spec.Lifecycle, + spec.Spec.NumberOfInstances, + spec.Status.PreviousNumberOfInstances, + ) + + // Stopped cluster with lifecycle=stopped: no reconciliation work. + if action == LifecycleActionNone && c.Status.Stopped() { return false, nil } - if err := c.refreshStatefulset(); err != nil { - return false, err + + switch action { + case LifecycleActionHibernate: + c.initiateHibernate(spec) + case LifecycleActionWakeUp: + c.initiateWakeUp(spec) + } + + if action == LifecycleActionHibernate || action == LifecycleActionWakeUp { + // Persist spec first (writes numInst/lifecycle + advances rv to N+1), + // then write the status subresource (advances rv to N+2). + pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), spec) + if err != nil { + return false, fmt.Errorf("could not update spec for lifecycle action: %w", err) + } + // UpdatePostgresCR returns the CR with a fresh resourceVersion but the + // status subresource may be empty/stale. Re-apply the lifecycle status + // fields before writing the status. + pgUpdated.Status.PreviousNumberOfInstances = spec.Status.PreviousNumberOfInstances + pgUpdated.Status.PreviousPoolerInstances = spec.Status.PreviousPoolerInstances + pgUpdated.Status.PostgresClusterStatus = spec.Status.PostgresClusterStatus + + pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) + if err != nil { + return false, fmt.Errorf("could not set status for lifecycle action: %w", err) + } + c.setSpec(pgUpdated) } - return detectStoppingCompleted(currentStatus, c.getStatefulsetReplicas()), nil + + return true, nil } // initiateHibernate prepares the cluster for hibernation by: @@ -132,12 +185,16 @@ func (c *Cluster) initiateHibernate(newSpec *acidv1.Postgresql) { } // initiateWakeUp prepares the cluster for wake-up by: -// - Restoring numberOfInstances from PreviousNumberOfInstances (if > 0) +// - Restoring numberOfInstances from PreviousNumberOfInstances (if > 0) +// - Scaling up connection pooler deployments (consumes PreviousPoolerInstances) +// - Resuming logical backup CronJob +// - Setting status to Updating +// - Clearing PreviousNumberOfInstances / PreviousPoolerInstances so they don't +// linger in the status subresource after the wake-up completes. The next +// hibernate overwrites them with fresh values. // -// - Setting status to Updating -// - Scaling up connection pooler deployments -// - Resuming logical backup CronJob -// If PreviousNumberOfInstances is 0, logs a warning but still sets status to Updating. +// If PreviousNumberOfInstances is 0, logs a warning but still sets status to +// Updating (operator-restart catch-up may have already cleared it). // Errors during pooler/backup operations are logged but do not fail the transition. func (c *Cluster) initiateWakeUp(newSpec *acidv1.Postgresql) { if newSpec.Status.PreviousNumberOfInstances > 0 { @@ -157,62 +214,9 @@ func (c *Cluster) initiateWakeUp(newSpec *acidv1.Postgresql) { if err := c.unsuspendLogicalBackupJob(); err != nil { c.logger.Warningf("[lifecycle] failed to resume logical backup job: %v", err) } -} - -// persistHibernateTransition persists the hibernate transition to Kubernetes by: -// 1. Updating the PostgresCR spec (numberOfInstances=0, previousNumberOfInstances stored) -// 2. Updating the PostgresCR status (status=Stopping, previousPoolerInstances stored) -// 3. Updating the local cluster spec -// Returns (handled=true, nil) on success, (handled=false, error) on failure. -func (c *Cluster) persistHibernateTransition(newSpec *acidv1.Postgresql) (bool, error) { - pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) - if err != nil { - return false, fmt.Errorf("could not update spec during hibernate: %w", err) - } - - pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances - pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus - pgUpdated.Status.PreviousPoolerInstances = newSpec.Status.PreviousPoolerInstances - - pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) - if err != nil { - return false, fmt.Errorf("could not update status during hibernate: %w", err) - } - c.setSpec(pgUpdated) - c.logger.Infof("[lifecycle] hibernate completed: cluster is stopping, numberOfInstances=0, previousNumberOfInstances=%d", - pgUpdated.Status.PreviousNumberOfInstances) - return true, nil -} - -// persistWakeUpTransition persists the wake-up transition to Kubernetes by: -// 1. Clearing PreviousNumberOfInstances and PreviousPoolerInstances -// 2. Updating the PostgresCR spec (numberOfInstances restored from previous) -// 3. Updating the PostgresCR status (status=Updating) -// 4. Updating the local cluster spec -// Returns (handled=true, nil) on success, (handled=false, error) on failure. -func (c *Cluster) persistWakeUpTransition(newSpec *acidv1.Postgresql) (bool, error) { newSpec.Status.PreviousNumberOfInstances = 0 newSpec.Status.PreviousPoolerInstances = nil - - pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) - if err != nil { - return false, fmt.Errorf("could not update spec during wake-up: %w", err) - } - - pgUpdated.Status.PreviousNumberOfInstances = newSpec.Status.PreviousNumberOfInstances - pgUpdated.Status.PreviousPoolerInstances = newSpec.Status.PreviousPoolerInstances - pgUpdated.Status.PostgresClusterStatus = newSpec.Status.PostgresClusterStatus - - pgUpdated, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), pgUpdated) - if err != nil { - return false, fmt.Errorf("could not update status during wake-up: %w", err) - } - - c.setSpec(pgUpdated) - c.logger.Infof("[lifecycle] wake-up completed: cluster is updating, numberOfInstances=%d", - pgUpdated.Spec.NumberOfInstances) - return true, nil } // persistStoppingCompletedTransition persists the Stopping->Stopped transition to Kubernetes @@ -230,63 +234,29 @@ func (c *Cluster) persistStoppingCompletedTransition(newSpec *acidv1.Postgresql) return true, nil } -// manageHibernateState handles cluster hibernate/wake-up state transitions during Sync(). -// This is the Sync path - it only modifies the in-memory spec. -// -// State transitions handled: -// - Running -> Stopping: Via initiateHibernate() (when lifecycle.phase="stopped") -// - Stopping -> Stopped: When StatefulSet replicas reach 0 (detected here) -// - Stopped -> Updating: Via initiateWakeUp() (when lifecycle cleared) -// -// Returns the detected LifecycleAction and a boolean indicating whether sync -// should continue (true) or return early (false, only when cluster is Stopped -// with lifecycle.phase="stopped" set). -func (c *Cluster) manageHibernateState(oldSpec acidv1.Postgresql, newSpec *acidv1.Postgresql) (LifecycleAction, bool) { - action := detectLifecycleTransition( - &newSpec.Status, - newSpec.Spec.Lifecycle, - newSpec.Spec.NumberOfInstances, - newSpec.Status.PreviousNumberOfInstances, - ) - - switch action { - case LifecycleActionHibernate: - c.initiateHibernate(newSpec) - return action, true - - case LifecycleActionWakeUp: - c.initiateWakeUp(newSpec) - return action, true +// completeStoppingTransition waits for the StatefulSet pods to actually terminate +// and transitions the cluster status from Stopping to Stopped. On timeout, returns +// an error so the caller can propagate it to the controller which will retry. +func (c *Cluster) completeStoppingTransition(newSpec *acidv1.Postgresql) error { + if err := c.waitStatefulsetPodsGone(); err != nil { + return fmt.Errorf("could not wait for pods to terminate: %w", err) } - // Check if Stopping -> Stopped transition is needed - done, err := c.checkStoppingCompleted(&newSpec.Status) + // Re-fetch the latest resourceVersion before writing Stopped to stop 409 and drift. + latest, err := c.KubeClient.Postgresqls(c.clusterNamespace()).Get( + context.TODO(), c.Name, metav1.GetOptions{}) if err != nil { - // Refresh failed; treat as not-completed and let the next Sync retry. - // Not returning the error so we don't mark the cluster SyncFailed on a - // transient API hiccup while the cluster is mid-stop. - c.logger.Warningf("[lifecycle] could not refresh statefulset for stopping check: %v", err) - } else if done { - newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped - c.logger.Info("[lifecycle] cluster has stopped, all pods are terminated") - return LifecycleActionStoppingCompleted, true + return fmt.Errorf("could not refresh postgresql before persisting Stopped: %w", err) } + newSpec.ResourceVersion = latest.ResourceVersion - // Skip sync if cluster is stopped and lifecycle.phase="stopped" is set - if newSpec.Status.Stopped() && newSpec.Spec.Lifecycle != nil && newSpec.Spec.Lifecycle.Phase == "stopped" { - return LifecycleActionNone, false + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped + c.setSpec(newSpec) + if _, err := c.persistStoppingCompletedTransition(newSpec); err != nil { + return err } - - return LifecycleActionNone, true -} - -// getStatefulsetReplicas returns the current replica count from the StatefulSet. -// Returns nil if StatefulSet is nil or Replicas field is nil. -func (c *Cluster) getStatefulsetReplicas() *int32 { - if c.Statefulset == nil || c.Statefulset.Spec.Replicas == nil { - return nil - } - return c.Statefulset.Spec.Replicas + c.logger.Info("[lifecycle] cluster has stopped") + return nil } // suspendLogicalBackupJob suspends the logical backup CronJob by setting spec.suspend=true. @@ -311,7 +281,7 @@ func (c *Cluster) suspendLogicalBackupJob() error { return fmt.Errorf("could not get logical backup job: %w", err) } - patchData := fmt.Sprintf(`{"spec":{"suspend":true}}`) + patchData := `{"spec":{"suspend":true}}` cronJob, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Patch( context.TODO(), c.getLogicalBackupJobName(), @@ -351,7 +321,7 @@ func (c *Cluster) unsuspendLogicalBackupJob() error { return fmt.Errorf("could not get logical backup job: %w", err) } - patchData := fmt.Sprintf(`{"spec":{"suspend":false}}`) + patchData := `{"spec":{"suspend":false}}` cronJob, err := c.KubeClient.CronJobsGetter.CronJobs(c.Namespace).Patch( context.TODO(), c.getLogicalBackupJobName(), @@ -448,4 +418,3 @@ func (c *Cluster) patchPoolerReplicas(role PostgresRole, replicas int32) error { } return nil } - diff --git a/pkg/cluster/lifecycle_test.go b/pkg/cluster/lifecycle_test.go index 0f1cdfad0..11a9730b1 100644 --- a/pkg/cluster/lifecycle_test.go +++ b/pkg/cluster/lifecycle_test.go @@ -25,40 +25,6 @@ var lifecycleEventRecorder = record.NewFakeRecorder(10) func int32Ptr(i int32) *int32 { return &i } -func newTestLifecycleCluster(status string, numberOfInstances int32, lifecyclePhase string) *Cluster { - pg := acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: acidv1.PostgresSpec{ - TeamID: "test-team", - NumberOfInstances: numberOfInstances, - Volume: acidv1.Volume{Size: "1Gi"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: status, - }, - } - - if lifecyclePhase != "" { - pg.Spec.Lifecycle = &acidv1.LifecycleSpec{ - Phase: lifecyclePhase, - } - } - - return &Cluster{ - Config: Config{ - OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", - }, - }, - Postgresql: pg, - logger: lifecycleLogger, - eventRecorder: lifecycleEventRecorder, - } -} - func newTestPoolerObjects(role PostgresRole, replicas int32) *ConnectionPoolerObjects { return &ConnectionPoolerObjects{ Deployment: &appsv1.Deployment{ @@ -82,46 +48,404 @@ func newFakeK8sClientForLifecycle() (*k8sutil.KubernetesClient, *fake.Clientset, acidClientSet := fakeacidv1.NewSimpleClientset() client := &k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - PostgresqlsGetter: acidClientSet.AcidV1(), - StatefulSetsGetter: clientSet.AppsV1(), - ServicesGetter: clientSet.CoreV1(), - SecretsGetter: clientSet.CoreV1(), + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), ConfigMapsGetter: clientSet.CoreV1(), - PodsGetter: clientSet.CoreV1(), - EndpointsGetter: clientSet.CoreV1(), + PodsGetter: clientSet.CoreV1(), + EndpointsGetter: clientSet.CoreV1(), + CronJobsGetter: clientSet.BatchV1(), } return client, clientSet, acidClientSet } -func createTestPostgresqlInClient(client *k8sutil.KubernetesClient, name, namespace string, spec *acidv1.PostgresSpec, status *acidv1.PostgresStatus) *acidv1.Postgresql { +// newLifecycleCluster builds a Cluster with the given current status and spec +// snapshot. The Postgres CR is pre-created in the fake clientset so K8s API +// calls work without a separate Create step. +func newLifecycleCluster( + client *k8sutil.KubernetesClient, + status string, + numberOfInstances int32, + lifecyclePhase string, + previousNumberOfInstances int32, + previousPoolerInstances map[string]int32, +) *Cluster { pg := &acidv1.Postgresql{ ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, + Name: "test-cluster", + Namespace: "default", + }, + Spec: acidv1.PostgresSpec{ + TeamID: "test-team", + NumberOfInstances: numberOfInstances, + Volume: acidv1.Volume{Size: "1Gi"}, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: status, + }, + } + if lifecyclePhase != "" { + pg.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: lifecyclePhase} + } + if previousNumberOfInstances > 0 { + pg.Status.PreviousNumberOfInstances = previousNumberOfInstances + } + if previousPoolerInstances != nil { + pg.Status.PreviousPoolerInstances = previousPoolerInstances + } + + created, err := client.Postgresqls("default").Create(context.TODO(), pg, metav1.CreateOptions{}) + if err != nil { + panic(fmt.Sprintf("failed to pre-create Postgresql: %v", err)) + } + + return &Cluster{ + Config: Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + LogicalBackup: config.LogicalBackup{ + LogicalBackupJobPrefix: "logical-backup-", + }, + }, + }, + Postgresql: *created, + KubeClient: *client, + logger: lifecycleLogger, + eventRecorder: lifecycleEventRecorder, + } +} + +func TestDetectLifecycleTransition(t *testing.T) { + tests := []struct { + name string + currentStatus string + newLifecyclePhase string // "" means nil lifecycle + newNumberOfInstances int32 + newPreviousNumberOfInstances int32 + want LifecycleAction + }{ + { + name: "Running + lifecycle.phase=stopped -> Hibernate", + currentStatus: acidv1.ClusterStatusRunning, + newLifecyclePhase: "stopped", + newNumberOfInstances: 3, + newPreviousNumberOfInstances: 0, + want: LifecycleActionHibernate, + }, + { + name: "Running + no lifecycle -> None", + currentStatus: acidv1.ClusterStatusRunning, + newLifecyclePhase: "", + newNumberOfInstances: 3, + newPreviousNumberOfInstances: 0, + want: LifecycleActionNone, + }, + { + name: "Stopping + lifecycle.phase=stopped -> None (already stopping)", + currentStatus: acidv1.ClusterStatusStopping, + newLifecyclePhase: "stopped", + newNumberOfInstances: 0, + newPreviousNumberOfInstances: 3, + want: LifecycleActionNone, + }, + { + name: "Stopped + lifecycle.phase=stopped -> None (still hibernated)", + currentStatus: acidv1.ClusterStatusStopped, + newLifecyclePhase: "stopped", + newNumberOfInstances: 0, + newPreviousNumberOfInstances: 3, + want: LifecycleActionNone, + }, + { + name: "Stopped + lifecycle cleared + has previous instances + numInst=0 -> WakeUp", + currentStatus: acidv1.ClusterStatusStopped, + newLifecyclePhase: "", + newNumberOfInstances: 0, + newPreviousNumberOfInstances: 3, + want: LifecycleActionWakeUp, + }, + { + name: "Running + lifecycle cleared + previous instances + numInst=0 -> WakeUp", + currentStatus: acidv1.ClusterStatusRunning, + newLifecyclePhase: "", + newNumberOfInstances: 0, + newPreviousNumberOfInstances: 3, + want: LifecycleActionWakeUp, + }, + { + name: "Running + lifecycle cleared + previous instances + numInst>0 -> None", + currentStatus: acidv1.ClusterStatusRunning, + newLifecyclePhase: "", + newNumberOfInstances: 3, + newPreviousNumberOfInstances: 3, + want: LifecycleActionNone, + }, + { + name: "Running + lifecycle cleared + no previous instances -> None", + currentStatus: acidv1.ClusterStatusRunning, + newLifecyclePhase: "", + newNumberOfInstances: 3, + newPreviousNumberOfInstances: 0, + want: LifecycleActionNone, + }, + { + name: "Stopped + lifecycle cleared + no previous instances -> WakeUp (operator restart catch-up)", + currentStatus: acidv1.ClusterStatusStopped, + newLifecyclePhase: "", + newNumberOfInstances: 0, + newPreviousNumberOfInstances: 0, + want: LifecycleActionWakeUp, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var lifecycle *acidv1.LifecycleSpec + if tt.newLifecyclePhase != "" { + lifecycle = &acidv1.LifecycleSpec{Phase: tt.newLifecyclePhase} + } + + status := acidv1.PostgresStatus{PostgresClusterStatus: tt.currentStatus} + + got := detectLifecycleTransition( + &status, + lifecycle, + tt.newNumberOfInstances, + tt.newPreviousNumberOfInstances, + ) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestInitiateHibernate(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusRunning, 3, "", 0, nil) + + newSpec := &acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster", Namespace: "default"}, + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 3, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: acidv1.ClusterStatusRunning, }, } - if spec != nil { - pg.Spec = *spec + + c.initiateHibernate(newSpec) + + assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances, "numberOfInstances should be set to 0") + assert.Equal(t, int32(3), newSpec.Status.PreviousNumberOfInstances, "previousNumberOfInstances should be stored") + assert.Equal(t, acidv1.ClusterStatusStopping, newSpec.Status.PostgresClusterStatus, "status should be Stopping") +} + +func TestInitiateWakeUp(t *testing.T) { + t.Run("restores numberOfInstances and clears Previous fields", func(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusStopped, 0, "", 0, nil) + + newSpec := &acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster", Namespace: "default"}, + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 0, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: acidv1.ClusterStatusStopped, + PreviousNumberOfInstances: 3, + PreviousPoolerInstances: map[string]int32{"master": 2, "replica": 0}, + }, + } + + c.initiateWakeUp(newSpec) + + assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances, "numberOfInstances should be restored") + assert.Equal(t, acidv1.ClusterStatusUpdating, newSpec.Status.PostgresClusterStatus, "status should be Updating") + assert.Equal(t, int32(0), newSpec.Status.PreviousNumberOfInstances, "previousNumberOfInstances should be cleared") + assert.Nil(t, newSpec.Status.PreviousPoolerInstances, "previousPoolerInstances should be cleared") + }) + + t.Run("previousNumberOfInstances=0 still transitions to Updating", func(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusStopped, 0, "", 0, nil) + + newSpec := &acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster", Namespace: "default"}, + Spec: acidv1.PostgresSpec{ + NumberOfInstances: 0, + }, + Status: acidv1.PostgresStatus{ + PostgresClusterStatus: acidv1.ClusterStatusStopped, + PreviousPoolerInstances: map[string]int32{"master": 1}, + }, + } + + c.initiateWakeUp(newSpec) + + assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances, "numberOfInstances stays 0 when previous is 0") + assert.Equal(t, acidv1.ClusterStatusUpdating, newSpec.Status.PostgresClusterStatus, "status should still be Updating") + assert.Equal(t, int32(0), newSpec.Status.PreviousNumberOfInstances, "previousNumberOfInstances stays 0") + assert.Nil(t, newSpec.Status.PreviousPoolerInstances, "previousPoolerInstances should be cleared") + }) +} + +func TestPrepareLifecycleTransition_Hibernate(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusRunning, 3, "", 0, nil) + + newSpec := c.Postgresql.DeepCopy() + newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: "stopped"} + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusRunning}, + } + + proceed, err := c.prepareLifecycleTransition(&newSpec, oldSpec) + + assert.NoError(t, err) + assert.True(t, proceed, "hibernate should proceed with sync") + assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances, "numberOfInstances should be set to 0") + assert.Equal(t, int32(3), newSpec.Status.PreviousNumberOfInstances, "previousNumberOfInstances should be stored") + assert.Equal(t, acidv1.ClusterStatusStopping, newSpec.Status.PostgresClusterStatus, "status should be Stopping") + + persisted, err := client.Postgresqls("default").Get(context.TODO(), "test-cluster", metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, acidv1.ClusterStatusStopping, persisted.Status.PostgresClusterStatus, "persisted status should be Stopping") + assert.Equal(t, int32(3), persisted.Status.PreviousNumberOfInstances, "persisted previousNumberOfInstances should be 3") + assert.Equal(t, int32(0), persisted.Spec.NumberOfInstances, "persisted numberOfInstances should be 0") +} + +func TestPrepareLifecycleTransition_WakeUp(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster( + client, + acidv1.ClusterStatusStopped, + 0, + "", + 3, + map[string]int32{"master": 2, "replica": 0}, + ) + + newSpec := c.Postgresql.DeepCopy() + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusStopped}, } - if status != nil { - pg.Status = *status + + proceed, err := c.prepareLifecycleTransition(&newSpec, oldSpec) + + assert.NoError(t, err) + assert.True(t, proceed, "wake-up should proceed with sync") + assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances, "numberOfInstances should be restored") + assert.Equal(t, acidv1.ClusterStatusUpdating, newSpec.Status.PostgresClusterStatus, "status should be Updating") + assert.Equal(t, int32(0), newSpec.Status.PreviousNumberOfInstances, "previousNumberOfInstances should be cleared after persistence") + assert.Nil(t, newSpec.Status.PreviousPoolerInstances, "previousPoolerInstances should be cleared after persistence") + + persisted, err := client.Postgresqls("default").Get(context.TODO(), "test-cluster", metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, acidv1.ClusterStatusUpdating, persisted.Status.PostgresClusterStatus, "persisted status should be Updating") + assert.Equal(t, int32(3), persisted.Spec.NumberOfInstances, "persisted numberOfInstances should be 3") + assert.Equal(t, int32(0), persisted.Status.PreviousNumberOfInstances, "persisted previousNumberOfInstances should be 0") + assert.Nil(t, persisted.Status.PreviousPoolerInstances, "persisted previousPoolerInstances should be nil") +} + +func TestPrepareLifecycleTransition_StoppedNoTransition(t *testing.T) { + client, _, acidClientSet := newFakeK8sClientForLifecycle() + c := newLifecycleCluster( + client, + acidv1.ClusterStatusStopped, + 0, + "stopped", + 3, + nil, + ) + + updateCalled := false + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + updateCalled = true + return false, nil, nil + }) + + newSpec := c.Postgresql.DeepCopy() + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusStopped}, } - created, err := client.Postgresqls(namespace).Create(context.TODO(), pg, metav1.CreateOptions{}) - if err == nil { - return created + proceed, err := c.prepareLifecycleTransition(&newSpec, oldSpec) + + assert.NoError(t, err) + assert.False(t, proceed, "stopped cluster with lifecycle=stopped should skip sync") + assert.False(t, updateCalled, "no K8s API writes should happen for stopped-no-transition") + assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances, "numberOfInstances should be unchanged") + assert.Equal(t, acidv1.ClusterStatusStopped, newSpec.Status.PostgresClusterStatus, "status should remain Stopped") +} + +func TestPrepareLifecycleTransition_NoTransitionRunning(t *testing.T) { + client, _, acidClientSet := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusRunning, 3, "", 0, nil) + + updateCalled := false + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + updateCalled = true + return false, nil, nil + }) + + newSpec := c.Postgresql.DeepCopy() + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusRunning}, } - return pg + + proceed, err := c.prepareLifecycleTransition(&newSpec, oldSpec) + + assert.NoError(t, err) + assert.True(t, proceed, "no-transition running cluster should proceed with sync") + assert.False(t, updateCalled, "no K8s API writes should happen for no-transition") + assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances, "numberOfInstances should be unchanged") + assert.Equal(t, acidv1.ClusterStatusRunning, newSpec.Status.PostgresClusterStatus, "status should remain Running") } -func updatePostgresqlInClient(client *k8sutil.KubernetesClient, pg *acidv1.Postgresql) (*acidv1.Postgresql, error) { - return client.Postgresqls(pg.Namespace).Update(context.TODO(), pg, metav1.UpdateOptions{}) +func TestPrepareLifecycleTransition_UpdateSpecFails(t *testing.T) { + client, _, acidClientSet := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusRunning, 3, "", 0, nil) + + acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("api server unavailable") + }) + + newSpec := c.Postgresql.DeepCopy() + newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: "stopped"} + + oldSpec := acidv1.Postgresql{ + Status: acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusRunning}, + } + + proceed, err := c.prepareLifecycleTransition(&newSpec, oldSpec) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "could not update spec for lifecycle action") + assert.False(t, proceed, "should not proceed when spec write fails") } -func updatePostgresqlStatusInClient(client *k8sutil.KubernetesClient, pg *acidv1.Postgresql) (*acidv1.Postgresql, error) { - return client.Postgresqls(pg.Namespace).UpdateStatus(context.TODO(), pg, metav1.UpdateOptions{}) +func TestPersistStoppingCompletedTransition(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusStopping, 0, "stopped", 3, nil) + + newSpec := c.Postgresql.DeepCopy() + newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusStopped + + handled, err := c.persistStoppingCompletedTransition(newSpec) + + assert.NoError(t, err) + assert.True(t, handled, "should report handled=true on success") + + persisted, err := client.Postgresqls("default").Get(context.TODO(), "test-cluster", metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, acidv1.ClusterStatusStopped, persisted.Status.PostgresClusterStatus, "persisted status should be Stopped") + assert.Equal(t, acidv1.ClusterStatusStopped, c.Postgresql.Status.PostgresClusterStatus, "cache should reflect Stopped") } func TestGetPoolerReplicas(t *testing.T) { @@ -203,944 +527,165 @@ func TestPatchPoolerReplicas(t *testing.T) { errContains string }{ { - name: "deployment exists, patch succeeds", - replicas: 0, + name: "deployment exists, patch succeeds", + replicas: 0, setupClient: func(clientSet *fake.Clientset) { - // Pre-create the deployment in the fake clientset - clientSet.AppsV1().Deployments("default").Create(context.TODO(), &appsv1.Deployment{ + _, _ = clientSet.AppsV1().Deployments("default").Create(context.TODO(), &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-pooler"}, - Spec: appsv1.DeploymentSpec{Replicas: int32Ptr(2)}, + Spec: appsv1.DeploymentSpec{Replicas: int32Ptr(2)}, }, metav1.CreateOptions{}) }, wantErr: false, }, { - name: "deployment not found", - replicas: 2, - setupClient: func(clientSet *fake.Clientset) { - // Don't create anything - will trigger NotFound - }, - wantErr: false, // NotFound is handled gracefully - }, - { - name: "patch returns error", + name: "deployment not found - returns nil", replicas: 2, - setupClient: func(clientSet *fake.Clientset) { - // Pre-create deployment but make patch fail via reactor - clientSet.AppsV1().Deployments("default").Create(context.TODO(), &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-pooler"}, - Spec: appsv1.DeploymentSpec{Replicas: int32Ptr(2)}, - }, metav1.CreateOptions{}) - clientSet.PrependReactor("patch", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { - return true, nil, fmt.Errorf("network error") - }) - }, - wantErr: true, - errContains: "could not patch pooler deployment", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - clientSet := fake.NewSimpleClientset() - if tt.setupClient != nil { - tt.setupClient(clientSet) - } - - kubeClient := &k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - } - - c := &Cluster{ - KubeClient: *kubeClient, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - }, - } - - err := c.patchPoolerReplicas(Master, tt.replicas) - if tt.wantErr { - assert.Error(t, err) - if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains) - } - } else { - assert.NoError(t, err) - } - }) - } -} - -func TestBlockLifecycleUpdate(t *testing.T) { - tests := []struct { - name string - currentStatus string - lifecyclePhase string - wantBlocked bool - wantErr bool - errContains string - }{ - { - name: "Running cluster, allows update", - currentStatus: "Running", - wantBlocked: false, - wantErr: false, - }, - { - name: "Stopping state, blocks update", - currentStatus: "Stopping", - wantBlocked: true, - wantErr: true, - errContains: "cannot update cluster while it is stopping", - }, - { - name: "Stopped with lifecycle phase, blocks update", - currentStatus: "Stopped", - lifecyclePhase: "stopped", - wantBlocked: true, - wantErr: true, - errContains: "cannot update cluster while stopped", + setupClient: func(clientSet *fake.Clientset) {}, + wantErr: false, }, - { - name: "Stopped without lifecycle phase, allows update (wake-up)", - currentStatus: "Stopped", - lifecyclePhase: "", - wantBlocked: false, - wantErr: false, - }, - { - name: "Stopped with nil lifecycle, allows update (wake-up)", - currentStatus: "Stopped", - lifecyclePhase: "", - wantBlocked: false, - wantErr: false, - }, - { - name: "Stopped with empty lifecycle phase, allows update (wake-up)", - currentStatus: "Stopped", - lifecyclePhase: "", - wantBlocked: false, - wantErr: false, - }, - { - name: "UpdateFailed state, allows update", - currentStatus: "UpdateFailed", - wantBlocked: false, - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c := newTestLifecycleCluster(tt.currentStatus, 3, tt.lifecyclePhase) - - newSpec := c.DeepCopy() - if tt.lifecyclePhase != "" && tt.currentStatus != "Stopped" { - newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: tt.lifecyclePhase} - } - - blocked, err := c.blockLifecycleUpdate(newSpec) - - if tt.wantErr { - assert.Error(t, err) - if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains) - } - } else { - assert.NoError(t, err) - } - assert.Equal(t, tt.wantBlocked, blocked) - }) - } -} - -func TestManageHibernateState(t *testing.T) { - tests := []struct { - name string - oldSpecStatus string - newSpecStatus string - newSpecLifecyclePhase string - numberOfInstances int32 - previousNumberOfInst int32 - statefulsetReplicas *int32 - wantContinue bool - wantNumberOfInstances *int32 - wantStatus string - }{ - { - name: "Running to Stopping - initiates hibernate", - oldSpecStatus: "Running", - newSpecStatus: "Running", - newSpecLifecyclePhase: "stopped", - numberOfInstances: 3, - statefulsetReplicas: int32Ptr(3), - wantContinue: true, - wantNumberOfInstances: int32Ptr(0), - wantStatus: "Stopping", - }, - { - name: "Stopping to Stopped - when replicas reach 0", - oldSpecStatus: "Stopping", - newSpecStatus: "Stopping", - numberOfInstances: 0, - statefulsetReplicas: int32Ptr(0), - wantContinue: true, - wantNumberOfInstances: int32Ptr(0), - wantStatus: "Stopped", - }, - { - name: "Stopping - replicas not yet 0", - oldSpecStatus: "Stopping", - newSpecStatus: "Stopping", - numberOfInstances: 0, - statefulsetReplicas: int32Ptr(2), // Still terminating - wantContinue: true, - wantNumberOfInstances: nil, // Should NOT change - wantStatus: "Stopping", // Should NOT change - }, - { - name: "Stopped to wake-up - restores numberOfInstances", - oldSpecStatus: "Stopped", - newSpecStatus: "Stopped", - newSpecLifecyclePhase: "", // Cleared - numberOfInstances: 0, - previousNumberOfInst: 3, - statefulsetReplicas: int32Ptr(0), - wantContinue: true, - wantNumberOfInstances: int32Ptr(3), - wantStatus: "Updating", - }, - { - name: "Stopped but lifecycle still 'stopped' - skip sync", - oldSpecStatus: "Stopped", - newSpecStatus: "Stopped", - newSpecLifecyclePhase: "stopped", - numberOfInstances: 0, - statefulsetReplicas: int32Ptr(0), - wantContinue: false, // Should skip sync - wantNumberOfInstances: nil, - wantStatus: "Stopped", - }, - { - name: "Running without lifecycle - continue normal", - oldSpecStatus: "Running", - newSpecStatus: "Running", - newSpecLifecyclePhase: "", - numberOfInstances: 3, - statefulsetReplicas: int32Ptr(3), - wantContinue: true, - wantNumberOfInstances: int32Ptr(3), // Unchanged - wantStatus: "Running", // Unchanged - }, - { - name: "Running to Updating - normal update", - oldSpecStatus: "Running", - newSpecStatus: "Updating", - newSpecLifecyclePhase: "", - numberOfInstances: 3, - statefulsetReplicas: int32Ptr(3), - wantContinue: true, - wantNumberOfInstances: int32Ptr(3), - wantStatus: "Updating", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() - - if tt.statefulsetReplicas != nil { - _, err := clientSet.AppsV1().StatefulSets("default").Create( - context.TODO(), - &appsv1.StatefulSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: appsv1.StatefulSetSpec{ - Replicas: tt.statefulsetReplicas, - }, - }, - metav1.CreateOptions{}, - ) - assert.NoError(t, err) - } - - c := &Cluster{ - logger: lifecycleLogger, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - }, - KubeClient: *kubeClient, - } - - oldSpec := acidv1.Postgresql{ - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: tt.oldSpecStatus, - }, - } - - newSpec := &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{ - NumberOfInstances: tt.numberOfInstances, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: tt.newSpecStatus, - PreviousNumberOfInstances: tt.previousNumberOfInst, - }, - } - - if tt.newSpecLifecyclePhase != "" { - newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{ - Phase: tt.newSpecLifecyclePhase, - } - } - - _, gotContinue := c.manageHibernateState(oldSpec, newSpec) - - assert.Equal(t, tt.wantContinue, gotContinue) - - if tt.wantNumberOfInstances != nil { - assert.Equal(t, *tt.wantNumberOfInstances, newSpec.Spec.NumberOfInstances) - } - assert.Equal(t, tt.wantStatus, newSpec.Status.PostgresClusterStatus) - }) - } -} - -func TestHandleHibernateAndWakeUp_Hibernate(t *testing.T) { - tests := []struct { - name string - currentStatus string - numberOfInstances int32 - lifecyclePhase string - poolerObjs map[PostgresRole]*ConnectionPoolerObjects - k8sUpdateSucceeds bool - k8sStatusSucceeds bool - poolerPatchSucceeds bool - wantHandled bool - wantErr bool - errContains string - }{ - { - name: "Running + lifecycle.phase=stopped - initiates hibernate", - currentStatus: "Running", - numberOfInstances: 3, - lifecyclePhase: "stopped", - k8sUpdateSucceeds: true, - k8sStatusSucceeds: true, - poolerPatchSucceeds: true, - wantHandled: true, - wantErr: false, - }, - { - name: "Running + lifecycle.phase=stopped - K8s update fails", - currentStatus: "Running", - numberOfInstances: 3, - lifecyclePhase: "stopped", - k8sUpdateSucceeds: false, - wantHandled: false, - wantErr: true, - errContains: "could not update spec during hibernate", - }, - { - name: "Running + lifecycle.phase=stopped - K8s status update fails", - currentStatus: "Running", - numberOfInstances: 3, - lifecyclePhase: "stopped", - k8sUpdateSucceeds: true, - k8sStatusSucceeds: false, - wantHandled: false, - wantErr: true, - errContains: "could not update status during hibernate", - }, - { - name: "Running + no lifecycle phase - no transition", - currentStatus: "Running", - numberOfInstances: 3, - lifecyclePhase: "", - wantHandled: false, - wantErr: false, - }, - { - name: "Stopped + lifecycle cleared - initiates wake-up", - currentStatus: "Stopped", - numberOfInstances: 0, - lifecyclePhase: "", - k8sUpdateSucceeds: true, - k8sStatusSucceeds: true, - wantHandled: true, - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - kubeClient, _, acidClientSet := newFakeK8sClientForLifecycle() - - if tt.k8sUpdateSucceeds { - acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { - updateAction := action.(k8stesting.UpdateAction) - pg := updateAction.GetObject().(*acidv1.Postgresql) - return true, pg, nil - }) - } - - if !tt.k8sStatusSucceeds { - acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { - if action.GetSubresource() == "status" { - return true, nil, fmt.Errorf("status update failed") - } - return false, nil, nil - }) - } - - c := &Cluster{ - Config: Config{ - OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", - }, - }, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: acidv1.PostgresSpec{ - TeamID: "test-team", - NumberOfInstances: tt.numberOfInstances, - Volume: acidv1.Volume{Size: "1Gi"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: tt.currentStatus, - }, - }, - KubeClient: *kubeClient, - ConnectionPooler: tt.poolerObjs, - logger: lifecycleLogger, - eventRecorder: lifecycleEventRecorder, - } - - newSpec := c.DeepCopy() - if tt.lifecyclePhase != "" { - newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: tt.lifecyclePhase} - } - - handled, err := c.handleHibernateAndWakeUp(newSpec) - - if tt.wantErr { - assert.Error(t, err) - if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains) - } - } else { - assert.NoError(t, err) - } - assert.Equal(t, tt.wantHandled, handled) - }) - } -} - -func TestHandleHibernateAndWakeUp_WakeUp(t *testing.T) { - tests := []struct { - name string - currentStatus string - numberOfInstances int32 - previousNumberOfInst int32 - previousPoolerInstances map[string]int32 - lifecyclePhase string - k8sUpdateSucceeds bool - k8sStatusSucceeds bool - poolerPatchSucceeds bool - wantHandled bool - wantErr bool - errContains string - }{ - { - name: "Stopped + lifecycle cleared - wake-up", - currentStatus: "Stopped", - numberOfInstances: 0, - previousNumberOfInst: 3, - previousPoolerInstances: map[string]int32{"master": 2, "replica": 0}, - lifecyclePhase: "", - k8sUpdateSucceeds: true, - k8sStatusSucceeds: true, - wantHandled: true, - wantErr: false, - }, - { - name: "Stopped + previousNumberOfInstances is 0 - still initiates wake-up", - currentStatus: "Stopped", - numberOfInstances: 0, - previousNumberOfInst: 0, - lifecyclePhase: "", - k8sUpdateSucceeds: true, - k8sStatusSucceeds: true, - wantHandled: true, - wantErr: false, - }, - { - name: "Stopped + lifecycle cleared + K8s update fails", - currentStatus: "Stopped", - numberOfInstances: 0, - previousNumberOfInst: 3, - lifecyclePhase: "", - k8sUpdateSucceeds: false, - wantHandled: false, - wantErr: true, - errContains: "could not update spec during wake-up", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - kubeClient, _, acidClientSet := newFakeK8sClientForLifecycle() - - if tt.k8sUpdateSucceeds { - acidClientSet.PrependReactor("update", "postgresqls", func(action k8stesting.Action) (bool, runtime.Object, error) { - updateAction := action.(k8stesting.UpdateAction) - pg := updateAction.GetObject().(*acidv1.Postgresql) - return true, pg, nil - }) - } - - c := &Cluster{ - Config: Config{ - OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", - }, - }, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: acidv1.PostgresSpec{ - TeamID: "test-team", - NumberOfInstances: tt.numberOfInstances, - Volume: acidv1.Volume{Size: "1Gi"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: tt.currentStatus, - PreviousNumberOfInstances: tt.previousNumberOfInst, - PreviousPoolerInstances: tt.previousPoolerInstances, - }, - }, - KubeClient: *kubeClient, - logger: lifecycleLogger, - eventRecorder: lifecycleEventRecorder, - } - - newSpec := c.DeepCopy() - - handled, err := c.handleHibernateAndWakeUp(newSpec) - - if tt.wantErr { - assert.Error(t, err) - if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains) - } - } else { - assert.NoError(t, err) - } - assert.Equal(t, tt.wantHandled, handled) - }) - } -} - -func TestScalePoolerDown(t *testing.T) { - tests := []struct { - name string - poolerObjs map[PostgresRole]*ConnectionPoolerObjects - wantStored map[string]int32 - }{ - { - name: "nil ConnectionPooler - no-op", - poolerObjs: nil, - wantStored: nil, - }, - { - name: "Master at 2 replicas", - poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{Master: newTestPoolerObjects(Master, 2)}, - wantStored: map[string]int32{"master": 2}, - }, - { - name: "Master already at 0 replicas", - poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{Master: newTestPoolerObjects(Master, 0)}, - wantStored: map[string]int32{"master": 0}, - }, - { - name: "Both Master and Replica", - poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{ - Master: newTestPoolerObjects(Master, 2), - Replica: newTestPoolerObjects(Replica, 1), - }, - wantStored: map[string]int32{"master": 2, "replica": 1}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - clientSet := fake.NewSimpleClientset() - kubeClient := &k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - } - - c := &Cluster{ - KubeClient: *kubeClient, - ConnectionPooler: tt.poolerObjs, - logger: lifecycleLogger, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - }, - } - - newSpec := &acidv1.Postgresql{} - err := c.scalePoolerDown(newSpec) - - assert.NoError(t, err) - assert.Equal(t, tt.wantStored, newSpec.Status.PreviousPoolerInstances) - }) - } -} - -func TestScalePoolerUp(t *testing.T) { - tests := []struct { - name string - prevInst map[string]int32 - wantErr bool - }{ - { - name: "nil PreviousPoolerInstances - no-op", - prevInst: nil, - }, - { - name: "Restore master to 2", - prevInst: map[string]int32{"master": 2}, - }, - { - name: "Restore both roles", - prevInst: map[string]int32{"master": 2, "replica": 1}, - }, - { - name: "Restore master to 0 (keep at 0)", - prevInst: map[string]int32{"master": 0}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - clientSet := fake.NewSimpleClientset() - kubeClient := &k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - } - - c := &Cluster{ - KubeClient: *kubeClient, - logger: lifecycleLogger, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - }, - } - - newSpec := &acidv1.Postgresql{ - Status: acidv1.PostgresStatus{ - PreviousPoolerInstances: tt.prevInst, - }, - } - err := c.scalePoolerUp(newSpec) - - assert.NoError(t, err) - }) - } -} - -func TestLifecycleStateTransitions(t *testing.T) { - t.Run("complete Hibernate flow: Running -> Stopping -> Stopped", func(t *testing.T) { - c := &Cluster{ - logger: lifecycleLogger, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - }, - } - - // Initial: Running - oldSpec := acidv1.Postgresql{ - Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, - } - newSpec := &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{ - NumberOfInstances: 3, - Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, - }, - Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, - } - - // Step 1: manageHibernateState should initiate hibernate - _, continueSync := c.manageHibernateState(oldSpec, newSpec) - assert.True(t, continueSync) - assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances) - assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) - assert.Equal(t, int32(3), newSpec.Status.PreviousNumberOfInstances) - }) - - t.Run("complete Wake-up flow: Stopped -> Updating -> Running", func(t *testing.T) { - c := &Cluster{ - logger: lifecycleLogger, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - }, - } - - // After hibernate: Stopped, replicas = 0 - c.Statefulset = &appsv1.StatefulSet{ - Spec: appsv1.StatefulSetSpec{Replicas: int32Ptr(0)}, - } - - oldSpec := acidv1.Postgresql{ - Status: acidv1.PostgresStatus{PostgresClusterStatus: "Stopped"}, - } - newSpec := &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{ - NumberOfInstances: 0, - // Lifecycle cleared by user - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Stopped", - PreviousNumberOfInstances: 3, - }, - } - - // Step 1: manageHibernateState should restore - _, continueSync := c.manageHibernateState(oldSpec, newSpec) - assert.True(t, continueSync) - assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) - assert.Equal(t, "Updating", newSpec.Status.PostgresClusterStatus) - }) -} - -func TestLifecycleUpdateBlocksDuringStopping(t *testing.T) { - kubeClient, _, _ := newFakeK8sClientForLifecycle() - - c := &Cluster{ - Config: Config{ - OpConfig: config.Config{}, - }, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: acidv1.PostgresSpec{ - NumberOfInstances: 0, - Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Stopping", - }, - }, - KubeClient: *kubeClient, - logger: lifecycleLogger, - } - - newSpec := c.DeepCopy() - blocked, err := c.blockLifecycleUpdate(newSpec) - - assert.True(t, blocked) - assert.Error(t, err) - assert.Contains(t, err.Error(), "cannot update cluster while it is stopping") -} - -func TestLifecycleUpdateBlocksWhenStoppedWithPhase(t *testing.T) { - kubeClient, _, _ := newFakeK8sClientForLifecycle() - - c := &Cluster{ - Config: Config{ - OpConfig: config.Config{}, - }, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: acidv1.PostgresSpec{ - NumberOfInstances: 0, - Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Stopped", - }, - }, - KubeClient: *kubeClient, - logger: lifecycleLogger, - } - - newSpec := c.DeepCopy() - blocked, err := c.blockLifecycleUpdate(newSpec) - - assert.True(t, blocked) - assert.Error(t, err) - assert.Contains(t, err.Error(), "cannot update cluster while stopped") -} - -func TestLifecycleUpdateAllowsWakeUp(t *testing.T) { - kubeClient, _, _ := newFakeK8sClientForLifecycle() - - c := &Cluster{ - Config: Config{ - OpConfig: config.Config{}, - }, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: acidv1.PostgresSpec{ - NumberOfInstances: 0, - // Lifecycle cleared by user - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Stopped", - PreviousNumberOfInstances: 3, + { + name: "patch returns error", + replicas: 2, + setupClient: func(clientSet *fake.Clientset) { + _, _ = clientSet.AppsV1().Deployments("default").Create(context.TODO(), &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-pooler"}, + Spec: appsv1.DeploymentSpec{Replicas: int32Ptr(2)}, + }, metav1.CreateOptions{}) + clientSet.PrependReactor("patch", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("network error") + }) }, + wantErr: true, + errContains: "could not patch pooler deployment", }, - KubeClient: *kubeClient, - logger: lifecycleLogger, } - newSpec := c.DeepCopy() - blocked, err := c.blockLifecycleUpdate(newSpec) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientSet := fake.NewSimpleClientset() + if tt.setupClient != nil { + tt.setupClient(clientSet) + } - assert.False(t, blocked) - assert.NoError(t, err) + kubeClient := &k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + } + + c := &Cluster{ + KubeClient: *kubeClient, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + } + + err := c.patchPoolerReplicas(Master, tt.replicas) + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + }) + } } -func TestManageHibernateState_EdgeCases(t *testing.T) { +func TestScalePoolerDown(t *testing.T) { tests := []struct { - name string - statefulsetReplicas *int32 - currentStatus string - newSpecLifecyclePhase string - newSpecNumberOfInst int32 - previousNumberOfInst int32 - wantContinue bool - wantNumberOfInstances *int32 - wantStatus string + name string + poolerObjs map[PostgresRole]*ConnectionPoolerObjects + wantStored map[string]int32 }{ { - name: "Stopping state with nil statefulset - should not transition to Stopped", - statefulsetReplicas: nil, - currentStatus: "Stopping", - newSpecLifecyclePhase: "stopped", - newSpecNumberOfInst: 0, - wantContinue: true, - wantNumberOfInstances: nil, - wantStatus: "Stopping", - }, - { - name: "Stopping with nil Lifecycle spec", - statefulsetReplicas: int32Ptr(0), - currentStatus: "Stopping", - newSpecLifecyclePhase: "", - newSpecNumberOfInst: 0, - wantContinue: true, - wantNumberOfInstances: nil, - wantStatus: "Stopped", + name: "nil ConnectionPooler - no-op", + poolerObjs: nil, + wantStored: nil, }, { - name: "Running to Stopping when numberOfInstances already 0", - statefulsetReplicas: int32Ptr(0), - currentStatus: "Running", - newSpecLifecyclePhase: "stopped", - newSpecNumberOfInst: 0, - wantContinue: true, - wantNumberOfInstances: int32Ptr(0), - wantStatus: "Stopping", + name: "Master at 2 replicas", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{Master: newTestPoolerObjects(Master, 2)}, + wantStored: map[string]int32{"master": 2}, }, { - name: "Stopped with previousNumberOfInstances=0 - sets Updating but warns", - statefulsetReplicas: int32Ptr(0), - currentStatus: "Stopped", - newSpecLifecyclePhase: "", - newSpecNumberOfInst: 0, - previousNumberOfInst: 0, - wantContinue: true, - wantNumberOfInstances: int32Ptr(0), - wantStatus: "Updating", + name: "Master already at 0 replicas", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{Master: newTestPoolerObjects(Master, 0)}, + wantStored: map[string]int32{"master": 0}, }, { - name: "Stopped with nil Lifecycle - wake-up", - statefulsetReplicas: int32Ptr(0), - currentStatus: "Stopped", - newSpecLifecyclePhase: "", - newSpecNumberOfInst: 0, - previousNumberOfInst: 3, - wantContinue: true, - wantNumberOfInstances: int32Ptr(3), - wantStatus: "Updating", + name: "Both Master and Replica", + poolerObjs: map[PostgresRole]*ConnectionPoolerObjects{ + Master: newTestPoolerObjects(Master, 2), + Replica: newTestPoolerObjects(Replica, 1), + }, + wantStored: map[string]int32{"master": 2, "replica": 1}, }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientSet := fake.NewSimpleClientset() + kubeClient := &k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), + } + + c := &Cluster{ + KubeClient: *kubeClient, + ConnectionPooler: tt.poolerObjs, + logger: lifecycleLogger, + Postgresql: acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + } + + newSpec := &acidv1.Postgresql{} + err := c.scalePoolerDown(newSpec) + + assert.NoError(t, err) + assert.Equal(t, tt.wantStored, newSpec.Status.PreviousPoolerInstances) + }) + } +} + +func TestScalePoolerUp(t *testing.T) { + tests := []struct { + name string + prevInst map[string]int32 + wantErr bool + }{ { - name: "Stopped with empty Lifecycle phase - wake-up", - statefulsetReplicas: int32Ptr(0), - currentStatus: "Stopped", - newSpecLifecyclePhase: "", - newSpecNumberOfInst: 0, - previousNumberOfInst: 2, - wantContinue: true, - wantNumberOfInstances: int32Ptr(2), - wantStatus: "Updating", + name: "nil PreviousPoolerInstances - no-op", + prevInst: nil, + wantErr: false, }, { - name: "Stopped but lifecycle still 'stopped' - skip sync", - statefulsetReplicas: int32Ptr(0), - currentStatus: "Stopped", - newSpecLifecyclePhase: "stopped", - newSpecNumberOfInst: 0, - previousNumberOfInst: 3, - wantContinue: false, - wantNumberOfInstances: nil, - wantStatus: "Stopped", + name: "Restore master to 2", + prevInst: map[string]int32{"master": 2}, + wantErr: false, }, { - name: "Running without lifecycle change - normal update", - statefulsetReplicas: int32Ptr(3), - currentStatus: "Running", - newSpecLifecyclePhase: "", - newSpecNumberOfInst: 3, - previousNumberOfInst: 0, - wantContinue: true, - wantNumberOfInstances: int32Ptr(3), - wantStatus: "Running", + name: "Restore both roles", + prevInst: map[string]int32{"master": 2, "replica": 1}, + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() - - if tt.statefulsetReplicas != nil { - _, err := clientSet.AppsV1().StatefulSets("default").Create( - context.TODO(), - &appsv1.StatefulSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: appsv1.StatefulSetSpec{ - Replicas: tt.statefulsetReplicas, - }, - }, - metav1.CreateOptions{}, - ) - assert.NoError(t, err) + clientSet := fake.NewSimpleClientset() + kubeClient := &k8sutil.KubernetesClient{ + DeploymentsGetter: clientSet.AppsV1(), } c := &Cluster{ - logger: lifecycleLogger, KubeClient: *kubeClient, + logger: lifecycleLogger, Postgresql: acidv1.Postgresql{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -1149,150 +694,22 @@ func TestManageHibernateState_EdgeCases(t *testing.T) { }, } - if tt.statefulsetReplicas != nil { - c.Statefulset = &appsv1.StatefulSet{ - Spec: appsv1.StatefulSetSpec{ - Replicas: tt.statefulsetReplicas, - }, - } - } - - oldSpec := acidv1.Postgresql{ - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: tt.currentStatus, - }, - } - newSpec := &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{ - NumberOfInstances: tt.newSpecNumberOfInst, - }, Status: acidv1.PostgresStatus{ - PostgresClusterStatus: tt.currentStatus, - PreviousNumberOfInstances: tt.previousNumberOfInst, + PreviousPoolerInstances: tt.prevInst, }, } + err := c.scalePoolerUp(newSpec) - if tt.newSpecLifecyclePhase != "" || tt.newSpecLifecyclePhase == "" && tt.currentStatus == "Stopped" { - newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{ - Phase: tt.newSpecLifecyclePhase, - } - } - - _, gotContinue := c.manageHibernateState(oldSpec, newSpec) - - assert.Equal(t, tt.wantContinue, gotContinue, "continue sync mismatch") - if tt.wantNumberOfInstances != nil { - assert.Equal(t, *tt.wantNumberOfInstances, newSpec.Spec.NumberOfInstances, "numberOfInstances mismatch") + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } - assert.Equal(t, tt.wantStatus, newSpec.Status.PostgresClusterStatus, "status mismatch") }) } } -func TestManageHibernateState_StateTransitionSequence(t *testing.T) { - t.Run("Running -> Stopping -> Stopped sequence", func(t *testing.T) { - kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() - _, err := clientSet.AppsV1().StatefulSets("default").Create( - context.TODO(), - &appsv1.StatefulSet{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: appsv1.StatefulSetSpec{Replicas: int32Ptr(3)}, - }, - metav1.CreateOptions{}, - ) - assert.NoError(t, err) - - c := &Cluster{ - logger: lifecycleLogger, - KubeClient: *kubeClient, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - }, - } - - oldSpec := acidv1.Postgresql{ - Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, - } - newSpec := &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{ - NumberOfInstances: 3, - Lifecycle: &acidv1.LifecycleSpec{Phase: "stopped"}, - }, - Status: acidv1.PostgresStatus{PostgresClusterStatus: "Running"}, - } - - _, continueSync := c.manageHibernateState(oldSpec, newSpec) - assert.True(t, continueSync) - assert.Equal(t, int32(0), newSpec.Spec.NumberOfInstances) - assert.Equal(t, int32(3), newSpec.Status.PreviousNumberOfInstances) - assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) - - // Simulate STS still terminating (replicas=2) - sset, err := clientSet.AppsV1().StatefulSets("default").Get(context.TODO(), "test", metav1.GetOptions{}) - assert.NoError(t, err) - sset.Spec.Replicas = int32Ptr(2) - _, err = clientSet.AppsV1().StatefulSets("default").Update(context.TODO(), sset, metav1.UpdateOptions{}) - assert.NoError(t, err) - - oldSpec = acidv1.Postgresql{ - Status: acidv1.PostgresStatus{PostgresClusterStatus: "Stopping"}, - } - newSpec.Status.PostgresClusterStatus = "Stopping" - - _, continueSync = c.manageHibernateState(oldSpec, newSpec) - assert.True(t, continueSync) - assert.Equal(t, "Stopping", newSpec.Status.PostgresClusterStatus) - - // Simulate STS scaled to 0 - sset, err = clientSet.AppsV1().StatefulSets("default").Get(context.TODO(), "test", metav1.GetOptions{}) - assert.NoError(t, err) - sset.Spec.Replicas = int32Ptr(0) - _, err = clientSet.AppsV1().StatefulSets("default").Update(context.TODO(), sset, metav1.UpdateOptions{}) - assert.NoError(t, err) - - _, continueSync = c.manageHibernateState(oldSpec, newSpec) - assert.True(t, continueSync) - assert.Equal(t, "Stopped", newSpec.Status.PostgresClusterStatus) - }) - - t.Run("Stopped -> Updating -> Running sequence", func(t *testing.T) { - c := &Cluster{ - logger: lifecycleLogger, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - }, - } - - oldSpec := acidv1.Postgresql{ - Status: acidv1.PostgresStatus{PostgresClusterStatus: "Stopped"}, - } - newSpec := &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{ - NumberOfInstances: 0, - }, - Status: acidv1.PostgresStatus{ - PostgresClusterStatus: "Stopped", - PreviousNumberOfInstances: 3, - }, - } - - _, continueSync := c.manageHibernateState(oldSpec, newSpec) - assert.True(t, continueSync) - assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) - assert.Equal(t, "Updating", newSpec.Status.PostgresClusterStatus) - - oldSpec = acidv1.Postgresql{ - Status: acidv1.PostgresStatus{PostgresClusterStatus: "Updating"}, - } - newSpec.Status.PostgresClusterStatus = "Updating" - - _, continueSync = c.manageHibernateState(oldSpec, newSpec) - assert.True(t, continueSync) - assert.Equal(t, int32(3), newSpec.Spec.NumberOfInstances) - }) -} - func TestSuspendLogicalBackupJob(t *testing.T) { tests := []struct { name string @@ -1324,7 +741,7 @@ func TestSuspendLogicalBackupJob(t *testing.T) { jobName := "logical-backup-test-cluster" if tt.jobExists { - clientSet.BatchV1().CronJobs("default").Create(context.TODO(), &batchv1.CronJob{ + _, _ = clientSet.BatchV1().CronJobs("default").Create(context.TODO(), &batchv1.CronJob{ ObjectMeta: metav1.ObjectMeta{ Name: jobName, Namespace: "default", @@ -1419,7 +836,7 @@ func TestUnsuspendLogicalBackupJob(t *testing.T) { if tt.jobExists { suspendTrue := true - clientSet.BatchV1().CronJobs("default").Create(context.TODO(), &batchv1.CronJob{ + _, _ = clientSet.BatchV1().CronJobs("default").Create(context.TODO(), &batchv1.CronJob{ ObjectMeta: metav1.ObjectMeta{ Name: jobName, Namespace: "default", @@ -1483,150 +900,109 @@ func TestUnsuspendLogicalBackupJob(t *testing.T) { } } -func TestRefreshStatefulset(t *testing.T) { - const stsName = "test-cluster" - - makeSTS := func(replicas *int32) *appsv1.StatefulSet { - return &appsv1.StatefulSet{ - ObjectMeta: metav1.ObjectMeta{Name: stsName, Namespace: "default"}, - Spec: appsv1.StatefulSetSpec{Replicas: replicas}, - } - } - - makeCluster := func(t *testing.T) (*Cluster, *fake.Clientset) { - t.Helper() - kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() - return &Cluster{ - logger: lifecycleLogger, - KubeClient: *kubeClient, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: stsName, Namespace: "default"}, - }, - }, clientSet +// blockLifecycleUpdate lives in cluster.go but is exercised here because the +// lifecycle subsystem owns its semantics (Stopped/Stopping states). +func TestBlockLifecycleUpdate(t *testing.T) { + tests := []struct { + name string + currentStatus string + lifecyclePhase string + wantBlocked bool + wantErr bool + errContains string + }{ + { + name: "Running cluster, allows update", + currentStatus: acidv1.ClusterStatusRunning, + wantBlocked: false, + wantErr: false, + }, + { + name: "Stopping state, blocks update", + currentStatus: acidv1.ClusterStatusStopping, + wantBlocked: true, + wantErr: true, + errContains: "cannot update cluster while it is stopping", + }, + { + name: "Stopped with lifecycle.phase=stopped, blocks update", + currentStatus: acidv1.ClusterStatusStopped, + lifecyclePhase: "stopped", + wantBlocked: true, + wantErr: true, + errContains: "cannot update cluster while stopped", + }, + { + name: "Stopped without lifecycle.phase, allows update (wake-up)", + currentStatus: acidv1.ClusterStatusStopped, + lifecyclePhase: "", + wantBlocked: false, + wantErr: false, + }, + { + name: "UpdateFailed state, allows update", + currentStatus: acidv1.ClusterStatusUpdateFailed, + wantBlocked: false, + wantErr: false, + }, } - t.Run("success: populates cache from API when c.Statefulset is nil", func(t *testing.T) { - c, clientSet := makeCluster(t) - _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(0)), metav1.CreateOptions{}) - assert.NoError(t, err) - assert.Nil(t, c.Statefulset) - - assert.NoError(t, c.refreshStatefulset()) - assert.NotNil(t, c.Statefulset) - assert.Equal(t, stsName, c.Statefulset.Name) - assert.Equal(t, int32(0), *c.Statefulset.Spec.Replicas) - }) - - t.Run("success: overwrites stale cache with fresh API value", func(t *testing.T) { - c, clientSet := makeCluster(t) - _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(0)), metav1.CreateOptions{}) - assert.NoError(t, err) - // Stale cache says replicas=3 - c.Statefulset = makeSTS(int32Ptr(3)) - - assert.NoError(t, c.refreshStatefulset()) - assert.Equal(t, int32(0), *c.Statefulset.Spec.Replicas, "should reflect API value, not cached value") - }) - - t.Run("not found: clears cache and returns nil", func(t *testing.T) { - c, _ := makeCluster(t) - c.Statefulset = makeSTS(int32Ptr(2)) // stale cache - assert.NotNil(t, c.Statefulset) - - assert.NoError(t, c.refreshStatefulset()) - assert.Nil(t, c.Statefulset, "cache should be cleared when STS is missing") - }) - - t.Run("not found from nil: leaves cache nil, returns nil", func(t *testing.T) { - c, _ := makeCluster(t) - assert.Nil(t, c.Statefulset) - - assert.NoError(t, c.refreshStatefulset()) - assert.Nil(t, c.Statefulset) - }) -} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, tt.currentStatus, 3, tt.lifecyclePhase, 0, nil) -func TestCheckStoppingCompleted(t *testing.T) { - const stsName = "test-cluster" + newSpec := c.Postgresql.DeepCopy() + if tt.lifecyclePhase != "" && tt.currentStatus != acidv1.ClusterStatusStopped { + newSpec.Spec.Lifecycle = &acidv1.LifecycleSpec{Phase: tt.lifecyclePhase} + } - makeSTS := func(replicas *int32) *appsv1.StatefulSet { - return &appsv1.StatefulSet{ - ObjectMeta: metav1.ObjectMeta{Name: stsName, Namespace: "default"}, - Spec: appsv1.StatefulSetSpec{Replicas: replicas}, - } - } + blocked, err := c.blockLifecycleUpdate(newSpec) - makeCluster := func(t *testing.T) (*Cluster, *fake.Clientset) { - t.Helper() - kubeClient, clientSet, _ := newFakeK8sClientForLifecycle() - return &Cluster{ - logger: lifecycleLogger, - KubeClient: *kubeClient, - Postgresql: acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{Name: stsName, Namespace: "default"}, - }, - }, clientSet + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.wantBlocked, blocked) + }) } +} - stoppingStatus := acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusStopping} - runningStatus := acidv1.PostgresStatus{PostgresClusterStatus: acidv1.ClusterStatusRunning} +func TestLifecycleUpdateBlocksDuringStopping(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusStopping, 0, "stopped", 3, nil) - t.Run("fast path: status not Stopping returns false without touching cache", func(t *testing.T) { - c, _ := makeCluster(t) - // Pre-populate cache to prove it's not refreshed - c.Statefulset = makeSTS(int32Ptr(0)) + newSpec := c.Postgresql.DeepCopy() + blocked, err := c.blockLifecycleUpdate(newSpec) - done, err := c.checkStoppingCompleted(&runningStatus) - assert.NoError(t, err) - assert.False(t, done) - assert.NotNil(t, c.Statefulset, "fast path should not refresh the cache") - }) + assert.True(t, blocked) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot update cluster while it is stopping") +} - t.Run("Stopping with API replicas=0 returns true (operator-restart scenario)", func(t *testing.T) { - c, clientSet := makeCluster(t) - // Operator just restarted: c.Statefulset is nil. - // API has the truth: replicas=0. - _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(0)), metav1.CreateOptions{}) - assert.NoError(t, err) - assert.Nil(t, c.Statefulset) - - done, err := c.checkStoppingCompleted(&stoppingStatus) - assert.NoError(t, err) - assert.True(t, done, "should transition to Stopped based on fresh API value") - assert.NotNil(t, c.Statefulset, "cache should be populated after refresh") - }) +func TestLifecycleUpdateBlocksWhenStoppedWithPhase(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusStopped, 0, "stopped", 3, nil) - t.Run("Stopping with API replicas=2 returns false", func(t *testing.T) { - c, clientSet := makeCluster(t) - _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(2)), metav1.CreateOptions{}) - assert.NoError(t, err) + newSpec := c.Postgresql.DeepCopy() + blocked, err := c.blockLifecycleUpdate(newSpec) - done, err := c.checkStoppingCompleted(&stoppingStatus) - assert.NoError(t, err) - assert.False(t, done) - }) + assert.True(t, blocked) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot update cluster while stopped") +} - t.Run("Stopping with stale cache (replicas=2) but API says 0 returns true", func(t *testing.T) { - c, clientSet := makeCluster(t) - _, err := clientSet.AppsV1().StatefulSets("default").Create(context.TODO(), makeSTS(int32Ptr(0)), metav1.CreateOptions{}) - assert.NoError(t, err) - // Stale cache says replicas=2 (e.g. another operator pod restarted and - // cached the pre-hibernate state). - c.Statefulset = makeSTS(int32Ptr(2)) - - done, err := c.checkStoppingCompleted(&stoppingStatus) - assert.NoError(t, err) - assert.True(t, done, "should use fresh API value, not stale cache") - assert.Equal(t, int32(0), *c.Statefulset.Spec.Replicas) - }) +func TestLifecycleUpdateAllowsWakeUp(t *testing.T) { + client, _, _ := newFakeK8sClientForLifecycle() + c := newLifecycleCluster(client, acidv1.ClusterStatusStopped, 0, "", 3, nil) - t.Run("Stopping with STS missing in API returns false", func(t *testing.T) { - c, _ := makeCluster(t) - c.Statefulset = makeSTS(int32Ptr(2)) + newSpec := c.Postgresql.DeepCopy() + blocked, err := c.blockLifecycleUpdate(newSpec) - done, err := c.checkStoppingCompleted(&stoppingStatus) - assert.NoError(t, err) - assert.False(t, done) - assert.Nil(t, c.Statefulset, "cache should be cleared when STS is missing") - }) -} \ No newline at end of file + assert.False(t, blocked) + assert.NoError(t, err) +} diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 8a0b68eaa..946906a00 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -36,10 +36,26 @@ var requirePrimaryRestartWhenDecreased = []string{ // Sync syncs the cluster, making sure the actual Kubernetes objects correspond to what is defined in the manifest. // Unlike the update, sync does not error out if some objects do not exist and takes care of creating them. func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { - var err error c.mu.Lock() defer c.mu.Unlock() + // operator-restart recovery for an in-flight hibernate. If the persisted + // status is Stopping but pods have not yet been confirmed gone, complete the + // transition inline. Lives at the Sync level so the existing defer's SyncFailed + // branch doesn't fire on the lifecycle wait timeout. + if c.Status.Stopping() { + return c.completeStoppingTransition(newSpec) + } + + return c.syncStateLocked(newSpec) +} + +// syncStateLocked performs reconciliation. The caller must hold c.mu. Detects +// lifecycle transitions in-memory and persists spec mutations before running +// the sync body, so a single call covers both lifecycle transitions (called +// from handleHibernateAndWakeUp) and periodic reconciliation. +func (c *Cluster) syncStateLocked(newSpec *acidv1.Postgresql) error { + var err error oldSpec := c.Postgresql c.setSpec(newSpec) @@ -47,7 +63,13 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { if err != nil { c.logger.Warningf("error while syncing cluster state: %v", err) newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusSyncFailed - } else if !c.Status.Running() && !c.Status.Stopping() && !c.Status.Stopped() { + } else if !c.Status.Running() && !c.Status.Stopping() && !c.Status.Stopped() && + oldSpec.Spec.NumberOfInstances > 0 { + // Wake-up Updates carry oldSpec.Spec.NumberOfInstances == 0 because + // initiateWakeUp hasn't mutated newSpec yet when oldSpec was captured + // Skip the Updating->Running transition here: the cluster + // isn't ready (pods are still starting), the write would 409 on stale + // rv, and the next periodic Sync writes Running with fresh rv. newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusRunning } @@ -65,25 +87,20 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { c.logger.Debugf("could not sync finalizers: %v", err) } - // Handle lifecycle hibernate/wake-up state transitions - lifecycleAction, continueSync := c.manageHibernateState(oldSpec, newSpec) - if !continueSync { + // Lifecycle handling lives in its own helper; syncStateLocked only does + // reconciliation work from here on. + proceed, perr := c.prepareLifecycleTransition(&newSpec, oldSpec) + if perr != nil { + return perr + } + if !proceed { return nil } - // Hibernate and WakeUp transitions modify newSpec (e.g. numberOfInstances). - // Persist them here so the cluster actually scales. Without this, a Sync - // that runs before any Update (e.g. operator restart catching up via the - // informer cache) would leave the spec unchanged in the API and the cluster - // would stay stuck in Stopping/Updating because no scaling occurs. - if lifecycleAction == LifecycleActionHibernate || lifecycleAction == LifecycleActionWakeUp { - pgUpdated, err := c.KubeClient.UpdatePostgresCR(c.clusterName(), newSpec) - if err != nil { - c.logger.Errorf("could not update spec after lifecycle transition in sync: %v", err) - return err - } - c.setSpec(pgUpdated) - newSpec = pgUpdated + // If prepareLifecycleTransition already persisted Stopping/Updating, resync + // oldSpec so the defer below doesn't redundantly re-write the same status (409). + if newSpec.Status.PostgresClusterStatus != oldSpec.Status.PostgresClusterStatus { + oldSpec.Status = *newSpec.Status.DeepCopy() } if err = c.initUsers(); err != nil { diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index 81e927d94..22fac2759 100644 --- a/pkg/cluster/util.go +++ b/pkg/cluster/util.go @@ -469,6 +469,29 @@ func (c *Cluster) waitStatefulsetPodsReady() error { return nil } +// waitStatefulsetPodsGone blocks until the StatefulSet reports zero replicas in +// its Status (i.e. all pods have been terminated) or the wait times out. Each +// iteration refreshes c.Statefulset so the in-memory cache stays current. +// NotFound is treated as success (the cluster may have been deleted entirely). +// On timeout, returns an error so callers can surface it to the controller. +func (c *Cluster) waitStatefulsetPodsGone() error { + c.setProcessName("waiting for statefulset pods to terminate") + return retryutil.Retry(c.OpConfig.ResourceCheckInterval, c.OpConfig.ResourceCheckTimeout, + func() (bool, error) { + sts, err := c.KubeClient.StatefulSets(c.Namespace).Get( + context.TODO(), c.statefulSetName(), metav1.GetOptions{}) + if err != nil { + if k8sutil.ResourceNotFound(err) { + c.Statefulset = nil + return true, nil + } + return false, fmt.Errorf("could not get statefulset: %w", err) + } + c.Statefulset = sts + return sts.Status.Replicas == 0, nil + }) +} + // Returns labels used to create or list k8s objects such as pods // For backward compatibility, shouldAddExtraLabels must be false // when listing k8s objects. See operator PR #252 From 0af47e5ceebf5e031a148c6d7bfa61ae7f030f0b Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Thu, 16 Jul 2026 13:01:35 +0530 Subject: [PATCH 12/13] SCF-830: Updated the user.md and cluster_manifest.md --- README.md | 1 + docs/reference/cluster_manifest.md | 2 ++ 2 files changed, 3 insertions(+) diff --git a/README.md b/README.md index 9e5bc886b..d80739716 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ pipelines with no access to Kubernetes API directly, promoting infrastructure as * Restore and cloning Postgres clusters on AWS, GCS and Azure * Additionally logical backups to S3 or GCS bucket can be configured * Standby cluster from S3 or GCS WAL archive +* Hibernate and wake-up of Postgres clusters (scale StatefulSet and pooler to 0 while preserving data, then restore) * Configurable for non-cloud environments * Basic credential and user management on K8s, eases application deployments * Support for custom TLS certificates diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md index 634320d82..9274123c2 100644 --- a/docs/reference/cluster_manifest.md +++ b/docs/reference/cluster_manifest.md @@ -496,11 +496,13 @@ Parameters to control cluster hibernate/wake-up behavior. * Store the current number of instances in the status * Scale down the StatefulSet to 0 replicas * Scale down the connection pooler to 0 replicas + * Suspend the logical backup CronJob (if enabled) * Set the cluster status to "Stopping", then "Stopped" When this field is removed from a stopped cluster, the operator will: * Restore the number of instances from the stored value * Scale up the StatefulSet and connection pooler + * Resume the logical backup CronJob (if it was suspended) * Set the cluster status to "Updating", then "Running" This field is optional. When not set, the cluster operates normally. From 858b873e8505e65471e73cd226fb95fadae13acc Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Thu, 16 Jul 2026 21:24:10 +0530 Subject: [PATCH 13/13] SCF-830: Ran go fmt for lifecycle and cluster changes --- pkg/cluster/cluster_test.go | 2 -- pkg/cluster/lifecycle.go | 2 +- pkg/cluster/lifecycle_test.go | 30 +++++++++++++++--------------- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index a3d88f7b1..e38540d3e 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -2354,5 +2354,3 @@ func TestUpdatePITRResources(t *testing.T) { }) } } - - diff --git a/pkg/cluster/lifecycle.go b/pkg/cluster/lifecycle.go index 8ee2c4086..8f4d0915a 100644 --- a/pkg/cluster/lifecycle.go +++ b/pkg/cluster/lifecycle.go @@ -103,7 +103,7 @@ func (c *Cluster) handleHibernateAndWakeUp(newSpec *acidv1.Postgresql) (bool, er // prepareLifecycleTransition inspects newSpec for a lifecycle transition // On detection it mutates the in-memory spec via initiateHibernate/initiateWakeUp -// and immediately persists BOTH spec and status to the K8s API so the new status +// and immediately persists BOTH spec and status to the K8s API so the new status // (Stopping/Updating) is visible in same reconciliation pass — without waiting for // syncStateLocked's defer. // Uses oldSpec for detection so it works correctly when handleHibernateAndWakeUp diff --git a/pkg/cluster/lifecycle_test.go b/pkg/cluster/lifecycle_test.go index 11a9730b1..9e074f804 100644 --- a/pkg/cluster/lifecycle_test.go +++ b/pkg/cluster/lifecycle_test.go @@ -48,15 +48,15 @@ func newFakeK8sClientForLifecycle() (*k8sutil.KubernetesClient, *fake.Clientset, acidClientSet := fakeacidv1.NewSimpleClientset() client := &k8sutil.KubernetesClient{ - DeploymentsGetter: clientSet.AppsV1(), - PostgresqlsGetter: acidClientSet.AcidV1(), - StatefulSetsGetter: clientSet.AppsV1(), - ServicesGetter: clientSet.CoreV1(), - SecretsGetter: clientSet.CoreV1(), - ConfigMapsGetter: clientSet.CoreV1(), - PodsGetter: clientSet.CoreV1(), - EndpointsGetter: clientSet.CoreV1(), - CronJobsGetter: clientSet.BatchV1(), + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), + ConfigMapsGetter: clientSet.CoreV1(), + PodsGetter: clientSet.CoreV1(), + EndpointsGetter: clientSet.CoreV1(), + CronJobsGetter: clientSet.BatchV1(), } return client, clientSet, acidClientSet @@ -105,7 +105,7 @@ func newLifecycleCluster( return &Cluster{ Config: Config{ OpConfig: config.Config{ - PodManagementPolicy: "ordered_ready", + PodManagementPolicy: "ordered_ready", LogicalBackup: config.LogicalBackup{ LogicalBackupJobPrefix: "logical-backup-", }, @@ -253,9 +253,9 @@ func TestInitiateWakeUp(t *testing.T) { NumberOfInstances: 0, }, Status: acidv1.PostgresStatus{ - PostgresClusterStatus: acidv1.ClusterStatusStopped, - PreviousNumberOfInstances: 3, - PreviousPoolerInstances: map[string]int32{"master": 2, "replica": 0}, + PostgresClusterStatus: acidv1.ClusterStatusStopped, + PreviousNumberOfInstances: 3, + PreviousPoolerInstances: map[string]int32{"master": 2, "replica": 0}, }, } @@ -277,8 +277,8 @@ func TestInitiateWakeUp(t *testing.T) { NumberOfInstances: 0, }, Status: acidv1.PostgresStatus{ - PostgresClusterStatus: acidv1.ClusterStatusStopped, - PreviousPoolerInstances: map[string]int32{"master": 1}, + PostgresClusterStatus: acidv1.ClusterStatusStopped, + PreviousPoolerInstances: map[string]int32{"master": 1}, }, }