K8SPG-915: add leader election options to `operator.yaml` by pooknull · Pull Request #1433 · percona/percona-postgresql-operator

pooknull · 2026-02-03T12:26:36Z

Due to the high volume of requests, we're unable to provide free service for this account. To continue using the service, please upgarde to a paid plan.

https://perconadev.atlassian.net/browse/K8SPG-915

DESCRIPTION

This PR exposes the following operator options through env vars:

PGO_CONTROLLER_LEASE_NAME - determines the name of the resource that leader election will use for holding the leader lock
PGO_CONTROLLER_LEASE_DURATION - duration that non-leader candidates will wait to force acquire leadership. This is measured against time of last observed ack
PGO_CONTROLLER_RENEW_DEADLINE - duration that the acting controlplane will retry refreshing leadership before giving up
PGO_CONTROLLER_RETRY_PERIOD - duration the LeaderElector clients should wait between tries of actions
PGO_CONTROLLER_LEADER_ELECTION_ENABLED - disable/enable leader election

CHECKLIST

Jira

Is the Jira ticket created and referenced properly?
Does the Jira ticket have the proper statuses for documentation (Needs Doc) and QA (Needs QA)?
Does the Jira ticket link to the proper milestone (Fix Version field)?

Tests

Is an E2E test/test case added for the new feature/change?
Are unit tests added where appropriate?

Config/Logging/Testability

Are all needed new/changed options added to default YAML files?
Are all needed new/changed options added to the Helm Chart?
Did we add proper logging messages for operator actions?
Did we ensure compatibility with the previous version or cluster upgrade process?
Does the change support oldest and newest supported PG version?
Does the change support oldest and newest supported Kubernetes version?

https://perconadev.atlassian.net/browse/K8SPG-915

egegunes

@pooknull users should be able to disable leader election completely

mayankshah1607

LGTM, lets address @egegunes comment

Copilot

Pull request overview

This PR adds configuration knobs for controller-runtime leader election timing to the operator’s manifests and wires those env vars into the operator’s manager initialization.

Changes:

Expose leader election env vars (PGO_CONTROLLER_LEASE_*) in deployment manifests (including generated bundle YAMLs).
Parse the duration env vars in initManager and set controller-runtime leader election options (LeaseDuration, RenewDeadline, RetryPeriod).
Extend initManager unit test to assert the new default duration values.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
deploy/operator.yaml	Adds leader election-related env vars to the main deployment manifest.
deploy/cw-operator.yaml	Adds the same env vars to the CW deployment manifest.
deploy/cw-bundle.yaml	Propagates env vars into the CW generated bundle.
deploy/bundle.yaml	Propagates env vars into the generated bundle.
config/manager/default/manager.yaml	Adds env vars to the controller manager default deployment.
cmd/postgres-operator/main.go	Reads duration env vars and sets controller-runtime leader election timing options.
cmd/postgres-operator/main_test.go	Asserts default leader election timing values and keeps zero-value checks working.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mayankshah1607 · 2026-02-12T04:58:03Z

 	}

