Merge pull request #6 from sbezverk/volume_name_volume_id

Making VolumeID != volName as per CSI spec

pkg/rbd/controllerserver.go

@@ -48,12 +48,20 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		glog.V(3).Infof("invalid create volume req: %v", req)
 		return nil, err
 	}
-	// Volume Name
+
-	volName := req.GetName()
+	volOptions, err := getRBDVolumeOptions(req.Parameters, cs.clientSet)
-	if len(volName) == 0 {
+	if err != nil {
-		volName = uuid.NewUUID().String()
+		return nil, err
 	}
 
+	// Generating Volume Name and Volume ID, as accoeding to CSI spec they MUST be different
+	volName := req.GetName()
+	uniqueID := uuid.NewUUID().String()
+	if len(volName) == 0 {
+		volName = volOptions.Pool + "-dynamic-pvc-" + uniqueID
+	}
+	volOptions.VolName = volName
+	volumeID := "csi-rbd-" + uniqueID
 	// Volume Size - Default is 1 GiB
 	volSizeBytes := int64(oneGB)
 	if req.GetCapacityRange() != nil {
@@ -61,14 +69,10 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	}
 	volSizeGB := int(volSizeBytes / 1024 / 1024 / 1024)
 
-	volOptions, err := getRBDVolumeOptions(req.Parameters, cs.clientSet)
-	if err != nil {
-		return nil, err
-	}
 	// Check if there is already RBD image with requested name
-	found, _, _ := rbdStatus(volName, volOptions)
+	found, _, _ := rbdStatus(volOptions)
 	if !found {
-		if err := createRBDImage(volName, volSizeGB, volOptions); err != nil {
+		if err := createRBDImage(volOptions, volSizeGB); err != nil {
 			if err != nil {
 				glog.Warningf("failed to create volume: %v", err)
 				return nil, err
@@ -78,13 +82,13 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	}
 	// Storing volInfo into a persistent file, will need info to delete rbd image
 	// in ControllerUnpublishVolume
-	if err := persistVolInfo(volName, path.Join(PluginFolder, "controller"), volOptions); err != nil {
+	if err := persistVolInfo(volumeID, path.Join(PluginFolder, "controller"), volOptions); err != nil {
 		glog.Warningf("rbd: failed to store volInfo with error: %v", err)
 	}
 
 	return &csi.CreateVolumeResponse{
 		VolumeInfo: &csi.VolumeInfo{
-			Id:            volName,
+			Id:            volumeID,
 			CapacityBytes: uint64(volSizeBytes),
 			Attributes:    req.GetParameters(),
 		},
@@ -112,12 +116,13 @@ func (cs *controllerServer) ValidateVolumeCapabilities(ctx context.Context, req
 func (cs *controllerServer) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) {
 
 	// For now the image get unconditionally deleted, but here retention policy can be checked
-	volName := req.GetVolumeId()
+	volumeID := req.GetVolumeId()
 	volOptions := &rbdVolumeOptions{}
-	if err := loadVolInfo(volName, path.Join(PluginFolder, "controller"), volOptions); err != nil {
+	if err := loadVolInfo(volumeID, path.Join(PluginFolder, "controller"), volOptions); err != nil {
 		return nil, err
 	}
 
+	volName := volOptions.VolName
 	// Recover rbd secret key value, for now by k8s specific call
 	id := volOptions.AdminID
 	secretName := volOptions.AdminSecretName
@@ -134,12 +139,12 @@ func (cs *controllerServer) ControllerUnpublishVolume(ctx context.Context, req *
 
 	// Deleting rbd image
 	glog.V(4).Infof("deleting volume %s", volName)
-	if err := deleteRBDImage(volName, volOptions); err != nil {
+	if err := deleteRBDImage(volOptions); err != nil {
 		glog.V(3).Infof("failed to delete rbd image: %s/%s with error: %v", volOptions.Pool, volName, err)
 		return nil, err
 	}
 	// Removing persistent storage file for the unmapped volume
-	if err := deleteVolInfo(volName, path.Join(PluginFolder, "controller")); err != nil {
+	if err := deleteVolInfo(volumeID, path.Join(PluginFolder, "controller")); err != nil {
 		return nil, err
 	}
 

pkg/rbd/nodeserver.go

@@ -17,8 +17,10 @@ limitations under the License.
 package rbd
 
 import (
+	"fmt"
 	"os"
 	"path"
+	"strings"
 
 	"github.com/golang/glog"
 	"golang.org/x/net/context"
@@ -40,6 +42,12 @@ type nodeServer struct {
 func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) {
 	targetPath := req.GetTargetPath()
 
+	if !strings.HasSuffix(targetPath, "/mount") {
+		return nil, fmt.Errorf("rnd: malformed the value of target path: %s", targetPath)
+	}
+	s := strings.Split(strings.TrimSuffix(targetPath, "/mount"), "/")
+	volName := s[len(s)-1]
+
 	notMnt, err := mount.New("").IsLikelyNotMountPoint(targetPath)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -59,9 +67,9 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	if err != nil {
 		return nil, err
 	}
-
+	volOptions.VolName = volName
 	// Mapping RBD image
-	devicePath, err := attachRBDImage(req.GetVolumeId(), volOptions)
+	devicePath, err := attachRBDImage(volOptions)
 	if err != nil {
 		return nil, err
 	}
@@ -86,7 +94,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	}
 	// Storing rbd device path
 
-	volOptions.ImageMapping = map[string]string{req.GetVolumeId(): devicePath}
+	volOptions.ImageMapping = map[string]string{volOptions.VolName: devicePath}
 	// Storing volInfo into a persistent file
 	if err := persistVolInfo(req.GetVolumeId(), path.Join(PluginFolder, "node"), volOptions); err != nil {
 		glog.Warningf("rbd: failed to store volInfo with error: %v", err)
@@ -97,11 +105,12 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 
 func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) {
 	targetPath := req.GetTargetPath()
-	volName := req.GetVolumeId()
+	volumeID := req.GetVolumeId()
 	volOptions := &rbdVolumeOptions{}
-	if err := loadVolInfo(volName, path.Join(PluginFolder, "node"), volOptions); err != nil {
+	if err := loadVolInfo(volumeID, path.Join(PluginFolder, "node"), volOptions); err != nil {
 		return nil, err
 	}
+	volName := volOptions.VolName
 
 	// Recover rbd secret key value, for now by k8s specific call
 	id := volOptions.AdminID
@@ -131,12 +140,12 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu
 	}
 	// Unmapping rbd device
 	glog.V(4).Infof("deleting volume %s", volName)
-	if err := detachRBDImage(volName, volOptions); err != nil {
+	if err := detachRBDImage(volOptions); err != nil {
 		glog.V(3).Infof("failed to unmap rbd device: %s with error: %v", volOptions.ImageMapping[volName], err)
 		return nil, err
 	}
 	// Removing persistent storage file for the unmapped volume
-	if err := deleteVolInfo(volName, path.Join(PluginFolder, "node")); err != nil {
+	if err := deleteVolInfo(volumeID, path.Join(PluginFolder, "node")); err != nil {
 		return nil, err
 	}
 

pkg/rbd/rbd_util.go

@@ -47,6 +47,7 @@ const (
 )
 
 type rbdVolumeOptions struct {
+	VolName              string            `json:"volName"`
 	Monitors             string            `json:"monitors"`
 	Pool                 string            `json:"pool"`
 	AdminSecretName      string            `json:"adminSecret"`
@@ -65,12 +66,13 @@ type rbdVolumeOptions struct {
 var attachdetachMutex = keymutex.NewKeyMutex()
 
 // CreateImage creates a new ceph image with provision and volume options.
-func createRBDImage(image string, volSz int, pOpts *rbdVolumeOptions) error {
+func createRBDImage(pOpts *rbdVolumeOptions, volSz int) error {
 	var output []byte
 	var err error
 
 	// rbd create
 	mon := pOpts.Monitors
+	image := pOpts.VolName
 	volSzGB := fmt.Sprintf("%dG", volSz)
 
 	if pOpts.ImageFormat == rbdImageFormat2 {
@@ -96,11 +98,12 @@ func createRBDImage(image string, volSz int, pOpts *rbdVolumeOptions) error {
 
 // rbdStatus checks if there is watcher on the image.
 // It returns true if there is a watcher onthe image, otherwise returns false.
-func rbdStatus(image string, b *rbdVolumeOptions) (bool, string, error) {
+func rbdStatus(b *rbdVolumeOptions) (bool, string, error) {
 	var err error
 	var output string
 	var cmd []byte
 
+	image := b.VolName
 	// If we don't have admin id/secret (e.g. attaching), fallback to user id/secret.
 	id := b.AdminID
 	secret := b.adminSecret
@@ -137,9 +140,10 @@ func rbdStatus(image string, b *rbdVolumeOptions) (bool, string, error) {
 }
 
 // DeleteImage deletes a ceph image with provision and volume options.
-func deleteRBDImage(image string, b *rbdVolumeOptions) error {
+func deleteRBDImage(b *rbdVolumeOptions) error {
 	var output []byte
-	found, _, err := rbdStatus(image, b)
+	image := b.VolName
+	found, _, err := rbdStatus(b)
 	if err != nil {
 		return err
 	}
@@ -242,10 +246,11 @@ func parseStorageClassSecret(secretName string, namespace string, client *kubern
 	return secret, nil
 }
 
-func attachRBDImage(image string, volOptions *rbdVolumeOptions) (string, error) {
+func attachRBDImage(volOptions *rbdVolumeOptions) (string, error) {
 	var err error
 	var output []byte
 
+	image := volOptions.VolName
 	devicePath, found := waitForPath(volOptions.Pool, image, 1)
 	if !found {
 		attachdetachMutex.LockKey(string(volOptions.Pool + image))
@@ -262,7 +267,7 @@ func attachRBDImage(image string, volOptions *rbdVolumeOptions) (string, error)
 			Steps:    rbdImageWatcherSteps,
 		}
 		err := wait.ExponentialBackoff(backoff, func() (bool, error) {
-			used, rbdOutput, err := rbdStatus(image, volOptions)
+			used, rbdOutput, err := rbdStatus(volOptions)
 			if err != nil {
 				return false, fmt.Errorf("fail to check rbd image status with: (%v), rbd output: (%s)", err, rbdOutput)
 			}
@@ -301,10 +306,11 @@ func attachRBDImage(image string, volOptions *rbdVolumeOptions) (string, error)
 	return devicePath, nil
 }
 
-func detachRBDImage(image string, volOptions *rbdVolumeOptions) error {
+func detachRBDImage(volOptions *rbdVolumeOptions) error {
 	var err error
 	var output []byte
 
+	image := volOptions.VolName
 	glog.V(1).Infof("rbd: unmap device %s", volOptions.ImageMapping[image])
 	// If we don't have admin id/secret (e.g. attaching), fallback to user id/secret.
 	id := volOptions.AdminID