Skip to content

Conversation

@shekharsorot
Copy link
Contributor

@shekharsorot shekharsorot commented Dec 13, 2025

fix(azure): Enable storage account and private endpoint reuse for AzureFileShare on re-runs

Problem

When LISA tests are re-run on the same VM (with keep_environment: true), the AzureFileShare feature was:

  1. Failing to reuse existing storage accounts - Creating duplicate resources
  2. Failing to reuse private endpoints - Causing conflicts and failures
  3. Corrupting /etc/fstab - Leaving stale mount entries that caused boot issues
  4. Credential file conflicts - Single credential file was overwritten when multiple storage accounts existed

Solution

This PR implements intelligent resource reuse with proper tracking and conditional cleanup.

Important Changes

1. Storage Account Reuse (features.py)

  • Added _storage_account_reused: bool tracking flag
  • Checks for existing storage accounts with lisasc* prefix before creating new ones
  • Replaced @lru_cache with module-level _storage_account_cache dict (fixes unhashable object error)

2. Private Endpoint Reuse (features.py, common.py)

  • Added _private_endpoint_created_by_lisa: bool tracking flag
  • create_update_private_endpoints() now returns Tuple[str, bool] (name, was_created)
  • New _get_private_endpoint_ip() helper with fallback methods for IP retrieval
  • Unique PE naming: <storage_account_name>-file-pe, ensures future implementations from other tests are not impacted.

3. Credential File Management (features.py)

  • Storage-account-specific credential files: /etc/smbcredentials/<storage_account_name>.cred
  • Added credential_file property for external access
  • Class constant CREDENTIAL_DIR = "/etc/smbcredentials"

4. Safe Cleanup Logic (features.py)

  • Extracted cleanup into focused sub-methods:
    • _delete_file_shares() - Always deletes file shares
    • _cleanup_private_endpoint_resources() - Conditional PE cleanup
    • _cleanup_storage_account() - Conditional storage account cleanup
    • _cleanup_vm_state() - Cleans fstab entries and credentials
  • Uses sed for surgical fstab entry removal (not backup restore)

5. Code Quality Improvements

  • Added comprehensive class-level docstring for AzureFileShare
  • Added class constants: STORAGE_ACCOUNT_PREFIX, PRIVATE_ENDPOINT_SUFFIX, CREDENTIAL_DIR
  • Enhanced docstrings with full Args/Returns/Raises documentation
  • Fixed type annotations throughout

Cleanup Behavior Matrix

Resource LISA Created Pre-existing
File Shares ✅ Delete ✅ Delete
Storage Account ✅ Delete ❌ Keep
Private Endpoint ✅ Delete ❌ Keep
fstab entries ✅ Remove ✅ Remove
Credential files ✅ Remove ✅ Remove

Resource Naming Conventions

Resource Pattern Example
Storage Account lisasc<random10> lisascab12cd34ef
Private Endpoint <storage>-file-pe lisascab12cd34ef-file-pe
Credential File /etc/smbcredentials/<storage>.cred /etc/smbcredentials/lisascab12cd34ef.cred

Files Changed

  • lisa/sut_orchestrator/azure/features.py - AzureFileShare class enhancements
  • lisa/sut_orchestrator/azure/common.py - PE helper functions, cache, list_file_shares
  • lisa/microsoft/testsuites/storage/storage.py - Simplified cleanup logic
  • lisa/microsoft/testsuites/xfstests/xfstesting.py - Uses credential_file property

Testing

  • ✅ All unit tests pass
  • ✅ CI pipeline passing
  • ✅ Verified re-run scenarios with keep_environment: true
  • ✅ Verified clean-run scenarios create resources correctly
  • ✅ Verified cleanup properly respects pre-existing resources

Impact

  • No breaking changes - Existing test configurations work unchanged
  • Improved reliability - Re-runs no longer fail due to resource conflicts
  • Cost efficiency - Reuses existing resources instead of creating duplicates
  • Safer cleanup - Never deletes resources LISA didn't create

…reFileShare on re-runs

fix(azure): Enable storage account and private endpoint reuse for AzureFileShare on re-runs

Problem:
When LISA was re-run with preserved resource groups (using deploy:false),
the AzureFileShare feature would create new storage accounts and file shares
instead of reusing existing ones, causing resource tracking and cleanup issues.
Additionally, re-runs would fail with IndexError when private endpoints
already existed.

Changes:

1. features.py - _initialize_fileshare_information():
   - Added logic to scan for existing 'lisasc*' storage accounts in the
     resource group and reuse them instead of generating new random names
   - Added import for get_storage_client

2. common.py - create_update_private_endpoints():
   - Added check for existing private endpoints before attempting creation
   - Added new helper function _get_private_endpoint_ip() that extracts
     IP address using two methods:
     a) From custom_dns_configs (original approach)
     b) From network interface (fallback for existing endpoints)
   - Fixes IndexError when private endpoint already exists but
     custom_dns_configs is empty

3. storage.py - verify_cifs_basic():
   - Fixed test to pass allow_shared_key_access=True since the test uses
     key-based authentication (credentials stored in /etc/smbcredentials/)
   - Added TODO comment to flag test for future validation with these changes

Tested:
- verify_cifs_basic: PASSED (fresh deployment)
- verify_azure_file_share: Requires validation with re-run scenario

Impact:
- Enables proper resource reuse when re-running LISA with preserved environments
- Prevents orphaned storage accounts on failed/retried test runs
- Fixes private endpoint IP retrieval for existing endpoints
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request enables resource reuse for AzureFileShare operations when LISA is re-run with preserved resource groups. The changes address issues where storage accounts and private endpoints were being recreated instead of reused, and fixes an IndexError when retrieving IP addresses from existing private endpoints.

Key changes include:

  • Storage account reuse logic to scan for existing "lisasc*" accounts before creating new ones
  • Private endpoint IP retrieval improvements with fallback methods and existence checking
  • Test fix to explicitly pass allow_shared_key_access=True for key-based authentication

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
lisa/sut_orchestrator/azure/features.py Adds storage account reuse logic by scanning existing accounts with "lisasc" prefix; adds get_storage_client import
lisa/sut_orchestrator/azure/common.py Adds private endpoint existence check and _get_private_endpoint_ip helper with fallback IP retrieval methods (custom_dns_configs and network interface)
lisa/microsoft/testsuites/core/storage.py Fixes test to pass allow_shared_key_access=True; adds TODO comment noting the test doesn't use private endpoints

Key Test Cases:
verify_cifs_basic|verify_azure_file_share

Impacted LISA Features:
AzureFileShare, Nfs

Tested Azure Marketplace Images:

  • canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
  • debian debian-12 12-gen2 latest
  • redhat rhel 95_gen2 latest

@LiliDeng LiliDeng merged commit 031a835 into microsoft:main Dec 23, 2025
27 checks passed
@shekharsorot shekharsorot deleted the shsorot/azurefileshare_enhance branch January 5, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants