Merge pull request #869 from leiyiz/rox-fix

pdcsi rox workflow fix

Author: k8s-ci-robot

deploy/kubernetes/images/alpha/image.yaml

@@ -0,0 +1,7 @@
+apiVersion: builtin
+kind: ImageTagTransformer
+metadata:
+  name: imagetag-csi-provisioner-alpha
+imageTag:
+  name: k8s.gcr.io/sig-storage/csi-provisioner
+  newTag: "v3.0.0"

deploy/kubernetes/images/alpha/kustomization.yaml

@@ -2,3 +2,4 @@ namespace:
   gce-pd-csi-driver
 resources:
 - ../stable-master/
+- image.yaml

deploy/kubernetes/overlays/alpha/controller_readonly.yaml

@@ -0,0 +1,20 @@
+kind: Deployment
+apiVersion: apps/v1
+metadata:
+  name: csi-gce-pd-controller
+spec:
+  template:
+    spec:
+      containers:
+        - name: csi-provisioner
+          args:
+            - "--v=5"
+            - "--csi-address=/csi/csi.sock"
+            - "--feature-gates=Topology=true"
+            - "--http-endpoint=:22011"
+            - "--leader-election-namespace=$(PDCSI_NAMESPACE)"
+            - "--timeout=250s"
+            - "--extra-create-metadata"
+            - "--leader-election"
+            - "--default-fstype=ext4"
+            - "--controller-publish-readonly=true"

deploy/kubernetes/overlays/alpha/kustomization.yaml

@@ -3,5 +3,7 @@ kind: Kustomization
 namespace: gce-pd-csi-driver
 resources:
 - ../../base/
+patchesStrategicMerge:
+- controller_readonly.yaml
 transformers:
 - ../../images/alpha

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

@@ -252,7 +252,12 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 				return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk from source volume %v is not ready", sourceVolKey))
 			}
 		}
+	} 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
 	var disk *gce.CloudDisk
 	switch params.ReplicationType {

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

@@ -358,19 +358,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/node.go

@@ -60,6 +60,7 @@ const (
 	volumeLimitBig       int64 = 127
 	defaultLinuxFsType         = "ext4"
 	defaultWindowsFsType       = "ntfs"
+	fsTypeExt3                 = "ext3"
 )
 
 func getDefaultFsType() string {
@@ -311,8 +312,29 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage
 		return &csi.NodeStageVolumeResponse{}, nil
 	}
 
+	readonly, _ := getReadOnlyFromCapability(volumeCapability)
+	if readonly {
+		options = append(options, "ro")
+		klog.V(4).Infof("CSI volume is read-only, mounting with extra option ro")
+	}
+
 	err = formatAndMount(devicePath, stagingTargetPath, fstype, options, ns.Mounter)
 	if err != nil {
+		// If a volume is created from a content source like snapshot or cloning, the filesystem might get marked
+		// as "dirty" even if it is otherwise consistent and ext3/4 will try to restore to a consistent state by replaying
+		// the journal which is not possible in read-only mode. So we'll try again with noload option to skip it. This may
+		// allow mounting of an actually inconsistent filesystem, but because the mount is read-only no further damage should
+		// be caused.
+		if readonly && (fstype == defaultLinuxFsType || fstype == fsTypeExt3) {
+			klog.V(4).Infof("Failed to mount CSI volume read-only, retry mounting with extra option noload")
+
+			options = append(options, "noload")
+			err = formatAndMount(devicePath, stagingTargetPath, fstype, options, ns.Mounter)
+			if err == nil {
+				klog.V(4).Infof("NodeStageVolume succeeded with \"noload\" option on %v to %s", volumeID, stagingTargetPath)
+				return &csi.NodeStageVolumeResponse{}, nil
+			}
+		}
 		return nil, status.Error(codes.Internal,
 			fmt.Sprintf("Failed to format and mount device from (%q) to (%q) with fstype (%q) and options (%q): %v",
 				devicePath, stagingTargetPath, fstype, options, err))

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)
+			}
+		}
+	}
+}