Merge pull request #148 from Madhu-1/add-parent-check

Add check for volumecontent source in CreateVolume
kubernetes-csi · Feb 13, 2020 · b9f759c · b9f759c
2 parents aba849b + 366d752
commit b9f759c
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 22 deletions.
diff --git a/pkg/hostpath/controllerserver.go b/pkg/hostpath/controllerserver.go
@@ -127,23 +127,36 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		// Since err is nil, it means the volume with the same name already exists
 		// need to check if the size of existing volume is the same as in new
 		// request
-		if exVol.VolSize >= int64(req.GetCapacityRange().GetRequiredBytes()) {
-			// existing volume is compatible with new request and should be reused.
-			// TODO (sbezverk) Do I need to make sure that volume still exists?
-			return &csi.CreateVolumeResponse{
-				Volume: &csi.Volume{
-					VolumeId:      exVol.VolID,
-					CapacityBytes: int64(exVol.VolSize),
-					VolumeContext: req.GetParameters(),
-					ContentSource: req.GetVolumeContentSource(),
-				},
-			}, nil
+		if exVol.VolSize < capacity {
+			return nil, status.Errorf(codes.AlreadyExists, "Volume with the same name: %s but with different size already exist", req.GetName())
+		}
+		if req.GetVolumeContentSource() != nil {
+			volumeSource := req.VolumeContentSource
+			switch volumeSource.Type.(type) {
+			case *csi.VolumeContentSource_Snapshot:
+				if volumeSource.GetSnapshot() != nil && exVol.ParentSnapID != volumeSource.GetSnapshot().GetSnapshotId() {
+					return nil, status.Error(codes.AlreadyExists, "existing volume source snapshot id not matching")
+				}
+			case *csi.VolumeContentSource_Volume:
+				if volumeSource.GetVolume() != nil && exVol.ParentVolID != volumeSource.GetVolume().GetVolumeId() {
+					return nil, status.Error(codes.AlreadyExists, "existing volume source volume id not matching")
+				}
+			default:
+				return nil, status.Errorf(codes.InvalidArgument, "%v not a proper volume source", volumeSource)
+			}
 		}
-		return nil, status.Errorf(codes.AlreadyExists, "Volume with the same name: %s but with different size already exist", req.GetName())
+		// TODO (sbezverk) Do I need to make sure that volume still exists?
+		return &csi.CreateVolumeResponse{
+			Volume: &csi.Volume{
+				VolumeId:      exVol.VolID,
+				CapacityBytes: int64(exVol.VolSize),
+				VolumeContext: req.GetParameters(),
+				ContentSource: req.GetVolumeContentSource(),
+			},
+		}, nil
 	}
 
 	volumeID := uuid.NewUUID().String()
-	path := getVolumePath(volumeID)
 
 	vol, err := createHostpathVolume(volumeID, req.GetName(), capacity, requestedAccessType, false /* ephemeral */)
 	if err != nil {
@@ -152,12 +165,21 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	glog.V(4).Infof("created volume %s at path %s", vol.VolID, vol.VolPath)
 
 	if req.GetVolumeContentSource() != nil {
-		contentSource := req.GetVolumeContentSource()
-		if snapshot := contentSource.GetSnapshot(); snapshot != nil {
-			err = loadFromSnapshot(capacity, snapshot.GetSnapshotId(), path)
-		}
-		if srcVolume := contentSource.GetVolume(); srcVolume != nil {
-			err = loadFromVolume(capacity, srcVolume.GetVolumeId(), path)
+		path := getVolumePath(volumeID)
+		volumeSource := req.VolumeContentSource
+		switch volumeSource.Type.(type) {
+		case *csi.VolumeContentSource_Snapshot:
+			if snapshot := volumeSource.GetSnapshot(); snapshot != nil {
+				err = loadFromSnapshot(capacity, snapshot.GetSnapshotId(), path)
+				vol.ParentSnapID = snapshot.GetSnapshotId()
+			}
+		case *csi.VolumeContentSource_Volume:
+			if srcVolume := volumeSource.GetVolume(); srcVolume != nil {
+				err = loadFromVolume(capacity, srcVolume.GetVolumeId(), path)
+				vol.ParentVolID = srcVolume.GetVolumeId()
+			}
+		default:
+			err = status.Errorf(codes.InvalidArgument, "%v not a proper volume source", volumeSource)
 		}
 		if err != nil {
 			if delErr := deleteHostpathVolume(volumeID); delErr != nil {

diff --git a/pkg/hostpath/hostpath.go b/pkg/hostpath/hostpath.go
@@ -17,6 +17,7 @@ limitations under the License.
 package hostpath
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -59,6 +60,8 @@ type hostPathVolume struct {
 	VolSize       int64      `json:"volSize"`
 	VolPath       string     `json:"volPath"`
 	VolAccessType accessType `json:"volAccessType"`
+	ParentVolID   string     `json:"parentVolID,omitempty"`
+	ParentSnapID  string     `json:"parentSnapID,omitempty"`
 	Ephemeral     bool       `json:"ephemeral"`
 }
 
@@ -93,15 +96,15 @@ func init() {
 
 func NewHostPathDriver(driverName, nodeID, endpoint string, ephemeral bool, maxVolumesPerNode int64, version string) (*hostPath, error) {
 	if driverName == "" {
-		return nil, fmt.Errorf("No driver name provided")
+		return nil, errors.New("no driver name provided")
 	}
 
 	if nodeID == "" {
-		return nil, fmt.Errorf("No node id provided")
+		return nil, errors.New("no node id provided")
 	}
 
 	if endpoint == "" {
-		return nil, fmt.Errorf("No driver endpoint provided")
+		return nil, errors.New("no driver endpoint provided")
 	}
 	if version != "" {
 		vendorVersion = version