Merge pull request #657 from xcp-ng/gln/tools-clean-improvements-xyzl

tools clean: clean snapshots and continue on error

Author: glehmann

lib/tools/cli.py

@@ -24,13 +24,13 @@ def _command_update(args: argparse.Namespace) -> None:
     update_pools(inventory, reboot=args.reboot, parallel=args.parallel)
 
 
-def _command_clean(args: argparse.Namespace) -> None:
+def _command_clean(args: argparse.Namespace) -> int:
     if args.inventory:
         inventory = load_inventory(args.inventory)
     else:
         inventory = into_inventory(args.hosts, [], args.hosting_pool)
 
-    clean_pools(inventory, dry_run=args.dry_run)
+    return clean_pools(inventory, dry_run=args.dry_run)
 
 
 def _command_exec(args: argparse.Namespace) -> int:
@@ -105,8 +105,8 @@ def cli() -> None:
     # subparser - command: clean
     subparser_cmd_clean = subparsers.add_parser(
         name="clean",
-        description="Remove all VMs and all VDIs on local storage from target pools",
-        help="Remove all VMs and all VDIs on local storage from target pools",
+        description="Remove all VMs, snapshorts and VDIs on local storage from target pools",
+        help="Remove all VMs, snapshorts and VDIs on local storage from target pools",
     )
     cmd_clean_excl_grp = subparser_cmd_clean.add_mutually_exclusive_group(required=True)
     cmd_clean_excl_grp.add_argument(

lib/tools/tasks/clean.py

@@ -16,7 +16,7 @@
 
 from .. import logger
 
-def clean_pools(inventory: Inventory, dry_run: bool = False) -> None:
+def clean_pools(inventory: Inventory, dry_run: bool = False) -> int:
     """Remove all VMs and all orphan VDIs on local storage from pool(s).
 
     .. note::
@@ -31,6 +31,8 @@ def clean_pools(inventory: Inventory, dry_run: bool = False) -> None:
         Each host (key) holds its own config data (values, eg: `enablerepos`).
     :param bool dry_run:
         When True, only log what would be removed without actually deleting.
+    :return:
+        The number of VMs/VDIs/snapshots that failed to be removed.
     """
     inventory_hosts = inventory["hosts"]
     pools: list[Pool] = []
@@ -42,44 +44,64 @@ def clean_pools(inventory: Inventory, dry_run: bool = False) -> None:
 
     with ThreadPoolExecutor() as executor:
         futures = {executor.submit(clean_pool, p, dry_run): p for p in pools}
+        failures = 0
         for future in futures:
             pool = futures[future]
             try:
-                future.result()
+                failures += future.result()
             except Exception as exc:
-                logger.error(f"Cleaning pool has failed! The master {pool.master} cannot be cleaned.")
-                raise exc
+                logger.error(f"Cleaning pool has failed! The master {pool.master} cannot be cleaned: {exc}")
+                failures += 1
+    return failures
 
-def clean_pool(pool: Pool, dry_run: bool) -> None:
-    """Remove all VMs and all orphan VDIs on local SRs from a single pool."""
+def clean_pool(pool: Pool, dry_run: bool) -> int:
+    """Remove all VMs and all orphan VDIs on local SRs from a single pool.
+
+    :return:
+        The number of VMs plus number VDIs that failed to be removed from pool
+    """
     master = pool.master
     log_prefix = 'Would remove' if dry_run else 'Removing'
+    failures = 0
 
     vm_uuids = safe_split(master.xe(
         'vm-list',
         {'is-control-domain': False, 'is-a-template': False},
         minimal=True,
+    )) + safe_split(master.xe(
+        'vm-list',
+        {'is-a-snapshot': True},
+        minimal=True,
     ))
     for vm_uuid in vm_uuids:
         vm = VM(vm_uuid, master)
         logger.info(f"[{master}] {log_prefix} VM {vm.uuid} ({vm.name()})")
         if not dry_run:
-            vm.destroy(verify=True)
+            try:
+                vm.destroy(verify=True)
+            except Exception as exc:
+                logger.error(f"[{master}] Failed to remove VM {vm.uuid} ({vm.name()}): {exc}")
+                failures += 1
 
     sr_uuids = local_sr_uuids(pool)
     for sr_uuid in sr_uuids:
         for vdi_uuid in SR(sr_uuid, pool).vdi_uuids(managed=True):
             vdi = VDI(vdi_uuid, sr=SR(sr_uuid, pool))
             logger.info(f"[{master}] {log_prefix} orphan VDI {vdi.uuid} from local SR {sr_uuid}")
             if not dry_run:
-                vdi.destroy()
+                try:
+                    vdi.destroy()
+                except Exception as exc:
+                    logger.error(f"[{master}] Failed to destroy VDI {vdi.uuid}: {exc}")
+                    failures += 1
 
-    if not dry_run:
+    if not dry_run and failures == 0:
         for sr_uuid in sr_uuids:
             wait_for_not(
                 lambda: len(SR(sr_uuid, pool).vdi_uuids(managed=True)) > 0,
                 f"Wait for local SR {sr_uuid} to be empty",
             )
+    return failures
 
 def local_sr_uuids(pool: Pool) -> list[str]:
     """Return the UUIDs of the pool's local (non-shared, user) SRs."""