From 7274bd09e548aee038506e1685b2d77fbc486497 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 25 Sep 2019 14:05:33 +0530 Subject: [PATCH] Fix volsize for cephfs and rbd Signed-off-by: Madhu Rajanna --- pkg/cephfs/controllerserver.go | 13 ++- pkg/cephfs/volumeoptions.go | 3 +- pkg/rbd/controllerserver.go | 10 +-- pkg/util/util.go | 46 ++++++++--- pkg/util/util_test.go | 144 +++++++++++++++++++++++++++++++++ 5 files changed, 195 insertions(+), 21 deletions(-) create mode 100644 pkg/util/util_test.go diff --git a/pkg/cephfs/controllerserver.go b/pkg/cephfs/controllerserver.go index 064769a5d..b8f45ffbf 100644 --- a/pkg/cephfs/controllerserver.go +++ b/pkg/cephfs/controllerserver.go @@ -84,17 +84,24 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol } defer cs.VolumeLocks.Release(requestName) - volOptions, err := newVolumeOptions(ctx, requestName, req.GetCapacityRange().GetRequiredBytes(), - req.GetParameters(), secret) + volOptions, err := newVolumeOptions(ctx, requestName, req.GetParameters(), secret) if err != nil { klog.Errorf(util.Log(ctx, "validation and extraction of volume options failed: %v"), err) return nil, status.Error(codes.InvalidArgument, err.Error()) } + if req.GetCapacityRange() != nil { + volOptions.Size = util.RoundOffBytes(req.GetCapacityRange().GetRequiredBytes()) + } + // TODO need to add check for 0 volume size + vID, err := checkVolExists(ctx, volOptions, secret) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } + + // TODO return error message if requested vol size greater than found volume return error + if vID != nil { return &csi.CreateVolumeResponse{ Volume: &csi.Volume{ @@ -132,7 +139,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return &csi.CreateVolumeResponse{ Volume: &csi.Volume{ VolumeId: vID.VolumeID, - CapacityBytes: req.GetCapacityRange().GetRequiredBytes(), + CapacityBytes: volOptions.Size, VolumeContext: req.GetParameters(), }, }, nil diff --git a/pkg/cephfs/volumeoptions.go b/pkg/cephfs/volumeoptions.go index 504019745..f3948324a 100644 --- a/pkg/cephfs/volumeoptions.go +++ b/pkg/cephfs/volumeoptions.go @@ -126,7 +126,7 @@ func getMonsAndClusterID(options map[string]string) (string, string, error) { // newVolumeOptions generates a new instance of volumeOptions from the provided // CSI request parameters -func newVolumeOptions(ctx context.Context, requestName string, size int64, volOptions, secret map[string]string) (*volumeOptions, error) { +func newVolumeOptions(ctx context.Context, requestName string, volOptions, secret map[string]string) (*volumeOptions, error) { var ( opts volumeOptions err error @@ -158,7 +158,6 @@ func newVolumeOptions(ctx context.Context, requestName string, size int64, volOp } opts.RequestName = requestName - opts.Size = size cr, err := util.NewAdminCredentials(secret) if err != nil { diff --git a/pkg/rbd/controllerserver.go b/pkg/rbd/controllerserver.go index 6d7f28717..76ce80b42 100644 --- a/pkg/rbd/controllerserver.go +++ b/pkg/rbd/controllerserver.go @@ -108,8 +108,8 @@ func (cs *ControllerServer) parseVolCreateRequest(ctx context.Context, req *csi. volSizeBytes = req.GetCapacityRange().GetRequiredBytes() } - // always round up the request size in bytes to the nearest MiB - rbdVol.VolSize = util.MiB * util.RoundUpToMiB(volSizeBytes) + // always round up the request size in bytes to the nearest MiB/GiB + rbdVol.VolSize = util.RoundOffBytes(volSizeBytes) // NOTE: rbdVol does not contain VolID and RbdImageName populated, everything // else is populated post create request parsing @@ -172,7 +172,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol } }() - err = cs.createBackingImage(ctx, rbdVol, req, util.RoundUpToMiB(rbdVol.VolSize)) + err = cs.createBackingImage(ctx, rbdVol, req, util.RoundOffVolSize(rbdVol.VolSize)) if err != nil { return nil, err } @@ -186,7 +186,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol }, nil } -func (cs *ControllerServer) createBackingImage(ctx context.Context, rbdVol *rbdVolume, req *csi.CreateVolumeRequest, volSizeMiB int64) error { +func (cs *ControllerServer) createBackingImage(ctx context.Context, rbdVol *rbdVolume, req *csi.CreateVolumeRequest, volSize int64) error { var err error // if VolumeContentSource is not nil, this request is for snapshot @@ -201,7 +201,7 @@ func (cs *ControllerServer) createBackingImage(ctx context.Context, rbdVol *rbdV } defer cr.DeleteCredentials() - err = createImage(ctx, rbdVol, volSizeMiB, cr) + err = createImage(ctx, rbdVol, volSize, cr) if err != nil { klog.Warningf(util.Log(ctx, "failed to create volume: %v"), err) return status.Error(codes.Internal, err.Error()) diff --git a/pkg/util/util.go b/pkg/util/util.go index 08dc6e396..48d5e838a 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -18,6 +18,7 @@ package util import ( "context" + "math" "os" "path" "strings" @@ -36,12 +37,43 @@ import ( const ( // MiB - MebiByte size MiB = 1024 * 1024 + GiB = MiB * 1024 ) -// RoundUpToMiB rounds up given quantity upto chunks of MiB -func RoundUpToMiB(size int64) int64 { +// RoundOffVolSize rounds up given quantity upto chunks of MiB/GiB +func RoundOffVolSize(size int64) int64 { requestBytes := size - return roundUpSize(requestBytes, MiB) + if requestBytes < GiB { + return roundUpSize(requestBytes, MiB) + } + size = roundUpSize(requestBytes, GiB) + // convert size back to MiB for rbd CLI + return size * GiB / MiB +} + +func roundUpSize(volumeSizeBytes, allocationUnitBytes int64) int64 { + roundedUp := volumeSizeBytes / allocationUnitBytes + if volumeSizeBytes%allocationUnitBytes > 0 { + roundedUp++ + } + return roundedUp +} + +// RoundOffBytes converts roundoff the size +// 1.1Mib will be round off to 2Mib same for GiB +// size less than will be round off to 1MiB +func RoundOffBytes(bytes int64) int64 { + var num int64 + floatBytes := float64(bytes) + // round off the value if its in decimal + if floatBytes < GiB { + num = int64(math.Ceil(floatBytes / MiB)) + num *= MiB + } else { + num = int64(math.Ceil(floatBytes / GiB)) + num *= GiB + } + return num } // variables which will be set during the build time @@ -82,14 +114,6 @@ type Config struct { } -func roundUpSize(volumeSizeBytes, allocationUnitBytes int64) int64 { - roundedUp := volumeSizeBytes / allocationUnitBytes - if volumeSizeBytes%allocationUnitBytes > 0 { - roundedUp++ - } - return roundedUp -} - // CreatePersistanceStorage creates storage path and initializes new cache func CreatePersistanceStorage(sPath, metaDataStore, pluginPath string) (CachePersister, error) { var err error diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go new file mode 100644 index 000000000..0a4cd3af2 --- /dev/null +++ b/pkg/util/util_test.go @@ -0,0 +1,144 @@ +/* +Copyright 2019 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package util + +import ( + "testing" +) + +func TestRoundOffBytes(t *testing.T) { + type args struct { + bytes int64 + } + tests := []struct { + name string + args args + want int64 + }{ + { + "1MiB conversions", + args{ + bytes: 1048576, + }, + 1048576, + }, + { + "1000kiB conversion", + args{ + bytes: 1000, + }, + 1048576, // equal to 1MiB + }, + { + "1.5Mib conversion", + args{ + bytes: 1572864, + }, + 2097152, // equal to 2MiB + }, + { + "1.1MiB conversion", + args{ + bytes: 1153434, + }, + 2097152, // equal to 2MiB + }, + { + "1.5GiB conversion", + args{ + bytes: 1610612736, + }, + 2147483648, // equal to 2GiB + }, + { + "1.1GiB conversion", + args{ + bytes: 1181116007, + }, + 2147483648, // equal to 2GiB + }, + } + for _, tt := range tests { + ts := tt + t.Run(ts.name, func(t *testing.T) { + if got := RoundOffBytes(ts.args.bytes); got != ts.want { + t.Errorf("RoundOffBytes() = %v, want %v", got, ts.want) + } + }) + } +} + +func TestRoundOffVolSize(t *testing.T) { + type args struct { + size int64 + } + tests := []struct { + name string + args args + want int64 + }{ + { + "1MiB conversions", + args{ + size: 1048576, + }, + 1, // MiB + }, + { + "1000kiB conversion", + args{ + size: 1000, + }, + 1, // MiB + }, + { + "1.5Mib conversion", + args{ + size: 1572864, + }, + 2, // MiB + }, + { + "1.1MiB conversion", + args{ + size: 1153434, + }, + 2, // MiB + }, + { + "1.5GiB conversion", + args{ + size: 1610612736, + }, + 2048, // MiB + }, + { + "1.1GiB conversion", + args{ + size: 1181116007, + }, + 2048, // MiB + }, + } + for _, tt := range tests { + ts := tt + t.Run(ts.name, func(t *testing.T) { + if got := RoundOffVolSize(ts.args.size); got != ts.want { + t.Errorf("RoundOffVolSize() = %v, want %v", got, ts.want) + } + }) + } +}