Skip to content

test(ci): fedora helm PostgreSQL 15 to 18 dump/restore upgrade path#5141

Draft
zdrapela wants to merge 12 commits into
redhat-developer:mainfrom
zdrapela:test/pg15-to-pg18-helm-upgrade-fedora
Draft

test(ci): fedora helm PostgreSQL 15 to 18 dump/restore upgrade path#5141
zdrapela wants to merge 12 commits into
redhat-developer:mainfrom
zdrapela:test/pg15-to-pg18-helm-upgrade-fedora

Conversation

@zdrapela

@zdrapela zdrapela commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

Companion to #5139 (RHEL POSTGRESQL_UPGRADE=copy 15→16→18) for RHIDP-14594.

This PR exercises the Fedora / community chart-managed PostgreSQL path:

PG15 → (CI verification) → pg_dumpall / wipe PVC / restore → PG18 → (CI verification)

  • Images: quay.io/fedora/postgresql-15quay.io/fedora/postgresql-18
  • Values: .ci/pipelines/value_files/values_showcase_15.yamlvalues_showcase_18.yaml
  • Orchestration: .ci/pipelines/jobs/ocp-pull.sh (handle_ocp_pull) + .ci/pipelines/lib/postgres.sh
  • Artifacts: showcase-pg15, showcase-pg18 (pod logs + Postgres diagnostics per phase)

Why dump/restore (and why skip PG16)

Upstream does not support in-container upgrades on Fedora (sclorg/postgresql-container@b86ea1a; upgrade tests only cover RHEL/c9s/c10s). The image’s POSTGRESQL_UPGRADE=copy|hardlink flow (container README — Upgrading Database) needs matching previous-version binaries inside the image.

In practice for Fedora images today:

  • quay.io/fedora/postgresql-18 advertises POSTGRESQL_PREV_VERSION=16 but ships postgresql-17 bins (no clean 16→18 hop)
  • There is no quay.io/fedora/postgresql-17 image to bridge majors
  • Logical dump/restore can jump 15 → 18 in one maintenance window (see PostgreSQL — Upgrading a PostgreSQL Cluster, especially Upgrading Data via pg_dumpall)

Product/RHEL path remains the two-hop copy upgrade in #5139.


Documentation A — General guidance (operators / docs)

Use this for product/community docs. Do not restate the full PostgreSQL upgrade procedure — follow upstream and only call out RHDH / OpenShift specifics.

Upstream procedure (source of truth)

Major-version upgrades that move data with a logical dump are documented here:

Also useful:

When this RHDH guidance applies

  • RHDH deployed with the Helm chart and embedded PostgreSQL (postgresql.enabled: true)
  • Postgres image family is Fedora (quay.io/fedora/postgresql-*), not RHEL
  • In-place POSTGRESQL_UPGRADE=copy is not reliable (see Summary)

RHDH / OpenShift delta (on top of upstream dump/restore)

Map the upstream dump/restore steps onto the chart-managed DB as follows:

  1. Downtime — Schedule a maintenance window; Backstage must not write while the volume is replaced.
  2. Baseline — Record SHOW server_version and the current Postgres container image.
  3. Dump — From the running PG15 pod, run pg_dumpall (logical backup of all DBs/roles). Keep the dump until verification succeeds.
    Note: Upstream prefers dumping with the newer client when both majors can run in parallel. With a single PVC this path dumps from the old pod before wipe.
  4. Replace the data volume — Delete the PostgreSQL StatefulSet and its PVC so the next Helm install can initdb a fresh PG18 datadir. Keep the Postgres password Secret so RHDH credentials stay valid.
  5. Install PG18 — Helm upgrade/install with postgresql.imagequay.io/fedora/postgresql-18. Do not set POSTGRESQL_UPGRADE (this is not a copy upgrade).
  6. Restore — Load the dump into the Ready PG18 pod with psql (see upstream). Image init may already create roles/globals; tolerate benign “already exists” errors, then confirm application databases.
  7. Collation — After the major jump, run ALTER DATABASE … REFRESH COLLATION VERSION on postgres, template1, and user databases (common post-major cleanup; not a substitute for upstream migration notes).
  8. Reconnect RHDH — Restart the developer-hub Deployment and wait until the previous pod UID is gone and a new Ready pod exists before trusting the instance.
  9. Verify — Confirm Postgres reports 18.x / fedora/postgresql-18, then run your normal RHDH smoke checks.

What not to do on Fedora

  • Do not rely on POSTGRESQL_UPGRADE=copy / hardlink for this Fedora jump.
  • Do not require a PG16 intermediate hop; dump/restore can go 15→18 directly.
  • Do not delete the Postgres password Secret when wiping the PVC unless you also reconfigure RHDH.

Documentation B — Exact process (as implemented in this PR)

Assumptions match CI: OpenShift, Helm release rhdh, namespace showcase, values under .ci/pipelines/value_files/.

CI also seeds a catalog entity and runs Playwright before/after the cutover. That is test evidence for this PR, not part of the upgrade procedure in Documentation A.

Flow (CI)

helm install PG15 (fedora/postgresql-15)
  → wait Ready + Backstage healthy
  → [CI only] seed catalog proof + Playwright (showcase-pg15)
  → capture RHDH pod UID
  → oc exec … pg_dumpall -U postgres  →  DUMPFILE
  → delete Postgres StatefulSet + PVC
  → helm upgrade -i … values_showcase_18.yaml (fedora/postgresql-18)
  → wait Ready (image contains postgresql-18)
  → oc exec -i … psql -U postgres  < DUMPFILE
  → ALTER DATABASE … REFRESH COLLATION VERSION (postgres, template1, user DBs)
  → oc rollout restart deployment/rhdh-developer-hub
  → wait previous RHDH pod UID terminated + new Ready pod
  → [CI only] API + Playwright proof (showcase-pg18)

Complete upgrade script (mirrors CI helpers)

Required env (fail fast if unset):

Variable Purpose
HELM_CHART_URL Chart repo/URL used by helm::install
CHART_VERSION Chart version
CLUSTER_ROUTER_BASE global.clusterRouterBase
IMAGE_REGISTRY / IMAGE_REPO / TAG_NAME RHDH container --set flags (helm::get_image_params)

Optional: NS (default showcase), RELEASE (default rhdh), DUMPFILE, VALUES_DIR, CATALOG_INDEX_*.

#!/usr/bin/env bash
# Fedora chart-managed Postgres: logical upgrade 15 → 18 (dump/restore).
# Mirrors handle_ocp_pull + postgres_* helpers in this PR (upgrade steps only).
set -euo pipefail

NS="${NS:-showcase}"
RELEASE="${RELEASE:-rhdh}"
DUMPFILE="${DUMPFILE:-/tmp/rhdh-pg15-dumpall.sql}"
VALUES_DIR="${VALUES_DIR:-.ci/pipelines/value_files}"
DEPLOY="${RELEASE}-developer-hub"

: "${HELM_CHART_URL:?set HELM_CHART_URL}"
: "${CHART_VERSION:?set CHART_VERSION}"
: "${CLUSTER_ROUTER_BASE:?set CLUSTER_ROUTER_BASE}"
: "${IMAGE_REGISTRY:?set IMAGE_REGISTRY}"
: "${IMAGE_REPO:?set IMAGE_REPO}"
: "${TAG_NAME:?set TAG_NAME}"

pg_pod() {
  oc get pods -n "${NS}" -l "app.kubernetes.io/name=postgresql" \
    -o jsonpath='{.items[0].metadata.name}'
}

wait_pg_ready() {
  local want_substr="${1:-}"
  local max="${2:-600}"
  local waited=0
  while (( waited < max )); do
    local pod phase ready image sts_ready
    pod=$(pg_pod 2>/dev/null || true)
    if [[ -n "${pod}" ]]; then
      phase=$(oc get pod -n "${NS}" "${pod}" -o jsonpath='{.status.phase}' 2>/dev/null || true)
      ready=$(oc get pod -n "${NS}" "${pod}" -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}' 2>/dev/null || true)
      image=$(oc get pod -n "${NS}" "${pod}" -o jsonpath='{.spec.containers[0].image}' 2>/dev/null || true)
      sts_ready=$(oc get statefulset -n "${NS}" -l "app.kubernetes.io/name=postgresql" \
        -o jsonpath='{.items[0].status.readyReplicas}' 2>/dev/null || true)
      if { [[ -z "${want_substr}" ]] || [[ "${image}" == *"${want_substr}"* ]]; } \
        && [[ "${phase}" == "Running" && "${ready}" == "True" && "${sts_ready:-0}" -ge 1 ]]; then
        echo "${pod}"
        return 0
      fi
    fi
    sleep 10
    waited=$((waited + 10))
  done
  echo "PostgreSQL not Ready (want image *${want_substr}*)" >&2
  return 1
}

get_rhdh_pod_uid() {
  local name uid
  while IFS=$'\t' read -r name uid; do
    if [[ -n "${name}" && "${name}" == "${DEPLOY}"* && -n "${uid}" ]]; then
      echo "${uid}"
      return 0
    fi
  done < <(oc get pods -n "${NS}" --field-selector=status.phase=Running \
    -o jsonpath='{range .items[*]}{.metadata.name}{"\t"}{.metadata.uid}{"\n"}{end}' 2>/dev/null || true)
}

wait_for_rhdh_pod_replaced() {
  local previous_uid=$1
  local max=${2:-600}
  local waited=0
  if [[ -z "${previous_uid}" ]]; then
    echo "No previous RHDH uid; skipping pod-replacement wait" >&2
    return 0
  fi
  while (( waited < max )); do
    local uids uid_still_present=false
    uids=$(oc get pods -n "${NS}" -o jsonpath='{range .items[*]}{.metadata.uid}{"\n"}{end}' 2>/dev/null || true)
    grep -qx "${previous_uid}" <<< "${uids}" && uid_still_present=true

    local new_name="" new_uid="" phase="" ready=""
    while IFS=$'\t' read -r new_name new_uid phase ready; do
      if [[ -n "${new_name}" && "${new_name}" == "${DEPLOY}"* ]]; then
        break
      fi
      new_name=""; new_uid=""
    done < <(oc get pods -n "${NS}" \
      -o jsonpath='{range .items[*]}{.metadata.name}{"\t"}{.metadata.uid}{"\t"}{.status.phase}{"\t"}{.status.conditions[?(@.type=="Ready")].status}{"\n"}{end}' 2>/dev/null || true)

    if [[ "${uid_still_present}" != "true" && -n "${new_uid}" && "${new_uid}" != "${previous_uid}" \
      && "${phase}" == "Running" && "${ready}" == "True" ]]; then
      echo "New RHDH pod Ready: ${new_name} uid=${new_uid} (previous ${previous_uid} terminated)"
      return 0
    fi
    sleep 5
    waited=$((waited + 5))
  done
  echo "Timed out waiting for RHDH pod replacement (previous uid=${previous_uid})" >&2
  return 1
}

