diff --git a/deploy/kubernetes/images/alpha/image.yaml b/deploy/kubernetes/images/alpha/image.yaml new file mode 100644 index 000000000..8c6f62d14 --- /dev/null +++ b/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" diff --git a/deploy/kubernetes/images/alpha/kustomization.yaml b/deploy/kubernetes/images/alpha/kustomization.yaml index 703ce473d..2dfbdfd45 100644 --- a/deploy/kubernetes/images/alpha/kustomization.yaml +++ b/deploy/kubernetes/images/alpha/kustomization.yaml @@ -2,3 +2,4 @@ namespace: gce-pd-csi-driver resources: - ../stable-master/ +- image.yaml diff --git a/deploy/kubernetes/overlays/alpha/controller_readonly.yaml b/deploy/kubernetes/overlays/alpha/controller_readonly.yaml new file mode 100644 index 000000000..7ff125cfd --- /dev/null +++ b/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" diff --git a/deploy/kubernetes/overlays/alpha/kustomization.yaml b/deploy/kubernetes/overlays/alpha/kustomization.yaml index ebf7e4566..383e6ab34 100644 --- a/deploy/kubernetes/overlays/alpha/kustomization.yaml +++ b/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 diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index 733eae21a..2ffa96e87 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/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 { diff --git a/pkg/gce-pd-csi-driver/controller_test.go b/pkg/gce-pd-csi-driver/controller_test.go index db8ad905e..14f203614 100644 --- a/pkg/gce-pd-csi-driver/controller_test.go +++ b/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", diff --git a/pkg/gce-pd-csi-driver/node.go b/pkg/gce-pd-csi-driver/node.go index c98e6636b..cd45e8f4f 100644 --- a/pkg/gce-pd-csi-driver/node.go +++ b/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)) diff --git a/pkg/gce-pd-csi-driver/utils.go b/pkg/gce-pd-csi-driver/utils.go index 9a394056e..5478971a7 100644 --- a/pkg/gce-pd-csi-driver/utils.go +++ b/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 diff --git a/pkg/gce-pd-csi-driver/utils_test.go b/pkg/gce-pd-csi-driver/utils_test.go index 53c134b18..2a0fd5f45 100644 --- a/pkg/gce-pd-csi-driver/utils_test.go +++ b/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) + } + } + } +}