Skip to content

Commit 2587458

Browse files
scooby87scooby87
committed
fix(qdrant)!: reclaim data PVC by name prefix, not volumeClaimTemplate label
The previous approach routed a release-unique label through qdrant.persistence.additionalLabels, which the upstream chart renders into the StatefulSet's volumeClaimTemplates[].metadata.labels. That field is immutable on an existing StatefulSet, so for any already-deployed Qdrant the API server would reject the upgrade and the <release>-system HelmRelease (retries: -1) would retry-fail forever — permanently wedging reconciliation. Label changes on a volumeClaimTemplate also never apply to PVCs that already exist, so the cleanup hook could not reclaim those instances anyway. Switch the post-delete hook to select PVCs by the deterministic StatefulSet naming prefix (qdrant-storage-<release>-) instead. PVC names derive purely from the release name, so this needs no StatefulSet change (upgrade-safe) and also reclaims PVCs from instances created before the hook existed. - qdrant.yaml: drop the additionalLabels passthrough entirely. - hooks/cleanup-pvc.yaml: delete by name prefix via kubectl get -o name | grep. - tests: assert the name-prefix command instead of the label passthrough. - e2e: assert qdrant-storage-<name>-0 is gone after delete (async-waited). BREAKING CHANGE: deleting a Qdrant app now permanently removes its data PVC and the vector data it holds. Assisted-By: Claude <[email protected]> Signed-off-by: Алексей Артамонов <[email protected]>
1 parent 26dc006 commit 2587458

4 files changed

Lines changed: 35 additions & 35 deletions

File tree

‎hack/e2e-apps/qdrant.bats‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,10 @@ EOF
2727
timeout 60 sh -ec "until kubectl -n tenant-test get pvc qdrant-storage-qdrant-$name-0 >/dev/null 2>&1; do sleep 2; done"
2828
kubectl -n tenant-test wait pvc qdrant-storage-qdrant-$name-0 --timeout=50s --for=jsonpath='{.status.phase}'=Bound
2929
kubectl -n tenant-test delete qdrant.apps.cozystack.io $name
30+
# Issue #3059: Kubernetes never garbage-collects StatefulSet-templated PVCs,
31+
# so the post-delete cleanup hook must reclaim the data PVC. The hook Job runs
32+
# asynchronously during helm uninstall — wait for the PVC to disappear and
33+
# fail loud (with the hook Job/Pod state) if it is left orphaned.
34+
timeout 120 sh -ec "while kubectl -n tenant-test get pvc qdrant-storage-qdrant-$name-0 >/dev/null 2>&1; do sleep 2; done" \
35+
|| { echo "❌ data PVC qdrant-storage-qdrant-$name-0 still present after Qdrant delete (issue #3059 regression)"; kubectl -n tenant-test get pvc -o wide; kubectl -n tenant-test get jobs,pods -l app.kubernetes.io/instance=qdrant-$name -o wide; false; }
3036
}

‎packages/apps/qdrant/templates/hooks/cleanup-pvc.yaml‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,24 @@ spec:
4646
- /bin/sh
4747
- -c
4848
- |
49-
echo "Deleting orphaned PVCs for {{ .Release.Name }}..."
50-
kubectl delete pvc -n {{ .Release.Namespace }} -l app.kubernetes.io/instance={{ .Release.Name }} --ignore-not-found \
51-
|| echo "WARNING: PVC cleanup failed for {{ .Release.Name }}; orphaned PVCs may remain" >&2
49+
# StatefulSet-templated data PVCs are named
50+
# qdrant-storage-<release>-<ordinal> (volumeClaimTemplate name
51+
# "qdrant-storage" + StatefulSet name <release> + pod ordinal).
52+
# Select by that deterministic name prefix rather than a label:
53+
# the upstream chart stamps volumeClaimTemplates labels, which are
54+
# immutable on an existing StatefulSet, so a label-based scheme
55+
# cannot be applied to already-deployed instances. The name prefix
56+
# needs no StatefulSet change and also reclaims PVCs created before
57+
# this hook existed. See #3059.
58+
echo "Deleting orphaned data PVCs for release {{ .Release.Name }}..."
59+
pvcs=$(kubectl get pvc -n {{ .Release.Namespace }} -o name \
60+
| grep "^persistentvolumeclaim/qdrant-storage-{{ .Release.Name }}-" || true)
61+
if [ -z "$pvcs" ]; then
62+
echo "No data PVCs found for release {{ .Release.Name }}; nothing to delete."
63+
else
64+
echo "$pvcs" | xargs kubectl delete -n {{ .Release.Namespace }} --ignore-not-found \
65+
|| echo "WARNING: PVC cleanup failed for {{ .Release.Name }}; orphaned PVCs may remain" >&2
66+
fi
5267
echo "PVC cleanup complete."
5368
volumeMounts:
5469
- name: tmp

‎packages/apps/qdrant/templates/qdrant.yaml‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,6 @@ spec:
3434
{{- with .Values.storageClass }}
3535
storageClassName: {{ . }}
3636
{{- end }}
37-
# Stamp a release-unique label onto the StatefulSet data PVC so the
38-
# post-delete cleanup hook can target only this release's PVC. The
39-
# upstream chart does NOT support persistentVolumeClaimRetentionPolicy
40-
# and its volumeClaimTemplate otherwise carries only `app: qdrant`
41-
# (the chart name, shared by every Qdrant in the namespace). See #3059.
42-
additionalLabels:
43-
app.kubernetes.io/instance: {{ .Release.Name }}
4437
metrics:
4538
serviceMonitor:
4639
enabled: true

‎packages/apps/qdrant/tests/cleanup_pvc_test.yaml‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,40 +4,23 @@ suite: qdrant data PVC cleanup on delete
44
# persistentVolumeClaimRetentionPolicy, so its StatefulSet-templated data PVC
55
# (qdrant-storage-<release>-0) is orphaned when the StatefulSet is deleted.
66
#
7-
# The fix has two parts:
8-
# 1. The HelmRelease passes persistence.additionalLabels through to the
9-
# upstream chart so the data PVC carries a release-unique label
10-
# (app.kubernetes.io/instance=<release>). Without it the volumeClaimTemplate
11-
# only stamps `app: qdrant` (the chart name), shared by every Qdrant in the
12-
# namespace.
13-
# 2. A post-delete hook deletes PVCs matching that label so the data is
14-
# reclaimed on release deletion.
7+
# A post-delete hook reclaims the data by deleting PVCs whose name matches the
8+
# deterministic StatefulSet template prefix (qdrant-storage-<release>-). A
9+
# name-prefix scheme is used instead of a label because volumeClaimTemplates
10+
# labels are immutable on an existing StatefulSet — routing a release-unique
11+
# label through the subchart would break reconciliation for already-deployed
12+
# instances and still would not match their pre-existing PVCs.
1513

1614
release:
1715
name: qdrant-test
1816
namespace: tenant-test
1917

2018
tests:
21-
#####################################################
22-
# PVC label passthrough on the HelmRelease values #
23-
#####################################################
24-
25-
- it: "stamps a release-unique instance label onto the data PVC"
26-
templates:
27-
- templates/qdrant.yaml
28-
documentSelector:
29-
path: kind
30-
value: HelmRelease
31-
asserts:
32-
- equal:
33-
path: spec.values.qdrant.persistence.additionalLabels["app.kubernetes.io/instance"]
34-
value: qdrant-test
35-
3619
#####################################################
3720
# post-delete cleanup hook #
3821
#####################################################
3922

40-
- it: "renders a post-delete Job that deletes the data PVC by instance label"
23+
- it: "renders a post-delete Job that deletes the data PVC by name prefix"
4124
templates:
4225
- templates/hooks/cleanup-pvc.yaml
4326
documentSelector:
@@ -54,7 +37,10 @@ tests:
5437
value: qdrant-test-qdrant-cleanup
5538
- matchRegex:
5639
path: spec.template.spec.containers[0].command[2]
57-
pattern: "kubectl delete pvc -n tenant-test -l app\\.kubernetes\\.io/instance=qdrant-test --ignore-not-found"
40+
pattern: "kubectl get pvc -n tenant-test -o name"
41+
- matchRegex:
42+
path: spec.template.spec.containers[0].command[2]
43+
pattern: "grep \"\\^persistentvolumeclaim/qdrant-storage-qdrant-test-\""
5844

5945
- it: "creates qdrant-scoped RBAC for the cleanup hook"
6046
templates:

0 commit comments

Comments
 (0)