SCF-830: Added state management for cluster hibernation by adshin21 · Pull Request #17 · cybertec-postgresql/postgres-operator

adshin21 · 2026-05-13T09:32:20Z

No description provided.

coveralls · 2026-05-13T09:36:56Z

Coverage Report for CI Build 29651611343

Warning

No base build found for commit d16504e on scalefield_v18.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 44.986%

Details

Patch coverage: 108 uncovered changes across 7 files (272 of 380 lines covered, 71.58%).

Uncovered Changes

File	Changed	Covered	%
pkg/cluster/lifecycle.go	242	187	77.27%
pkg/cluster/util.go	14	0	0.0%
pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go	23	12	52.17%
pkg/cluster/cluster.go	57	47	82.46%
pkg/apis/acid.zalan.do/v1/util.go	9	3	33.33%
pkg/cluster/sync.go	29	23	79.31%
pkg/util/k8sutil/k8sutil.go	6	0	0.0%

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats


Relevant Lines:	15747
Covered Lines:	7084
Line Coverage:	44.99%
Coverage Strength:	16.37 hits per line

💛 - Coveralls

…er too

… in update

serdardalgic

Some minor issues @adshin21. After that, I would like to review it again.

serdardalgic · 2026-06-22T10:23:55Z

+	if c.Statefulset == nil || c.Statefulset.Spec.Replicas == nil {
+		return nil
+	}
+	return c.Statefulset.Spec.Replicas


Suggested change

return c.Statefulset.Spec.Replicas

return c.Statefulset.Status.Replicas

Shouldn't this be Status as you want to get the current replica count of the StatefulSet?

Yes, but to get the current status, we need to make an API call to k8s.

serdardalgic · 2026-06-22T11:02:06Z

