From 5d9612c2ae3539f5a135c6b5f555ced1a4ddc135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 24 Nov 2020 10:03:32 +0100 Subject: [PATCH] ceph: fix metadata device passed by-id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code was assuming that devices were passed by the user as "/dev/sda", this is bad! We all know people should be using paths like /dev/disk/by-id so we must support them. Closes: https://github.com/rook/rook/issues/6685 Signed-off-by: Sébastien Han --- pkg/daemon/ceph/osd/volume.go | 12 +++- pkg/daemon/ceph/osd/volume_test.go | 110 +++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/pkg/daemon/ceph/osd/volume.go b/pkg/daemon/ceph/osd/volume.go index 9e8b183d2..390e97feb 100644 --- a/pkg/daemon/ceph/osd/volume.go +++ b/pkg/daemon/ceph/osd/volume.go @@ -582,9 +582,15 @@ func (a *OsdAgent) initializeDevices(context *clusterd.Context, devices *DeviceO } mdArgs = append(mdArgs, strings.Split(conf["devices"], " ")...) + // Do not change device names if udev persistent names are passed + mdPath := md + if !strings.HasPrefix(mdPath, "/dev") { + mdPath = path.Join("/dev", md) + } + mdArgs = append(mdArgs, []string{ dbDeviceFlag, - path.Join("/dev", md), + mdPath, }...) // Reporting @@ -613,8 +619,8 @@ func (a *OsdAgent) initializeDevices(context *clusterd.Context, devices *DeviceO return errors.Wrap(err, "failed to unmarshal ceph-volume report json") } - if path.Join("/dev", md) != cvReport.Vg.Devices { - return errors.Errorf("ceph-volume did not use the expected metadataDevice [%s]", md) + if mdPath != cvReport.Vg.Devices { + return errors.Errorf("ceph-volume did not use the expected metadataDevice [%s]", mdPath) } // execute ceph-volume batching up multiple devices diff --git a/pkg/daemon/ceph/osd/volume_test.go b/pkg/daemon/ceph/osd/volume_test.go index 9d2b02ab8..90abf85d3 100644 --- a/pkg/daemon/ceph/osd/volume_test.go +++ b/pkg/daemon/ceph/osd/volume_test.go @@ -678,6 +678,116 @@ func TestInitializeBlock(t *testing.T) { assert.NoError(t, err, "failed crush device class test") logger.Info("success, go to next test") } + + // Test with metadata devices + { + devices := &DeviceOsdMapping{ + Entries: map[string]*DeviceOsdIDEntry{ + "sda": {Data: -1, Metadata: nil, Config: DesiredDevice{Name: "/dev/sda", 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] == "1" && args[11] == "/dev/sda" && args[12] == "--db-devices" && args[13] == "/dev/sdb" { + return nil + } + + // Second command + if args[9] == "--osds-per-device" && args[10] == "1" && 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.MockExecuteCommandWithCombinedOutput = 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] == "1" && args[11] == "/dev/sda" && args[12] == "--db-devices" && args[13] == "/dev/sdb" { + return `{"vg": {"devices": "/dev/sdb"}}`, nil + } + + return "", errors.Errorf("unknown command %s %s", command, args) + } + a := &OsdAgent{clusterInfo: &cephclient.ClusterInfo{CephVersion: cephver.CephVersion{Major: 14, Minor: 2, Extra: 8}}, nodeName: "node1"} + context := &clusterd.Context{Executor: executor} + + err := a.initializeDevices(context, devices) + assert.NoError(t, err, "failed metadata test") + logger.Info("success, go to next test") + } + + // Test with metadata devices with dev by-id + { + devices := &DeviceOsdMapping{ + Entries: map[string]*DeviceOsdIDEntry{ + "sda": {Data: -1, Metadata: nil, Config: DesiredDevice{Name: "/dev/sda", MetadataDevice: "/dev/disk/by-id/nvme-Samsung_SSD_970_EVO_Plus_1TB_XXX"}}, + }, + } + + 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] == "1" && args[11] == "/dev/sda" && args[12] == "--db-devices" && args[13] == "/dev/disk/by-id/nvme-Samsung_SSD_970_EVO_Plus_1TB_XXX" { + return nil + } + + // Second command + if args[9] == "--osds-per-device" && args[10] == "1" && args[11] == "/dev/sda" && args[12] == "--db-devices" && args[13] == "/dev/disk/by-id/nvme-Samsung_SSD_970_EVO_Plus_1TB_XXX" && args[14] == "--report" { + return nil + } + + return errors.Errorf("unknown command %s %s", command, args) + } + + executor.MockExecuteCommandWithCombinedOutput = 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] == "1" && args[11] == "/dev/sda" && args[12] == "--db-devices" && args[13] == "/dev/disk/by-id/nvme-Samsung_SSD_970_EVO_Plus_1TB_XXX" { + return `{"vg": {"devices": "/dev/disk/by-id/nvme-Samsung_SSD_970_EVO_Plus_1TB_XXX"}}`, nil + } + + return "", errors.Errorf("unknown command %s %s", command, args) + } + a := &OsdAgent{clusterInfo: &cephclient.ClusterInfo{CephVersion: cephver.CephVersion{Major: 14, Minor: 2, Extra: 8}}, nodeName: "node1"} + context := &clusterd.Context{Executor: executor} + + err := a.initializeDevices(context, devices) + assert.NoError(t, err, "failed metadata device by-id test") + logger.Info("success, go to next test") + } } func TestInitializeBlockPVC(t *testing.T) {