Some changes based on code review. Will squash before merge. Add e2e

test for block volume resize

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

@@ -377,6 +377,11 @@ 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))
@@ -388,7 +393,7 @@ func (ns *GCENodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpa
 	}
 
 	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))
 

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

@@ -348,7 +348,7 @@ func TestNodeExpandVolume(t *testing.T) {
 	testCases := []struct {
 		name         string
 		req          *csi.NodeExpandVolumeRequest
-		blockDevice  bool
+		fsOrBlock    string
 		expRespBytes int64
 		expErrCode   codes.Code
 	}{
@@ -361,7 +361,7 @@ func TestNodeExpandVolume(t *testing.T) {
 					RequiredBytes: resizedBytes,
 				},
 			},
-			blockDevice:  false,
+			fsOrBlock:    "ext4",
 			expRespBytes: resizedBytes,
 		},
 		{
@@ -373,28 +373,50 @@ 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)
+				}
+				for _, arg := range args {
+					if arg == tc.req.VolumePath {
+						return nil, nil
+					}
 				}
-				return nil, nil
+				t.Errorf("xfs_growfs args did not contain volume path %s", tc.req.VolumePath)
 			}
 			return nil, fmt.Errorf("fake exec got unknown call to %v %v", cmd, args)
 		}

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()

test/e2e/utils/utils.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/klog"
 	boskosclient "k8s.io/test-infra/boskos/client"
 	"k8s.io/test-infra/boskos/common"
+	utilcommon "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
 	remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/remote"
 )
 
@@ -169,10 +170,10 @@ func ReadFile(instance *remote.InstanceInfo, filePath string) (string, error) {
 	return output, nil
 }
 
-func GetFSSizeInGb(instance *remote.InstanceInfo, path string) (int64, error) {
-	output, err := instance.SSHNoSudo("df", "--output=size", "-BG", path, "|", "awk", "'NR==2'")
+func GetFSSizeInGb(instance *remote.InstanceInfo, mountPath string) (int64, error) {
+	output, err := instance.SSHNoSudo("df", "--output=size", "-BG", mountPath, "|", "awk", "'NR==2'")
 	if err != nil {
-		return -1, fmt.Errorf("failed to get size of path %s. Output: %v, error: %v", path, output, err)
+		return -1, fmt.Errorf("failed to get size of path %s. Output: %v, error: %v", mountPath, output, err)
 	}
 	output = strings.TrimSuffix(strings.TrimSpace(output), "G")
 	n, err := strconv.ParseInt(output, 10, 64)
@@ -182,6 +183,18 @@ func GetFSSizeInGb(instance *remote.InstanceInfo, path string) (int64, error) {
 	return n, nil
 }
 
+func GetBlockSizeInGb(instance *remote.InstanceInfo, devicePath string) (int64, error) {
+	output, err := instance.SSH("blockdev", "--getsize64", devicePath)
+	if err != nil {
+		return -1, fmt.Errorf("failed to get size of path %s. Output: %v, error: %v", devicePath, output, err)
+	}
+	n, err := strconv.ParseInt(strings.TrimSpace(output), 10, 64)
+	if err != nil {
+		return -1, fmt.Errorf("failed to parse size %s into int", output)
+	}
+	return utilcommon.BytesToGb(n), nil
+}
+
 func RmAll(instance *remote.InstanceInfo, filePath string) error {
 	output, err := instance.SSH("rm", "-rf", filePath)
 	if err != nil {

test/remote/client-wrappers.go

@@ -36,6 +36,14 @@ var (
 			Mode: csipb.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
 		},
 	}
+	blockVolCap = &csipb.VolumeCapability{
+		AccessType: &csipb.VolumeCapability_Block{
+			Block: &csipb.VolumeCapability_BlockVolume{},
+		},
+		AccessMode: &csipb.VolumeCapability_AccessMode{
+			Mode: csipb.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
+		},
+	}
 	stdVolCaps = []*csipb.VolumeCapability{
 		stdVolCap,
 	}
@@ -139,11 +147,19 @@ func (c *CsiClient) ControllerUnpublishVolume(volId, nodeId string) error {
 	return err
 }
 
-func (c *CsiClient) NodeStageVolume(volId, stageDir string) error {
+func (c *CsiClient) NodeStageExt4Volume(volId, stageDir string) error {
+	return c.NodeStageVolume(volId, stageDir, stdVolCap)
+}
+
+func (c *CsiClient) NodeStageBlockVolume(volId, stageDir string) error {
+	return c.NodeStageVolume(volId, stageDir, blockVolCap)
+}
+
+func (c *CsiClient) NodeStageVolume(volId, stageDir string, volumeCap *csipb.VolumeCapability) error {
 	nodeStageReq := &csipb.NodeStageVolumeRequest{
 		VolumeId:          volId,
 		StagingTargetPath: stageDir,
-		VolumeCapability:  stdVolCap,
+		VolumeCapability:  volumeCap,
 	}
 	_, err := c.nodeClient.NodeStageVolume(context.Background(), nodeStageReq)
 	return err
@@ -179,6 +195,18 @@ func (c *CsiClient) NodePublishVolume(volumeId, stageDir, publishDir string) err
 	return err
 }
 
+func (c *CsiClient) NodePublishBlockVolume(volumeId, stageDir, publishDir string) error {
+	nodePublishReq := &csipb.NodePublishVolumeRequest{
+		VolumeId:          volumeId,
+		StagingTargetPath: stageDir,
+		TargetPath:        publishDir,
+		VolumeCapability:  blockVolCap,
+		Readonly:          false,
+	}
+	_, err := c.nodeClient.NodePublishVolume(context.Background(), nodePublishReq)
+	return err
+}
+
 func (c *CsiClient) ControllerExpandVolume(volumeId string, sizeGb int64) error {
 	controllerExpandReq := &csipb.ControllerExpandVolumeRequest{
 		VolumeId: volumeId,
@@ -190,16 +218,15 @@ func (c *CsiClient) ControllerExpandVolume(volumeId string, sizeGb int64) error
 	return err
 }
 
-func (c *CsiClient) NodeExpandVolume(volumeId, volumePath string, sizeGb int64) error {
+func (c *CsiClient) NodeExpandVolume(volumeId, volumePath string, sizeGb int64) (*csipb.NodeExpandVolumeResponse, error) {
 	nodeExpandReq := &csipb.NodeExpandVolumeRequest{
 		VolumeId:   volumeId,
 		VolumePath: volumePath,
 		CapacityRange: &csipb.CapacityRange{
 			RequiredBytes: common.GbToBytes(sizeGb),
 		},
 	}
-	_, err := c.nodeClient.NodeExpandVolume(context.Background(), nodeExpandReq)
-	return err
+	return c.nodeClient.NodeExpandVolume(context.Background(), nodeExpandReq)
 }
 
 func (c *CsiClient) NodeGetInfo() (*csipb.NodeGetInfoResponse, error) {

test/run-e2e-local.sh

@@ -5,4 +5,4 @@ set -o errexit
 
 readonly PKGDIR=sigs.k8s.io/gcp-compute-persistent-disk-csi-driver
 
-ginkgo --v "test/e2e/tests" -- --project ${PROJECT} --service-account ${IAM_NAME} --v=4 --logtostderr
+ginkgo --focus="resize\scontroller\sand\snode\sfor\san\sblock\svolume" --v "test/e2e/tests" -- --project ${PROJECT} --service-account ${IAM_NAME} --v=4 --logtostderr