From c7aec775ff5530e2c0407d8de49f15e8150f9248 Mon Sep 17 00:00:00 2001
From: Arne Wiebalck <Arne.Wiebalck@cern.ch>
Date: Mon, 28 Sep 2020 10:31:15 +0200
Subject: [PATCH] Software RAID: Don't delete partitions too early

Partions on the holder disk should only be deleted after
all RAID devices have been deleted. Otherwise, super blocks
on partitions which reside on the same disks cannot be cleaned.

Story: #2008199
Task: #40979
Change-Id: I19293f5b992cd1fa68957d6f306dcec8f3b7a820
---
 ironic_python_agent/hardware.py               | 25 +++++++++++++------
 .../tests/unit/test_hardware.py               |  8 +++---
 ...ix_partition_cleanup-46491861c930db12.yaml |  6 +++++
 3 files changed, 27 insertions(+), 12 deletions(-)
 create mode 100644 releasenotes/notes/fix_partition_cleanup-46491861c930db12.yaml

diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 3141c2c66..edc9390f5 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -1881,6 +1881,7 @@ class GenericHardwareManager(HardwareManager):
             raise errors.SoftwareRAIDError(msg)
 
     def _delete_config_pass(self, raid_devices):
+        all_holder_disks = []
         for raid_device in raid_devices:
             component_devices = _get_component_devices(raid_device.name)
             if not component_devices:
@@ -1933,14 +1934,11 @@ class GenericHardwareManager(HardwareManager):
                     LOG.warning('Failed to remove superblock from %s: %s',
                                 raid_device.name, e)
 
-            # Remove the partitions we created during create_configuration.
-            for holder_disk in holder_disks:
-                LOG.debug('Removing partitions on %s', holder_disk)
-                try:
-                    utils.execute('wipefs', '-af', holder_disk)
-                except processutils.ProcessExecutionError:
-                    LOG.warning('Failed to remove partitions on %s',
-                                holder_disk)
+            # NOTE(arne_wiebalck): We cannot delete the partitions right
+            # away since there may be other partitions on the same disks
+            # which are members of other RAID devices. So we remember them
+            # for later.
+            all_holder_disks.extend(holder_disks)
 
             LOG.info('Deleted Software RAID device %s', raid_device.name)
 
@@ -1986,6 +1984,17 @@ class GenericHardwareManager(HardwareManager):
                 LOG.warning('Failed to remove superblock from %s: %s',
                             raid_device.name, e)
 
+        # Erase all partition tables we created
+        all_holder_disks_uniq = list(
+            collections.OrderedDict.fromkeys(all_holder_disks))
+        for holder_disk in all_holder_disks_uniq:
+            LOG.info('Removing partitions on holder disk %s', holder_disk)
+            try:
+                utils.execute('wipefs', '-af', holder_disk)
+            except processutils.ProcessExecutionError as e:
+                LOG.warning('Failed to remove partitions on %s',
+                            holder_disk, e)
+
         LOG.debug("Finished deleting Software RAID(s)")
 
     def validate_configuration(self, raid_config, node):
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index 5dc76b504..88934b9ba 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -4016,8 +4016,6 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             mock.call('mdadm', '--examine', '/dev/sdb1',
                       use_standard_locale=True),
             mock.call('mdadm', '--zero-superblock', '/dev/sdb1'),
-            mock.call('wipefs', '-af', '/dev/sda'),
-            mock.call('wipefs', '-af', '/dev/sdb'),
             mock.call('wipefs', '-af', '/dev/md1'),
             mock.call('mdadm', '--stop', '/dev/md1'),
             mock.call('mdadm', '--examine', '/dev/sda2',
@@ -4026,15 +4024,17 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             mock.call('mdadm', '--examine', '/dev/sdb2',
                       use_standard_locale=True),
             mock.call('mdadm', '--zero-superblock', '/dev/sdb2'),
-            mock.call('wipefs', '-af', '/dev/sda'),
-            mock.call('wipefs', '-af', '/dev/sdb'),
             mock.call('mdadm', '--examine', '/dev/sdc',
                       use_standard_locale=True),
             mock.call('mdadm', '--zero-superblock', '/dev/sdc'),
             mock.call('mdadm', '--examine', '/dev/sdb',
                       use_standard_locale=True),
+            mock.call('mdadm', '--zero-superblock', '/dev/sdb'),
             mock.call('mdadm', '--examine', '/dev/sda',
                       use_standard_locale=True),
+            mock.call('mdadm', '--zero-superblock', '/dev/sda'),
+            mock.call('wipefs', '-af', '/dev/sda'),
+            mock.call('wipefs', '-af', '/dev/sdb'),
             mock.call('mdadm', '--assemble', '--scan', check_exit_code=False),
         ])
 
diff --git a/releasenotes/notes/fix_partition_cleanup-46491861c930db12.yaml b/releasenotes/notes/fix_partition_cleanup-46491861c930db12.yaml
new file mode 100644
index 000000000..79584ff45
--- /dev/null
+++ b/releasenotes/notes/fix_partition_cleanup-46491861c930db12.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes a bug where the partitions created during software RAID setup are cleaned
+    too early and therefore may prevent the proper cleaning of the md superblocks.
+    Leaving superblocks behind will impact the creation of new md devices later on.