From 921804525b8c4eaf840ac4c4956ce271712d3692 Mon Sep 17 00:00:00 2001 From: chrislusf Date: Wed, 3 Dec 2025 19:50:42 -0800 Subject: fix: propagate errors instead of just logging warnings Address gemini-code-assist review feedback: 1. Return error from volume.Quota() failure in stageNewVolume - quota failures should fail the staging operation 2. Return error from cleanupStaleStagingPath() in NodeStageVolume - fail fast if cleanup fails rather than attempting to stage anyway 3. Return error from cleanupStaleStagingPath() in NodePublishVolume - same fail-fast behavior for consistency 4. Return error from mount.CleanupMountPoint() in Volume.Unstage() - propagate cleanup errors to caller as expected --- pkg/driver/nodeserver.go | 6 ++++-- pkg/driver/volume.go | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/driver/nodeserver.go b/pkg/driver/nodeserver.go index d861ddc..e8890dd 100644 --- a/pkg/driver/nodeserver.go +++ b/pkg/driver/nodeserver.go @@ -75,7 +75,8 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol if _, err := os.Stat(stagingTargetPath); err == nil || mount.IsCorruptedMnt(err) { glog.Infof("volume %s has stale staging path at %s, cleaning up", volumeID, stagingTargetPath) if err := cleanupStaleStagingPath(stagingTargetPath); err != nil { - glog.Warningf("failed to cleanup stale staging path %s: %v, will try to stage anyway", stagingTargetPath, err) + ns.removeVolumeMutex(volumeID) + return nil, status.Errorf(codes.Internal, "failed to cleanup stale staging path %s: %v", stagingTargetPath, err) } } @@ -149,7 +150,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis // Clean up stale staging path if it exists if err := cleanupStaleStagingPath(stagingTargetPath); err != nil { - glog.Warningf("failed to cleanup stale staging path %s: %v", stagingTargetPath, err) + return nil, status.Errorf(codes.Internal, "failed to cleanup stale staging path %s: %v", stagingTargetPath, err) } // Re-stage the volume using the shared helper @@ -379,6 +380,7 @@ func (ns *NodeServer) stageNewVolume(volumeID, stagingTargetPath string, volCont if capacity, err := k8s.GetVolumeCapacity(volumeID); err == nil { if err := volume.Quota(capacity); err != nil { glog.Warningf("failed to apply quota for volume %s: %v", volumeID, err) + return nil, err } } else { glog.V(4).Infof("orchestration system is not compatible with the k8s api, error is: %s", err) diff --git a/pkg/driver/volume.go b/pkg/driver/volume.go index 023abf9..6cc777b 100644 --- a/pkg/driver/volume.go +++ b/pkg/driver/volume.go @@ -125,7 +125,8 @@ func (vol *Volume) Unstage(stagingTargetPath string) error { // Clean up using mount utilities. This will also handle unmounting. if err := mount.CleanupMountPoint(stagingTargetPath, mountutil, true); err != nil { - glog.Warningf("error cleaning up mount point for volume %s: %v", vol.VolumeId, err) + glog.Errorf("error cleaning up mount point for volume %s: %v", vol.VolumeId, err) + return err } return nil -- cgit v1.2.3