Skip to content

Commit

Permalink
osd: correct counting the devices when metadataDevice is set
Browse files Browse the repository at this point in the history
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: rook#13637

Signed-off-by: Satoru Takeuchi <[email protected]>
  • Loading branch information
satoru-takeuchi committed Feb 7, 2024
1 parent 4e0c4f6 commit 753bbfe
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 2 deletions.
7 changes: 6 additions & 1 deletion pkg/daemon/ceph/osd/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}

Expand Down
65 changes: 64 additions & 1 deletion pkg/daemon/ceph/osd/volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 753bbfe

Please sign in to comment.