Skip to content

K8SPG-786 K8SPG-1040 add upgrade test and fix operator behaviuor with extentions. - #1734

Draft
nmarukovich wants to merge 29 commits into
mainfrom
K8SPG-786
Draft

K8SPG-786 K8SPG-1040 add upgrade test and fix operator behaviuor with extentions. #1734
nmarukovich wants to merge 29 commits into
mainfrom
K8SPG-786

Conversation

@nmarukovich

@nmarukovich nmarukovich commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

CHANGE DESCRIPTION

Problem:
When a cluster has pg_cron installed through spec.extensions.custom and the operator gets upgraded, the new operator drops the extension together with its data. Two reasons:

  1. The internal reconcile only checks the builtin flag. pg_cron installed as custom has no builtin flag, the flag defaults to false — so the reconcile drops it.
  2. Even with a check against the custom list, there is a window right after the operator upgrade: the internal PostgresCluster spec was written by the old operator and has neither the flag nor the custom list. The first reconcile runs on that stale spec and drops the extension before the new operator rewrites it.

Solution:

  1. Copy the custom extension names into the internal spec (extensions.custom). Before dropping pg_cron or set_user, check both: drop only when the builtin flag is false AND the extension is not in the custom list.
  2. Add a custom-extensions-synced annotation. The operator stamps it every time it writes the internal spec. Drops are skipped while the annotation is missing — that means the spec is still the one from the old operator and can't be trusted. The next rewrite brings the annotation and the real custom list. The old spec never gets a drop, only a short delay.

The annotation is temporary: it only matters for upgrades from versions that didn't write the custom list (3.0.0 and older). Once those versions are out of the supported upgrade path, it can be removed.

new upgrade e2e tests
custom-extensions and upgrade-minor were extended.

Limitations:
This fixes pg_cron and set_user only. Other builtin extensions (pgaudit, pg_stat_monitor, pg_stat_statements, pgvector, pg_repack) have the same problem: if you install one of them as custom and its builtin flag is off, the operator drops it on every reconcile.
Question: do we fix the rest in a follow-up, or do we say "don't install builtin extensions as custom"? If the latter — let's reject it in validation, silent drop is worse.

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?

nmarukovich and others added 8 commits August 4, 2026 12:41
… into K8SPG-786

# Conflicts:
#	e2e-tests/functions
#	e2e-tests/run-pr.csv
#	e2e-tests/run-release.csv
#	e2e-tests/tests/custom-extensions/04-check-extensions.yaml
#	e2e-tests/tests/custom-extensions/08-check-extensions.yaml
#	e2e-tests/tests/custom-extensions/13-check-extensions.yaml
#	e2e-tests/tests/custom-extensions/16-check-extensions.yaml
#	pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go
#	pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go
#	pkg/apis/upstream.pgv2.percona.com/v1beta1/zz_generated.deepcopy.go
@nmarukovich nmarukovich changed the title K8 spg 786 K8SPG-786 Aug 9, 2026
@nmarukovich

Copy link
Copy Markdown
Contributor Author

copilot

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses K8SPG-786 by making pg_cron and set_user builtin extension management tri-state so the operator can distinguish between “unset/leave as-is” vs “explicitly disable/drop”, preventing unintended DROP EXTENSION (and data loss) during upgrades and when migrating extensions from spec.extensions.custom to builtin flags. It also expands E2E coverage to validate operator upgrades, minor upgrades, and cert-manager upgrade scenarios without triggering unwanted rollouts or data loss.

Changes:

  • Change upstream PostgresCluster extension flags for PGCron/SetUser from bool to *bool (nil = leave alone), and update deepcopy behavior.
  • Update reconciliation logic to respect nil vs false/true, and prevent custom-extension removal from dropping extensions that are now enabled via builtin flags.
  • Add/extend E2E tests for operator upgrades (extensions + cert-manager) and minor upgrade flow (backup/restore, rollout checks, log checks).

