From d2cfa9e8db5f0f8c24c2e8b7225a2cc757bb5d98 Mon Sep 17 00:00:00 2001 From: shaun-agent Date: Thu, 30 Apr 2026 13:14:24 +0000 Subject: [PATCH 1/2] fix(helm): retain chart PVCs on uninstall Keep Helm-created PVCs by default, add existingClaim support for externally managed PVCs, and document the retention behavior. Supersedes the implementation from PR #166. Co-authored-by: Yen-Ying Lee --- charts/openab/templates/NOTES.txt | 10 +++++++ charts/openab/templates/deployment.yaml | 2 +- charts/openab/templates/pvc.yaml | 4 ++- charts/openab/tests/persistence_test.yaml | 35 +++++++++++++++++++++++ charts/openab/values.yaml | 4 +++ docs/ai-install-upgrade.md | 4 ++- 6 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 charts/openab/tests/persistence_test.yaml diff --git a/charts/openab/templates/NOTES.txt b/charts/openab/templates/NOTES.txt index b18488d7..ce750aa8 100644 --- a/charts/openab/templates/NOTES.txt +++ b/charts/openab/templates/NOTES.txt @@ -46,5 +46,15 @@ Agents deployed: Restart after auth: kubectl rollout restart deployment/{{ include "openab.agentFullname" (dict "ctx" $ "agent" $name) }} +{{- if not (eq (include "openab.persistenceEnabled" $cfg) "false") }} + + Persistence: +{{- if and $cfg.persistence $cfg.persistence.existingClaim }} + using existing PVC {{ $cfg.persistence.existingClaim }} (not managed by this chart) +{{- else }} + PVC {{ include "openab.agentFullname" (dict "ctx" $ "agent" $name) }} is kept on helm uninstall. + To delete stored auth/session data, delete the PVC manually. +{{- end }} +{{- end }} {{- end }} {{- end }} diff --git a/charts/openab/templates/deployment.yaml b/charts/openab/templates/deployment.yaml index 499a5042..4484820b 100644 --- a/charts/openab/templates/deployment.yaml +++ b/charts/openab/templates/deployment.yaml @@ -137,7 +137,7 @@ spec: {{- if $pvcEnabled }} - name: data persistentVolumeClaim: - claimName: {{ include "openab.agentFullname" $d }} + claimName: {{ (and $cfg.persistence $cfg.persistence.existingClaim) | default (include "openab.agentFullname" $d) }} {{- end }} - name: tmp emptyDir: {} diff --git a/charts/openab/templates/pvc.yaml b/charts/openab/templates/pvc.yaml index e771e608..f6984109 100644 --- a/charts/openab/templates/pvc.yaml +++ b/charts/openab/templates/pvc.yaml @@ -1,6 +1,6 @@ {{- range $name, $cfg := .Values.agents }} {{- if ne (include "openab.agentEnabled" $cfg) "false" }} -{{- if not (eq (include "openab.persistenceEnabled" $cfg) "false") }} +{{- if and (not (eq (include "openab.persistenceEnabled" $cfg) "false")) (not (and $cfg.persistence $cfg.persistence.existingClaim)) }} {{- $d := dict "ctx" $ "agent" $name "cfg" $cfg }} --- apiVersion: v1 @@ -9,6 +9,8 @@ metadata: name: {{ include "openab.agentFullname" $d }} labels: {{- include "openab.labels" $d | nindent 4 }} + annotations: + "helm.sh/resource-policy": keep spec: accessModes: - ReadWriteOnce diff --git a/charts/openab/tests/persistence_test.yaml b/charts/openab/tests/persistence_test.yaml new file mode 100644 index 00000000..2d1746be --- /dev/null +++ b/charts/openab/tests/persistence_test.yaml @@ -0,0 +1,35 @@ +suite: persistence PVC rendering +templates: + - templates/pvc.yaml + - templates/deployment.yaml +tests: + - it: keeps chart-created PVCs on helm uninstall + template: templates/pvc.yaml + asserts: + - equal: + path: metadata.annotations["helm.sh/resource-policy"] + value: keep + + - it: mounts the chart-created PVC by default + template: templates/deployment.yaml + asserts: + - equal: + path: spec.template.spec.volumes[1].persistentVolumeClaim.claimName + value: RELEASE-NAME-openab-kiro + + - it: skips PVC creation when existingClaim is set + template: templates/pvc.yaml + set: + agents.kiro.persistence.existingClaim: openab-existing-kiro + asserts: + - hasDocuments: + count: 0 + + - it: mounts existingClaim when set + template: templates/deployment.yaml + set: + agents.kiro.persistence.existingClaim: openab-existing-kiro + asserts: + - equal: + path: spec.template.spec.volumes[1].persistentVolumeClaim.claimName + value: openab-existing-kiro diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index d5bfd0af..6464546d 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -50,6 +50,7 @@ agents: # removeAfterReply: false # persistence: # enabled: true + # existingClaim: "" # set to reuse an existing PVC (skips PVC creation) # storageClass: "" # size: 1Gi # # ⚠️ When set, this ConfigMap mount shadows any file at the same path on the PVC. @@ -82,6 +83,7 @@ agents: # removeAfterReply: false # persistence: # enabled: true + # existingClaim: "" # set to reuse an existing PVC (skips PVC creation) # storageClass: "" # size: 1Gi # agentsMd: "" @@ -111,6 +113,7 @@ agents: # removeAfterReply: false # persistence: # enabled: true + # existingClaim: "" # set to reuse an existing PVC (skips PVC creation) # storageClass: "" # size: 1Gi # image: "ghcr.io/openabdev/openab-cursor:latest" @@ -193,6 +196,7 @@ agents: cronjobs: [] persistence: enabled: true + existingClaim: "" # set to reuse an existing PVC (skips PVC creation) storageClass: "" size: 1Gi # defaults to 1Gi if not set # ⚠️ When set, this ConfigMap mount shadows any file at the same path on the PVC. diff --git a/docs/ai-install-upgrade.md b/docs/ai-install-upgrade.md index 606ad8a8..5278fdb6 100644 --- a/docs/ai-install-upgrade.md +++ b/docs/ai-install-upgrade.md @@ -156,7 +156,7 @@ rollback openab per the upgrade SOP — the upgrade to v0.7.7 failed Step ① Uninstall failed deployment ┌──────────┐ │ helm │──► release gone - │ uninstall│──► delete leftover PVC/secrets + │ uninstall│──► chart PVCs are retained └────┬─────┘ ▼ Step ② Reinstall previous version @@ -182,6 +182,8 @@ rollback openab per the upgrade SOP — the upgrade to v0.7.7 failed └────────────────────────────────────────────── ``` +> **PVC retention:** OpenAB chart-created PVCs are kept on `helm uninstall` to protect auth/session data. Delete a retained PVC manually only when you intentionally want to discard that state. `persistence.existingClaim` PVCs are owned outside the chart and are never created or deleted by OpenAB. + --- ## Quick Reference From 9aa88455325e1c90236356d2a82af0520a0abd32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B6=85=E6=B8=A1=E6=B3=95=E5=B8=AB?= Date: Fri, 1 May 2026 03:59:32 +0000 Subject: [PATCH 2/2] review: add persistence.enabled:false tests, cleanup cmd, upgrade note - Add 2 regression tests for persistence.enabled: false (pvc + deployment) - Add kubectl delete pvc command example to NOTES.txt - Add upgrade path note to docs/ai-install-upgrade.md Addresses review feedback from masami-agent. --- charts/openab/templates/NOTES.txt | 3 ++- charts/openab/tests/persistence_test.yaml | 16 ++++++++++++++++ docs/ai-install-upgrade.md | 2 ++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/charts/openab/templates/NOTES.txt b/charts/openab/templates/NOTES.txt index ce750aa8..4604fbb4 100644 --- a/charts/openab/templates/NOTES.txt +++ b/charts/openab/templates/NOTES.txt @@ -53,7 +53,8 @@ Agents deployed: using existing PVC {{ $cfg.persistence.existingClaim }} (not managed by this chart) {{- else }} PVC {{ include "openab.agentFullname" (dict "ctx" $ "agent" $name) }} is kept on helm uninstall. - To delete stored auth/session data, delete the PVC manually. + To delete stored auth/session data, delete the PVC manually: + kubectl delete pvc {{ include "openab.agentFullname" (dict "ctx" $ "agent" $name) }} {{- end }} {{- end }} {{- end }} diff --git a/charts/openab/tests/persistence_test.yaml b/charts/openab/tests/persistence_test.yaml index 2d1746be..d4bb1b83 100644 --- a/charts/openab/tests/persistence_test.yaml +++ b/charts/openab/tests/persistence_test.yaml @@ -33,3 +33,19 @@ tests: - equal: path: spec.template.spec.volumes[1].persistentVolumeClaim.claimName value: openab-existing-kiro + + - it: skips PVC when persistence is disabled + template: templates/pvc.yaml + set: + agents.kiro.persistence.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: skips data volume mount when persistence is disabled + template: templates/deployment.yaml + set: + agents.kiro.persistence.enabled: false + asserts: + - notExists: + path: spec.template.spec.volumes[1].persistentVolumeClaim diff --git a/docs/ai-install-upgrade.md b/docs/ai-install-upgrade.md index 5278fdb6..e4c6901d 100644 --- a/docs/ai-install-upgrade.md +++ b/docs/ai-install-upgrade.md @@ -183,6 +183,8 @@ rollback openab per the upgrade SOP — the upgrade to v0.7.7 failed ``` > **PVC retention:** OpenAB chart-created PVCs are kept on `helm uninstall` to protect auth/session data. Delete a retained PVC manually only when you intentionally want to discard that state. `persistence.existingClaim` PVCs are owned outside the chart and are never created or deleted by OpenAB. +> +> **Upgrade path:** Existing installations will gain the `helm.sh/resource-policy: keep` annotation on their PVCs upon the next `helm upgrade`. This is an additive-only change — it does not alter runtime behavior and only takes effect on a subsequent `helm uninstall`. ---