turn on controller-publish-readonly flag and add validation in pd-csi driver for when readonly is on

Author: leiyiz

deploy/kubernetes/base/controller/controller.yaml

@@ -36,6 +36,7 @@ spec:
           # - "--run-node-service=false"        # disable the node service of the CSI driver
             - "--leader-election"
             - "--default-fstype=ext4"
+            - "--controller-publish-readonly=true"
           env:
             - name: PDCSI_NAMESPACE
               valueFrom:

deploy/kubernetes/images/stable-master/image.yaml

@@ -4,7 +4,7 @@ metadata:
   name: imagetag-csi-provisioner
 imageTag:
   name: k8s.gcr.io/sig-storage/csi-provisioner
-  newTag: "v2.2.1"
+  newTag: "v3.0.0"
 
 ---
 apiVersion: builtin

pkg/gce-pd-csi-driver/controller.go

@@ -204,6 +204,10 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 				return nil, status.Errorf(codes.NotFound, "CreateVolume source snapshot %s does not exist", snapshotID)
 			}
 		}
+	} else { // if VolumeContentSource is nil, validate access mode is not read only
+		if readonly, _ := getReadOnlyFromCapabilities(volumeCapabilities); readonly {
+			return nil, status.Error(codes.InvalidArgument, "VolumeContentSource must be provided when AccessMode is set to read only")
+		}
 	}
 
 	// Create the disk

pkg/gce-pd-csi-driver/controller_test.go

@@ -356,19 +356,14 @@ func TestCreateVolumeArguments(t *testing.T) {
 			},
 		},
 		{
-			name: "success with MULTI_NODE_READER_ONLY",
+			name: "fail with MULTI_NODE_READER_ONLY",
 			req: &csi.CreateVolumeRequest{
 				Name:               "test-name",
 				CapacityRange:      stdCapRange,
 				VolumeCapabilities: createVolumeCapabilities(csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY),
 				Parameters:         stdParams,
 			},
-			expVol: &csi.Volume{
-				CapacityBytes:      common.GbToBytes(20),
-				VolumeId:           testVolumeID,
-				VolumeContext:      nil,
-				AccessibleTopology: stdTopology,
-			},
+			expErrCode: codes.InvalidArgument,
 		},
 		{
 			name: "fail with mount/MULTI_NODE_MULTI_WRITER capabilities",

pkg/gce-pd-csi-driver/utils.go

@@ -160,6 +160,31 @@ func getMultiWriterFromCapabilities(vcs []*csi.VolumeCapability) (bool, error) {
 	return false, nil
 }
 
+func getReadOnlyFromCapability(vc *csi.VolumeCapability) (bool, error) {
+	if vc.GetAccessMode() == nil {
+		return false, errors.New("access mode is nil")
+	}
+	mode := vc.GetAccessMode().GetMode()
+	return (mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY ||
+		mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY), nil
+}
+
+func getReadOnlyFromCapabilities(vcs []*csi.VolumeCapability) (bool, error) {
+	if vcs == nil {
+		return false, errors.New("volume capabilities is nil")
+	}
+	for _, vc := range vcs {
+		readOnly, err := getReadOnlyFromCapability(vc)
+		if err != nil {
+			return false, err
+		}
+		if readOnly {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 func collectMountOptions(fsType string, mntFlags []string) []string {
 	var options []string
 

pkg/gce-pd-csi-driver/utils_test.go

@@ -234,3 +234,60 @@ func TestGetMultiWriterFromCapabilities(t *testing.T) {
 		}
 	}
 }
+
+func TestGetReadOnlyFromCapabilities(t *testing.T) {
+	testCases := []struct {
+		name   string
+		vc     []*csi.VolumeCapability
+		expVal bool
+		expErr bool
+	}{
+		{
+			name:   "false with empty capabilities",
+			vc:     []*csi.VolumeCapability{},
+			expVal: false,
+		},
+		{
+			name: "fail with capabilities no access mode",
+			vc: []*csi.VolumeCapability{
+				{
+					AccessType: &csi.VolumeCapability_Mount{
+						Mount: &csi.VolumeCapability_MountVolume{},
+					},
+				},
+			},
+			expErr: true,
+		},
+		{
+			name:   "false with SINGLE_NODE_WRITER capabilities",
+			vc:     createVolumeCapabilities(csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER),
+			expVal: false,
+		},
+		{
+			name:   "true with MULTI_NODE_READER_ONLY capabilities",
+			vc:     createBlockVolumeCapabilities(csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY),
+			expVal: true,
+		},
+		{
+			name:   "true with SINGLE_NODE_READER_ONLY capabilities",
+			vc:     createVolumeCapabilities(csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY),
+			expVal: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Logf("Running test: %v", tc.name)
+		val, err := getReadOnlyFromCapabilities(tc.vc)
+		if tc.expErr && err == nil {
+			t.Fatalf("Expected error but didn't get any")
+		}
+		if !tc.expErr && err != nil {
+			t.Fatalf("Did not expect error but got: %v", err)
+		}
+		if err != nil {
+			if tc.expVal != val {
+				t.Fatalf("Expected '%t' but got '%t'", tc.expVal, val)
+			}
+		}
+	}
+}