Reviewed changes

Copilot reviewed 70 out of 72 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/apis/upstream.pgv2.percona.com/v1beta1/zz_generated.deepcopy.go DeepCopy updates for new *bool extension fields.
pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go Convert PGCron/SetUser to tri-state *bool in upstream API.
pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go Stop defaulting PGCron/SetUser; map Percona CR -> upstream tri-state fields.
pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go Add unit tests covering tri-state passthrough + stored-spec clearing behavior.
percona/controller/pgcluster/controller.go Avoid dropping custom extensions when the same extension is enabled via builtin flags.
internal/controller/postgrescluster/postgres.go Treat nil builtin-extension flags as “do not touch”; keep explicit enable/disable behavior.
internal/controller/postgrescluster/controller.go Only add shared-preload params when tri-state flags are explicitly enabled.
e2e-tests/tests/upgrade-minor/conf/upgrade-minor.yaml New base config for richer upgrade-minor scenario (multi-repo, resources, etc.).
e2e-tests/tests/upgrade-minor/conf/log-allowlist.txt New allowlist for upgrade-minor log scanning.
e2e-tests/tests/upgrade-minor/13-check-logs.yaml Add post-upgrade log scan step.
e2e-tests/tests/upgrade-minor/12-collation-mismatch.sql Add SQL used to detect collation mismatch after upgrade.
e2e-tests/tests/upgrade-minor/12-check-collation-mismatch.yaml Point check to the new SQL file name/location.
e2e-tests/tests/upgrade-minor/11-read-from-primary.yaml Fix configmap naming to match step index.
e2e-tests/tests/upgrade-minor/11-assert.yaml Assert data read-back from primary after additional write.
e2e-tests/tests/upgrade-minor/10-write-data.yaml Add extra write step (post-upgrade) for read verification.
e2e-tests/tests/upgrade-minor/09-upgrade-cluster.yaml Strengthen upgrade step (timeouts, image waits, consistency checks, preconditions).
e2e-tests/tests/upgrade-minor/09-assert.yaml Update expected generations and status assertions for new flow.
e2e-tests/tests/upgrade-minor/08-restore.yaml Add restore step to validate cross-version backup/restore behavior.
e2e-tests/tests/upgrade-minor/08-assert.yaml Assert restore CR succeeded and cluster returned to ready.
e2e-tests/tests/upgrade-minor/07-backup-old-crversion.yaml Add backup step while cluster still has released crVersion.
e2e-tests/tests/upgrade-minor/07-assert.yaml Assert backup succeeded.
e2e-tests/tests/upgrade-minor/06-check-logs.yaml Add mid-upgrade log scan step.
e2e-tests/tests/upgrade-minor/05-sleep-after-operator-update.yaml Remove fixed sleep step (replaced by consistency/no-rollout checks).
e2e-tests/tests/upgrade-minor/05-check-no-rollout.yaml Add explicit “no operator-driven rollout” + heartbeat gap assertion.
e2e-tests/tests/upgrade-minor/05-assert.yaml Update expected generations/status due to new backup/flow behavior.
e2e-tests/tests/upgrade-minor/04-upgrade-operator.yaml Add baseline snapshot + heartbeat before operator update.
e2e-tests/tests/upgrade-minor/02-write-data.yaml Extend to include a full backup taken by released operator.
e2e-tests/tests/upgrade-minor/02-assert.yaml Assert pre-upgrade backup succeeded.
e2e-tests/tests/upgrade-minor/01-create-cluster.yaml Compose CR from released template + suite config; pin init image; repo2 wiring.
e2e-tests/tests/upgrade-minor/01-assert.yaml Increase timeout and assert repo2 stanza creation.
e2e-tests/tests/upgrade-minor/00-deploy-operator.yaml Add MinIO + repo2 credentials/bootstrap for cross-version backup/restore.
e2e-tests/tests/operator-upgrade-extensions/conf/operator-upgrade-extensions.yaml New suite config for extension survival across operator/cluster upgrade.
e2e-tests/tests/operator-upgrade-extensions/conf/log-allowlist.txt New allowlist for operator-upgrade-extensions log scanning.
e2e-tests/tests/operator-upgrade-extensions/99-remove-cluster-gracefully.yaml New cleanup step for suite teardown.
e2e-tests/tests/operator-upgrade-extensions/08-check-logs.yaml New log scan step after upgrade.
e2e-tests/tests/operator-upgrade-extensions/07-check-extensions-after-cluster-upgrade.yaml Validate extension set + pg_cron ticking + vector data after cluster upgrade.
e2e-tests/tests/operator-upgrade-extensions/06-upgrade-cluster.yaml Cluster upgrade step for the suite.
e2e-tests/tests/operator-upgrade-extensions/05-check-extensions-survived.yaml Validate no rollout and extension set unchanged after operator update.
e2e-tests/tests/operator-upgrade-extensions/04-upgrade-operator.yaml Operator upgrade step for the suite.
e2e-tests/tests/operator-upgrade-extensions/03-enable-extensions-and-write-data.yaml Install extensions + create data that must survive upgrades.
e2e-tests/tests/operator-upgrade-extensions/02-add-custom-pg-cron.yaml Add pg_cron as custom extension as part of migration scenario.
e2e-tests/tests/operator-upgrade-extensions/01-create-cluster.yaml Create suite cluster from released CR + suite config; canary for CRD pruning.
e2e-tests/tests/operator-upgrade-extensions/01-assert.yaml Assert suite cluster readiness.
e2e-tests/tests/operator-upgrade-extensions/00-deploy-operator.yaml Deploy released operator + MinIO/custom extensions bootstrap for suite.
e2e-tests/tests/operator-upgrade-extensions/00-assert.yaml Assert operator and MinIO are ready.
e2e-tests/tests/operator-upgrade-cert-manager/conf/log-allowlist.txt New allowlist for operator-upgrade-cert-manager log scanning.
e2e-tests/tests/operator-upgrade-cert-manager/99-remove-cluster-gracefully.yaml New cleanup step for suite teardown.
e2e-tests/tests/operator-upgrade-cert-manager/10-check-logs.yaml New log scan step after upgrade.
e2e-tests/tests/operator-upgrade-cert-manager/09-check-restored-data.yaml Verify restored data correctness.
e2e-tests/tests/operator-upgrade-cert-manager/08-restore.yaml Restore step to validate cross-version backup/restore.
e2e-tests/tests/operator-upgrade-cert-manager/08-assert.yaml Assert restore succeeded and cluster is ready post-restore.
e2e-tests/tests/operator-upgrade-cert-manager/07-backup-after-update.yaml Backup step after operator upgrade.
e2e-tests/tests/operator-upgrade-cert-manager/07-assert.yaml Assert post-upgrade backup succeeded.
e2e-tests/tests/operator-upgrade-cert-manager/06-check-tls-unchanged.yaml Validate cert-manager presence doesn’t take over PKI-issued secrets or roll pods.
e2e-tests/tests/operator-upgrade-cert-manager/05-upgrade-operator.yaml Operator upgrade step for cert-manager scenario.
e2e-tests/tests/operator-upgrade-cert-manager/04-backup.yaml Backup step before operator upgrade.
e2e-tests/tests/operator-upgrade-cert-manager/04-assert.yaml Assert pre-upgrade backup succeeded.
e2e-tests/tests/operator-upgrade-cert-manager/03-deploy-cert-manager.yaml Introduce cert-manager after PKI cluster exists (K8SPG-1017 trigger).
e2e-tests/tests/operator-upgrade-cert-manager/02-write-data-and-save-tls.yaml Write data + snapshot TLS/revisions baseline; premise checks.
e2e-tests/tests/operator-upgrade-cert-manager/01-create-cluster.yaml Create released cluster pinned to released init image; add repo2 config.
e2e-tests/tests/operator-upgrade-cert-manager/01-assert.yaml Assert cert-manager suite cluster readiness.
e2e-tests/tests/operator-upgrade-cert-manager/00-deploy-operator.yaml Deploy released operator + MinIO/TLS bootstrap for repo2 backups.
e2e-tests/tests/operator-upgrade-cert-manager/00-assert.yaml Assert operator and MinIO are ready.
e2e-tests/tests/custom-extensions/21-check-migration.yaml New migration verification: custom -> builtin must not drop pg_cron or its data.
e2e-tests/tests/custom-extensions/20-migrate-to-builtin.yaml Add migration step enabling builtin pg_cron while removing it from custom list.
e2e-tests/tests/custom-extensions/20-assert.yaml Assert expected status after migration step.
e2e-tests/tests/custom-extensions/19-enable-pg_cron.yaml Install pg_cron extension + create durable data (cron.job) for migration test.
e2e-tests/tests/custom-extensions/17-install-pg_cron.yaml Install pg_cron as custom extension to set up migration scenario.
e2e-tests/tests/custom-extensions/17-assert.yaml Assert expected status after custom pg_cron installation.
e2e-tests/run-release.csv Add new upgrade suites to release run list.
e2e-tests/run-pr.csv Add new upgrade suites to PR run list.
e2e-tests/functions Add shared helper functions for rollout/TLS/log assertions; adjust AWS endpoint.
Suppressed comments (1)

pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go:556

    1. Problem: new(*cr.Spec.Extensions.SetUser.Enabled) is invalid Go (the built-in new requires a type, not a value), so this code will not compile.
  1. Why it matters: This breaks PerconaPGCluster.ToCrunchy compilation and prevents reconciliation/builds.
  2. Fix: Allocate the bool pointer via ptr.To(*...) (or b := *...; postgresCluster.Spec.Extensions.SetUser = &b).

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

Comment on lines 547 to 551
if cr.Spec.Extensions.PGCron.Enabled != nil {
postgresCluster.Spec.Extensions.PGCron = *cr.Spec.Extensions.PGCron.Enabled
postgresCluster.Spec.Extensions.PGCron = new(*cr.Spec.Extensions.PGCron.Enabled)
} else {
postgresCluster.Spec.Extensions.PGCron = nil
}
@nmarukovich nmarukovich changed the title K8SPG-786 add upgrade test and fix operator behaviuor with extentions. K8SPG-786 K8SPG-1040 add upgrade test and fix operator behaviuor with extentions. Aug 16, 2026
@egegunes

Copy link
Copy Markdown
Contributor

my version question was not about the patch version. what i'm trying to understand how to enable users who had pg_cron as a custom extension to start using the built-in pg_cron.

because with these changes, once you have pg_cron custom extension you're stuck with it. at least we need to document how to do the migration

