Skip to content

Commit 1ed5050

Browse files
davidz627davidz627
committed
Some changes based on code review. Will squash before merge. Add e2e test for block volume resize. Add nodeExpand validation
1 parent 7713644 commit 1ed5050

File tree

10 files changed

+525
-136
lines changed

10 files changed

+525
-136
lines changed

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

+7
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@ func (gceIdentity *GCEIdentityServer) GetPluginCapabilities(ctx context.Context,
6666
},
6767
},
6868
},
69+
{
70+
Type: &csi.PluginCapability_VolumeExpansion_{
71+
VolumeExpansion: &csi.PluginCapability_VolumeExpansion{
72+
Type: csi.PluginCapability_VolumeExpansion_OFFLINE,
73+
},
74+
},
75+
},
6976
},
7077
}, nil
7178
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ func TestGetPluginCapabilities(t *testing.T) {
6262
if capability.GetVolumeExpansion() != nil {
6363
switch capability.GetVolumeExpansion().GetType() {
6464
case csi.PluginCapability_VolumeExpansion_ONLINE:
65+
case csi.PluginCapability_VolumeExpansion_OFFLINE:
6566
default:
6667
t.Fatalf("Unknown capability: %v", capability.GetVolumeExpansion().GetType())
6768
}

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

+68-3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package gceGCEDriver
1717
import (
1818
"fmt"
1919
"os"
20+
"strconv"
2021
"strings"
2122
"sync"
2223

@@ -377,23 +378,55 @@ func (ns *GCENodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpa
377378
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("ControllerExpandVolume capacity range is invalid: %v", err))
378379
}
379380

381+
volumePath := req.GetVolumePath()
382+
if len(volumePath) == 0 {
383+
return nil, status.Error(codes.InvalidArgument, "ControllerExpandVolume volume path must be provided")
384+
}
385+
380386
volKey, err := common.VolumeIDToKey(volumeID)
381387
if err != nil {
382388
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("ControllerExpandVolume volume ID is invalid: %v", err))
383389
}
384390

385391
devicePath, err := ns.getDevicePath(volumeID, "")
386392
if err != nil {
387-
return nil, status.Error(codes.Internal, fmt.Sprintf("Error when getting device path for %s: %v", volumeID, err))
393+
return nil, status.Error(codes.Internal, fmt.Sprintf("ControllerExpandVolume error when getting device path for %s: %v", volumeID, err))
388394
}
389395

390396
resizer := resizefs.NewResizeFs(ns.Mounter)
391-
resized, err := resizer.Resize(devicePath, "")
397+
resized, err := resizer.Resize(devicePath, volumePath)
392398
if err != nil {
393-
return nil, status.Error(codes.Internal, fmt.Sprintf("Error when resizing volume %s: %v", volKey.String(), err))
399+
return nil, status.Error(codes.Internal, fmt.Sprintf("ControllerExpandVolume error when resizing volume %s: %v", volKey.String(), err))
394400

395401
}
396402

403+
// Verify the resize actually happened.
404+
format, err := ns.Mounter.GetDiskFormat(devicePath)
405+
if err != nil {
406+
return nil, status.Error(codes.Internal, fmt.Sprintf("ControllerExpandVolume error checking format for device %s: %v", devicePath, err))
407+
}
408+
if format == "" {
409+
// Block volume
410+
gotSizeGb, err := ns.getBlockSizeGb(devicePath)
411+
if err != nil {
412+
return nil, status.Error(codes.Internal, fmt.Sprintf("ControllerExpandVolume error when getting size of block volume at path %s: %v", devicePath, err))
413+
}
414+
if gotSizeGb != common.BytesToGb(reqBytes) {
415+
return nil, status.Errorf(codes.Internal, "ControllerExpandVolume resize requested for %v but after resize volume was size %v", common.BytesToGb(reqBytes), gotSizeGb)
416+
}
417+
} else {
418+
// Some sort of formatted volume
419+
gotSizeGb, err := ns.getFSSizeGb(devicePath)
420+
421+
if err != nil {
422+
return nil, status.Errorf(codes.Internal, "ControllerExpandVolume resize could not get fs size of %s: %v", volumePath, err)
423+
}
424+
if gotSizeGb != common.BytesToGb(reqBytes) {
425+
return nil, status.Errorf(codes.Internal, "ControllerExpandVolume resize requested for size %v but after resize volume was size %v", common.BytesToGb(reqBytes), gotSizeGb)
426+
}
427+
}
428+
429+
// Respond
397430
resp := &csi.NodeExpandVolumeResponse{}
398431
if resized {
399432
resp.CapacityBytes = reqBytes
@@ -435,3 +468,35 @@ func (ns *GCENodeServer) getDevicePath(volumeID string, partition string) (strin
435468
}
436469
return devicePath, nil
437470
}
471+
472+
func (ns *GCENodeServer) getBlockSizeGb(devicePath string) (int64, error) {
473+
output, err := ns.Mounter.Exec.Run("blockdev", "--getsize64", devicePath)
474+
if err != nil {
475+
return -1, fmt.Errorf("error when getting size of block volume at path %s: output: %s, err: %v", devicePath, string(output), err)
476+
}
477+
strOut := strings.TrimSpace(string(output))
478+
gotSizeBytes, err := strconv.ParseInt(strOut, 10, 64)
479+
if err != nil {
480+
return -1, fmt.Errorf("failed to parse size %s into int a size", strOut)
481+
}
482+
return common.BytesToGb(gotSizeBytes), nil
483+
}
484+
485+
func (ns *GCENodeServer) getFSSizeGb(volumePath string) (int64, error) {
486+
o, err := ns.Mounter.Exec.Run("df", "--output=size", "-BG", volumePath)
487+
if err != nil {
488+
return -1, fmt.Errorf("failed to run command, output: %s, err: %v", string(o), err)
489+
}
490+
491+
tokens := strings.Split(strings.TrimSpace(string(o)), "\n")
492+
if len(tokens) != 2 {
493+
return -1, fmt.Errorf("got unexpected df output: %s", string(o))
494+
}
495+
496+
output := strings.TrimSuffix(strings.TrimSpace(tokens[1]), "G")
497+
n, err := strconv.ParseInt(output, 10, 64)
498+
if err != nil {
499+
return -1, fmt.Errorf("failed to parse %s into int", output)
500+
}
501+
return n, nil
502+
}

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

+40-11
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package gceGCEDriver
1717
import (
1818
"errors"
1919
"fmt"
20+
"strconv"
2021
"testing"
2122

2223
"context"
@@ -26,6 +27,7 @@ import (
2627
"google.golang.org/grpc/status"
2728
"k8s.io/kubernetes/pkg/util/mount"
2829
utilexec "k8s.io/utils/exec"
30+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
2931
metadataservice "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/metadata"
3032
mountmanager "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/mount-manager"
3133
)
@@ -348,7 +350,7 @@ func TestNodeExpandVolume(t *testing.T) {
348350
testCases := []struct {
349351
name string
350352
req *csi.NodeExpandVolumeRequest
351-
blockDevice bool
353+
fsOrBlock string
352354
expRespBytes int64
353355
expErrCode codes.Code
354356
}{
@@ -361,7 +363,7 @@ func TestNodeExpandVolume(t *testing.T) {
361363
RequiredBytes: resizedBytes,
362364
},
363365
},
364-
blockDevice: false,
366+
fsOrBlock: "ext4",
365367
expRespBytes: resizedBytes,
366368
},
367369
{
@@ -373,29 +375,56 @@ func TestNodeExpandVolume(t *testing.T) {
373375
RequiredBytes: resizedBytes,
374376
},
375377
},
376-
blockDevice: true,
378+
fsOrBlock: "block",
379+
},
380+
{
381+
name: "xfs fs expand",
382+
req: &csi.NodeExpandVolumeRequest{
383+
VolumeId: volumeID,
384+
VolumePath: "some-path",
385+
CapacityRange: &csi.CapacityRange{
386+
RequiredBytes: resizedBytes,
387+
},
388+
},
389+
fsOrBlock: "xfs",
390+
expRespBytes: resizedBytes,
377391
},
378392
}
379393
for _, tc := range testCases {
380394
t.Logf("Test case: %s", tc.name)
381395

382396
execCallback := func(cmd string, args ...string) ([]byte, error) {
383-
if cmd == "blkid" {
384-
if tc.blockDevice {
397+
switch cmd {
398+
case "blkid":
399+
if tc.fsOrBlock == "block" {
385400
// blkid returns exit code 2 when run on unformatted device
386401
return nil, utilexec.CodeExitError{
387402
Err: errors.New("this is an exit error"),
388403
Code: 2,
389404
}
390-
} else {
391-
return []byte("DEVNAME=/dev/sdb\nTYPE=ext4"), nil
392405
}
393-
} else if cmd == "resize2fs" {
394-
if tc.blockDevice {
395-
t.Fatalf("resize fs called on block device")
406+
return []byte(fmt.Sprintf("DEVNAME=/dev/sdb\nTYPE=%s", tc.fsOrBlock)), nil
407+
case "resize2fs":
408+
if tc.fsOrBlock == "ext4" {
409+
return nil, nil
410+
}
411+
t.Fatalf("resize fs called on device with %s", tc.fsOrBlock)
412+
case "xfs_growfs":
413+
if tc.fsOrBlock != "xfs" {
414+
t.Fatalf("xfs_growfs called on device with %s", tc.fsOrBlock)
396415
}
397-
return nil, nil
416+
for _, arg := range args {
417+
if arg == tc.req.VolumePath {
418+
return nil, nil
419+
}
420+
}
421+
t.Errorf("xfs_growfs args did not contain volume path %s", tc.req.VolumePath)
422+
case "df":
423+
return []byte(fmt.Sprintf("FOO\n%v", common.BytesToGb(resizedBytes))), nil
424+
case "blockdev":
425+
return []byte(strconv.Itoa(int(resizedBytes))), nil
398426
}
427+
399428
return nil, fmt.Errorf("fake exec got unknown call to %v %v", cmd, args)
400429
}
401430
mounter := mountmanager.NewFakeSafeMounterWithCustomExec(mount.NewFakeExec(execCallback))

‎test/e2e/tests/multi_zone_e2e_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,9 @@ func testAttachWriteReadDetach(volID string, volName string, instance *remote.In
165165

166166
// Stage Disk
167167
stageDir := filepath.Join("/tmp/", volName, "stage")
168-
err = client.NodeStageVolume(volID, stageDir)
168+
err = client.NodeStageExt4Volume(volID, stageDir)
169169
if err != nil {
170-
return fmt.Errorf("NodeStageVolume failed with error: %v", err)
170+
return fmt.Errorf("NodeStageExt4Volume failed with error: %v", err)
171171
}
172172

173173
defer func() {

0 commit comments

Comments
 (0)