Helm Chart: Add existingClaim to persistence

[Maddin-M]

Hello!

This change aims to make it possible for users to provide their own PVC instead of the Chart to create one or not use persistence at all.

Important

I also noticed that the current version of the Helm chart (9.17.0) fails by default, because there is no 9.17.0 pgadmin4 Docker image, only 9.17. I fixed this by hand in my setup, but should be fixed by actually uploading the correct tag the Helm chart is requesting

Thank you

Summary by CodeRabbit

• New Features
  • Added support for using an existing persistent volume claim through the persistence.existingClaim Helm setting.
  • Charts now create a new claim only when persistence is enabled and no existing claim is specified.
• Documentation
  • Documented the new persistence setting and its default behavior.

[coderabbitai]

Walkthrough

The Helm chart adds persistence.existingClaim. When set, the deployment uses that PVC name, and the chart does not render a new PVC resource. Documentation describes the new value.

Changes

Helm persistence configuration

Layer / File(s)	Summary
Existing PVC selection and conditional creation pkg/helm/values.yaml, pkg/helm/templates/pvc.yaml, pkg/helm/templates/deployment.yaml, pkg/helm/README.md	The chart defines persistence.existingClaim with an empty default. The deployment uses the configured claim or the generated fullname. PVC creation is skipped when an existing claim is configured. Documentation describes the value. The change also removes an extra blank line and normalizes a template directive.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: adding existingClaim support to persistence in the Helm chart.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/helm/templates/pvc.yaml`:
- Line 1: Update the PVC template condition around persistence.existingClaim so
an existing-claim migration does not remove the previously generated PVC from
Helm-managed resources; add the chart’s retention or deletion-protection
mechanism for this transition, and cover the upgrade path with a test.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7fa265c8-7169-454d-bd53-bd4fafca299e

📥 Commits

Reviewing files that changed from the base of the PR and between 067f7af and 4679f3c.

📒 Files selected for processing (4)

• pkg/helm/README.md
• pkg/helm/templates/deployment.yaml
• pkg/helm/templates/pvc.yaml
• pkg/helm/values.yaml

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

old_manifest="$(mktemp)"
new_manifest="$(mktemp)"
trap 'rm -f "$old_manifest" "$new_manifest"' EXIT

helm template review pkg/helm \
  --set persistence.enabled=true \
  --set persistence.existingClaim="" >"$old_manifest"

helm template review pkg/helm \
  --set persistence.enabled=true \
  --set persistence.existingClaim=precreated-pgadmin >"$new_manifest"

grep -q '^kind: PersistentVolumeClaim$' "$old_manifest"

if grep -q '^kind: PersistentVolumeClaim$' "$new_manifest"; then
  echo "The existing-claim render still creates a PVC" >&2
  exit 1
fi

echo "The existing-claim configuration removes the generated PVC from the target manifest."

Repository: pgadmin-org/pgadmin4

Length of output: 200

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Locate pvc.yaml and related values:"
fd -a 'pvc\.yaml$|values\.yaml$|README' . | sed 's#^\./##' | head -50

echo
echo "pvc.yaml excerpt:"
if [ -f pkg/helm/templates/pvc.yaml ]; then
  cat -n pkg/helm/templates/pvc.yaml
fi

echo
echo "values snippets:"
for f in $(fd 'values\.ya?ml$' .); do
  echo "--- $f"
  rg -n "persistence|existingClaim|persistence" "$f" -C 3 || true
done

echo
echo "Search for persistence.existingClaim usages:"
rg -n "existingClaim|existingClaimName|retentionPolicy|deleteOn" .

Repository: pgadmin-org/pgadmin4

Length of output: 2336

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Deployment claimName logic:"
cat -n pkg/helm/templates/deployment.yaml | sed -n '55,80p'

echo
echo "helm-specific docs for existingClaim/persistence:"
cat -n pkg/helm/README.md | sed -n '30,45p'

echo
echo "Behavioral probe with the template condition as data:"
python3 - <<'PY'
values = {
    "persistence": {"enabled": True, "existingClaim": ""},
}
old = bool(values["persistence"]["enabled"]) and not values["persistence"]["existingClaim"]
values = {
    "persistence": {"enabled": True, "existingClaim": "precreated-pgadmin"},
}
new = bool(values["persistence"]["enabled"]) and not values["persistence"]["existingClaim"]
print("enabled, existingClaim='':", old)
print("enabled, existingClaim='precreated-pgadmin':", new)
PY

Repository: pgadmin-org/pgadmin4

Length of output: 3203

---

Preserve the generated PVC when migrating to an existing claim.

Setting persistence.existingClaim changes the deployment to claimName: precreated-pgadmin, but the generated PVC block is removed from the manifest by the condition at pkg/helm/templates/pvc.yaml:1. Helm can then delete that chart-managed PVC during the upgrade. Add a retention/migration path or an explicit deletion-protection flag, and cover this upgrade path.

🧰 Tools

🪛 YAMLlint (1.37.1)

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/helm/templates/pvc.yaml` at line 1, Update the PVC template condition
around persistence.existingClaim so an existing-claim migration does not remove
the previously generated PVC from Helm-managed resources; add the chart’s
retention or deletion-protection mechanism for this transition, and cover the
upgrade path with a test.