BugFix: install iperf3 source build dependencies - #4664
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Iperf3 source-installation failures by restoring the intended Tool.install() workflow so declared dependencies are installed before the tool-specific install logic runs (including adding Gcc as an explicit dependency for source builds).
Changes:
- Rename
Iperf3.install()toIperf3._install()so dependency loading is handled by the inheritedTool.install()implementation. - Add
GcctoIperf3.dependenciesalongsideGitandMake. - Add a unit test to validate dependency-loading behavior during
Iperf3.install().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| selftests/test_iperf3.py | Adds a regression unit test around Iperf3.install() dependency loading. |
| lisa/tools/iperf3.py | Routes installation through Tool.install() and adds Gcc as a declared dependency. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@microsoft-github-policy-service agree company="Microsoft" |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
lisa/tools/iperf3.py:154
- Minor: Avoid catching broad
Exceptionhere.Posix.install_packages()failures are raised asLisaException(and subclasses likeRepoNotExistException/ReleaseEndOfLifeException), so catchingExceptioncan hide programming errors and make unexpected failures harder to diagnose. CatchLisaException(or a narrower subset) instead, and let other exception types surface.
def _install(self) -> bool:
posix_os: Posix = cast(Posix, self.node.os)
try:
posix_os.install_packages("iperf3")
except Exception as e:
| def test_source_build_installs_cbl_mariner_linker_dependencies(self) -> None: | ||
| cbl_mariner = MagicMock(spec=CBLMariner) | ||
| iperf3 = Iperf3.__new__(Iperf3) | ||
| iperf3.node = MagicMock(os=cbl_mariner) |
There was a problem hiding this comment.
MagicMock(spec=CBLMariner) passes isinstance(mock, CBLMariner) because a specced mock exposes the spec class through class. I verified this behavior directly, and the focused unit test passes successfully. Therefore, no change is required here.
|
|
||
| def _install_dep_packages(self) -> None: | ||
| if isinstance(self.node.os, CBLMariner): | ||
| self.node.os.install_packages(["binutils", "glibc-devel"]) |
There was a problem hiding this comment.
Please double check mariner3 and mariner4, the package names may be different.
There was a problem hiding this comment.
Thanks for pointing this out. I double-checked the Azure Linux 3.0 and 4.0 package specs, and both versions use binutils and glibc-devel. These package names are also used by the existing CBLMariner paths in LISA tools such as Netperf and NTTTCP. The Dom0 validation successfully installed both packages and completed the Iperf3 source build, so no version-specific package mapping is needed.
Description
Fixes Iperf3 source installation failures when a C compiler is not already installed.
Iperf3declaredGitandMakeas dependencies, but it overrode the publicinstall()method. This bypassedTool.install(), which is responsible for loading declared tool dependencies before calling the tool-specific installation implementation.When the package-manager installation was unavailable or an installed Iperf3 version needed to be replaced, LISA attempted to build Iperf3 from source. The build then failed during
./configurebecause no C compiler was available.This change:
Iperf3.install()toIperf3._install()so installation runs through the inheritedTool.install()workflow.Gccas a direct Iperf3 dependency alongsideGitandMake.Iperf3._install()is called.Gccis declared directly rather than relying onMaketo load it transitively. IfMakeis already installed or cached, its installation method may not run, so its dependencies are not guaranteed to be processed.Related Issue
Type of Change
Checklist
Test Validation
Completed locally:
python -m unittest selftests.test_iperf3 -v— passedIperf3.install is Tool.install— passed[Git, Make, Gcc]— passedgit diff --check— passed