Enhance VM naming, cleanup logic, and xfstests orchestration by psachdeva-ms · Pull Request #4702 · microsoft/lisa

Piyush Sachdeva (psachdeva-ms) · 2026-09-04T10:00:35Z

This pull request introduces improvements in two main areas: (1) enhanced handling and reporting of "expunged" test cases in the xfstests suite, and (2) improved Azure resource cleanup logic to avoid storage leaks and better handle resource group deletion scenarios. The changes add a new test status (EXPUNGED), ensure accurate reporting and notification of such cases, and introduce logic to disable Azure file share soft-delete to prevent capacity leaks during repeated test runs.

Xfstests "expunged" test case support and reporting:

Added a new EXPUNGED status to the TestStatus enum, updated logic to recognize, process, and report expunged xfstests cases throughout the test result pipeline, including result aggregation, JUnit output, and summary display.
Refactored xfstests result parsing and notification logic to accurately extract expunged cases from raw output, distinguish them from other skipped cases, and emit them with the correct status and counts.

Azure resource cleanup and reliability improvements:

Added _disable_share_soft_delete to programmatically disable Azure file share soft-delete, preventing provisioned capacity leaks from previous test runs by ensuring deleted shares are actually removed. \
Improved logic in environment deletion to perform per-resource cleanup when the resource group is specified and deploy is true, with clear logging and error handling.

Dependency and import updates:

Updated Azure SDK imports to include new types required for file share property management and resource cleanup.

These changes improve test reporting accuracy and Azure resource management, reducing the risk of resource leaks and making test results more informative and actionable.## Description

Related Issue

Type of Change

Checklist

Description is filled in above
No credentials, secrets, or internal details are included
Peer review requested (if not, add required peer reviewers after raising PR)
Tests executed and results posted below

Adding a datetime parameter to make the vm name unique. When the user specifies a target resource group, we now set the vm nameing convention as: "<resource_group_name>-<date>-<time>-n0", to prevent name collision arising from the default naming convention: "<resource_group_name>-n0". Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com> Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>

The existing cleanup code in platform only deletes the resource group and all its resources when a name is not given by the user and lisa ends up generating one. This is based on the assumption that the user wants to use an already deployed resource group. Hence, if a resource group is specified none of its resources are deleted at cleanup. This patch allows deletion of the VM and its attached resource, if the VM was deployed by LISA's platform section (only), depending on the value of `keep_environment` in it. Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com> Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>

Copilot

🟡 Changes recommended

It introduces EXPUNGED/soft-delete behavior but leaves soft-delete disabling unhooked and produces blank messages for expunged subtests, reducing the usefulness of the new reporting.

Key Test Cases:
verify_azure_file_share|verify_azure_file_share_nfsv4|verify_reboot_in_platform

Impacted LISA Features:
NetworkInterface, Disk, Nfs

Tested Azure Marketplace Images:

canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
redhat rhel 9_5 latest

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves Azure orchestration and xfstests reporting by (a) introducing an EXPUNGED test status and plumbing it through reporting pipelines, and (b) enhancing Azure environment cleanup and VM naming to reduce resource leaks and collisions when reusing a user-specified resource group.

Changes:

Added TestStatus.EXPUNGED support across runner summary output and JUnit XML generation.
Refactored xfstests subtest parsing/notification to detect expunged cases and track skipped/expunged counts in aggregated worker results.
Added per-resource cleanup when a resource group is user-specified (and deploy is true), plus VM name generation that includes a timestamp for uniqueness.

File summaries

File	Description
lisa/sut_orchestrator/azure/platform_.py	Adds per-resource cleanup path for user-specified RGs and generates unique VM names with timestamps.
lisa/sut_orchestrator/azure/common.py	Introduces soft-delete disabling helper for Azure Files shares and updates Azure storage SDK model imports.
lisa/runner.py	Hides ATTEMPTED/EXPUNGED from summary output when counts are zero.
lisa/notifiers/junit.py	Treats EXPUNGED as a skipped test in JUnit output.
lisa/microsoft/testsuites/xfstests/xfstests.py	Adds expunged/skipped accounting and emits EXPUNGED subtest messages based on parsed output.
lisa/messages.py	Adds `EXPUNGED` to `TestStatus` and marks it as a completed status.

Review details

Files reviewed: 7/7 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+                result = XfstestsResult(
                    name=case,
-                    status=TestStatus.PASSED,
+                    status=status,
                    message=self.extract_case_content(case, raw_message),
                )


Copilot

🟡 Changes recommended

The Azure Files soft-delete mitigation is currently unused and the new cleanup/reporting paths include correctness/robustness issues that can lead to leaks or unstable output.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

lisa/sut_orchestrator/azure/common.py:2301

_disable_share_soft_delete() is introduced but never called anywhere in the repo, so share soft-delete will remain enabled and the capacity-leak mitigation described in the PR won’t take effect. Either wire this helper into the Azure Files provisioning path (e.g., before deleting/recreating shares) or remove it to avoid dead code.

def _disable_share_soft_delete(

lisa/microsoft/testsuites/xfstests/xfstests.py:1361

Returning sets from _parse_xfstests_cases causes non-deterministic per-case emission order (and potential flaky ordering in downstream reporting). Return lists for empty not_run/fail collections in this expunged-only path to keep ordering stable and consistent with other paths.

                return [], set(), set(), expunged_cases

lisa/microsoft/testsuites/xfstests/xfstests.py:1390

_parse_xfstests_cases returns not_run/fail cases as sets, which loses the original output order and makes subtest notification ordering non-deterministic. Return the original lists (while still using sets only for membership checks) so results are stable across runs.

        return pass_cases, not_run_cases_set, fail_cases_set, []

Files reviewed: 7/7 changed files
Comments generated: 5
Review effort level: Lite


+    def _parse_xfstests_cases(
+        self, raw_message: str, test_section: str
+    ) -> Optional[tuple[List[str], set[str], set[str], List[str]]]:


+                nic = network_client.network_interfaces.get(
+                    resource_group_name, nic_name
+                )
+                for ip_config in nic.ip_configurations:
+                    if ip_config.public_ip_address and ip_config.public_ip_address.id:
+                        pip_name = get_matched_str(
+                            ip_config.public_ip_address.id,
+                            PATTERN_PUBLIC_IP_NAME,
+                        )
+                        if pip_name:
+                            public_ip_names.append(pip_name)
+            except Exception as e:


                self.log.info(
                    f"Worker {worker_result.run_id}: PASSED "
-                    f"({worker_result.total_count} tests)"
+                    f"({worker_result.total_count} tests, "
+                    f"{skipped} skipped, "
+                    f"{worker_result.expunged_count} expunged)"


            return
        assert self._azure_runbook

        if not environment_context.resource_group_is_specified:


+            except Exception as e:
+                log.debug(f"error deleting {resource_type} {name}: {e}")


Copilot

🟡 Changes recommended

The updated xfstests aggregation/parsing introduces inaccurate skipped/expunged reporting and non-deterministic case emission ordering that can make results misleading or flaky.

Key Test Cases:
verify_generic_standard_datadisk|verify_azure_file_share|verify_azure_file_share_nfsv4

Impacted LISA Features:
AzureFileShare, NetworkInterface, Disk

Tested Azure Marketplace Images:

canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
redhat rhel 9_5 latest

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

lisa/microsoft/testsuites/xfstests/xfstests.py:520

The worker PASSED log reports "{skipped} skipped" where skipped already includes expunged, and then also reports "{expunged_count} expunged" separately, which double-counts expunged cases and makes the output misleading. Consider formatting as "{skipped} skipped ({expunged_count} expunged)" if you want skipped to include expunged.

                    f"({worker_result.total_count} tests, "
                    f"{skipped} skipped, "
                    f"{worker_result.expunged_count} expunged)"

lisa/microsoft/testsuites/xfstests/xfstests.py:528

The worker FAILED log reports "{skipped} skipped" where skipped already includes expunged, and then also reports expunged again, which double-counts expunged cases and makes the output misleading. Consider formatting as "{skipped} skipped ({expunged_count} expunged)" if you want skipped to include expunged.

                    f"tests failed, {skipped} skipped, "
                    f"{worker_result.expunged_count} expunged)"

lisa/microsoft/testsuites/xfstests/xfstests.py:1390

This return currently emits sets for not_run_cases/fail_cases, which makes the per-test notifications order non-deterministic across runs. Return the original lists to preserve ordering from xfstests output (and to align with the intended API contract).

        return pass_cases, not_run_cases_set, fail_cases_set, []

Files reviewed: 7/7 changed files
Comments generated: 2
Review effort level: Lite


+    def _parse_xfstests_cases(
+        self, raw_message: str, test_section: str
+    ) -> Optional[tuple[List[str], set[str], set[str], List[str]]]:


+                except Exception as e:
+                    log.warning(
+                        f"error during per-resource cleanup in "
+                        f"{resource_group_name}: {e}"
+                    )


LiliDeng · 2026-09-04T15:01:55Z

+                try:
+                    self._delete_nodes_resources(environment, log)
+                except Exception as e:
+                    log.warning(


don't use log.warning

LiliDeng · 2026-09-07T02:30:05Z

+            # Genuine setup failure: no recognizable test output was
+            # produced. Log a warning and skip sending notifications for
+            # this section instead of marking the run as failed.
+            self._log.warning(


don't use warning

LiliDeng · 2026-09-07T02:30:22Z

+            )
+            log.debug(f"Disabled share soft-delete on storage account {account_name}")
+    except Exception as e:
+        log.warning(f"Failed to disable share soft-delete on {account_name}: {e}")


don't use warning

LiliDeng · 2026-09-07T02:39:05Z

+                )
+                wait_operation(operation, failure_identity=f"delete VM {vm_name}")
+            except Exception as e:
+                log.warning(f"error deleting VM {vm_name}: {e}")


don't use warning

Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com> Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>

The status of a test run has been modified to not be marked as a failure when no tests are run due to being expunged or skipped. Refactoring was needed as the code complexity increased and was violating the style guidelines: Split the parse logic into _parse_xfstests_cases and the per-case send logic into _send_case_result_message, leaving create_send_subtest_msg as a thin orchestrator: parse -> update counts -> emit. The four duplicate result-building loops are collapsed into one ordered (cases, status) loop, preserving the fail -> pass -> not_run -> expunged emission order consumers rely on. Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com> Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>

Collapse the duplicated SMB and NFS parallel-xfstests paths into a single protocol-parameterized implementation. - Merge _setup_azure_nfs_workers into _setup_azure_file_share_workers, which now takes a `protocol` parameter (default SMB). Shared scaffolding -- share creation, mount-point setup, the worker loop, set_local_config and set_excluded_tests -- is common; only the deploy kwargs, mount options and excluded-test list stay gated on the protocol. - Extract _prepare_worker_devices to build each worker's (test_dev, scratch_dev) and perform the NFS mount, leaving the worker loop free of per-iteration protocol branching. This also drops a storage-account-name local that was only bound on the NFS path. - Merge the verify_azure_file_share and verify_azure_file_share_nfsv4 bodies into _run_azure_file_share_xfstests(protocol); both test methods are now thin delegations that keep their own TestCaseMetadata. - Trim the oversized module docstring and repeated comment banners. SMB and NFS runtime behavior is unchanged; only some log strings differ. Net ~285 fewer lines. Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com> Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>

Piyush Sachdeva (psachdeva-ms) · 2026-09-07T23:37:57Z

LiliDeng - resolved all log.warning(), and copilot review suggestions.

Piyush Sachdeva (psachdeva-ms) · 2026-09-07T23:40:59Z

        return []


+def _disable_share_soft_delete(


LiliDeng - This function would also disable soft delete for any user-provided SA, irrespective of if it is enabled or not.
Is that a problem that we need to take care of, as I don't think xfstests right now has the ability to take a user-provided SA, at least not in the upstream code?

Copilot

🟡 Changes recommended

The xfstests module introduces a Python 3.8-incompatible type annotation (tuple[...]) that can break imports at runtime.

Key Test Cases:
verify_azure_file_share|verify_azure_file_share_nfsv4

Impacted LISA Features:
NetworkInterface, Disk

Tested Azure Marketplace Images:

canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
debian debian-12 12 latest

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 7/7 changed files
Comments generated: 2
Review effort level: Lite

+    def _parse_xfstests_cases(
+        self, raw_message: str, test_section: str
+    ) -> Optional[tuple[List[str], List[str], List[str], List[str]]]:
+        """Parse the Ran/Not run/Failures lines from xfstests output.


+                try:
+                    self._delete_nodes_resources(environment, log)
+                except Exception as e:
+                    log.info(
+                        f"error during per-resource cleanup in "
+                        f"{resource_group_name}: {e}"
+                    )


Piyush Sachdeva (psachdeva-ms) added 2 commits September 4, 2026 01:26

Piyush Sachdeva (psachdeva-ms) requested review from LiliDeng, paxue and Shekhar Sorot ( MSFT ) (shekharsorot) as code owners September 4, 2026 10:00

Copilot AI lite review requested due to automatic review settings September 4, 2026 10:00

Piyush Sachdeva (psachdeva-ms) requested a review from Johnson (johnsongeorge-w) as a code owner September 4, 2026 10:00

Copilot started reviewing on behalf of Piyush Sachdeva (psachdeva-ms) September 4, 2026 10:02 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread lisa/sut_orchestrator/azure/common.py

Comment thread lisa/microsoft/testsuites/xfstests/xfstests.py

Comment on lines +1471 to 1475

result = XfstestsResult(

name=case,

status=TestStatus.PASSED,

status=status,

message=self.extract_case_content(case, raw_message),

)

Piyush Sachdeva (psachdeva-ms) force-pushed the psachdeva/for-master branch from 371899d to 4bf750a Compare September 4, 2026 10:46

Copilot AI review requested due to automatic review settings September 4, 2026 10:46

Copilot started reviewing on behalf of Piyush Sachdeva (psachdeva-ms) September 4, 2026 10:47 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Piyush Sachdeva (psachdeva-ms) force-pushed the psachdeva/for-master branch from 4bf750a to d173fb1 Compare September 4, 2026 12:10

Copilot AI review requested due to automatic review settings September 4, 2026 12:10

Copilot started reviewing on behalf of Piyush Sachdeva (psachdeva-ms) September 4, 2026 12:11 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

LiliDeng reviewed Sep 4, 2026

View reviewed changes

LiliDeng reviewed Sep 7, 2026

View reviewed changes

Piyush Sachdeva (psachdeva-ms) added 4 commits September 8, 2026 05:04

lisa: Disabling soft delete for new Azure Storage Accounts

f14f019

Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com> Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>

xfstests: Print results once after all workers are done

253783e

Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com> Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>

Copilot AI review requested due to automatic review settings September 7, 2026 23:37

Piyush Sachdeva (psachdeva-ms) force-pushed the psachdeva/for-master branch from d173fb1 to c65a04d Compare September 7, 2026 23:37

Copilot started reviewing on behalf of Piyush Sachdeva (psachdeva-ms) September 7, 2026 23:37 View session

Piyush Sachdeva (psachdeva-ms) commented Sep 7, 2026

View reviewed changes

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Navigation Menu

Enhance VM naming, cleanup logic, and xfstests orchestration - #4702

Enhance VM naming, cleanup logic, and xfstests orchestration#4702
Piyush Sachdeva (psachdeva-ms) wants to merge 6 commits into
microsoft:mainfrom
psachdeva-ms:psachdeva/for-master

Piyush Sachdeva (psachdeva-ms) commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

LiliDeng Sep 4, 2026

Uh oh!

LiliDeng Sep 7, 2026

Uh oh!

LiliDeng Sep 7, 2026

Uh oh!

LiliDeng Sep 7, 2026

Uh oh!

Piyush Sachdeva (psachdeva-ms) commented Sep 7, 2026

Uh oh!

Piyush Sachdeva (psachdeva-ms) Sep 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		except Exception as e:
		log.debug(f"error deleting {resource_type} {name}: {e}")

Conversation

Piyush Sachdeva (psachdeva-ms) commented Sep 4, 2026

Related Issue

Type of Change

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

LiliDeng Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

LiliDeng Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

LiliDeng Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

LiliDeng Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

Piyush Sachdeva (psachdeva-ms) commented Sep 7, 2026

Uh oh!

Piyush Sachdeva (psachdeva-ms) Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants