Repository: kubernetes-sigs/azuredisk-csi-driver (go)

Issue:
Mounted volume size issue when cloning a larger size pvc
<!-- Please use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!
-->


**What happened**:
When cloned a target PVC with a larger size(16G) than the original PVC(8G), the mounted volume size for the container is accurately 8G.

**What you expected to happen**:
When cloned a target PVC with a larger size(16G) than the original PVC(8G), the mounted volume size for the container should equal to 16G

**How to reproduce it**:
Create original pvc(8G)
clone target pvc(16G)
Create pod and consume target pvc

**Anything else we need to know?**:
Some check on the worker:

$ df -h | grep pvc-58344a0e-3a4b-4732-8f67-d41d8affa7c8
/dev/sdd                              7.9G  4.5M  7.9G   1% /var/lib/kubelet/plugins/kubernetes.io/csi/pv/pvc-58344a0e-3a4b-4732-8f67-d41d8affa7c8/globalmount

$ lsblk | grep pvc-58344a0e-3a4b-4732-8f67-d41d8affa7c8
sdd                            8:48   0    16G  0 disk /var/lib/kubelet/pods/5a8ef385-efe0-4f0d-91b2-0d9bbcd39bbc/volumes/kubernetes.io~csi/pvc-58344a0e-3a4b-4732-8f67-d41d8affa7c8/mount


**Environment**:
- CSI Driver version: azuredisk-csi:v0.7.0
- Kubernetes version (use `kubectl version`):  latest happen on 1.19, but seen in 1.18 as well
- OS (e.g. from /etc/os-release):
- Kernel (e.g. `uname -a`):
- Install tools:
- Others:


Pull request:
fix: resize filesystem if cloned volume capacity is larger than the source volume
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://github.com/kubernetes/community/blob/master/contributors/devel/sig-release/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-testing/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind bug

**What this PR does / why we need it**:
**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #528 

**Requirements**:
- [ ] uses [conventional commit messages](https://www.conventionalcommits.org/)
  <!-- Common commit types:
        build: Build 🏭
        chore: Maintenance 🔧
        ci: Continuous Integration 💜
        docs: Documentation 📘
        feat: Features 🌈
        fix: Bug Fixes 🐞
        perf: Performance Improvements 🚀
        refactor: Code Refactoring 💎
        revert: Revert Change ◀️
        style: Code Style 🎶
        security: Security Fix 🛡️
        test: Testing 💚 -->
- [ ] includes documentation
- [x] adds unit tests
- [ ] tested upgrade from previous version

**Special notes for your reviewer**:


**Release note**:
```
none
```


Patch:
diff --git a/pkg/azuredisk/controllerserver.go b/pkg/azuredisk/controllerserver.go
index e33494470..7af7d4efd 100644
--- a/pkg/azuredisk/controllerserver.go
+++ b/pkg/azuredisk/controllerserver.go
@@ -19,6 +19,7 @@ package azuredisk
 import (
 	"context"
 	"fmt"
+	"path"
 	"strconv"
 	"strings"
 
@@ -63,11 +64,13 @@ var (
 )
 
 const (
-	sourceSnapshot         = "snapshot"
-	sourceVolume           = "volume"
-	azureDiskCSIDriverName = "azuredisk_csi_driver"
-	NotFound               = "NotFound"
-	CreatedForPVNameKey    = "kubernetes.io-created-for-pv-name"
+	sourceSnapshot           = "snapshot"
+	sourceVolume             = "volume"
+	azureDiskCSIDriverName   = "azuredisk_csi_driver"
+	NotFound                 = "NotFound"
+	CreatedForPVNameKey      = "kubernetes.io-created-for-pv-name"
+	resizeRequired           = "resizeRequired"
+	sourceDiskSearchMaxDepth = 10
 )
 
 // CreateVolume provisions an azure disk
@@ -160,6 +163,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 			//return nil, fmt.Errorf("AzureDisk - invalid option %s in storage class", k)
 		}
 	}
+	parameters[resizeRequired] = strconv.FormatBool(false)
 
 	if IsAzureStackCloud(d.cloud) {
 		if maxShares > 1 {
@@ -258,6 +262,11 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 				},
 			}
 
+			ctx, cancel := context.WithCancel(context.Background())
+			if sourceGiB, _ := d.GetSourceDiskSize(ctx, resourceGroup, path.Base(sourceID), 0, sourceDiskSearchMaxDepth); sourceGiB != nil && *sourceGiB < int32(requestGiB) {
+				parameters[resizeRequired] = strconv.FormatBool(true)
+			}
+			cancel()
 		}
 	}
 
@@ -796,6 +805,33 @@ func (d *Driver) getSnapshotByID(ctx context.Context, resourceGroup, snapshotID,
 	return generateCSISnapshot(sourceVolumeID, &snapshot)
 }
 
