-
Notifications
You must be signed in to change notification settings - Fork 226
Azure Files NFSv4 Support for XFStesting #4253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 adds comprehensive Azure Files NFSv4.1 protocol support to LISA's testing framework. The changes introduce a unified AzureFileShare class that handles both SMB and NFS protocols through a configurable set_protocol() method, while preserving the legacy Nfs class for backward compatibility.
Changes:
- Adds protocol-aware Azure File Share management with NFS and SMB support via new enums and
set_protocol()configuration - Implements parallel NFSv4.1 xfstests validation (
verify_azure_file_share_nfsv4) with 73 validated test cases across 4 workers - Refactors storage test to use unified AzureFileShare class (
verify_nfsv4_basic, renamed fromverify_azure_file_share_nfs)
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lisa/sut_orchestrator/azure/features.py | Adds FileShareProtocol/AuthMode/Connectivity enums, unified AzureFileShare with set_protocol() method, NFS-specific storage account configuration, preserves legacy Nfs class |
| lisa/sut_orchestrator/azure/common.py | Implements protocol-aware file share creation/deletion using ARM API for NFS (shared key disabled) and data plane API for SMB |
| lisa/microsoft/testsuites/xfstests/xfstests.py | Adds deferred notification support for parallel workers, file existence timeout handling, random delay to prevent SSH connection pool contention |
| lisa/microsoft/testsuites/xfstests/xfstesting.py | Implements verify_azure_file_share_nfsv4 with parallel workers, NFS worker setup helper, unified cleanup method for SMB/NFS protocols |
| lisa/microsoft/testsuites/core/storage.py | Renames verify_azure_file_share_nfs to verify_nfsv4_basic, migrates from legacy Nfs class to unified AzureFileShare with NFS protocol |
| def create_share( | ||
| self, | ||
| quota_in_gb: int = 100, | ||
| ) -> None: | ||
| """ | ||
| Create a file share using the configured protocol. | ||
| For NFS: Creates Premium storage account with private endpoint. | ||
| For SMB: Creates Standard storage account (public or private endpoint). | ||
| Must call set_protocol() before this method to configure NFS. | ||
| """ | ||
| platform: AzurePlatform = self._platform # type: ignore | ||
| node_context = self._node.capability.get_extended_runbook(AzureNodeSchema) | ||
| location = node_context.location | ||
| resource_group_name = self._resource_group_name | ||
|
|
||
| random_str = generate_random_chars(string.ascii_lowercase + string.digits, 10) | ||
| self._file_share_names = [f"lisa{random_str}fs"] | ||
| self._private_endpoint_name = f"{self._storage_account_name}-file-pe" | ||
|
|
||
| if self._protocol == FileShareProtocol.NFS: | ||
| # NFS requires Premium_LRS, FileStorage, HTTPS disabled | ||
| check_or_create_storage_account( | ||
| credential=platform.credential, | ||
| subscription_id=platform.subscription_id, | ||
| cloud=platform.cloud, | ||
| account_name=self._storage_account_name, | ||
| resource_group_name=resource_group_name, | ||
| location=location, | ||
| log=self._log, | ||
| sku="Premium_LRS", | ||
| kind="FileStorage", | ||
| enable_https_traffic_only=False, | ||
| ) | ||
| get_or_create_file_share( | ||
| credential=platform.credential, | ||
| subscription_id=platform.subscription_id, | ||
| cloud=platform.cloud, | ||
| account_name=self._storage_account_name, | ||
| file_share_name=self._file_share_names[0], | ||
| resource_group_name=resource_group_name, | ||
| protocols="NFS", | ||
| log=self._log, | ||
| quota_in_gb=quota_in_gb, | ||
| ) | ||
| else: | ||
| # SMB uses Standard storage | ||
| check_or_create_storage_account( | ||
| credential=platform.credential, | ||
| subscription_id=platform.subscription_id, | ||
| cloud=platform.cloud, | ||
| account_name=self._storage_account_name, | ||
| resource_group_name=resource_group_name, | ||
| location=location, | ||
| log=self._log, | ||
| sku="Standard_LRS", | ||
| kind="StorageV2", | ||
| enable_https_traffic_only=True, | ||
| allow_shared_key_access=False, | ||
| ) | ||
| get_or_create_file_share( | ||
| credential=platform.credential, | ||
| subscription_id=platform.subscription_id, | ||
| cloud=platform.cloud, | ||
| account_name=self._storage_account_name, | ||
| file_share_name=self._file_share_names[0], | ||
| resource_group_name=resource_group_name, | ||
| log=self._log, | ||
| quota_in_gb=quota_in_gb, | ||
| ) | ||
|
|
||
| # Create private endpoint if required | ||
| if self._connectivity == FileShareConnectivity.PRIVATE_ENDPOINT: | ||
| self._setup_private_endpoint(location) |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The create_share() method has a subtle bug when called for NFS shares without calling set_protocol() first. If a caller uses create_share() directly for NFS (as shown in the docstring "Must call set_protocol() before this method"), but forgets to call set_protocol(), the method will:
- Use the default SMB protocol from
_initialize()(line 4183) - Use the SMB storage account prefix from
_initialize_fileshare_information()(line 4193) - Create an NFS share with the wrong storage account naming convention
This creates a mismatch where the storage account has the "lisasmb" prefix but contains NFS shares.
Consider either:
- Add validation at the start of
create_share()to check protocol compatibility with the requested share type - Or remove
create_share()and require callers to useset_protocol()+create_file_share()instead for explicit protocol handling
| sku="Standard_LRS", | ||
| kind="StorageV2", | ||
| enable_https_traffic_only=True, | ||
| allow_shared_key_access=False, |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The create_share() method for SMB protocol hardcodes allow_shared_key_access=False (line 4110), but the default authentication mode is FileShareAuthMode.SHARED_KEY (set in _initialize() at line 4186). This creates an inconsistency:
- The auth mode says "use shared key"
- But the storage account disables shared key access
This will cause authentication failures when attempting to mount SMB shares created via create_share(). The method should either:
- Set
allow_shared_key_access=Truefor SMB when_auth_mode == FileShareAuthMode.SHARED_KEY - Or explicitly document that
create_share()only supports managed identity/Kerberos authentication for SMB
Note: This issue doesn't affect existing create_file_share() callers since they explicitly pass allow_shared_key_access as a parameter.
| allow_shared_key_access=False, | |
| allow_shared_key_access=( | |
| self._auth_mode == FileShareAuthMode.SHARED_KEY | |
| ), |
| if self._connectivity == FileShareConnectivity.PRIVATE_ENDPOINT: | ||
| delete_private_dns_zone_groups( | ||
| platform, | ||
| resource_group_name, | ||
| self._log, | ||
| private_endpoint_name=self._private_endpoint_name, | ||
| ) | ||
| delete_virtual_network_links(platform, resource_group_name, self._log) | ||
| delete_record_sets(platform, resource_group_name, self._log) | ||
| delete_private_zones(platform, resource_group_name, self._log) | ||
| delete_private_endpoints( | ||
| platform, | ||
| resource_group_name, | ||
| self._log, | ||
| private_endpoint_name=self._private_endpoint_name, | ||
| ) |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The delete_share() method unconditionally deletes private endpoint resources when _connectivity == PRIVATE_ENDPOINT (lines 4137-4152), without checking if the private endpoint was created by LISA (_private_endpoint_created_by_lisa flag).
This could delete pre-existing private endpoints that LISA reused rather than created. The cleanup logic should respect the _private_endpoint_created_by_lisa flag, similar to how delete_azure_fileshare() does at lines 4374-4421.
The fix should be:
if self._connectivity == FileShareConnectivity.PRIVATE_ENDPOINT and self._private_endpoint_created_by_lisa:
# delete private endpoint resourcesThis ensures pre-existing private endpoints are preserved when LISA reuses existing infrastructure.
| # Check if share exists using ARM API | ||
| try: | ||
| storage_client.file_shares.get( | ||
| resource_group_name=resource_group_name, | ||
| account_name=account_name, | ||
| share_name=file_share_name, | ||
| ) | ||
| log.debug(f"file share {file_share_name} already exists") | ||
| except Exception: | ||
| # Share doesn't exist, create it | ||
| log.debug( | ||
| f"creating file share {file_share_name} with protocols {protocols}" | ||
| ) | ||
| from azure.mgmt.storage.models import FileShare | ||
|
|
||
| file_share = FileShare( | ||
| enabled_protocols=protocols, | ||
| share_quota=quota_in_gb, | ||
| ) | ||
| storage_client.file_shares.create( | ||
| resource_group_name=resource_group_name, | ||
| account_name=account_name, | ||
| share_name=file_share_name, | ||
| file_share=file_share, | ||
| ) |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exception handling at line 2284 catches all exceptions with a bare except Exception:, which will mask actual errors (like authentication failures, network errors, or resource group not found errors).
This should be refined to only catch the specific exception for "resource not found" (typically ResourceNotFoundError from azure.core.exceptions). Other exceptions should propagate to help diagnose configuration or permission issues.
Recommended fix:
from azure.core.exceptions import ResourceNotFoundError
try:
storage_client.file_shares.get(...)
log.debug(f"file share {file_share_name} already exists")
except ResourceNotFoundError:
# Share doesn't exist, create it
...This makes error handling more precise and helps debugging when real issues occur.
Pull Request Summary: Azure Files NFSv4 Support
Overview
This PR adds comprehensive NFSv4.1 protocol support for Azure Files testing in LISA, including:
AzureFileShareclass that handles both SMB and NFS protocolsverify_azure_file_share_nfsv4) with parallel worker executionverify_nfsv4_basic) for quick validationNfsclass is preserved for backward compatibility (to be removed in future)Key Changes
1. Unified
AzureFileShareClass with Protocol SupportFile:
lisa/sut_orchestrator/azure/features.pyThe
AzureFileShareclass now supports both SMB and NFS protocols viaset_protocol():New Enums Added:
FileShareProtocolFileShareAuthModeNfsSecurityModeFileShareConnectivityStorage Account Naming:
lisasmb<random10chars>lisanfs<random10chars>2. NFS xfstests Test Case:
verify_azure_file_share_nfsv4File:
lisa/microsoft/testsuites/xfstests/xfstesting.pyFully implemented xfstests validation with parallel worker execution:
vers=4,minorversion=1,sec=sysNFS Test Configuration:
3. NFS Smoke Test:
verify_nfsv4_basicFile:
lisa/microsoft/testsuites/core/storage.pyRenamed from
verify_azure_file_share_nfstoverify_nfsv4_basic. Simple NFS mount validation:4. Legacy
NfsClass PreservedFile:
lisa/sut_orchestrator/azure/features.pyThe original
Nfsclass is unchanged and preserved for backward compatibility:AzureFeatureMixin, features.Nfs5. NFS Worker Setup Helper
File:
lisa/microsoft/testsuites/xfstests/xfstesting.pyNew helper method
_setup_azure_nfs_workers()that:AzureFileSharefor NFS protocol_deploy_azure_file_share()NFSClienttoollocal.configfor each workerTechnical Details
Azure Files NFS Requirements
Premium_LRSorPremiumV2_LRS(Default)FileStorageNFS Mount Options
Resource Naming Conventions
lisasmb<random10chars>lisanfs<random10chars><storageaccount>-file-peArchitecture
Protocol Selection Flow
Parallel Worker Architecture (xfstests)
Files Changed
lisa/sut_orchestrator/azure/features.pyAzureFileSharewithset_protocol(), preservedNfsclasslisa/microsoft/testsuites/xfstests/xfstesting.pyverify_azure_file_share_nfsv4,_setup_azure_nfs_workers(), NFS test configlisa/microsoft/testsuites/core/storage.pyverify_azure_file_share_nfs→verify_nfsv4_basic, uses unifiedAzureFileShareTesting
Test Cases
verify_azure_file_share_nfsv4verify_nfsv4_basicverify_azure_file_shareRun Commands
Recommended Test Images
canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latestcanonical ubuntu-24_04-lts server latestredhat rhel 9_5 latestmicrosoftcblmariner azure-linux-3 azure-linux-3-gen2 latestNFS Test Exclusions
Tests excluded due to Azure Files NFS limitations:
link()syscall not supportedmknod/mkfifonot supportedFuture Work
Nfsclass once all consumers migrate toAzureFileShareFileShareAuthMode.MANAGED_IDENTITY)Breaking Changes
verify_azure_file_share_nfsrenamedverify_nfsv4_basicBackward Compatibility
verify_azure_file_share(SMB xfstests)AzureFileShareclassNfsclass (lisa.sut_orchestrator.azure.features)Nfsbase class (lisa.features.nfs)References
Key Test Cases:
verify_azure_file_share_nfsv4 | verify_nfsv4_basicImpacted LISA Features: AzureFileShare, Nfs
Tested Azure Marketplace Images:
canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latestcanonical ubuntu-24_04-lts server latestredhat rhel 9_5 latestmicrosoftcblmariner azure-linux-3 azure-linux-3-gen2 latest