From 4908ff874383e630cb300514e86083cbb02063ef Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Tue, 15 Jun 2021 09:23:47 +0200 Subject: [PATCH] rbd: no need to flatten thick-provisioned images Thick-provisioned images are independent, cloned images or snapshots are deep-flattened during creation. There is no need to try and flatten them again. Signed-off-by: Niels de Vos --- internal/rbd/clone.go | 5 +++++ internal/rbd/controllerserver.go | 13 +++++++++---- internal/rbd/controllerserver_test.go | 15 +++++++-------- internal/rbd/nodeserver.go | 2 ++ 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/internal/rbd/clone.go b/internal/rbd/clone.go index f2297c5ba..88d987468 100644 --- a/internal/rbd/clone.go +++ b/internal/rbd/clone.go @@ -264,6 +264,11 @@ func (rv *rbdVolume) doSnapClone(ctx context.Context, parentVol *rbdVolume) erro } func (rv *rbdVolume) flattenCloneImage(ctx context.Context) error { + if rv.ThickProvision { + // thick-provisioned images do not need flattening + return nil + } + tempClone := rv.generateTempClone() // reducing the limit for cloned images to make sure the limit is in range, // If the intermediate clone reaches the depth we may need to return ABORT diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 2fd0e2fb7..9c6481744 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -117,7 +117,7 @@ func (cs *ControllerServer) parseVolCreateRequest(ctx context.Context, req *csi. return nil, status.Error(codes.InvalidArgument, err.Error()) } - rbdVol.ThickProvision = cs.isThickProvisionRequest(req) + rbdVol.ThickProvision = isThickProvisionRequest(req.GetParameters()) rbdVol.RequestName = req.GetName() @@ -343,7 +343,7 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C case vcs == nil: // continue/restart allocating the volume in case it // should be thick-provisioned - if cs.isThickProvisionRequest(req) { + if isThickProvisionRequest(req.GetParameters()) { err := rbdVol.RepairThickProvision() if err != nil { return nil, status.Error(codes.Internal, err.Error()) @@ -379,6 +379,11 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C // are more than the `minSnapshotOnImage` Add a task to flatten all the // temporary cloned images. func flattenTemporaryClonedImages(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) error { + if rbdVol.ThickProvision { + // thick-provisioned images do not need flattening + return nil + } + snaps, err := rbdVol.listSnapshots() if err != nil { if errors.Is(err, ErrImageNotFound) { @@ -1171,10 +1176,10 @@ func (cs *ControllerServer) ControllerExpandVolume(ctx context.Context, req *csi // isThickProvisionRequest returns true in case the request contains the // `thickProvision` option set to `true`. -func (cs *ControllerServer) isThickProvisionRequest(req *csi.CreateVolumeRequest) bool { +func isThickProvisionRequest(parameters map[string]string) bool { tp := "thickProvision" - thick, ok := req.GetParameters()[tp] + thick, ok := parameters[tp] if !ok || thick == "" { return false } diff --git a/internal/rbd/controllerserver_test.go b/internal/rbd/controllerserver_test.go index 32905a53f..509cadf42 100644 --- a/internal/rbd/controllerserver_test.go +++ b/internal/rbd/controllerserver_test.go @@ -23,7 +23,6 @@ import ( ) func TestIsThickProvisionRequest(t *testing.T) { - cs := &ControllerServer{} req := &csi.CreateVolumeRequest{ Name: "fake", Parameters: map[string]string{ @@ -32,38 +31,38 @@ func TestIsThickProvisionRequest(t *testing.T) { } // pass disabled/invalid values for "thickProvision" option - if cs.isThickProvisionRequest(req) { + if isThickProvisionRequest(req.GetParameters()) { t.Error("request is not for thick-provisioning") } req.Parameters["thickProvision"] = "" - if cs.isThickProvisionRequest(req) { + if isThickProvisionRequest(req.GetParameters()) { t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"]) } req.Parameters["thickProvision"] = "false" - if cs.isThickProvisionRequest(req) { + if isThickProvisionRequest(req.GetParameters()) { t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"]) } req.Parameters["thickProvision"] = "off" - if cs.isThickProvisionRequest(req) { + if isThickProvisionRequest(req.GetParameters()) { t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"]) } req.Parameters["thickProvision"] = "no" - if cs.isThickProvisionRequest(req) { + if isThickProvisionRequest(req.GetParameters()) { t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"]) } req.Parameters["thickProvision"] = "**true**" - if cs.isThickProvisionRequest(req) { + if isThickProvisionRequest(req.GetParameters()) { t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"]) } // only "true" should enable thick provisioning req.Parameters["thickProvision"] = "true" - if !cs.isThickProvisionRequest(req) { + if !isThickProvisionRequest(req.GetParameters()) { t.Errorf("request should be for thick-provisioning: %s", req.Parameters["thickProvision"]) } } diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index 4eaa6271a..1f1ce673b 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -171,6 +171,8 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol return nil, status.Error(codes.Internal, err.Error()) } + volOptions.ThickProvision = isThickProvisionRequest(req.GetVolumeContext()) + // get rbd image name from the volume journal // for static volumes, the image name is actually the volume ID itself switch {