Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cinder-csi-plugin] NodeGetVolumeStats() implementation #941

Merged
merged 6 commits into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/csi/cinder/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions pkg/csi/cinder/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/util/mount"
)

var FakeCluster = "cluster"
Expand Down Expand Up @@ -97,3 +98,20 @@ var FakeVolListEmpty = []volumes.Volume{}
var FakeInstanceID = "321a8b81-3660-43e5-bab8-6470b65ee4e8"

const FakeMaxVolume int64 = 256

var FakeFsStats = &mount.DeviceStats{
Block: false,

AvailableBytes: 2100,
TotalBytes: 2121,
UsedBytes: 21,
AvailableInodes: 150,
TotalInodes: 200,
UsedInodes: 50,
}

var FakeBlockDeviceStats = &mount.DeviceStats{
Block: true,

TotalBytes: 536870912,
}
45 changes: 43 additions & 2 deletions pkg/csi/cinder/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,49 @@ 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("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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should comply with csi spec on required fields etc.
please run make test-cinder-csi-sanity and fix the issues respective to this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All sanity test errors related to NodeGetVolumeStats are fixed. Some tests fail on other methods, but it seems to be out of the scope of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, Awesome!! Thanks.

if len(volumePath) == 0 {
return nil, status.Error(codes.InvalidArgument, "Volume path not provided")
}

exists, err := ns.Mount.PathExists(volumePath)
if err != nil {
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)
}

stats, err := ns.Mount.GetDeviceStats(volumePath)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get stats by path: %s", err)
}

if stats.Block {
return &csi.NodeGetVolumeStatsResponse{
Usage: []*csi.VolumeUsage{
{
Total: stats.TotalBytes,
Unit: csi.VolumeUsage_BYTES,
},
},
}, nil
}

return &csi.NodeGetVolumeStatsResponse{
Usage: []*csi.VolumeUsage{
{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
}

func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) {
Expand Down
56 changes: 56 additions & 0 deletions pkg/csi/cinder/nodeserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,3 +380,59 @@ func TestNodeExpandVolume(t *testing.T) {
assert.Equal(expectedRes, actualRes)

}

func TestNodeGetVolumeStatsBlock(t *testing.T) {

// Init assert
assert := assert.New(t)
mmock.ExpectedCalls = nil

// Fake request
fakeReq := &csi.NodeGetVolumeStatsRequest{
VolumeId: FakeVolName,
VolumePath: FakeDevicePath,
}

mmock.On("PathExists", FakeDevicePath).Return(true, nil)
// Invoke NodeGetVolumeStats with a block device
mmock.On("GetDeviceStats", FakeDevicePath).Return(FakeBlockDeviceStats, nil)
expectedBlockRes := &csi.NodeGetVolumeStatsResponse{
Usage: []*csi.VolumeUsage{
{Total: FakeBlockDeviceStats.TotalBytes, 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)
mmock.ExpectedCalls = nil

// Fake request
fakeReq := &csi.NodeGetVolumeStatsRequest{
VolumeId: FakeVolName,
VolumePath: FakeDevicePath,
}

mmock.On("PathExists", FakeDevicePath).Return(true, 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},
{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)

}
9 changes: 9 additions & 0 deletions pkg/csi/cinder/sanity/fakemount.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package sanity

import (
"k8s.io/cloud-provider-openstack/pkg/csi/cinder"
mount2 "k8s.io/cloud-provider-openstack/pkg/util/mount"
"k8s.io/utils/mount"
)

Expand Down Expand Up @@ -46,3 +47,11 @@ func (m *fakemount) MakeDir(pathname string) error {
func (m *fakemount) MakeFile(pathname string) error {
return nil
}

func (m *fakemount) PathExists(path string) (bool, error) {
return false, nil
}

func (m *fakemount) GetDeviceStats(path string) (*mount2.DeviceStats, error) {
return nil, nil
}
19 changes: 16 additions & 3 deletions pkg/util/blockdevice/blockdevice_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"path/filepath"
"strings"

"golang.org/x/sys/unix"

"k8s.io/klog"
)

Expand All @@ -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
Expand All @@ -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
}
Expand Down
63 changes: 63 additions & 0 deletions pkg/util/mount/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import (
"strings"
"time"

"k8s.io/cloud-provider-openstack/pkg/util/blockdevice"

"golang.org/x/sys/unix"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/klog"
"k8s.io/utils/exec"
Expand All @@ -49,6 +52,66 @@ type IMount interface {
GetInstanceID() (string, error)
MakeFile(pathname string) error
MakeDir(pathname string) error
PathExists(path string) (bool, error)
GetDeviceStats(path string) (*DeviceStats, error)
}

type DeviceStats struct {
Block bool

AvailableBytes int64
TotalBytes int64
UsedBytes int64
AvailableInodes int64
TotalInodes int64
UsedInodes int64
}

func (m *Mount) PathExists(volumePath string) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:nit: i think this func exists in k8s.io/util/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has an additional check for a "corrupted mount". I'm not sure that this is required and this may introduce a potential false-positive.

https://github.com/kubernetes/utils/blob/d1ab8797c55812f4fefe2c7b00a0d04a4740a93c/mount/mount_helper_common.go#L99-L100

_, 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) GetDeviceStats(path string) (*DeviceStats, error) {
isBlock, err := blockdevice.IsBlockDevice(path)

if isBlock {
size, err := blockdevice.GetBlockDeviceSize(path)
if err != nil {
return nil, err
}

return &DeviceStats{
Block: true,
TotalBytes: size,
}, nil
}

var statfs unix.Statfs_t
// See http://man7.org/linux/man-pages/man2/statfs.2.html for details.
err = unix.Statfs(path, &statfs)
if err != nil {
return nil, err
}

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,

AvailableInodes: int64(statfs.Ffree),
TotalInodes: int64(statfs.Files),
UsedInodes: int64(statfs.Files) - int64(statfs.Ffree),
}, nil
}

type Mount struct {
Expand Down
12 changes: 12 additions & 0 deletions pkg/util/mount/mount_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ 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) GetDeviceStats(path string) (*DeviceStats, error) {
ret := _m.Called(path)

return ret.Get(0).(*DeviceStats), ret.Error(1)
}

// NewFakeMounter returns fake mounter instance
func NewFakeMounter() *mount.FakeMounter {
return &mount.FakeMounter{
Expand Down