fix(pvc): prevent PV rebinding on upgrade and fix missing annotations block (#102)

* fix(pvc): prevent PV rebinding on upgrade and fix missing annotations block

Add `helm.sh/resource-policy: keep` unconditionally so the PVC survives
helm uninstall, fix the annotations block which was previously absent when
no custom annotations were set, and use `lookup` to pin `volumeName` on
upgrades to prevent the PVC from being re-bound to a different PV.

Also add some test coverage.

Fixes #74

* Address CodeRabbit review comments.

- Renamed `$volumeName` → `$pvcName` (line 2) — accurately reflects it holds the PVC name
- Extracted `lookup` into `$existingPvc` (line 3) at the top level — called once, outside the `storageClass` guard
- Moved `volumeName` block (lines 32-34) outside the `storageClass` guard — now always evaluated regardless of whether `storageClass`` is set

Author: fatmcgav

charts/sourcebot/templates/pvc.yaml

@@ -1,4 +1,6 @@
 {{- if and .Values.sourcebot.persistence.enabled (not .Values.sourcebot.persistence.existingClaim) }}
+{{- $pvcName := printf "%s-%s" (include "sourcebot.fullname" .) "data" -}}
+{{- $existingPvc := (lookup "v1" "PersistentVolumeClaim" $.Release.Namespace $pvcName) -}}
 ---
 apiVersion: v1
 kind: PersistentVolumeClaim
@@ -7,10 +9,11 @@ metadata:
   namespace: {{ $.Release.Namespace }}
   labels:
     {{- include "sourcebot.labels" . | nindent 4 }}
-  {{- with .Values.sourcebot.persistence.annotations }}
   annotations:
+    helm.sh/resource-policy: keep
+    {{- with .Values.sourcebot.persistence.annotations }}
     {{- toYaml . | nindent 4 }}
-  {{- end }}
+    {{- end }}
 spec:
   accessModes:
     {{- range .Values.sourcebot.persistence.accessModes }}
@@ -26,5 +29,8 @@ spec:
   storageClassName: {{ .Values.sourcebot.persistence.storageClass }}
   {{- end }}
   {{- end }}
+  {{- if and $existingPvc $existingPvc.spec.volumeName }}
+  volumeName: {{ $existingPvc.spec.volumeName }}
+  {{- end }}
 {{- end }}
 

charts/sourcebot/tests/basic_test.yaml

@@ -181,6 +181,31 @@ tests:
               claimName: my-existing-pvc
         template: deployment.yaml
 
+  - it: should always have keep resource policy annotation on PVC
+    values:
+      - ../values.lint.yaml
+    asserts:
+      - equal:
+          path: metadata.annotations["helm.sh/resource-policy"]
+          value: keep
+        template: pvc.yaml
+
+  - it: should merge custom annotations with helm resource policy on PVC
+    values:
+      - ../values.lint.yaml
+    set:
+      sourcebot.persistence.annotations:
+        custom.io/annotation: "my-value"
+    asserts:
+      - equal:
+          path: metadata.annotations["helm.sh/resource-policy"]
+          value: keep
+        template: pvc.yaml
+      - equal:
+          path: metadata.annotations["custom.io/annotation"]
+          value: my-value
+        template: pvc.yaml
+
   - it: should set service type correctly
     values:
       - ../values.lint.yaml