Skip to content

Commit 1492c70

Browse files
dtantsurTheMipmap
dtantsur
authored and
TheMipmap
committed
Fix software RAID creation on different physical devices
When creating multiple software RAID logical disks that use different sets of physical devices, the partition indices were incorrectly shared across all devices. This caused the second RAID array creation to fail because it tried to use partition indices that didn't exist on those specific devices. This change fixes the issue by tracking partition indices separately for each physical device, ensuring that each device's partitions are numbered correctly starting from their first available index. Closes-Bug: #2115211 Change-Id: I440db4654f3d1d54274d1eee8c4b21c2b0a18d22 Signed-off-by: Mohammed Naser <mnaser@vexxhost.com> Signed-off-by: Dmitry Tantsur <dtantsur@protonmail.com> (cherry picked from commit 521811c)
1 parent 797c3f3 commit 1492c70

5 files changed

Lines changed: 159 additions & 7 deletions

File tree

‎ironic_python_agent/hardware.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3057,9 +3057,11 @@ def _get_volume_names_of_existing_raids():
30573057

30583058
parted_start_dict[device] = end
30593059

3060-
# Create the RAID devices.
3060+
# Create the RAID devices. The indices mapping tracks the last used
3061+
# partition index for each physical device.
3062+
indices = {}
30613063
for index, logical_disk in enumerate(logical_disks):
3062-
raid_utils.create_raid_device(index, logical_disk)
3064+
raid_utils.create_raid_device(index, logical_disk, indices)
30633065

30643066
LOG.info("Successfully created Software RAID")
30653067

@@ -3354,6 +3356,8 @@ def validate_configuration(self, raid_config, node):
33543356
size2 = logical_disks[1]['size_gb']
33553357

33563358
# Only one logical disk is allowed to span the whole device.
3359+
# FIXME(dtantsur): this logic is not correct when logical disks use
3360+
# different physical devices.
33573361
if size1 == 'MAX' and size2 == 'MAX':
33583362
msg = ("Software RAID can have only one RAID device with "
33593363
"size 'MAX'")

‎ironic_python_agent/raid_utils.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,24 +202,31 @@ def _get_actual_component_devices(raid_device):
202202
return component_devices
203203

204204

205-
def create_raid_device(index, logical_disk):
205+
def create_raid_device(index, logical_disk, indices=None):
206206
"""Create a raid device.
207207
208208
:param index: the index of the resulting md device.
209209
:param logical_disk: the logical disk containing the devices used to
210210
crete the raid.
211+
:param indices: Mapping to track the last used partition index for each
212+
physical device across calls to create_raid_device.
211213
:raise: errors.SoftwareRAIDError if not able to create the raid device
212214
or fails to re-add a device to a raid.
213215
"""
214216
md_device = '/dev/md%d' % index
215217
component_devices = []
218+
if indices is None:
219+
# Backward compatibility with custom hardware managers
220+
indices = {dev: index for dev in logical_disk['block_devices']}
216221
for device in logical_disk['block_devices']:
217222
# The partition delimiter for all common harddrives (sd[a-z]+)
218223
part_delimiter = ''
219224
if 'nvme' in device:
220225
part_delimiter = 'p'
226+
index_on_device = indices.get(device, 0)
221227
component_devices.append(
222-
device + part_delimiter + str(index + RAID_PARTITION))
228+
device + part_delimiter + str(index_on_device + RAID_PARTITION))
229+
indices[device] = index_on_device + 1
223230
raid_level = logical_disk['raid_level']
224231
# The schema check allows '1+0', but mdadm knows it as '10'.
225232
if raid_level == '1+0':

‎ironic_python_agent/tests/unit/test_hardware.py‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4308,6 +4308,117 @@ def test_create_configuration_with_hints(self, mocked_execute,
43084308
mock.call(x) for x in ['/dev/sda', '/dev/sdb']
43094309
])
43104310

