From f8acafdbeec04e3812aa59a73a10fb0fd904fd94 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Thu, 29 Aug 2024 10:03:53 +0000 Subject: [PATCH 1/2] fix: delete volume error in archive deletion mode --- pkg/smb/controllerserver.go | 17 +++++++++++++++++ pkg/smb/smb.go | 5 +++++ 2 files changed, 22 insertions(+) diff --git a/pkg/smb/controllerserver.go b/pkg/smb/controllerserver.go index 9f9eb6a9cd5..e0252d052ff 100644 --- a/pkg/smb/controllerserver.go +++ b/pkg/smb/controllerserver.go @@ -28,6 +28,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "k8s.io/klog/v2" + azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" ) const ( @@ -147,6 +148,11 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) return &csi.DeleteVolumeResponse{}, nil } + if acquired := d.volumeLocks.TryAcquire(volumeID); !acquired { + return nil, status.Errorf(codes.Aborted, volumeOperationAlreadyExistsFmt, volumeID) + } + defer d.volumeLocks.Release(volumeID) + var volCap *csi.VolumeCapability secrets := req.GetSecrets() mountOptions := getMountOptions(secrets) @@ -167,6 +173,16 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) if len(req.GetSecrets()) > 0 && !strings.EqualFold(smbVol.onDelete, retain) { klog.V(2).Infof("begin to delete or archive subdirectory since secret is provided") + // check whether volumeID is in the cache + cache, err := d.volDeletionCache.Get(volumeID, azcache.CacheReadTypeDefault) + if err != nil { + return nil, status.Errorf(codes.Internal, err.Error()) + } + if cache != nil { + klog.V(2).Infof("DeleteVolume: volume %s is already deleted", volumeID) + return &csi.DeleteVolumeResponse{}, nil + } + // mount smb base share so we can delete or archive the subdirectory if err = d.internalMount(ctx, smbVol, volCap, secrets); err != nil { return nil, status.Errorf(codes.Internal, "failed to mount smb server: %v", err.Error()) @@ -211,6 +227,7 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) klog.V(2).Infof("DeleteVolume(%s) does not delete subdirectory", volumeID) } + d.volDeletionCache.Set(volumeID, "") return &csi.DeleteVolumeResponse{}, nil } diff --git a/pkg/smb/smb.go b/pkg/smb/smb.go index d98de18dae2..a62a83dedba 100644 --- a/pkg/smb/smb.go +++ b/pkg/smb/smb.go @@ -82,6 +82,8 @@ type Driver struct { enableGetVolumeStats bool // a timed cache storing volume stats volStatsCache azcache.Resource + // a timed cache storing volume deletion records + volDeletionCache azcache.Resource // this only applies to Windows node removeSMBMappingDuringUnmount bool krb5CacheDirectory string @@ -120,6 +122,9 @@ func NewDriver(options *DriverOptions) *Driver { if driver.volStatsCache, err = azcache.NewTimedCache(time.Duration(options.VolStatsCacheExpireInMinutes)*time.Minute, getter, false); err != nil { klog.Fatalf("%v", err) } + if driver.volDeletionCache, err = azcache.NewTimedCache(time.Minute, getter, false); err != nil { + klog.Fatalf("%v", err) + } return &driver } From 0cd70c4b0c55f5a7477c9aa0115e436367910587 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Fri, 30 Aug 2024 14:38:28 +0000 Subject: [PATCH 2/2] fix: add lock in CreateVolume to avoid race condition --- pkg/smb/controllerserver.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/smb/controllerserver.go b/pkg/smb/controllerserver.go index e0252d052ff..377b21f8b40 100644 --- a/pkg/smb/controllerserver.go +++ b/pkg/smb/controllerserver.go @@ -99,6 +99,11 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) } } + if acquired := d.volumeLocks.TryAcquire(name); !acquired { + return nil, status.Errorf(codes.Aborted, volumeOperationAlreadyExistsFmt, name) + } + defer d.volumeLocks.Release(name) + if createSubDir { // Mount smb base share so we can create a subdirectory if err := d.internalMount(ctx, smbVol, volCap, secrets); err != nil {