Some changes based on code review. Will squash before merge. Add e2e test for block volume resize. Add nodeExpand validation

Author: davidz627

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

@@ -66,6 +66,13 @@ func (gceIdentity *GCEIdentityServer) GetPluginCapabilities(ctx context.Context,
 					},
 				},
 			},
+			{
+				Type: &csi.PluginCapability_VolumeExpansion_{
+					VolumeExpansion: &csi.PluginCapability_VolumeExpansion{
+						Type: csi.PluginCapability_VolumeExpansion_OFFLINE,
+					},
+				},
+			},
 		},
 	}, nil
 }

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

@@ -62,6 +62,7 @@ func TestGetPluginCapabilities(t *testing.T) {
 		if capability.GetVolumeExpansion() != nil {
 			switch capability.GetVolumeExpansion().GetType() {
 			case csi.PluginCapability_VolumeExpansion_ONLINE:
+			case csi.PluginCapability_VolumeExpansion_OFFLINE:
 			default:
 				t.Fatalf("Unknown capability: %v", capability.GetVolumeExpansion().GetType())
 			}

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

@@ -17,6 +17,7 @@ package gceGCEDriver
 import (
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -377,23 +378,55 @@ func (ns *GCENodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpa
 		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("ControllerExpandVolume capacity range is invalid: %v", err))
 	}
 
+	volumePath := req.GetVolumePath()
+	if len(volumePath) == 0 {
+		return nil, status.Error(codes.InvalidArgument, "ControllerExpandVolume volume path must be provided")
+	}
+
 	volKey, err := common.VolumeIDToKey(volumeID)
 	if err != nil {
 		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("ControllerExpandVolume volume ID is invalid: %v", err))
 	}
 
 	devicePath, err := ns.getDevicePath(volumeID, "")
 	if err != nil {
-		return nil, status.Error(codes.Internal, fmt.Sprintf("Error when getting device path for %s: %v", volumeID, err))
+		return nil, status.Error(codes.Internal, fmt.Sprintf("ControllerExpandVolume error when getting device path for %s: %v", volumeID, err))
 	}
 
 	resizer := resizefs.NewResizeFs(ns.Mounter)
-	resized, err := resizer.Resize(devicePath, "")
+	resized, err := resizer.Resize(devicePath, volumePath)
 	if err != nil {
-		return nil, status.Error(codes.Internal, fmt.Sprintf("Error when resizing volume %s: %v", volKey.String(), err))
+		return nil, status.Error(codes.Internal, fmt.Sprintf("ControllerExpandVolume error when resizing volume %s: %v", volKey.String(), err))
 
 	}
 
+	// Verify the resize actually happened.
+	format, err := ns.Mounter.GetDiskFormat(devicePath)
+	if err != nil {
+		return nil, status.Error(codes.Internal, fmt.Sprintf("ControllerExpandVolume error checking format for device %s: %v", devicePath, err))
+	}
+	if format == "" {
+		// Block volume
+		gotSizeGb, err := ns.getBlockSizeGb(devicePath)
+		if err != nil {
+			return nil, status.Error(codes.Internal, fmt.Sprintf("ControllerExpandVolume error when getting size of block volume at path %s: %v", devicePath, err))
+		}
+		if gotSizeGb != common.BytesToGb(reqBytes) {
+			return nil, status.Errorf(codes.Internal, "ControllerExpandVolume resize requested for %v but after resize volume was size %v", common.BytesToGb(reqBytes), gotSizeGb)
+		}
+	} else {
+		// Some sort of formatted volume
+		gotSizeGb, err := ns.getFSSizeGb(devicePath)
+
+		if err != nil {
+			return nil, status.Errorf(codes.Internal, "ControllerExpandVolume resize could not get fs size of %s: %v", volumePath, err)
+		}
+		if gotSizeGb != common.BytesToGb(reqBytes) {
+			return nil, status.Errorf(codes.Internal, "ControllerExpandVolume resize requested for size %v but after resize volume was size %v", common.BytesToGb(reqBytes), gotSizeGb)
+		}
+	}
+
+	// Respond
 	resp := &csi.NodeExpandVolumeResponse{}
 	if resized {
 		resp.CapacityBytes = reqBytes
@@ -435,3 +468,35 @@ func (ns *GCENodeServer) getDevicePath(volumeID string, partition string) (strin
 	}
 	return devicePath, nil
 }
+
+func (ns *GCENodeServer) getBlockSizeGb(devicePath string) (int64, error) {
+	output, err := ns.Mounter.Exec.Run("blockdev", "--getsize64", devicePath)
+	if err != nil {
+		return -1, fmt.Errorf("error when getting size of block volume at path %s: output: %s, err: %v", devicePath, string(output), err)
+	}
+	strOut := strings.TrimSpace(string(output))
+	gotSizeBytes, err := strconv.ParseInt(strOut, 10, 64)
+	if err != nil {
+		return -1, fmt.Errorf("failed to parse size %s into int a size", strOut)
+	}
+	return common.BytesToGb(gotSizeBytes), nil
+}
+
+func (ns *GCENodeServer) getFSSizeGb(volumePath string) (int64, error) {
+	o, err := ns.Mounter.Exec.Run("df", "--output=size", "-BG", volumePath)
+	if err != nil {
+		return -1, fmt.Errorf("failed to run command, output: %s, err: %v", string(o), err)
+	}
+
+	tokens := strings.Split(strings.TrimSpace(string(o)), "\n")
+	if len(tokens) != 2 {
+		return -1, fmt.Errorf("got unexpected df output: %s", string(o))
+	}
+
+	output := strings.TrimSuffix(strings.TrimSpace(tokens[1]), "G")
+	n, err := strconv.ParseInt(output, 10, 64)
+	if err != nil {
+		return -1, fmt.Errorf("failed to parse %s into int", output)
+	}
+	return n, nil
+}

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

@@ -17,6 +17,7 @@ package gceGCEDriver
 import (
 	"errors"
 	"fmt"
+	"strconv"
 	"testing"
 
 	"context"
@@ -26,6 +27,7 @@ import (
 	"google.golang.org/grpc/status"
 	"k8s.io/kubernetes/pkg/util/mount"
 	utilexec "k8s.io/utils/exec"
+	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
 	metadataservice "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/metadata"
 	mountmanager "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/mount-manager"
 )
@@ -348,7 +350,7 @@ func TestNodeExpandVolume(t *testing.T) {
 	testCases := []struct {
 		name         string
 		req          *csi.NodeExpandVolumeRequest
-		blockDevice  bool
+		fsOrBlock    string
 		expRespBytes int64
 		expErrCode   codes.Code
 	}{
@@ -361,7 +363,7 @@ func TestNodeExpandVolume(t *testing.T) {
 					RequiredBytes: resizedBytes,
 				},
 			},
-			blockDevice:  false,
+			fsOrBlock:    "ext4",
 			expRespBytes: resizedBytes,
 		},
 		{
@@ -373,29 +375,56 @@ func TestNodeExpandVolume(t *testing.T) {
 					RequiredBytes: resizedBytes,
 				},
 			},
-			blockDevice: true,
+			fsOrBlock: "block",
+		},
+		{
+			name: "xfs fs expand",
+			req: &csi.NodeExpandVolumeRequest{
+				VolumeId:   volumeID,
+				VolumePath: "some-path",
+				CapacityRange: &csi.CapacityRange{
+					RequiredBytes: resizedBytes,
+				},
+			},
+			fsOrBlock:    "xfs",
+			expRespBytes: resizedBytes,
 		},
 	}
 	for _, tc := range testCases {
 		t.Logf("Test case: %s", tc.name)
 
 		execCallback := func(cmd string, args ...string) ([]byte, error) {
-			if cmd == "blkid" {
-				if tc.blockDevice {
+			switch cmd {
+			case "blkid":
+				if tc.fsOrBlock == "block" {
 					// blkid returns exit code 2 when run on unformatted device
 					return nil, utilexec.CodeExitError{
 						Err:  errors.New("this is an exit error"),
 						Code: 2,
 					}
-				} else {
-					return []byte("DEVNAME=/dev/sdb\nTYPE=ext4"), nil
 				}
-			} else if cmd == "resize2fs" {
-				if tc.blockDevice {
-					t.Fatalf("resize fs called on block device")
+				return []byte(fmt.Sprintf("DEVNAME=/dev/sdb\nTYPE=%s", tc.fsOrBlock)), nil
+			case "resize2fs":
+				if tc.fsOrBlock == "ext4" {
+					return nil, nil
+				}
+				t.Fatalf("resize fs called on device with %s", tc.fsOrBlock)
+			case "xfs_growfs":
+				if tc.fsOrBlock != "xfs" {
+					t.Fatalf("xfs_growfs called on device with %s", tc.fsOrBlock)
 				}
-				return nil, nil
+				for _, arg := range args {
+					if arg == tc.req.VolumePath {
+						return nil, nil
+					}
+				}
+				t.Errorf("xfs_growfs args did not contain volume path %s", tc.req.VolumePath)
+			case "df":
+				return []byte(fmt.Sprintf("FOO\n%v", common.BytesToGb(resizedBytes))), nil
+			case "blockdev":
+				return []byte(strconv.Itoa(int(resizedBytes))), nil
 			}
+
 			return nil, fmt.Errorf("fake exec got unknown call to %v %v", cmd, args)
 		}
 		mounter := mountmanager.NewFakeSafeMounterWithCustomExec(mount.NewFakeExec(execCallback))

test/e2e/tests/multi_zone_e2e_test.go

@@ -165,9 +165,9 @@ func testAttachWriteReadDetach(volID string, volName string, instance *remote.In
 
 	// Stage Disk
 	stageDir := filepath.Join("/tmp/", volName, "stage")
-	err = client.NodeStageVolume(volID, stageDir)
+	err = client.NodeStageExt4Volume(volID, stageDir)
 	if err != nil {
-		return fmt.Errorf("NodeStageVolume failed with error: %v", err)
+		return fmt.Errorf("NodeStageExt4Volume failed with error: %v", err)
 	}
 
 	defer func() {

test/e2e/tests/single_zone_e2e_test.go

@@ -101,7 +101,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
 
 	})
 
-	It("Should resize controller and node", func() {
+	It("Should resize controller and node for an ext4 volume", func() {
 		testContext := getRandomTestContext()
 
 		p, z, _ := testContext.Instance.GetIdentity()
@@ -153,7 +153,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
 
 		// Stage Disk
 		stageDir := filepath.Join("/tmp/", volName, "stage")
-		err = client.NodeStageVolume(volID, stageDir)
+		err = client.NodeStageExt4Volume(volID, stageDir)
 		Expect(err).To(BeNil(), "Node Stage volume failed")
 
 		defer func() {
@@ -199,7 +199,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
 		Expect(cloudDisk.SizeGb).To(Equal(newSizeGb))
 
 		// Resize node
-		err = client.NodeExpandVolume(volID, publishDir, newSizeGb)
+		_, err = client.NodeExpandVolume(volID, publishDir, newSizeGb)
 		Expect(err).To(BeNil(), "Node expand volume failed")
 
 		// Verify disk size
@@ -209,6 +209,115 @@ var _ = Describe("GCE PD CSI Driver", func() {
 
 	})
 
+	It("Should resize controller and node for an block volume", func() {
+		testContext := getRandomTestContext()
+
+		p, z, _ := testContext.Instance.GetIdentity()
+		client := testContext.Client
+		instance := testContext.Instance
+
+		// Create Disk
+		volName := testNamePrefix + string(uuid.NewUUID())
+		volID, err := client.CreateVolume(volName, nil, defaultSizeGb,
+			&csi.TopologyRequirement{
+				Requisite: []*csi.Topology{
+					{
+						Segments: map[string]string{common.TopologyKeyZone: z},
+					},
+				},
+			})
+		Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
+
+		// Validate Disk Created
+		cloudDisk, err := computeService.Disks.Get(p, z, volName).Do()
+		Expect(err).To(BeNil(), "Could not get disk from cloud directly")
+		Expect(cloudDisk.Type).To(ContainSubstring(standardDiskType))
+		Expect(cloudDisk.Status).To(Equal(readyState))
+		Expect(cloudDisk.SizeGb).To(Equal(defaultSizeGb))
+		Expect(cloudDisk.Name).To(Equal(volName))
+
+		defer func() {
+			// Delete Disk
+			client.DeleteVolume(volID)
+			Expect(err).To(BeNil(), "DeleteVolume failed")
+
+			// Validate Disk Deleted
+			_, err = computeService.Disks.Get(p, z, volName).Do()
+			Expect(gce.IsGCEError(err, "notFound")).To(BeTrue(), "Expected disk to not be found")
+		}()
+
+		// Attach Disk
+		err = client.ControllerPublishVolume(volID, instance.GetNodeID())
+		Expect(err).To(BeNil(), "Controller publish volume failed")
+
+		defer func() {
+			// Detach Disk
+			err = client.ControllerUnpublishVolume(volID, instance.GetNodeID())
+			if err != nil {
+				klog.Errorf("Failed to detach disk: %v", err)
+			}
+
+		}()
+
+		// Stage Disk
+		stageDir := filepath.Join("/tmp/", volName, "stage")
+		err = client.NodeStageBlockVolume(volID, stageDir)
+		Expect(err).To(BeNil(), "Node Stage volume failed")
+
+		defer func() {
+			// Unstage Disk
+			err = client.NodeUnstageVolume(volID, stageDir)
+			if err != nil {
+				klog.Errorf("Failed to unstage volume: %v", err)
+			}
+			fp := filepath.Join("/tmp/", volName)
+			err = testutils.RmAll(instance, fp)
+			if err != nil {
+				klog.Errorf("Failed to rm file path %s: %v", fp, err)
+			}
+		}()
+
+		// Mount Disk
+		publishDir := filepath.Join("/tmp/", volName, "mount")
+		err = client.NodePublishBlockVolume(volID, stageDir, publishDir)
+		Expect(err).To(BeNil(), "Node publish volume failed")
+
+		defer func() {
+			// Unmount Disk
+			err = client.NodeUnpublishVolume(volID, publishDir)
+			if err != nil {
+				klog.Errorf("NodeUnpublishVolume failed with error: %v", err)
+			}
+		}()
+
+		// Verify pre-resize fs size
+		sizeGb, err := testutils.GetBlockSizeInGb(instance, publishDir)
+		Expect(err).To(BeNil(), "Failed to get block device size in GB")
+		Expect(sizeGb).To(Equal(defaultSizeGb), "Old size should be equal")
+
+		// Resize controller
+		var newSizeGb int64 = 10
+		err = client.ControllerExpandVolume(volID, newSizeGb)
+
+		Expect(err).To(BeNil(), "Controller expand volume failed")
+
+		// Verify cloud size
+		cloudDisk, err = computeService.Disks.Get(p, z, volName).Do()
+		Expect(err).To(BeNil(), "Get cloud disk failed")
+		Expect(cloudDisk.SizeGb).To(Equal(newSizeGb))
+
+		// Resize node
+		resp, err := client.NodeExpandVolume(volID, publishDir, newSizeGb)
+		Expect(err).To(BeNil(), "Node expand volume failed")
+		Expect(resp.CapacityBytes).To(Equal(int64(0)), "Node expand should not do anything")
+
+		// Verify disk size
+		sizeGb, err = testutils.GetBlockSizeInGb(instance, publishDir)
+		Expect(err).To(BeNil(), "Failed to get block device size in GB")
+		Expect(sizeGb).To(Equal(newSizeGb), "New size should be equal")
+
+	})
+
 	It("Should create disks in correct zones when topology is specified", func() {
 		Expect(testContexts).ToNot(BeEmpty())
 		testContext := getRandomTestContext()