Skip to content

Commit

Permalink
Merge pull request #135 from Madhu-1/parse-cleanup
Browse files Browse the repository at this point in the history 
Remove redundant function parameters
  • Loading branch information
@humblec
humblec authored Jan 7, 2019
2 parents b26c7a7 + df20225 commit 9583911
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 45 deletions.
71 changes: 38 additions & 33 deletions pkg/glusterfs/controllerserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,22 @@ const (

var errVolumeNotFound = errors.New("volume not found")

// ControllerServer struct of GlusterFS CSI driver with supported methods of CSI controller server spec.
// ControllerServer struct of GlusterFS CSI driver with supported methods of CSI
// controller server spec.
type ControllerServer struct {
*GfDriver
}

// CsiDrvParam stores csi driver specific request parameters.
// This struct will be used to gather specific fields of CSI driver:
// For eg. csiDrvName, csiDrvVersion..etc
// and also gather parameters passed from SC which not part of gluster volcreate api.
// For eg. csiDrvName, csiDrvVersion..etc and also gather
// parameters passed from SC which not part of gluster volcreate api.
// GlusterCluster - The resturl of gluster cluster
// GlusterUser - The gluster username who got access to the APIs.
// GlusterUserToken - The password/token of glusterUser to connect to glusterCluster
// GlusterVersion - Says the version of the glustercluster running in glusterCluster endpoint.
// GlusterUserToken - The password/token of glusterUser to connect to
// glusterCluster.
// GlusterVersion - Says the version of the glustercluster
// running in glusterCluster endpoint.
// All of these fields are optional and can be used if needed.
type CsiDrvParam struct {
GlusterCluster string
Expand All @@ -52,13 +55,15 @@ type CsiDrvParam struct {
CsiDrvVersion string
}

// ProvisionerConfig is the combined configuration of gluster cli vol create request and CSI driver specific input
// ProvisionerConfig is the combined configuration of gluster cli vol create
// request and CSI driver specific input
type ProvisionerConfig struct {
gdVolReq *api.VolCreateReq
//csiConf *CsiDrvParam
// csiConf *CsiDrvParam
}

// ParseCreateVolRequest parse incoming volume create request and fill ProvisionerConfig.
// ParseCreateVolRequest parse incoming volume create request and fill
// ProvisionerConfig.
func (cs *ControllerServer) ParseCreateVolRequest(req *csi.CreateVolumeRequest) (*ProvisionerConfig, error) {

var reqConf ProvisionerConfig
Expand All @@ -79,7 +84,7 @@ func (cs *ControllerServer) ParseCreateVolRequest(req *csi.CreateVolumeRequest)

case "replicas":
replicas := v
replicaCount, err = parseVolumeParamInt(replicas, minReplicaCount, maxReplicaCount)
replicaCount, err = parseVolumeParamInt(replicas)
if err != nil {
return nil, fmt.Errorf("invalid value for parameter '%s', %v", k, err)
}
Expand All @@ -94,18 +99,18 @@ func (cs *ControllerServer) ParseCreateVolRequest(req *csi.CreateVolumeRequest)
return &reqConf, nil
}

func parseVolumeParamInt(valueString string, min int, max int) (int, error) {
func parseVolumeParamInt(valueString string) (int, error) {

count, err := strconv.Atoi(valueString)
if err != nil {
return 0, fmt.Errorf("value '%s' must be an integer between %d and %d", valueString, min, max)
return 0, fmt.Errorf("value '%s' must be an integer between %d and %d", valueString, minReplicaCount, maxReplicaCount)
}

if count < min {
return 0, fmt.Errorf("value '%s' must be >= %v", valueString, min)
if count < minReplicaCount {
return 0, fmt.Errorf("value '%s' must be >= %v", valueString, minReplicaCount)
}
if count > max {
return 0, fmt.Errorf("value '%s' must be <= %v", valueString, max)
if count > maxReplicaCount {
return 0, fmt.Errorf("value '%s' must be <= %v", valueString, maxReplicaCount)
}

return count, nil
Expand Down Expand Up @@ -155,9 +160,9 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
}
err = cs.client.VolumeStart(volumeName, true)
if err != nil {
//we dont need to delete the volume if volume start fails
//as we are listing the volumes and starting it again
//before sending back the response
// we dont need to delete the volume if volume start fails as we are
// listing the volumes and starting it again before sending back the
// response
glog.Errorf("failed to start volume: %v", err)
return nil, status.Errorf(codes.Internal, "failed to start volume: %v", err)
}
Expand Down Expand Up @@ -197,7 +202,7 @@ func (cs *ControllerServer) checkExistingSnapshot(snapName, volName string) erro
snapInfo, err := cs.client.SnapshotInfo(snapName)
if err != nil {
errResp := cs.client.LastErrorResponse()
//errResp will be nil in case of No route to host error
// errResp will be nil in case of No route to host error
if errResp != nil && errResp.StatusCode == http.StatusNotFound {
return status.Errorf(codes.NotFound, "failed to get snapshot info %s", err.Error())
}
Expand All @@ -214,7 +219,7 @@ func (cs *ControllerServer) checkExistingSnapshot(snapName, volName string) erro
return status.Errorf(codes.Internal, "failed to activate snapshot %s", err.Error())
}
}
//create snapshot clone
// create snapshot clone
err = cs.createSnapshotClone(snapName, volName)
return err
}
Expand Down Expand Up @@ -269,7 +274,7 @@ func (cs *ControllerServer) doVolumeCreate(volumeName string, volSizeBytes int64
if err != nil {
glog.Errorf("failed to create volume: %v", err)
errResp := cs.client.LastErrorResponse()
//errResp will be nil in case of `No route to host` error
// errResp will be nil in case of `No route to host` error
if errResp != nil && errResp.StatusCode == http.StatusConflict {
return status.Errorf(codes.Aborted, "volume create already in process: %v", err)
}
Expand All @@ -285,7 +290,7 @@ func (cs *ControllerServer) checkExistingVolume(volumeName string, volSizeBytes
vol, err := cs.client.Volumes(volumeName)
if err != nil {
errResp := cs.client.LastErrorResponse()
//errResp will be nil in case of `No route to host` error
// errResp will be nil in case of `No route to host` error
if errResp != nil && errResp.StatusCode == http.StatusNotFound {
return errVolumeNotFound
}
Expand All @@ -304,7 +309,7 @@ func (cs *ControllerServer) checkExistingVolume(volumeName string, volSizeBytes
return status.Errorf(codes.AlreadyExists, "volume %s already exits with different size: %d", volInfo.Name, volInfo.Capacity)
}

//volume has not started, start the volume
// volume has not started, start the volume
if volInfo.State != api.VolStarted {
err = cs.client.VolumeStart(volInfo.Name, true)
if err != nil {
Expand Down Expand Up @@ -362,7 +367,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol

if err != nil {
errResp := cs.client.LastErrorResponse()
//errResp will be nil in case of `No route to host` error
// errResp will be nil in case of `No route to host` error
if errResp != nil && errResp.StatusCode == http.StatusNotFound {
return &csi.DeleteVolumeResponse{}, nil
}
Expand All @@ -376,7 +381,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
err = cs.client.VolumeDelete(req.VolumeId)
if err != nil {
errResp := cs.client.LastErrorResponse()
//errResp will be nil in case of No route to host error
// errResp will be nil in case of No route to host error
if errResp != nil && errResp.StatusCode == http.StatusNotFound {
return &csi.DeleteVolumeResponse{}, nil
}
Expand Down Expand Up @@ -455,7 +460,7 @@ func (cs *ControllerServer) ValidateVolumeCapabilities(ctx context.Context, req

// ListVolumes returns a list of volumes
func (cs *ControllerServer) ListVolumes(ctx context.Context, req *csi.ListVolumesRequest) (*csi.ListVolumesResponse, error) {
//Fetch all the volumes in the TSP
// Fetch all the volumes in the TSP
volumes, err := cs.client.Volumes("")
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
Expand Down Expand Up @@ -523,7 +528,7 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
if err != nil {
glog.Errorf("failed to get snapshot info for %v with Error %v", req.GetName(), err.Error())
errResp := cs.client.LastErrorResponse()
//errResp will be nil in case of No route to host error
// errResp will be nil in case of No route to host error
if errResp != nil && errResp.StatusCode != http.StatusNotFound {

return nil, status.Errorf(codes.Internal, "CreateSnapshot - failed to get snapshot info %s", err.Error())
Expand Down Expand Up @@ -583,7 +588,7 @@ func (cs *ControllerServer) doSnapshot(name, sourceVolID string) (*api.SnapCreat
if err != nil {
glog.Errorf("failed to create snapshot %v", err)
errResp := cs.client.LastErrorResponse()
//errResp will be nil in case of `No route to host` error
// errResp will be nil in case of `No route to host` error
if errResp != nil && errResp.StatusCode == http.StatusConflict {
return nil, status.Errorf(codes.Aborted, "snapshot create already in process: %v", err)
}
Expand Down Expand Up @@ -613,7 +618,7 @@ func (cs *ControllerServer) validateCreateSnapshotReq(req *csi.CreateSnapshotReq
return status.Error(codes.InvalidArgument, "CreateSnapshot - sourceVolumeId is nil")
}
if req.GetName() == req.GetSourceVolumeId() {
//In glusterd2 we cannot create a snapshot as same name as volume name
// In glusterd2 we cannot create a snapshot as same name as volume name
return status.Error(codes.InvalidArgument, "CreateSnapshot - sourceVolumeId and snapshot name cannot be same")
}
return nil
Expand Down Expand Up @@ -673,7 +678,7 @@ func (cs *ControllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap
return cs.listSnapshotFromID(req.GetSnapshotId())
}

//If volume id is sent
// If volume id is sent
if len(req.GetSourceVolumeId()) != 0 {
snaplist, err = cs.client.SnapshotList(req.SourceVolumeId)
if err != nil {
Expand All @@ -686,7 +691,7 @@ func (cs *ControllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap
return nil, status.Errorf(codes.Internal, "ListSnapshot - failed to get snapshots %s", err.Error())
}
} else {
//Get all snapshots
// Get all snapshots
snaplist, err = cs.client.SnapshotList("")
if err != nil {
glog.Errorf("failed to list snapshots %v", err)
Expand Down Expand Up @@ -755,8 +760,8 @@ func (cs *ControllerServer) doPagination(req *csi.ListSnapshotsRequest, snapList

}

//TODO need to remove paginating code once glusterd2 issue
//https://github.com/gluster/glusterd2/issues/372 is merged
// TODO need to remove paginating code once glusterd2 issue
// https://github.com/gluster/glusterd2/issues/372 is merged
var (
maximumEntries = req.MaxEntries
nextToken int32
Expand Down
6 changes: 4 additions & 2 deletions pkg/glusterfs/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ const (
glusterfsCSIDriverVersion = "0.0.9"
)

// GfDriver is the struct embedding information about the connection to gluster cluster and configuration of CSI driver.
// GfDriver is the struct embedding information about the connection to gluster
// cluster and configuration of CSI driver.
type GfDriver struct {
client *restclient.Client
*utils.Config
Expand Down Expand Up @@ -63,7 +64,8 @@ func NewIdentityServer(g *GfDriver) *IdentityServer {
}
}

// Run start a non-blocking grpc controller,node and identityserver for GlusterFS CSI driver which can serve multiple parallel requests
// Run start a non-blocking grpc controller,node and identityserver for
// GlusterFS CSI driver which can serve multiple parallel requests
func (g *GfDriver) Run() {
srv := csicommon.NewNonBlockingGRPCServer()
srv.Start(g.Endpoint, NewIdentityServer(g), NewControllerServer(g), NewNodeServer(g))
Expand Down
3 changes: 2 additions & 1 deletion pkg/glusterfs/identityserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import (
"github.com/golang/glog"
)

// IdentityServer struct of Glusterfs CSI driver with supported methods of CSI identity server spec.
// IdentityServer struct of Glusterfs CSI driver with supported methods of CSI
// identity server spec.
type IdentityServer struct {
*GfDriver
}
Expand Down
11 changes: 7 additions & 4 deletions pkg/glusterfs/nodeserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import (
"k8s.io/kubernetes/pkg/volume/util"
)

// NodeServer struct of Glusterfs CSI driver with supported methods of CSI node server spec.
// NodeServer struct of Glusterfs CSI driver with supported methods of CSI node
// server spec.
type NodeServer struct {
*GfDriver
}
Expand All @@ -31,7 +32,8 @@ func (ns *NodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
return nil, status.Error(codes.Unimplemented, "")
}

// NodePublishVolume mounts the volume mounted to the staging path to the target path
// NodePublishVolume mounts the volume mounted to the staging path to the target
// path
func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) {
glog.V(2).Infof("received node publish volume request %+v", req)

Expand Down Expand Up @@ -82,10 +84,11 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
return &csi.NodePublishVolumeResponse{}, nil
}

// NodeGetVolumeStats returns volume capacity statistics available for the volume
// NodeGetVolumeStats returns volume capacity statistics available for the
// volume
func (ns *NodeServer) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) {

//TODO need to implement volume status call
// TODO need to implement volume status call
return nil, status.Error(codes.Unimplemented, "")

}
Expand Down
10 changes: 5 additions & 5 deletions pkg/glusterfs/utils/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ const (
TB int64 = 1000 * GB
)

// RoundUpSize calculates how many allocation units are needed to accommodate
// a volume of given size.
// RoundUpSize(1500 * 1000*1000, 1000*1000*1000) returns '2'
// (2 GB is the smallest allocatable volume that can hold 1500MiB)
// RoundUpSize calculates how many allocation units are needed to accommodate a
// volume of given size.
// RoundUpSize(1500 * 1000*1000, 1000*1000*1000) returns
// '2' (2 GB is the smallest allocatable volume that can hold 1500MiB)
func RoundUpSize(volumeSizeBytes int64, allocationUnitBytes int64) int64 {
return (volumeSizeBytes + allocationUnitBytes - 1) / allocationUnitBytes
}
Expand All @@ -30,7 +30,7 @@ type Config struct {
RestSecret string // GD2 user password
}

//NewConfig returns config struct to initialize new driver
// NewConfig returns config struct to initialize new driver
func NewConfig() *Config {
return &Config{}
}

0 comments on commit 9583911

Please sign in to comment.