Adding AZIHSM tests - #4687
Conversation
of the out-of-tree module, as well as the fuinctional tests of the device. These tests were written with the intention that they will be usable for both Mariner and Ubuntu (and possibly others).
…nstead fo using tux-dev
… run for every test case
directly instead of going through the tool. The tool lacks support for signed-by which we need to use. As part of this, we added a param to add_repository() and ensured that the API signature for this function is consnstant through out the file, which males pylint happy.
There was a problem hiding this comment.
Pull request overview
Adds AZIHSM driver and package validation tests, along with repository configuration API updates.
Changes:
- Adds AZIHSM installation, lifecycle, uninstall, driver, and SDK tests.
- Updates repository file handling and Azure Core repository configuration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 18 comments.
| File | Review summary |
|---|---|
lisa/operating_system.py |
Repository API compatibility, key configuration, and remote repository setup issues remain. |
lisa/microsoft/testsuites/azihsm/azihsm.py |
Critical test-state, module lifecycle, validation, repository security, and error-handling issues remain. |
Suppressed comments (10)
lisa/microsoft/testsuites/azihsm/azihsm.py:237
- The suite is restricted to
CBLMarinerandUbuntu, but this case-level requirement replaces the inherited requirement withunsupported_os=[]. The same setting is used on every case, so the tests become eligible on Windows and other distros where the OS-specific package name/list is never initialized. Remove the case-level requirement or setsupported_os=[CBLMariner, Ubuntu]consistently.
requirement=simple_requirement(unsupported_os=[]),
lisa/microsoft/testsuites/azihsm/azihsm.py:273
- This assertion uses assertpy's local filesystem matcher against a path that exists on the remote VM. It will check the controller's
/lib/modules/...tree and can fail even after a successful remote install. Check the path throughnode.shell/a remote tool instead.
assert_that(AziHsm.kmodpath).is_file()
lisa/microsoft/testsuites/azihsm/azihsm.py:281
contents_of()reads a local file, butmodules.depis on the target node. This check therefore cannot validatedepmodon the VM and will normally raise a localFileNotFoundError; read the file with a node tool such as remoteCatbefore asserting its contents.
contents = contents_of(f"/lib/modules/{AziHsm.kernel_version}/modules.dep", "ascii")
lisa/microsoft/testsuites/azihsm/azihsm.py:467
- This is another local filesystem assertion against a remote module path.
does_not_exist()can pass simply because the controller lacks that path, even if the.kofile remains installed on the VM; verify the remote path instead.
assert_that(AziHsm.kmodpath).does_not_exist()
lisa/microsoft/testsuites/azihsm/azihsm.py:417
- Fixed sleeps make module-cycle validation flaky: a slow VM may still be settling after 0.5 seconds, while a fast VM waits unnecessarily. Replace these sleeps with a bounded remote state wait/retry so each cycle observes the module transition.
time.sleep(0.5)
modprobe.remove(AZIHSM_NAME)
time.sleep(0.5)
lisa/microsoft/testsuites/azihsm/azihsm.py:446
- This fixed one-second delay does not guarantee that the remote module has finished unloading before the package is removed, so cleanup can race the package manager. Replace it with a bounded module-state wait.
time.sleep(1)
lisa/microsoft/testsuites/azihsm/azihsm.py:154
packages_installedis set before the loop has installed anything. If any package is unavailable or installation fails, later cases return immediately and invoke tests with missing user-space binaries. Mark this flag only after the loop completes successfully and make it node-scoped.
if packages_installed is True:
return
# Indicate we have done this step already
packages_installed = True
lisa/microsoft/testsuites/azihsm/azihsm.py:550
- Catching
Exceptionhere also swallows programmer and infrastructure errors, converts them into a generic SDK failure, and lets the loop continue. Catch only the expected command-execution exception so unexpected failures remain visible.
except Exception as e:
cmd_output = ""
log.info("Failed:", e)
all_tests_passed = False
lisa/microsoft/testsuites/azihsm/azihsm.py:559
- This assertion uses a positional message instead of the repository's contextual
described_asform, so failures do not follow the test assertion convention. Attach the explanation withdescribed_as(...)beforeis_true().
assert_that(all_tests_passed, "Not all SDK tests passed").is_true()
lisa/microsoft/testsuites/azihsm/azihsm.py:407
cycles = 3controls the coverage of this test but has no inline explanation. Add a short comment documenting why three load/unload repetitions are the intended threshold, as required for test-behavior magic numbers.
cycles = 3
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| testing_repo_added = False | ||
| packages_installed = False |
| log.info(f"Installing {AziHsm.AziHsmDrvPkgName}") | ||
| node.os.install_packages(AziHsm.AziHsmDrvPkgName) |
| log.info(f"Load/unload cycle {i}/{cycles}") | ||
| modprobe.load(AZIHSM_NAME) | ||
| time.sleep(0.5) | ||
| modprobe.remove(AZIHSM_NAME) |
| try: | ||
| node.os.uninstall_packages(AziHsm.AziHsmDrvPkgName) | ||
| except Exception as e: | ||
| log.error(f"Uninstall failed {e}") | ||
| finally: | ||
| log.info("Package successfully removed") |
| try: | ||
| modprobe.load(AZIHSM_NAME) | ||
| except Exception: | ||
| log.info("modprobe correctly failed") |
| node.os.add_repository( | ||
| repo=( | ||
| "deb [signed-by=/usr/share/keyrings/microsoft-prod.gpg] " | ||
| "https://packages.microsoft.com/ubuntu/" | ||
| f"{node.os.information.release}/prod testing main" | ||
| ), | ||
| repo_file="microsoft-testing.list", | ||
| repo_name="AZIHSM Packages", |
| node.os.add_repository( | ||
| repo=( | ||
| "http://packages.microsoft.com/azurelinux/" | ||
| f"{node.os.information.release}/preview/" | ||
| f"ms-oss/{arch_name}/" | ||
| ), | ||
| repo_file="preview-ms-oss.repo", | ||
| repo_name="AZIHSM Packages", |
| def add_repository( | ||
| self, | ||
| repo: str, | ||
| repo_file: str, |
| keys = [ | ||
| "https://packages.microsoft.com/keys/microsoft.asc", | ||
| "https://packages.microsoft.com/keys/msopentech.asc", | ||
| "https://packages.microsoft.com/keys/microsoft-roling.asc", |
| # is too inconsistant accross the distro/release combinations, and it is not | ||
| # able to handle the signed-by option which we need to be useing. Instead, | ||
| # It is trivial to to just create the file directly | ||
| with open("/etc/apt/sources.list.d/" + repo_file, "w") as f: |
Description
This PR adds test for the AZIHSM device and driver. It also adds some updates/fixes for the add_Repository() API in operating_systems.py.
Related Issue
Type of Change
Checklist
Test Validation
Key Test Cases:
AziHsm.package_installation_tests
AziHsm.verify_azihsm_modinfo
AziHsm.verify_azihsm_module_load_unload
AziHsm.verify_azihsm_module_reload_cycles
AziHsm.verify_azihsm_package_uninstallation
AziHsm.run_azihsm_driver_tests
Impacted LISA Features:
This adds support for the AziHSM device
Tested Azure Marketplace Images:
azure-linux-3-gen2
ubuntu-24_04-lts
Test Results
Note: Failures are cause by the outdated FW which is currently deployed in the fleet. The tests are known to pass on the latest FW.