-
Notifications
You must be signed in to change notification settings - Fork 822
[hyperv-api] Add support for AZs #4967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,7 +58,6 @@ using hcs::HCS; | |
| using virtdisk::VirtDisk; | ||
|
|
||
| constexpr auto log_category = "HyperV-Virtual-Machine-Factory"; | ||
| constexpr auto default_hyperv_switch_guid = "C08CB7B8-9B3C-408E-8E30-5E16A3AEB444"; | ||
| constexpr auto extra_interface_vswitch_name_fmtstr = "Multipass vSwitch ({})"; | ||
| /** | ||
| * Regex pattern to extract the origin network name and GUID from an extra interface | ||
|
|
@@ -72,7 +71,8 @@ HCSVirtualMachineFactory::HCSVirtualMachineFactory(const Path& data_dir, | |
| MP_UTILS.derive_instances_dir(data_dir, | ||
| HCSVirtualMachineFactory::get_backend_directory_name(), | ||
| instances_subdir), | ||
| az_manager) | ||
| az_manager), | ||
| az_network_guids{create_az_bridges(az_manager.get_zones())} | ||
| { | ||
| } | ||
|
|
||
|
|
@@ -81,7 +81,7 @@ VirtualMachine::UPtr HCSVirtualMachineFactory::create_virtual_machine( | |
| const SSHKeyProvider& key_provider, | ||
| VMStatusMonitor& monitor) | ||
| { | ||
| return std::make_unique<HCSVirtualMachine>(default_hyperv_switch_guid, | ||
| return std::make_unique<HCSVirtualMachine>(az_network_guids.at(desc.zone), | ||
| desc, | ||
| monitor, | ||
| key_provider, | ||
|
|
@@ -343,4 +343,34 @@ void HCSVirtualMachineFactory::hypervisor_health_check() | |
| } | ||
| } | ||
|
|
||
| std::unordered_map<std::string, std::string> HCSVirtualMachineFactory::create_az_bridges( | ||
| const AvailabilityZoneManager::Zones& zones) | ||
| { | ||
| std::unordered_map<std::string, std::string> az_mapping; | ||
| for (const auto& i : zones) | ||
| { | ||
| const auto& zone = i.get(); | ||
| hcn::CreateNetworkParameters network_params{ | ||
| .name = fmt::format("Multipass vNetwork ({})", zone.get_name()), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default vSwitch had its DNS suffix set to .mshome.net, which allowed users to do this: Also, this is how we discover the VMs assigned IP right now, and since the new AZ networks have no DNS suffix, we can't use it anymore, and VMs get stuck at start: To support this, we need to add the DNS suffix parameter to the create_network call. It overlaps with the hostname resolution task I have. I'll update the HCN code in a separate PR to support this. It should be trivial -- I'll ping you once it's done.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively, we can use the HCN API to query the assigned IP address to the endpoint, but it would require more work. I'd say it would be a better choice -- something to consider in the future.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've opted to implement querying the assigned IP from HCN instead. resolve_hostname approach is fragile since the hosts.ics file is plagued with a 25-year-old corruption bug.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm,
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jimporter Weird... I don't think it should be possible by default since the vSwitches we create don't have the mshome.net suffix on this branch -- I'll test the build on my 4 Windows VMs to confirm.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jimporter I've tested the branch -- the launches were successful everywhere except my dev-w11-home VM. I recall changing some registry settings during the hostname resolution experiments, which may explain why it fails on dev VM. Logs are below for completeness. |
||
| .type = hcn::HcnNetworkType::Ics(), | ||
| .flags = hcn::HcnNetworkFlags::enable_dhcp_server, | ||
| .guid = utils::make_uuid(network_params.name), | ||
| .ipams = {{.type = hcn::HcnIpamType::Static(), | ||
| .subnets = {{.ip_address_prefix = {zone.get_subnet().to_cidr()}}}}}}; | ||
|
|
||
| const auto create_network_result = HCN().create_network(network_params); | ||
| if (!create_network_result && | ||
| static_cast<HRESULT>(create_network_result.code) != HCN_E_NETWORK_ALREADY_EXISTS) | ||
| { | ||
| throw CreateNetworkException{"Could not create network for {}, status: {}", | ||
| zone.get_name(), | ||
| create_network_result}; | ||
| } | ||
|
|
||
| az_mapping.emplace(zone.get_name(), network_params.guid); | ||
| } | ||
|
|
||
| return az_mapping; | ||
| } | ||
|
|
||
| } // namespace multipass::hyperv | ||
Uh oh!
There was an error while loading. Please reload this page.