From cc73e20cfe8233ff8ad3dbd040866f164225f6f8 Mon Sep 17 00:00:00 2001 From: Anusha Ramineni Date: Tue, 9 Jul 2019 12:48:02 +0530 Subject: [PATCH] Adds cloud instance to nodeserver (#676) This commit adds cloud instance to nodeserver and adds error codes as per csi spec. --- pkg/csi/cinder/driver.go | 2 +- pkg/csi/cinder/nodeserver.go | 43 +++++++++++++++++++++++++++++-- pkg/csi/cinder/nodeserver_test.go | 18 ++++++++----- pkg/csi/cinder/utils.go | 3 ++- 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/pkg/csi/cinder/driver.go b/pkg/csi/cinder/driver.go index 6f2bf59a98..c385117603 100644 --- a/pkg/csi/cinder/driver.go +++ b/pkg/csi/cinder/driver.go @@ -135,7 +135,7 @@ func (d *CinderDriver) SetupDriver(cloud openstack.IOpenStack, mount mount.IMoun d.ids = NewIdentityServer(d) d.cs = NewControllerServer(d, cloud) - d.ns = NewNodeServer(d, mount, metadata) + d.ns = NewNodeServer(d, mount, metadata, cloud) } diff --git a/pkg/csi/cinder/nodeserver.go b/pkg/csi/cinder/nodeserver.go index dd2c44d441..11ea66c374 100644 --- a/pkg/csi/cinder/nodeserver.go +++ b/pkg/csi/cinder/nodeserver.go @@ -29,6 +29,7 @@ import ( "k8s.io/cloud-provider-openstack/pkg/csi/cinder/mount" "k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack" + cpoerrors "k8s.io/cloud-provider-openstack/pkg/util/errors" "k8s.io/cloud-provider-openstack/pkg/util/metadata" ) @@ -36,19 +37,29 @@ type nodeServer struct { Driver *CinderDriver Mount mount.IMount Metadata openstack.IMetadata + Cloud openstack.IOpenStack } func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { klog.V(4).Infof("NodePublishVolume: called with args %+v", *req) + volumeID := req.GetVolumeId() source := req.GetStagingTargetPath() targetPath := req.GetTargetPath() volumeCapability := req.GetVolumeCapability() - if len(source) == 0 || len(targetPath) == 0 || volumeCapability == nil { + if len(volumeID) == 0 || len(source) == 0 || len(targetPath) == 0 || volumeCapability == nil { return nil, status.Error(codes.InvalidArgument, "NodePublishVolume missing required arguments") } + _, err := ns.Cloud.GetVolume(volumeID) + if err != nil { + if cpoerrors.IsNotFound(err) { + return nil, status.Error(codes.NotFound, "Volume not found") + } + return nil, status.Error(codes.Internal, fmt.Sprintf("GetVolume failed with error %v", err)) + } + mountOptions := []string{"bind"} if req.GetReadonly() { mountOptions = append(mountOptions, "ro") @@ -127,10 +138,21 @@ func nodePublishVolumeForBlock(req *csi.NodePublishVolumeRequest, ns *nodeServer func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { klog.V(4).Infof("NodeUnPublishVolume: called with args %+v", *req) + volumeID := req.GetVolumeId() targetPath := req.GetTargetPath() if len(targetPath) == 0 { return nil, status.Error(codes.InvalidArgument, "NodeUnpublishVolume Target Path must be provided") } + if len(volumeID) == 0 { + return nil, status.Error(codes.InvalidArgument, "NodeUnpublishVolume volumeID must be provided") + } + _, err := ns.Cloud.GetVolume(volumeID) + if err != nil { + if cpoerrors.IsNotFound(err) { + return nil, status.Error(codes.NotFound, "Volume not found") + } + return nil, status.Error(codes.Internal, fmt.Sprintf("GetVolume failed with error %v", err)) + } m := ns.Mount @@ -168,6 +190,14 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol return nil, status.Error(codes.InvalidArgument, "NodeStageVolume Volume Capability must be provided") } + _, err := ns.Cloud.GetVolume(volumeID) + if err != nil { + if cpoerrors.IsNotFound(err) { + return nil, status.Error(codes.NotFound, "Volume not found") + } + return nil, status.Error(codes.Internal, fmt.Sprintf("GetVolume failed with error %v", err)) + } + m := ns.Mount // Do not trust the path provided by cinder, get the real path on node devicePath, err := getDevicePath(volumeID, m) @@ -211,7 +241,8 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) { klog.V(4).Infof("NodeUnstageVolume: called with args %+v", *req) - if len(req.GetVolumeId()) == 0 { + volumeID := req.GetVolumeId() + if len(volumeID) == 0 { return nil, status.Error(codes.InvalidArgument, "Volume Id not provided") } @@ -220,6 +251,14 @@ func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag return nil, status.Error(codes.InvalidArgument, "NodeUnstageVolume Staging Target Path must be provided") } + _, err := ns.Cloud.GetVolume(volumeID) + if err != nil { + if cpoerrors.IsNotFound(err) { + return nil, status.Error(codes.NotFound, "Volume not found") + } + return nil, status.Error(codes.Internal, fmt.Sprintf("GetVolume failed with error %v", err)) + } + m := ns.Mount notMnt, err := m.IsLikelyNotMountPointDetach(stagingTargetPath) diff --git a/pkg/csi/cinder/nodeserver_test.go b/pkg/csi/cinder/nodeserver_test.go index d6bdad4010..a20722de1d 100644 --- a/pkg/csi/cinder/nodeserver_test.go +++ b/pkg/csi/cinder/nodeserver_test.go @@ -29,7 +29,7 @@ import ( var fakeNs *nodeServer var mmock *mount.MountMock -var metamock *openstack.OpenStackMock +var omock *openstack.OpenStackMock // Init Node Server func init() { @@ -43,9 +43,10 @@ func init() { mmock = new(mount.MountMock) mount.MInstance = mmock - metamock = new(openstack.OpenStackMock) - openstack.MetadataService = metamock - fakeNs = NewNodeServer(d, mount.MInstance, openstack.MetadataService) + omock = new(openstack.OpenStackMock) + openstack.MetadataService = omock + openstack.OsInstance = omock + fakeNs = NewNodeServer(d, mount.MInstance, openstack.MetadataService, openstack.OsInstance) } } @@ -55,9 +56,9 @@ func TestNodeGetInfo(t *testing.T) { // GetInstanceID() (string, error) mmock.On("GetInstanceID").Return(FakeNodeID, nil) - metamock.On("GetAvailabilityZone").Return(FakeAvailability, nil) + omock.On("GetAvailabilityZone").Return(FakeAvailability, nil) - osmock.On("GetMaxVolumeLimit").Return(FakeMaxVolume) + omock.On("GetMaxVolumeLimit").Return(FakeMaxVolume) // Init assert assert := assert.New(t) @@ -91,7 +92,7 @@ func TestNodePublishVolume(t *testing.T) { mmock.On("IsLikelyNotMountPointAttach", FakeTargetPath).Return(true, nil) // Mount(source string, target string, fstype string, options []string) error mmock.On("Mount", FakeStagingTargetPath, FakeTargetPath, mock.AnythingOfType("string"), []string{"bind", "rw"}).Return(nil) - + omock.On("GetVolume", FakeVolID).Return(FakeVol, nil) // Init assert assert := assert.New(t) @@ -134,6 +135,7 @@ func TestNodeStageVolume(t *testing.T) { mmock.On("IsLikelyNotMountPointAttach", FakeStagingTargetPath).Return(true, nil) // FormatAndMount(source string, target string, fstype string, options []string) error mmock.On("FormatAndMount", FakeDevicePath, FakeStagingTargetPath, "ext4", []string(nil)).Return(nil) + omock.On("GetVolume", FakeVolID).Return(FakeVol, nil) // Init assert assert := assert.New(t) @@ -208,6 +210,7 @@ func TestNodeUnpublishVolume(t *testing.T) { mmock.On("IsLikelyNotMountPointDetach", FakeTargetPath).Return(false, nil) // UnmountPath(mountPath string) error mmock.On("UnmountPath", FakeTargetPath).Return(nil) + omock.On("GetVolume", FakeVolID).Return(FakeVol, nil) // Init assert assert := assert.New(t) @@ -238,6 +241,7 @@ func TestNodeUnstageVolume(t *testing.T) { mmock.On("IsLikelyNotMountPointDetach", FakeStagingTargetPath).Return(false, nil) // UnmountPath(mountPath string) error mmock.On("UnmountPath", FakeStagingTargetPath).Return(nil) + omock.On("GetVolume", FakeVolID).Return(FakeVol, nil) // Init assert assert := assert.New(t) diff --git a/pkg/csi/cinder/utils.go b/pkg/csi/cinder/utils.go index 3453fd5fbd..4201225947 100644 --- a/pkg/csi/cinder/utils.go +++ b/pkg/csi/cinder/utils.go @@ -49,11 +49,12 @@ func NewIdentityServer(d *CinderDriver) *identityServer { } } -func NewNodeServer(d *CinderDriver, mount mount.IMount, metadata openstack.IMetadata) *nodeServer { +func NewNodeServer(d *CinderDriver, mount mount.IMount, metadata openstack.IMetadata, cloud openstack.IOpenStack) *nodeServer { return &nodeServer{ Driver: d, Mount: mount, Metadata: metadata, + Cloud: cloud, } }