Skip to content

Commit 2e24709

Browse files
myasnikovdaniilclaude
myasnikovdaniil
and
claude
committed
fix(apps/kubernetes): make pre-delete hook steps 3/5/6 fire-and-forget (#3271)
The Step-2 fix already merged with #2826 bounded the child-HelmRelease wait, but the pre-delete hook's remaining teardown steps still inherited kubectl's default --wait=true with no timeout: Step 3 delete KamajiControlPlane Step 5 delete DataVolumes Step 6 delete LoadBalancer Services Unlike the child HelmReleases in Step 2 — whose Flux finalizer only clears once helm-controller can uninstall INSIDE the tenant cluster, which is impossible on a nodeless tenant — these three target resources are finalized by controllers in THIS management cluster (Kamaji/CAPI, CDI + the CSI driver, the LB controller). Their teardown does not depend on tenant node health, so there is nothing to wait for on the Kubernetes-CR finalization path. Waiting only hurt: - Step 5's unbounded wait blocked the pre-cleanup Job until every worker-disk PVC was physically reclaimed; under storage pressure (the exact #3271 failure mode) that exceeds the parent Helm uninstall budget and wedges teardown. - Step 3's unbounded wait ran ahead of Step 4 on the same TCP teardown, so it could defeat Step 4's bounded wait + Step 4b datastore-secret backstop. Make all three --wait=false (fire-and-forget) and non-fatal (|| echo WARNING), so the Job completes fast, the parent Helm uninstall succeeds and the CR finalizes. The actual VM/PVC drain is waited on separately and best-effort by the caller (the e2e cozy_wait_tenant_drained barrier), not on the CR-finalization path — this restores the layering the kubernetes e2e harness was built around. Deliberately do NOT force-clear a stuck PVC/DataVolume finalizer: that would orphan the LINSTOR backing volume; a genuinely wedged PVC surfaces honestly as a Terminating namespace instead of being masked. Step 2 (bounded wait + child-HR finalizer force-clear) and Step 4/4b are left untouched — those are the paths that legitimately need a wait. Pins the new behavior in tests/delete_hook_test.yaml with step-local regexes (anchored to each command's own continuation lines, not a loose gap that would match a later step's flag and pass vacuously — mutation-verified). Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]> Assisted-By: Claude <[email protected]>
1 parent 99a28d8 commit 2e24709

2 files changed

Lines changed: 97 additions & 3 deletions

File tree

‎packages/apps/kubernetes/templates/delete.yaml‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,20 @@ spec:
9090
fi
9191
9292
echo "Step 3: Deleting KamajiControlPlane {{ .Release.Name }}"
93+
# Fire-and-forget (--wait=false): unlike the child HelmReleases in
94+
# Step 2 — whose Flux finalizer only clears once helm-controller can
95+
# uninstall INSIDE the tenant cluster (impossible with no Ready
96+
# nodes) — the KamajiControlPlane is finalized by the Kamaji/CAPI
97+
# controllers in THIS management cluster, so its teardown does not
98+
# depend on tenant node health and cannot hang on the nodeless
99+
# condition (#3271). kubectl delete defaults to --wait=true; waiting
100+
# here would only block this hook on the same TCP teardown that
101+
# Step 4 already handles with a bounded wait + Step 4b backstop — an
102+
# unbounded wait here would run first and defeat that backstop. So
103+
# trigger deletion and move on; Step 4 owns the bounded TCP wait.
93104
kubectl -n {{ .Release.Namespace }} delete kamajicontrolplanes.controlplane.cluster.x-k8s.io {{ .Release.Name }} \
94-
--ignore-not-found=true
105+
--ignore-not-found=true --wait=false \
106+
|| echo "WARNING: failed to trigger deletion of KamajiControlPlane {{ .Release.Name }}; Step 4 will still attempt the TenantControlPlane" >&2
95107
96108
echo "Step 4: Deleting TenantControlPlane {{ .Release.Name }} and waiting for it to be removed"
97109
# --wait=true blocks until the TenantControlPlane object is fully
@@ -138,15 +150,38 @@ spec:
138150
fi
139151
140152
echo "Step 5: Cleaning up DataVolumes"
153+
# Fire-and-forget (--wait=false): DataVolumes and their backing PVCs
154+
# live in THIS management cluster and are finalized by CDI and the
155+
# CSI driver here, independent of tenant node health — so, like
156+
# Step 3, deletion cannot hang on the nodeless condition (#3271).
157+
# kubectl delete defaults to --wait=true, which would block this
158+
# hook until every worker-disk PVC is physically reclaimed; under
159+
# storage pressure that can exceed the parent Helm uninstall budget
160+
# and wedge the whole teardown. Deleting the Kubernetes CR only
161+
# TRIGGERS this reclaim anyway — the actual VM/PVC drain is waited
162+
# on separately and best-effort by the caller (e.g. the e2e
163+
# cozy_wait_tenant_drained barrier), NOT on the CR-finalization
164+
# path. Do NOT force-clear a stuck PVC/DataVolume finalizer here:
165+
# that would orphan the LINSTOR backing volume. A genuinely wedged
166+
# PVC is a storage-layer problem and surfaces honestly as a
167+
# Terminating namespace rather than being masked.
141168
kubectl -n {{ .Release.Namespace }} delete datavolumes \
142169
-l "cluster.x-k8s.io/cluster-name={{ .Release.Name }}" \
143-
--ignore-not-found=true
170+
--ignore-not-found=true --wait=false \
171+
|| echo "WARNING: failed to trigger deletion of DataVolumes for {{ .Release.Name }}" >&2
144172
145173
echo "Step 6: Cleaning up LoadBalancer Services"
174+
# Fire-and-forget (--wait=false): these LoadBalancer Services are
175+
# reconciled by the management cluster's LB controller (MetalLB /
176+
# kube-vip), so their teardown does not depend on tenant node
177+
# health. kubectl delete defaults to --wait=true; there is nothing
178+
# to wait for on the CR-finalization path here — trigger the delete
179+
# and let the LB controller release the address asynchronously.
146180
kubectl -n {{ .Release.Namespace }} delete services \
147181
-l "cluster.x-k8s.io/cluster-name={{ .Release.Name }}" \
148182
--field-selector spec.type=LoadBalancer \
149-
--ignore-not-found=true
183+
--ignore-not-found=true --wait=false \
184+
|| echo "WARNING: failed to trigger deletion of LoadBalancer Services for {{ .Release.Name }}" >&2
150185
151186
echo "Cleanup completed successfully"
152187
volumeMounts:

‎packages/apps/kubernetes/tests/delete_hook_test.yaml‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,62 @@ tests:
6868
- matchRegex:
6969
path: spec.template.spec.containers[0].command[2]
7070
pattern: 'get tenantcontrolplanes\.kamaji\.clastix\.io test-k8s[\s\S]*still present[\s\S]*patch secret test-k8s-datastore-config'
71+
72+
# ---- issue #3271: the hook must not hang teardown on a nodeless tenant ----
73+
74+
# The patterns below anchor --wait=false / --wait=true to each command's OWN
75+
# continuation lines (`\\?\s+`), not a loose `[\s\S]*?` gap: a loose gap would
76+
# match a *later* step's flag and pass even if this step's were removed
77+
# (verified: such a pattern survives mutation, i.e. it is vacuous).
78+
79+
- it: Step 2 keeps a bounded wait and force-clears child HR finalizers on timeout (#3271)
80+
documentSelector:
81+
path: kind
82+
value: Job
83+
asserts:
84+
# Child HelmReleases carry a Flux finalizer that only clears once
85+
# helm-controller can uninstall INSIDE the tenant — impossible with no
86+
# Ready nodes. So Step 2 must stay bounded (not an unbounded --wait) and
87+
# force-clear the leftover finalizer so teardown proceeds.
88+
- matchRegex:
89+
path: spec.template.spec.containers[0].command[2]
90+
pattern: 'delete helmreleases\.helm\.toolkit\.fluxcd\.io \\?\s+-l "cozystack\.io/target-cluster-name=test-k8s" \\?\s+--ignore-not-found=true --wait=true --timeout=120s'
91+
- matchRegex:
92+
path: spec.template.spec.containers[0].command[2]
93+
pattern: 'Force-clearing finalizers on \$hr'
94+
95+
- it: Step 3 deletes the KamajiControlPlane fire-and-forget, never an unbounded wait (#3271)
96+
documentSelector:
97+
path: kind
98+
value: Job
99+
asserts:
100+
# The KCP is finalized by mgmt-cluster controllers, so its deletion cannot
101+
# hang on tenant node health — trigger it and let Step 4 own the bounded
102+
# TCP wait. An unbounded wait here would run first and defeat Step 4/4b.
103+
- matchRegex:
104+
path: spec.template.spec.containers[0].command[2]
105+
pattern: 'delete kamajicontrolplanes\.controlplane\.cluster\.x-k8s\.io test-k8s \\?\s+--ignore-not-found=true --wait=false'
106+
107+
- it: Step 5 deletes DataVolumes fire-and-forget so a stuck PVC cannot wedge the Job (#3271)
108+
documentSelector:
109+
path: kind
110+
value: Job
111+
asserts:
112+
- matchRegex:
113+
path: spec.template.spec.containers[0].command[2]
114+
pattern: 'delete datavolumes \\?\s+-l "cluster\.x-k8s\.io/cluster-name=test-k8s" \\?\s+--ignore-not-found=true --wait=false'
115+
# It must NOT force-clear a stuck PVC/DataVolume finalizer — that would
116+
# orphan the LINSTOR backing volume. Only the child-HR (Step 2) and the
117+
# Kamaji datastore Secret (Step 4b) are ever finalizer-stripped.
118+
- notMatchRegex:
119+
path: spec.template.spec.containers[0].command[2]
120+
pattern: 'delete datavolumes[\s\S]*?patch[\s\S]*?finalizers'
121+
122+
- it: Step 6 deletes LoadBalancer Services fire-and-forget (#3271)
123+
documentSelector:
124+
path: kind
125+
value: Job
126+
asserts:
127+
- matchRegex:
128+
path: spec.template.spec.containers[0].command[2]
129+
pattern: 'delete services \\?\s+-l "cluster\.x-k8s\.io/cluster-name=test-k8s" \\?\s+--field-selector spec\.type=LoadBalancer \\?\s+--ignore-not-found=true --wait=false'

0 commit comments

Comments
 (0)