From 50dc6d1a84409e0412ab585273012c4009164ee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Bedn=C3=A1=C5=99?= Date: Mon, 9 Jan 2023 10:39:29 +0100 Subject: [PATCH] filesystem is not resized when restoring from snapshot/cloning to larger size than origin (#100) * remove unnecessary code VolumeMountUtils is already instantiated in init() of ibmcsidriver module so there is no need for this line of code. * allow creating driver with fake commands In order to allow testing of code that includes command sequence we need to change the driver so it can accept a list of fake commands. * resize fs in NodeStageVolume This is the actual fix for resizing filesystem when volume is restored from a snapshot to larger size * resize with mounter during node expand * cleanup resize code * update to latest ibm-csi-common --- go.mod | 4 +- go.sum | 4 +- pkg/ibmcsidriver/ibm_csi_driver_test.go | 12 +- pkg/ibmcsidriver/node.go | 25 +--- pkg/ibmcsidriver/node_test.go | 117 +++++++++++++++--- .../pkg/mountmanager/fake-safe-mounter.go | 2 +- .../pkg/mountmanager/mount_linux.go | 2 +- .../pkg/mountmanager/mount_unsupported.go | 4 +- vendor/modules.txt | 2 +- 9 files changed, 124 insertions(+), 48 deletions(-) diff --git a/go.mod b/go.mod index cc18e525..206f1d60 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/kubernetes-sigs/ibm-vpc-block-csi-driver go 1.18 require ( - github.com/IBM/ibm-csi-common v1.1.2 + github.com/IBM/ibm-csi-common v1.1.3 github.com/IBM/ibmcloud-volume-interface v1.1.1 github.com/IBM/ibmcloud-volume-vpc v1.1.2 github.com/container-storage-interface/spec v1.6.0 @@ -19,6 +19,7 @@ require ( google.golang.org/protobuf v1.28.1 k8s.io/kubernetes v1.25.4 k8s.io/mount-utils v0.25.4 + k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed ) require ( @@ -102,7 +103,6 @@ require ( k8s.io/component-base v0.25.4 // indirect k8s.io/klog/v2 v2.70.1 // indirect k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1 // indirect - k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed // indirect sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect sigs.k8s.io/yaml v1.2.0 // indirect diff --git a/go.sum b/go.sum index b797df3b..65f617b3 100644 --- a/go.sum +++ b/go.sum @@ -39,8 +39,8 @@ github.com/IBM-Cloud/ibm-cloud-cli-sdk v0.6.7 h1:eHgfQl6IeSmzWUyiSi13CvoFYsovoyq github.com/IBM-Cloud/ibm-cloud-cli-sdk v0.6.7/go.mod h1:RiUvKuHKTBmBApDMUQzBL14pQUGKcx/IioKQPIcRQjs= github.com/IBM/go-sdk-core/v5 v5.9.1 h1:06pXbD9Rgmqqe2HA5YAeQbB4eYRRFgIoOT+Kh3cp1zo= github.com/IBM/go-sdk-core/v5 v5.9.1/go.mod h1:axE2JrRq79gIJTjKPBwV6gWHswvVptBjbcvvCPIxARM= -github.com/IBM/ibm-csi-common v1.1.2 h1:g74+5uIaKZnZ3aIJyBZkpA2///wsG0fyDCJB8AQJW0Q= -github.com/IBM/ibm-csi-common v1.1.2/go.mod h1:5Ld7WgeLVO8uLoQtqGZhJgHQyCqwUEv1fBq1YpwIqQ0= +github.com/IBM/ibm-csi-common v1.1.3 h1:03vsgi46mFWcp66D2+DR6bkXyBKK/9TbYhm8Qyvrx7s= +github.com/IBM/ibm-csi-common v1.1.3/go.mod h1:5Ld7WgeLVO8uLoQtqGZhJgHQyCqwUEv1fBq1YpwIqQ0= github.com/IBM/ibmcloud-volume-interface v1.1.1 h1:RlwEj8bq+aASdb90Lng4rINMYR96/G6yUvyk/ATmptA= github.com/IBM/ibmcloud-volume-interface v1.1.1/go.mod h1:coaQ/FD5NRgFfaAnjkoxv+e6MNSl2UAeQwC38szQ5G8= github.com/IBM/ibmcloud-volume-vpc v1.1.2 h1:LbgRC/CQRptt6+lfg6k/LL2W2GyDNrxirzJXZtDY9ik= diff --git a/pkg/ibmcsidriver/ibm_csi_driver_test.go b/pkg/ibmcsidriver/ibm_csi_driver_test.go index 58f2e669..7ae572cf 100644 --- a/pkg/ibmcsidriver/ibm_csi_driver_test.go +++ b/pkg/ibmcsidriver/ibm_csi_driver_test.go @@ -18,6 +18,7 @@ limitations under the License. package ibmcsidriver import ( + testingexec "k8s.io/utils/exec/testing" "testing" cloudProvider "github.com/IBM/ibm-csi-common/pkg/ibmcloudprovider" @@ -27,7 +28,7 @@ import ( "github.com/stretchr/testify/assert" ) -func initIBMCSIDriver(t *testing.T) *IBMCSIDriver { +func initIBMCSIDriver(t *testing.T, fakeActions ...testingexec.FakeCommandAction) *IBMCSIDriver { vendorVersion := "test-vendor-version-1.1.2" driver := "mydriver" // Creating test logger @@ -36,7 +37,14 @@ func initIBMCSIDriver(t *testing.T) *IBMCSIDriver { icDriver := GetIBMCSIDriver() // Create fake provider and mounter provider, _ := cloudProvider.NewFakeIBMCloudStorageProvider("", logger) - mounter := mountManager.NewFakeNodeMounter() + + var mounter mountManager.Mounter + if len(fakeActions) != 0 { + mounter = mountManager.NewFakeNodeMounterWithCustomActions(fakeActions) + } else { + mounter = mountManager.NewFakeNodeMounter() + } + statsUtil := &MockStatUtils{} fakeNodeData := nodeMetadata.FakeNodeMetadata{} diff --git a/pkg/ibmcsidriver/node.go b/pkg/ibmcsidriver/node.go index 47aaea45..f823d4d9 100644 --- a/pkg/ibmcsidriver/node.go +++ b/pkg/ibmcsidriver/node.go @@ -60,11 +60,6 @@ type StatsUtils interface { IsDevicePathNotExist(devicePath string) bool } -// MountUtils ... -type MountUtils interface { - Resize(mounter mountmanager.Mounter, devicePath string, deviceMountPath string) (bool, error) -} - // VolumeStatUtils ... type VolumeStatUtils struct { } @@ -105,11 +100,6 @@ const ( ) var _ csi.NodeServer = &CSINodeServer{} -var mountmgr MountUtils - -func init() { - mountmgr = &VolumeMountUtils{} -} // NodePublishVolume ... func (csiNS *CSINodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { @@ -307,6 +297,10 @@ func (csiNS *CSINodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeSt return nil, commonError.GetCSIError(ctxLogger, commonError.FormatAndMountFailed, requestID, err, source, stagingTargetPath) } + if _, err := csiNS.Mounter.Resize(devicePath, stagingTargetPath); err != nil { + return nil, commonError.GetCSIError(ctxLogger, commonError.FileSystemResizeFailed, requestID, err) + } + nodeStageVolumeResponse := &csi.NodeStageVolumeResponse{} return nodeStageVolumeResponse, err } @@ -497,7 +491,7 @@ func (csiNS *CSINodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeE return nil, commonError.GetCSIError(ctxLogger, commonError.EmptyDevicePath, requestID, err) } - if _, err := mountmgr.Resize(csiNS.Mounter, devicePath, volumePath); err != nil { + if _, err := csiNS.Mounter.Resize(devicePath, volumePath); err != nil { return nil, commonError.GetCSIError(ctxLogger, commonError.FileSystemResizeFailed, requestID, err) } return &csi.NodeExpandVolumeResponse{CapacityBytes: req.CapacityRange.RequiredBytes}, nil @@ -541,12 +535,3 @@ func (su *VolumeStatUtils) IsDevicePathNotExist(devicePath string) bool { } return false } - -// Resize expands the fs -func (volMountUtils *VolumeMountUtils) Resize(mounter mountmanager.Mounter, devicePath string, deviceMountPath string) (bool, error) { - r := mount.NewResizeFs(mounter.GetSafeFormatAndMount().Exec) - if _, err := r.Resize(devicePath, deviceMountPath); err != nil { - return false, err - } - return true, nil -} diff --git a/pkg/ibmcsidriver/node_test.go b/pkg/ibmcsidriver/node_test.go index 4acda06d..e62ee641 100644 --- a/pkg/ibmcsidriver/node_test.go +++ b/pkg/ibmcsidriver/node_test.go @@ -20,13 +20,14 @@ package ibmcsidriver import ( "errors" "fmt" + "k8s.io/utils/exec" + testingexec "k8s.io/utils/exec/testing" "os" "reflect" "runtime" "strings" "testing" - "github.com/IBM/ibm-csi-common/pkg/mountmanager" "github.com/IBM/ibm-csi-common/pkg/utils" csi "github.com/container-storage-interface/spec/lib/go/csi" "github.com/stretchr/testify/assert" @@ -47,19 +48,6 @@ const notBlockDevice = "/for/notblocktest" type MockStatUtils struct { } -type MockMountUtils struct { -} - -// Resize expands the fs -func (mu *MockMountUtils) Resize(mounter mountmanager.Mounter, devicePath string, deviceMountPath string) (bool, error) { - if strings.Contains(deviceMountPath, "fake-") { - return false, fmt.Errorf("failed to resize fs") - } else if strings.Contains(deviceMountPath, "valid-") { - return true, nil - } - return false, fmt.Errorf("failed to resize fs") -} - func (su *MockStatUtils) FSInfo(path string) (int64, int64, int64, int64, int64, int64, error) { return 1, 1, 1, 1, 1, 1, nil } @@ -347,7 +335,60 @@ func TestNodeStageVolume(t *testing.T) { }, } - icDriver := initIBMCSIDriver(t) + actionList := []testingexec.FakeCommandAction{ + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte("DEVNAME=/dev/sdb\nTYPE=ext4"), nil, nil + }, + }, + }, + "blkid", + ), + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte("1"), nil, nil + }, + }, + }, + "mkfs.ext2", + ), + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte("1"), nil, nil + }, + }, + }, + "blockdev", + ), + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte("DEVNAME=/dev/sdb\nTYPE=ext4"), nil, nil + }, + }, + }, + "blkid", + ), + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte("block size: 1\nblock count: 1"), nil, nil + }, + }, + }, + "dumpe2fs", + ), + } + + icDriver := initIBMCSIDriver(t, actionList...) for _, tc := range testCases { t.Logf("Test case: %s", tc.name) _, err := icDriver.ns.NodeStageVolume(context.Background(), tc.req) @@ -649,10 +690,43 @@ func TestNodeExpandVolume(t *testing.T) { expErrCode: codes.NotFound, }, } - icDriver := initIBMCSIDriver(t) + + actionList := []testingexec.FakeCommandAction{ + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte("1"), nil, nil + }, + }, + }, + "blockdev", + ), + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte("DEVNAME=/dev/sdb\nTYPE=ext4"), nil, nil + }, + }, + }, + "blkid", + ), + makeFakeCmd( + &testingexec.FakeCmd{ + CombinedOutputScript: []testingexec.FakeAction{ + func() ([]byte, []byte, error) { + return []byte("block size: 1\nblock count: 1"), nil, nil + }, + }, + }, + "dumpe2fs", + ), + } + + icDriver := initIBMCSIDriver(t, actionList...) _ = os.MkdirAll("valid-vol-path", os.FileMode(0755)) _ = icDriver.ns.Mounter.Mount("valid-devicePath", "valid-vol-path", "ext4", []string{}) - mountmgr = &MockMountUtils{} for _, tc := range testCases { t.Logf("Test case: %s", tc.name) _, err := icDriver.ns.NodeExpandVolume(context.Background(), tc.req) @@ -762,3 +836,12 @@ func TestDeviceInfo(t *testing.T) { }*/ } } + +func makeFakeCmd(fakeCmd *testingexec.FakeCmd, cmd string, args ...string) testingexec.FakeCommandAction { + c := cmd + a := args + return func(cmd string, args ...string) exec.Cmd { + command := testingexec.InitFakeCmd(fakeCmd, c, a...) + return command + } +} diff --git a/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/fake-safe-mounter.go b/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/fake-safe-mounter.go index bd8fd92f..cb8af187 100644 --- a/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/fake-safe-mounter.go +++ b/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/fake-safe-mounter.go @@ -76,7 +76,7 @@ func (f *FakeNodeMounter) PathExists(pathname string) (bool, error) { return false, nil } -// NewSafeFormatAndMount ... +// GetSafeFormatAndMount returns the existing SafeFormatAndMount object of NodeMounter. func (f *FakeNodeMounter) GetSafeFormatAndMount() *mount.SafeFormatAndMount { return f.SafeFormatAndMount } diff --git a/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/mount_linux.go b/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/mount_linux.go index d344618d..2fa01fe8 100644 --- a/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/mount_linux.go +++ b/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/mount_linux.go @@ -56,7 +56,7 @@ func (m *NodeMounter) PathExists(path string) (bool, error) { return mount.PathExists(path) } -// NewSafeFormatAndMount returns the new object of SafeFormatAndMount. +// GetSafeFormatAndMount returns the existing SafeFormatAndMount object of NodeMounter. func (m *NodeMounter) GetSafeFormatAndMount() *mount.SafeFormatAndMount { return m.SafeFormatAndMount } diff --git a/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/mount_unsupported.go b/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/mount_unsupported.go index b4d5dbfe..87c35c54 100644 --- a/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/mount_unsupported.go +++ b/vendor/github.com/IBM/ibm-csi-common/pkg/mountmanager/mount_unsupported.go @@ -43,8 +43,8 @@ func (m *NodeMounter) PathExists(pathname string) (bool, error) { return true, errors.New("not implemented") } -// NewSafeFormatAndMount ... -func (m *NodeMounter) NewSafeFormatAndMount() *mount.SafeFormatAndMount { +// GetSafeFormatAndMount returns the existing SafeFormatAndMount object of NodeMounter. +func (m *NodeMounter) GetSafeFormatAndMount() *mount.SafeFormatAndMount { return nil } diff --git a/vendor/modules.txt b/vendor/modules.txt index 01dc05f2..75d4f221 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -8,7 +8,7 @@ github.com/IBM-Cloud/ibm-cloud-cli-sdk/common/rest # github.com/IBM/go-sdk-core/v5 v5.9.1 ## explicit; go 1.14 github.com/IBM/go-sdk-core/v5/core -# github.com/IBM/ibm-csi-common v1.1.2 +# github.com/IBM/ibm-csi-common v1.1.3 ## explicit; go 1.18 github.com/IBM/ibm-csi-common/pkg/ibmcloudprovider github.com/IBM/ibm-csi-common/pkg/messages