K8SPG-1013: fix test leftovers for AKS and Openshift - #1773
Draft
valmiranogueira wants to merge 17 commits into
Draft
K8SPG-1013: fix test leftovers for AKS and Openshift#1773valmiranogueira wants to merge 17 commits into
valmiranogueira wants to merge 17 commits into
Conversation
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes E2E test leftovers for AKS and OpenShift, including Vault, PostgreSQL TDE, PV migration, and certificate recovery workflows.
Changes:
- Adds PostgreSQL-version-aware TDE testing and shared assertions.
- Updates Vault lifecycle, endpoints, and chaos network handling.
- Adjusts migration, AKS detection, certificate waits, and test timeouts.
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Summary | Review notes |
|---|---|---|
e2e-tests/tests/start-from-backup-tde/07-create-cluster.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/start-from-backup-tde/02-create-source-cluster.yaml |
Updates Vault endpoint. | Critical (2 votes): The suite still unconditionally deploys Vault and enables TDE, so PG versions below 17 fail instead of being skipped. |
e2e-tests/tests/standby-streaming/99-remove-cluster-gracefully.yaml |
Adds conditional Vault cleanup. | — |
e2e-tests/tests/standby-streaming/13-verify-encryption-standby.yaml |
Gates TDE verification by PostgreSQL version. | — |
e2e-tests/tests/standby-streaming/13-assert.yaml |
Uses helper-based encryption assertions. | Critical (2 votes): The verification step does not create 13-verify-wal-encryption-standby; the assertion therefore checks a missing ConfigMap on PG ≥17 runs. |
e2e-tests/tests/standby-streaming/11-write-data.yaml |
Makes table creation version-aware. | — |
e2e-tests/tests/standby-streaming/10-assert.yaml |
Uses shared readiness and promotion checks. | — |
e2e-tests/tests/standby-streaming/09-verify-encryption-standby.yaml |
Adds non-TDE data verification. | — |
e2e-tests/tests/standby-streaming/09-assert.yaml |
Adds conditional assertions. | — |
e2e-tests/tests/standby-streaming/08-verify-encryption-source.yaml |
Adds non-TDE data verification. | — |
e2e-tests/tests/standby-streaming/08-assert.yaml |
Adds helper-based data and TDE assertions. | — |
e2e-tests/tests/standby-streaming/07-write-data.yaml |
Makes table creation version-aware. | — |
e2e-tests/tests/standby-streaming/03-create-standby-cluster.yaml |
Makes standby TDE configuration version-aware. | — |
e2e-tests/tests/standby-streaming/03-assert.yaml |
Uses reusable TDE checks. | — |
e2e-tests/tests/standby-streaming/02-create-source-cluster.yaml |
Makes source TDE configuration version-aware. | — |
e2e-tests/tests/standby-streaming/02-assert.yaml |
Refactors source-cluster assertions. | — |
e2e-tests/tests/standby-streaming/01-deploy-vault.yaml |
Skips Vault for older PostgreSQL versions. | — |
e2e-tests/tests/standby-streaming/01-assert.yaml |
Adds conditional Vault assertions. | — |
e2e-tests/tests/standby-pgbackrest/99-remove-cluster-gracefully.yaml |
Adds conditional Vault cleanup. | — |
e2e-tests/tests/standby-pgbackrest/13-verify-encryption-standby.yaml |
Gates TDE verification by PostgreSQL version. | — |
e2e-tests/tests/standby-pgbackrest/13-assert.yaml |
Uses helper-based encryption assertions. | — |
e2e-tests/tests/standby-pgbackrest/11-write-data.yaml |
Makes table creation version-aware. | — |
e2e-tests/tests/standby-pgbackrest/10-assert.yaml |
Uses shared readiness and promotion checks. | — |
e2e-tests/tests/standby-pgbackrest/09-verify-encryption-standby.yaml |
Adds version-aware replication checks. | — |
e2e-tests/tests/standby-pgbackrest/09-assert.yaml |
Adds conditional assertions. | — |
e2e-tests/tests/standby-pgbackrest/08-verify-encryption-source.yaml |
Adds non-TDE data verification. | — |
e2e-tests/tests/standby-pgbackrest/08-assert.yaml |
Adds conditional assertions. | — |
e2e-tests/tests/standby-pgbackrest/07-write-data.yaml |
Makes table creation version-aware. | — |
e2e-tests/tests/standby-pgbackrest/03-create-standby-cluster.yaml |
Makes standby TDE configuration version-aware. | — |
e2e-tests/tests/standby-pgbackrest/03-assert.yaml |
Uses reusable TDE checks. | — |
e2e-tests/tests/standby-pgbackrest/02-create-source-cluster.yaml |
Makes source TDE configuration version-aware. | — |
e2e-tests/tests/standby-pgbackrest/02-assert.yaml |
Refactors source-cluster assertions. | — |
e2e-tests/tests/standby-pgbackrest/01-deploy-vault.yaml |
Skips Vault for older PostgreSQL versions. | — |
e2e-tests/tests/standby-pgbackrest/01-assert.yaml |
Adds conditional Vault assertions. | — |
e2e-tests/tests/pg-tde/99-remove-cluster-gracefully.yaml |
Uses shared Vault cleanup. | — |
e2e-tests/tests/pg-tde/11-disable-pgtde.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/pg-tde/09-change-vault-provider.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/pg-tde/02-create-cluster.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/pg-tde/00-deploy-operator.yaml |
Adds PostgreSQL version validation. | Critical (3 votes): The check rejects PostgreSQL 17 with -le 17; it should use < 17. |
e2e-tests/tests/pg-tde-wal-encrypt/99-remove-cluster-gracefully.yaml |
Uses shared Vault cleanup. | — |
e2e-tests/tests/pg-tde-wal-encrypt/15-disable-pgtde.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/pg-tde-wal-encrypt/10-change-vault-provider.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/pg-tde-wal-encrypt/03-enable-wal-encryption.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/pg-tde-wal-encrypt/02-create-cluster.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/pg-tde-wal-encrypt/00-deploy-operator.yaml |
Adds PostgreSQL version validation. | Critical (3 votes): The check rejects PostgreSQL 17 with -le 17; it should use < 17. |
e2e-tests/tests/migration-from-crunchy-pv/conf/percona-migrated.yaml |
Removes unsupported PVC selectors. | — |
e2e-tests/tests/migration-from-crunchy-pv/07-restore.yaml |
Updates restore documentation. | — |
e2e-tests/tests/migration-from-crunchy-pv/07-assert.yaml |
Updates post-restore replica expectations. | Critical (2 votes): StatefulSet status fields still require 1 after the cluster is scaled to 3 replicas; update the expectation to 3. |
e2e-tests/tests/migration-from-crunchy-pv/05-verify-data.yaml |
Removes PV binding before scale-up. | — |
e2e-tests/tests/migration-from-crunchy-pv/04-create-percona-cluster.yaml |
Pre-binds the migrated PV. | — |
e2e-tests/tests/major-upgrade-17-to-18/99-remove-cluster-gracefully.yaml |
Adds Vault cleanup. | — |
e2e-tests/tests/major-upgrade-17-to-18/01-create-cluster.yaml |
Updates Vault endpoint. | — |
e2e-tests/tests/huge-pages/01-assert.yaml |
Increases assertion timeout. | — |
e2e-tests/tests/cert-manager-tls/24-verify-external-cluster-issuer.yaml |
Adds issuer and readiness waits. | — |
e2e-tests/tests/cert-manager-tls/18-cert-deletion-recovery.yaml |
Refactors certificate recreation checks. | Moderate (2 votes): Wait for the owner-referenced Secret to be deleted before retrying the Certificate. |
e2e-tests/tests/cert-manager-tls/02-verify-internal-pki.yaml |
Skips unsupported Rancher verification. | — |
e2e-tests/tests/cert-manager-tls/00-deploy-operator.yaml |
Removes Rancher test failure. | — |
e2e-tests/functions |
Adds shared assertions, Vault cleanup, AKS detection, and network-namespace handling. | Moderate (2 votes): The start-from-backup TDE cleanup does not call stop_vault, leaving the Helm release, namespace, and Vault resources running.Moderate (3 votes): vault_tls generates CSR names with vault-csr-${RANDOM}, while cleanup deletes vault-csr-${name}, leaving the CSR behind.Critical (1 vote): The network-namespace fallback can select the host namespace; fail when no non-host namespace can be resolved instead. |
Suppressed comments (2)
e2e-tests/tests/cert-manager-tls/18-cert-deletion-recovery.yaml:28
- The old recovery test validated all required Secret keys after recreation (including
tls.keyand, for the pgBouncer Secret,ca.crt), but this helper only comparestls.crt. A partially recreated Secret can therefore pass while leaving TLS unusable; restore the per-certificate key-presence checks before reporting success.
cert_after=$(kubectl -n "$NAMESPACE" get secret "$secret_name" -o jsonpath='{.data.tls\.crt}')
if [[ "$cert_before" == "$cert_after" ]]; then
echo "Secret $secret_name data did not change after Certificate $cert_name recreation"
return 1
fi
e2e-tests/tests/standby-pgbackrest/13-assert.yaml:16
- The previous declarative assertion checked
13-verify-wal-encryption-standby, and the verification step still creates it, but this refactor no longer asserts its value. A WAL-encryption failure can now pass this step; retain thepg_tde_wal_encrypt == ' on'assertion.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 59 changed files in this pull request and generated 2 comments.
Suppressed comments (12)
e2e-tests/functions:1253
- If no non-host candidate is found, this fallback selects the last matching cgroup without applying the host-netns filter. In that case the iptables command can run in the daemon/node network namespace and affect unrelated pods; fail when the target netns cannot be identified instead of bypassing the safety check.
if [ -z "$netns" ]; then
netns=$(grep -l "$1" /proc/[0-9]*/cgroup 2>/dev/null | tail -n1)
netns=${netns%/cgroup}/ns/net
fi
e2e-tests/functions:692
- All current callers pass only six arguments, so
timeout_secondsdefaults to 0 and this loop never returns a failure when the expected condition is impossible. The previous inline waits were bounded at 480 seconds; use a finite default or pass a timeout from each assertion so a bad condition fails with diagnostics.
local timeout_seconds="${7:-0}"
local interval="${8:-5}"
e2e-tests/functions:2650
vault_tlscreates the CSR asvault-csr-${RANDOM}, so this deterministic delete ofvault-csr-${name}never matches the CSR created for the Vault instance. Repeated test runs therefore leave approved, cluster-scoped CSRs behind; share the generated CSR name with cleanup or use one deterministic name in both functions.
kubectl delete csr "vault-csr-${name}" --ignore-not-found || :
e2e-tests/functions:2645
- The new fixed-namespace Vault helper is not used by
start-from-backup-tde/99-remove-cluster-gracefully.yaml, even though that test callsstart_vault vault-service. A successful run therefore leaves the Helm release and the cluster-scopedvaultnamespace behind, which can pollute or interfere with subsequent tests; addstop_vaultto that test's cleanup.
function stop_vault() {
e2e-tests/tests/cert-manager-tls/18-cert-deletion-recovery.yaml:24
- After deleting the Certificate, the old target Secret is no longer waited out. With certificate owner references enabled, the recreated Certificate can become Ready from the still-present Secret, making
cert_afterequalcert_beforeor masking a failed reissuance. Wait forsecret_nameto be deleted before waiting for the replacement Certificate.
kubectl -n "$NAMESPACE" delete certificate "$cert_name" --wait=true --timeout=120s
retry 6 5 kubectl -n "$NAMESPACE" get certificate "$cert_name"
kubectl -n "$NAMESPACE" wait --for=condition=Ready "certificate/${cert_name}" --timeout=120s
cert_after=$(kubectl -n "$NAMESPACE" get secret "$secret_name" -o jsonpath='{.data.tls\.crt}')
e2e-tests/tests/migration-from-crunchy-pv/07-assert.yaml:40
- This change updates the Percona cluster status expectation to three replicas, but the StatefulSet assertion above still requires
availableReplicas,readyReplicas,replicas, andupdatedReplicasto be 1. After step 5 scales the instance set to three, the assertion will not match the actual StatefulSet status; update those four fields to 3 as well.
readyReplicas: 3
replicas: 3
updatedReplicas: 3
e2e-tests/tests/pg-tde-wal-encrypt/00-deploy-operator.yaml:13
PG_VER=17is rejected by this guard (-le 17), although the new version gating is intended to allow PostgreSQL 17 (the standby suites use-lt 17). A PG17 matrix run therefore fails before deploying the operator; use a< 17gate and skip the whole unsupported test case instead of failing it.
if [[ ${PG_VER} -le 17 ]]; then
echo "ERROR: pg-tde-wal-encrypt requires PostgreSQL greater than 17; got ${PG_VER}"
exit 1
e2e-tests/tests/pg-tde/00-deploy-operator.yaml:13
PG_VER=17is rejected by this guard (-le 17), although the new version gating is intended to allow PostgreSQL 17 (the standby suites use-lt 17). A PG17 matrix run therefore fails before deploying the operator; use a< 17gate and skip the whole unsupported test case instead of failing it.
if [[ ${PG_VER} -le 17 ]]; then
echo "ERROR: pg-tde requires PostgreSQL greater than 17; got ${PG_VER}"
exit 1
e2e-tests/tests/standby-pgbackrest/13-assert.yaml:16
- The verification step still creates
13-verify-wal-encryption-standby, but this refactor no longer asserts it. The pgBackRest standby test can now pass without checking WAL encryption, which was covered by the previous manifest.
e2e-tests/tests/standby-streaming/13-assert.yaml:16 - This asserts a ConfigMap that
13-verify-encryption-standby.yamlnever creates; that step checkspg_tde.wal_encryptdirectly instead. Every PostgreSQL 17+ run will therefore fail with a missing13-verify-wal-encryption-standbyConfigMap. Remove this assertion or create the ConfigMap in the verification step.
e2e-tests/tests/cert-manager-tls/18-cert-deletion-recovery.yaml:21 - Certificate recreation is retried only six times with a five-second delay, so a replacement Certificate must appear within roughly 25 seconds. The previous test allowed up to ten minutes, and the operator/cert-manager reconciliation can exceed this window even though the step has a 900-second timeout; restore a longer retry window.
kubectl -n "$NAMESPACE" delete certificate "$cert_name" --wait=true --timeout=120s
retry 6 5 kubectl -n "$NAMESPACE" get certificate "$cert_name"
kubectl -n "$NAMESPACE" wait --for=condition=Ready "certificate/${cert_name}" --timeout=120s
e2e-tests/tests/start-from-backup-tde/00-deploy-operator.yaml:14
- For PostgreSQL versions below 17 this branch exits with status 1, so KUTTL reports a failed test rather than skipping it. Because later steps are still present, changing only this to
exit 0would also let them run; gate the entire suite/test selection or add zero-exit guards to every step.
valmiranogueira
force-pushed
the
K8SPG-1013
branch
from
August 20, 2026 23:16
223651d to
92a4267
Compare
valmiranogueira
force-pushed
the
K8SPG-1013
branch
from
August 20, 2026 23:32
92a4267 to
0aabf04
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHANGE DESCRIPTION
Problem:
AKS:
Openshift:
Additional changes:
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability