From b5431d18f02f5a3b8f7c256eff80837385af9e39 Mon Sep 17 00:00:00 2001 From: Andrey Klimentyev Date: Wed, 19 Feb 2020 11:44:47 +0300 Subject: [PATCH 1/6] NodeGetVolumeStats() implementation Signed-off-by: Andrey Klimentyev --- pkg/csi/cinder/driver.go | 1 + pkg/csi/cinder/nodeserver.go | 19 +++++++++++++++++-- pkg/csi/cinder/nodeserver_test.go | 19 +++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/pkg/csi/cinder/driver.go b/pkg/csi/cinder/driver.go index eff7b2b0b7..939999bae4 100644 --- a/pkg/csi/cinder/driver.go +++ b/pkg/csi/cinder/driver.go @@ -79,6 +79,7 @@ func NewDriver(nodeID, endpoint, cluster string) *CinderDriver { []csi.NodeServiceCapability_RPC_Type{ csi.NodeServiceCapability_RPC_STAGE_UNSTAGE_VOLUME, csi.NodeServiceCapability_RPC_EXPAND_VOLUME, + csi.NodeServiceCapability_RPC_GET_VOLUME_STATS, }) return d diff --git a/pkg/csi/cinder/nodeserver.go b/pkg/csi/cinder/nodeserver.go index 12e5337a23..347aa46d9b 100644 --- a/pkg/csi/cinder/nodeserver.go +++ b/pkg/csi/cinder/nodeserver.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc/status" "k8s.io/klog" "k8s.io/kubernetes/pkg/util/resizefs" + "k8s.io/kubernetes/pkg/volume/util/fs" utilpath "k8s.io/utils/path" "k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack" @@ -478,8 +479,22 @@ func (ns *nodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetC }, nil } -func (ns *nodeServer) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) { - return nil, status.Error(codes.Unimplemented, fmt.Sprintf("NodeGetVolumeStats is not yet implemented")) +func (ns *nodeServer) NodeGetVolumeStats(_ context.Context, req *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) { + klog.V(4).Infof("NodeExpandVolume: called with args %+v", *req) + + volumePath := req.GetVolumePath() + + available, capacity, usage, inodes, inodesFree, inodesUsed, err := fs.FsInfo(volumePath) + if err != nil { + return nil, status.Errorf(codes.Internal, + "Unable to statfs target %s, err: %s", volumePath, err) + } + return &csi.NodeGetVolumeStatsResponse{ + Usage: []*csi.VolumeUsage{ + {Total: capacity, Available: available, Used: usage, Unit: 1}, + {Total: inodes, Available: inodesFree, Used: inodesUsed, Unit: 2}, + }, + }, nil } func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) { diff --git a/pkg/csi/cinder/nodeserver_test.go b/pkg/csi/cinder/nodeserver_test.go index f86de62fa1..8991ba4107 100644 --- a/pkg/csi/cinder/nodeserver_test.go +++ b/pkg/csi/cinder/nodeserver_test.go @@ -20,6 +20,9 @@ import ( "fmt" "testing" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "github.com/container-storage-interface/spec/lib/go/csi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -380,3 +383,19 @@ func TestNodeExpandVolume(t *testing.T) { assert.Equal(expectedRes, actualRes) } + +func TestNodeGetVolumeStats(t *testing.T) { + + // Init assert + assert := assert.New(t) + + // Fake request + fakeReq := &csi.NodeGetVolumeStatsRequest{ + VolumeId: FakeVolName, + VolumePath: FakeDevicePath, + } + + // Invoke NodeGetVolumeStats + _, err := fakeNs.NodeGetVolumeStats(FakeCtx, fakeReq) + assert.Equal(status.Error(codes.Internal, "Unable to statfs target /dev/xxx, err: no such file or directory"), err) +} From 58fcbf58a7b4eae11aaf81168b10051fc5178c69 Mon Sep 17 00:00:00 2001 From: Andrey Klimentyev Date: Tue, 25 Feb 2020 11:39:53 +0300 Subject: [PATCH 2/6] Sanity checks and removal of magic numbers Signed-off-by: Andrey Klimentyev --- pkg/csi/cinder/nodeserver.go | 37 ++++++++++++++++++++++++++++--- pkg/csi/cinder/nodeserver_test.go | 2 +- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/pkg/csi/cinder/nodeserver.go b/pkg/csi/cinder/nodeserver.go index 347aa46d9b..711eae8949 100644 --- a/pkg/csi/cinder/nodeserver.go +++ b/pkg/csi/cinder/nodeserver.go @@ -480,9 +480,21 @@ func (ns *nodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetC } func (ns *nodeServer) NodeGetVolumeStats(_ context.Context, req *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) { - klog.V(4).Infof("NodeExpandVolume: called with args %+v", *req) + klog.V(4).Infof("NodeGetVolumeStats: called with args %+v", *req) + + volumeID := req.GetVolumeId() + if len(volumeID) == 0 { + return nil, status.Error(codes.InvalidArgument, "Volume Id not provided") + } volumePath := req.GetVolumePath() + if len(volumePath) == 0 { + return nil, status.Error(codes.InvalidArgument, "Volume path not provided") + } + + if err := verifyTargetDir(volumePath); err != nil { + return nil, err + } available, capacity, usage, inodes, inodesFree, inodesUsed, err := fs.FsInfo(volumePath) if err != nil { @@ -491,8 +503,8 @@ func (ns *nodeServer) NodeGetVolumeStats(_ context.Context, req *csi.NodeGetVolu } return &csi.NodeGetVolumeStatsResponse{ Usage: []*csi.VolumeUsage{ - {Total: capacity, Available: available, Used: usage, Unit: 1}, - {Total: inodes, Available: inodesFree, Used: inodesUsed, Unit: 2}, + {Total: capacity, Available: available, Used: usage, Unit: csi.VolumeUsage_BYTES}, + {Total: inodes, Available: inodesFree, Used: inodesUsed, Unit: csi.VolumeUsage_INODES}, }, }, nil } @@ -601,3 +613,22 @@ func getNodeID(mount mount.IMount, iMetadata openstack.IMetadata, order string) } return nodeID, nil } + +func verifyTargetDir(target string) error { + if target == "" { + return status.Error(codes.InvalidArgument, + "target path required") + } + + _, err := os.Stat(target) + if err != nil { + if os.IsNotExist(err) { + return status.Errorf(codes.NotFound, + "target: %s not found", target) + } + return status.Errorf(codes.Internal, + "failed to stat target, err: %s", err.Error()) + } + + return nil +} diff --git a/pkg/csi/cinder/nodeserver_test.go b/pkg/csi/cinder/nodeserver_test.go index 8991ba4107..53cea0fb5e 100644 --- a/pkg/csi/cinder/nodeserver_test.go +++ b/pkg/csi/cinder/nodeserver_test.go @@ -397,5 +397,5 @@ func TestNodeGetVolumeStats(t *testing.T) { // Invoke NodeGetVolumeStats _, err := fakeNs.NodeGetVolumeStats(FakeCtx, fakeReq) - assert.Equal(status.Error(codes.Internal, "Unable to statfs target /dev/xxx, err: no such file or directory"), err) + assert.Equal(status.Error(codes.NotFound, "target: /dev/xxx not found"), err) } From 50fa6e30da8e76c31ae91989674c8ad90e8d4ed6 Mon Sep 17 00:00:00 2001 From: Andrey Klimentyev Date: Tue, 17 Mar 2020 12:19:32 +0300 Subject: [PATCH 3/6] Rewrite Signed-off-by: Andrey Klimentyev --- pkg/csi/cinder/fake.go | 11 +++++ pkg/csi/cinder/nodeserver.go | 58 +++++++++++++------------ pkg/csi/cinder/nodeserver_test.go | 47 +++++++++++++++++--- pkg/csi/cinder/sanity/fakemount.go | 17 ++++++++ pkg/util/mount/mount.go | 70 ++++++++++++++++++++++++++++++ pkg/util/mount/mount_mock.go | 43 ++++++++++++++++++ 6 files changed, 213 insertions(+), 33 deletions(-) diff --git a/pkg/csi/cinder/fake.go b/pkg/csi/cinder/fake.go index dd58806a82..82598fc4d5 100644 --- a/pkg/csi/cinder/fake.go +++ b/pkg/csi/cinder/fake.go @@ -20,6 +20,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/snapshots" "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes" "golang.org/x/net/context" + "k8s.io/cloud-provider-openstack/pkg/csi/cinder/mount" ) var FakeCluster = "cluster" @@ -97,3 +98,13 @@ var FakeVolListEmpty = []volumes.Volume{} var FakeInstanceID = "321a8b81-3660-43e5-bab8-6470b65ee4e8" const FakeMaxVolume int64 = 256 + +var FakeFsStats = mount.FsStats{ + AvailableBytes: 2100, + TotalBytes: 2121, + UsedBytes: 21, + AvailableInodes: 150, + TotalInodes: 200, + UsedInodes: 50, +} +var FakeBlockDeviceSize = 536870912 diff --git a/pkg/csi/cinder/nodeserver.go b/pkg/csi/cinder/nodeserver.go index 711eae8949..28ea30111d 100644 --- a/pkg/csi/cinder/nodeserver.go +++ b/pkg/csi/cinder/nodeserver.go @@ -30,7 +30,6 @@ import ( "google.golang.org/grpc/status" "k8s.io/klog" "k8s.io/kubernetes/pkg/util/resizefs" - "k8s.io/kubernetes/pkg/volume/util/fs" utilpath "k8s.io/utils/path" "k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack" @@ -492,19 +491,43 @@ func (ns *nodeServer) NodeGetVolumeStats(_ context.Context, req *csi.NodeGetVolu return nil, status.Error(codes.InvalidArgument, "Volume path not provided") } - if err := verifyTargetDir(volumePath); err != nil { - return nil, err + exists, err := ns.Mount.PathExists(volumePath) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to stats volumePath: %s", err) + } else if !exists { + return nil, status.Errorf(codes.NotFound, "target: %s not found", volumePath) + } + + isBlock, err := ns.Mount.IsBlockDevice(volumePath) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to determine if volume is a block device: %s", err) + } + + if isBlock { + size, err := ns.Mount.GetBlockDeviceSize(volumePath) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to get block device size: %s", err) + } + + return &csi.NodeGetVolumeStatsResponse{ + Usage: []*csi.VolumeUsage{ + { + Total: size, + Unit: csi.VolumeUsage_BYTES, + }, + }, + }, nil } - available, capacity, usage, inodes, inodesFree, inodesUsed, err := fs.FsInfo(volumePath) + fsStats, err := ns.Mount.GetFileSystemStats(volumePath) if err != nil { - return nil, status.Errorf(codes.Internal, - "Unable to statfs target %s, err: %s", volumePath, err) + return nil, status.Errorf(codes.Internal, "failed to get filesystem size: %s", err) } + return &csi.NodeGetVolumeStatsResponse{ Usage: []*csi.VolumeUsage{ - {Total: capacity, Available: available, Used: usage, Unit: csi.VolumeUsage_BYTES}, - {Total: inodes, Available: inodesFree, Used: inodesUsed, Unit: csi.VolumeUsage_INODES}, + {Total: fsStats.TotalBytes, Available: fsStats.AvailableBytes, Used: fsStats.UsedBytes, Unit: csi.VolumeUsage_BYTES}, + {Total: fsStats.TotalInodes, Available: fsStats.AvailableInodes, Used: fsStats.UsedInodes, Unit: csi.VolumeUsage_INODES}, }, }, nil } @@ -613,22 +636,3 @@ func getNodeID(mount mount.IMount, iMetadata openstack.IMetadata, order string) } return nodeID, nil } - -func verifyTargetDir(target string) error { - if target == "" { - return status.Error(codes.InvalidArgument, - "target path required") - } - - _, err := os.Stat(target) - if err != nil { - if os.IsNotExist(err) { - return status.Errorf(codes.NotFound, - "target: %s not found", target) - } - return status.Errorf(codes.Internal, - "failed to stat target, err: %s", err.Error()) - } - - return nil -} diff --git a/pkg/csi/cinder/nodeserver_test.go b/pkg/csi/cinder/nodeserver_test.go index 53cea0fb5e..dfc958cf34 100644 --- a/pkg/csi/cinder/nodeserver_test.go +++ b/pkg/csi/cinder/nodeserver_test.go @@ -20,9 +20,6 @@ import ( "fmt" "testing" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - "github.com/container-storage-interface/spec/lib/go/csi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -395,7 +392,45 @@ func TestNodeGetVolumeStats(t *testing.T) { VolumePath: FakeDevicePath, } - // Invoke NodeGetVolumeStats - _, err := fakeNs.NodeGetVolumeStats(FakeCtx, fakeReq) - assert.Equal(status.Error(codes.NotFound, "target: /dev/xxx not found"), err) + mmock.On("PathExists", FakeDevicePath).Return(true, nil) + // Invoke NodeGetVolumeStats with a block device + mmock.On("IsBlockDevice", FakeDevicePath).Return(true, nil) + mmock.On("GetBlockDeviceSize", FakeDevicePath).Return(int64(FakeBlockDeviceSize), nil) + expectedBlockRes := &csi.NodeGetVolumeStatsResponse{ + Usage: []*csi.VolumeUsage{ + {Total: int64(FakeBlockDeviceSize), Unit: csi.VolumeUsage_BYTES}, + }, + } + + blockRes, err := fakeNs.NodeGetVolumeStats(FakeCtx, fakeReq) + + assert.NoError(err) + assert.Equal(expectedBlockRes, blockRes) + +} + +func TestNodeGetVolumeStatsFs(t *testing.T) { + // Init assert + assert := assert.New(t) + + // Fake request + fakeReq := &csi.NodeGetVolumeStatsRequest{ + VolumeId: FakeVolName, + VolumePath: FakeDevicePath, + } + + mmock.On("PathExists", FakeDevicePath).Return(true, nil) + mmock.On("IsBlockDevice", FakeDevicePath).Return(false, nil) + mmock.On("GetFileSystemStats", FakeDevicePath).Return(FakeFsStats, nil) + expectedFsRes := &csi.NodeGetVolumeStatsResponse{ + Usage: []*csi.VolumeUsage{ + {Total: FakeFsStats.TotalBytes, Available: FakeFsStats.AvailableBytes, Used: FakeFsStats.UsedBytes, Unit: csi.VolumeUsage_BYTES}, + {Total: FakeFsStats.TotalInodes, Available: FakeFsStats.AvailableInodes, Used: FakeFsStats.UsedInodes, Unit: csi.VolumeUsage_INODES}, + }, + } + + fsRes, err := fakeNs.NodeGetVolumeStats(FakeCtx, fakeReq) + + assert.NoError(err) + assert.Equal(expectedFsRes, fsRes) } diff --git a/pkg/csi/cinder/sanity/fakemount.go b/pkg/csi/cinder/sanity/fakemount.go index 3c822d5635..e7a481dd4c 100644 --- a/pkg/csi/cinder/sanity/fakemount.go +++ b/pkg/csi/cinder/sanity/fakemount.go @@ -2,6 +2,7 @@ package sanity import ( "k8s.io/cloud-provider-openstack/pkg/csi/cinder" + mount2 "k8s.io/cloud-provider-openstack/pkg/csi/cinder/mount" "k8s.io/utils/mount" ) @@ -46,3 +47,19 @@ func (m *fakemount) MakeDir(pathname string) error { func (m *fakemount) MakeFile(pathname string) error { return nil } + +func (m *fakemount) PathExists(devicePath string) (bool, error) { + return false, nil +} + +func (m *fakemount) GetBlockDeviceSize(volumePath string) (int64, error) { + return 0, nil +} + +func (m *fakemount) GetFileSystemStats(volumePath string) (mount2.FsStats, error) { + return mount2.FsStats{}, nil +} + +func (m *fakemount) IsBlockDevice(devicePath string) (bool, error) { + return true, nil +} diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index bfee324c84..70d5c53d0c 100644 --- a/pkg/util/mount/mount.go +++ b/pkg/util/mount/mount.go @@ -21,9 +21,11 @@ import ( "io/ioutil" "os" "path" + "strconv" "strings" "time" + "golang.org/x/sys/unix" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog" "k8s.io/utils/exec" @@ -49,6 +51,74 @@ type IMount interface { GetInstanceID() (string, error) MakeFile(pathname string) error MakeDir(pathname string) error + PathExists(devicePath string) (bool, error) + GetBlockDeviceSize(volumePath string) (int64, error) + GetFileSystemStats(volumePath string) (FsStats, error) + IsBlockDevice(devicePath string) (bool, error) +} + +type FsStats struct { + AvailableBytes int64 + TotalBytes int64 + UsedBytes int64 + AvailableInodes int64 + TotalInodes int64 + UsedInodes int64 +} + +func (m *Mount) PathExists(volumePath string) (bool, error) { + _, err := os.Stat(volumePath) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, fmt.Errorf("failed to stat target, err: %s", err) + } + + return true, nil +} + +func (m *Mount) IsBlockDevice(devicePath string) (bool, error) { + var stat unix.Stat_t + err := unix.Stat(devicePath, &stat) + if err != nil { + return false, fmt.Errorf("failed to stat() %q: %s", devicePath, err) + } + + return (stat.Mode & unix.S_IFMT) == unix.S_IFBLK, nil +} + +func (m *Mount) GetBlockDeviceSize(volumePath string) (int64, error) { + // See http://man7.org/linux/man-pages/man8/blockdev.8.html for details + output, err := m.GetBaseMounter().Exec.Command("blockdev", "--getsize64", volumePath).CombinedOutput() + if err != nil { + return 0, fmt.Errorf("error when getting size of block volume at path %s: output: %s, err: %v", volumePath, string(output), err) + } + strOut := strings.TrimSpace(string(output)) + gotSizeBytes, err := strconv.ParseInt(strOut, 10, 64) + if err != nil { + return 0, fmt.Errorf("failed to parse size %s into int", strOut) + } + + return gotSizeBytes, nil +} +func (m *Mount) GetFileSystemStats(volumePath string) (FsStats, error) { + var statfs unix.Statfs_t + // See http://man7.org/linux/man-pages/man2/statfs.2.html for details. + err := unix.Statfs(volumePath, &statfs) + if err != nil { + return FsStats{}, err + } + + return FsStats{ + AvailableBytes: int64(statfs.Bavail) * statfs.Bsize, + TotalBytes: int64(statfs.Blocks) * statfs.Bsize, + UsedBytes: (int64(statfs.Blocks) - int64(statfs.Bfree)) * statfs.Bsize, + + AvailableInodes: int64(statfs.Ffree), + TotalInodes: int64(statfs.Files), + UsedInodes: int64(statfs.Files) - int64(statfs.Ffree), + }, nil } type Mount struct { diff --git a/pkg/util/mount/mount_mock.go b/pkg/util/mount/mount_mock.go index 4a331c201e..3b8721a1f3 100644 --- a/pkg/util/mount/mount_mock.go +++ b/pkg/util/mount/mount_mock.go @@ -29,6 +29,44 @@ type MountMock struct { mock.Mock } +func (_m *MountMock) PathExists(volumePath string) (bool, error) { + ret := _m.Called(volumePath) + + return ret.Bool(0), ret.Error(1) +} + +func (_m *MountMock) GetBlockDeviceSize(volumePath string) (int64, error) { + ret := _m.Called(volumePath) + + return ret.Get(0).(int64), ret.Error(1) +} + +func (_m *MountMock) GetFileSystemStats(volumePath string) (FsStats, error) { + ret := _m.Called(volumePath) + + return ret.Get(0).(FsStats), ret.Error(1) +} + +func (_m *MountMock) IsBlockDevice(devicePath string) (bool, error) { + ret := _m.Called(devicePath) + + var r0 bool + if rf, ok := ret.Get(0).(func(string) bool); ok { + r0 = rf(devicePath) + } else { + r0 = ret.Bool(0) + } + + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(devicePath) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // NewFakeMounter returns fake mounter instance func NewFakeMounter() *mount.FakeMounter { return &mount.FakeMounter{ @@ -173,6 +211,11 @@ func (_m *MountMock) GetBaseMounter() *mount.SafeFormatAndMount { output: []byte("UUID=\"1b47881a-1563-4896-a178-eec887b759de\" \n TYPE=\"ext4\""), err: nil, }, + { + cmd: "blockdev", + output: []byte("536870912"), + err: nil, + }, } fakeexec := &exec.FakeExec{} From 1b09a6b7b1a4cc1691a31b23204652ef0ad88d4d Mon Sep 17 00:00:00 2001 From: Andrey Klimentyev Date: Tue, 17 Mar 2020 12:26:00 +0300 Subject: [PATCH 4/6] + --- pkg/csi/cinder/fake.go | 2 +- pkg/csi/cinder/nodeserver_test.go | 2 +- pkg/csi/cinder/sanity/fakemount.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/csi/cinder/fake.go b/pkg/csi/cinder/fake.go index 82598fc4d5..760cd1088a 100644 --- a/pkg/csi/cinder/fake.go +++ b/pkg/csi/cinder/fake.go @@ -20,7 +20,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/snapshots" "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes" "golang.org/x/net/context" - "k8s.io/cloud-provider-openstack/pkg/csi/cinder/mount" + "k8s.io/cloud-provider-openstack/pkg/util/mount" ) var FakeCluster = "cluster" diff --git a/pkg/csi/cinder/nodeserver_test.go b/pkg/csi/cinder/nodeserver_test.go index dfc958cf34..51aa95a5b4 100644 --- a/pkg/csi/cinder/nodeserver_test.go +++ b/pkg/csi/cinder/nodeserver_test.go @@ -381,7 +381,7 @@ func TestNodeExpandVolume(t *testing.T) { } -func TestNodeGetVolumeStats(t *testing.T) { +func TestNodeGetVolumeStatsBlock(t *testing.T) { // Init assert assert := assert.New(t) diff --git a/pkg/csi/cinder/sanity/fakemount.go b/pkg/csi/cinder/sanity/fakemount.go index e7a481dd4c..a1682f2c74 100644 --- a/pkg/csi/cinder/sanity/fakemount.go +++ b/pkg/csi/cinder/sanity/fakemount.go @@ -2,7 +2,7 @@ package sanity import ( "k8s.io/cloud-provider-openstack/pkg/csi/cinder" - mount2 "k8s.io/cloud-provider-openstack/pkg/csi/cinder/mount" + mount2 "k8s.io/cloud-provider-openstack/pkg/util/mount" "k8s.io/utils/mount" ) From 63f98cc7114c5d272a0cba1e678f7ede3a499b4c Mon Sep 17 00:00:00 2001 From: Andrey Klimentyev Date: Tue, 17 Mar 2020 13:23:21 +0300 Subject: [PATCH 5/6] + --- pkg/csi/cinder/nodeserver_test.go | 2 ++ pkg/util/mount/mount_mock.go | 5 ----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pkg/csi/cinder/nodeserver_test.go b/pkg/csi/cinder/nodeserver_test.go index 51aa95a5b4..fe426fb2b0 100644 --- a/pkg/csi/cinder/nodeserver_test.go +++ b/pkg/csi/cinder/nodeserver_test.go @@ -385,6 +385,7 @@ func TestNodeGetVolumeStatsBlock(t *testing.T) { // Init assert assert := assert.New(t) + mmock.ExpectedCalls = nil // Fake request fakeReq := &csi.NodeGetVolumeStatsRequest{ @@ -412,6 +413,7 @@ func TestNodeGetVolumeStatsBlock(t *testing.T) { func TestNodeGetVolumeStatsFs(t *testing.T) { // Init assert assert := assert.New(t) + mmock.ExpectedCalls = nil // Fake request fakeReq := &csi.NodeGetVolumeStatsRequest{ diff --git a/pkg/util/mount/mount_mock.go b/pkg/util/mount/mount_mock.go index 3b8721a1f3..3cab435460 100644 --- a/pkg/util/mount/mount_mock.go +++ b/pkg/util/mount/mount_mock.go @@ -211,11 +211,6 @@ func (_m *MountMock) GetBaseMounter() *mount.SafeFormatAndMount { output: []byte("UUID=\"1b47881a-1563-4896-a178-eec887b759de\" \n TYPE=\"ext4\""), err: nil, }, - { - cmd: "blockdev", - output: []byte("536870912"), - err: nil, - }, } fakeexec := &exec.FakeExec{} From 07dee2a95ca6ccc2205939b766d0e4028b6ca8ce Mon Sep 17 00:00:00 2001 From: Andrey Klimentyev Date: Thu, 26 Mar 2020 23:34:40 +0300 Subject: [PATCH 6/6] Latest fixes Signed-off-by: Andrey Klimentyev --- pkg/csi/cinder/fake.go | 11 ++++- pkg/csi/cinder/nodeserver.go | 27 ++++-------- pkg/csi/cinder/nodeserver_test.go | 10 ++--- pkg/csi/cinder/sanity/fakemount.go | 14 ++---- pkg/util/blockdevice/blockdevice_linux.go | 19 ++++++-- pkg/util/mount/mount.go | 53 ++++++++++------------- pkg/util/mount/mount_mock.go | 32 ++------------ 7 files changed, 68 insertions(+), 98 deletions(-) diff --git a/pkg/csi/cinder/fake.go b/pkg/csi/cinder/fake.go index 760cd1088a..793a243898 100644 --- a/pkg/csi/cinder/fake.go +++ b/pkg/csi/cinder/fake.go @@ -99,7 +99,9 @@ var FakeInstanceID = "321a8b81-3660-43e5-bab8-6470b65ee4e8" const FakeMaxVolume int64 = 256 -var FakeFsStats = mount.FsStats{ +var FakeFsStats = &mount.DeviceStats{ + Block: false, + AvailableBytes: 2100, TotalBytes: 2121, UsedBytes: 21, @@ -107,4 +109,9 @@ var FakeFsStats = mount.FsStats{ TotalInodes: 200, UsedInodes: 50, } -var FakeBlockDeviceSize = 536870912 + +var FakeBlockDeviceStats = &mount.DeviceStats{ + Block: true, + + TotalBytes: 536870912, +} diff --git a/pkg/csi/cinder/nodeserver.go b/pkg/csi/cinder/nodeserver.go index 28ea30111d..536c1277c1 100644 --- a/pkg/csi/cinder/nodeserver.go +++ b/pkg/csi/cinder/nodeserver.go @@ -493,41 +493,32 @@ func (ns *nodeServer) NodeGetVolumeStats(_ context.Context, req *csi.NodeGetVolu exists, err := ns.Mount.PathExists(volumePath) if err != nil { - return nil, status.Errorf(codes.Internal, "failed to stats volumePath: %s", err) - } else if !exists { + return nil, status.Errorf(codes.Internal, "failed to check whether volumePath exists: %s", err) + } + if !exists { return nil, status.Errorf(codes.NotFound, "target: %s not found", volumePath) } - isBlock, err := ns.Mount.IsBlockDevice(volumePath) + stats, err := ns.Mount.GetDeviceStats(volumePath) if err != nil { - return nil, status.Errorf(codes.Internal, "failed to determine if volume is a block device: %s", err) + return nil, status.Errorf(codes.Internal, "failed to get stats by path: %s", err) } - if isBlock { - size, err := ns.Mount.GetBlockDeviceSize(volumePath) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to get block device size: %s", err) - } - + if stats.Block { return &csi.NodeGetVolumeStatsResponse{ Usage: []*csi.VolumeUsage{ { - Total: size, + Total: stats.TotalBytes, Unit: csi.VolumeUsage_BYTES, }, }, }, nil } - fsStats, err := ns.Mount.GetFileSystemStats(volumePath) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to get filesystem size: %s", err) - } - return &csi.NodeGetVolumeStatsResponse{ Usage: []*csi.VolumeUsage{ - {Total: fsStats.TotalBytes, Available: fsStats.AvailableBytes, Used: fsStats.UsedBytes, Unit: csi.VolumeUsage_BYTES}, - {Total: fsStats.TotalInodes, Available: fsStats.AvailableInodes, Used: fsStats.UsedInodes, Unit: csi.VolumeUsage_INODES}, + {Total: stats.TotalBytes, Available: stats.AvailableBytes, Used: stats.UsedBytes, Unit: csi.VolumeUsage_BYTES}, + {Total: stats.TotalInodes, Available: stats.AvailableInodes, Used: stats.UsedInodes, Unit: csi.VolumeUsage_INODES}, }, }, nil } diff --git a/pkg/csi/cinder/nodeserver_test.go b/pkg/csi/cinder/nodeserver_test.go index fe426fb2b0..1f68874b69 100644 --- a/pkg/csi/cinder/nodeserver_test.go +++ b/pkg/csi/cinder/nodeserver_test.go @@ -395,11 +395,10 @@ func TestNodeGetVolumeStatsBlock(t *testing.T) { mmock.On("PathExists", FakeDevicePath).Return(true, nil) // Invoke NodeGetVolumeStats with a block device - mmock.On("IsBlockDevice", FakeDevicePath).Return(true, nil) - mmock.On("GetBlockDeviceSize", FakeDevicePath).Return(int64(FakeBlockDeviceSize), nil) + mmock.On("GetDeviceStats", FakeDevicePath).Return(FakeBlockDeviceStats, nil) expectedBlockRes := &csi.NodeGetVolumeStatsResponse{ Usage: []*csi.VolumeUsage{ - {Total: int64(FakeBlockDeviceSize), Unit: csi.VolumeUsage_BYTES}, + {Total: FakeBlockDeviceStats.TotalBytes, Unit: csi.VolumeUsage_BYTES}, }, } @@ -411,6 +410,7 @@ func TestNodeGetVolumeStatsBlock(t *testing.T) { } func TestNodeGetVolumeStatsFs(t *testing.T) { + // Init assert assert := assert.New(t) mmock.ExpectedCalls = nil @@ -422,8 +422,7 @@ func TestNodeGetVolumeStatsFs(t *testing.T) { } mmock.On("PathExists", FakeDevicePath).Return(true, nil) - mmock.On("IsBlockDevice", FakeDevicePath).Return(false, nil) - mmock.On("GetFileSystemStats", FakeDevicePath).Return(FakeFsStats, nil) + mmock.On("GetDeviceStats", FakeDevicePath).Return(FakeFsStats, nil) expectedFsRes := &csi.NodeGetVolumeStatsResponse{ Usage: []*csi.VolumeUsage{ {Total: FakeFsStats.TotalBytes, Available: FakeFsStats.AvailableBytes, Used: FakeFsStats.UsedBytes, Unit: csi.VolumeUsage_BYTES}, @@ -435,4 +434,5 @@ func TestNodeGetVolumeStatsFs(t *testing.T) { assert.NoError(err) assert.Equal(expectedFsRes, fsRes) + } diff --git a/pkg/csi/cinder/sanity/fakemount.go b/pkg/csi/cinder/sanity/fakemount.go index a1682f2c74..d02dc0e9c2 100644 --- a/pkg/csi/cinder/sanity/fakemount.go +++ b/pkg/csi/cinder/sanity/fakemount.go @@ -48,18 +48,10 @@ func (m *fakemount) MakeFile(pathname string) error { return nil } -func (m *fakemount) PathExists(devicePath string) (bool, error) { +func (m *fakemount) PathExists(path string) (bool, error) { return false, nil } -func (m *fakemount) GetBlockDeviceSize(volumePath string) (int64, error) { - return 0, nil -} - -func (m *fakemount) GetFileSystemStats(volumePath string) (mount2.FsStats, error) { - return mount2.FsStats{}, nil -} - -func (m *fakemount) IsBlockDevice(devicePath string) (bool, error) { - return true, nil +func (m *fakemount) GetDeviceStats(path string) (*mount2.DeviceStats, error) { + return nil, nil } diff --git a/pkg/util/blockdevice/blockdevice_linux.go b/pkg/util/blockdevice/blockdevice_linux.go index 5a7da1f5a8..1ff8775daa 100644 --- a/pkg/util/blockdevice/blockdevice_linux.go +++ b/pkg/util/blockdevice/blockdevice_linux.go @@ -24,6 +24,8 @@ import ( "path/filepath" "strings" + "golang.org/x/sys/unix" + "k8s.io/klog" ) @@ -43,8 +45,19 @@ func findBlockDeviceRescanPath(path string) (string, error) { return "", fmt.Errorf("illegal path for device " + devicePath) } -// getBlockDeviceSize returns the size of the block device by path -func getBlockDeviceSize(path string) (int64, error) { +// IsBlockDevice checks whether device on the path is a block device +func IsBlockDevice(path string) (bool, error) { + var stat unix.Stat_t + err := unix.Stat(path, &stat) + if err != nil { + return false, fmt.Errorf("failed to stat() %q: %s", path, err) + } + + return (stat.Mode & unix.S_IFMT) == unix.S_IFBLK, nil +} + +// GetBlockDeviceSize returns the size of the block device by path +func GetBlockDeviceSize(path string) (int64, error) { fd, err := os.Open(path) if err != nil { return 0, err @@ -59,7 +72,7 @@ func getBlockDeviceSize(path string) (int64, error) { func checkBlockDeviceSize(devicePath string, deviceMountPath string, newSize int64) error { klog.V(4).Infof("Detecting %q volume size", deviceMountPath) - size, err := getBlockDeviceSize(devicePath) + size, err := GetBlockDeviceSize(devicePath) if err != nil { return err } diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index 70d5c53d0c..233979f6d5 100644 --- a/pkg/util/mount/mount.go +++ b/pkg/util/mount/mount.go @@ -21,10 +21,11 @@ import ( "io/ioutil" "os" "path" - "strconv" "strings" "time" + "k8s.io/cloud-provider-openstack/pkg/util/blockdevice" + "golang.org/x/sys/unix" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog" @@ -51,13 +52,13 @@ type IMount interface { GetInstanceID() (string, error) MakeFile(pathname string) error MakeDir(pathname string) error - PathExists(devicePath string) (bool, error) - GetBlockDeviceSize(volumePath string) (int64, error) - GetFileSystemStats(volumePath string) (FsStats, error) - IsBlockDevice(devicePath string) (bool, error) + PathExists(path string) (bool, error) + GetDeviceStats(path string) (*DeviceStats, error) } -type FsStats struct { +type DeviceStats struct { + Block bool + AvailableBytes int64 TotalBytes int64 UsedBytes int64 @@ -78,39 +79,31 @@ func (m *Mount) PathExists(volumePath string) (bool, error) { return true, nil } -func (m *Mount) IsBlockDevice(devicePath string) (bool, error) { - var stat unix.Stat_t - err := unix.Stat(devicePath, &stat) - if err != nil { - return false, fmt.Errorf("failed to stat() %q: %s", devicePath, err) - } +func (m *Mount) GetDeviceStats(path string) (*DeviceStats, error) { + isBlock, err := blockdevice.IsBlockDevice(path) - return (stat.Mode & unix.S_IFMT) == unix.S_IFBLK, nil -} + if isBlock { + size, err := blockdevice.GetBlockDeviceSize(path) + if err != nil { + return nil, err + } -func (m *Mount) GetBlockDeviceSize(volumePath string) (int64, error) { - // See http://man7.org/linux/man-pages/man8/blockdev.8.html for details - output, err := m.GetBaseMounter().Exec.Command("blockdev", "--getsize64", volumePath).CombinedOutput() - if err != nil { - return 0, fmt.Errorf("error when getting size of block volume at path %s: output: %s, err: %v", volumePath, string(output), err) - } - strOut := strings.TrimSpace(string(output)) - gotSizeBytes, err := strconv.ParseInt(strOut, 10, 64) - if err != nil { - return 0, fmt.Errorf("failed to parse size %s into int", strOut) + return &DeviceStats{ + Block: true, + TotalBytes: size, + }, nil } - return gotSizeBytes, nil -} -func (m *Mount) GetFileSystemStats(volumePath string) (FsStats, error) { var statfs unix.Statfs_t // See http://man7.org/linux/man-pages/man2/statfs.2.html for details. - err := unix.Statfs(volumePath, &statfs) + err = unix.Statfs(path, &statfs) if err != nil { - return FsStats{}, err + return nil, err } - return FsStats{ + return &DeviceStats{ + Block: false, + AvailableBytes: int64(statfs.Bavail) * statfs.Bsize, TotalBytes: int64(statfs.Blocks) * statfs.Bsize, UsedBytes: (int64(statfs.Blocks) - int64(statfs.Bfree)) * statfs.Bsize, diff --git a/pkg/util/mount/mount_mock.go b/pkg/util/mount/mount_mock.go index 3cab435460..0ad3e9da01 100644 --- a/pkg/util/mount/mount_mock.go +++ b/pkg/util/mount/mount_mock.go @@ -35,36 +35,10 @@ func (_m *MountMock) PathExists(volumePath string) (bool, error) { return ret.Bool(0), ret.Error(1) } -func (_m *MountMock) GetBlockDeviceSize(volumePath string) (int64, error) { - ret := _m.Called(volumePath) - - return ret.Get(0).(int64), ret.Error(1) -} - -func (_m *MountMock) GetFileSystemStats(volumePath string) (FsStats, error) { - ret := _m.Called(volumePath) - - return ret.Get(0).(FsStats), ret.Error(1) -} - -func (_m *MountMock) IsBlockDevice(devicePath string) (bool, error) { - ret := _m.Called(devicePath) - - var r0 bool - if rf, ok := ret.Get(0).(func(string) bool); ok { - r0 = rf(devicePath) - } else { - r0 = ret.Bool(0) - } +func (_m *MountMock) GetDeviceStats(path string) (*DeviceStats, error) { + ret := _m.Called(path) - var r1 error - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(devicePath) - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return ret.Get(0).(*DeviceStats), ret.Error(1) } // NewFakeMounter returns fake mounter instance