@nmarukovich

Copy link
Copy Markdown
Contributor Author

my version question was not about the patch version. what i'm trying to understand how to enable users who had pg_cron as a custom extension to start using the built-in pg_cron.

because with these changes, once you have pg_cron custom extension you're stuck with it. at least we need to document how to do the migration

The migration is supported in this PR. You do it in a single apply:

extensions:
  pg_cron:
    enabled: true      # builtin takes over
  custom: []           # pg_cron removed from custom

The extension and its data survive. This PR added a guard for exactly this case: when the builtin flag is enabled, removing the extension from custom does not drop it — the builtin path takes over (its CREATE EXTENSION IF NOT EXISTS is a no-op on the existing extension):

if builtInExtensionEnabled(cr, ext) {
    continue   // don't DROP: the user moved it from custom to builtin
}

It's covered by e2e (custom-extensions steps 17–21): install pg_cron as custom → create a cron.schedule entry → switch to builtin in one apply → assert the extension is still there and cron.job still has the row.

Without this PR you really were stuck: removing it from custom dropped the extension with all schedules, and the builtin path would re-create it empty.

One rule to document : enable the builtin flag in the same apply as removing the custom entry. Removing from custom alone is an uninstall and drops the extension — that part is intentional.

egegunes
egegunes previously approved these changes Aug 18, 2026
@nmarukovich
nmarukovich marked this pull request as draft August 19, 2026 13:06
@nmarukovich
nmarukovich requested review from egegunes, mayankshah1607 and oksana-grishchenko and a lite review from Copilot August 20, 2026 16:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