4311+
@mock.patch.object(raid_utils, '_get_actual_component_devices',
4312+
autospec=True)
4313+
@mock.patch.object(utils, 'get_node_boot_mode', lambda node: 'bios')
4314+
@mock.patch.object(disk_utils, 'list_partitions', autospec=True,
4315+
return_value=[])
4316+
@mock.patch.object(utils, 'execute', autospec=True)
4317+
def test_create_configuration_with_different_disks(self, mocked_execute,
4318+
mock_list_parts,
4319+
mocked_actual_comp):
4320+
node = self.node
4321+
raid_config = {
4322+
"logical_disks": [
4323+
{
4324+
"size_gb": "10",
4325+
"raid_level": "1",
4326+
"controller": "software",
4327+
"physical_disks": [{'name': '/dev/sda'},
4328+
{'name': '/dev/sdb'}],
4329+
},
4330+
{
4331+
"size_gb": "MAX",
4332+
"raid_level": "0",
4333+
"controller": "software",
4334+
"physical_disks": [{'name': '/dev/sdc'},
4335+
{'name': '/dev/sdd'}],
4336+
},
4337+
]
4338+
}
4339+
node['target_raid_config'] = raid_config
4340+
device1 = hardware.BlockDevice('/dev/sda', 'sda', 107374182400, True)
4341+
device2 = hardware.BlockDevice('/dev/sdb', 'sdb', 107374182400, True)
4342+
device3 = hardware.BlockDevice('/dev/sdc', 'sdc', 107374182400, True)
4343+
device4 = hardware.BlockDevice('/dev/sdd', 'sdd', 107374182400, True)
4344+
self.hardware.list_block_devices = mock.Mock()
4345+
self.hardware.list_block_devices.return_value = [
4346+
device1,
4347+
device2,
4348+
device3,
4349+
device4,
4350+
]
4351+
4352+
mocked_execute.side_effect = [
4353+
None, # mklabel sda
4354+
('42', None), # sgdisk -F sda
4355+
None, # mklabel sda
4356+
('42', None), # sgdisk -F sdb
4357+
None, # mklabel sdc
4358+
('42', None), # sgdisk -F sdc
4359+
None, # mklabel sdd
4360+
('42', None), # sgdisk -F sdd
4361+
None, None, None, # parted + partx + udevadm_settle sda
4362+
None, None, None, # parted + partx + udevadm_settle sdb
4363+
None, None, None, # parted + partx + udevadm_settle sdc
4364+
None, None, None, # parted + partx + udevadm_settle sdd
4365+
None, None, None, # parted + partx + udevadm_settle sda
4366+
None, None, None, # parted + partx + udevadm_settle sdb
4367+
None, None, None, # parted + partx + udevadm_settle sdc
4368+
None, None, None, # parted + partx + udevadm_settle sdd
4369+
None, None # mdadms
4370+
]
4371+
4372+
mocked_actual_comp.side_effect = [
4373+
('/dev/sda1', '/dev/sdb1'),
4374+
('/dev/sdc1', '/dev/sdd1'),
4375+
]
4376+
4377+
result = self.hardware.create_configuration(node, [])
4378+
4379+
mocked_execute.assert_has_calls([
4380+
mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', 'msdos'),
4381+
mock.call('sgdisk', '-F', '/dev/sda'),
4382+
mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', 'msdos'),
4383+
mock.call('sgdisk', '-F', '/dev/sdb'),
4384+
mock.call('parted', '/dev/sdc', '-s', '--', 'mklabel', 'msdos'),
4385+
mock.call('sgdisk', '-F', '/dev/sdc'),
4386+
mock.call('parted', '/dev/sdd', '-s', '--', 'mklabel', 'msdos'),
4387+
mock.call('sgdisk', '-F', '/dev/sdd'),
4388+
mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--',
4389+
'mkpart', 'primary', '42s', '10GiB'),
4390+
mock.call('partx', '-av', '/dev/sda', attempts=3,
4391+
delay_on_retry=True),
4392+
mock.call('udevadm', 'settle'),
4393+
mock.call('parted', '/dev/sdb', '-s', '-a', 'optimal', '--',
4394+
'mkpart', 'primary', '42s', '10GiB'),
4395+
mock.call('partx', '-av', '/dev/sdb', attempts=3,
4396+
delay_on_retry=True),
4397+
mock.call('udevadm', 'settle'),
4398+
mock.call('parted', '/dev/sdc', '-s', '-a', 'optimal', '--',
4399+
'mkpart', 'primary', '42s', '-1'),
4400+
mock.call('partx', '-av', '/dev/sdc', attempts=3,
4401+
delay_on_retry=True),
4402+
mock.call('udevadm', 'settle'),
4403+
mock.call('parted', '/dev/sdd', '-s', '-a', 'optimal', '--',
4404+
'mkpart', 'primary', '42s', '-1'),
4405+
mock.call('partx', '-av', '/dev/sdd', attempts=3,
4406+
delay_on_retry=True),
4407+
mock.call('udevadm', 'settle'),
4408+
mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
4409+
'--metadata=1', '--level', '1', '--name', 'md0',
4410+
'--raid-devices', 2, '/dev/sda1', '/dev/sdb1'),
4411+
mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
4412+
'--metadata=1', '--level', '0', '--name', 'md1',
4413+
'--raid-devices', 2, '/dev/sdc1', '/dev/sdd1')])
4414+
self.assertEqual(raid_config, result)
4415+
4416+
self.assertEqual(4, mock_list_parts.call_count)
4417+
mock_list_parts.assert_has_calls([
4418+
mock.call(x) for x in ['/dev/sda', '/dev/sdb',
4419+
'/dev/sdc', '/dev/sdd']
4420+
])
4421+
43114422
@mock.patch.object(utils, 'execute', autospec=True)
43124423
@mock.patch.object(os.path, 'isdir', autospec=True, return_value=False)
43134424
def test_create_configuration_invalid_raid_config(self,

‎ironic_python_agent/tests/unit/test_raid_utils.py‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def test_create_raid_device(self, mock_execute, mocked_components):
5252
'/dev/sdb1',
5353
'/dev/sdc1']
5454

