diff --git a/docs/parameters.md b/docs/parameters.md index 48742f1335..49bedca02b 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -14,6 +14,7 @@ The AWS EBS CSI Driver supports [tagging](tagging.md) through `StorageClass.para | "encrypted" | true, false | false | Whether the volume should be encrypted or not. Valid values are "true" or "false". | | "blockExpress" | true, false | false | Enables the creation of [io2 Block Express volumes](https://aws.amazon.com/ebs/provisioned-iops/#Introducing_io2_Block_Express) by increasing the IOPS limit for io2 volumes to 256000. Volumes created with more than 64000 IOPS will fail to mount on instances that do not support io2 Block Express. | | "kmsKeyId" | | | The full ARN of the key to use when encrypting the volume. If not specified, AWS will use the default KMS key for the region the volume is in. This will be an auto-generated key called `/aws/ebs` if not changed. | +| "blockSize" | | | The block size to use when formatting the underlying filesystem. Only supported on linux nodes and with fstype `ext2`, `ext3`, `ext4`, or `xfs`. | **Appendix** * `gp3` is currently not supported on outposts. Outpost customers need to use a different type for their volumes. diff --git a/go.mod b/go.mod index 995dd6fc84..1bbced2ca3 100644 --- a/go.mod +++ b/go.mod @@ -20,9 +20,9 @@ require ( k8s.io/component-base v0.25.3 k8s.io/klog/v2 v2.80.1 k8s.io/kubernetes v1.25.3 - k8s.io/mount-utils v0.25.3 + k8s.io/mount-utils v0.26.0-rc.0 k8s.io/pod-security-admission v0.25.3 - k8s.io/utils v0.0.0-20221012122500-cfd413dd9e85 + k8s.io/utils v0.0.0-20221107191617-1a15be271d1d ) require ( @@ -132,7 +132,7 @@ replace ( k8s.io/kubelet => k8s.io/kubelet v0.25.3 k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.25.3 k8s.io/metrics => k8s.io/metrics v0.25.3 - k8s.io/mount-utils => k8s.io/mount-utils v0.25.3 + k8s.io/mount-utils => k8s.io/mount-utils v0.26.0-rc.0 k8s.io/node-api => k8s.io/node-api v0.25.3 k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.25.3 k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.25.3 diff --git a/go.sum b/go.sum index bb0a8a43b7..e567186f88 100644 --- a/go.sum +++ b/go.sum @@ -948,14 +948,14 @@ k8s.io/kubelet v0.25.3 h1:PjT3Xo0VL1BpRilBpZrRN8pSy6w5pGQ0YDQQeQWSHvQ= k8s.io/kubelet v0.25.3/go.mod h1:YopVc6vLhveZb22I7AzcoWPap+t3/KJKqRZDa2MZmyE= k8s.io/kubernetes v1.25.3 h1:Ljx/Ew9+dt7rN9ob3V+N/aoDy7nDSbmr35IbYGRTyqE= k8s.io/kubernetes v1.25.3/go.mod h1:lvEY+3iJhh+sGIK1LorGkI56rW0eLGsfalnp68wQwYU= -k8s.io/mount-utils v0.25.3 h1:Eb4MDClmozX3Vrz4ZtoG0bQ/pGhT5gyo28p3f+0r9EE= -k8s.io/mount-utils v0.25.3/go.mod h1:odpFnGwJfFjN3SRnjfGS0902ubcj/W6hDOrNDmSSINo= +k8s.io/mount-utils v0.26.0-rc.0 h1:+eYfAi3jbs0to42UCMhad7I3W/as5YMxaF0ih3PlnNA= +k8s.io/mount-utils v0.26.0-rc.0/go.mod h1:mFGnSO6Hc1qzZsFZ73pe724HgXtjXZBWfmkJwEXtQjU= k8s.io/pod-security-admission v0.25.3 h1:2HnXWKUIDSez2sWtvxeGgGVUFvYnJJHutL4AI1MIuwk= k8s.io/pod-security-admission v0.25.3/go.mod h1:xSaLkcMPD6cGKrZ//ZUrCNs0BewZzQdOEcC9LuXBGR4= k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= -k8s.io/utils v0.0.0-20221012122500-cfd413dd9e85 h1:cTdVh7LYu82xeClmfzGtgyspNh6UxpwLWGi8R4sspNo= -k8s.io/utils v0.0.0-20221012122500-cfd413dd9e85/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20221107191617-1a15be271d1d h1:0Smp/HP1OH4Rvhe+4B8nWGERtlqAGSftbSbbmm45oFs= +k8s.io/utils v0.0.0-20221107191617-1a15be271d1d/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= diff --git a/pkg/driver/constants.go b/pkg/driver/constants.go index ca3bf091b8..57dbd6eea5 100644 --- a/pkg/driver/constants.go +++ b/pkg/driver/constants.go @@ -72,6 +72,9 @@ const ( // BlockExpressKey increases the iops limit for io2 volumes to the block express limit BlockExpressKey = "blockexpress" + // BlockSizeKey configures the block size when formatting a volume + BlockSizeKey = "blocksize" + // TagKeyPrefix contains the prefix of a volume parameter that designates it as // a tag to be attached to the resource TagKeyPrefix = "tagSpecification" diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 1019cccde0..58198aa849 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -132,6 +132,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol cloud.VolumeNameTagKey: volName, cloud.AwsEbsDriverTagKey: isManagedByDriver, } + blockSize string ) tProps := new(template.Props) @@ -178,6 +179,12 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol if value == "true" { blockExpress = true } + case BlockSizeKey: + _, err = strconv.Atoi(value) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "Could not parse blockSize: %v", err) + } + blockSize = value default: if strings.HasPrefix(key, TagKeyPrefix) { scTags = append(scTags, value) @@ -187,6 +194,26 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol } } + if len(blockSize) > 0 { + for _, volCap := range req.GetVolumeCapabilities() { + switch volCap.GetAccessType().(type) { + case *csi.VolumeCapability_Block: + return nil, status.Error(codes.InvalidArgument, "Cannot use block size with block volume") + } + + mountVolume := volCap.GetMount() + if mountVolume == nil { + return nil, status.Error(codes.InvalidArgument, "CreateVolume: mount is nil within volume capability") + } + + fsType := mountVolume.GetFsType() + + if fsType != "ext2" && fsType != "ext3" && fsType != "ext4" && fsType != "xfs" { + return nil, status.Errorf(codes.InvalidArgument, "Cannot use block size with fstype %s", fsType) + } + } + } + if volumeType == cloud.VolumeTypeIO1 { if iopsPerGB == 0 { return nil, status.Errorf(codes.InvalidArgument, "The parameter IOPSPerGB must be specified for io1 volumes") @@ -265,7 +292,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol } return nil, status.Errorf(errCode, "Could not create volume %q: %v", volName, err) } - return newCreateVolumeResponse(disk), nil + return newCreateVolumeResponse(disk, blockSize), nil } func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error { @@ -754,7 +781,7 @@ func getOutpostArn(requirement *csi.TopologyRequirement) string { return "" } -func newCreateVolumeResponse(disk *cloud.Disk) *csi.CreateVolumeResponse { +func newCreateVolumeResponse(disk *cloud.Disk, blockSize string) *csi.CreateVolumeResponse { var src *csi.VolumeContentSource if disk.SnapshotID != "" { src = &csi.VolumeContentSource{ @@ -777,11 +804,16 @@ func newCreateVolumeResponse(disk *cloud.Disk) *csi.CreateVolumeResponse { segments[AwsOutpostIDKey] = strings.ReplaceAll(arn.Resource, "outpost/", "") } + context := map[string]string{} + if len(blockSize) > 0 { + context[BlockSizeKey] = blockSize + } + return &csi.CreateVolumeResponse{ Volume: &csi.Volume{ VolumeId: disk.VolumeID, CapacityBytes: util.GiBToBytes(disk.CapacityGiB), - VolumeContext: map[string]string{}, + VolumeContext: context, AccessibleTopology: []*csi.Topology{ { Segments: segments, diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 1e704fd3ee..0f3765e1d3 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -147,7 +147,9 @@ func TestCreateVolume(t *testing.T) { stdVolCap := []*csi.VolumeCapability{ { AccessType: &csi.VolumeCapability_Mount{ - Mount: &csi.VolumeCapability_MountVolume{}, + Mount: &csi.VolumeCapability_MountVolume{ + FsType: "ext4", + }, }, AccessMode: &csi.VolumeCapability_AccessMode{ Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, @@ -1585,6 +1587,57 @@ func TestCreateVolume(t *testing.T) { checkExpectedErrorCode(t, err, codes.AlreadyExists) }, }, + { + name: "success with block size", + testFunc: func(t *testing.T) { + req := &csi.CreateVolumeRequest{ + Name: "random-vol-name", + CapacityRange: stdCapRange, + VolumeCapabilities: stdVolCap, + Parameters: map[string]string{ + BlockSizeKey: "4096", + }, + } + + ctx := context.Background() + + mockDisk := &cloud.Disk{ + VolumeID: req.Name, + AvailabilityZone: expZone, + CapacityGiB: util.BytesToGiB(stdVolSize), + } + + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockCloud := cloud.NewMockCloud(mockCtl) + mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.Name), gomock.Any()).Return(mockDisk, nil) + + awsDriver := controllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + driverOptions: &DriverOptions{}, + } + + response, err := awsDriver.CreateVolume(ctx, req) + if err != nil { + srvErr, ok := status.FromError(err) + if !ok { + t.Fatalf("Could not get error status code from error: %v", srvErr) + } + t.Fatalf("Unexpected error: %v", srvErr.Code()) + } + + context := response.Volume.VolumeContext + if blockSize, ok := context[BlockSizeKey]; ok { + if blockSize != "4096" { + t.Fatalf("Invalid %s in VolumeContext (got %s expected 4096)", BlockSizeKey, blockSize) + } + } else { + t.Fatalf("Missing key %s in VolumeContext", BlockSizeKey) + } + }, + }, } for _, tc := range testCases { diff --git a/pkg/driver/mock_mount.go b/pkg/driver/mock_mount.go index 5d337508a9..d2e89433f4 100644 --- a/pkg/driver/mock_mount.go +++ b/pkg/driver/mock_mount.go @@ -36,18 +36,18 @@ func (m *MockMounter) EXPECT() *MockMounterMockRecorder { return m.recorder } -// FormatAndMount mocks base method. -func (m *MockMounter) FormatAndMount(source, target, fstype string, options []string) error { +// FormatAndMountSensitiveWithFormatOptions mocks base method. +func (m *MockMounter) FormatAndMountSensitiveWithFormatOptions(source, target, fstype string, options, sensitiveOptions, formatOptions []string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "FormatAndMount", source, target, fstype, options) + ret := m.ctrl.Call(m, "FormatAndMountSensitiveWithFormatOptions", source, target, fstype, options, sensitiveOptions, formatOptions) ret0, _ := ret[0].(error) return ret0 } -// FormatAndMount indicates an expected call of FormatAndMount. -func (mr *MockMounterMockRecorder) FormatAndMount(source, target, fstype, options interface{}) *gomock.Call { +// FormatAndMountSensitiveWithFormatOptions indicates an expected call of FormatAndMountSensitiveWithFormatOptions. +func (mr *MockMounterMockRecorder) FormatAndMountSensitiveWithFormatOptions(source, target, fstype, options, sensitiveOptions, formatOptions interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FormatAndMount", reflect.TypeOf((*MockMounter)(nil).FormatAndMount), source, target, fstype, options) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FormatAndMountSensitiveWithFormatOptions", reflect.TypeOf((*MockMounter)(nil).FormatAndMountSensitiveWithFormatOptions), source, target, fstype, options, sensitiveOptions, formatOptions) } // GetDeviceNameFromMount mocks base method. diff --git a/pkg/driver/mount.go b/pkg/driver/mount.go index d1322ba1eb..4057eef27d 100644 --- a/pkg/driver/mount.go +++ b/pkg/driver/mount.go @@ -32,7 +32,7 @@ import ( type Mounter interface { mountutils.Interface - FormatAndMount(source string, target string, fstype string, options []string) error + FormatAndMountSensitiveWithFormatOptions(source string, target string, fstype string, options []string, sensitiveOptions []string, formatOptions []string) error IsCorruptedMnt(err error) bool GetDeviceNameFromMount(mountPath string) (string, int, error) MakeFile(path string) error @@ -55,7 +55,7 @@ type NodeMounter struct { } func newNodeMounter() (Mounter, error) { - // mounter.NewSafeMounter returns a SafeFormatAndMount + // mounter.NewSafeMounter returns a SafeormatAndMount safeMounter, err := mounter.NewSafeMounter() if err != nil { return nil, err diff --git a/pkg/driver/mount_windows.go b/pkg/driver/mount_windows.go index fd0b7e99b8..c9a9e1e46f 100644 --- a/pkg/driver/mount_windows.go +++ b/pkg/driver/mount_windows.go @@ -27,12 +27,12 @@ import ( "regexp" ) -func (m NodeMounter) FormatAndMount(source string, target string, fstype string, options []string) error { +func (m NodeMounter) FormatAndMountSensitiveWithFormatOptions(source string, target string, fstype string, options []string, sensitiveOptions []string, formatOptions []string) error { proxyMounter, ok := m.SafeFormatAndMount.Interface.(*mounter.CSIProxyMounter) if !ok { return fmt.Errorf("failed to cast mounter to csi proxy mounter") } - return proxyMounter.FormatAndMount(source, target, fstype, options) + return proxyMounter.FormatAndMountSensitiveWithFormatOptions(source, target, fstype, options, sensitiveOptions, formatOptions) } // GetDeviceNameFromMount returns the volume ID for a mount path. diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 9b33f41c15..46fc466604 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -21,6 +21,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" csi "github.com/container-storage-interface/spec/lib/go/csi" @@ -111,6 +112,17 @@ func newNodeService(driverOptions *DriverOptions) nodeService { func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRequest) (*csi.NodeStageVolumeResponse, error) { klog.V(4).Infof("NodeStageVolume: called with args %+v", *req) + context := req.GetVolumeContext() + blockSize, ok := context[BlockSizeKey] + if ok { + // This check is already performed on the controller side + // However, because it is potentially security-sensitive, we redo it here to be safe + _, err := strconv.Atoi(blockSize) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "Invalid blockSize (aborting!): %v", err) + } + } + volumeID := req.GetVolumeId() if len(volumeID) == 0 { return nil, status.Error(codes.InvalidArgument, "Volume ID not provided") @@ -150,7 +162,7 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol fsType = defaultFsType } - _, ok := ValidFSTypes[strings.ToLower(fsType)] + _, ok = ValidFSTypes[strings.ToLower(fsType)] if !ok { return nil, status.Errorf(codes.InvalidArgument, "NodeStageVolume: invalid fstype %s", fsType) } @@ -219,7 +231,11 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol // FormatAndMount will format only if needed klog.V(4).Infof("NodeStageVolume: formatting %s and mounting at %s with fstype %s", source, target, fsType) - err = d.mounter.FormatAndMount(source, target, fsType, mountOptions) + formatOptions := []string{} + if len(blockSize) > 0 { + formatOptions = append(formatOptions, "-b", blockSize) + } + err = d.mounter.FormatAndMountSensitiveWithFormatOptions(source, target, fsType, mountOptions, nil, formatOptions) if err != nil { msg := fmt.Sprintf("could not format %q and mount it at %q: %v", source, target, err) return nil, status.Error(codes.Internal, msg) diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index f5ad975cce..e7404775a8 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -64,7 +64,7 @@ func TestNodeStageVolume(t *testing.T) { stdVolContext = map[string]string{VolumeAttributePartition: "1"} devicePathWithPartition = devicePath + "1" // With few exceptions, all "success" non-block cases have roughly the same - // expected calls and only care about testing the FormatAndMount call. The + // expected calls and only care about testing the FormatAndMountSensitiveWithFormatOptions call. The // exceptions should not call this, instead they should define expectMock // from scratch. successExpectMock = func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { @@ -93,7 +93,7 @@ func TestNodeStageVolume(t *testing.T) { }, expectMock: func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { successExpectMock(mockMounter, mockDeviceIdentifier) - mockMounter.EXPECT().FormatAndMount(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any()) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any(), gomock.Nil(), gomock.Len(0)) }, }, { @@ -112,7 +112,7 @@ func TestNodeStageVolume(t *testing.T) { VolumeId: volumeID, }, expectMock: func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { - mockMounter.EXPECT().FormatAndMount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Nil(), gomock.Len(0)).Times(0) }, }, { @@ -134,7 +134,7 @@ func TestNodeStageVolume(t *testing.T) { }, expectMock: func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { successExpectMock(mockMounter, mockDeviceIdentifier) - mockMounter.EXPECT().FormatAndMount(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(FSTypeExt4), gomock.Eq([]string{"dirsync", "noexec"})) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(FSTypeExt4), gomock.Eq([]string{"dirsync", "noexec"}), gomock.Nil(), gomock.Len(0)) }, }, { @@ -156,7 +156,7 @@ func TestNodeStageVolume(t *testing.T) { }, expectMock: func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { successExpectMock(mockMounter, mockDeviceIdentifier) - mockMounter.EXPECT().FormatAndMount(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(FSTypeExt3), gomock.Any()) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(FSTypeExt3), gomock.Any(), gomock.Nil(), gomock.Len(0)) }, }, { @@ -178,7 +178,7 @@ func TestNodeStageVolume(t *testing.T) { }, expectMock: func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { successExpectMock(mockMounter, mockDeviceIdentifier) - mockMounter.EXPECT().FormatAndMount(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(FSTypeExt4), gomock.Any()) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(FSTypeExt4), gomock.Any(), gomock.Nil(), gomock.Len(0)) }, }, { @@ -195,7 +195,7 @@ func TestNodeStageVolume(t *testing.T) { mockMounter.EXPECT().PathExists(gomock.Eq(devicePath)).Return(true, nil) mockDeviceIdentifier.EXPECT().Lstat(gomock.Eq(devicePath)).Return(deviceFileInfo, nil) - mockMounter.EXPECT().FormatAndMount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Nil(), gomock.Len(0)).Times(0) }, }, { @@ -216,12 +216,12 @@ func TestNodeStageVolume(t *testing.T) { // The publish context device path may not exist but the driver should // find the canonical device path (see TestFindDevicePath), compare it // to the one returned by GetDeviceNameFromMount, and then skip - // FormatAndMount + // FormatAndMountSensitiveWithFormatOptions mockMounter.EXPECT().PathExists(gomock.Eq(devicePath)).Return(false, nil) mockDeviceIdentifier.EXPECT().Lstat(gomock.Eq(nvmeName)).Return(symlinkFileInfo, nil) mockDeviceIdentifier.EXPECT().EvalSymlinks(gomock.Eq(symlinkFileInfo.Name())).Return(nvmeDevicePath, nil) - mockMounter.EXPECT().FormatAndMount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Nil(), gomock.Len(0)).Times(0) }, }, { @@ -243,7 +243,7 @@ func TestNodeStageVolume(t *testing.T) { // The device path argument should be canonicalized to contain the // partition mockMounter.EXPECT().NeedResize(gomock.Eq(devicePathWithPartition), gomock.Eq(targetPath)).Return(false, nil) - mockMounter.EXPECT().FormatAndMount(gomock.Eq(devicePathWithPartition), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any()) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePathWithPartition), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any(), gomock.Nil(), gomock.Len(0)) }, }, { @@ -257,7 +257,21 @@ func TestNodeStageVolume(t *testing.T) { }, expectMock: func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { successExpectMock(mockMounter, mockDeviceIdentifier) - mockMounter.EXPECT().FormatAndMount(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any()) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any(), gomock.Nil(), gomock.Len(0)) + }, + }, + { + name: "success with block size", + request: &csi.NodeStageVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: devicePath}, + StagingTargetPath: targetPath, + VolumeCapability: stdVolCap, + VolumeId: volumeID, + VolumeContext: map[string]string{BlockSizeKey: "1024"}, + }, + expectMock: func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { + successExpectMock(mockMounter, mockDeviceIdentifier) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any(), gomock.Nil(), gomock.Eq([]string{"-b", "1024"})) }, }, { diff --git a/pkg/driver/sanity_test.go b/pkg/driver/sanity_test.go index 58ad7fd929..22e75e199c 100644 --- a/pkg/driver/sanity_test.go +++ b/pkg/driver/sanity_test.go @@ -354,7 +354,7 @@ func (f *fakeMounter) GetMountRefs(pathname string) ([]string, error) { return []string{}, nil } -func (f *fakeMounter) FormatAndMount(source string, target string, fstype string, options []string) error { +func (f *fakeMounter) FormatAndMountSensitiveWithFormatOptions(source string, target string, fstype string, options []string, sensitiveOptions []string, formatOptions []string) error { return nil } diff --git a/pkg/mounter/mock_mount_windows.go b/pkg/mounter/mock_mount_windows.go index d4821285c9..aaf2fc225e 100644 --- a/pkg/mounter/mock_mount_windows.go +++ b/pkg/mounter/mock_mount_windows.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: ./pkg/mounter/safe_mounter_windows.go +// Source: pkg/mounter/safe_mounter_windows.go // Package mounter is a generated GoMock package. package mounter @@ -65,10 +65,10 @@ func (mr *MockProxyMounterMockRecorder) FindDiskByLun(lun interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FindDiskByLun", reflect.TypeOf((*MockProxyMounter)(nil).FindDiskByLun), lun) } -// FormatAndMount mocks base method. -func (m *MockProxyMounter) FormatAndMount(source, target, fstype string, options []string) error { +// FormatAndMountSensitiveWithFormatOptions mocks base method. +func (m *MockProxyMounter) FormatAndMountSensitiveWithFormatOptions(source, target, fstype string, options []string, sensitiveOptions []string, formatOptions []string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "FormatAndMount", source, target, fstype, options) + ret := m.ctrl.Call(m, "FormatAndMount", source, target, fstype, options, sensitiveOptions, formatOptions) ret0, _ := ret[0].(error) return ret0 } diff --git a/pkg/mounter/safe_mounter_windows.go b/pkg/mounter/safe_mounter_windows.go index b1f9a39cb9..4a3fd9a718 100644 --- a/pkg/mounter/safe_mounter_windows.go +++ b/pkg/mounter/safe_mounter_windows.go @@ -21,6 +21,7 @@ package mounter import ( "context" + "errors" "fmt" "os" "strconv" @@ -53,7 +54,7 @@ type ProxyMounter interface { ExistsPath(path string) (bool, error) Rescan() error FindDiskByLun(lun string) (diskNum string, err error) - FormatAndMount(source, target, fstype string, options []string) error + FormatAndMountSensitiveWithFormatOptions(source string, target string, fstype string, options []string, sensitiveOptions []string, formatOptions []string) error ResizeVolume(deviceMountPath string) (bool, error) GetVolumeSizeInBytes(deviceMountPath string) (int64, error) GetDeviceSize(devicePath string) (int64, error) @@ -313,7 +314,17 @@ func (mounter *CSIProxyMounter) FindDiskByLun(lun string) (diskNum string, err e } // FormatAndMount - accepts the source disk number, target path to mount, the fstype to format with and options to be used. -func (mounter *CSIProxyMounter) FormatAndMount(source string, target string, fstype string, options []string) error { +func (mounter *CSIProxyMounter) FormatAndMountSensitiveWithFormatOptions(source string, target string, fstype string, options []string, sensitiveOptions []string, formatOptions []string) error { + // sensitiveOptions is not supported on Windows because we have no reasonable way to control what the csi-proxy does + if len(sensitiveOptions) > 0 { + return errors.New("sensitiveOptions not supported on Windows!") + } + // formatOptions is not supported on Windows because the csi-proxy does not allow supplying format arguments + // This limitation will be addressed in the future with privileged Windows containers + if len(formatOptions) > 0 { + return errors.New("formatOptions not supported on Windows!") + } + diskNumber, err := strconv.Atoi(source) if err != nil { return err