Skip to content

Commit

Permalink
fix: delete volume failure when management api is throttled
Browse files Browse the repository at this point in the history
fix
  • Loading branch information
andyzhangx committed Sep 12, 2022
1 parent 35d1be9 commit d2829ad
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 16 deletions.
28 changes: 21 additions & 7 deletions pkg/azurefile/azurefile.go
Expand Up @@ -223,8 +223,8 @@ type Driver struct {
accountCacheMap *azcache.TimedCache
// a map storing all secret names created by this driver <secretCacheKey, "">
secretCacheMap *azcache.TimedCache
// a map storing all volumes using data plane API <volumeID, "">, <accountName, "">
dataPlaneAPIVolMap sync.Map
// a timed cache storing all volumeIDs and storage accounts that are using data plane API
dataPlaneAPIVolCache *azcache.TimedCache
// a timed cache storing account search history (solve account list throttling issue)
accountSearchCache *azcache.TimedCache
// a timed cache storing tag removing history (solve account update throttling issue)
Expand Down Expand Up @@ -271,6 +271,10 @@ func NewDriver(options *DriverOptions) *Driver {
klog.Fatalf("%v", err)
}

if driver.dataPlaneAPIVolCache, err = azcache.NewTimedcache(10*time.Minute, getter); err != nil {
klog.Fatalf("%v", err)
}

return &driver
}

Expand Down Expand Up @@ -768,7 +772,7 @@ func (d *Driver) DeleteFileShare(resourceGroup, accountName, shareName string, s
klog.Warningf("DeleteFileShare(%s) on account(%s) failed with error(%v), waiting for retrying", shareName, accountName, err)
if strings.Contains(strings.ToLower(err.Error()), strings.ToLower(tooManyRequests)) {
klog.Warningf("switch to use data plane API instead for account %s since it's throttled", accountName)
d.dataPlaneAPIVolMap.Store(accountName, "")
d.dataPlaneAPIVolCache.Set(accountName, "")
return true, err
}
return false, nil
Expand Down Expand Up @@ -889,11 +893,21 @@ func (d *Driver) getSubnetResourceID() string {
}

func (d *Driver) useDataPlaneAPI(volumeID, accountName string) bool {
_, useDataPlaneAPI := d.dataPlaneAPIVolMap.Load(volumeID)
if !useDataPlaneAPI {
_, useDataPlaneAPI = d.dataPlaneAPIVolMap.Load(accountName)
cache, err := d.dataPlaneAPIVolCache.Get(volumeID, azcache.CacheReadTypeDefault)
if err != nil {
klog.Errorf("get(%s) from dataPlaneAPIVolCache failed with error: %v", volumeID, err)
}
if cache != nil {
return true
}
return useDataPlaneAPI
cache, err = d.dataPlaneAPIVolCache.Get(accountName, azcache.CacheReadTypeDefault)
if err != nil {
klog.Errorf("get(%s) from dataPlaneAPIVolCache failed with error: %v", accountName, err)
}
if cache != nil {
return true
}
return false
}

func (d *Driver) SetAzureCredentials(ctx context.Context, accountName, accountKey, secretName, secretNamespace string) (string, error) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/azurefile/controllerserver.go
Expand Up @@ -532,8 +532,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
volumeID = fmt.Sprintf(volumeIDTemplate, resourceGroup, accountName, validFileShareName, diskName, uuid, secretNamespace)

if useDataPlaneAPI {
d.dataPlaneAPIVolMap.Store(volumeID, "")
d.dataPlaneAPIVolMap.Store(accountName, "")
d.dataPlaneAPIVolCache.Set(volumeID, "")
d.dataPlaneAPIVolCache.Set(accountName, "")
}

isOperationSucceeded = true
Expand Down Expand Up @@ -691,8 +691,8 @@ func (d *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controlle
if !strings.HasSuffix(diskName, vhdSuffix) {
klog.V(2).Infof("skip ControllerPublishVolume(%s) since it's not vhd disk attach", volumeID)
if useDataPlaneAPI(volContext) {
d.dataPlaneAPIVolMap.Store(volumeID, "")
d.dataPlaneAPIVolMap.Store(accountName, "")
d.dataPlaneAPIVolCache.Set(volumeID, "")
d.dataPlaneAPIVolCache.Set(accountName, "")
}
return &csi.ControllerPublishVolumeResponse{}, nil
}
Expand Down
12 changes: 7 additions & 5 deletions pkg/azurefile/controllerserver_test.go
Expand Up @@ -26,9 +26,11 @@ import (
"strings"
"sync"
"testing"
"time"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network"
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/subnetclient/mocksubnetclient"
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-07-01/compute"
"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage"
Expand Down Expand Up @@ -1496,8 +1498,8 @@ func TestDeleteVolume(t *testing.T) {
},
},
}
d.dataPlaneAPIVolMap = sync.Map{}
d.dataPlaneAPIVolMap.Store("vol_1#f5713de20cde511e8ba4900#fileshare#diskname.vhd##secret", "1")
d.dataPlaneAPIVolCache, _ = azcache.NewTimedcache(10*time.Minute, func(key string) (interface{}, error) { return nil, nil })
d.dataPlaneAPIVolCache.Set("vol_1#f5713de20cde511e8ba4900#fileshare#diskname.vhd##secret", "1")
d.cloud = &azure.Cloud{}

expectedErr := status.Errorf(codes.NotFound, "get account info from(vol_1#f5713de20cde511e8ba4900#fileshare#diskname.vhd##secret) failed with error: could not get account key from secret(azure-storage-account-f5713de20cde511e8ba4900-secret): KubeClient is nil")
Expand Down Expand Up @@ -1748,7 +1750,7 @@ func TestControllerPublishVolume(t *testing.T) {
d.cloud = azure.GetTestCloud(ctrl)
d.cloud.Location = "centralus"
d.cloud.ResourceGroup = "rg"
d.dataPlaneAPIVolMap = sync.Map{}
d.dataPlaneAPIVolCache, _ = azcache.NewTimedcache(10*time.Minute, func(key string) (interface{}, error) { return nil, nil })
nodeName := "vm1"
instanceID := fmt.Sprintf("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/%s", nodeName)
vm := compute.VirtualMachine{
Expand Down Expand Up @@ -2234,8 +2236,8 @@ func TestControllerExpandVolume(t *testing.T) {
ResourceGroup: "vol_2",
},
}
d.dataPlaneAPIVolMap = sync.Map{}
d.dataPlaneAPIVolMap.Store("#f5713de20cde511e8ba4900#filename##secret", "1")
d.dataPlaneAPIVolCache, _ = azcache.NewTimedcache(10*time.Minute, func(key string) (interface{}, error) { return nil, nil })
d.dataPlaneAPIVolCache.Set("#f5713de20cde511e8ba4900#filename##secret", "1")

ctrl := gomock.NewController(t)
defer ctrl.Finish()
Expand Down

0 comments on commit d2829ad

Please sign in to comment.