Azure: check SRIOV on all nics and retry switch_sriov

[mcgov (mcgov)]

Part 6 of 9 of a stacked series that reworks the DPDK SRIOV hot plug tests. Stacked on #4658, review only the last commit.

• is_enabled_sriov only inspected the primary nic, so a multi nic VM reported accelerated networking as enabled while a secondary nic was still being updated. It now checks every nic on the VM.
• switch_sriov failed outright on transient HttpResponseError replies from the network resource provider, so it is retried a few times before giving up.

Key Test Cases:
verify_dpdk_sriov_rescind_failover_send_only|verify_sriov_disable_enable|verify_sriov_add_max_nics

Impacted LISA Features:
Sriov, NetworkInterface

Tested Azure Marketplace Images:

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

[Copilot]

Pull request overview

This PR updates Azure SR-IOV handling in LISA’s Azure feature layer to make SR-IOV enablement checks accurate for multi-NIC VMs and to improve resiliency of SR-IOV toggling against transient Azure Network RP/ARM errors.

Changes:

• Add retry handling to switch_sriov to tolerate transient HttpResponseError responses.
• Update is_enabled_sriov to validate accelerated networking state across all VM NICs (not just the primary).

---

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

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lisa/sut_orchestrator/azure/features.py:1048

• nic_info.enable_accelerated_networking can be None (it’s treated as optional elsewhere in this file), so using &= can raise TypeError (bool & NoneType) and also reads like a bitwise operation. Use a boolean and check (or is True) so the method always returns a real bool and treats None as disabled.

            sriov_enabled &= nic_info.enable_accelerated_networking

lisa/sut_orchestrator/azure/features.py:991

• The retry decorator uses a fixed 30s delay with no backoff/jitter. This can significantly extend test runtime (up to ~60s extra plus Azure calls) on transient failures, and it’s inconsistent with other HttpResponseError retries in this module that use short delays with backoff. Consider adding a brief rationale for the 30s delay and/or using a backoff strategy.

    @retry(HttpResponseError, tries=3, delay=30)  # type: ignore

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lisa/sut_orchestrator/azure/features.py:1048

• nic_info.enable_accelerated_networking from the Azure SDK can be None (Optional[bool]). Using sriov_enabled &= nic_info.enable_accelerated_networking can raise a TypeError when the value is None, and &= is a bitwise op that’s less clear than logical AND for booleans. Coerce the SDK value to a boolean (and treat None as disabled) while keeping the result strictly bool.

            sriov_enabled &= nic_info.enable_accelerated_networking

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lisa/sut_orchestrator/azure/features.py:1048

• Major: nic_info.enable_accelerated_networking can be None (elsewhere this field is compared with is False), and sriov_enabled &= None will raise TypeError. Coerce the value to a strict bool (treat None as disabled) before combining results.

                self._resource_group_name, nic_name
            )
            sriov_enabled &= nic_info.enable_accelerated_networking

lisa/sut_orchestrator/azure/features.py:991

• Minor: @retry(HttpResponseError, tries=3, delay=30) retries all HttpResponseError cases with a fixed 30s delay, which can make permanent failures slower and is inconsistent with the shorter/backoff-based HttpResponseError retries elsewhere in this file. Consider either (a) documenting why 30s is needed here, or (b) aligning with the backoff pattern used in other Azure RP retries / narrowing the retried conditions to transient cases.

    @retry(HttpResponseError, tries=3, delay=30)  # type: ignore

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lisa/sut_orchestrator/azure/features.py:1049

• sriov_enabled &= nic_info.enable_accelerated_networking relies on enable_accelerated_networking being a strict bool. Elsewhere in this file the code checks x.enable_accelerated_networking is False (around line ~809), which suggests this field may also be None. In that case, &= will raise TypeError: unsupported operand type(s) for &= 'bool' and 'NoneType'. Consider using logical and and coercing None to False.

                self._resource_group_name, nic_name
            )
            sriov_enabled &= nic_info.enable_accelerated_networking
        return sriov_enabled

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lisa/sut_orchestrator/azure/features.py:1052

• is_enabled_sriov() currently accumulates NIC state with sriov_enabled &= (...), which is harder to read than a simple early-return and also forces an Azure NIC GET for every NIC even after one NIC is already known to be disabled. Consider short-circuiting on the first NIC that isn't explicitly True for enable_accelerated_networking.

            accelnet_setting = nic_info.enable_accelerated_networking
            # check if all nics have accelnet enabled
            sriov_enabled &= (
                accelnet_setting if isinstance(accelnet_setting, bool) else False
            )

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

lisa/sut_orchestrator/azure/features.py:991

• Consider linking the related issue for traceability.

    @retry(HttpResponseError, tries=3, delay=30)  # type: ignore

lisa/sut_orchestrator/azure/features.py:991

• Major: Wrapping the whole operation in retry resets status_changed on every attempt. If Azure accepts an update but the following get raises HttpResponseError, the retry can observe the NIC already in the target state, leave status_changed false, and return without _check_sriov_enabled even when wait=True; the guest VF state may not have converged. Retry only the failing SDK operation, or preserve and re-run the post-update wait after a retry.

    @retry(HttpResponseError, tries=3, delay=30)  # type: ignore

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lisa/sut_orchestrator/azure/features.py:1052

• is_enabled_sriov() can be simplified and made more efficient: it currently accumulates with &= and continues querying Azure NIC resources even after one NIC is found with accelerated networking disabled/unknown. Using early-return makes the intent clearer and avoids extra network_interfaces.get() calls on multi-NIC VMs. Also, using assert for nic.id in runtime code can be optimized away with python -O; raising a LisaException keeps the validation in all runtimes.

            accelnet_setting = nic_info.enable_accelerated_networking
            # check if all nics have accelnet enabled
            sriov_enabled &= (
                accelnet_setting if isinstance(accelnet_setting, bool) else False
            )

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lisa/sut_orchestrator/azure/features.py:1049

• is_enabled_sriov accumulates NIC state using &= on enable_accelerated_networking, which may be None (Optional[bool]) and will raise TypeError with bitwise-AND. It’s also harder to read than an early-return/all() pattern. Consider treating anything other than True as disabled and returning early.

            nic_info = network_client.network_interfaces.get(
                self._resource_group_name, nic_name
            )
            sriov_enabled &= nic_info.enable_accelerated_networking
        return sriov_enabled

lisa/sut_orchestrator/azure/features.py:991

• The new retry uses hard-coded values (tries=3, delay=30) without explaining why they’re appropriate. Adding a brief rationale inline will make future tuning/debugging easier (and clarifies the added ~60s worst-case delay).

    @retry(HttpResponseError, tries=3, delay=30)  # type: ignore

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lisa/sut_orchestrator/azure/features.py:1040

• Minor: is_enabled_sriov uses bitwise &= and a type guard to combine NIC settings, which is harder to read and doesn’t short-circuit. Since enable_accelerated_networking is effectively a tri-state (bool/None), a direct is not True check is clearer and avoids relying on bitwise boolean behavior.

    def is_enabled_sriov(self) -> bool:
        # check whether sriov is enabled on all nics
        azure_platform: AzurePlatform = self._platform  # type: ignore
        network_client = get_network_client(azure_platform)
        sriov_enabled: bool = True

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.