helm_image_params() {
  local params=(
    --set "upstream.backstage.image.registry=${IMAGE_REGISTRY}"
    --set "upstream.backstage.image.repository=${IMAGE_REPO}"
    --set "upstream.backstage.image.tag=${TAG_NAME}"
  )
  if [[ -n "${CATALOG_INDEX_IMAGE:-}" ]]; then
    params+=(
      --set "global.catalogIndex.image.registry=${CATALOG_INDEX_REGISTRY}"
      --set "global.catalogIndex.image.repository=${CATALOG_INDEX_REPO}"
      --set "global.catalogIndex.image.tag=${CATALOG_INDEX_TAG}"
    )
  fi
  printf '%q ' "${params[@]}"
}

echo "== Baseline PG15 =="
POD=$(wait_pg_ready "postgresql-15" 600)
oc exec -n "${NS}" "${POD}" -- psql -U postgres -c "SHOW server_version;"
oc get pod -n "${NS}" "${POD}" -o jsonpath='image={.spec.containers[0].image}{"\n"}'

RHDH_UID=$(get_rhdh_pod_uid)
echo "RHDH pod uid before upgrade: ${RHDH_UID:-<none>}"

echo "== Dump all databases from PG15 (postgres_dumpall_to_file) =="
mkdir -p "$(dirname "${DUMPFILE}")"
oc exec -n "${NS}" "${POD}" -- pg_dumpall -U postgres > "${DUMPFILE}"
BYTES=$(wc -c < "${DUMPFILE}" | tr -d ' ')
echo "dump size: ${BYTES} bytes"
[[ "${BYTES}" -gt 0 ]] || { echo "Empty dump; aborting" >&2; exit 1; }

echo "== Wipe Postgres StatefulSet + PVC (postgres_wipe_persistent_volume) =="
oc delete statefulset -n "${NS}" -l "app.kubernetes.io/name=postgresql" --wait=true --timeout=5m || true
oc delete pod -n "${NS}" -l "app.kubernetes.io/name=postgresql" --force --grace-period=0 || true
PVC=$(oc get pvc -n "${NS}" -o name 2>/dev/null | grep -i postgres | head -1 || true)
if [[ -n "${PVC}" ]]; then
  oc delete -n "${NS}" "${PVC}" --wait=true --timeout=5m
else
  echo "Warn: no postgres PVC found" >&2
fi

echo "== Helm upgrade to fedora/postgresql-18 (helm::install values_showcase_18.yaml) =="
# shellcheck disable=SC2046
helm upgrade -i "${RELEASE}" -n "${NS}" \
  "${HELM_CHART_URL}" --version "${CHART_VERSION}" \
  -f "${VALUES_DIR}/values_showcase_18.yaml" \
  --set "global.clusterRouterBase=${CLUSTER_ROUTER_BASE}" \
  $(helm_image_params)

echo "== Wait for PG18 Ready =="
POD=$(wait_pg_ready "postgresql-18" 600)
oc exec -n "${NS}" "${POD}" -- psql -U postgres -c "SHOW server_version;"

echo "== Restore dump (postgres_restore_dumpall_file) =="
oc exec -i -n "${NS}" "${POD}" -- psql -U postgres -v ON_ERROR_STOP=0 < "${DUMPFILE}"

echo "== Refresh collation versions (refresh_postgres_collation_versions) =="
DBS=$(oc exec -n "${NS}" "${POD}" -- psql -U postgres -t -A -c \
  "SELECT datname FROM pg_database WHERE datistemplate = false AND datname NOT IN ('postgres');" 2>/dev/null \
  | grep -E '^[A-Za-z0-9_.:-]+$' || true)
for db in postgres template1 ${DBS}; do
  [[ -z "${db}" ]] && continue
  oc exec -n "${NS}" "${POD}" -- psql -U postgres -v ON_ERROR_STOP=1 -c \
    "ALTER DATABASE \"${db}\" REFRESH COLLATION VERSION;" \
    || echo "Warn: collation refresh failed for ${db}" >&2
done

echo "== Restart RHDH and wait for previous pod UID to terminate =="
oc rollout restart "deployment/${DEPLOY}" -n "${NS}" || true
current=$(get_rhdh_pod_uid || true)
if [[ -n "${RHDH_UID}" && "${current}" == "${RHDH_UID}" ]]; then
  echo "UID unchanged after restart request; waiting for replacement…"
fi
wait_for_rhdh_pod_replaced "${RHDH_UID}" 600
oc rollout status "deployment/${DEPLOY}" -n "${NS}" --timeout=10m

echo "== Verify Postgres major =="
oc exec -n "${NS}" "$(pg_pod)" -- psql -U postgres -t -A -c "SHOW server_version;"
oc get pod -n "${NS}" "$(pg_pod)" -o jsonpath='image={.spec.containers[0].image}{"\n"}'
echo "Done. Retain ${DUMPFILE} until application verification passes."

CI entrypoints (source of truth in this PR)