+	// K8SPG-915
+	getEnvDuration := func(name string, defaultDur time.Duration) (time.Duration, error) {


Can we try to use a library like https://github.com/kelseyhightower/envconfig?tab=readme-ov-file#struct-tag-support ? It will save us a lot of this boilerplate because it supports time.Duration type as well. Example

type LeaderElectionConfig struct { LeaderElectionEnabled bool `default:"true" envconfig:"PGO_LEADER_ELECTION_ENABLED"` LeaderElectionID string `default:"everest-leader-election" envconfig:"PGO_LEADER_ELECTION_ID"` LeaseDuration time.Duration `default:"15s" envconfig:"PGO_CONRTOLLER_LEASE_DURATION"` RenewDeadline time.Duration `default:"10s" envconfig:"PGO_CONRTOLLER_RENEW_DEADLINE"` RetryPeriod time.Duration `default:"2s" envconfig:"PGO_CONRTOLLER_RETRY_PERIOD"` } func main() { leaderElectionConfig := &LeaderElectionConfig{} err := envconfig.Process("", leaderElectionConfig) if err != nil { // handle err } leaderElectionConfig.IntoOptions(options) // parse into controller runtime }

I don't like an extra dependency a small feature, but I like this approach, so I implemented it without the dependency: 518e892

I'd argue that this is a small dependency (~400 LOC) meant for exactly such use-cases - avoiding boilerplate when parsing envvars. I think we should use it rather than re-inventing it ourselves, I don't see the benefit in avoiding it

Having a small dependency is not the worst thing. On the other hand, the boilerplate Andrii needed to add is not huge either ¯_(ツ)_/¯

Only advantage of using the dependency is the ability to pass durations as env vars I guess. @pooknull if you're not fundamentally against using it, let's use this thing

The only "issue" is that the library seems unmaintained. The last release happened in 2019 and there is not answer in the open Github issues or PRs from anyone. We should keep that in mind.

Alternative - https://github.com/caarlos0/env

it's a simple library with no external dependencies. I am not sure if we need to worry about its last release date.

I dont fully agree here. The risk is low, but it’s not zero: newer Go versions can introduce subtle reflect or type-system changes, and if something breaks we fully own the fix. Given the lack of maintainer activity, even small bugs become our responsibility, and some orgs do flag unmaintained dependencies regardless of size.

anyway, let's move on for now and we can evaluate this again if needed

egegunes

LGTM but consider @mayankshah1607's suggestion

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-13T10:19:30Z

+type envConfig struct {
+	LeaderElection          bool   `default:"true" envconfig:"PGO_CONTROLLER_LEADER_ELECTION_ENABLED"`
+	LeaderElectionID        string `envconfig:"PGO_CONTROLLER_LEASE_NAME"`
+	LeaderElectionNamespace string `envconfig:"PGO_NAMESPACE"`
+


Problem: envConfig uses unexported (lowercase) struct fields, which github.com/kelseyhightower/envconfig cannot populate via reflection, so defaults/env vars may not be applied.

Why it matters: This can silently disable leader election and set zero durations, potentially breaking operator behavior at runtime.

Fix: Export the fields (e.g., LeaderElection, LeaseDuration, etc.) or switch to a parsing approach that does not rely on setting unexported fields.

Copilot · 2026-02-13T10:19:30Z

+	// K8SPG-915
+	envs := new(envConfig)
+	if err := envconfig.Process("", envs); err != nil {
+		return options, errors.Wrap(err, "parse env vars")
+	}


Problem: envconfig.Process now makes initManager fail when PGO_WORKERS is set to a non-integer value (e.g., 3.14), whereas previously invalid values were logged and ignored.

Why it matters: This is a behavior/compatibility change that can prevent the operator from starting if a bad value is present in an existing deployment.

Fix: If fail-fast isn’t intended, parse PGO_WORKERS separately (string -> int) and keep the previous log-and-ignore behavior for invalid values, while still using envconfig for the new leader-election durations.

Copilot · 2026-02-13T10:19:30Z

 				options, err := initManager(ctx)
-				assert.NilError(t, err)
+				if v == "3.14" {
+					assert.ErrorContains(t, err, "parse env vars: envconfig.Process: assigning PGO_WORKERS to Workers: converting '3.14' to type int. details: strconv.ParseInt: parsing \"3.14\": invalid syntax")


Problem: The test asserts an overly specific substring for the envconfig parse error, including library-internal wording.

Why it matters: Minor changes in envconfig (or Go’s strconv error text) can break the test even when behavior is still correct.

Fix: Assert on a few stable substrings (e.g., "parse env vars", "PGO_WORKERS", and "invalid syntax") instead of the full error text.

Suggested change

assert.ErrorContains(t, err, "parse env vars: envconfig.Process: assigning PGO_WORKERS to Workers: converting '3.14' to type int. details: strconv.ParseInt: parsing \"3.14\": invalid syntax")

assert.ErrorContains(t, err, "parse env vars")

assert.ErrorContains(t, err, "PGO_WORKERS")

assert.ErrorContains(t, err, "invalid syntax")

mayankshah1607

LGTM, thanks for making all the changes

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.

Copilot · 2026-02-18T13:46:00Z

+          value: 60s
+        - name: PGO_CONTROLLER_RENEW_DEADLINE
+          value: 40s
+        - name: PGO_CONTROLLER_RETRY_PERIOD
+          value: 10s


The duration values in this YAML file are inconsistent with config/manager/default/manager.yaml. Here they are unquoted (e.g., 60s), while in config/manager/default/manager.yaml they are quoted (e.g., "60s"). While both formats are technically valid YAML, consistency across deployment files is important for maintainability. All other similar YAML files in this PR (deploy/operator.yaml, deploy/cw-bundle.yaml, deploy/bundle.yaml) use unquoted values. Consider quoting these values to match config/manager/default/manager.yaml, or vice versa, to maintain consistency across all deployment files.

Suggested change

value: 60s

- name: PGO_CONTROLLER_RENEW_DEADLINE

value: 40s

- name: PGO_CONTROLLER_RETRY_PERIOD

value: 10s

value: "60s"

- name: PGO_CONTROLLER_RENEW_DEADLINE

value: "40s"

- name: PGO_CONTROLLER_RETRY_PERIOD

value: "10s"

Copilot · 2026-02-18T13:46:01Z

+          value: 60s
+        - name: PGO_CONTROLLER_RENEW_DEADLINE
+          value: 40s
+        - name: PGO_CONTROLLER_RETRY_PERIOD
+          value: 10s


The duration values in this YAML file are inconsistent with config/manager/default/manager.yaml. Here they are unquoted (e.g., 60s), while in config/manager/default/manager.yaml they are quoted (e.g., "60s"). While both formats are technically valid YAML, consistency across deployment files is important for maintainability. Consider quoting these values to match config/manager/default/manager.yaml, or vice versa, to maintain consistency across all deployment files.

Suggested change

value: 60s

- name: PGO_CONTROLLER_RENEW_DEADLINE

value: 40s

- name: PGO_CONTROLLER_RETRY_PERIOD

value: 10s

value: "60s"

- name: PGO_CONTROLLER_RENEW_DEADLINE

value: "40s"

- name: PGO_CONTROLLER_RETRY_PERIOD

value: "10s"

Copilot · 2026-02-18T13:46:01Z

+          value: 60s
+        - name: PGO_CONTROLLER_RENEW_DEADLINE
+          value: 40s
+        - name: PGO_CONTROLLER_RETRY_PERIOD
+          value: 10s


The duration values in this YAML file are inconsistent with config/manager/default/manager.yaml. Here they are unquoted (e.g., 60s), while in config/manager/default/manager.yaml they are quoted (e.g., "60s"). While both formats are technically valid YAML, consistency across deployment files is important for maintainability. Consider quoting these values to match config/manager/default/manager.yaml, or vice versa, to maintain consistency across all deployment files.

Copilot · 2026-02-18T13:46:01Z

+          value: 60s
+        - name: PGO_CONTROLLER_RENEW_DEADLINE
+          value: 40s
+        - name: PGO_CONTROLLER_RETRY_PERIOD
+          value: 10s


The duration values in this YAML file are inconsistent with config/manager/default/manager.yaml. Here they are unquoted (e.g., 60s), while in config/manager/default/manager.yaml they are quoted (e.g., "60s"). While both formats are technically valid YAML, consistency across deployment files is important for maintainability. Consider quoting these values to match config/manager/default/manager.yaml, or vice versa, to maintain consistency across all deployment files.

Copilot · 2026-02-18T13:46:02Z

+		assert.Assert(t, options.LeaseDuration.Seconds() == 60)
+		assert.Assert(t, options.RenewDeadline.Seconds() == 40)
+		assert.Assert(t, options.RetryPeriod.Seconds() == 10)



The test directly dereferences options.LeaseDuration without first checking if it's nil. While the code currently sets this to a pointer to a non-zero duration, if envconfig.Process were to fail to parse the environment variable or set it to nil for any reason, this test would panic. Consider adding a nil check before dereferencing, similar to how options.Cache.SyncPeriod is checked on line 22-24.

Suggested change

assert.Assert(t, options.LeaseDuration.Seconds() == 60)

assert.Assert(t, options.RenewDeadline.Seconds() == 40)

assert.Assert(t, options.RetryPeriod.Seconds() == 10)

if assert.Check(t, options.LeaseDuration != nil) {

assert.Assert(t, options.LeaseDuration.Seconds() == 60)

}

if assert.Check(t, options.RenewDeadline != nil) {

assert.Assert(t, options.RenewDeadline.Seconds() == 40)

}

if assert.Check(t, options.RetryPeriod != nil) {

assert.Assert(t, options.RetryPeriod.Seconds() == 10)

}

Copilot · 2026-02-18T13:46:02Z

+		t.Setenv("PGO_CONTROLLER_RETRY_PERIOD", "3s")
+		options, err := initManager(ctx)
+		assert.NilError(t, err)
+


The test directly dereferences options.LeaseDuration, options.RenewDeadline, and options.RetryPeriod without first checking if they're nil. While the code currently sets these to pointers to non-zero durations, if envconfig.Process were to fail to parse the environment variables or set them to nil for any reason, this test would panic. Consider adding nil checks before dereferencing, similar to how options.Cache.SyncPeriod is checked on line 22-24.

Suggested change

if options.LeaseDuration == nil {

t.Fatalf("LeaseDuration is nil; expected non-nil *time.Duration")

}

if options.RenewDeadline == nil {

t.Fatalf("RenewDeadline is nil; expected non-nil *time.Duration")

}

if options.RetryPeriod == nil {

t.Fatalf("RetryPeriod is nil; expected non-nil *time.Duration")

}

gkech

approving but my main concern here still stands: #1433 (comment)

JNKPercona · 2026-02-19T11:29:07Z

Test Name	Result	Time
backup-enable-disable	passed	00:00:00
builtin-extensions	passed	00:00:00
custom-envs	passed	00:00:00
custom-extensions	passed	00:00:00
custom-tls	passed	00:00:00
database-init-sql	passed	00:00:00
demand-backup	passed	00:00:00
demand-backup-offline-snapshot	passed	00:00:00
dynamic-configuration	passed	00:00:00
finalizers	passed	00:00:00
init-deploy	passed	00:00:00
huge-pages	passed	00:00:00
monitoring	passed	00:08:13
monitoring-pmm3	passed	00:00:00
one-pod	passed	00:00:00
operator-self-healing	passed	00:00:00
pitr	passed	00:00:00
scaling	passed	00:00:00
scheduled-backup	passed	00:00:00
self-healing	passed	00:00:00
sidecars	passed	00:00:00
standby-pgbackrest	passed	00:00:00
standby-streaming	passed	00:00:00
start-from-backup	passed	00:00:00
tablespaces	passed	00:00:00
telemetry-transfer	passed	00:00:00
upgrade-consistency	passed	00:00:00
upgrade-minor	passed	00:00:00
users	passed	00:00:00

Summary	Value
Tests Run	29/29
Job Duration	00:22:43
Total Test Time	00:08:13

commit: 49fedb1
image: perconalab/percona-postgresql-operator:PR-1433-49fedb1ac

pooknull added 3 commits February 3, 2026 14:26

K8SPG-915: add leader election options to operator.yaml

3c498ff

https://perconadev.atlassian.net/browse/K8SPG-915

fix unit-tests

304a5a6

Merge remote-tracking branch 'origin/main' into K8SPG-915

8ac5ebc

pooknull marked this pull request as ready for review February 10, 2026 11:55

pooknull requested review from egegunes, gkech, hors, mayankshah1607, nmarukovich and oksana-grishchenko as code owners February 10, 2026 11:55

egegunes requested changes Feb 11, 2026

View reviewed changes

gkech previously approved these changes Feb 11, 2026

View reviewed changes

Merge branch 'main' into K8SPG-915

bf18afe

Copilot AI review requested due to automatic review settings February 11, 2026 09:45

mayankshah1607 reviewed Feb 11, 2026

View reviewed changes

Copilot started reviewing on behalf of mayankshah1607 February 11, 2026 09:45 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Comment thread cmd/postgres-operator/main.go Outdated

Comment thread cmd/postgres-operator/main.go Outdated

Comment thread cmd/postgres-operator/main_test.go

add PGO_CONTROLLER_LEADER_ELECTION_ENABLED env var

5ab1f96

pooknull dismissed gkech’s stale review via 5ab1f96 February 11, 2026 10:17

pooknull requested review from egegunes, gkech and mayankshah1607 February 11, 2026 10:18

hors previously approved these changes Feb 11, 2026

View reviewed changes

mayankshah1607 requested changes Feb 12, 2026

View reviewed changes

egegunes previously approved these changes Feb 12, 2026

View reviewed changes

move env var logic to env.go

518e892

Copilot AI review requested due to automatic review settings February 12, 2026 10:59

pooknull dismissed stale reviews from egegunes and hors via 518e892 February 12, 2026 10:59

Copilot started reviewing on behalf of pooknull February 12, 2026 10:59 View session

pooknull requested a review from egegunes February 12, 2026 11:03

Copilot AI reviewed Feb 12, 2026

View reviewed changes

Comment thread cmd/postgres-operator/env.go Outdated

Comment thread cmd/postgres-operator/main.go

Comment thread cmd/postgres-operator/main.go Outdated

copilot suggestions

ec54157

egegunes added this to the v2.9.0 milestone Feb 12, 2026

use kelseyhightower/envconfig

8cc91bf

Copilot AI review requested due to automatic review settings February 13, 2026 10:14

Merge branch 'main' into K8SPG-915

8e4ece2

Copilot started reviewing on behalf of pooknull February 13, 2026 10:14 View session

small fix

403100a

Copilot AI reviewed Feb 13, 2026

View reviewed changes

egegunes previously approved these changes Feb 16, 2026

View reviewed changes

mayankshah1607 previously approved these changes Feb 16, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into K8SPG-915

9d46b97

pooknull dismissed stale reviews from mayankshah1607 and egegunes via 9d46b97 February 18, 2026 07:50

gkech previously approved these changes Feb 18, 2026

View reviewed changes

hors requested review from egegunes and mayankshah1607 February 18, 2026 12:42

Merge remote-tracking branch 'origin/main' into K8SPG-915

919a8a6

Copilot AI review requested due to automatic review settings February 18, 2026 13:40

pooknull dismissed gkech’s stale review via 919a8a6 February 18, 2026 13:40

Copilot started reviewing on behalf of pooknull February 18, 2026 13:41 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

egegunes approved these changes Feb 19, 2026

View reviewed changes

mayankshah1607 approved these changes Feb 19, 2026

View reviewed changes

Merge branch 'main' into K8SPG-915

49fedb1

gkech approved these changes Feb 19, 2026

View reviewed changes

hors merged commit 9ace0ef into main Feb 19, 2026
16 checks passed

hors deleted the K8SPG-915 branch February 19, 2026 11:38

-					assert.ErrorContains(t, err, "parse env vars: envconfig.Process: assigning PGO_WORKERS to Workers: converting '3.14' to type int. details: strconv.ParseInt: parsing \"3.14\": invalid syntax")
+					assert.ErrorContains(t, err, "parse env vars")
+					assert.ErrorContains(t, err, "PGO_WORKERS")
+					assert.ErrorContains(t, err, "invalid syntax")

+		if options.LeaseDuration == nil {
+			t.Fatalf("LeaseDuration is nil; expected non-nil *time.Duration")
+		}
+		if options.RenewDeadline == nil {
+			t.Fatalf("RenewDeadline is nil; expected non-nil *time.Duration")
+		}
+		if options.RetryPeriod == nil {
+			t.Fatalf("RetryPeriod is nil; expected non-nil *time.Duration")
+		}

Conversation

pooknull commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DESCRIPTION

CHECKLIST

Uh oh!

egegunes left a comment

Choose a reason for hiding this comment

Uh oh!

mayankshah1607 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mayankshah1607 Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egegunes left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

mayankshah1607 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

pooknull commented Feb 3, 2026 •

edited

Loading

mayankshah1607 Feb 12, 2026 •

edited

Loading