Merge pull request #1053 from kubernetes-sigs/fix-internal-error

andyzhangx · web-flow · commit 616deaafc0b2 · 2022-07-15T10:02:51.000+08:00
fix: CSI function should return internal error
diff --git a/charts/README.md b/charts/README.md
@@ -147,7 +147,7 @@ The following table lists the configurable parameters of the latest Azure File C
 | `linux.resources.livenessProbe.requests.cpu`           | liveness-probe cpu requests                    | 10m                                                            |
 | `linux.resources.livenessProbe.requests.memory`        | liveness-probe memory requests                 | 20Mi                                                           |
 | `linux.resources.nodeDriverRegistrar.limits.memory`    | csi-node-driver-registrar memory limits               | 100Mi                                                          |
-| `linux.resources.nodeDriverRegistrar.requests.cpu`     | csi-node-driver-registrar cpu requests         | 10m                                                            |
+| `linux.resources.nodeDriverRegistrar.requests.cpu`     | csi-node-driver-registrar cpu requests         | 30m                                                            |
 | `linux.resources.nodeDriverRegistrar.requests.memory`  | csi-node-driver-registrar memory requests      | 20Mi                                                           |
 | `linux.resources.azurefile.limits.memory`              | azurefile memory limits                         | 200Mi                                                         |
 | `linux.resources.azurefile.requests.cpu`               | azurefile cpu requests                   | 10m                                                            |
diff --git a/charts/latest/azurefile-csi-driver-v0.0.0.tgz b/charts/latest/azurefile-csi-driver-v0.0.0.tgz
diff --git a/charts/latest/azurefile-csi-driver/values.yaml b/charts/latest/azurefile-csi-driver/values.yaml
@@ -209,7 +209,7 @@ windows:
       limits:
         memory: 100Mi
       requests:
-        cpu: 10m
+        cpu: 30m
         memory: 40Mi
     azurefile:
       limits:
diff --git a/deploy/csi-azurefile-node-windows.yaml b/deploy/csi-azurefile-node-windows.yaml
@@ -88,7 +88,7 @@ spec:
             limits:
               memory: 100Mi
             requests:
-              cpu: 10m
+              cpu: 30m
               memory: 40Mi
         - name: azurefile
           image: mcr.microsoft.com/k8s/csi/azurefile-csi:latest
diff --git a/pkg/azurefile/controllerserver.go b/pkg/azurefile/controllerserver.go
@@ -333,7 +333,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 
 	tags, err := ConvertTagsToMap(customTags)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.InvalidArgument, err.Error())
 	}
 
 	accountOptions := &azure.AccountOptions{
@@ -367,7 +367,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 			// search in cache first
 			cache, err := d.accountSearchCache.Get(lockKey, azcache.CacheReadTypeDefault)
 			if err != nil {
-				return nil, err
+				return nil, status.Errorf(codes.Internal, err.Error())
 			}
 			if cache != nil {
 				accountName = cache.(string)
@@ -415,7 +415,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	if len(secret) == 0 && useDataPlaneAPI {
 		if accountKey == "" {
 			if accountKey, err = d.GetStorageAccesskey(ctx, accountOptions, secret, secretName, secretNamespace); err != nil {
-				return nil, fmt.Errorf("failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
+				return nil, status.Errorf(codes.Internal, "failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
 			}
 		}
 		secret = createStorageAccountSecret(accountName, accountKey)
@@ -457,20 +457,20 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 			d.volumeLocks.Release(volName)
 			// clean search cache
 			if err := d.accountSearchCache.Delete(lockKey); err != nil {
-				return nil, err
+				return nil, status.Errorf(codes.Internal, err.Error())
 			}
 			// remove the volName from the volMap to stop it matching the same storage account
 			d.volMap.Delete(volName)
 			return d.CreateVolume(ctx, req)
 		}
-		return nil, fmt.Errorf("failed to create file share(%s) on account(%s) type(%s) rg(%s) location(%s) size(%d), error: %v", validFileShareName, account, sku, resourceGroup, location, fileShareSize, err)
+		return nil, status.Errorf(codes.Internal, "failed to create file share(%s) on account(%s) type(%s) rg(%s) location(%s) size(%d), error: %v", validFileShareName, account, sku, resourceGroup, location, fileShareSize, err)
 	}
 	klog.V(2).Infof("create file share %s on storage account %s successfully", validFileShareName, accountName)
 
 	if isDiskFsType(fsType) && !strings.HasSuffix(diskName, vhdSuffix) {
 		if accountKey == "" {
 			if accountKey, err = d.GetStorageAccesskey(ctx, accountOptions, req.GetSecrets(), secretName, secretNamespace); err != nil {
-				return nil, fmt.Errorf("failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
+				return nil, status.Errorf(codes.Internal, "failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
 			}
 		}
 		if fileShareName == "" {
@@ -503,7 +503,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		if !useSeretCache {
 			if accountKey == "" {
 				if accountKey, err = d.GetStorageAccesskey(ctx, accountOptions, req.GetSecrets(), secretName, secretNamespace); err != nil {
-					return nil, fmt.Errorf("failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
+					return nil, status.Errorf(codes.Internal, "failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
 				}
 			}
 			storeSecretName, err := d.SetAzureCredentials(ctx, accountName, accountKey, secretName, secretNamespace)
diff --git a/pkg/azurefile/nodeserver.go b/pkg/azurefile/nodeserver.go
@@ -103,7 +103,7 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu
 	}
 
 	if err = preparePublishPath(target, d.mounter); err != nil {
-		return nil, fmt.Errorf("prepare publish failed for %s with error: %v", target, err)
+		return nil, status.Errorf(codes.Internal, "prepare publish failed for %s with error: %v", target, err)
 	}
 
 	klog.V(2).Infof("NodePublishVolume: mounting %s at %s with mountOptions: %v", source, target, mountOptions)
@@ -313,7 +313,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 			mountFsType = nfs
 		}
 		if err := prepareStagePath(cifsMountPath, d.mounter); err != nil {
-			return nil, fmt.Errorf("prepare stage path failed for %s with error: %v", cifsMountPath, err)
+			return nil, status.Errorf(codes.Internal, "prepare stage path failed for %s with error: %v", cifsMountPath, err)
 		}
 		if err := wait.PollImmediate(1*time.Second, 2*time.Minute, func() (bool, error) {
 			return true, SMBMount(d.mounter, source, cifsMountPath, mountFsType, mountOptions, sensitiveMountOptions)

Original file line number	Diff line number	Diff line change
`@@ -333,7 +333,7 @@ func (d Driver) CreateVolume(ctx context.Context, req csi.CreateVolumeRequest)`
`333`	`333`
`334`	`334`	`tags, err := ConvertTagsToMap(customTags)`
`335`	`335`	`if err != nil {`
`336`		`- return nil, err`
	`336`	`+ return nil, status.Errorf(codes.InvalidArgument, err.Error())`
`337`	`337`	`}`
`338`	`338`
`339`	`339`	`accountOptions := &azure.AccountOptions{`
`@@ -367,7 +367,7 @@ func (d Driver) CreateVolume(ctx context.Context, req csi.CreateVolumeRequest)`
`367`	`367`	`// search in cache first`
`368`	`368`	`cache, err := d.accountSearchCache.Get(lockKey, azcache.CacheReadTypeDefault)`
`369`	`369`	`if err != nil {`
`370`		`- return nil, err`
	`370`	`+ return nil, status.Errorf(codes.Internal, err.Error())`
`371`	`371`	`}`
`372`	`372`	`if cache != nil {`
`373`	`373`	`accountName = cache.(string)`
`@@ -415,7 +415,7 @@ func (d Driver) CreateVolume(ctx context.Context, req csi.CreateVolumeRequest)`
`415`	`415`	`if len(secret) == 0 && useDataPlaneAPI {`
`416`	`416`	`if accountKey == "" {`
`417`	`417`	`if accountKey, err = d.GetStorageAccesskey(ctx, accountOptions, secret, secretName, secretNamespace); err != nil {`
`418`		`- return nil, fmt.Errorf("failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)`
	`418`	`+ return nil, status.Errorf(codes.Internal, "failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)`
`419`	`419`	`}`
`420`	`420`	`}`
`421`	`421`	`secret = createStorageAccountSecret(accountName, accountKey)`
`@@ -457,20 +457,20 @@ func (d Driver) CreateVolume(ctx context.Context, req csi.CreateVolumeRequest)`
`457`	`457`	`d.volumeLocks.Release(volName)`
`458`	`458`	`// clean search cache`
`459`	`459`	`if err := d.accountSearchCache.Delete(lockKey); err != nil {`
`460`		`- return nil, err`
	`460`	`+ return nil, status.Errorf(codes.Internal, err.Error())`
`461`	`461`	`}`
`462`	`462`	`// remove the volName from the volMap to stop it matching the same storage account`
`463`	`463`	`d.volMap.Delete(volName)`
`464`	`464`	`return d.CreateVolume(ctx, req)`
`465`	`465`	`}`
`466`		`- return nil, fmt.Errorf("failed to create file share(%s) on account(%s) type(%s) rg(%s) location(%s) size(%d), error: %v", validFileShareName, account, sku, resourceGroup, location, fileShareSize, err)`
	`466`	`+ return nil, status.Errorf(codes.Internal, "failed to create file share(%s) on account(%s) type(%s) rg(%s) location(%s) size(%d), error: %v", validFileShareName, account, sku, resourceGroup, location, fileShareSize, err)`
`467`	`467`	`}`
`468`	`468`	`klog.V(2).Infof("create file share %s on storage account %s successfully", validFileShareName, accountName)`
`469`	`469`
`470`	`470`	`if isDiskFsType(fsType) && !strings.HasSuffix(diskName, vhdSuffix) {`
`471`	`471`	`if accountKey == "" {`
`472`	`472`	`if accountKey, err = d.GetStorageAccesskey(ctx, accountOptions, req.GetSecrets(), secretName, secretNamespace); err != nil {`
`473`		`- return nil, fmt.Errorf("failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)`
	`473`	`+ return nil, status.Errorf(codes.Internal, "failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)`
`474`	`474`	`}`
`475`	`475`	`}`
`476`	`476`	`if fileShareName == "" {`
`@@ -503,7 +503,7 @@ func (d Driver) CreateVolume(ctx context.Context, req csi.CreateVolumeRequest)`
`503`	`503`	`if !useSeretCache {`
`504`	`504`	`if accountKey == "" {`
`505`	`505`	`if accountKey, err = d.GetStorageAccesskey(ctx, accountOptions, req.GetSecrets(), secretName, secretNamespace); err != nil {`
`506`		`- return nil, fmt.Errorf("failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)`
	`506`	`+ return nil, status.Errorf(codes.Internal, "failed to GetStorageAccesskey on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)`
`507`	`507`	`}`
`508`	`508`	`}`
`509`	`509`	`storeSecretName, err := d.SetAzureCredentials(ctx, accountName, accountKey, secretName, secretNamespace)`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ func (d Driver) NodePublishVolume(ctx context.Context, req csi.NodePublishVolu`
`103`	`103`	`}`
`104`	`104`
`105`	`105`	`if err = preparePublishPath(target, d.mounter); err != nil {`
`106`		`- return nil, fmt.Errorf("prepare publish failed for %s with error: %v", target, err)`
	`106`	`+ return nil, status.Errorf(codes.Internal, "prepare publish failed for %s with error: %v", target, err)`
`107`	`107`	`}`
`108`	`108`
`109`	`109`	`klog.V(2).Infof("NodePublishVolume: mounting %s at %s with mountOptions: %v", source, target, mountOptions)`
`@@ -313,7 +313,7 @@ func (d Driver) NodeStageVolume(ctx context.Context, req csi.NodeStageVolumeRe`
`313`	`313`	`mountFsType = nfs`
`314`	`314`	`}`
`315`	`315`	`if err := prepareStagePath(cifsMountPath, d.mounter); err != nil {`
`316`		`- return nil, fmt.Errorf("prepare stage path failed for %s with error: %v", cifsMountPath, err)`
	`316`	`+ return nil, status.Errorf(codes.Internal, "prepare stage path failed for %s with error: %v", cifsMountPath, err)`
`317`	`317`	`}`
`318`	`318`	`if err := wait.PollImmediate(1time.Second, 2time.Minute, func() (bool, error) {`
`319`	`319`	`return true, SMBMount(d.mounter, source, cifsMountPath, mountFsType, mountOptions, sensitiveMountOptions)`