From d7b3dc2411dc1171189a5fc733d0685ccedb5cc5 Mon Sep 17 00:00:00 2001
From: SrikanthMyakam <m.srikanth.m@gmail.com>
Date: Sat, 25 Jan 2025 22:34:30 +0530
Subject: [PATCH] [Hyper-v platform] Delete disks on deletion of VM

When a VM is deleted, its attached disks are not automatically removed, which can lead to disk space issues. This change will address the problem by ensuring that both the OS and data disks attached to the VM are deleted across all storage controllers on VM deletion.
---
 lisa/tools/hyperv.py | 77 +++++++++++++++++++++++++++++++++++++++++---
 lisa/tools/ls.py     |  2 +-
 2 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/lisa/tools/hyperv.py b/lisa/tools/hyperv.py
index c2f42c5a82..c071360c42 100644
--- a/lisa/tools/hyperv.py
+++ b/lisa/tools/hyperv.py
@@ -5,7 +5,7 @@
 import time
 from dataclasses import dataclass
 from enum import Enum
-from typing import Any, Dict, Optional, Set
+from typing import Any, Dict, List, Optional, Set
 
 from assertpy import assert_that
 from dataclasses_json import dataclass_json
@@ -14,6 +14,7 @@
 from lisa.executable import Tool
 from lisa.operating_system import Windows
 from lisa.tools.powershell import PowerShell
+from lisa.tools.rm import Rm
 from lisa.tools.windows_feature import WindowsFeatureManagement
 from lisa.util import LisaException
 from lisa.util.process import Process
@@ -31,6 +32,25 @@ class VMSwitch:
     type: HypervSwitchType = HypervSwitchType.EXTERNAL
 
 
+class ControllerType(Enum):
+    IDE = 0
+    SCSI = 1
+
+
+@dataclass
+class VMDisk:
+    # The unique identifier of the virtual hard disk.
+    id: str = ""
+    # The file path of the virtual hard disk (VHDX) file.
+    path: str = ""
+    # The type of the controller (IDE or SCSI).
+    controller_type: ControllerType = ControllerType.IDE
+    # The number of the controller to which the virtual hard disk is attached.
+    controller_number: int = 0
+    # The location of the controller to which the virtual hard disk is attached.
+    controller_location: int = 0
+
+
 class HyperV(Tool):
     # Internal NAT network configuration
     INTERNAL_NAT_ROUTER = "192.168.5.1"
@@ -62,8 +82,52 @@ def exists_vm(self, name: str) -> bool:
 
         return bool(output.strip() != "")
 
+    def get_vm_disks(self, name: str) -> List[VMDisk]:
+        vm_disks: List[VMDisk] = []
+        output = self.node.tools[PowerShell].run_cmdlet(
+            f"Get-VMHardDiskDrive -VMName {name} ",
+            force_run=True,
+            output_json=True,
+        )
+        if not output:
+            return []
+        # above command returns a list of disks if there are multiple disks.
+        # if there is only one disk, it returns a single disk but not a list.
+        # so convert the output to a list if it is not already a list
+        if not isinstance(output, list):
+            output = [output]
+        for disk in output:
+            vm_disks.append(
+                VMDisk(
+                    id=disk["Id"],
+                    path=disk["Path"],
+                    controller_type=disk["ControllerType"],
+                    controller_number=disk["ControllerNumber"],
+                    controller_location=disk["ControllerLocation"],
+                )
+            )
+        return vm_disks
+
+    def delete_vm_disks(self, name: str) -> None:
+        # get vm disks
+        vm_disks = self.get_vm_disks(name)
+
+        # delete vm disks
+        for disk in vm_disks:
+            self.node.tools[PowerShell].run_cmdlet(
+                f"Remove-VMHardDiskDrive -VMName {name} "
+                f"-ControllerType {disk.controller_type} "
+                f"-ControllerNumber {disk.controller_number} "
+                f"-ControllerLocation {disk.controller_location}",
+                force_run=True,
+            )
+            self.node.tools[Rm].remove_file(
+                disk.path,
+                sudo=True,
+            )
+
     def delete_vm_async(self, name: str) -> Optional[Process]:
-        # check if vm is present
+        # check if VM is present
         if not self.exists_vm(name):
             return None
 
@@ -72,9 +136,14 @@ def delete_vm_async(self, name: str) -> Optional[Process]:
         self.delete_nat_mapping(internal_ip=internal_ip)
 
         # stop and delete vm
+        # stop the VM before deleting it
         self.stop_vm(name=name)
-        powershell = self.node.tools[PowerShell]
-        return powershell.run_cmdlet_async(
+
+        # delete VM disks before deleting vm
+        self.delete_vm_disks(name)
+
+        # delete vm
+        return self.node.tools[PowerShell].run_cmdlet_async(
             f"Remove-VM -Name {name} -Force",
             force_run=True,
         )
diff --git a/lisa/tools/ls.py b/lisa/tools/ls.py
index ec2db80622..915c8702f2 100644
--- a/lisa/tools/ls.py
+++ b/lisa/tools/ls.py
@@ -92,7 +92,7 @@ def is_file(self, path: PurePath, sudo: bool = False) -> bool:
 
     def path_exists(self, path: str, sudo: bool = False) -> bool:
         output = self.node.tools[PowerShell].run_cmdlet(
-            f"Test-Path {path}",
+            f"Test-Path '{path}'",
             force_run=True,
             sudo=sudo,
         )