55-
raid_utils.create_raid_device(0, logical_disk)
55+
raid_utils.create_raid_device(0, logical_disk, {})
5656

5757
mock_execute.assert_called_once_with(
5858
'mdadm', '--create', '/dev/md0', '--force', '--run',
@@ -73,7 +73,7 @@ def test_create_raid_device_with_volume_name(self, mock_execute,
7373
'/dev/sdb1',
7474
'/dev/sdc1']
7575

76-
raid_utils.create_raid_device(0, logical_disk)
76+
raid_utils.create_raid_device(0, logical_disk, {})
7777

7878
mock_execute.assert_called_once_with(
7979
'mdadm', '--create', '/dev/md0', '--force', '--run',
@@ -92,7 +92,7 @@ def test_create_raid_device_missing_device(self, mock_execute,
9292
mocked_components.return_value = ['/dev/sda1',
9393
'/dev/sdc1']
9494

95-
raid_utils.create_raid_device(0, logical_disk)
95+
raid_utils.create_raid_device(0, logical_disk, {})
9696

9797
expected_calls = [
9898
mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
@@ -105,6 +105,29 @@ def test_create_raid_device_missing_device(self, mock_execute,
105105
self.assertEqual(mock_execute.call_count, 2)
106106
mock_execute.assert_has_calls(expected_calls)
107107

108+
@mock.patch.object(raid_utils, '_get_actual_component_devices',
109+
autospec=True)
110+
@mock.patch.object(utils, 'execute', autospec=True)
111+
def test_create_raid_device_with_indices(self, mock_execute,
112+
mocked_components):
113+
indices = {'/dev/sda': 1, '/dev/sdb': 2}
114+
logical_disk = {
115+
"block_devices": ['/dev/sda', '/dev/sdb', '/dev/sdc'],
116+
"raid_level": "1",
117+
}
118+
mocked_components.return_value = ['/dev/sda2',
119+
'/dev/sdb3',
120+
'/dev/sdc1']
121+
122+
raid_utils.create_raid_device(0, logical_disk, indices)
123+
124+
mock_execute.assert_called_once_with(
125+
'mdadm', '--create', '/dev/md0', '--force', '--run',
126+
'--metadata=1', '--level', '1', '--name', 'md0',
127+
'--raid-devices', 3, '/dev/sda2', '/dev/sdb3', '/dev/sdc1')
128+
self.assertEqual(
129+
{'/dev/sda': 2, '/dev/sdb': 3, '/dev/sdc': 1}, indices)
130+
108131
@mock.patch.object(utils, 'execute', autospec=True)
109132
def test_create_raid_device_fail_create_device(self, mock_execute):
110133
logical_disk = {

‎releasenotes/notes/fix-software-raid-different-devices-3298f735fdbd3a05.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
fixes:
3+
- |
4+
Software RAID creation now correctly handles configurations where
5+
logical disks use different sets of physical devices. Previously,
6+
partition indices were incorrectly shared across all devices, causing
7+
failures when creating multiple RAID arrays on different disks.

0 commit comments

Comments
 (0)