From 83cc1b0e5804b0bcd04bd5433891fa505fc51ab8 Mon Sep 17 00:00:00 2001 From: Prasanna Kumar Kalever Date: Fri, 13 May 2022 12:32:11 +0530 Subject: [PATCH] rbd: handle when krbdFeatures is zero krbdFeatures is set to zero when kernel version < 3.8, i.e. in case where /sys/bus/rbd/supported_features is absent and we are unable to prepare the krbd attributes based on kernel version. When krbdFeatures is set to zero fallback to NBD only when autofallback is turned ON. Fixes: #2678 Signed-off-by: Prasanna Kumar Kalever --- internal/rbd/nodeserver.go | 12 ++++++++++-- internal/rbd/rbd_util.go | 9 +++++++-- internal/rbd/rbd_util_test.go | 10 ++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index 64798deb5..ddc84cba1 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -144,6 +144,7 @@ func healerStageTransaction(ctx context.Context, cr *util.Credentials, volOps *r // 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 // like genVolFromVolumeOptions() make use of secrets. +// nolint:gocyclo,cyclop // reduce complexity func populateRbdVol( ctx context.Context, req *csi.NodeStageVolumeRequest, @@ -241,8 +242,15 @@ func populateRbdVol( return nil, status.Error(codes.Internal, err.Error()) } - if rv.Mounter == rbdDefaultMounter && - !isKrbdFeatureSupported(ctx, strings.Join(rv.ImageFeatureSet.Names(), ",")) { + features := strings.Join(rv.ImageFeatureSet.Names(), ",") + isFeatureExist, err := isKrbdFeatureSupported(ctx, features) + if err != nil && !errors.Is(err, os.ErrNotExist) { + log.ErrorLog(ctx, "failed checking krbd features %q: %v", features, err) + + return nil, status.Error(codes.Internal, err.Error()) + } + + if rv.Mounter == rbdDefaultMounter && !isFeatureExist { if !parseBoolOption(ctx, req.GetVolumeContext(), tryOtherMounters, false) { log.ErrorLog(ctx, "unsupported krbd Feature, set `tryOtherMounters:true` or fix krbd driver") err = errors.New("unsupported krbd Feature") diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 8e4538bb9..ce60e3ee3 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -328,7 +328,12 @@ func HexStringToInteger(hexString string) (uint, error) { // isKrbdFeatureSupported checks if a given Image Feature is supported by krbd // driver or not. -func isKrbdFeatureSupported(ctx context.Context, imageFeatures string) bool { +func isKrbdFeatureSupported(ctx context.Context, imageFeatures string) (bool, error) { + // return false when /sys/bus/rbd/supported_features is absent and we are + // not in a position to prepare krbd feature attributes, i.e. if kernel <= 3.8 + if krbdFeatures == 0 { + return false, os.ErrNotExist + } arr := strings.Split(imageFeatures, ",") log.UsefulLog(ctx, "checking for ImageFeatures: %v", arr) imageFeatureSet := librbd.FeatureSetFromNames(arr) @@ -343,7 +348,7 @@ func isKrbdFeatureSupported(ctx context.Context, imageFeatures string) bool { } } - return supported + return supported, nil } // Connect an rbdVolume to the Ceph cluster. diff --git a/internal/rbd/rbd_util_test.go b/internal/rbd/rbd_util_test.go index 8ab310da0..e373a9e61 100644 --- a/internal/rbd/rbd_util_test.go +++ b/internal/rbd/rbd_util_test.go @@ -18,6 +18,7 @@ package rbd import ( "context" + "errors" "io/ioutil" "os" "strings" @@ -346,8 +347,13 @@ func TestIsKrbdFeatureSupported(t *testing.T) { if err != nil { t.Errorf("HexStringToInteger(%s) failed", krbdSupportedFeaturesAttr) } - supported := isKrbdFeatureSupported(ctx, tc.featureName) - if supported != tc.isSupported { + // In case /sys/bus/rbd/supported_features is absent and we are + // not in a position to prepare krbd feature attributes, + // isKrbdFeatureSupported returns error ErrNotExist + supported, err := isKrbdFeatureSupported(ctx, tc.featureName) + if err != nil && !errors.Is(err, os.ErrNotExist) { + t.Errorf("isKrbdFeatureSupported(%s) returned error: %v", tc.featureName, err) + } else if supported != tc.isSupported { t.Errorf("isKrbdFeatureSupported(%s) returned supported status, expected: %t, got: %t", tc.featureName, tc.isSupported, supported) }