cleanup: refactor deeply nested if statements in internal/rbd

Refactored deeply nested if statement in internal/rbd to
reduce cognitive complexity.

Signed-off-by: Rakshith R <rar@redhat.com>
This commit is contained in:
Rakshith R 2021-04-05 10:10:00 +05:30 committed by mergify[bot]
parent d4cfd7bef9
commit 020cded581
5 changed files with 79 additions and 77 deletions

View File

@ -74,7 +74,8 @@ func (rv *rbdVolume) checkCloneImage(ctx context.Context, parentVol *rbdVolume)
err = tempClone.checkSnapExists(snap) err = tempClone.checkSnapExists(snap)
if err != nil { if err != nil {
if errors.Is(err, ErrSnapNotFound) { switch {
case errors.Is(err, ErrSnapNotFound):
// check temporary image needs flatten, if yes add task to flatten the // check temporary image needs flatten, if yes add task to flatten the
// temporary clone // temporary clone
err = tempClone.flattenRbdImage(ctx, rv.conn.Creds, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth) err = tempClone.flattenRbdImage(ctx, rv.conn.Creds, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth)
@ -88,7 +89,7 @@ func (rv *rbdVolume) checkCloneImage(ctx context.Context, parentVol *rbdVolume)
return false, err return false, err
} }
return true, nil return true, nil
} else if !errors.Is(err, ErrImageNotFound) { case !errors.Is(err, ErrImageNotFound):
// any error other than image not found return error // any error other than image not found return error
return false, err return false, err
} }

View File

@ -809,7 +809,8 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
defer vol.Destroy() defer vol.Destroy()
err = vol.flattenRbdImage(ctx, cr, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth) err = vol.flattenRbdImage(ctx, cr, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth)
if errors.Is(err, ErrFlattenInProgress) { switch {
case errors.Is(err, ErrFlattenInProgress):
return &csi.CreateSnapshotResponse{ return &csi.CreateSnapshotResponse{
Snapshot: &csi.Snapshot{ Snapshot: &csi.Snapshot{
SizeBytes: rbdSnap.SizeBytes, SizeBytes: rbdSnap.SizeBytes,
@ -819,15 +820,13 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
ReadyToUse: false, ReadyToUse: false,
}, },
}, nil }, nil
} case err != nil:
if err != nil {
uErr := undoSnapshotCloning(ctx, vol, rbdSnap, vol, cr) uErr := undoSnapshotCloning(ctx, vol, rbdSnap, vol, cr)
if uErr != nil { if uErr != nil {
util.WarningLog(ctx, "failed undoing reservation of snapshot: %s %v", req.GetName(), uErr) util.WarningLog(ctx, "failed undoing reservation of snapshot: %s %v", req.GetName(), uErr)
} }
return nil, status.Errorf(codes.Internal, err.Error()) return nil, status.Errorf(codes.Internal, err.Error())
} default:
return &csi.CreateSnapshotResponse{ return &csi.CreateSnapshotResponse{
Snapshot: &csi.Snapshot{ Snapshot: &csi.Snapshot{
SizeBytes: rbdSnap.SizeBytes, SizeBytes: rbdSnap.SizeBytes,
@ -838,6 +837,7 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
}, },
}, nil }, nil
} }
}
err = flattenTemporaryClonedImages(ctx, rbdVol, cr) err = flattenTemporaryClonedImages(ctx, rbdVol, cr)
if err != nil { if err != nil {

View File

@ -267,8 +267,7 @@ func (ns *NodeServer) stageTransaction(ctx context.Context, req *csi.NodeStageVo
return transaction, err return transaction, err
} }
} }
if !util.CheckKernelSupport(kernelRelease, deepFlattenSupport) { if !util.CheckKernelSupport(kernelRelease, deepFlattenSupport) && !skipForceFlatten {
if !skipForceFlatten {
feature, err = volOptions.checkImageChainHasFeature(ctx, librbd.FeatureDeepFlatten) feature, err = volOptions.checkImageChainHasFeature(ctx, librbd.FeatureDeepFlatten)
if err != nil { if err != nil {
return transaction, err return transaction, err
@ -280,7 +279,6 @@ func (ns *NodeServer) stageTransaction(ctx context.Context, req *csi.NodeStageVo
} }
} }
} }
}
// Mapping RBD image // Mapping RBD image
var devicePath string var devicePath string
devicePath, err = attachRBDImage(ctx, volOptions, cr) devicePath, err = attachRBDImage(ctx, volOptions, cr)
@ -468,9 +466,10 @@ func (ns *NodeServer) mountVolumeToStagePath(ctx context.Context, req *csi.NodeS
if existingFormat == "" && !staticVol && !readOnly { if existingFormat == "" && !staticVol && !readOnly {
args := []string{} args := []string{}
if fsType == "ext4" { switch fsType {
case "ext4":
args = []string{"-m0", "-Enodiscard,lazy_itable_init=1,lazy_journal_init=1", devicePath} args = []string{"-m0", "-Enodiscard,lazy_itable_init=1,lazy_journal_init=1", devicePath}
} else if fsType == "xfs" { case "xfs":
args = []string{"-K", devicePath} args = []string{"-K", devicePath}
// always disable reflink // always disable reflink
// TODO: make enabling an option, see ceph/ceph-csi#1256 // TODO: make enabling an option, see ceph/ceph-csi#1256
@ -531,8 +530,12 @@ func (ns *NodeServer) mountVolume(ctx context.Context, stagingPath string, req *
func (ns *NodeServer) createTargetMountPath(ctx context.Context, mountPath string, isBlock bool) (bool, error) { func (ns *NodeServer) createTargetMountPath(ctx context.Context, mountPath string, isBlock bool) (bool, error) {
// Check if that mount path exists properly // Check if that mount path exists properly
notMnt, err := mount.IsNotMountPoint(ns.mounter, mountPath) notMnt, err := mount.IsNotMountPoint(ns.mounter, mountPath)
if err != nil { if err == nil {
if os.IsNotExist(err) { return notMnt, err
}
if !os.IsNotExist(err) {
return false, status.Error(codes.Internal, err.Error())
}
if isBlock { if isBlock {
// #nosec // #nosec
pathFile, e := os.OpenFile(mountPath, os.O_CREATE|os.O_RDWR, 0750) pathFile, e := os.OpenFile(mountPath, os.O_CREATE|os.O_RDWR, 0750)
@ -545,16 +548,12 @@ func (ns *NodeServer) createTargetMountPath(ctx context.Context, mountPath strin
return notMnt, status.Error(codes.Internal, err.Error()) return notMnt, status.Error(codes.Internal, err.Error())
} }
} else { } else {
// Create a directory // Create a mountpath directory
if err = util.CreateMountPoint(mountPath); err != nil { if err = util.CreateMountPoint(mountPath); err != nil {
return notMnt, status.Error(codes.Internal, err.Error()) return notMnt, status.Error(codes.Internal, err.Error())
} }
} }
notMnt = true notMnt = true
} else {
return false, status.Error(codes.Internal, err.Error())
}
}
return notMnt, err return notMnt, err
} }

View File

@ -277,10 +277,10 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er
// Need to check cloned info here not on createvolume, // Need to check cloned info here not on createvolume,
if parentVol != nil { if parentVol != nil {
found, cErr := rv.checkCloneImage(ctx, parentVol) found, cErr := rv.checkCloneImage(ctx, parentVol)
if found && cErr == nil { switch {
case found && cErr == nil:
return true, nil return true, nil
} case cErr != nil:
if cErr != nil {
return false, cErr return false, cErr
} }
} }

View File

@ -605,7 +605,9 @@ func (rv *rbdVolume) flattenRbdImage(ctx context.Context, cr *util.Credentials,
util.ExtendedLog(ctx, "clone depth is (%d), configured softlimit (%d) and hardlimit (%d) for %s", depth, softlimit, hardlimit, rv) util.ExtendedLog(ctx, "clone depth is (%d), configured softlimit (%d) and hardlimit (%d) for %s", depth, softlimit, hardlimit, rv)
} }
if forceFlatten || (depth >= hardlimit) || (depth >= softlimit) { if !forceFlatten && (depth < hardlimit) && (depth < softlimit) {
return nil
}
args := []string{"flatten", rv.String(), "--id", cr.ID, "--keyfile=" + cr.KeyFile, "-m", rv.Monitors} args := []string{"flatten", rv.String(), "--id", cr.ID, "--keyfile=" + cr.KeyFile, "-m", rv.Monitors}
supported, err := addRbdManagerTask(ctx, rv, args) supported, err := addRbdManagerTask(ctx, rv, args)
if supported { if supported {
@ -636,7 +638,7 @@ func (rv *rbdVolume) flattenRbdImage(ctx context.Context, cr *util.Credentials,
} }
} }
} }
}
return nil return nil
} }