K8SPG-957: backup controller must watch VolumeSnapshots only if API is installed by mayankshah1607 · Pull Request #1464 · percona/percona-postgresql-operator

mayankshah1607 · 2026-02-27T05:58:25Z

CHANGE DESCRIPTION

Problem:
PG operator crash on EKS.

2026-02-26T08:58:31.770Z        ERROR   Could not wait for Cache to sync        {"controller": "
perconapgbackup", "controllerGroup": "pgv2.percona.com", "controllerKind": "PerconaPGBackup", "source": "kind source: *v1.VolumeSnapshot", "error": "failed to wait for perconapgbackup caches to sync kind source: *v1.VolumeSnapshot: timed out waiting for cache to be synced for Kind *v1.VolumeSnapshot"}                                  
runtime.goexit                                  
        /usr/local/go/src/runtime/asm_amd64.s:1771

Cause:
The backup controller tries to watch VolumeSnapshots, but the the API does not come pre-installed in EKS (unlike GKE). As a result, the caches fail to start.

Solution:
The watcher must be enabled only if the API is installed and feature gate enabled.

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?

Signed-off-by: Mayank Shah <mayank.shah@percona.com>

Copilot

Pull request overview

This PR prevents the PerconaPGBackup controller from crashing on clusters (e.g., EKS) where the VolumeSnapshot API is not installed by conditionally registering the VolumeSnapshot watch only when the feature gate is enabled and the API is discoverable.

Changes:

Added a Kubernetes discovery helper (GroupVersionKindExists) to detect whether a GroupVersion/Kind is available on the API server.
Updated the pgbackup controller setup to only Owns(VolumeSnapshot) when feature.BackupSnapshots is enabled and the VolumeSnapshot API is installed.
Updated operator wiring to pass ctx into the updated pgbackup controller SetupWithManager.

Reviewed changes

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

File	Description
`percona/k8s/util.go`	Adds API discovery helper to check for GroupVersion/Kind availability.
`percona/controller/pgbackup/controller.go`	Gates the VolumeSnapshot watch behind feature flag + API discovery to avoid cache sync failure.
`cmd/postgres-operator/main.go`	Updates pgbackup controller initialization to pass context into `SetupWithManager`.

Copilot · 2026-02-27T06:02:55Z

+	if r.DiscoveryClient == nil {
+		var err error
+		r.DiscoveryClient, err = discovery.NewDiscoveryClientForConfig(mgr.GetConfig())
+		if err != nil {
+			return errors.Wrap(err, "failed to create discovery client")
+		}
+	}


Problem: DiscoveryClient is always initialized in SetupWithManager, even when feature.BackupSnapshots is disabled (the only current use site).

Why it matters: This adds unnecessary client construction and discovery traffic during startup for the default/disabled case.

Fix: Only create the discovery client inside the feature.Enabled(ctx, feature.BackupSnapshots) block (or create a local client there) so it is only initialized when the conditional watch is actually needed.

Copilot · 2026-02-27T06:02:55Z

+	// Watch VolumeSnapshots if the feature is enabled and the API is available.
+	if feature.Enabled(ctx, feature.BackupSnapshots) {
+		gvk := volumesnapshotv1.SchemeGroupVersion.WithKind(pNaming.KindVolumeSnapshot)
+		gv := gvk.GroupVersion().String()
+		if ok, err := k8s.GroupVersionKindExists(r.DiscoveryClient, gv, gvk.Kind); err != nil {
+			return errors.Wrap(err, "check VolumeSnapshot API availability")
+		} else if ok {
+			b = b.Owns(&volumesnapshotv1.VolumeSnapshot{})
+		}
+	}


Problem: The new conditional watch behavior for VolumeSnapshots (to prevent startup crashes when the API is missing) is not covered by tests.

Why it matters: This is a regression fix for an operator crash path; without a test, it’s easy to reintroduce the unconditional watch and break EKS again.

Fix: Add a regression test (preferably envtest) that starts a manager without the VolumeSnapshot CRDs installed and asserts PGBackupReconciler.SetupWithManager(...) succeeds (and/or that it only adds the watch when the feature gate is enabled and the API is present).

Copilot · 2026-02-27T06:02:55Z

+// GroupVersionKindExists checks to see whether a given Kind for a given
+// GroupVersion exists in the Kubernetes API Server.
+func GroupVersionKindExists(dc *discovery.DiscoveryClient, groupVersion, kind string) (bool, error) {
+	if dc == nil {
+		return false, errors.New("discovery client is nil")
+	}
+
+	resourceList, err := dc.ServerResourcesForGroupVersion(groupVersion)
+	if err != nil {
+		if k8serrors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, err
+	}
+
+	for _, resource := range resourceList.APIResources {
+		if resource.Kind == kind {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}


Problem: GroupVersionKindExists duplicates the existing Reconciler.GroupVersionKindExists helper in internal/controller/postgrescluster/controller.go with near-identical discovery logic.

Why it matters: Duplicated helpers tend to drift (different nil/return behavior, error handling, etc.), making future changes harder and increasing the chance of inconsistent behavior.

Fix: Consolidate this logic into a single shared helper (e.g., keep it in percona/k8s and have the PostgresCluster reconciler call it), or remove this helper and reuse the existing one where appropriate.

computer has a point

Signed-off-by: Mayank Shah <mayank.shah@percona.com>

Copilot

Pull request overview

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

gkech · 2026-03-02T09:34:36Z

-	return (builder.ControllerManagedBy(mgr).
+
+	if r.DiscoveryClient == nil {
+		var err error


what's the reason for having this var instead of doing r.DiscoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig())? The error is returned, and it is already scoped in the if-clause

That's invalid syntax, you cannot use := with r.DiscoveryClient since it is not a new variable. That means err also must be declared first in order to use with =:

basically..

non-name r.DiscoveryClient on left side of := [BadDecl]

oh yes, you are right!

Signed-off-by: Mayank Shah <mayank.shah@percona.com>

JNKPercona · 2026-03-02T11:22:48Z

Test Name	Result	Time
backup-enable-disable	passed	00:13:26
builtin-extensions	passed	00:05:52
cert-manager-tls	passed	00:04:42
custom-envs	passed	00:20:11
custom-extensions	passed	00:15:09
custom-tls	passed	00:06:55
database-init-sql	passed	00:02:16
demand-backup	passed	00:25:08
demand-backup-offline-snapshot	passed	00:15:03
dynamic-configuration	passed	00:03:50
finalizers	passed	00:04:00
init-deploy	passed	00:02:53
huge-pages	passed	00:03:02
monitoring	passed	00:07:29
monitoring-pmm3	passed	00:08:13
one-pod	passed	00:06:17
operator-self-healing	passed	00:11:05
pg-tde	passed	00:09:41
pitr	passed	00:12:02
scaling	passed	00:05:38
scheduled-backup	passed	00:27:18
self-healing	passed	00:09:47
sidecars	passed	00:02:52
standby-pgbackrest	passed	00:13:34
standby-streaming	passed	00:09:43
start-from-backup	passed	00:12:28
tablespaces	passed	00:07:19
telemetry-transfer	passed	00:04:51
upgrade-consistency	passed	00:05:24
upgrade-minor	passed	00:05:39
users	passed	00:05:01

Summary	Value
Tests Run	31/31
Job Duration	01:28:23
Total Test Time	04:47:05

commit: 0abbe2f
image: perconalab/percona-postgresql-operator:PR-1464-0abbe2ff5

fix: watch VolumeSnapshots only when API installed

1ae7957

Signed-off-by: Mayank Shah <mayank.shah@percona.com>

Copilot AI review requested due to automatic review settings February 27, 2026 05:58

Copilot started reviewing on behalf of mayankshah1607 February 27, 2026 05:58 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

mayankshah1607 marked this pull request as ready for review March 2, 2026 04:04

mayankshah1607 requested review from egegunes, gkech, hors, nmarukovich, oksana-grishchenko and pooknull as code owners March 2, 2026 04:04

egegunes added this to the v2.9.0 milestone Mar 2, 2026

consolidate helpers

f365a73

Signed-off-by: Mayank Shah <mayank.shah@percona.com>

egegunes previously approved these changes Mar 2, 2026

View reviewed changes

Merge branch 'main' into K8SPG-957

eee40ae

Copilot AI review requested due to automatic review settings March 2, 2026 09:36

Copilot started reviewing on behalf of mayankshah1607 March 2, 2026 09:37 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

gkech reviewed Mar 2, 2026

View reviewed changes

improved validation and error wrapping

0abbe2f

Signed-off-by: Mayank Shah <mayank.shah@percona.com>

mayankshah1607 dismissed egegunes’s stale review via 0abbe2f March 2, 2026 09:53

mayankshah1607 requested review from egegunes and gkech March 2, 2026 09:53

hors approved these changes Mar 2, 2026

View reviewed changes

egegunes approved these changes Mar 3, 2026

View reviewed changes

gkech approved these changes Mar 3, 2026

View reviewed changes

mayankshah1607 merged commit 51080c5 into main Mar 3, 2026
16 checks passed

mayankshah1607 deleted the K8SPG-957 branch March 3, 2026 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

K8SPG-957: backup controller must watch VolumeSnapshots only if API is installed#1464

K8SPG-957: backup controller must watch VolumeSnapshots only if API is installed#1464
mayankshah1607 merged 4 commits into
mainfrom
K8SPG-957

mayankshah1607 commented Feb 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

egegunes Mar 2, 2026

Uh oh!

mayankshah1607 Mar 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

gkech Mar 2, 2026

Uh oh!

mayankshah1607 Mar 2, 2026 •

edited

Loading

Uh oh!

gkech Mar 3, 2026

Uh oh!

Uh oh!

Uh oh!

JNKPercona commented Mar 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

mayankshah1607 commented Feb 27, 2026

CHANGE DESCRIPTION

CHECKLIST

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

egegunes Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

mayankshah1607 Mar 2, 2026

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!

gkech Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

mayankshah1607 Mar 2, 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!

gkech Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JNKPercona commented Mar 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mayankshah1607 Mar 2, 2026 •

edited

Loading