Step Code
Overall sequence handle_ocp_pull in .ci/pipelines/jobs/ocp-pull.sh
Wait Ready + image substr wait_for_postgres_ready
pg_dumpall postgres_dumpall_to_file
Wipe STS/PVC postgres_wipe_persistent_volume
Helm install helm::install + helm::get_image_params
Restore postgres_restore_dumpall_file
Collation refresh_postgres_collation_versions
Pod certainty get_rhdh_pod_uid / ensure_rhdh_pod_replaced / wait_for_rhdh_pod_replaced
CI data proof (not in upgrade script) seed_pg_upgrade_data_proof / assert_pg_upgrade_data_proof_api
CI UI proof e2e-tests/playwright/e2e/pg-upgrade-data-proof.spec.ts (PG_UPGRADE_DATA_PROOF=1)

Values delta (Postgres image only)

# values_showcase_15.yaml
postgresql:
  image:
    registry: quay.io
    repository: fedora/postgresql-15
    tag: latest

# values_showcase_18.yaml
postgresql:
  image:
    registry: quay.io
    repository: fedora/postgresql-18
    tag: latest

No POSTGRESQL_UPGRADE env is set on the Fedora path.

CI-only persistence proof (not required for the upgrade itself)

Used in this PR to prove catalog rows survive dump/restore:

  1. Seed (PG15): POST /api/catalog/locations with a public GitHub blob URL to .ci/pipelines/resources/pg-upgrade/catalog-info.yaml
  2. After restore (PG18): GET …/entities/by-name/component/default/pg-upgrade-data-proof still contains RHIDP-14594
  3. UI: Playwright → Catalog → Component → PG Upgrade Data Proof

Test plan

  • /test e2e-ocp-helm — PG15 and PG18 Playwright phases green
  • Artifacts under showcase-pg15 and showcase-pg18
  • Version evidence: Fedora 15.x → 18.x (no PG16 hop)
  • Previous RHDH pod UID terminated before post-upgrade Playwright
  • Catalog persistence proof (API + UI) after dump/restore

Related

zdrapela added 7 commits July 22, 2026 21:19
Validate the sclorg-supported two-hop path (POSTGRESQL_UPGRADE=copy)
for RHIDP-14594 evidence on e2e-ocp-helm.
Avoid racing Playwright against a recreating postgresql pod after
removing POSTGRESQL_UPGRADE. Log server_version for upgrade evidence.
Start from fedora/postgresql-15 (not rhel9 default), require the
expected image before Ready, raise upgrade timeouts, enlarge PVC for
copy mode, and dump pod logs when a hop stalls.
Command substitution was swallowing oc describe/logs on hop failure, and
psql warnings were being treated as database names during collation refresh.
fedora/postgresql-18 advertises PREV_VERSION=16 but only ships
postgresql-17 binaries, so POSTGRESQL_UPGRADE=copy from 16 never becomes Ready.
Exercise PG15 → tests → PG16 → tests → PG18 → tests, and always persist
pod logs plus Playwright output under unique ARTIFACT_DIR subdirs per hop.
Use fedora/postgresql-* images. Keep POSTGRESQL_UPGRADE=copy for 15→16;
use dump/restore for 16→18 because fedora/postgresql-18 lacks PG16 bins.
Persist unique ARTIFACT_DIR outputs after each major.
@zdrapela

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

