Repository: kubernetes-sigs/aws-ebs-csi-driver
Pull request #1207 (2022-04-12 15:26:14)

Add volume resizing support for windows
Signed-off-by: Eddie Torres <torredil@amazon.com>

**Is this a bug fix or adding new feature?**

- New feature

**What is this PR about? / Why do we need it?**

- When attempting to resize a volume on Windows by editing the PVC, you will encounter the error: "Resize is not supported for this build".
- In the current implementation, the [NodeExpandVolume](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/a304d3eaa3a246c630a56551005e6ee630c3800c/pkg/driver/node.go#L280) function in the node service is not platform agnostic because it uses the mount-utils package to resize the file system. The mount-utils package does not implement the [Resize](https://github.com/kubernetes/mount-utils/blob/6e81bcc03fc8c22aa460c3c4bd32a7ad602abd6c/resizefs_unsupported.go#L39) function for windows environments.
- Added support for resizing on Windows using the [CSI Proxy](https://github.com/kubernetes-csi/csi-proxy), which allows the driver to issue privileged operations through a gRPC interface exposed over named pipes.
- Created custom resizefs package maintained for the windows implementation only which will be used to return an instance of NewResizeFs and implement the resize interface.
- Invoked in node.go by calling mounter.NewResizeFs() to create an instance of NewResizeFs based on the environment context, and finally, calling Resize on that instance of NewResizeFs

**What testing is done?** 
- make test succeeds.
1. Build and upload container images to a registry.
2. Change the image in the helm chart.
3. Test resizing functionality using the new helm chart.



Patch:
diff --git a/examples/kubernetes/resizing/manifests/pod.yaml b/examples/kubernetes/resizing/manifests/pod.yaml
index d35fd7d2..4a72260f 100644
--- a/examples/kubernetes/resizing/manifests/pod.yaml
+++ b/examples/kubernetes/resizing/manifests/pod.yaml
@@ -7,7 +7,7 @@ spec:
   - name: app
     image: centos
     command: ["/bin/sh"]
-    args: ["tail -f /dev/null"]
+    args: ["-c", "while true; do echo $(date -u) >> /data/out.txt; sleep 5; done"]
     volumeMounts:
     - name: persistent-storage
       mountPath: /data
diff --git a/pkg/driver/constants.go b/pkg/driver/constants.go
index 3b044f62..1dc092be 100644
--- a/pkg/driver/constants.go
+++ b/pkg/driver/constants.go
@@ -118,3 +118,9 @@ const (
 const (
 	DefaultCSIEndpoint = "unix://tmp/csi.sock"
 )
+
+// constants for disk block size
+const (
+	//DefaultBlockSize represents the default block size (4KB)
+	DefaultBlockSize = 4096
+)
diff --git a/pkg/driver/mock_mount.go b/pkg/driver/mock_mount.go
index e86cf464..d9a6f840 100644
--- a/pkg/driver/mock_mount.go
+++ b/pkg/driver/mock_mount.go
@@ -9,7 +9,7 @@ import (
 	reflect "reflect"
 
 	gomock "github.com/golang/mock/gomock"
-	mount "k8s.io/mount-utils"
+	mount_utils "k8s.io/mount-utils"
 )
 
 // MockMounter is a mock of Mounter interface.
@@ -110,10 +110,10 @@ func (mr *MockMounterMockRecorder) IsLikelyNotMountPoint(file interface{}) *gomo
 }
 
 // List mocks base method.
-func (m *MockMounter) List() ([]mount.MountPoint, error) {
+func (m *MockMounter) List() ([]mount_utils.MountPoint, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "List")
-	ret0, _ := ret[0].([]mount.MountPoint)
+	ret0, _ := ret[0].([]mount_utils.MountPoint)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
@@ -209,6 +209,21 @@ func (mr *MockMounterMockRecorder) NeedResize(devicePath, deviceMountPath interf
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NeedResize", reflect.TypeOf((*MockMounter)(nil).NeedResize), devicePath, deviceMountPath)
 }
 
+// NewResizeFs mocks base method.
+func (m *MockMounter) NewResizeFs() (Resizefs, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "NewResizeFs")
+	ret0, _ := ret[0].(Resizefs)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// NewResizeFs indicates an expected call of NewResizeFs.
+func (mr *MockMounterMockRecorder) NewResizeFs() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewResizeFs", reflect.TypeOf((*MockMounter)(nil).NewResizeFs))
+}
+
 // PathExists mocks base method.
 func (m *MockMounter) PathExists(path string) (bool, error) {
 	m.ctrl.T.Helper()
@@ -266,6 +281,44 @@ func (mr *MockMounterMockRecorder) Unstage(path interface{}) *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Unstage", reflect.TypeOf((*MockMounter)(nil).Unstage), path)
 }
 
+// MockResizefs is a mock of Resizefs interface.
+type MockResizefs struct {
+	ctrl     *gomock.Controller
+	recorder *MockResizefsMockRecorder
+}
+
+// MockResizefsMockRecorder is the mock recorder for MockResizefs.
+type MockResizefsMockRecorder struct {
+	mock *MockResizefs
+}
+
+// NewMockResizefs creates a new mock instance.
+func NewMockResizefs(ctrl *gomock.Controller) *MockResizefs {
+	mock := &MockResizefs{ctrl: ctrl}
+	mock.recorder = &MockResizefsMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockResizefs) EXPECT() *MockResizefsMockRecorder {
+	return m.recorder
+}
+
+// Resize mocks base method.
+func (m *MockResizefs) Resize(devicePath, deviceMountPath string) (bool, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Resize", devicePath, deviceMountPath)
+	ret0, _ := ret[0].(bool)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Resize indicates an expected call of Resize.
+func (mr *MockResizefsMockRecorder) Resize(devicePath, deviceMountPath interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Resize", reflect.TypeOf((*MockResizefs)(nil).Resize), devicePath, deviceMountPath)
+}
+
 // MockDeviceIdentifier is a mock of DeviceIdentifier interface.
 type MockDeviceIdentifier struct {
 	ctrl     *gomock.Controller
diff --git a/pkg/driver/mount.go b/pkg/driver/mount.go
index 62ef2716..9ff53817 100644
--- a/pkg/driver/mount.go
+++ b/pkg/driver/mount.go
@@ -43,6 +43,11 @@ type Mounter interface {
 	NeedResize(devicePath string, deviceMountPath string) (bool, error)
 	Unpublish(path string) error
 	Unstage(path string) error
+	NewResizeFs() (Resizefs, error)
+}
+
+type Resizefs interface {
+	Resize(devicePath, deviceMountPath string) (bool, error)
 }
 
 // NodeMounter implements Mounter.
diff --git a/pkg/driver/mount_linux.go b/pkg/driver/mount_linux.go
index 7ea6f66d..93f22bb6 100644
--- a/pkg/driver/mount_linux.go
+++ b/pkg/driver/mount_linux.go
@@ -197,3 +197,7 @@ func (m *NodeMounter) Unpublish(path string) error {
 func (m *NodeMounter) Unstage(path string) error {
 	return m.Unmount(path)
 }
+
+func (m *NodeMounter) NewResizeFs() (Resizefs, error) {
+	return mountutils.NewResizeFs(m.Exec), nil
+}
diff --git a/pkg/driver/mount_windows.go b/pkg/driver/mount_windows.go
index bffbed0c..fd0b7e99 100644
--- a/pkg/driver/mount_windows.go
+++ b/pkg/driver/mount_windows.go
@@ -21,10 +21,10 @@ package driver
 
 import (
 	"fmt"
-	"regexp"
-
 	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/mounter"
+	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/resizefs"
 	mountutils "k8s.io/mount-utils"
+	"regexp"
 )
 
 func (m NodeMounter) FormatAndMount(source string, target string, fstype string, options []string) error {
@@ -99,9 +99,26 @@ func (m *NodeMounter) PathExists(path string) (bool, error) {
 	return proxyMounter.ExistsPath(path)
 }
 
-func (m *NodeMounter) NeedResize(devicePath string, deviceMountPath string) (bool, error) {
-	// TODO this is called at NodeStage to ensure file system is the correct size
-	// Implement it to respect spec v1.4.0 https://github.com/container-storage-interface/spec/pull/452
+// NeedResize called at NodeStage to ensure file system is the correct size
+func (m *NodeMounter) NeedResize(devicePath, deviceMountPath string) (bool, error) {
+	proxyMounter, ok := m.SafeFormatAndMount.Interface.(*mounter.CSIProxyMounter)
+	if !ok {
+		return false, fmt.Errorf("failed to cast mounter to csi proxy mounter")
+	}
+
+	deviceSize, err := proxyMounter.GetDeviceSize(devicePath)
+	if err != nil {
+		return false, err
+	}
+
+	fsSize, err := proxyMounter.GetVolumeSizeInBytes(deviceMountPath)
+	if err != nil {
+		return false, err
+	}
+	// Tolerate one block difference (4096 bytes)
+	if deviceSize <= DefaultBlockSize+fsSize {
+		return true, nil
+	}
 	return false, nil
 }
 
@@ -140,3 +157,11 @@ func (m *NodeMounter) Unstage(target string) error {
 	}
 	return nil
 }
+
+func (m *NodeMounter) NewResizeFs() (Resizefs, error) {
+	proxyMounter, ok := m.SafeFormatAndMount.Interface.(*mounter.CSIProxyMounter)
+	if !ok {
+		return nil, fmt.Errorf("failed to cast mounter to csi proxy mounter")
+	}
+	return resizefs.NewResizeFs(proxyMounter), nil
+}
diff --git a/pkg/driver/node.go b/pkg/driver/node.go
index 4d95d40f..902d6a45 100644
--- a/pkg/driver/node.go
+++ b/pkg/driver/node.go
@@ -29,7 +29,6 @@ import (
 	"google.golang.org/grpc/status"
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/volume"
-	mountutils "k8s.io/mount-utils"
 )
 
 const (
@@ -217,8 +216,12 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not determine if volume %q (%q) need to be resized:  %v", req.GetVolumeId(), source, err)
 	}
+
 	if needResize {
-		r := mountutils.NewResizeFs(d.mounter.(*NodeMounter).Exec)
+		r, err := d.mounter.NewResizeFs()
+		if err != nil {
+			return nil, status.Errorf(codes.Internal, "Error attempting to create new ResizeFs:  %v", err)
+		}
 		klog.V(2).Infof("Volume %s needs resizing", source)
 		if _, err := r.Resize(source, target); err != nil {
 			return nil, status.Errorf(codes.Internal, "Could not resize volume %q (%q):  %v", volumeID, source, err)
@@ -329,7 +332,10 @@ func (d *nodeService) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV
 		return nil, status.Errorf(codes.Internal, "failed to find device path for device name %s for mount %s: %v", deviceName, req.GetVolumePath(), err)
 	}
 
-	r := mountutils.NewResizeFs(d.mounter.(*NodeMounter).Exec)
+	r, err := d.mounter.NewResizeFs()
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "Error attempting to create new ResizeFs:  %v", err)
+	}
 
 	// TODO: lock per volume ID to have some idempotency
 	if _, err := r.Resize(devicePath, volumePath); err != nil {
diff --git a/pkg/driver/node_windows.go b/pkg/driver/node_windows.go
index dca292ed..b0adc74b 100644
--- a/pkg/driver/node_windows.go
+++ b/pkg/driver/node_windows.go
@@ -21,7 +21,6 @@ package driver
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -89,11 +88,22 @@ func (d *nodeService) preparePublishTarget(target string) error {
 	return nil
 }
 
-// IsBlock checks if the given path is a block device
+// IsBlockDevice checks if the given path is a block device
 func (d *nodeService) IsBlockDevice(fullPath string) (bool, error) {
-	return false, errors.New("unsupported")
+	return false, nil
 }
 
+// getBlockSizeBytes gets the size of the disk in bytes
 func (d *nodeService) getBlockSizeBytes(devicePath string) (int64, error) {
-	return 0, errors.New("unsupported")
+	proxyMounter, ok := (d.mounter.(*NodeMounter)).SafeFormatAndMount.Interface.(*mounter.CSIProxyMounter)
+	if !ok {
+		return -1, fmt.Errorf("failed to cast mounter to csi proxy mounter")
+	}
+
+	sizeInBytes, err := proxyMounter.GetDeviceSize(devicePath)
+	if err != nil {
+		return -1, err
+	}
+
+	return sizeInBytes, nil
 }
diff --git a/pkg/mounter/safe_mounter_windows.go b/pkg/mounter/safe_mounter_windows.go
index e569716b..a314bc0f 100644
--- a/pkg/mounter/safe_mounter_windows.go
+++ b/pkg/mounter/safe_mounter_windows.go
@@ -351,23 +351,66 @@ func (mounter *CSIProxyMounter) FormatAndMount(source string, target string, fst
 	return nil
 }
 
-// ResizeVolume resizes the volume to the maximum available size.
-func (mounter *CSIProxyMounter) ResizeVolume(devicePath string) error {
-	req := &volume.ResizeVolumeRequest{VolumeId: devicePath, SizeBytes: 0}
+// ResizeVolume resizes the volume at given mount path
+func (mounter *CSIProxyMounter) ResizeVolume(deviceMountPath string) (bool, error) {
+	// Find the volume id
+	getVolumeIdRequest := &volume.GetVolumeIDFromTargetPathRequest{
+		TargetPath: normalizeWindowsPath(deviceMountPath),
+	}
+	volumeIdResponse, err := mounter.VolumeClient.GetVolumeIDFromTargetPath(context.Background(), getVolumeIdRequest)
+	if err != nil {
+		return false, err
+	}
+	volumeId := volumeIdResponse.GetVolumeId()
 
-	_, err := mounter.VolumeClient.ResizeVolume(context.Background(), req)
+	// Resize volume
+	resizeVolumeRequest := &volume.ResizeVolumeRequest{
+		VolumeId: volumeId,
+	}
+	_, err = mounter.VolumeClient.ResizeVolume(context.Background(), resizeVolumeRequest)
 	if err != nil {
-		return err
+		return false, err
 	}
 
-	return nil
+	return true, nil
+}
+
+// GetVolumeSizeInBytes returns the size of the volume in bytes
+func (mounter *CSIProxyMounter) GetVolumeSizeInBytes(deviceMountPath string) (int64, error) {
+	// Find the volume id
+	getVolumeIdRequest := &volume.GetVolumeIDFromTargetPathRequest{
+		TargetPath: normalizeWindowsPath(deviceMountPath),
+	}
+	volumeIdResponse, err := mounter.VolumeClient.GetVolumeIDFromTargetPath(context.Background(), getVolumeIdRequest)
+	if err != nil {
+		return -1, err
+	}
+	volumeId := volumeIdResponse.GetVolumeId()
+
+	// Get size of the volume
+	getVolumeStatsRequest := &volume.GetVolumeStatsRequest{
+		VolumeId: volumeId,
+	}
+	resp, err := mounter.VolumeClient.GetVolumeStats(context.Background(), getVolumeStatsRequest)
+	if err != nil {
+		return -1, err
+	}
+
+	return resp.TotalBytes, nil
 }
 
-// GetVolumeSizeInBytes returns the size of the volume in bytes.
-func (mounter *CSIProxyMounter) GetVolumeSizeInBytes(devicePath string) (int64, error) {
-	req := &volume.GetVolumeStatsRequest{VolumeId: devicePath}
+// GetDeviceSize returns the size of the disk in bytes
+func (mounter *CSIProxyMounter) GetDeviceSize(devicePath string) (int64, error) {
+	diskNumber, err := strconv.Atoi(devicePath)
+	if err != nil {
+		return -1, err
+	}
 
-	resp, err := mounter.VolumeClient.GetVolumeStats(context.Background(), req)
+	//Get size of the disk
+	getDiskStatsRequest := &disk.GetDiskStatsRequest{
+		DiskNumber: uint32(diskNumber),
+	}
+	resp, err := mounter.DiskClient.GetDiskStats(context.Background(), getDiskStatsRequest)
 	if err != nil {
 		return -1, err
 	}
diff --git a/pkg/resizefs/resizefs_windows.go b/pkg/resizefs/resizefs_windows.go
new file mode 100644
index 00000000..c63a2aa6
--- /dev/null
+++ b/pkg/resizefs/resizefs_windows.go
@@ -0,0 +1,25 @@
+//go:build windows
+// +build windows
+
+package resizefs
+
+import (
+	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/mounter"
+	"k8s.io/klog"
+)
+
+// resizeFs provides support for resizing file systems
+type resizeFs struct {
+	proxy *mounter.CSIProxyMounter
+}
+
+// NewResizeFs returns an instance of resizeFs
+func NewResizeFs(proxy *mounter.CSIProxyMounter) *resizeFs {
+	return &resizeFs{proxy: proxy}
+}
+
+// Resize performs resize of file system
+func (r *resizeFs) Resize(_, deviceMountPath string) (bool, error) {
+	klog.V(3).Infof("Resize - Expanding mounted volume %s", deviceMountPath)
+	return r.proxy.ResizeVolume(deviceMountPath)
+}


Test patch:
diff --git a/pkg/driver/sanity_test.go b/pkg/driver/sanity_test.go
index 4d5c7736..8b6f5bb0 100644
--- a/pkg/driver/sanity_test.go
+++ b/pkg/driver/sanity_test.go
@@ -330,6 +330,10 @@ func (f *fakeMounter) Unpublish(target string) error {
 	return nil
 }
 
+func (f *fakeMounter) NewResizeFs() (Resizefs, error) {
+	return nil, nil
+}
+
 func (f *fakeMounter) List() ([]mount.MountPoint, error) {
 	return []mount.MountPoint{}, nil
 }