Merge "Software RAID: Get component devices by md UUID"6.4.0

3 files changed, 152 insertions, 20 deletions

diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index edc9390f..3724effb 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -136,33 +136,70 @@ def _check_for_iscsi():
                     "Error: %s", e)
 
 
-def _get_component_devices(raid_device):
-    """Get the component devices of a Software RAID device.
-
-    Examine an md device and return its constituent devices.
+def _get_md_uuid(raid_device):
+    """Get the md UUID of a Software RAID device.
 
     :param raid_device: A Software RAID block device name.
-    :returns: A list of the component devices.
+    :returns: A string containing the UUID of an md device.
     """
-    if not raid_device:
-        return []
-
     try:
         out, _ = utils.execute('mdadm', '--detail', raid_device,
                                use_standard_locale=True)
     except processutils.ProcessExecutionError as e:
-        msg = ('Could not get component devices of %(dev)s: %(err)s' %
+        msg = ('Could not get the details of %(dev)s: %(err)s' %
                {'dev': raid_device, 'err': e})
         LOG.warning(msg)
-        return []
+        return
 
-    component_devices = []
     lines = out.splitlines()
     # the first line contains the md device itself
     for line in lines[1:]:
-        device = re.findall(r'/dev/\w+', line)
-        component_devices += device
+        match = re.search(r'UUID : ([a-f0-9:]+)', line)
+        if match:
+            return match.group(1)
+
+
+def _get_component_devices(raid_device):
+    """Get the component devices of a Software RAID device.
+
+    Get the UUID of the md device and scan all other devices
+    for the same md UUID.
+
+    :param raid_device: A Software RAID block device name.
+    :returns: A list of the component devices.
+    """
+    if not raid_device:
+        return []
+
+    md_uuid = _get_md_uuid(raid_device)
+    if not md_uuid:
+        return []
+    LOG.debug('%s has UUID %s', raid_device, md_uuid)
+
+    component_devices = []
+    block_devices = list_all_block_devices()
+    block_devices.extend(list_all_block_devices(block_type='part',
+                                                ignore_raid=True))
+    for bdev in block_devices:
+        try:
+            out, _ = utils.execute('mdadm', '--examine', bdev.name,
+                                   use_standard_locale=True)
+        except processutils.ProcessExecutionError as e:
+            if "No md superblock detected" in str(e):
+                # actually not a component device
+                LOG.debug('Not a component device %s', bdev.name)
+                continue
+            else:
+                LOG.warning("Failed to examine device %s: %s",
+                            bdev.name, e)
+                continue
+        lines = out.splitlines()
+        for line in lines:
+            if md_uuid in line:
+                component_devices.append(bdev.name)
 
+    LOG.info('Found component devices for %s:',
+             raid_device, component_devices)
     return component_devices
 
 
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index 88934b9b..b9348912 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -801,6 +801,64 @@ MDADM_DETAIL_OUTPUT_BROKEN_RAID0 = ("""/dev/md126:
 """)
 
 
+MDADM_EXAMINE_OUTPUT_MEMBER = ("""/dev/sda1:
+          Magic : a92b4efc
+        Version : 1.2
+    Feature Map : 0x0
+     Array UUID : 83143055:2781ddf5:2c8f44c7:9b45d92e
+           Name : horse.cern.ch:1  (local to host abc.xyz.com)
+  Creation Time : Tue Jun 11 12:43:37 2019
+     Raid Level : raid1
+   Raid Devices : 2
+
+ Avail Dev Size : 2093056 sectors (1022.00 MiB 1071.64 MB)
+     Array Size : 1046528 KiB (1022.00 MiB 1071.64 MB)
+    Data Offset : 2048 sectors
+   Super Offset : 8 sectors
+   Unused Space : before=1968 sectors, after=0 sectors
+          State : clean
+    Device UUID : 88bf2723:d082f14f:f95e87cf:b7c59b83
+
+    Update Time : Sun Sep 27 01:00:08 2020
+  Bad Block Log : 512 entries available at offset 16 sectors
+       Checksum : 340a1610 - correct
+         Events : 178
+
+
+   Device Role : Active device 0
+   Array State : A. ('A' == active, '.' == missing, 'R' == replacing)
+""")
+
+
+MDADM_EXAMINE_OUTPUT_NON_MEMBER = ("""/dev/sdz1:
+          Magic : a92b4efc
+        Version : 1.2
+    Feature Map : 0x0
+     Array UUID : 83143055:2781ddf5:2c8f44c7:9b45d92f
+           Name : horse.cern.ch:1  (local to host abc.xyz.com)
+  Creation Time : Tue Jun 11 12:43:37 2019
+     Raid Level : raid1
+   Raid Devices : 2
+
+ Avail Dev Size : 2093056 sectors (1022.00 MiB 1071.64 MB)
+     Array Size : 1046528 KiB (1022.00 MiB 1071.64 MB)
+    Data Offset : 2048 sectors
+   Super Offset : 8 sectors
+   Unused Space : before=1968 sectors, after=0 sectors
+          State : clean
+    Device UUID : 88bf2723:d082f14f:f95e87cf:b7c59b84
+
+    Update Time : Sun Sep 27 01:00:08 2020
+  Bad Block Log : 512 entries available at offset 16 sectors
+       Checksum : 340a1610 - correct
+         Events : 178
+
+
+   Device Role : Active device 0
+   Array State : A. ('A' == active, '.' == missing, 'R' == replacing)
+""")
+
+
 class FakeHardwareManager(hardware.GenericHardwareManager):
     def __init__(self, hardware_support):
         self._hardware_support = hardware_support
@@ -3908,16 +3966,47 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                                self.node, [])
 
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test__get_component_devices(self, mocked_execute):
+    def test__get_md_uuid(self, mocked_execute):
         mocked_execute.side_effect = [(MDADM_DETAIL_OUTPUT, '')]
-        component_devices = hardware._get_component_devices('/dev/md0')
-        self.assertEqual(['/dev/vde1', '/dev/vdf1'], component_devices)
+        md_uuid = hardware._get_md_uuid('/dev/md0')
+        self.assertEqual('83143055:2781ddf5:2c8f44c7:9b45d92e', md_uuid)
 
+    @mock.patch.object(hardware, '_get_md_uuid', autospec=True)
+    @mock.patch.object(hardware, 'list_all_block_devices', autospec=True)
     @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, '')]
-        component_devices = hardware._get_component_devices('/dev/md126')
-        self.assertEqual(['/dev/sda2'], component_devices)
+    def test__get_component_devices(self, mocked_execute,
+                                    mocked_list_all_block_devices,
+                                    mocked_md_uuid):
+        raid_device1 = hardware.BlockDevice('/dev/md0', 'RAID-1',
+                                            107374182400, True)
+        sda = hardware.BlockDevice('/dev/sda', 'model12', 21, True)
+        sdz = hardware.BlockDevice('/dev/sdz', 'model12', 21, True)
+        sda1 = hardware.BlockDevice('/dev/sda1', 'model12', 21, True)
+        sdz1 = hardware.BlockDevice('/dev/sdz1', 'model12', 21, True)
+
+        mocked_md_uuid.return_value = '83143055:2781ddf5:2c8f44c7:9b45d92e'
+        hardware.list_all_block_devices.side_effect = [
+            [sda, sdz],    # list_all_block_devices
+            [sda1, sdz1],  # list_all_block_devices partitions
+        ]
+        mocked_execute.side_effect = [
+            ['mdadm --examine output for sda', '_'],
+            ['mdadm --examine output for sdz', '_'],
+            [MDADM_EXAMINE_OUTPUT_MEMBER, '_'],
+            [MDADM_EXAMINE_OUTPUT_NON_MEMBER, '_'],
+        ]
+
+        component_devices = hardware._get_component_devices(raid_device1)
+        self.assertEqual(['/dev/sda1'], component_devices)
+        mocked_execute.assert_has_calls([
+            mock.call('mdadm', '--examine', '/dev/sda',
+                      use_standard_locale=True),
+            mock.call('mdadm', '--examine', '/dev/sdz',
+                      use_standard_locale=True),
+            mock.call('mdadm', '--examine', '/dev/sda1',
+                      use_standard_locale=True),
+            mock.call('mdadm', '--examine', '/dev/sdz1',
+                      use_standard_locale=True)])
 
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_holder_disks(self, mocked_execute):
diff --git a/releasenotes/notes/get_md_components_by_uuid-7f08d423ea9e7c94.yaml b/releasenotes/notes/get_md_components_by_uuid-7f08d423ea9e7c94.yaml
new file mode 100644
index 00000000..89317535
--- /dev/null
+++ b/releasenotes/notes/get_md_components_by_uuid-7f08d423ea9e7c94.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Detects md component devices by their UUID, rather than by scanning the
+    output of mdadm. This will prevent that devices miss md superblock
+    cleanup when they are currently not part of an array.