rbd: avoid supplying map options on unmap

Thanks to the random unmap failure on my local machine:

I0901 17:08:37.841890 2617035 cephcmds.go:55] ID: 11 Req-ID:
0001-0024-fed5480a-f00f-417a-a51d-31d8a8144c03-0000000000000003-024983f3-0b47-11ec-8fcb-e671f0b9f58e
an error (exit status 22) occurred while running rbd args: [unmap
rbd-pool/csi-vol-024983f3-0b47-11ec-8fcb-e671f0b9f58e --device-type nbd
--options try-netlink --options reattach-timeout=300 --options
io-timeout=0]

Noticed the map args are also getting passed to/as unmap args, which is not
correct. We have separate things for mapOptions and unmapOptions. This PR
makes sure that the map args are not passed at the time of unmap.

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

internal/rbd/rbd_attach.go

@@ -243,19 +243,11 @@ func attachRBDImage(ctx context.Context, volOptions *rbdVolume, device string, c
 	return devicePath, err
 }
 
-func appendDeviceTypeAndOptions(cmdArgs []string, isNbd, isThick bool, userOptions string) []string {
+func appendNbdDeviceTypeAndOptions(cmdArgs []string, isThick bool, userOptions string) []string {
-	accessType := accessTypeKRbd
+	cmdArgs = append(cmdArgs, "--device-type", accessTypeNbd)
-	if isNbd {
-		accessType = accessTypeNbd
-	}
 
-	cmdArgs = append(cmdArgs, "--device-type", accessType)
+	isUnmap := CheckSliceContains(cmdArgs, "unmap")
-	if !isNbd {
+	if !isUnmap {
-		// Enable mapping and unmapping images from a non-initial network
-		// namespace (e.g. for Multus CNI).  The network namespace must be
-		// owned by the initial user namespace.
-		cmdArgs = append(cmdArgs, "--options", "noudev")
-	} else {
 		if !strings.Contains(userOptions, useNbdNetlink) {
 			cmdArgs = append(cmdArgs, "--options", useNbdNetlink)
 		}
@@ -265,12 +257,40 @@ func appendDeviceTypeAndOptions(cmdArgs []string, isNbd, isThick bool, userOptio
 		if !strings.Contains(userOptions, setNbdIOTimeout) {
 			cmdArgs = append(cmdArgs, "--options", fmt.Sprintf("%s=%d", setNbdIOTimeout, defaultNbdIOTimeout))
 		}
+
+		if isThick {
+			// When an image is thick-provisioned, any discard/unmap/trim
+			// requests should not free extents.
+			cmdArgs = append(cmdArgs, "--options", "notrim")
+		}
 	}
-	if isThick {
+
-		// When an image is thick-provisioned, any discard/unmap/trim
+	if userOptions != "" {
-		// requests should not free extents.
+		// userOptions is appended after, possibly overriding the above
-		cmdArgs = append(cmdArgs, "--options", "notrim")
+		// default options.
+		cmdArgs = append(cmdArgs, "--options", userOptions)
 	}
+
+	return cmdArgs
+}
+
+func appendKRbdDeviceTypeAndOptions(cmdArgs []string, isThick bool, userOptions string) []string {
+	cmdArgs = append(cmdArgs, "--device-type", accessTypeKRbd)
+
+	isUnmap := CheckSliceContains(cmdArgs, "unmap")
+	if !isUnmap {
+		if isThick {
+			// When an image is thick-provisioned, any discard/unmap/trim
+			// requests should not free extents.
+			cmdArgs = append(cmdArgs, "--options", "notrim")
+		}
+	}
+
+	// Enable mapping and unmapping images from a non-initial network
+	// namespace (e.g. for Multus CNI).  The network namespace must be
+	// owned by the initial user namespace.
+	cmdArgs = append(cmdArgs, "--options", "noudev")
+
 	if userOptions != "" {
 		// userOptions is appended after, possibly overriding the above
 		// default options.
@@ -338,7 +358,11 @@ func createPath(ctx context.Context, volOpt *rbdVolume, device string, cr *util.
 		mapArgs = appendRbdNbdCliOptions(mapArgs, volOpt.MapOptions)
 	} else {
 		mapArgs = append(mapArgs, "map", imagePath)
-		mapArgs = appendDeviceTypeAndOptions(mapArgs, isNbd, isThick, volOpt.MapOptions)
+		if isNbd {
+			mapArgs = appendNbdDeviceTypeAndOptions(mapArgs, isThick, volOpt.MapOptions)
+		} else {
+			mapArgs = appendKRbdDeviceTypeAndOptions(mapArgs, isThick, volOpt.MapOptions)
+		}
 	}
 
 	if volOpt.readOnly {
@@ -443,7 +467,11 @@ func detachRBDImageOrDeviceSpec(
 	}
 
 	unmapArgs := []string{"unmap", dArgs.imageOrDeviceSpec}
-	unmapArgs = appendDeviceTypeAndOptions(unmapArgs, dArgs.isNbd, false, dArgs.unmapOptions)
+	if dArgs.isNbd {
+		unmapArgs = appendNbdDeviceTypeAndOptions(unmapArgs, false, dArgs.unmapOptions)
+	} else {
+		unmapArgs = appendKRbdDeviceTypeAndOptions(unmapArgs, false, dArgs.unmapOptions)
+	}
 
 	_, stderr, err := util.ExecCommand(ctx, rbd, unmapArgs...)
 	if err != nil {

internal/rbd/rbd_util.go

@@ -2011,3 +2011,14 @@ func getCephClientLogFileName(id, logDir, prefix string) string {
 
 	return fmt.Sprintf("%s/%s-%s.log", logDir, prefix, id)
 }
+
+// CheckSliceContains checks the slice for string.
+func CheckSliceContains(options []string, opt string) bool {
+	for _, o := range options {
+		if o == opt {
+			return true
+		}
+	}
+
+	return false
+}