Merge pull request #901 from huww98/attach-error-code

disk: refine attach error codes
kubernetes-sigs · Jan 8, 2024 · 5409261 · 5409261
2 parents f16f197 + 21a7239
commit 5409261
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 8 deletions.
diff --git a/pkg/disk/cloud.go b/pkg/disk/cloud.go
@@ -59,7 +59,7 @@ func attachDisk(tenantUserUID, diskID, nodeID string, isSharedDisk bool) (string
 		return "", status.Errorf(codes.Internal, "AttachDisk: find disk: %s with error: %s", diskID, err.Error())
 	}
 	if disk == nil {
-		return "", status.Errorf(codes.Internal, "AttachDisk: csi can't find disk: %s in region: %s, Please check if the cloud disk exists, if the region is correct, or if the csi permissions are correct", diskID, GlobalConfigVar.Region)
+		return "", status.Errorf(codes.NotFound, "AttachDisk: csi can't find disk: %s in region: %s, Please check if the cloud disk exists, if the region is correct, or if the csi permissions are correct", diskID, GlobalConfigVar.Region)
 	}
 
 	if !GlobalConfigVar.ADControllerEnable {
@@ -188,12 +188,18 @@ func attachDisk(tenantUserUID, diskID, nodeID string, isSharedDisk bool) (string
 	}
 	response, err := ecsClient.AttachDisk(attachRequest)
 	if err != nil {
-		if strings.Contains(err.Error(), DiskLimitExceeded) {
-			return "", status.Error(codes.Internal, err.Error()+", Node("+nodeID+")exceed the limit attachments of disk")
-		} else if strings.Contains(err.Error(), DiskNotPortable) {
-			return "", status.Error(codes.Internal, err.Error()+", Disk("+diskID+") should be \"Pay by quantity\", not be \"Annual package\", please check and modify the charge type, and refer to: https://help.aliyun.com/document_detail/134767.html")
-		} else if strings.Contains(err.Error(), NotSupportDiskCategory) {
-			return "", status.Error(codes.Internal, err.Error()+", Disk("+diskID+") is not supported by instance, please refer to: https://help.aliyun.com/document_detail/25378.html")
+		var aliErr *alicloudErr.ServerError
+		if errors.As(err, &aliErr) {
+			switch aliErr.ErrorCode() {
+			case InstanceNotFound:
+				return "", status.Errorf(codes.NotFound, "Node(%s) not found, request ID: %s", nodeID, aliErr.RequestId())
+			case DiskLimitExceeded:
+				return "", status.Errorf(codes.Internal, "%v, Node(%s) exceed the limit attachments of disk", err, nodeID)
+			case DiskNotPortable:
+				return "", status.Errorf(codes.Internal, "%v, Disk(%s) should be \"Pay by quantity\", not be \"Annual package\", please check and modify the charge type, and refer to: https://help.aliyun.com/document_detail/134767.html", err, diskID)
+			case NotSupportDiskCategory:
+				return "", status.Errorf(codes.Internal, "%v, Disk(%s) is not supported by instance, please refer to: https://help.aliyun.com/document_detail/25378.html", err, diskID)
+			}
 		}
 		return "", status.Errorf(codes.Aborted, "NodeStageVolume: Error happens to attach disk %s to instance %s, %v", diskID, nodeID, err)
 	}

diff --git a/pkg/disk/constants.go b/pkg/disk/constants.go
@@ -112,6 +112,7 @@ const (
 	DiskNotPortable              = "DiskNotPortable"
 	IdempotentParameterMismatch  = "IdempotentParameterMismatch"
 	SnapshotNotFound             = "InvalidSnapshotId.NotFound"
+	InstanceNotFound             = "InvalidInstanceId.NotFound"
 
 	// DiskHighAvail tag
 	DiskHighAvail = "available"

diff --git a/pkg/disk/controllerserver.go b/pkg/disk/controllerserver.go
@@ -391,7 +391,7 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs
 	_, err := attachDisk(req.VolumeContext[TenantUserUID], req.VolumeId, req.NodeId, isSharedDisk)
 	if err != nil {
 		log.Log.Errorf("ControllerPublishVolume: attach disk: %s to node: %s with error: %s", req.VolumeId, req.NodeId, err.Error())
-		return nil, status.Error(codes.Aborted, err.Error())
+		return nil, err
 	}
 	log.Log.Infof("ControllerPublishVolume: Successful attach disk: %s to node: %s", req.VolumeId, req.NodeId)
 	return &csi.ControllerPublishVolumeResponse{}, nil