Skip to content

Commit b4f6fb6

Browse files
mattia-eleuterimattia-eleuteri
committed
fix(vm-disk): let CDI adopt the PVC a Velero restore recreates
A Velero restore of a VMDisk or a VMInstance deletes the disk's DataVolume and restores the PVC under its name, with no owner. The restored release is in sync, so nothing recreates the DataVolume until the next Helm upgrade, typically a platform update. That upgrade is refused by CDI's admission webhook ("Destination PVC ... already exists") and the release stays failed, followed by the VMInstance release that looks the disk up. CDI admits a DataVolume over an existing claim only when the DataVolume carries cdi.kubevirt.io/allowClaimAdoption or the DataVolumeClaimAdoption feature gate is on. The restore controller puts the annotation on the restored PVC, where CDI never reads it, and the CDI CR does not enable the gate. Setting it on the DataVolume keeps the scope to vm-disk instead of every DataVolume in the cluster, and does not depend on a gate CDI intends to remove. A claim CDI created for this DataVolume carries its uid in createdForDataVolume and is never treated as adoptable, so a fresh import is unaffected. The VMDisk backup round-trip now forces an upgrade after the restore, which is what reaches the failure, and checks that the DataVolume adopted the restored PVC instead of replacing it. Signed-off-by: Mattia Eleuteri <[email protected]> Assisted-by: LLM
1 parent aed7fa8 commit b4f6fb6

3 files changed

Lines changed: 104 additions & 0 deletions

File tree

‎examples/backups/vmdisk/05-restore-in-place.sh‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,5 +155,29 @@ hr_generation=$(kubectl -n "$NAMESPACE" get hr vm-disk-backup-src -o jsonpath='{
155155
wait_for_field hr vm-disk-backup-src '{.status.observedGeneration}' "$hr_generation" "$NAMESPACE" 120
156156
wait_hr_ready vm-disk-backup-src 180
157157

158+
# The restore deleted the DataVolume, and a restored release matches its chart
159+
# and values, so Flux does not recreate it: the next Helm upgrade does, usually
160+
# a platform update long after the restore. Force that upgrade here. CDI must
161+
# admit a DataVolume over the claim Velero restored and adopt it as populated;
162+
# refusing it leaves the release failed on every upgrade from then on, and
163+
# re-importing it would overwrite the restored data.
164+
log_step "Forcing a Helm upgrade so the restored VMDisk recreates its DataVolume..."
165+
force_at=$(date -u +%Y-%m-%dT%H:%M:%SZ)
166+
kubectl -n "$NAMESPACE" annotate hr vm-disk-backup-src --overwrite \
167+
"reconcile.fluxcd.io/requestedAt=${force_at}" "reconcile.fluxcd.io/forceAt=${force_at}"
168+
wait_for_field hr vm-disk-backup-src '{.status.lastHandledForceAt}' "$force_at" "$NAMESPACE" 180
169+
wait_hr_ready vm-disk-backup-src 180
170+
wait_for_field datavolume vm-disk-backup-src '{.status.phase}' Succeeded "$NAMESPACE" 120 Failed
171+
read -r adopted_uid owner_kind < <(kubectl -n "$NAMESPACE" get pvc "$PVC" \
172+
-o jsonpath='{.metadata.uid} {.metadata.ownerReferences[?(@.controller==true)].kind}') || true
173+
[[ "${adopted_uid:-}" == "$new_uid" ]] || {
174+
log_error "PVC $PVC changed uid from $new_uid to '${adopted_uid:-}' — the DataVolume replaced the restored disk instead of adopting it"
175+
exit 1
176+
}
177+
[[ "${owner_kind:-}" == "DataVolume" ]] || {
178+
log_error "PVC $PVC is controlled by '${owner_kind:-nothing}', not by its DataVolume — CDI did not adopt the restored disk"
179+
exit 1
180+
}
181+
158182
kubectl get restorejob vmdisk-restore-in-place -n "$NAMESPACE" -o wide >&2 || true
159183
log_success "Restore completed; VMDisk 'backup-src' recreated from backup (${restore_label}, uid ${new_uid}), Bound and Ready"

‎packages/apps/vm-disk/templates/dv.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,15 @@ metadata:
256256
{{ $storageClassAnnotation }}: {{ $requestedSC | quote }}
257257
{{- end }}
258258
vm-disk.cozystack.io/optical: "{{ .Values.optical }}"
259+
{{- /* A Velero restore deletes the DataVolume and recreates the PVC under
260+
its name, so the next Helm upgrade creates this DataVolume over a
261+
claim CDI did not make for it. CDI v1.66 accepts that only with this
262+
annotation on the DataVolume or with the DataVolumeClaimAdoption
263+
feature gate; the same annotation on the PVC is never read. It cannot
264+
capture a claim CDI created for this very DataVolume, whose
265+
createdForDataVolume is the DataVolume's uid, so a fresh import is
266+
unchanged. */}}
267+
cdi.kubevirt.io/allowClaimAdoption: "true"
259268
labels:
260269
app.kubernetes.io/instance: {{ .Release.Name }}
261270
name: {{ .Release.Name }}

‎packages/apps/vm-disk/tests/dv_claim_adoption_test.yaml‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
suite: vm-disk DataVolume claim adoption
2+
templates:
3+
- templates/dv.yaml
4+
tests:
5+
# The state a Velero restore leaves behind: the restore controller deleted the
6+
# DataVolume and Velero recreated the PVC under the DataVolume's name, without
7+
# an owner. CDI's admission webhook refuses a DataVolume over such a claim
8+
# unless the DataVolume itself carries the annotation, so the release would
9+
# stay failed on every upgrade after the restore.
10+
- it: lets CDI adopt the PVC a restore recreated without its DataVolume
11+
set:
12+
source: {http: {url: https://example.com/disk.img}}
13+
kubernetesProvider:
14+
scheme:
15+
"cdi.kubevirt.io/v1beta1/DataVolume":
16+
gvr: {group: cdi.kubevirt.io, version: v1beta1, resource: datavolumes}
17+
namespaced: true
18+
"v1/PersistentVolumeClaim":
19+
gvr: {group: "", version: v1, resource: persistentvolumeclaims}
20+
namespaced: true
21+
objects:
22+
- kind: PersistentVolumeClaim
23+
apiVersion: v1
24+
metadata:
25+
name: RELEASE-NAME
26+
namespace: NAMESPACE
27+
annotations:
28+
cdi.kubevirt.io/createdForDataVolume: 2e53a035-aa93-49a8-b712-1f74ac18eb11
29+
labels:
30+
velero.io/restore-name: tenant-test.restore-disk
31+
asserts:
32+
- equal:
33+
path: metadata.annotations["cdi.kubevirt.io/allowClaimAdoption"]
34+
value: "true"
35+
# The creation path, not the existing-DataVolume one: this is the render
36+
# Helm uses to recreate the deleted DataVolume.
37+
- equal:
38+
path: spec.source
39+
value: {http: {url: https://example.com/disk.img}}
40+
41+
# The existing-DataVolume path renders its own annotation block. Dropping the
42+
# annotation there would make every upgrade after an adoption patch it away
43+
# and the next restore put it back.
44+
- it: keeps the annotation on an existing DataVolume
45+
set:
46+
storageClass: replicated
47+
source: {http: {url: https://example.com/original.qcow2}}
48+
kubernetesProvider:
49+
scheme:
50+
"cdi.kubevirt.io/v1beta1/DataVolume":
51+
gvr: {group: cdi.kubevirt.io, version: v1beta1, resource: datavolumes}
52+
namespaced: true
53+
objects:
54+
- kind: DataVolume
55+
apiVersion: cdi.kubevirt.io/v1beta1
56+
metadata:
57+
name: RELEASE-NAME
58+
namespace: NAMESPACE
59+
annotations:
60+
vm-disk.cozystack.io/source: '{"http":{"url":"https://example.com/original.qcow2"}}'
61+
vm-disk.cozystack.io/storage-class: replicated
62+
spec:
63+
source: {http: {url: https://example.com/original.qcow2}}
64+
storage: {resources: {requests: {storage: 9Gi}}, storageClassName: replicated}
65+
asserts:
66+
- equal:
67+
path: spec.storage.resources.requests.storage
68+
value: 9Gi
69+
- equal:
70+
path: metadata.annotations["cdi.kubevirt.io/allowClaimAdoption"]
71+
value: "true"

0 commit comments

Comments
 (0)