e2e-tests/functions:580

  • The AWS CLI invocation that downloads extension artifacts from AWS S3 no longer sets AWS_DEFAULT_REGION, unlike other aws-cli calls in this test harness; this can cause aws to fail with "You must specify a region" depending on the container/config and makes the behavior inconsistent.

Set AWS_DEFAULT_REGION for the first copy step (and remove the now-empty line) to keep the script robust and consistent.

		bash -c "
		AWS_ACCESS_KEY_ID=$(get_aws_access_key_id aws-s3-secret) \
		AWS_SECRET_ACCESS_KEY=$(get_aws_access_key aws-s3-secret) \
    
		/usr/bin/aws --endpoint-url https://s3.amazonaws.com s3 cp s3://pg-extensions/ /tmp/ --recursive &&

Comment thread e2e-tests/functions
local gap
gap=$(run_psql_local 'SELECT COALESCE(ceil(EXTRACT(EPOCH FROM max(ts - prev)))::int,0) FROM (SELECT ts, lag(ts) OVER (ORDER BY ts) AS prev FROM heartbeat) g' "$uri" | tr -d '[:space:]')
echo "heartbeat max gap: ${gap}s (limit ${max_gap}s)"
if [[ -z "$gap" || "$gap" -gt "$max_gap" ]]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shfmt] reported by reviewdog 🐶

Suggested change
if [[ -z "$gap" || "$gap" -gt "$max_gap" ]]; then
if [[ -z $gap || $gap -gt $max_gap ]]; then

@JNKPercona

Copy link
Copy Markdown
Collaborator
Test Name Result Time
backup-enable-disable passed 00:00:00
builtin-extensions passed 00:00:00
custom-extensions passed 00:00:00
cert-manager-tls passed 00:00:00
cert-management-policy passed 00:00:00
custom-envs 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
extra-volumes passed 00:00:00
finalizers passed 00:00:00
init-deploy passed 00:00:00
huge-pages passed 00:00:00
logical-replica passed 00:00:00
major-upgrade-14-to-15 passed 00:00:00
major-upgrade-15-to-16 passed 00:00:00
major-upgrade-16-to-17 passed 00:00:00
major-upgrade-17-to-18 passed 00:00:00
ldap passed 00:00:00
ldap-tls passed 00:00:00
logcollection passed 00:00:00
monitoring passed 00:00:00
operator-upgrade-cert-manager passed 00:15:57
operator-upgrade-extensions passed 00:00:00
one-pod passed 00:00:00
repo-host-autogrow passed 00:00:00
operator-self-healing passed 00:00:00
pgbouncer passed 00:00:00
pg-tde passed 00:00:00
pg-tde-wal-encrypt 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
start-from-volumes passed 00:00:00
tablespaces passed 00:00:00
telemetry-transfer passed 00:00:00
upgrade-consistency passed 00:00:00
upgrade-minor failure 00:10:45
users passed 00:00:00
migration-from-crunchy-standby passed 00:00:00
migration-from-crunchy-pv passed 00:00:00
migration-from-crunchy-backup-restore passed 00:00:00
Summary Value
Tests Run 49/49
Job Duration 00:46:50
Total Test Time 00:26:42

commit: 28ecf4c
image: perconalab/percona-postgresql-operator:PR-1734-28ecf4cbc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants