Skip to content

Commit 9bf65f3

Browse files
scooby87scooby87
committed
fix(clickhouse)!: clean up orphaned keeper and data PVCs on delete (#3057)
After deleting a ClickHouse app CR, the keeper PVC and the main data/log PVCs were left orphaned in the tenant namespace, consuming storage permanently. The clickhouse-keeper-operator does not delete data PVCs on CR removal (data-safety default) and the CRD exposes no persistentVolumeClaimRetentionPolicy equivalent. Add a post-delete cleanup hook (modeled on the MariaDB hook) that deletes every PVC carrying the release label app.kubernetes.io/instance=<release>. Also stamp that label on the keeper volumeClaimTemplates in chkeeper.yaml: the keeper PVC previously had no release label (the chart labelled only the keeper pod template, not the PVC), so a single selector would have missed the very PVC the issue reports. BREAKING CHANGE: deleting a ClickHouse app now permanently removes all of its PVCs (keeper + data + logs) and the data they hold. Nothing is left behind to recover from. This matches the existing MariaDB cleanup-hook behaviour and the general cozystack model where deleting a managed app reclaims its storage. Signed-off-by: Алексей Артамонов <[email protected]>
1 parent 665e7e4 commit 9bf65f3

4 files changed

Lines changed: 179 additions & 0 deletions

File tree

‎packages/apps/clickhouse/Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ generate:
77
cozyvalues-gen -m 'clickhouse' -v values.yaml -s values.schema.json -r README.md -g ../../../api/apps/v1alpha1/clickhouse/types.go
88
../../../hack/update-crd.sh
99

10+
.PHONY: test
1011
test:
1112
helm unittest .
1213

‎packages/apps/clickhouse/templates/chkeeper.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ spec:
6464

6565
volumeClaimTemplates:
6666
- name: default
67+
metadata:
68+
labels:
69+
app.kubernetes.io/instance: "{{ .Release.Name }}"
6770
spec:
6871
accessModes:
6972
- ReadWriteOnce

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
---
2+
apiVersion: batch/v1
3+
kind: Job
4+
metadata:
5+
name: {{ .Release.Name }}-cleanup
6+
labels:
7+
app.kubernetes.io/instance: {{ .Release.Name }}
8+
annotations:
9+
"helm.sh/hook": post-delete
10+
"helm.sh/hook-weight": "10"
11+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
12+
spec:
13+
template:
14+
metadata:
15+
labels:
16+
app.kubernetes.io/instance: {{ .Release.Name }}
17+
policy.cozystack.io/allow-to-apiserver: "true"
18+
spec:
19+
serviceAccountName: {{ .Release.Name }}-cleanup
20+
restartPolicy: Never
21+
securityContext:
22+
runAsNonRoot: true
23+
runAsUser: 65534
24+
runAsGroup: 65534
25+
seccompProfile:
26+
type: RuntimeDefault
27+
containers:
28+
- name: cleanup
29+
image: docker.io/clastix/kubectl:v1.32
30+
securityContext:
31+
allowPrivilegeEscalation: false
32+
readOnlyRootFilesystem: true
33+
capabilities:
34+
drop: ["ALL"]
35+
env:
36+
- name: HOME
37+
value: /tmp
38+
resources:
39+
requests:
40+
cpu: 10m
41+
memory: 64Mi
42+
limits:
43+
cpu: 200m
44+
memory: 128Mi
45+
command:
46+
- /bin/sh
47+
- -c
48+
- |
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
52+
echo "PVC cleanup complete."
53+
volumeMounts:
54+
- name: tmp
55+
mountPath: /tmp
56+
volumes:
57+
- name: tmp
58+
emptyDir: {}
59+
---
60+
apiVersion: v1
61+
kind: ServiceAccount
62+
metadata:
63+
name: {{ .Release.Name }}-cleanup
64+
labels:
65+
app.kubernetes.io/instance: {{ .Release.Name }}
66+
annotations:
67+
"helm.sh/hook": post-delete
68+
helm.sh/hook-weight: "0"
69+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
70+
---
71+
apiVersion: rbac.authorization.k8s.io/v1
72+
kind: Role
73+
metadata:
74+
name: {{ .Release.Name }}-cleanup
75+
labels:
76+
app.kubernetes.io/instance: {{ .Release.Name }}
77+
annotations:
78+
"helm.sh/hook": post-delete
79+
"helm.sh/hook-weight": "5"
80+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
81+
rules:
82+
- apiGroups: [""]
83+
resources: ["persistentvolumeclaims"]
84+
verbs: ["get", "list", "delete"]
85+
---
86+
apiVersion: rbac.authorization.k8s.io/v1
87+
kind: RoleBinding
88+
metadata:
89+
name: {{ .Release.Name }}-cleanup
90+
labels:
91+
app.kubernetes.io/instance: {{ .Release.Name }}
92+
annotations:
93+
"helm.sh/hook": post-delete
94+
helm.sh/hook-weight: "5"
95+
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
96+
roleRef:
97+
apiGroup: rbac.authorization.k8s.io
98+
kind: Role
99+
name: {{ .Release.Name }}-cleanup
100+
subjects:
101+
- kind: ServiceAccount
102+
name: {{ .Release.Name }}-cleanup

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
suite: post-delete PVC cleanup hook
2+
3+
# Verifies the post-delete hook that deletes orphaned PVCs left behind by the
4+
# clickhouse-keeper-operator (and the ClickHouse operator), which do not delete
5+
# data PVCs when their CR is removed. The cleanup Job selects PVCs by the
6+
# release-scoped label app.kubernetes.io/instance, which is set on both the
7+
# keeper volumeClaimTemplates and the main ClickHouse data/log volumeClaimTemplates.
8+
9+
release:
10+
name: ch-test
11+
namespace: tenant-test
12+
13+
set:
14+
_cluster:
15+
cluster-domain: cluster.local
16+
17+
templates:
18+
- templates/hooks/cleanup-pvc.yaml
19+
20+
tests:
21+
- it: "renders a post-delete cleanup Job scoped to the release"
22+
documentSelector:
23+
path: kind
24+
value: Job
25+
asserts:
26+
- equal:
27+
path: metadata.name
28+
value: ch-test-cleanup
29+
- equal:
30+
path: metadata.annotations["helm.sh/hook"]
31+
value: post-delete
32+
- equal:
33+
path: metadata.annotations["helm.sh/hook-delete-policy"]
34+
value: before-hook-creation,hook-succeeded
35+
- equal:
36+
path: spec.template.spec.serviceAccountName
37+
value: ch-test-cleanup
38+
- matchRegex:
39+
path: spec.template.spec.containers[0].command[2]
40+
pattern: 'kubectl delete pvc -n tenant-test -l app\.kubernetes\.io/instance=ch-test'
41+
42+
- it: "grants only get/list/delete on persistentvolumeclaims"
43+
documentSelector:
44+
path: kind
45+
value: Role
46+
asserts:
47+
- equal:
48+
path: rules[0].apiGroups[0]
49+
value: ""
50+
- equal:
51+
path: rules[0].resources[0]
52+
value: persistentvolumeclaims
53+
- equal:
54+
path: rules[0].verbs
55+
value:
56+
- get
57+
- list
58+
- delete
59+
60+
- it: "binds the cleanup ServiceAccount to the cleanup Role"
61+
documentSelector:
62+
path: kind
63+
value: RoleBinding
64+
asserts:
65+
- equal:
66+
path: roleRef.name
67+
value: ch-test-cleanup
68+
- equal:
69+
path: subjects[0].kind
70+
value: ServiceAccount
71+
- equal:
72+
path: subjects[0].name
73+
value: ch-test-cleanup

0 commit comments

Comments
 (0)