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

@@ -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)
 
 }
 

pkg/csi/cinder/nodeserver.go

@@ -29,26 +29,37 @@ 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"
 )
 
 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)

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)

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,
 	}
 }