+// GetSourceDiskSize recursively searches for the sourceDisk and returns: sourceDisk disk size, error
+func (d *Driver) GetSourceDiskSize(ctx context.Context, resourceGroup, diskName string, curDepth, maxDepth int) (*int32, error) {
+	if curDepth > maxDepth {
+		return nil, status.Error(codes.Internal, fmt.Sprintf("current depth (%d) surpassed the max depth (%d) while searching for the source disk size", curDepth, maxDepth))
+	}
+	result, rerr := d.cloud.DisksClient.Get(ctx, resourceGroup, diskName)
+	if rerr != nil {
+		return nil, rerr.Error()
+	}
+	if result.DiskProperties == nil {
+		return nil, status.Error(codes.Internal, fmt.Sprintf("DiskProperty not found for disk (%s) in resource group (%s)", diskName, resourceGroup))
+	}
+
+	if result.DiskProperties.CreationData != nil && (*result.DiskProperties.CreationData).CreateOption == "Copy" {
+		klog.V(2).Infof("Clone source disk has a parent source")
+		sourceResourceID := *result.DiskProperties.CreationData.SourceResourceID
+		parentResourceGroup, _ := GetResourceGroupFromURI(sourceResourceID)
+		parentDiskName := path.Base(sourceResourceID)
+		return d.GetSourceDiskSize(ctx, parentResourceGroup, parentDiskName, curDepth+1, maxDepth)
+	}
+
+	if (*result.DiskProperties).DiskSizeGB == nil {
+		return nil, status.Error(codes.Internal, fmt.Sprintf("DiskSizeGB for disk (%s) in resourcegroup (%s) is nil", diskName, resourceGroup))
+	}
+	return (*result.DiskProperties).DiskSizeGB, nil
+}
+
 func isValidVolumeCapabilities(volCaps []*csi.VolumeCapability) bool {
 	hasSupport := func(cap *csi.VolumeCapability) bool {
 		for _, c := range volumeCaps {
diff --git a/pkg/azuredisk/nodeserver.go b/pkg/azuredisk/nodeserver.go
index e4f1b72fb..f6df8741c 100644
--- a/pkg/azuredisk/nodeserver.go
+++ b/pkg/azuredisk/nodeserver.go
@@ -131,6 +131,21 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 	}
 	klog.V(2).Infof("NodeStageVolume: format %s and mounting at %s successfully.", source, target)
 
+	// if resize is required, resize filesystem
+	if required, ok := req.GetVolumeContext()[resizeRequired]; ok && required == "true" {
+		klog.V(2).Infof("NodeStageVolume: fs resize initiating on target(%s) volumeid(%s)", target, diskURI)
+		if err != nil {
+			return nil, status.Errorf(codes.Internal, "NodeStageVolume: Could not get volume path for %s: %v", target, err)
+		}
+
+		resizer := resizefs.NewResizeFs(d.mounter)
+		if _, err := resizer.Resize(source, target); err != nil {
+			return nil, status.Errorf(codes.Internal, "NodeStageVolume: Could not resize volume %q (%q):  %v", diskURI, source, err)
+		}
+
+		klog.V(2).Infof("NodeStageVolume: fs resize successful on target(%s) volumeid(%s).", target, diskURI)
+	}
+
 	return &csi.NodeStageVolumeResponse{}, nil
 }
 
@@ -216,6 +231,7 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu
 	if err := d.mounter.Mount(source, target, "", mountOptions); err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err)
 	}
+
 	klog.V(2).Infof("NodePublishVolume: mount %s at %s successfully", source, target)
 
 	return &csi.NodePublishVolumeResponse{}, nil
@@ -409,8 +425,7 @@ func (d *Driver) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolume
 	// In kubernetes 1.19 above, it will be passed by kubelet.
 	devicePath := req.GetStagingTargetPath()
 	if devicePath == "" {
-		args := []string{"-o", "source", "--noheadings", "--mountpoint", volumePath}
-		output, err := d.mounter.Exec.Command("findmnt", args...).Output()
+		output, err := d.getDevicePathWithMountPath(volumePath)
 		if err != nil {
 			return nil, status.Errorf(codes.NotFound, "Could not determine device path: %v", err)
 		}
@@ -543,6 +558,21 @@ func (d *Driver) getDevicePathWithLUN(lunStr string) (string, error) {
 	return newDevicePath, err
 }
 
+func (d *Driver) getDevicePathWithMountPath(mountPath string) (string, error) {
+	args := []string{"-o", "source", "--noheadings", "--mountpoint", mountPath}
+	output, err := d.mounter.Exec.Command("findmnt", args...).Output()
+	if err != nil {
+		return "", status.Errorf(codes.Internal, "Could not determine device path: %v", err)
+	}
+
+	devicePath := strings.TrimSpace(string(output))
+	if len(devicePath) == 0 {
+		return "", status.Errorf(codes.Internal, "Could not get valid device for mount path: %q", mountPath)
+	}
+
+	return devicePath, nil
+}
+
 func (d *Driver) getBlockSizeBytes(devicePath string) (int64, error) {
 	output, err := d.mounter.Exec.Command("blockdev", "--getsize64", devicePath).Output()
 	if err != nil {


Test patch:
diff --git a/pkg/azuredisk/controllerserver_test.go b/pkg/azuredisk/controllerserver_test.go
index 6d9279236..d65410249 100644
--- a/pkg/azuredisk/controllerserver_test.go
+++ b/pkg/azuredisk/controllerserver_test.go
@@ -1784,3 +1784,105 @@ func TestPickAvailabilityZone(t *testing.T) {
 		t.Run(tc.name, tc.testFunc)
 	}
 }
+
+func TestGetSourceDiskSize(t *testing.T) {
+	testCases := []struct {
+		name     string
+		testFunc func(t *testing.T)
+	}{
+		{
+			name: "max depth reached",
+			testFunc: func(t *testing.T) {
+				d, _ := NewFakeDriver(t)
+				_, err := d.GetSourceDiskSize(context.Background(), "test-rg", "test-disk", 2, 1)
+				expectedErr := status.Errorf(codes.Internal, "current depth (2) surpassed the max depth (1) while searching for the source disk size")
+				if !reflect.DeepEqual(err, expectedErr) {
+					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
+				}
+			},
+		},
+		{
+			name: "diskproperty not found",
+			testFunc: func(t *testing.T) {
+				d, _ := NewFakeDriver(t)
+				disk := compute.Disk{}
+				d.cloud.DisksClient.(*mockdiskclient.MockInterface).EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(disk, nil).AnyTimes()
+				_, err := d.GetSourceDiskSize(context.Background(), "test-rg", "test-disk", 0, 1)
+				expectedErr := status.Error(codes.Internal, "DiskProperty not found for disk (test-disk) in resource group (test-rg)")
+				if !reflect.DeepEqual(err, expectedErr) {
+					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
+				}
+			},
+		},
+		{
+			name: "nil DiskSizeGB",
+			testFunc: func(t *testing.T) {
+				d, _ := NewFakeDriver(t)
+				diskProperties := compute.DiskProperties{}
+				disk := compute.Disk{
+					DiskProperties: &diskProperties,
+				}
+				d.cloud.DisksClient.(*mockdiskclient.MockInterface).EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(disk, nil).AnyTimes()
+				_, err := d.GetSourceDiskSize(context.Background(), "test-rg", "test-disk", 0, 1)
+				expectedErr := status.Error(codes.Internal, "DiskSizeGB for disk (test-disk) in resourcegroup (test-rg) is nil")
+				if !reflect.DeepEqual(err, expectedErr) {
+					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
+				}
+			},
+		},
+		{
+			name: "successful search: depth 1",
+			testFunc: func(t *testing.T) {
+				d, _ := NewFakeDriver(t)
+				diskSizeGB := int32(8)
+				diskProperties := compute.DiskProperties{
+					DiskSizeGB: &diskSizeGB,
+				}
+				disk := compute.Disk{
+					DiskProperties: &diskProperties,
+				}
+				d.cloud.DisksClient.(*mockdiskclient.MockInterface).EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(disk, nil).AnyTimes()
+				size, _ := d.GetSourceDiskSize(context.Background(), "test-rg", "test-disk", 0, 1)
+				expectedOutput := diskSizeGB
+				if *size != expectedOutput {
+					t.Errorf("actualOutput: (%v), expectedOutput: (%v)", *size, expectedOutput)
+				}
+			},
+		},
+		{
+			name: "successful serach: depth 2",
+			testFunc: func(t *testing.T) {
+				d, _ := NewFakeDriver(t)
+				diskSizeGB1 := int32(16)
+				diskSizeGB2 := int32(8)
+				sourceURI := "/subscriptions/xxxxxxxx/resourcegroups/test-rg/providers/microsoft.compute/disks/test-disk-1"
+				creationData := compute.CreationData{
+					CreateOption: "Copy",
+					SourceURI:    &sourceURI,
+				}
+				diskProperties1 := compute.DiskProperties{
+					CreationData: &creationData,
+					DiskSizeGB:   &diskSizeGB1,
+				}
+				diskProperties2 := compute.DiskProperties{
+					DiskSizeGB: &diskSizeGB2,
+				}
+				disk1 := compute.Disk{
+					DiskProperties: &diskProperties1,
+				}
+				disk2 := compute.Disk{
+					DiskProperties: &diskProperties2,
+				}
+				d.cloud.DisksClient.(*mockdiskclient.MockInterface).EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(disk1, nil).Return(disk2, nil).AnyTimes()
+				size, _ := d.GetSourceDiskSize(context.Background(), "test-rg", "test-disk-1", 0, 2)
+				expectedOutput := diskSizeGB2
+				if *size != expectedOutput {
+					t.Errorf("actualOutput: (%v), expectedOutput: (%v)", *size, expectedOutput)
+				}
+			},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, tc.testFunc)
+	}
+}
diff --git a/test/e2e/dynamic_provisioning_test.go b/test/e2e/dynamic_provisioning_test.go
index e1b28cade..4b9910b03 100644
--- a/test/e2e/dynamic_provisioning_test.go
+++ b/test/e2e/dynamic_provisioning_test.go
@@ -350,10 +350,6 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
 	ginkgo.It("should clone a volume from an existing volume and read from it [disk.csi.azure.com]", func() {
 		skipIfTestingInWindowsCluster()
 		skipIfUsingInTreeVolumePlugin()
-		if !isMultiZone {
-			// todo: remove this when single-az tests are runnong on 1.16
-			ginkgo.Skip("test case not supported by single-az test since it's running on 1.15")
-		}
 
 		pod := testsuites.PodDetails{
 			Cmd: "echo 'hello world' > /mnt/test-1/data",
@@ -383,6 +379,41 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
 		test.Run(cs, ns)
 	})
 
+	ginkgo.It("should clone a volume of larger size than the source volume and make sure the filesystem is appropriately adjusted [disk.csi.azure.com]", func() {
+		skipIfTestingInWindowsCluster()
+		skipIfUsingInTreeVolumePlugin()
+
+		pod := testsuites.PodDetails{
+			Volumes: t.normalizeVolumes([]testsuites.VolumeDetails{
+				{
+					FSType:    "ext4",
+					ClaimSize: "10Gi",
+					VolumeMount: testsuites.VolumeMountDetails{
+						NameGenerate:      "test-volume-",
+						MountPathGenerate: "/mnt/test-",
+					},
+				},
+			}, isMultiZone),
+		}
+		clonedVolumeSize := "20Gi"
+
+		podWithClonedVolume := testsuites.PodDetails{
+			Cmd: "df -h | grep /mnt/test- | awk '{print $2}' | grep 20.0G",
+		}
+
+		test := testsuites.DynamicallyProvisionedVolumeCloningTest{
+			CSIDriver:           testDriver,
+			Pod:                 pod,
+			PodWithClonedVolume: podWithClonedVolume,
+			ClonedVolumeSize:    clonedVolumeSize,
+			StorageClassParameters: map[string]string{
+				"skuName": "Standard_LRS",
+				"fsType":  "xfs",
+			},
+		}
+		test.Run(cs, ns)
+	})
+
 	ginkgo.It("should create multiple PV objects, bind to PVCs and attach all to a single pod [kubernetes.io/azure-disk] [disk.csi.azure.com] [Windows]", func() {
 		pods := []testsuites.PodDetails{
 			{
diff --git a/test/e2e/testsuites/dynamically_provisioned_volume_cloning_tester.go b/test/e2e/testsuites/dynamically_provisioned_volume_cloning_tester.go
index ed08ca2d0..61d068a6a 100644
--- a/test/e2e/testsuites/dynamically_provisioned_volume_cloning_tester.go
+++ b/test/e2e/testsuites/dynamically_provisioned_volume_cloning_tester.go
@@ -25,10 +25,12 @@ import (
 )
 
 // DynamicallyProvisionedVolumeCloningTest will provision required StorageClass(es), PVC(s) and Pod(s)
+// ClonedVolumeSize optional for when testing for cloned volume with different size to the original volume
 type DynamicallyProvisionedVolumeCloningTest struct {
 	CSIDriver              driver.DynamicPVTestDriver
 	Pod                    PodDetails
 	PodWithClonedVolume    PodDetails
+	ClonedVolumeSize       string
 	StorageClassParameters map[string]string
 }
 
@@ -57,6 +59,11 @@ func (t *DynamicallyProvisionedVolumeCloningTest) Run(client clientset.Interface
 		Kind: VolumePVCKind,
 	}
 	clonedVolume.StorageClass = tsc.storageClass
+
+	if t.ClonedVolumeSize != "" {
+		clonedVolume.ClaimSize = t.ClonedVolumeSize
+	}
+
 	t.PodWithClonedVolume.Volumes = []VolumeDetails{clonedVolume}
 	tpod, cleanups = t.PodWithClonedVolume.SetupWithDynamicVolumes(client, namespace, t.CSIDriver, t.StorageClassParameters)
 	for i := range cleanups {