Skip to content

Commit 5999196

Browse files
committed
cephadm: remove restriction for crush device classes
A restriction has been introduced here (ceph@6c6cb2f) which doesn't let OSDs be created with custom crush device classes. Crush Device Class is the key that helps the crush distinguish between multiple storage classes, so it must accept any custom names. Fixes: https://tracker.ceph.com/issues/64382 Signed-off-by: Seena Fallah <[email protected]>
1 parent 4a87939 commit 5999196

File tree

2 files changed

+21
-59
lines changed

2 files changed

+21
-59
lines changed

src/python-common/ceph/deployment/translate.py

Lines changed: 9 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,7 @@
1313
# TODO refactor this to a DriveSelection method
1414
class to_ceph_volume(object):
1515

16-
_supported_device_classes = [
17-
"hdd", "ssd", "nvme"
18-
]
16+
NO_CRUSH = '_NO_CRUSH'
1917

2018
def __init__(self,
2119
selection, # type: DriveSelection
@@ -34,20 +32,6 @@ def prepare_devices(self):
3432

3533
lvcount: Dict[str, List[str]] = dict()
3634

37-
"""
38-
Default entry for the global crush_device_class definition;
39-
if there's no global definition at spec level, we do not want
40-
to apply anything to the provided devices, hence we need to run
41-
a ceph-volume command without that option, otherwise we init an
42-
entry for the globally defined crush_device_class.
43-
"""
44-
if self.spec.crush_device_class:
45-
lvcount[self.spec.crush_device_class] = []
46-
47-
# entry where the drives that don't require a crush_device_class
48-
# option are collected
49-
lvcount["no_crush"] = []
50-
5135
"""
5236
for each device, check if it's just a path or it has a crush_device
5337
class definition, and append an entry to the right crush_device_
@@ -57,35 +41,16 @@ class group
5741
# iterate on List[Device], containing both path and
5842
# crush_device_class
5943
path = device.path
60-
crush_device_class = device.crush_device_class
44+
crush_device_class = (
45+
device.crush_device_class
46+
or self.spec.crush_device_class
47+
or self.NO_CRUSH
48+
)
6149

6250
if path is None:
6351
raise ValueError("Device path can't be empty")
6452

65-
"""
66-
if crush_device_class is specified for the current Device path
67-
we should either init the array for this group or append the
68-
drive path to the existing entry
69-
"""
70-
if crush_device_class:
71-
if crush_device_class in lvcount.keys():
72-
lvcount[crush_device_class].append(path)
73-
else:
74-
lvcount[crush_device_class] = [path]
75-
continue
76-
77-
"""
78-
if no crush_device_class is specified for the current path
79-
but a global definition is present in the spec, so we group
80-
the drives together
81-
"""
82-
if crush_device_class is None and self.spec.crush_device_class:
83-
lvcount[self.spec.crush_device_class].append(path)
84-
continue
85-
else:
86-
# default use case
87-
lvcount["no_crush"].append(path)
88-
continue
53+
lvcount.setdefault(crush_device_class, []).append(path)
8954

9055
return lvcount
9156

@@ -136,7 +101,7 @@ def run(self):
136101
cmd += " --block.db {}".format(db_devices.pop())
137102
if wal_devices:
138103
cmd += " --block.wal {}".format(wal_devices.pop())
139-
if d in self._supported_device_classes:
104+
if d != self.NO_CRUSH:
140105
cmd += " --crush-device-class {}".format(d)
141106

142107
cmds.append(cmd)
@@ -159,7 +124,7 @@ def run(self):
159124
if self.spec.block_db_size:
160125
cmd += " --block-db-size {}".format(self.spec.block_db_size)
161126

162-
if d in self._supported_device_classes:
127+
if d != self.NO_CRUSH:
163128
cmd += " --crush-device-class {}".format(d)
164129
cmds.append(cmd)
165130

@@ -180,17 +145,6 @@ def run(self):
180145
cmds[i] += " --yes"
181146
cmds[i] += " --no-systemd"
182147

183-
# set the --crush-device-class option when:
184-
# - crush_device_class is specified at spec level (global for all the osds) # noqa E501
185-
# - crush_device_class is allowed
186-
# - there's no override at osd level
187-
if (
188-
self.spec.crush_device_class and
189-
self.spec.crush_device_class in self._supported_device_classes and # noqa E501
190-
"crush-device-class" not in cmds[i]
191-
):
192-
cmds[i] += " --crush-device-class {}".format(self.spec.crush_device_class) # noqa E501
193-
194148
if self.preview:
195149
cmds[i] += " --report"
196150
cmds[i] += " --format json"

src/python-common/ceph/tests/test_drive_group.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,12 @@ def test_ceph_volume_command_12(test_input2):
392392
drive = drive_selection.DriveSelection(spec, spec.data_devices.paths)
393393
cmds = translate.to_ceph_volume(drive, []).run()
394394

395-
assert (cmds[0] == 'lvm batch --no-auto /dev/sdb --crush-device-class ssd --yes --no-systemd') # noqa E501
396-
assert (cmds[1] == 'lvm batch --no-auto /dev/sda --crush-device-class hdd --yes --no-systemd') # noqa E501
395+
expected_cmds = [
396+
'lvm batch --no-auto /dev/sdb --crush-device-class ssd --yes --no-systemd',
397+
'lvm batch --no-auto /dev/sda --crush-device-class hdd --yes --no-systemd',
398+
]
399+
assert len(cmds) == len(expected_cmds), f"Expected {expected_cmds} got {cmds}"
400+
assert all(cmd in cmds for cmd in expected_cmds), f'Expected {expected_cmds} got {cmds}'
397401

398402

399403
@pytest.mark.parametrize("test_input3",
@@ -418,8 +422,12 @@ def test_ceph_volume_command_13(test_input3):
418422
drive = drive_selection.DriveSelection(spec, spec.data_devices.paths)
419423
cmds = translate.to_ceph_volume(drive, []).run()
420424

421-
assert (cmds[0] == 'lvm batch --no-auto /dev/sdb --yes --no-systemd') # noqa E501
422-
assert (cmds[1] == 'lvm batch --no-auto /dev/sda --crush-device-class hdd --yes --no-systemd') # noqa E501
425+
expected_cmds = [
426+
'lvm batch --no-auto /dev/sdb --yes --no-systemd',
427+
'lvm batch --no-auto /dev/sda --crush-device-class hdd --yes --no-systemd',
428+
]
429+
assert len(cmds) == len(expected_cmds), f"Expected {expected_cmds} got {cmds}"
430+
assert all(cmd in cmds for cmd in expected_cmds), f'Expected {expected_cmds} got {cmds}'
423431

424432

425433
@pytest.mark.parametrize("test_input4",

0 commit comments

Comments
 (0)