Skip to content

Commit

Permalink
Critical fix to prperly create and delete volumes
Browse files Browse the repository at this point in the history
- Fix path references in ControllerServer
- Refactor volume creation and deletion into createHostpathVolume()/deleteHospathVolume() functions
- Update Node and Controller servers to use volume creation functions
  • Loading branch information
vladimirvivien committed Mar 25, 2019
1 parent b8652fe commit d9c5039
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 32 deletions.
36 changes: 17 additions & 19 deletions pkg/hostpath/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
requestedAccessType = mountAccess
}

// Check for maximum available capacity
capacity := int64(req.GetCapacityRange().GetRequiredBytes())
if capacity >= maxStorageCapacity {
return nil, status.Errorf(codes.OutOfRange, "Requested capacity %d exceeds maximum allowed %d", capacity, maxStorageCapacity)
}

// Need to check for already existing volume name, and if found
// check for the requested capacity and already allocated capacity
if exVol, err := getVolumeByName(req.GetName()); err == nil {
Expand All @@ -132,11 +138,6 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
}
return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("Volume with the same name: %s but with different size already exist", req.GetName()))
}
// Check for maximum available capacity
capacity := int64(req.GetCapacityRange().GetRequiredBytes())
if capacity >= maxStorageCapacity {
return nil, status.Errorf(codes.OutOfRange, "Requested capacity %d exceeds maximum allowed %d", capacity, maxStorageCapacity)
}

volumeID := uuid.NewUUID().String()
path := getVolumePath(volumeID)
Expand Down Expand Up @@ -164,11 +165,11 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to attach device: %v", err))
}
case mountAccess:
err := os.MkdirAll(path, 0777)
vol, err := createHospathVolume(volumeID, req.GetName(), capacity, requestedAccessType)
if err != nil {
glog.V(3).Infof("failed to create volume: %v", err)
return nil, err
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to create volume: %s", err))
}
glog.V(4).Infof("created volume %s at path %s", vol.VolID, vol.VolPath)
}

if req.GetVolumeContentSource() != nil {
Expand All @@ -191,14 +192,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
}
}
}
glog.V(4).Infof("create volume %s", path)
hostPathVol := hostPathVolume{}
hostPathVol.VolName = req.GetName()
hostPathVol.VolID = volumeID
hostPathVol.VolSize = capacity
hostPathVol.VolPath = path
hostPathVol.VolAccessType = requestedAccessType
hostPathVolumes[volumeID] = hostPathVol

return &csi.CreateVolumeResponse{
Volume: &csi.Volume{
VolumeId: volumeID,
Expand Down Expand Up @@ -231,7 +225,7 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol

volPathHandler := volumepathhandler.VolumePathHandler{}
// Get the associated loop device.
device, err := volPathHandler.GetLoopDevice(provisionRoot + vol.VolID)
device, err := volPathHandler.GetLoopDevice(getVolumePath(vol.VolID))
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to get the loop device: %v", err))
}
Expand All @@ -245,8 +239,12 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
}
}

os.RemoveAll(vol.VolPath)
delete(hostPathVolumes, vol.VolID)
if err := deleteHostpathVolume(vol.VolID); err != nil && !os.IsNotExist(err) {
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to delete volume: %s", err))
}

glog.V(4).Infof("volume deleted ok: %s", vol.VolID)

return &csi.DeleteVolumeResponse{}, nil
}

Expand Down
17 changes: 13 additions & 4 deletions pkg/hostpath/hostpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,21 +144,30 @@ func getVolumePath(volID string) string {

// createVolume create the directory for the hostpath volume.
// It returns the volume path or err if one occurs.
func createVolumeDir(volID string) (string, error) {
func createHospathVolume(volID, name string, cap int64, volAccessType accessType) (*hostPathVolume, error) {
path := getVolumePath(volID)
err := os.MkdirAll(path, 0777)
if err != nil {
return "", err
return nil, err
}

return path, nil
hostpathVol := hostPathVolume{
VolID: volID,
VolName: name,
VolSize: cap,
VolPath: path,
VolAccessType: volAccessType,
}
hostPathVolumes[volID] = hostpathVol
return &hostpathVol, nil
}

// deleteVolume deletes the directory for the hostpath volume.
func deleteVolumeDir(volID string) error {
func deleteHostpathVolume(volID string) error {
path := getVolumePath(volID)
if err := os.RemoveAll(path); err != nil {
return err
}
delete(hostPathVolumes, volID)
return nil
}
25 changes: 16 additions & 9 deletions pkg/hostpath/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
return nil, status.Error(codes.InvalidArgument, "cannot have both block and mount access type")
}

// if ephemeral is specified, create volume here to avoid errors
if ns.ephemeral {
volID := req.GetVolumeId()
volName := fmt.Sprintf("ephemeral-%s", volID)
vol, err := createHospathVolume(req.GetVolumeId(), volName, maxStorageCapacity, mountAccess)
if err != nil && !os.IsExist(err) {
glog.Error("ephemeral mode failed to create volume: ", err)
return nil, status.Error(codes.Internal, err.Error())
}
glog.V(4).Infof("ephemeral mode: created volume: %s", vol.VolPath)
}

vol, err := getVolumeByID(req.GetVolumeId())
if err != nil {
return nil, status.Error(codes.NotFound, err.Error())
Expand Down Expand Up @@ -153,14 +165,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
options = append(options, "ro")
}
mounter := mount.New("")
path := provisionRoot + volumeId
if ns.ephemeral {
volPath, err := createVolumeDir(volumeId)
if err != nil && !os.IsExist(err) {
return nil, status.Error(codes.Internal, err.Error())
}
glog.V(4).Infof("ephemeral mode: created volume: %s", volPath)
}
path := getVolumePath(volumeId)

if err := mounter.Mount(path, targetPath, "", options); err != nil {
var errList strings.Builder
Expand Down Expand Up @@ -215,7 +220,9 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu

if ns.ephemeral {
glog.V(4).Infof("deleting volume %s", volumeID)
deleteVolumeDir(volumeID)
if err := deleteHostpathVolume(volumeID); err != nil && !os.IsNotExist(err) {
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to delete volume: %s", err))
}
}

return &csi.NodeUnpublishVolumeResponse{}, nil
Expand Down

0 comments on commit d9c5039

Please sign in to comment.