+const (
+	LifecycleActionNone LifecycleAction = iota
+	LifecycleActionHibernate          // Running -> Stopping (initiate hibernate)
+	LifecycleActionStoppingCompleted // Stopping -> Stopped (pods fully terminated)


Are you using LifecycleActionStoppingCompleted ? If not, please remove it.

…sist lifecycle spec changes in Sync

serdardalgic

So, some stuff that caught my attention, here it is.

serdardalgic · 2026-07-16T15:17:35Z

+	isWakingUpSimple := newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped"
+	hasPreviousInstances := newSpecPreviousNumberOfInstances > 0
+	needsRestore := newSpecNumberOfInstances == 0
+
+	isWakingUp := isWakingUpSimple && hasPreviousInstances && needsRestore
+
+	if currentStatus.Stopped() || isWakingUp {
+		if isWakingUp || newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped" {
+			return LifecycleActionWakeUp
+		}
+	}
+
+	if newSpecLifecycle != nil &&
+		newSpecLifecycle.Phase == "stopped" &&
+		!currentStatus.Stopping() &&
+		!currentStatus.Stopped() {


I'd write this part a bit simpler.
isWakingUp already requires isWakingUpSimple to be true (it's isWakingUpSimple && hasPreviousInstances && needsRestore), so checking isWakingUp || isWakingUpSimple in the inner if is redundant, it's the same as just checking isWakingUpSimple. That makes the nested if hard to follow: you have to trace three variables back to see that one term never actually changes the outcome. Splitting it into two flat, named checks makes the same logic easier to read:

Suggested change

isWakingUpSimple := newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped"

hasPreviousInstances := newSpecPreviousNumberOfInstances > 0

needsRestore := newSpecNumberOfInstances == 0

isWakingUp := isWakingUpSimple && hasPreviousInstances && needsRestore

if currentStatus.Stopped() || isWakingUp {

if isWakingUp || newSpecLifecycle == nil || newSpecLifecycle.Phase != "stopped" {

return LifecycleActionWakeUp

}

}

if newSpecLifecycle != nil &&

newSpecLifecycle.Phase == "stopped" &&

!currentStatus.Stopping() &&

!currentStatus.Stopped() {

wantsStopped := newSpecLifecycle != nil && newSpecLifecycle.Phase == "stopped"

// The cluster was in the middle of hibernating (already scaled down,

// old replica count saved) when the spec changed its mind and no

// longer wants it stopped.

if !wantsStopped && newSpecPreviousNumberOfInstances > 0 && newSpecNumberOfInstances == 0 {

return LifecycleActionWakeUp

}

// Already stopped and the spec no longer asks to stay stopped.

if currentStatus.Stopped() && !wantsStopped {

return LifecycleActionWakeUp

}

if wantsStopped && !currentStatus.Stopping() && !currentStatus.Stopped() {

return LifecycleActionHibernate

}

I checked this produces the same result as the original in every case, and all 9 existing detectLifecycleTransition unit tests still pass with no changes needed.

serdardalgic · 2026-07-16T15:20:45Z

+  * 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:


One small suggestion, just to be more precise here: this doesn't just trigger on removing the field, setting phase to anything other than "stopped" (including "") also wakes the cluster up:

Suggested change

When this field is removed from a stopped cluster, the operator will:

When this field is removed, or set to any value other than `"stopped"`

(including an empty string), on a stopped cluster, the operator will:

serdardalgic · 2026-07-16T15:51:18Z


+// LifecycleSpec describes the lifecycle state of a Postgres cluster.
+type LifecycleSpec struct {
+	Phase string `json:"phase,omitempty"`


Phase is a bare string and the only value the code ever checks for is "stopped", everything else means "not stopped." Since there's no plan for other phase values at least for now, I'd suggest making that explicit rather than leaving it open-ended:

A typed constant, e.g. type LifecyclePhase string + LifecyclePhaseStopped LifecyclePhase = "stopped" added to pkg/apis/acid.zalan.do/v1/const.go, used in lifecycle.go:39 and cluster.go:1288 instead of the literal "stopped".

A CRD enum: ["", "stopped"] on spec.lifecycle.phase (manifests/postgresql.crd.yaml) so a typo like "Stopped" is rejected at admission instead of silently no-op'ing.

Since this PR is the one introducing spec.lifecycle.phase in the first place, there's no existing-cluster migration risk, worth just adding the enum: now rather than as a follow-up.

serdardalgic · 2026-07-16T15:52:50Z

+
+

Suggested change

This is also a mistake, I guess.

Replace the four-line flag-juggling (isWakingUpSimple / hasPreviousInstances / needsRestore) with a wantsStopped flag plus three self-documenting guards. This removes the latent bug where the inner isWakingUp check was being dropped (Stopped + phase=stopped incorrectly returned WakeUp).

…enum Introduce LifecyclePhase string type with LifecyclePhaseStopped constant and Phase.Stopped() helper; the CRD enum accepts "" or "stopped" via +kubebuilder:validation:Enum="";stopped. Tighten blockLifecycleUpdate error message and cluster_manifest.md wake-up text to mention setting lifecycle.phase to an empty string. Generate DeepCopy for PostgresStatus.PreviousPoolerInstances and use it in Postgresql.DeepCopyInto so the operator's in-memory cache does not share map state with the cluster object.

Tighten the second wake-up guard in detectLifecycleTransition to require PreviousNumberOfInstances > 0. Without a stored replica count, transitioning to Updating with 0 replicas leaves the cluster stuck indefinitely: the sync defer skips Running when oldSpec had 0 instances, and the next periodic Sync sees the same dead-end state. The "Stopped + cleared + no previous instances" test case is renamed and updated to expect LifecycleActionNone, since the previous behavior was a recovery attempt on a corrupted state that produced a worse outcome than the cluster staying in Stopped. Update initiateWakeUp's doc comment to note that the PreviousNumberOfInstances == 0 branch is now unreachable in practice (detect refuses the transition upstream).

* Rewrite the misleading comment in syncStateLocked's defer. The old wording claimed oldSpec.Spec.NumberOfInstances == 0 during wake-up because initiateWakeUp hadn't run yet; the real reason is that c.Postgresql was already hibernated when the reconcile started. * Skip 0-replica entries in scalePoolerDown's stored map. A pooler already at 0 has nothing to restore on wake-up; storing it produced a spurious no-op patch on the way up. Update the corresponding test case to expect nil instead of {"master": 0}. * Document the PreviousNumberOfInstances == 0 limitation in cluster_manifest.md: the operator will not transition out of Stopped if the field is missing (e.g. due to a partial write or a manual edit); the user must edit spec.numberOfInstances to wake the cluster up manually.

…dates Fix bug encountered in e2e tests: user annotations (e.g. last-major-upgrade-failure) were silently dropped when a watch event arrived mid-lifecycle, causing the v16->v18 major version upgrade to proceed despite the failure annotation.

SCF-830: Added state management for cluster hibernation

19ca281

adshin21 added 5 commits May 13, 2026 15:20

SCF-830: Steamlined cluster update function

8d75f56

SCF-830: Handled pooler hibernation while stopping/starting the clust…

599a263

…er too

SCF-830: Added feature documentation for cluster hibernation

ea8438f

SCF-830: Added unit test from hibernate

9424f7c

SCF-830: Added cronjob pause/resume while cluster hibernation

0f4eabe

adshin21 requested review from ermajn, hlihhovac and serdardalgic May 21, 2026 07:26

adshin21 marked this pull request as ready for review May 21, 2026 07:26

adshin21 requested a review from hughcapet as a code owner May 21, 2026 07:26

SCF-830: Refactored into a common function for spec update, pesisting…

8c8147a

… in update

serdardalgic requested a review from ants May 22, 2026 09:20

Retriggering pipeline

18a42b8

serdardalgic requested changes Jun 22, 2026

View reviewed changes

adshin21 added 2 commits July 14, 2026 00:40

SCF-830: Refactor detect lifecycle, fix Update 409 follow-up, and per…

8e7018b

…sist lifecycle spec changes in Sync

SCF-830: Added fresh replica status checking while stopping

356aa78

adshin21 requested a review from serdardalgic July 14, 2026 08:15

adshin21 added 3 commits July 16, 2026 01:58

SCF-830: Refactored and made hibernation sync cycle free

293ebfc

SCF-830: Updated the user.md and cluster_manifest.md

0af47e5

SCF-830: Ran go fmt for lifecycle and cluster changes

858b873

serdardalgic requested changes Jul 16, 2026

View reviewed changes

adshin21 added 5 commits July 18, 2026 15:22

adshin21 force-pushed the scalefield_v18-SCF-830-cluster-hibernation branch from 7f48801 to c378463 Compare July 18, 2026 16:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SCF-830: Added state management for cluster hibernation#17

SCF-830: Added state management for cluster hibernation#17
adshin21 wants to merge 18 commits into
scalefield_v18from
scalefield_v18-SCF-830-cluster-hibernation

adshin21 commented May 13, 2026

Uh oh!

coveralls commented May 13, 2026 •

edited

Loading

Uh oh!

serdardalgic left a comment

Uh oh!

serdardalgic Jun 22, 2026

Uh oh!

adshin21 Jul 9, 2026

Uh oh!

Uh oh!

serdardalgic Jun 22, 2026

Uh oh!

serdardalgic left a comment

Uh oh!

Uh oh!

serdardalgic Jul 16, 2026

Uh oh!

serdardalgic Jul 16, 2026

Uh oh!

serdardalgic Jul 16, 2026

Uh oh!

serdardalgic Jul 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	return c.Statefulset.Spec.Replicas
	return c.Statefulset.Status.Replicas

	When this field is removed from a stopped cluster, the operator will:
	When this field is removed, or set to any value other than `"stopped"`
	(including an empty string), on a stopped cluster, the operator will:

Uh oh!

Conversation

adshin21 commented May 13, 2026

Uh oh!

coveralls commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 29651611343

Coverage: 44.986%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

serdardalgic left a comment

Choose a reason for hiding this comment

Uh oh!

serdardalgic Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

adshin21 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serdardalgic Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

serdardalgic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serdardalgic Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

serdardalgic Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

serdardalgic Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

serdardalgic Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented May 13, 2026 •

edited

Loading