cleanup: embed args into struct and pass it to detachRBDImageOrDeviceSpec

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

internal/rbd/nodeserver.go

@@ -819,13 +819,16 @@ func (ns *NodeServer) NodeUnstageVolume(
 
 	// Unmapping rbd device
 	imageSpec := imgInfo.String()
-	if err = detachRBDImageOrDeviceSpec(
+
-		ctx, imageSpec,
+	dArgs := detachRBDImageArgs{
-		true,
+		imageOrDeviceSpec: imageSpec,
-		imgInfo.NbdAccess,
+		isImageSpec:       true,
-		imgInfo.Encrypted,
+		isNbd:             imgInfo.NbdAccess,
-		req.GetVolumeId(),
+		encrypted:         imgInfo.Encrypted,
-		imgInfo.UnmapOptions); err != nil {
+		volumeID:          req.GetVolumeId(),
+		unmapOptions:      imgInfo.UnmapOptions,
+	}
+	if err = detachRBDImageOrDeviceSpec(ctx, dArgs); err != nil {
 		util.ErrorLog(
 			ctx,
 			"error unmapping volume (%s) from staging path (%s): (%v)",

internal/rbd/rbd_attach.go

@@ -81,6 +81,15 @@ type nbdDeviceInfo struct {
 	Device         string `json:"device"`
 }
 
+type detachRBDImageArgs struct {
+	imageOrDeviceSpec string
+	isImageSpec       bool
+	isNbd             bool
+	encrypted         bool
+	volumeID          string
+	unmapOptions      string
+}
+
 // rbdGetDeviceList queries rbd about mapped devices and returns a list of rbdDeviceInfo
 // It will selectively list devices mapped using krbd or nbd as specified by accessType.
 func rbdGetDeviceList(ctx context.Context, accessType string) ([]rbdDeviceInfo, error) {
@@ -325,14 +334,15 @@ func createPath(ctx context.Context, volOpt *rbdVolume, device string, cr *util.
 		util.WarningLog(ctx, "rbd: map error %v, rbd output: %s", err, stderr)
 		// unmap rbd image if connection timeout
 		if strings.Contains(err.Error(), rbdMapConnectionTimeout) {
-			detErr := detachRBDImageOrDeviceSpec(
+			dArgs := detachRBDImageArgs{
-				ctx,
+				imageOrDeviceSpec: imagePath,
-				imagePath,
+				isImageSpec:       true,
-				true,
+				isNbd:             isNbd,
-				isNbd,
+				encrypted:         volOpt.isEncrypted(),
-				volOpt.isEncrypted(),
+				volumeID:          volOpt.VolID,
-				volOpt.VolID,
+				unmapOptions:      volOpt.UnmapOptions,
-				volOpt.UnmapOptions)
+			}
+			detErr := detachRBDImageOrDeviceSpec(ctx, dArgs)
 			if detErr != nil {
 				util.WarningLog(ctx, "rbd: %s unmap error %v", imagePath, detErr)
 			}
@@ -375,22 +385,29 @@ func detachRBDDevice(ctx context.Context, devicePath, volumeID, unmapOptions str
 		nbdType = true
 	}
 
-	return detachRBDImageOrDeviceSpec(ctx, devicePath, false, nbdType, encrypted, volumeID, unmapOptions)
+	dArgs := detachRBDImageArgs{
+		imageOrDeviceSpec: devicePath,
+		isImageSpec:       false,
+		isNbd:             nbdType,
+		encrypted:         encrypted,
+		volumeID:          volumeID,
+		unmapOptions:      unmapOptions,
+	}
+
+	return detachRBDImageOrDeviceSpec(ctx, dArgs)
 }
 
 // detachRBDImageOrDeviceSpec detaches an rbd imageSpec or devicePath, with additional checking
 // when imageSpec is used to decide if image is already unmapped.
 func detachRBDImageOrDeviceSpec(
 	ctx context.Context,
-	imageOrDeviceSpec string,
+	dArgs detachRBDImageArgs) error {
-	isImageSpec, isNbd, encrypted bool,
+	if dArgs.encrypted {
-	volumeID, unmapOptions string) error {
+		mapperFile, mapperPath := util.VolumeMapper(dArgs.volumeID)
-	if encrypted {
-		mapperFile, mapperPath := util.VolumeMapper(volumeID)
 		mappedDevice, mapper, err := util.DeviceEncryptionStatus(ctx, mapperPath)
 		if err != nil {
 			util.ErrorLog(ctx, "error determining LUKS device on %s, %s: %s",
-				mapperPath, imageOrDeviceSpec, err)
+				mapperPath, dArgs.imageOrDeviceSpec, err)
 
 			return err
 		}
@@ -399,31 +416,31 @@ func detachRBDImageOrDeviceSpec(
 			err = util.CloseEncryptedVolume(ctx, mapperFile)
 			if err != nil {
 				util.ErrorLog(ctx, "error closing LUKS device on %s, %s: %s",
-					mapperPath, imageOrDeviceSpec, err)
+					mapperPath, dArgs.imageOrDeviceSpec, err)
 
 				return err
 			}
-			imageOrDeviceSpec = mappedDevice
+			dArgs.imageOrDeviceSpec = mappedDevice
 		}
 	}
 
-	unmapArgs := []string{"unmap", imageOrDeviceSpec}
+	unmapArgs := []string{"unmap", dArgs.imageOrDeviceSpec}
-	unmapArgs = appendDeviceTypeAndOptions(unmapArgs, isNbd, false, unmapOptions)
+	unmapArgs = appendDeviceTypeAndOptions(unmapArgs, dArgs.isNbd, false, dArgs.unmapOptions)
 
 	_, stderr, err := util.ExecCommand(ctx, rbd, unmapArgs...)
 	if err != nil {
 		// Messages for krbd and nbd differ, hence checking either of them for missing mapping
 		// This is not applicable when a device path is passed in
-		if isImageSpec &&
+		if dArgs.isImageSpec &&
-			(strings.Contains(stderr, fmt.Sprintf(rbdUnmapCmdkRbdMissingMap, imageOrDeviceSpec)) ||
+			(strings.Contains(stderr, fmt.Sprintf(rbdUnmapCmdkRbdMissingMap, dArgs.imageOrDeviceSpec)) ||
-				strings.Contains(stderr, fmt.Sprintf(rbdUnmapCmdNbdMissingMap, imageOrDeviceSpec))) {
+				strings.Contains(stderr, fmt.Sprintf(rbdUnmapCmdNbdMissingMap, dArgs.imageOrDeviceSpec))) {
 			// Devices found not to be mapped are treated as a successful detach
-			util.TraceLog(ctx, "image or device spec (%s) not mapped", imageOrDeviceSpec)
+			util.TraceLog(ctx, "image or device spec (%s) not mapped", dArgs.imageOrDeviceSpec)
 
 			return nil
 		}
 
-		return fmt.Errorf("rbd: unmap for spec (%s) failed (%w): (%s)", imageOrDeviceSpec, err, stderr)
+		return fmt.Errorf("rbd: unmap for spec (%s) failed (%w): (%s)", dArgs.imageOrDeviceSpec, err, stderr)
 	}
 
 	return nil