diff --git a/docs/kubernetes/development.md b/docs/kubernetes/development.md index abaca7aba..02b2c533a 100644 --- a/docs/kubernetes/development.md +++ b/docs/kubernetes/development.md @@ -102,9 +102,6 @@ You may see errors during driver deployment. ### Sanity tests -> ***NOTE:*** Sanity tests are currently failing, tracked by -> https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues/990. - ``` ./test/run-sanity.sh ``` diff --git a/pkg/deviceutils/fake-device-utils.go b/pkg/deviceutils/fake-device-utils.go index 747340761..d17c43e55 100644 --- a/pkg/deviceutils/fake-device-utils.go +++ b/pkg/deviceutils/fake-device-utils.go @@ -17,12 +17,15 @@ package deviceutils import "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/resizefs" type fakeDeviceUtils struct { + skipResize bool } var _ DeviceUtils = &fakeDeviceUtils{} -func NewFakeDeviceUtils() *fakeDeviceUtils { - return &fakeDeviceUtils{} +func NewFakeDeviceUtils(skipResize bool) *fakeDeviceUtils { + return &fakeDeviceUtils{ + skipResize: skipResize, + } } // Returns list of all /dev/disk/by-id/* paths for given PD. @@ -41,6 +44,9 @@ func (_ *fakeDeviceUtils) DisableDevice(devicePath string) error { return nil } -func (_ *fakeDeviceUtils) Resize(resizer resizefs.Resizefs, devicePath string, deviceMountPath string) (bool, error) { - return false, nil +func (du *fakeDeviceUtils) Resize(resizer resizefs.Resizefs, devicePath string, deviceMountPath string) (bool, error) { + if du.skipResize { + return false, nil + } + return resizer.Resize(devicePath, deviceMountPath) } diff --git a/pkg/gce-pd-csi-driver/node.go b/pkg/gce-pd-csi-driver/node.go index 10029ea29..68d6f7f58 100644 --- a/pkg/gce-pd-csi-driver/node.go +++ b/pkg/gce-pd-csi-driver/node.go @@ -342,10 +342,12 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage // Part 4: Resize filesystem. // https://github.com/kubernetes/kubernetes/issues/94929 - resizer := resizefs.NewResizeFs(ns.Mounter) - _, err = ns.DeviceUtils.Resize(resizer, devicePath, stagingTargetPath) - if err != nil { - return nil, status.Error(codes.Internal, fmt.Sprintf("error when resizing volume %s from device '%s' at path '%s': %v", volumeID, devicePath, stagingTargetPath, err.Error())) + if !readonly { + resizer := resizefs.NewResizeFs(ns.Mounter) + _, err = ns.DeviceUtils.Resize(resizer, devicePath, stagingTargetPath) + if err != nil { + return nil, status.Error(codes.Internal, fmt.Sprintf("error when resizing volume %s from device '%s' at path '%s': %v", volumeID, devicePath, stagingTargetPath, err.Error())) + } } klog.V(4).Infof("NodeStageVolume succeeded on %v to %s", volumeID, stagingTargetPath) @@ -501,6 +503,15 @@ func (ns *GCENodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpa klog.V(4).Infof("NodeExpandVolume succeeded on %v to %s, capability is block so this is a no-op", volumeID, volumePath) return &csi.NodeExpandVolumeResponse{}, nil } + + readonly, err := getReadOnlyFromCapability(volumeCapability) + if err != nil { + return nil, status.Error(codes.Internal, fmt.Sprintf("failed to check if capability for volume %s is readonly: %v", volumeID, err)) + } + if readonly { + klog.V(4).Infof("NodeExpandVolume succeeded on %v to %s, capability access is readonly so this is a no-op", volumeID, volumePath) + return &csi.NodeExpandVolumeResponse{}, nil + } } // TODO(#328): Use requested size in resize if provided diff --git a/pkg/gce-pd-csi-driver/node_test.go b/pkg/gce-pd-csi-driver/node_test.go index 3125c0309..91a65f7b3 100644 --- a/pkg/gce-pd-csi-driver/node_test.go +++ b/pkg/gce-pd-csi-driver/node_test.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" "k8s.io/utils/exec" @@ -40,11 +41,11 @@ const defaultTargetPath = "/mnt/test" const defaultStagingPath = "/staging" func getTestGCEDriver(t *testing.T) *GCEDriver { - return getCustomTestGCEDriver(t, mountmanager.NewFakeSafeMounter(), deviceutils.NewFakeDeviceUtils(), metadataservice.NewFakeService()) + return getCustomTestGCEDriver(t, mountmanager.NewFakeSafeMounter(), deviceutils.NewFakeDeviceUtils(false), metadataservice.NewFakeService()) } func getTestGCEDriverWithCustomMounter(t *testing.T, mounter *mount.SafeFormatAndMount) *GCEDriver { - return getCustomTestGCEDriver(t, mounter, deviceutils.NewFakeDeviceUtils(), metadataservice.NewFakeService()) + return getCustomTestGCEDriver(t, mounter, deviceutils.NewFakeDeviceUtils(false), metadataservice.NewFakeService()) } func getCustomTestGCEDriver(t *testing.T, mounter *mount.SafeFormatAndMount, deviceUtils deviceutils.DeviceUtils, metaService metadataservice.MetadataService) *GCEDriver { @@ -60,7 +61,7 @@ func getCustomTestGCEDriver(t *testing.T, mounter *mount.SafeFormatAndMount, dev func getTestBlockingGCEDriver(t *testing.T, readyToExecute chan chan struct{}) *GCEDriver { gceDriver := GetGCEDriver() mounter := mountmanager.NewFakeSafeBlockingMounter(readyToExecute) - nodeServer := NewNodeServer(gceDriver, mounter, deviceutils.NewFakeDeviceUtils(), metadataservice.NewFakeService(), mountmanager.NewFakeStatter(mounter)) + nodeServer := NewNodeServer(gceDriver, mounter, deviceutils.NewFakeDeviceUtils(false), metadataservice.NewFakeService(), mountmanager.NewFakeStatter(mounter)) err := gceDriver.SetupGCEDriver(driver, "test-vendor", nil, nil, nil, nodeServer) if err != nil { t.Fatalf("Failed to setup GCE Driver: %v", err) @@ -381,26 +382,61 @@ func TestNodeStageVolume(t *testing.T) { stagingPath := filepath.Join(tempDir, defaultStagingPath) testCases := []struct { - name string - req *csi.NodeStageVolumeRequest - expErrCode codes.Code + name string + req *csi.NodeStageVolumeRequest + deviceSize int + blockExtSize int + readonlyBit string + expResize bool + expErrCode codes.Code }{ { - name: "Valid request", + name: "Valid request, no resize because block and filesystem sizes match", req: &csi.NodeStageVolumeRequest{ VolumeId: volumeID, StagingTargetPath: stagingPath, VolumeCapability: stdVolCap, }, + deviceSize: 1, + blockExtSize: 1, + readonlyBit: "0", + expResize: false, }, { - name: "Invalid request (Bad Access Mode)", + name: "Valid request, no resize bc readonly", req: &csi.NodeStageVolumeRequest{ VolumeId: volumeID, StagingTargetPath: stagingPath, - VolumeCapability: createVolumeCapability(csi.VolumeCapability_AccessMode_UNKNOWN), + VolumeCapability: stdVolCap, }, - expErrCode: codes.InvalidArgument, + deviceSize: 1, + blockExtSize: 1, + readonlyBit: "1", + expResize: false, + }, + { + name: "Valid request, resize bc size", + req: &csi.NodeStageVolumeRequest{ + VolumeId: volumeID, + StagingTargetPath: stagingPath, + VolumeCapability: stdVolCap, + }, + deviceSize: 5, + blockExtSize: 1, + readonlyBit: "0", + expResize: true, + }, + { + name: "Valid request, no resize bc readonly capability", + req: &csi.NodeStageVolumeRequest{ + VolumeId: volumeID, + StagingTargetPath: stagingPath, + VolumeCapability: createVolumeCapability(csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY), + }, + deviceSize: 5, + blockExtSize: 1, + readonlyBit: "0", + expResize: false, }, { name: "Invalid request (Bad Access Mode)", @@ -448,6 +484,7 @@ func TestNodeStageVolume(t *testing.T) { } for _, tc := range testCases { t.Logf("Test case: %s", tc.name) + resizeCalled := false actionList := []testingexec.FakeCommandAction{ makeFakeCmd( &testingexec.FakeCmd{ @@ -458,26 +495,40 @@ func TestNodeStageVolume(t *testing.T) { }, }, "blkid", + strings.Split("-p -s TYPE -s PTTYPE -o export /dev/disk/fake-path", " ")..., ), makeFakeCmd( &testingexec.FakeCmd{ CombinedOutputScript: []testingexec.FakeAction{ func() ([]byte, []byte, error) { - return []byte("1"), nil, nil + return []byte(""), nil, nil + }, + }, + }, + "fsck", + strings.Split("-a /dev/disk/fake-path", " ")..., + ), + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte(tc.readonlyBit), nil, nil }, }, }, "blockdev", + strings.Split("--getro /dev/disk/fake-path", " ")..., ), makeFakeCmd( &testingexec.FakeCmd{ CombinedOutputScript: []testingexec.FakeAction{ func() ([]byte, []byte, error) { - return []byte("1"), nil, nil + return []byte(fmt.Sprintf("%d", tc.deviceSize)), nil, nil }, }, }, "blockdev", + strings.Split("--getsize64 /dev/disk/fake-path", " ")..., ), makeFakeCmd( &testingexec.FakeCmd{ @@ -488,18 +539,48 @@ func TestNodeStageVolume(t *testing.T) { }, }, "blkid", + strings.Split("-p -s TYPE -s PTTYPE -o export /dev/disk/fake-path", " ")..., ), makeFakeCmd( &testingexec.FakeCmd{ CombinedOutputScript: []testingexec.FakeAction{ func() ([]byte, []byte, error) { - return []byte(fmt.Sprintf("block size: 1\nblock count: 1")), nil, nil + return []byte(fmt.Sprintf("block size: %d\nblock count: 1", tc.blockExtSize)), nil, nil }, }, }, "dumpe2fs", + strings.Split("-h /dev/disk/fake-path", " ")..., ), } + + if tc.expResize { + actionList = append(actionList, []testingexec.FakeCommandAction{ + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte(fmt.Sprintf("DEVNAME=/dev/sdb\nTYPE=ext4")), nil, nil + }, + }, + }, + "blkid", + strings.Split("-p -s TYPE -s PTTYPE -o export /dev/disk/fake-path", " ")..., + ), + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + resizeCalled = true + return []byte(fmt.Sprintf("DEVNAME=/dev/sdb\nTYPE=ext4")), nil, nil + }, + }, + }, + "resize2fs", + strings.Split("/dev/disk/fake-path", " ")..., + ), + }...) + } mounter := mountmanager.NewFakeSafeMounterWithCustomExec(&testingexec.FakeExec{CommandScript: actionList}) gceDriver := getTestGCEDriverWithCustomMounter(t, mounter) ns := gceDriver.ns @@ -517,6 +598,12 @@ func TestNodeStageVolume(t *testing.T) { if tc.expErrCode != codes.OK { t.Fatalf("Expected error: %v, got no error", tc.expErrCode) } + if tc.expResize == true && resizeCalled == false { + t.Fatalf("Test did not call resize, but it was expected.") + } + if tc.expResize == false && resizeCalled == true { + t.Fatalf("Test called resize, but it was not expected.") + } } } diff --git a/test/sanity/sanity_test.go b/test/sanity/sanity_test.go index 16360da67..358d401a8 100644 --- a/test/sanity/sanity_test.go +++ b/test/sanity/sanity_test.go @@ -62,7 +62,7 @@ func TestSanity(t *testing.T) { } mounter := mountmanager.NewFakeSafeMounter() - deviceUtils := deviceutils.NewFakeDeviceUtils() + deviceUtils := deviceutils.NewFakeDeviceUtils(true) //Initialize GCE Driver identityServer := driver.NewIdentityServer(gceDriver)