diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 37e36f7cd..6f9ebe29e 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -168,9 +168,20 @@ def get_holder_disks(raid_device): lines = out.splitlines() # the first line contains the md device itself + + holder_parts = [] for line in lines[1:]: - device = re.findall(r'/dev/\D+', line) - holder_disks += device + device = re.findall(r'/dev/\w+', line) + holder_parts += device + + for part in holder_parts: + + device = utils.extract_device(part) + if not device: + msg = ('Could not get holder disks of %s: unexpected pattern ' + 'for partition %s') % (raid_device, part) + raise errors.SoftwareRAIDError(msg) + holder_disks.append(device) return holder_disks diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index a84969349..9f4eb47be 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -639,6 +639,34 @@ Consistency Policy : resync 1 253 80 1 active sync /dev/vdf1 """) +MDADM_DETAIL_OUTPUT_NVME = ("""/dev/md0: + Version : 1.2 + Creation Time : Wed Aug 7 13:47:27 2019 + Raid Level : raid1 + Array Size : 439221248 (418.87 GiB 449.76 GB) + Used Dev Size : 439221248 (418.87 GiB 449.76 GB) + Raid Devices : 2 + Total Devices : 2 + Persistence : Superblock is persistent + + Intent Bitmap : Internal + + Update Time : Wed Aug 7 14:37:21 2019 + State : clean + Active Devices : 2 +Working Devices : 2 + Failed Devices : 0 + Spare Devices : 0 + + Name : rescue:0 (local to host rescue) + UUID : abe222bc:98735860:ab324674:e4076313 + Events : 426 + + Number Major Minor RaidDevice State + 0 259 2 0 active sync /dev/nvme0n1p1 + 1 259 3 1 active sync /dev/nvme1n1p1 +""") + MDADM_DETAIL_OUTPUT_BROKEN_RAID0 = ("""/dev/md126: Version : 1.2 @@ -2872,33 +2900,43 @@ class TestGenericHardwareManager(base.IronicAgentTest): @mock.patch.object(utils, 'execute', autospec=True) def test__get_component_devices(self, mocked_execute): mocked_execute.side_effect = [(MDADM_DETAIL_OUTPUT, '')] - raid_device = hardware.BlockDevice('/dev/md0', 'RAID-1', - 1073741824, True) - component_devices = hardware._get_component_devices(raid_device.name) + component_devices = hardware._get_component_devices('/dev/md0') self.assertEqual(['/dev/vde1', '/dev/vdf1'], component_devices) @mock.patch.object(utils, 'execute', autospec=True) def test__get_component_devices_broken_raid0(self, mocked_execute): mocked_execute.side_effect = [(MDADM_DETAIL_OUTPUT_BROKEN_RAID0, '')] - raid_device = hardware.BlockDevice('/dev/md126', 'RAID-0', - 1073741824, True) - component_devices = hardware._get_component_devices(raid_device.name) + component_devices = hardware._get_component_devices('/dev/md126') self.assertEqual(['/dev/sda2'], component_devices) @mock.patch.object(utils, 'execute', autospec=True) def test_get_holder_disks(self, mocked_execute): mocked_execute.side_effect = [(MDADM_DETAIL_OUTPUT, '')] - raid_device = hardware.BlockDevice('/dev/md0', 'RAID-1', - 1073741824, True) - holder_disks = hardware.get_holder_disks(raid_device.name) + holder_disks = hardware.get_holder_disks('/dev/md0') self.assertEqual(['/dev/vde', '/dev/vdf'], holder_disks) + @mock.patch.object(utils, 'execute', autospec=True) + def test_get_holder_disks_with_nvme(self, mocked_execute): + mocked_execute.side_effect = [(MDADM_DETAIL_OUTPUT_NVME, '')] + holder_disks = hardware.get_holder_disks('/dev/md0') + self.assertEqual(['/dev/nvme0n1', '/dev/nvme1n1'], holder_disks) + + @mock.patch.object(utils, 'execute', autospec=True) + def test_get_holder_disks_unexpected_devices(self, mocked_execute): + side_effect = MDADM_DETAIL_OUTPUT_NVME.replace('nvme1n1p1', + 'notmatching1a') + mocked_execute.side_effect = [(side_effect, '')] + self.assertRaisesRegex( + errors.SoftwareRAIDError, + r'^Software RAID caused unknown error: Could not get holder disks ' + r'of /dev/md0: unexpected pattern for partition ' + r'/dev/notmatching1a$', + hardware.get_holder_disks, '/dev/md0') + @mock.patch.object(utils, 'execute', autospec=True) def test_get_holder_disks_broken_raid0(self, mocked_execute): mocked_execute.side_effect = [(MDADM_DETAIL_OUTPUT_BROKEN_RAID0, '')] - raid_device = hardware.BlockDevice('/dev/md126', 'RAID-0', - 1073741824, True) - holder_disks = hardware.get_holder_disks(raid_device.name) + holder_disks = hardware.get_holder_disks('/dev/md126') self.assertEqual(['/dev/sda'], holder_disks) @mock.patch.object(hardware, 'get_holder_disks', autospec=True) diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index 24b5994f0..4ee0ebd39 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -556,3 +556,39 @@ class TestUtils(testtools.TestCase): keyfile='spam', certfile='ham') self.assertEqual((True, ('ham', 'spam')), utils.get_ssl_client_options(conf)) + + def test_device_extractor(self): + self.assertEqual( + 'md0', + utils.extract_device('md0p1') + ) + self.assertEqual( + '/dev/md0', + utils.extract_device('/dev/md0p1') + ) + self.assertEqual( + 'sda', + utils.extract_device('sda12') + ) + self.assertEqual( + '/dev/sda', + utils.extract_device('/dev/sda12') + ) + self.assertEqual( + 'nvme0n1', + utils.extract_device('nvme0n1p12') + ) + self.assertEqual( + '/dev/nvme0n1', + utils.extract_device('/dev/nvme0n1p12') + ) + self.assertEqual( + '/dev/hello', + utils.extract_device('/dev/hello42') + ) + self.assertIsNone( + utils.extract_device('/dev/sda') + ) + self.assertIsNone( + utils.extract_device('whatevernotmatchin12a') + ) diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index 2c71827a6..0257cfb1a 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -17,6 +17,7 @@ import errno import glob import io import os +import re import shutil import subprocess import tarfile @@ -60,6 +61,9 @@ COLLECT_LOGS_COMMANDS = { } +DEVICE_EXTRACTOR = re.compile(r'^(?:(.*\d)p|(.*\D))(?:\d+)$') + + def execute(*cmd, **kwargs): """Convenience wrapper around ironic_lib's execute() method. @@ -436,3 +440,16 @@ def get_ssl_client_options(conf): else: cert = None return verify, cert + + +def extract_device(part): + """Extract the device from a partition name or path. + + :param part: the partition + :return: a device if success, None otherwise + """ + + m = DEVICE_EXTRACTOR.match(part) + if not m: + return None + return (m.group(1) or m.group(2)) diff --git a/releasenotes/notes/get-holder-disks-with-nvme-7d5fa75df2fd5904.yaml b/releasenotes/notes/get-holder-disks-with-nvme-7d5fa75df2fd5904.yaml new file mode 100644 index 000000000..9974d1dda --- /dev/null +++ b/releasenotes/notes/get-holder-disks-with-nvme-7d5fa75df2fd5904.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes an issue where md devices disk holders could not be listed + correctly if they were nvme drives.