Merge pull request #559 from chrisThePattyEater/main

Allow multiple volume handles with different mount options

Author: chrisThePattyEater

pkg/csi_driver/controller.go

@@ -76,6 +76,9 @@ func (s *controllerServer) ControllerGetCapabilities(_ context.Context, _ *csi.C
 func (s *controllerServer) ValidateVolumeCapabilities(ctx context.Context, req *csi.ValidateVolumeCapabilitiesRequest) (*csi.ValidateVolumeCapabilitiesResponse, error) {
 	// Validate arguments
 	volumeID := req.GetVolumeId()
+	if req.GetVolumeContext()[VolumeContextKeyEphemeral] != util.TrueStr {
+		volumeID = parseVolumeID(volumeID)
+	}
 	if len(volumeID) == 0 {
 		return nil, status.Error(codes.InvalidArgument, "ValidateVolumeCapabilities volumeID must be provided")
 	}

pkg/csi_driver/utils.go

@@ -68,6 +68,8 @@ const (
 	tokenServerSidecarMinVersion        = "v1.12.2-gke.0" // #nosec G101
 )
 
+var volumeIDRegEx = regexp.MustCompile(`:.*$`)
+
 func NewVolumeCapabilityAccessMode(mode csi.VolumeCapability_AccessMode_Mode) *csi.VolumeCapability_AccessMode {
 	return &csi.VolumeCapability_AccessMode{Mode: mode}
 }
@@ -271,14 +273,13 @@ func parseRequestArguments(req *csi.NodePublishVolumeRequest) (string, string, [
 	}
 
 	vc := req.GetVolumeContext()
-	bucketName := req.GetVolumeId()
+	bucketName := parseVolumeID(req.GetVolumeId())
 	if vc[VolumeContextKeyEphemeral] == util.TrueStr {
 		bucketName = vc[VolumeContextKeyBucketName]
 		if len(bucketName) == 0 {
 			return "", "", nil, false, false, fmt.Errorf("NodePublishVolume VolumeContext %q must be provided for ephemeral storage", VolumeContextKeyBucketName)
 		}
 	}
-
 	fuseMountOptions := []string{}
 	if req.GetReadonly() {
 		fuseMountOptions = joinMountOptions(fuseMountOptions, []string{"ro"})
@@ -302,6 +303,13 @@ func parseRequestArguments(req *csi.NodePublishVolumeRequest) (string, string, [
 	return targetPath, bucketName, fuseMountOptions, skipCSIBucketAccessCheck, enableMetricsCollection, nil
 }
 
+// The format allows customers to specify a fake volume handle for static provisioning,
+// enabling multiple PVs in the same pod to mount the same bucket. This prevents Kubelet from
+// skipping mounts of volumes with the same volume handle, which can cause the pod to be stuck in container creation.
+func parseVolumeID(bucketHandle string) string {
+	return volumeIDRegEx.ReplaceAllString(bucketHandle, "")
+}
+
 func putExitFile(pod *corev1.Pod, targetPath string) error {
 	podIsTerminating := pod.DeletionTimestamp != nil
 	podRestartPolicyIsNever := pod.Spec.RestartPolicy == corev1.RestartPolicyNever

pkg/csi_driver/utils_test.go

@@ -29,6 +29,56 @@ const (
 	TraceStr = "trace"
 )
 
+func TestRemoveBucketSuffixIfPresentAndReturnVolumeId(t *testing.T) {
+	t.Parallel()
+	t.Run("removing bucket suffix if present and returning volume id", func(t *testing.T) {
+		t.Parallel()
+		testCases := []struct {
+			name          string
+			bucketName    string
+			expectedValue string
+		}{
+			{
+				name:          "should return bucket name without suffix, no special character",
+				bucketName:    "bucket-name:1234567890123456789",
+				expectedValue: "bucket-name",
+			},
+			{
+				name:          "should return bucket name without suffix, special character",
+				bucketName:    "bucket-name:1234567890123456789@us-central1",
+				expectedValue: "bucket-name",
+			},
+			{
+				name:          "should return bucket name without suffix, no colon with special character",
+				bucketName:    "bucket-name@us-central1",
+				expectedValue: "bucket-name@us-central1",
+			},
+			{
+				name:          "should return bucket name without suffix, two colons",
+				bucketName:    "bucket-name:1234567890123456789:12",
+				expectedValue: "bucket-name",
+			},
+			{
+				name:          "should return bucket name without suffix, no special character and no colon",
+				bucketName:    "bucket-name",
+				expectedValue: "bucket-name",
+			},
+			{
+				name:          "should return bucket name without suffix, no bucket name",
+				bucketName:    ":xyz",
+				expectedValue: "",
+			},
+		}
+
+		for _, tc := range testCases {
+			t.Logf("test case: %s", tc.name)
+			actual := parseVolumeID(tc.bucketName)
+			if actual != tc.expectedValue {
+				t.Errorf("Got value %v, but expected %v", actual, tc.expectedValue)
+			}
+		}
+	})
+}
 func TestJoinMountOptions(t *testing.T) {
 	t.Parallel()
 	t.Run("joining mount options into one", func(t *testing.T) {