Skip to content

Conversation

@Sploder12
Copy link
Contributor

Adds subnets to AZs and assigns them to VMs made in that zone. This PR only covers Linux QEMU using dnsmasq.

MULTI-1942

@codecov
Copy link

codecov bot commented May 5, 2025

Codecov Report

❌ Patch coverage is 96.05263% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.50%. Comparing base (6c91f0b) to head (bf74556).
⚠️ Report is 8 commits behind head on availability-zones.

Files with missing lines Patch % Lines
...backends/qemu/linux/qemu_platform_detail_linux.cpp 94.73% 2 Missing ⚠️
...tform/backends/qemu/linux/dnsmasq_process_spec.cpp 96.29% 1 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           availability-zones    #4071      +/-   ##
======================================================
+ Coverage               89.48%   89.50%   +0.02%     
======================================================
  Files                     268      268              
  Lines                   18003    18034      +31     
======================================================
+ Hits                    16110    16142      +32     
+ Misses                   1893     1892       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Sploder12 Sploder12 force-pushed the az-networking branch 2 times, most recently from 06b9b28 to 9de8b73 Compare May 5, 2025 19:17
@Sploder12 Sploder12 marked this pull request as ready for review May 5, 2025 20:35
@Sploder12 Sploder12 force-pushed the az-networking branch 2 times, most recently from 4e7f1b7 to 66fede5 Compare May 14, 2025 10:20
@sharder996 sharder996 requested review from levkropp and xmkg June 27, 2025 15:27
Base automatically changed from az-integration to availability-zones July 10, 2025 17:57
Copy link
Member

@xmkg xmkg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid piece of work @Sploder12! I have a couple of comments.

@Sploder12 Sploder12 force-pushed the availability-zones branch 3 times, most recently from 2331482 to 3c8321a Compare August 25, 2025 21:55
@Sploder12 Sploder12 force-pushed the az-networking branch 4 times, most recently from 29bbe5a to d500e33 Compare August 27, 2025 05:28
namespace multipass
{
class QemuPlatform : private DisabledCopyMove
class QemuPlatform : protected DisabledCopyMove
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be the only use of protected DisabledCopyMove in the entire multipass codebase. Is there a reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, QemuPlatformDetail::Subnet needs to derive DisabledCopyMove and because QemuPlatform already derives it as private QemuPlatformDetail::Subnet cannot access it. But it turns out this is because name lookup is going for the derived DisabledCopyMove and not multipass::DisabledCopyMove so being explicit removes the need to make this protected

@Sploder12 Sploder12 requested review from levkropp and xmkg September 2, 2025 13:27
Copy link
Contributor

@levkropp levkropp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Tested by launching a MySQL replication setup with SSL across 2 VMs in different zones and there were no issues with networking

@Sploder12 Sploder12 force-pushed the availability-zones branch 2 times, most recently from f21b770 to 3098c8f Compare September 15, 2025 09:26
@Sploder12 Sploder12 changed the title Az networking Az networking Linux Sep 22, 2025
-1 is not a valid value for --count (-c).
We were interpreting ping's invalid value return as failing to ping.
Copy link
Member

@xmkg xmkg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Tested locally, VM gets its IP address from AZ1. Also, the branch passes all the CLI tests too.

I've noticed that it takes longer to start the daemon on the first start, but the time is rather negligible. The subsequent daemon starts does not suffer from this.

@Sploder12 Sploder12 merged commit 7f27dd1 into availability-zones Sep 26, 2025
37 of 39 checks passed
@Sploder12 Sploder12 deleted the az-networking branch September 26, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants