Skip to content

Commit 88a770f

Browse files
fix(csi): verify VMI Ready after Publish to surface stuck-PVC failures
Upstream ControllerPublishVolume has a fast path that returns success based only on the volume's presence in VM.spec.template.spec.volumes (via EnsureVolumeAvailableVM). If a prior Publish wrote that entry but the volume never became Ready in VMI status - e.g. the infra PVC is stuck ClaimPending because LINSTOR cannot autoplace - the retry hits the fast path and reports success. external-attacher marks the VolumeAttachment Attached=true, but QEMU was never sent device_add, so the tenant kubelet fails NodeStageVolume with: rpc error: code = Unknown desc = couldn't find device by serial id Wrap upstream Publish for hotplug volumes (non-NFS) with a post-call check that the volume reached VolumeReady in VMI.Status.VolumeStatus. When it has not, the RPC returns codes.Unavailable carrying the actual reason (e.g. "PVC is in phase ClaimPending"), so external-attacher keeps retrying and the underlying provisioning failure is visible at the CSI layer rather than as a confusing "device not found by serial id" on the tenant kubelet. This is a wrapper-side workaround for an upstream defect in kubevirt.io/csi-driver; the structural fix belongs upstream (tighten the fast path to require VMI Ready) and will be filed separately. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent e4f9a18 commit 88a770f

1 file changed

Lines changed: 47 additions & 2 deletions

File tree

‎packages/apps/kubernetes/images/kubevirt-csi-driver/controller.go‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"k8s.io/client-go/tools/cache"
2222
"k8s.io/client-go/util/retry"
2323
"k8s.io/klog/v2"
24+
kubevirtv1 "kubevirt.io/api/core/v1"
2425
cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
2526

2627
kubevirtclient "kubevirt.io/csi-driver/pkg/kubevirt"
@@ -237,12 +238,56 @@ func (w *WrappedControllerService) determineDvSource(ctx context.Context, req *c
237238
return res, nil
238239
}
239240

241+
// publishHotplugVolume delegates to upstream for the hotplug attach, then re-verifies
242+
// that the volume reached VolumeReady in VMI.Status.VolumeStatus before returning
243+
// success. Upstream's fast path (EnsureVolumeAvailableVM) considers the volume
244+
// attached based only on its presence in VM.spec.template.spec.volumes — so when an
245+
// earlier attempt wrote that entry but the volume never became Ready (e.g. the infra
246+
// PVC is stuck ClaimPending because the storage backend cannot provision), the
247+
// retried Publish reports success and external-attacher sets Attached=true.
248+
// QEMU never received device_add, and the tenant kubelet's NodeStageVolume fails
249+
// with "couldn't find device by serial id". Verifying VMI Ready independently of
250+
// upstream propagates the not-ready condition back to external-attacher so it keeps
251+
// retrying instead of silently declaring the volume attached.
252+
func (w *WrappedControllerService) publishHotplugVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
253+
resp, err := w.ControllerService.ControllerPublishVolume(ctx, req)
254+
if err != nil {
255+
return nil, err
256+
}
257+
258+
vmNamespace, vmName, parseErr := cache.SplitMetaNamespaceKey(req.GetNodeId())
259+
if parseErr != nil {
260+
klog.Warningf("Cannot verify VMI readiness after publish: failed to parse node ID %q: %v", req.GetNodeId(), parseErr)
261+
return resp, nil
262+
}
263+
dvName := req.GetVolumeId()
264+
vmi, getErr := w.virtClient.GetVirtualMachine(ctx, vmNamespace, vmName)
265+
if getErr != nil {
266+
klog.Warningf("Cannot verify VMI %s/%s readiness after publish of %s: %v", vmNamespace, vmName, dvName, getErr)
267+
return resp, nil
268+
}
269+
for _, vs := range vmi.Status.VolumeStatus {
270+
if vs.Name != dvName {
271+
continue
272+
}
273+
if vs.Phase == kubevirtv1.VolumeReady {
274+
return resp, nil
275+
}
276+
return nil, status.Errorf(codes.Unavailable,
277+
"volume %s is in VM %s/%s spec but not Ready in VMI status (phase=%q, message=%q)",
278+
dvName, vmNamespace, vmName, vs.Phase, vs.Message)
279+
}
280+
return nil, status.Errorf(codes.Unavailable,
281+
"volume %s not present in VMI %s/%s status after publish", dvName, vmNamespace, vmName)
282+
}
283+
240284
// ControllerPublishVolume for NFS volumes: annotates infra PVC for WFFC binding,
241285
// waits for PVC bound, extracts NFS export from PV, and creates CiliumNetworkPolicy.
242-
// For RWO volumes, delegates to upstream (hotplug SCSI).
286+
// For hotplug volumes (RWO and RWX Block), delegates to upstream and then verifies
287+
// the volume is actually Ready in VMI.Status.VolumeStatus before returning success.
243288
func (w *WrappedControllerService) ControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
244289
if req.GetVolumeContext()[nfsVolumeKey] != "true" {
245-
return w.ControllerService.ControllerPublishVolume(ctx, req)
290+
return w.publishHotplugVolume(ctx, req)
246291
}
247292

248293
if len(req.GetVolumeId()) == 0 {

0 commit comments

Comments
 (0)