search

canonical / multipass

Multipass orchestrates virtual Ubuntu instances
https://multipass.run
GNU General Public License v3.0
7.51k stars 632 forks source link

Fix `get local.instance.bridged` #3513

Closed luis4a0 closed 1 month ago

luis4a0 commented 2 months ago

The answer was wrong in some backends. This PR uses a more reliable mechanism, inspecting the contents of the platform networks data structure.

Fixes #3489.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 89.47368% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 88.80%. Comparing base (2a16af2) to head (2f4ac45).

Files Patch % Lines
...orm/backends/qemu/qemu_virtual_machine_factory.cpp 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3513 +/- ## ========================================== - Coverage 88.82% 88.80% -0.02% ========================================== Files 254 254 Lines 14115 14115 ========================================== - Hits 12537 12535 -2 - Misses 1578 1580 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

luis4a0 commented 1 month ago

Hey @townsend2010!

Why is there anything qemu related, particularly on Linux, in this PR? Shouldn't this be done in https://github.com/canonical/multipass/pull/3463 once this PR is in?

On one hand, moving to platform detail is needed because the code will be different for macOS QEMU. On the other hand, you are right in the sense that the particular function implementation will be needed after bridging is implemented in Linux QEMU. I can make the detail function on Linux throw, and then in the other PR introduce the code again.

luis4a0 commented 1 month ago

Hey @townsend2010, @ricab! I addressed all the concerns above, and added some tests. This is ready for review now.