1 similar comment
@zdrapela

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.96%. Comparing base (7d73af5) to head (d5feae8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5141      +/-   ##
==========================================
- Coverage   63.69%   59.96%   -3.74%     
==========================================
  Files         123      111      -12     
  Lines        2424     2198     -226     
  Branches      573      547      -26     
==========================================
- Hits         1544     1318     -226     
  Misses        878      878              
  Partials        2        2              
Flag Coverage Δ
rhdh 59.96% <ø> (-3.74%) ⬇️
Components Coverage Δ
Backend plugins ∅ <ø> (∅)
Backend app 66.66% <ø> (ø)
Frontend app 58.89% <ø> (ø)
Plugin utils ∅ <ø> (∅)
Dynamic plugins utils ∅ <ø> (∅)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d73af5...d5feae8. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jul 23, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 🔗 Cross-repo conflicts (1) 📜 Skill insights (0)

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 46 rules
✅ Cross-repo context
  Explored: repo: redhat-developer/rhdh-chart (sha: e476f298)
  Not relevant to this PR: redhat-developer/rhdh-plugins
  Not relevant to this PR: redhat-developer/rhdh-operator
  Not relevant to this PR: redhat-developer/rhdh-local

Grey Divider


Action required

1. Hardcoded externalAccess auth token ✓ Resolved 📜 Skill insight ⛨ Security
Description
New Helm values files hardcode backend.auth.externalAccess.options.token/subject
(test-token/test-subject), which are credentials committed in source. This violates the
requirement to avoid hardcoded secrets/credentials and instead source them from secrets or
environment variables.
Code

.ci/pipelines/value_files/values_showcase_15.yaml[R66-70]

+          externalAccess:
+            - type: static
+              options:
+                token: test-token
+                subject: test-subject
Relevance

⭐⭐ Medium

No repo history on rejecting/accepting hardcoded “test-token” values in CI Helm values files.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist forbids committing hardcoded secrets/credentials in code. The added values files set a
static external access token and subject directly in YAML (test-token, test-subject), which
are credential-like values committed to the repo rather than being pulled from a secret source.

Rule 2137: Secrets must not be hardcoded in source files
.ci/pipelines/value_files/values_showcase_15.yaml[64-70]
.ci/pipelines/value_files/values_showcase_16.yaml[64-70]
.ci/pipelines/value_files/values_showcase_16_upgrade.yaml[63-70]
.ci/pipelines/value_files/values_showcase_18.yaml[64-70]
.ci/pipelines/value_files/values_showcase_18_upgrade.yaml[63-70]
Skill: e2e-verify-fix

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Several newly added CI Helm values files hardcode an auth `token` and `subject` under `upstream.backstage.appConfig.backend.auth.externalAccess`. Even if intended for CI, these are credentials committed to the repository and must be sourced from a Secret or environment variable instead.

## Issue Context
The values files are used by CI jobs to deploy Backstage with static external access enabled; hardcoding a token/subject in git violates the repository secret/credential policy.

## Fix Focus Areas
- .ci/pipelines/value_files/values_showcase_15.yaml[66-70]
- .ci/pipelines/value_files/values_showcase_16.yaml[66-70]
- .ci/pipelines/value_files/values_showcase_16_upgrade.yaml[66-70]
- .ci/pipelines/value_files/values_showcase_18.yaml[66-70]
- .ci/pipelines/value_files/values_showcase_18_upgrade.yaml[66-70]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unsafe PVC wipe logic 🐞 Bug ☼ Reliability
Description
postgres_wipe_persistent_volume deletes the first PVC whose name matches 'postgres' and suppresses
all deletion errors, so the PG16→PG18 dump/restore may proceed without a real wipe or may delete an
unintended PVC. This can make the upgrade test flaky or destructive within the namespace.
Code

.ci/pipelines/lib/postgres.sh[R188-203]

+postgres_wipe_persistent_volume() {
+  local namespace=$1
+
+  log::info "Wiping PostgreSQL StatefulSet + PVC in ${namespace} for dump/restore upgrade"
+  oc delete statefulset -n "${namespace}" -l "app.kubernetes.io/name=postgresql" --wait=true --timeout=5m 2>&1 || true
+  oc delete pod -n "${namespace}" -l "app.kubernetes.io/name=postgresql" --force --grace-period=0 2>&1 || true
+
+  local pvc
+  pvc=$(oc get pvc -n "${namespace}" -o name 2> /dev/null | grep -i postgres | head -1 || true)
+  if [[ -n "${pvc}" ]]; then
+    log::info "Deleting ${pvc}"
+    oc delete -n "${namespace}" "${pvc}" --wait=true --timeout=5m 2>&1 || true
+  else
+    log::warn "No postgres PVC found to delete"
+  fi
+}
Relevance

⭐⭐ Medium

No historical evidence on stricter PVC selection/error handling in CI cleanup; could be seen as
acceptable best-effort.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The wipe function chooses a PVC by a name substring + first match and suppresses deletion failures.
The caller proceeds immediately to install PG18 and restore, without verifying the wipe succeeded.

.ci/pipelines/lib/postgres.sh[185-203]
.ci/pipelines/jobs/ocp-pull.sh[120-123]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`postgres_wipe_persistent_volume` uses `oc get pvc ... | grep -i postgres | head -1` and ignores failures for StatefulSet/pod/PVC deletions. That makes it possible to delete the wrong PVC and/or continue even when the intended PVC was not actually removed.

### Issue Context
This function is used to enforce a clean initdb before restoring into PG18.

### Fix Focus Areas
- .ci/pipelines/lib/postgres.sh[188-203]
- .ci/pipelines/jobs/ocp-pull.sh[120-123]

### Suggested fix
- Identify the PVC(s) to delete via labels/ownership rather than substring match. Options:
 - Use a label selector aligned with the chart (e.g. `app.kubernetes.io/name=postgresql` and `app.kubernetes.io/instance=${RELEASE_NAME}`), and require exactly 1 match.
 - Or derive the PVC name from the PostgreSQL StatefulSet name and its volumeClaimTemplates.
- Do not blanket `|| true` destructive operations:
 - Return non-zero if the StatefulSet/PVC delete fails or times out.
 - After delete, poll until the PVC is truly gone before continuing.
- In `ocp-pull.sh`, check the return code and abort the hop if wipe fails (don’t proceed to `helm::install` + restore).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. DB dump leaked artifacts ✓ Resolved 🐞 Bug ⛨ Security
Description
handle_ocp_pull uploads a full pg_dumpall output into ARTIFACT_DIR, which can expose database
contents and role/password hashes to anyone with access to CI artifacts. This materially increases
risk if the test DB ever contains tokens/secrets or sensitive fixtures.
Code

.ci/pipelines/jobs/ocp-pull.sh[R112-119]

+  log::info "Phase PG18: dump/restore 16 → 18 (fedora copy upgrade unsupported)"
+  local dumpfile="/tmp/rhdh-pg16-dumpall.sql"
+  if ! postgres_dumpall_to_file "${NAME_SPACE}" "${dumpfile}"; then
+    _pg_save_phase_artifacts "${art_pg18}" "PG18 dump failed"
+    return 1
+  fi
+  common::save_artifact "${art_pg18}" "${dumpfile}" "postgres" || true
+
Relevance

⭐⭐ Medium

No close historical evidence on publishing pg_dumpall to artifacts; team sometimes rejects CI
hardening changes (PR #4798).

PR-#4798

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR writes a full cluster dump to a local file and then unconditionally rsyncs it into
ARTIFACT_DIR via common::save_artifact, which is the standard mechanism used throughout these
pipelines for publishing CI artifacts.

.ci/pipelines/jobs/ocp-pull.sh[112-119]
.ci/pipelines/lib/postgres.sh[162-183]
.ci/pipelines/lib/common.sh[205-227]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The PG16→PG18 path saves `/tmp/rhdh-pg16-dumpall.sql` as a CI artifact, which can leak DB contents (including globals like role/password hashes).

### Issue Context
This dump is only needed transiently to restore into PG18; it does not need to be published externally.

### Fix Focus Areas
- .ci/pipelines/jobs/ocp-pull.sh[112-119]

### Suggested fix
- Do **not** call `common::save_artifact` on the raw dump file on the success path.
- If debug retention is needed, gate it behind an opt-in env var (e.g. `SAVE_DB_DUMP_ARTIFACT=true`) and/or redact globals:
 - Prefer `pg_dumpall --no-role-passwords` (if supported) or avoid dumping globals entirely (use `pg_dump` per DB instead of `pg_dumpall`), and/or filter out `CREATE ROLE/ALTER ROLE ... PASSWORD` lines before saving.
- Ensure any retained dump is stored in a restricted location (if your CI supports private artifacts).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. RBAC pull coverage removed 🐞 Bug ☼ Reliability
Description
handle_ocp_pull no longer deploys or runs Playwright against the RBAC instance, reducing presubmit
CI coverage and allowing RBAC regressions to merge undetected until nightly. This is a behavioral
change in the /test e2e-ocp-helm workflow.
Code

.ci/pipelines/jobs/ocp-pull.sh[R75-77]

handle_ocp_pull() {
  export NAME_SPACE="${NAME_SPACE:-showcase}"
  export NAME_SPACE_RBAC="${NAME_SPACE_RBAC:-showcase-rbac}"
Relevance

⭐⭐⭐ High

Team invests in RBAC E2E stability/coverage; RBAC-related CI/test fixes accepted previously (e.g.,
PR #4526).

PR-#4526

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The pull handler exports RBAC-related namespaces but only runs base_deployment and then runs
Playwright for the base URL; nightly still runs both base and RBAC via initiate_deployments and
testing::check_and_test for each.

.ci/pipelines/jobs/ocp-pull.sh[75-110]
.ci/pipelines/jobs/ocp-nightly.sh[26-47]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The pull job now only runs the base (showcase) deployment/tests and does not exercise the RBAC deployment at all.

### Issue Context
Nightly still runs both base + RBAC, but presubmit is typically where developers rely on fast regression detection.

### Fix Focus Areas
- .ci/pipelines/jobs/ocp-pull.sh[75-98]
- .ci/pipelines/jobs/ocp-nightly.sh[26-46]

### Suggested fix
- Re-introduce RBAC deployment + Playwright execution in `handle_ocp_pull`, either:
 - Run RBAC once (baseline) in parallel with the PG15 baseline phase, and keep the PG upgrade sequence scoped to the base namespace only; or
 - Split this PG upgrade-path scenario into a dedicated job script so the standard pull job can continue to validate both base and RBAC.
- If intentionally removed, explicitly document that presubmit no longer covers RBAC and ensure an alternative presubmit exists.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Postgres secret override bypass 🔗 Cross-repo conflict ☼ Reliability
Description
The new CI values files hardcode POSTGRESQL_ADMIN_PASSWORD’s secretKeyRef (`{{ .Release.Name
}}-postgresql / postgres-password`) instead of using rhdh-chart’s helper-based secret indirection,
so they won’t work with chart-supported overrides like global.postgresql.auth.existingSecret /
secretKeys and may drift as rhdh-chart evolves. This repo’s CI values are a consumer of rhdh-chart
and should follow the same helper-based contract the chart defaults use for this env var.
Code

.ci/pipelines/value_files/values_showcase_15.yaml[R80-84]

+      - name: POSTGRESQL_ADMIN_PASSWORD
+        valueFrom:
+          secretKeyRef:
+            key: postgres-password
+            name: "{{ .Release.Name }}-postgresql"
Relevance

⭐⭐ Medium

No historical evidence that CI values must follow rhdh-chart helper indirection for secret
overrides.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR hardcodes the secret name/key for POSTGRESQL_ADMIN_PASSWORD, while rhdh-chart defaults wire
this env var through helper templates that support secret overrides, and the upstream Backstage
chart renders extraEnvVars via a tpl-aware renderer (so helper expressions are the intended
contract).

.ci/pipelines/value_files/values_showcase_15.yaml[74-85]
External repo: redhat-developer/rhdh-chart, charts/backstage/values.yaml [283-294]
External repo: redhat-developer/rhdh-chart, charts/backstage/templates/_helpers.tpl [25-51]
External repo: redhat-developer/rhdh-chart, charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml [206-235]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CI Helm values files hardcode the Backstage env var `POSTGRESQL_ADMIN_PASSWORD` to read from a specific Secret name/key. In rhdh-chart, this env var is intentionally wired through helper templates so installs remain compatible when the PostgreSQL Secret name/key is overridden (e.g., via `global.postgresql.auth.existingSecret` or secretKeys changes).

## Issue Context
- rhdh-chart’s default values set `upstream.backstage.extraEnvVars` for `POSTGRESQL_ADMIN_PASSWORD` using `{{- include "rhdh.postgresql.secretName" . }}` and `{{- include "rhdh.postgresql.adminPasswordKey" . }}`.
- The upstream Backstage chart renders `backstage.extraEnvVars` through a tpl-aware renderer, so helper expressions inside values are supported.
- The new CI values (`values_showcase_15.yaml`, `values_showcase_16.yaml`, `values_showcase_18.yaml`) override this with a hardcoded Secret convention.

## Fix Focus Areas
- .ci/pipelines/value_files/values_showcase_15.yaml[74-85]
- .ci/pipelines/value_files/values_showcase_16.yaml[74-85]
- .ci/pipelines/value_files/values_showcase_18.yaml[74-85]

## What to change
In each values file, replace the hardcoded `secretKeyRef` for `POSTGRESQL_ADMIN_PASSWORD` with the same helper-based references used by rhdh-chart defaults, e.g.:

```yaml
- name: POSTGRESQL_ADMIN_PASSWORD
 valueFrom:
   secretKeyRef:
     key: '{{- include "rhdh.postgresql.adminPasswordKey" . }}'
     name: '{{- include "rhdh.postgresql.secretName" . }}'
```

This keeps CI aligned with rhdh-chart’s supported configuration surface and avoids drift when the chart secret naming/keying is customized.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

6. Hardcoded Helm release name 🐞 Bug ◔ Observability
Description
dump_postgres_diagnostics runs helm get values rhdh instead of using the configured
RELEASE_NAME, so the Helm-values snapshot can be wrong/missing when the release name is
overridden. This reduces the usefulness of the diagnostics output without making it obvious (errors
are suppressed).
Code

.ci/pipelines/lib/postgres.sh[R41-44]

+    oc logs -n "${namespace}" "${pg_pod}" --previous --tail=200 2>&1 || true
+    log::info "Helm values snapshot (postgresql image):"
+    helm get values rhdh -n "${namespace}" 2>&1 | grep -A20 -E 'postgresql:|^  image:' || true
+  } >&2
Relevance

⭐⭐ Medium

No historical evidence found for hardcoded release name vs RELEASE_NAME in diagnostics scripts.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The diagnostics helper explicitly hardcodes rhdh in the Helm command, while the pipeline supports
a configurable RELEASE_NAME (defaulting to rhdh).

.ci/pipelines/lib/postgres.sh[21-45]
.ci/pipelines/env_variables.sh[71-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`dump_postgres_diagnostics` hardcodes the Helm release name (`rhdh`) when capturing values, so it won’t correctly snapshot values for non-default release names.

### Issue Context
`RELEASE_NAME` is set to `rhdh` by default but can be overridden in CI/local runs.

### Fix Focus Areas
- .ci/pipelines/lib/postgres.sh[41-44]
- .ci/pipelines/env_variables.sh[71-83]

### Suggested fix
- Change `helm get values rhdh` to `helm get values "${RELEASE_NAME:-rhdh}"` (or accept an explicit `release_name` argument to `dump_postgres_diagnostics(namespace, release)` and pass it from callers).
- Consider logging a warning if the Helm command fails (instead of silently swallowing it) so missing diagnostics are visible.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Fedora copy upgrades are unsupported; jump straight from postgresql-15
to postgresql-18 via pg_dumpall / wipe PVC / restore.
@zdrapela

Copy link
Copy Markdown
Member Author

Updated: skip PG16 entirely — Fedora dump/restore 15 → 18 directly.

/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@zdrapela zdrapela changed the title test(ci): Fedora Helm PostgreSQL 15-16-18 upgrade path test(ci): Fedora Helm PostgreSQL 15→18 dump/restore upgrade path Jul 23, 2026
@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jul 23, 2026

Copy link
Copy Markdown

PR Summary by Qodo

CI: exercise Helm internal PostgreSQL 15→16→18 Fedora upgrade with Playwright per hop

🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add an e2e-ocp-helm CI flow to validate PG15→PG16→PG18 upgrade behavior.
• Run Playwright after each major and store per-hop artifacts and pod logs.
• Add PostgreSQL diagnostics, readiness/image gating, and dump/restore for unsupported copy
 upgrades.
Diagram

graph TD
  CI["ocp-pull.sh"] --> CFG[/"values_showcase_*.yaml"/] --> HELM["helm::install (per hop)"] --> PG[("PostgreSQL StatefulSet/PVC") ] --> LIB["postgres.sh"] --> BS["Backstage deployment"] --> PW["Playwright tests"] --> ART[/"ARTIFACT_DIR per hop"/]
  CI --> LIB
  subgraph Legend
    direction LR
    _proc["Process/Script"] ~~~ _cfg[/"Config/Artifacts"/] ~~~ _db[("Database")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use an intermediate PG17 hop for copy upgrades
  • ➕ Keeps the flow fully chart-native (POSTGRESQL_UPGRADE=copy) across all majors
  • ➕ Avoids PVC wipe and potential dump/restore edge cases
  • ➖ Adds time/cost to CI (extra helm hop + Playwright phase)
  • ➖ Requires maintaining additional values files and verifying Fedora image availability/behavior
2. Standardize upgrade-path testing on RHEL images only
  • ➕ Aligns with supported enterprise base images and expected upgrade tooling
  • ➕ Reduces distro-specific upgrade quirks in CI
  • ➖ Does not provide Fedora-specific evidence requested by RHIDP-14594
  • ➖ May miss regressions unique to Fedora container packaging
3. Build/consume a Fedora PG18 image that includes required prev-version binaries
  • ➕ Restores the intended 16→18 copy-upgrade semantics
  • ➕ Simplifies test flow back to uniform upgrade mechanics
  • ➖ Higher maintenance overhead (custom image pipeline/ownership)
  • ➖ Less representative of upstream Fedora image behavior if the goal is to test it as-is

Recommendation: Given the Fedora PG18 packaging mismatch (PREV_VERSION=16 but missing required binaries), the chosen dump/restore for 16→18 is the most pragmatic way to keep CI coverage moving while still validating application behavior on PG18. If the goal shifts from “evidence of app compatibility” to “evidence of chart copy-upgrade viability,” consider adding a PG17 intermediate hop or switching to images that actually support copy upgrades end-to-end.

Files changed (8) +1162 / -4

Enhancement (2) +230 / -0
postgres.shIntroduce PostgreSQL CI helpers for readiness, diagnostics, and dump/restore +228/-0

Introduce PostgreSQL CI helpers for readiness, diagnostics, and dump/restore

• Adds a new library providing robust Postgres readiness waiting (optionally verifying the running image), server_version logging, and rich diagnostics collection designed not to be swallowed by command substitution. Implements collation refresh after major upgrades, plus pg_dumpall dump/restore and PVC wipe helpers to support non-copy upgrade paths.

.ci/pipelines/lib/postgres.sh

utils.shSource postgres.sh helpers in CI utilities +2/-0

Source postgres.sh helpers in CI utilities

• Wires the new postgres helper library into shared CI utility sourcing so jobs can use readiness/diagnostic/upgrade helpers.

.ci/pipelines/utils.sh

Tests (1) +111 / -4
ocp-pull.shRun PG15→PG16→PG18 upgrade sequence with Playwright per major +111/-4

Run PG15→PG16→PG18 upgrade sequence with Playwright per major

• Adds a multi-phase CI flow that deploys Fedora PostgreSQL 15, upgrades to 16 via POSTGRESQL_UPGRADE=copy, then upgrades to 18 via dump/restore due to unsupported copy-upgrade behavior. Introduces per-hop readiness checks (including expected image gating), version evidence logging, Backstage health gating, and unique artifact subdirectories with pod logs and diagnostics saved for each phase.

.ci/pipelines/jobs/ocp-pull.sh

Other (5) +821 / -0
values_showcase_15.yamlAdd Helm values for Fedora PostgreSQL 15 baseline +163/-0

Add Helm values for Fedora PostgreSQL 15 baseline

• Introduces a values file that pins the chart-managed PostgreSQL image to quay.io/fedora/postgresql-15 and sets persistence sizing for the baseline phase used in the upgrade test flow.

.ci/pipelines/value_files/values_showcase_15.yaml

values_showcase_16.yamlAdd Helm values for Fedora PostgreSQL 16 steady-state +163/-0

Add Helm values for Fedora PostgreSQL 16 steady-state

• Adds a values file pinning internal PostgreSQL to Fedora 16 for the post-upgrade steady-state validation phase.

.ci/pipelines/value_files/values_showcase_16.yaml

values_showcase_16_upgrade.yamlAdd Helm values to trigger 15→16 copy upgrade +165/-0

Add Helm values to trigger 15→16 copy upgrade

• Adds an upgrade-specific values file that sets POSTGRESQL_UPGRADE=copy while pinning the Fedora 16 image, enabling the chart-managed major upgrade from 15 to 16 before switching back to steady-state values.

.ci/pipelines/value_files/values_showcase_16_upgrade.yaml

values_showcase_18.yamlAdd Helm values for Fedora PostgreSQL 18 steady-state +163/-0

Add Helm values for Fedora PostgreSQL 18 steady-state

• Introduces a values file pinning internal PostgreSQL to Fedora 18 for the final validation phase after dump/restore.

.ci/pipelines/value_files/values_showcase_18.yaml

values_showcase_18_upgrade.yamlDocument Fedora 16→18 copy-upgrade values (kept for reference) +167/-0

Document Fedora 16→18 copy-upgrade values (kept for reference)

• Adds a reference upgrade values file for Fedora 18 with POSTGRESQL_UPGRADE=copy, annotated to explain why the CI flow uses dump/restore instead due to Fedora PG18 image limitations.

.ci/pipelines/value_files/values_showcase_18_upgrade.yaml

@zdrapela zdrapela changed the title test(ci): Fedora Helm PostgreSQL 15→18 dump/restore upgrade path test(ci): fedora helm PostgreSQL 15 to 18 dump/restore upgrade path Jul 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Image was built and published successfully. It is available at:

Multi-phase PG upgrade CI left kubectl redis port-forwards bound on 16379
because stop() only signaled the shell parent. Spawn detached, kill the
process group, use direct kubectl args, and fail fast when OVERALL_RESULT
is already non-zero after a phase.
@zdrapela

Copy link
Copy Markdown
Member Author

Fix for multi-phase Playwright: tear down redis port-forward process groups + fail fast when a phase already set OVERALL_RESULT=1.

/test e2e-ocp-helm

@zdrapela

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@zdrapela

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

…grades

Capture the Backstage pod UID before dump/restore, wait until the previous
UID is gone before Playwright, and seed a unique catalog Component on PG15
verified via API + UI after the PG18 restore.
@zdrapela

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

In-cluster http://*.svc targets return 201 for location create but never
ingest entities under Backstage URL reader / SSRF restrictions. Use a
GitHub blob URL at PULL_PULL_SHA instead.
@zdrapela

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

The Catalog table renders metadata.title ("PG Upgrade Data Proof"), so
exact-match on metadata.name never appears after search.
@zdrapela

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@sonarqubecloud

Copy link
Copy Markdown

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant