Skip to content

Commit e01b725

Browse files
scooby87scooby87
committed
fix(monitoring): stamp release-scoping label on VM/VL storage PVCs so cleanup matches
Re-review on #3094 (IvanHunters, lexfrei): the release-scoped selector added in ba5797f was a silent no-op on the shipped vm-operator v0.68.4. The apps.cozystack.io/application.name label lived only under the VMCluster/VLCluster spec.managedMetadata.labels, and v0.68.4 does not propagate managedMetadata onto PVCs (that lands only in v0.71.0). So the delete matched nothing and the #3091 storage leak stayed open. Stamp the label where it actually reaches the PVCs: on spec.{vmselect,vmstorage,vlstorage}.storage.volumeClaimTemplate.metadata.labels of the VMCluster/VLCluster CRs (packages/system/monitoring). vm-operator copies claim-template labels onto the PVCs (IntoSTSVolume) even on v0.68.4, so the existing cleanup selector (managed-by=vm-operator,apps.cozystack.io/application.name= <release>-system) now matches and stays scoped to this release. Verified by rendering the CRs (label resolves to monitoring-system, matching the hook). Also add the explicit subjects[].namespace on the cleanup RoleBinding (lexfrei non-blocking) and a helm-unittest suite asserting the label lands on all three storage claim templates. Corrected the misleading test comment about how the label reaches the PVCs. Signed-off-by: Алексей Артамонов <[email protected]> Assisted-By: Claude <[email protected]>
1 parent 69c9e69 commit e01b725

5 files changed

Lines changed: 77 additions & 1 deletion

File tree

‎packages/extra/monitoring/templates/hooks/cleanup.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,4 @@ roleRef:
108108
subjects:
109109
- kind: ServiceAccount
110110
name: {{ .Release.Name }}-cleanup
111+
namespace: {{ .Release.Namespace }}

‎packages/extra/monitoring/tests/cleanup_test.yaml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@ tests:
6666
# managed-by=vm-operator: the bare label matches every VMCluster/VLCluster
6767
# in the namespace, so tearing one release down could delete another's
6868
# storage. apps.cozystack.io/application.name is stamped on the VM CRs'
69-
# managedMetadata and propagated by vm-operator onto the PVCs.
69+
# storage.volumeClaimTemplate.metadata.labels (packages/system/monitoring),
70+
# which vm-operator copies onto the PVCs via IntoSTSVolume even on v0.68.4
71+
# (managedMetadata itself reaches PVCs only from v0.71.0).
7072
- matchRegex:
7173
path: spec.template.spec.containers[0].command[2]
7274
pattern: 'kubectl delete pvc -n tenant-root -l managed-by=vm-operator,apps\.cozystack\.io/application\.name=monitoring-system --ignore-not-found --wait=false'
@@ -151,3 +153,6 @@ tests:
151153
- equal:
152154
path: subjects[0].name
153155
value: monitoring-cleanup
156+
- equal:
157+
path: subjects[0].namespace
158+
value: tenant-root

‎packages/system/monitoring/templates/vlogs/vlogs.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ spec:
2222
resources: {}
2323
storage:
2424
volumeClaimTemplate:
25+
metadata:
26+
# Stamped on the claim template so vm-operator copies it onto the
27+
# storage PVCs (IntoSTSVolume) even on v0.68.4, which does not yet
28+
# propagate managedMetadata to PVCs. The monitoring post-delete
29+
# cleanup hook selects PVCs by this label to stay release-scoped.
30+
labels:
31+
apps.cozystack.io/application.name: {{ $.Release.Name }}
2532
spec:
2633
{{- with .storageClassName }}
2734
storageClassName: {{ . }}

‎packages/system/monitoring/templates/vm/vmcluster.yaml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ spec:
2929
cacheMountPath: /select-cache
3030
storage:
3131
volumeClaimTemplate:
32+
metadata:
33+
# Stamped on the claim template so vm-operator copies it onto the
34+
# storage PVCs (IntoSTSVolume) even on v0.68.4, which does not yet
35+
# propagate managedMetadata to PVCs. The monitoring post-delete
36+
# cleanup hook selects PVCs by this label to stay release-scoped.
37+
labels:
38+
apps.cozystack.io/application.name: {{ $.Release.Name }}
3239
spec:
3340
{{- with .storageClassName }}
3441
storageClassName: {{ . }}
@@ -41,6 +48,13 @@ spec:
4148
resources: {}
4249
storage:
4350
volumeClaimTemplate:
51+
metadata:
52+
# Stamped on the claim template so vm-operator copies it onto the
53+
# storage PVCs (IntoSTSVolume) even on v0.68.4, which does not yet
54+
# propagate managedMetadata to PVCs. The monitoring post-delete
55+
# cleanup hook selects PVCs by this label to stay release-scoped.
56+
labels:
57+
apps.cozystack.io/application.name: {{ $.Release.Name }}
4458
spec:
4559
{{- with .storageClassName }}
4660
storageClassName: {{ . }}

‎packages/system/monitoring/tests/storage_pvc_label_test.yaml‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
suite: VM/VL storage claim templates carry the release-scoping cleanup label
2+
3+
# The extra/monitoring post-delete cleanup hook deletes orphaned VM/VL storage
4+
# PVCs scoped to this release via apps.cozystack.io/application.name=<release>.
5+
# On the shipped vm-operator v0.68.4 managedMetadata is NOT propagated to PVCs
6+
# (only from v0.71.0), so the label must be stamped on the claim template
7+
# itself — vm-operator copies volumeClaimTemplate.metadata.labels onto the PVCs
8+
# (IntoSTSVolume) even on v0.68.4. release.name mirrors the flux release of this
9+
# chart (monitoring-system), which is exactly what the hook selector expects.
10+
11+
release:
12+
name: monitoring-system
13+
namespace: tenant-root
14+
15+
# Dunder values so the rest of the chart (grafana httproute etc.) renders;
16+
# they do not affect the VMCluster/VLCluster storage templates under test.
17+
set:
18+
_namespace:
19+
host: example.org
20+
ingress: tenant-root
21+
_cluster:
22+
expose-ingress: ingress
23+
host: example.org
24+
25+
tests:
26+
- it: stamps the cleanup label on vmstorage and vmselect claim templates
27+
templates:
28+
- templates/vm/vmcluster.yaml
29+
documentSelector:
30+
path: metadata.name
31+
value: shortterm
32+
asserts:
33+
- equal:
34+
path: spec.vmstorage.storage.volumeClaimTemplate.metadata.labels["apps.cozystack.io/application.name"]
35+
value: monitoring-system
36+
- equal:
37+
path: spec.vmselect.storage.volumeClaimTemplate.metadata.labels["apps.cozystack.io/application.name"]
38+
value: monitoring-system
39+
40+
- it: stamps the cleanup label on the vlstorage claim template
41+
templates:
42+
- templates/vlogs/vlogs.yaml
43+
documentSelector:
44+
path: metadata.name
45+
value: generic
46+
asserts:
47+
- equal:
48+
path: spec.vlstorage.storage.volumeClaimTemplate.metadata.labels["apps.cozystack.io/application.name"]
49+
value: monitoring-system

0 commit comments

Comments
 (0)