From 0ac976295e8a2ffbb35151b9f235e37a0c07c33d Mon Sep 17 00:00:00 2001 From: Prasanna Kumar Kalever Date: Tue, 16 Nov 2021 18:40:11 +0530 Subject: [PATCH] rbd: provide a way to supply mounter specific mapOptions from sc Uses the below schema to supply mounter specific map/unmapOptions to the nodeplugin based on the discussion we all had at https://github.com/ceph/ceph-csi/pull/2636 This should specifically be really helpful with the `tryOthermonters` set to true, i.e with fallback mechanism settings turned ON. mapOption: "kbrd:v1,v2,v3;nbd:v1,v2,v3" - By omitting `krbd:` or `nbd:`, the option(s) apply to rbdDefaultMounter which is krbd. - A user can _override_ the options for a mounter by specifying `krbd:` or `nbd:`. mapOption: "v1,v2,v3;nbd:v1,v2,v3" is effectively the same as the 1st example. - Sections are split by `;`. - If users want to specify common options for both `krbd` and `nbd`, they should mention them twice. Signed-off-by: Prasanna Kumar Kalever --- internal/rbd/nodeserver.go | 70 +++++++++++++++++++++++++++++-- internal/rbd/nodeserver_test.go | 73 +++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 3 deletions(-) diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index 0425278f72c2..4c766e27e4cc 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -100,6 +100,9 @@ var ( // xfsHasReflink is set by xfsSupportsReflink(), use the function when // checking the support for reflink. xfsHasReflink = xfsReflinkUnset + + errMounterNotFound = errors.New("unknown mounter type") + errBadlyFormattedOpts = errors.New("badly formatted options") ) // parseBoolOption checks if parameters contain option and parse it. If it is @@ -140,6 +143,65 @@ func healerStageTransaction(ctx context.Context, cr *util.Credentials, volOps *r return nil } +// parseMapOptions helps parse formatted mapOptions and unmapOptions and +// returns mounter specific options. +func parseMapOptions(mapOptions string) (string, string, error) { + var krbdMapOptions, nbdMapOptions string + const ( + noKeyLength = 1 + validLength = 2 + ) + for _, item := range strings.Split(mapOptions, ";") { + var mounter, options string + if item == "" { + continue + } + s := strings.Split(item, ":") + switch len(s) { + case noKeyLength: + options = strings.TrimSpace(s[0]) + krbdMapOptions = options + case validLength: + mounter = strings.TrimSpace(s[0]) + options = strings.TrimSpace(s[1]) + switch strings.ToLower(mounter) { + case accessTypeKRbd: + krbdMapOptions = options + case accessTypeNbd: + nbdMapOptions = options + default: + return "", "", errMounterNotFound + } + default: + return "", "", errBadlyFormattedOpts + } + } + + return krbdMapOptions, nbdMapOptions, nil +} + +// getMapOptions is a wrapper func, calls parse map/unmap funcs and feeds the +// rbdVolume object. +func getMapOptions(req *csi.NodeStageVolumeRequest, rv *rbdVolume) error { + krbdMapOptions, nbdMapOptions, err := parseMapOptions(req.GetVolumeContext()["mapOptions"]) + if err != nil { + return err + } + krbdUnmapOptions, nbdUnmapOptions, err := parseMapOptions(req.GetVolumeContext()["unmapOptions"]) + if err != nil { + return err + } + if rv.Mounter == rbdDefaultMounter { + rv.MapOptions = krbdMapOptions + rv.UnmapOptions = krbdUnmapOptions + } else if rv.Mounter == rbdNbdMounter { + rv.MapOptions = nbdMapOptions + rv.UnmapOptions = nbdUnmapOptions + } + + return nil +} + // populateRbdVol update the fields in rbdVolume struct based on the request it received. // this function also receive the credentials and secrets args as it differs in its data. // The credentials are used directly by functions like voljournal.Connect() and other functions @@ -224,12 +286,14 @@ func populateRbdVol( return nil, status.Errorf(codes.Internal, "unsupported krbd Feature") } // fallback to rbd-nbd, - // ignore the mapOptions and unmapOptions as they are meant for krbd use. rv.Mounter = rbdNbdMounter } else { rv.Mounter = req.GetVolumeContext()["mounter"] - rv.MapOptions = req.GetVolumeContext()["mapOptions"] - rv.UnmapOptions = req.GetVolumeContext()["unmapOptions"] + } + + err = getMapOptions(req, rv) + if err != nil { + return nil, err } rv.VolID = volID diff --git a/internal/rbd/nodeserver_test.go b/internal/rbd/nodeserver_test.go index 871a6ec76d8f..ab0ce2a4811f 100644 --- a/internal/rbd/nodeserver_test.go +++ b/internal/rbd/nodeserver_test.go @@ -18,6 +18,7 @@ package rbd import ( "context" + "errors" "testing" "github.com/container-storage-interface/spec/lib/go/csi" @@ -105,3 +106,75 @@ func TestParseBoolOption(t *testing.T) { } } } + +func TestParseMapOptions(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + mapOption string + expectKrbdOptions string + expectNbdOptions string + expectErr error + }{ + { + name: "with old format", + mapOption: "kOp1,kOp2", + expectKrbdOptions: "kOp1,kOp2", + expectNbdOptions: "", + expectErr: nil, + }, + { + name: "with new format", + mapOption: "krbd:kOp1,kOp2;nbd:nOp1,nOp2", + expectKrbdOptions: "kOp1,kOp2", + expectNbdOptions: "nOp1,nOp2", + expectErr: nil, + }, + { + name: "without krbd: label", + mapOption: "kOp1,kOp2;nbd:nOp1,nOp2", + expectKrbdOptions: "kOp1,kOp2", + expectNbdOptions: "nOp1,nOp2", + expectErr: nil, + }, + { + name: "with only nbd label", + mapOption: "nbd:nOp1,nOp2", + expectKrbdOptions: "", + expectNbdOptions: "nOp1,nOp2", + expectErr: nil, + }, + { + name: "unknown mounter used", + mapOption: "xyz:xOp1,xOp2", + expectKrbdOptions: "", + expectNbdOptions: "", + expectErr: errMounterNotFound, + }, + { + name: "bad formatted options", + mapOption: "nbd:nOp1:nOp2;", + expectKrbdOptions: "", + expectNbdOptions: "", + expectErr: errBadlyFormattedOpts, + }, + } + for _, tt := range tests { + tc := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + krbdOpts, nbdOpts, err := parseMapOptions(tc.mapOption) + if krbdOpts != tc.expectKrbdOptions || nbdOpts != tc.expectNbdOptions || !errors.Is(err, tc.expectErr) { + t.Errorf("parseMapOptions(%s) "+ + "krbdOpts => [expected :%q, got: %q]; "+ + "nbdOpts => [expected :%q, got: %q]; "+ + "err => [exp: %v, got: %v];", + tc.mapOption, + tc.expectKrbdOptions, krbdOpts, + tc.expectNbdOptions, nbdOpts, + tc.expectErr, err) + } + }) + } +}