feat: use a timed account cache

fix golint
kubernetes-sigs · Oct 9, 2021 · 26856c0 · 26856c0
1 parent eea17ea
commit 26856c0
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 34 deletions.
diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go
@@ -179,8 +179,8 @@ type Driver struct {
 	volumeLocks *volumeLocks
 	// a map storing all volumes created by this driver <volumeName, accountName>
 	volMap sync.Map
-	// a map storing all account name and keys retrieved by this driver <accountName, accountkey>
-	accountMap sync.Map
+	// a timed cache storing all account name and keys retrieved by this driver <accountName, accountkey>
+	accountCacheMap *azcache.TimedCache
 	// a map storing all secret names created by this driver <secretCacheKey, "">
 	secretCacheMap sync.Map
 	// a map storing all volumes using data plane API <volumeID, "">, <accountName, "">
@@ -208,16 +208,20 @@ func NewDriver(options *DriverOptions) *Driver {
 	getter := func(key string) (interface{}, error) {
 		return nil, nil
 	}
-	cache, err := azcache.NewTimedcache(time.Minute, getter)
-	if err != nil {
+
+	var err error
+	if driver.accountSearchCache, err = azcache.NewTimedcache(time.Minute, getter); err != nil {
 		klog.Fatalf("%v", err)
 	}
-	driver.accountSearchCache = cache
-	cache, err = azcache.NewTimedcache(time.Minute, getter)
-	if err != nil {
+
+	if driver.removeTagCache, err = azcache.NewTimedcache(time.Minute, getter); err != nil {
+		klog.Fatalf("%v", err)
+	}
+
+	if driver.accountCacheMap, err = azcache.NewTimedcache(5*time.Minute, getter); err != nil {
 		klog.Fatalf("%v", err)
 	}
-	driver.removeTagCache = cache
+
 	return &driver
 }
 
@@ -556,8 +560,12 @@ func (d *Driver) GetAccountInfo(ctx context.Context, volumeID string, secrets, r
 
 	if len(secrets) == 0 {
 		// read account key from cache first
-		if v, ok := d.accountMap.Load(accountName); ok {
-			accountKey = v.(string)
+		cache, errCache := d.accountCacheMap.Get(accountName, azcache.CacheReadTypeDefault)
+		if errCache != nil {
+			return rgName, accountName, accountKey, fileShareName, diskName, errCache
+		}
+		if cache != nil {
+			accountKey = cache.(string)
 		} else {
 			if secretName == "" && accountName != "" {
 				secretName = fmt.Sprintf(secretNameTemplate, accountName)
@@ -583,7 +591,7 @@ func (d *Driver) GetAccountInfo(ctx context.Context, volumeID string, secrets, r
 	}
 
 	if err == nil && accountKey != "" {
-		d.accountMap.Store(accountName, accountKey)
+		d.accountCacheMap.Set(accountName, accountKey)
 	}
 	return rgName, accountName, accountKey, fileShareName, diskName, err
 }
@@ -730,9 +738,13 @@ func (d *Driver) GetStorageAccesskey(ctx context.Context, accountOptions *azure.
 	}
 
 	accountName := accountOptions.Name
-	// read from cache first
-	if v, ok := d.accountMap.Load(accountName); ok {
-		return v.(string), nil
+	// read account key from cache first
+	cache, err := d.accountCacheMap.Get(accountName, azcache.CacheReadTypeDefault)
+	if err != nil {
+		return "", err
+	}
+	if cache != nil {
+		return cache.(string), nil
 	}
 
 	// read from k8s secret first
@@ -746,7 +758,7 @@ func (d *Driver) GetStorageAccesskey(ctx context.Context, accountOptions *azure.
 	}
 
 	if err == nil && accountKey != "" {
-		d.accountMap.Store(accountName, accountKey)
+		d.accountCacheMap.Set(accountName, accountKey)
 	}
 	return accountKey, err
 }

diff --git a/pkg/azurefile/controllerserver.go b/pkg/azurefile/controllerserver.go
@@ -309,7 +309,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 				d.accountSearchCache.Set(lockKey, accountName)
 				d.volMap.Store(volName, accountName)
 				if accountKey != "" {
-					d.accountMap.Store(accountName, accountKey)
+					d.accountCacheMap.Set(accountName, accountKey)
 				}
 			}
 		}

diff --git a/pkg/azurefile/controllerserver_test.go b/pkg/azurefile/controllerserver_test.go
@@ -1396,15 +1396,6 @@ func TestControllerPublishVolume(t *testing.T) {
 			},
 			expectedErr: nil,
 		},
-		{
-			desc: "Get account info returns error",
-			req: &csi.ControllerPublishVolumeRequest{
-				VolumeId:         "vol_2#f5713de20cde511e8ba4900#fileshare#diskname",
-				VolumeCapability: &stdVolCap,
-				NodeId:           "vm3",
-			},
-			expectedErr: status.Error(codes.InvalidArgument, "GetAccountInfo(vol_2#f5713de20cde511e8ba4900#fileshare#diskname) failed with error: Retriable: false, RetryAfter: 0s, HTTPStatusCode: 502, RawError: instance not found"),
-		},
 		{
 			desc: "Unsupported access mode",
 			req: &csi.ControllerPublishVolumeRequest{
@@ -1485,15 +1476,6 @@ func TestControllerUnpublishVolume(t *testing.T) {
 			},
 			expectedErr: nil,
 		},
-		{
-			desc: "Get account info returns error",
-			req: &csi.ControllerUnpublishVolumeRequest{
-				VolumeId: "vol_2#f5713de20cde511e8ba4901#fileshare#diskname#",
-				NodeId:   fakeNodeID,
-				Secrets:  map[string]string{},
-			},
-			expectedErr: status.Error(codes.InvalidArgument, "GetAccountInfo(vol_2#f5713de20cde511e8ba4901#fileshare#diskname#) failed with error: Retriable: false, RetryAfter: 0s, HTTPStatusCode: 502, RawError: instance not found"),
-		},
 	}
 
 	for _, test := range tests {