Merge ~blake-rouse/maas:fix-1815091-2.5 into maas:2.5

[Blake Rouse]

Fixes LP: #1815091 - Unable to clear the nodes storage configuration when a virtual block device exists on top of another virtual block device.

Backport of 5fcf2bf0a46767b2aab9d76603768818464251f7.

Backport: https://code.launchpad.net/~blake-rouse/maas/+git/maas/+merge/363710

[Blake Rouse (blake-rouse)]

Self-approving backport.

review: Approve

diff --git a/src/maasserver/exceptions.py b/src/maasserver/exceptions.py
index 49fcb48..cc6328a 100644
--- a/src/maasserver/exceptions.py
+++ b/src/maasserver/exceptions.py
@@ -214,3 +214,8 @@ class PodProblem(MAASAPIException):
 
 class NoScriptsFound(MAASException):
     """Raised when no Scripts are found based on user input."""
+
+
+class StorageClearProblem(MAASAPIException):
+    """Raised when an issue occur's that prevents the clearing of a machine's
+    storage configuration."""
diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index 03c890e..87b5042 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -95,6 +95,7 @@ from maasserver.exceptions import (
     NodeStateViolation,
     NoScriptsFound,
     PowerProblem,
+    StorageClearProblem,
 )
 from maasserver.fields import (
     MAASIPAddressField,
@@ -3271,14 +3272,31 @@ class Node(CleanSave, TimestampedModel):
             block_device__id__in=block_device_ids).delete()
         Filesystem.objects.filter(
             block_device__id__in=block_device_ids).delete()
-        for block_device in self.virtualblockdevice_set.all():
-            try:
-                block_device.filesystem_group.delete(force=True)
-            except FilesystemGroup.DoesNotExist:
-                # When a filesystem group has multiple virtual block devices
-                # it is possible that accessing `filesystem_group` will
-                # result in it already being deleted.
-                pass
+        virtual_devices = list(reversed(self.virtualblockdevice_set.all()))
+        for _ in range(10):  # 10 times gives enough tries to remove.
+            for virtual_bd in virtual_devices[:]:  # Iterate on copy.
+                try:
+                    virtual_bd.filesystem_group.delete(force=True)
+                    virtual_devices.remove(virtual_bd)
+                except FilesystemGroup.DoesNotExist:
+                    # When a filesystem group has multiple virtual block
+                    # devices it is possible that accessing `filesystem_group`
+                    # will result in it already being deleted.
+                    virtual_devices.remove(virtual_bd)
+                except ValidationError:
+                    # Cannot be deleted because another device depends on it.
+                    # Next loop through will delete the device.
+                    pass
+            if not virtual_devices:
+                # Done, all have been removed.
+                break
+        if virtual_devices:
+            raise StorageClearProblem(
+                "Failed to remove %d virtual devices: %s" % (
+                    len(virtual_devices), ', '.join([
+                        virtual_bd.get_name()
+                        for virtual_bd in virtual_devices
+                    ])))
 
     def _create_acquired_filesystems(self):
         """Copy all filesystems that have a user mountable filesystem to be
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 2db8120..dd964f4 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -4075,6 +4075,41 @@ class TestNode(MAASServerTestCase):
             reload_object(filesystem_on_iscsi_vbd1), Is(None),
             "Filesystem on virtual block device should have been removed.")
 
+    def test__clear_full_storage_configuration_lp1815091(self):
+        node = factory.make_Node()
+        physical_block_devices = [
+            factory.make_PhysicalBlockDevice(node=node, size=10 * 1000 ** 3)
+            for _ in range(4)
+            ]
+        raid5_filesystems = [
+            factory.make_Filesystem(
+                block_device=bd,
+                fstype=FILESYSTEM_TYPE.RAID)
+            for bd in physical_block_devices[:3]
+        ]
+        raid5 = factory.make_FilesystemGroup(
+            group_type=FILESYSTEM_GROUP_TYPE.RAID_5,
+            filesystems=raid5_filesystems).virtual_device
+        backing_filesystem = factory.make_Filesystem(
+            block_device=raid5,
+            fstype=FILESYSTEM_TYPE.BCACHE_BACKING)
+        cacheset = factory.make_CacheSet(
+            block_device=physical_block_devices[3])
+        root = factory.make_FilesystemGroup(
+            group_type=FILESYSTEM_GROUP_TYPE.BCACHE,
+            filesystems=[backing_filesystem], cache_set=cacheset)
+        node._clear_full_storage_configuration()
+        for pbd in physical_block_devices:
+            self.expectThat(
+                reload_object(pbd), Not(Is(None)),
+                "Physical block device should not have been deleted.")
+        self.expectThat(
+            reload_object(root), Is(None),
+            "Bcache should have been removed.")
+        self.expectThat(
+            reload_object(raid5), Is(None),
+            "Raid should have been removed.")
+
     def test__create_acquired_filesystems(self):
         node = factory.make_Node(status=NODE_STATUS.ALLOCATED)
         block_device = factory.make_PhysicalBlockDevice(node=node)