K8SPG-911: Fix pg_tde status reporting on standbys by egegunes · Pull Request #1750 · percona/percona-postgresql-operator

egegunes · 2026-08-12T18:51:25Z

CHANGE DESCRIPTION

Problem:
Operator doesn't (and can't) run any SQL statements on standby clusters to enable and configure pg_tde. This results in status conditions related to pg_tde not appear. Consequences of this is bigger than just status reporting: enabling WAL encryption depends on these conditions being true.

Solution:
Reconcile pg_tde on standby clusters separately and observe if pg_tde is enabled and configured in database by read queries.

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?

oksana-grishchenko · 2026-08-13T09:08:19Z

 			"the condition should name the Secret that could not be read")
 		assert.Equal(t, patched, 1,
 			"the failure condition is useless unless it is written to the API")
-		assertEvent(t, r.Recorder, "PGTDEVaultProviderChangeFailed")


Is the deletion intended? I didn't find the logic that affects the event and the test still passes with this line added back

i deleted it by mistake, will restore

gkech

Mostly tests related comments, logic seems ok. let me know if we should defer them for the future.

gkech · 2026-08-17T10:04:09Z

+// K8SPG-911
+func TestReconcilePGTDEStandby(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()


In some tests of the same PR we use t.context, here we use background. We should be consistent and use t.context everywhere.

gkech · 2026-08-17T10:11:28Z

+			Recorder: events.NewRecorder(t, runtime.Scheme),
+			PodExec:  execResponder(&calls, answer(true, nil)),


these are not needed for this test case

we don't need Recorder: 487ac81

gkech · 2026-08-17T10:26:41Z

+			Recorder: events.NewRecorder(t, runtime.Scheme),
+			PodExec:  execResponder(&calls, answer(false, nil)),


again here we dont need these

we don't need Recorder: 487ac81

gkech · 2026-08-17T10:27:21Z

+		}
+
+		r.reconcilePGTDEStandby(ctx, cluster, standbyObserved())
+		assert.Equal(t, len(calls), 0,


why on some of these check we have messages while on others we do not? In general, I think that they are not very useful

¯_(ツ)_/¯

if it's okay, i won't do this change

gkech · 2026-08-17T10:29:52Z

+			assert.Equal(t, call.pod, "pgc1-instance1-abcd-0")
+			assert.Equal(t, call.container, naming.ContainerDatabase)
+			assert.Equal(t, call.command[0], "psql")
+			assert.Assert(t, argsContain(call.command, "--tuples-only"))


since we decided to assert that, why we only check tuples only and not no align?

it's claude adding a test for every change, why I use these flags described in comment:

percona-postgresql-operator/internal/pgtde/postgres.go

Lines 117 to 119 in ee3efcc

// readOnlyPSQLArgs make psql print a single value and nothing else, so the

// result of a one-row, one-column query is exactly "t" or "f".

var readOnlyPSQLArgs = []string{"--no-align", "--tuples-only"}

gkech · 2026-08-17T10:40:40Z

+		assert.NilError(t, r.reconcilePGTDEProviders(ctx, cluster, observed, failPatch(t)))
+		assert.Equal(t, cluster.Status.PGTDERevision, "")
+		assert.Assert(t, meta.FindStatusCondition(cluster.Status.Conditions,


still trying to understand why on a file we introduced ourselves we are using gotest.tools/v3/assert instead of github.com/stretchr/testify/assert, which is much more expressive

all tests in internal directory is using gotest.tools/v3/assert, that's why claude continued to use it

Copilot

Pull request overview

This PR addresses pg_tde status reporting for standby (recovery) clusters, where the operator cannot run SQL that writes system catalogs. It introduces a dedicated standby reconcile path that observes (via read-only queries on the standby leader) whether pg_tde is present and whether its principal key is reachable, then reports appropriate status conditions so downstream logic (notably WAL encryption gating) can behave correctly.

Changes:

Added PostgresCluster.IsStandby() helper and unit tests for standby detection.
Implemented read-only pg_tde observation (ObserveExtension, VerifyPrincipalKey) and standby-specific condition reporting (ReportStandby).
Added a standby reconcile path that execs into the Patroni standby leader to observe pg_tde state, while ensuring the writable-only pg_tde provider reconciliation is skipped on standbys.

Reviewed changes

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

Show a summary per file

File	Description
pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go	Adds `IsStandby()` helper on the API type.
pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_test.go	Adds unit coverage for `IsStandby()`.
internal/pgtde/postgres.go	Adds read-only psql helpers plus standby-oriented status reporting logic; adjusts WAL-encryption gating behavior for standby clusters.
internal/pgtde/postgres_test.go	Adds tests for standby WAL parameter behavior and new pg_tde observation/reporting helpers.
internal/controller/postgrescluster/postgres.go	Adds `reconcilePGTDEStandby` and prevents writable-only provider reconciliation from running on standbys.
internal/controller/postgrescluster/pgtde_test.go	Adds tests validating standby behavior (no writable SQL, correct status reporting) and helper utilities for exec simulation.
internal/controller/postgrescluster/instance.go	Adds `standbyLeaderPod()` selection logic for Patroni standby leader pods.
internal/controller/postgrescluster/instance_test.go	Adds unit tests for `standbyLeaderPod()`.
internal/controller/postgrescluster/controller.go	Invokes `reconcilePGTDEStandby` in the main reconcile flow.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

* K8SPG-911: Fix pg_tde status reporting on standbys * address review comments

K8SPG-911: Fix pg_tde status reporting on standbys

ee3efcc

egegunes requested review from gkech, hors, mayankshah1607, nmarukovich, oksana-grishchenko and pooknull as code owners August 12, 2026 18:51

egegunes added this to the v3.1.0 milestone Aug 12, 2026

oksana-grishchenko previously approved these changes Aug 13, 2026

View reviewed changes

mayankshah1607 previously approved these changes Aug 13, 2026

View reviewed changes

gkech reviewed Aug 17, 2026

View reviewed changes

hors requested a lite review from Copilot August 17, 2026 10:52

Copilot started reviewing on behalf of hors August 17, 2026 10:53 View session

Copilot AI reviewed Aug 17, 2026

View reviewed changes

address review comments

487ac81

egegunes dismissed stale reviews from mayankshah1607 and oksana-grishchenko via 487ac81 August 17, 2026 11:07

Merge branch 'main' into K8SPG-911-standby

bf04749

egegunes requested a review from gkech August 17, 2026 11:08

oksana-grishchenko approved these changes Aug 17, 2026

View reviewed changes

gkech approved these changes Aug 17, 2026

View reviewed changes

hors merged commit 4ac2e27 into main Aug 17, 2026
17 of 18 checks passed

hors deleted the K8SPG-911-standby branch August 17, 2026 11:45

egegunes added a commit that referenced this pull request Aug 17, 2026

K8SPG-911: Fix pg_tde status reporting on standbys (#1750)

9890e09

* K8SPG-911: Fix pg_tde status reporting on standbys * address review comments

egegunes added a commit that referenced this pull request Aug 17, 2026

K8SPG-911: Fix pg_tde status reporting on standbys (#1750)

5809473

* K8SPG-911: Fix pg_tde status reporting on standbys * address review comments

		Recorder: events.NewRecorder(t, runtime.Scheme),
		PodExec: execResponder(&calls, answer(true, nil)),

		Recorder: events.NewRecorder(t, runtime.Scheme),
		PodExec: execResponder(&calls, answer(false, nil)),

	// readOnlyPSQLArgs make psql print a single value and nothing else, so the
	// result of a one-row, one-column query is exactly "t" or "f".
	var readOnlyPSQLArgs = []string{"--no-align", "--tuples-only"}

Uh oh!

Conversation

egegunes commented Aug 12, 2026

CHANGE DESCRIPTION

CHECKLIST

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkech left a comment

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!

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants