From 753bbfe1c421576ffd6bbee12129acb2f2077fcf Mon Sep 17 00:00:00 2001
From: Satoru Takeuchi <satoru.takeuchi@gmail.com>
Date: Fri, 2 Feb 2024 07:39:44 +0000
Subject: [PATCH] osd: correct counting the devices when metadataDevice is set

The validation logic of checking the number of devices is
wrong when `metadataDevice` is set and `osdsPerDevice` > 1.
`len(cvReports)` is the expected number of OSDs and is
the number of specified data devices multiplied
by `osdsPerDevice`.

Closes: https://github.com/rook/rook/issues/13637

Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
---
 pkg/daemon/ceph/osd/volume.go      |  7 +++-
 pkg/daemon/ceph/osd/volume_test.go | 65 +++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/pkg/daemon/ceph/osd/volume.go b/pkg/daemon/ceph/osd/volume.go
index b127f265d0b7..a1e403e182dc 100644
--- a/pkg/daemon/ceph/osd/volume.go
+++ b/pkg/daemon/ceph/osd/volume.go
@@ -722,11 +722,16 @@ func (a *OsdAgent) initializeDevicesLVMMode(context *clusterd.Context, devices *
 	for md, conf := range metadataDevices {
 
 		mdArgs := batchArgs
+		osdsPerDevice := 1
 		if _, ok := conf["osdsperdevice"]; ok {
 			mdArgs = append(mdArgs, []string{
 				osdsPerDeviceFlag,
 				conf["osdsperdevice"],
 			}...)
+			v, _ := strconv.Atoi(conf["osdsperdevice"])
+			if v > 1 {
+				osdsPerDevice = v
+			}
 		}
 		if _, ok := conf["deviceclass"]; ok {
 			mdArgs = append(mdArgs, []string{
@@ -779,7 +784,7 @@ func (a *OsdAgent) initializeDevicesLVMMode(context *clusterd.Context, devices *
 			return errors.Wrap(err, "failed to unmarshal ceph-volume report json")
 		}
 
-		if len(strings.Split(conf["devices"], " ")) != len(cvReports) {
+		if len(strings.Split(conf["devices"], " "))*osdsPerDevice != len(cvReports) {
 			return errors.Errorf("failed to create enough required devices, required: %s, actual: %v", cvOut, cvReports)
 		}
 
diff --git a/pkg/daemon/ceph/osd/volume_test.go b/pkg/daemon/ceph/osd/volume_test.go
index 95366f040b53..46f3ea9d4883 100644
--- a/pkg/daemon/ceph/osd/volume_test.go
+++ b/pkg/daemon/ceph/osd/volume_test.go
@@ -769,7 +769,7 @@ func TestInitializeBlock(t *testing.T) {
 		logger.Info("success, go to next test")
 	}
 
-	// Test with metadata devices
+	// Test with metadata device and one osd per device
 	{
 		devices := &DeviceOsdMapping{
 			Entries: map[string]*DeviceOsdIDEntry{
@@ -832,6 +832,69 @@ func TestInitializeBlock(t *testing.T) {
 		}
 	}
 
+	// Test with metadata device and multiple osds per device
+	{
+		devices := &DeviceOsdMapping{
+			Entries: map[string]*DeviceOsdIDEntry{
+				"sda": {Data: -1, Metadata: nil, Config: DesiredDevice{Name: "/dev/sda", OSDsPerDevice: 2, MetadataDevice: "sdb"}},
+			},
+		}
+
+		executor := &exectest.MockExecutor{}
+		executor.MockExecuteCommand = func(command string, args ...string) error {
+			logger.Infof("%s %v", command, args)
+
+			// Validate base common args
+			err := testBaseArgs(args)
+			if err != nil {
+				return err
+			}
+
+			// First command
+			if args[9] == "--osds-per-device" && args[10] == "2" && args[11] == "/dev/sda" && args[12] == "--db-devices" && args[13] == "/dev/sdb" {
+				return nil
+			}
+
+			// Second command
+			if args[9] == "--osds-per-device" && args[10] == "2" && args[11] == "/dev/sda" && args[12] == "--db-devices" && args[13] == "/dev/sdb" && args[14] == "--report" {
+				return nil
+			}
+
+			return errors.Errorf("unknown command %s %s", command, args)
+		}
+
+		executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
+			logger.Infof("%s %v", command, args)
+
+			// Validate base common args
+			err := testBaseArgs(args)
+			if err != nil {
+				return "", err
+			}
+
+			// First command
+			if args[9] == "--osds-per-device" && args[10] == "2" && args[11] == "/dev/sda" && args[12] == "--db-devices" && args[13] == "/dev/sdb" {
+				return fmt.Sprintf(`[{"block_db": "%s", "data": "%s"}, {"block_db": "%s", "data": "%s"}]`, "/dev/sdb", "/dev/sda", "/dev/sdb", "/dev/sda"), nil
+			}
+
+			return "", errors.Errorf("unknown command %s %s", command, args)
+		}
+		a := &OsdAgent{clusterInfo: &cephclient.ClusterInfo{CephVersion: cephver.CephVersion{Major: 16, Minor: 2, Extra: 0}}, nodeName: "node1", storeConfig: config.StoreConfig{StoreType: "bluestore"}}
+		context := &clusterd.Context{
+			Executor: executor,
+			Devices: []*sys.LocalDisk{
+				{Name: "sda"}, {Name: "sdb"},
+			},
+		}
+
+		err := a.initializeDevicesLVMMode(context, devices)
+		if err != nil {
+			assert.NoError(t, err, "failed metadata test")
+		} else {
+			logger.Info("success, go to next test")
+		}
+	}
+
 	// Test with metadata devices with dev by-id
 	{
 		metadataDeviceByIDPath := "/dev/disk/by-id/nvme-Samsung_SSD_970_EVO_Plus_1TB_XXX"