Hyper-V API: Use HcnQueryEndpointProperties for retrieving VM's IP address - #5097
Hyper-V API: Use HcnQueryEndpointProperties for retrieving VM's IP address#5097xmkg wants to merge 17 commits into
Conversation
The hostname resolution approach is unreliable because the hosts.ics file tends to get corrupted, leading to connectivity issues.
There was a problem hiding this comment.
Pull request overview
This PR replaces the Hyper-V VM IP discovery mechanism (previously relying on hostname/DNS resolution via *.mshome.net) with an HCN-based approach that queries endpoint properties via HcnQueryEndpointProperties, making IP resolution more robust on Windows.
Changes:
- Added
HCNWrapper::query_endpoint()(and supporting API surface) to fetch and parse endpoint IP configuration from HCN JSON properties. - Updated
HCSVirtualMachineto derive SSH hostname from the VM’s management IPv4 (and throwIPUnavailableExceptionwhen unavailable). - Expanded unit/integration/component tests and mocks to cover endpoint queries, failure handling, and IPv4 selection.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/hyperv_api/test_ut_hyperv_hcs_virtual_machine.cpp | Updates VM SSH hostname/IP tests to use endpoint queries; adds management IPv4 behavior tests. |
| tests/unit/hyperv_api/test_ut_hyperv_hcn_api.cpp | Adds unit tests for endpoint query behavior and JSON parsing/merging rules. |
| tests/unit/hyperv_api/test_it_hyperv_hcn_api.cpp | Extends integration coverage to validate querying endpoint properties in real HCN flows. |
| tests/unit/hyperv_api/test_bb_cit_hyperv.cpp | Adds bounded wait logic verifying endpoint IPv4 becomes available asynchronously; improves cleanup. |
| tests/unit/hyperv_api/mock_hyperv_hcn_wrapper.h | Adds query_endpoint mock for unit tests. |
| tests/unit/hyperv_api/mock_hyperv_hcn_api.h | Adds HcnQueryEndpointProperties mock method. |
| src/platform/backends/hyperv_api/hcs_virtual_machine.cpp | Switches SSH hostname to management IPv4 and implements endpoint-based IP lookup. |
| src/platform/backends/hyperv_api/hcn/hyperv_hcn_wrapper.h | Adds query_endpoint API to the wrapper interface. |
| src/platform/backends/hyperv_api/hcn/hyperv_hcn_wrapper.cpp | Implements endpoint open + HcnQueryEndpointProperties JSON parsing to extract IPs. |
| src/platform/backends/hyperv_api/hcn/hyperv_hcn_endpoint_info.h | Introduces a struct to return endpoint IP addresses. |
| src/platform/backends/hyperv_api/hcn/hyperv_hcn_api.h | Adds HcnQueryEndpointProperties to the HCN API abstraction. |
| src/platform/backends/hyperv_api/hcn/hyperv_hcn_api.cpp | Implements the new API forwarding call to the Windows HCN function. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I've found an issue. I'll mark this PR as ready once I fix it. |
| std::stringstream s(ip); | ||
| s >> a >> ch >> b >> ch >> c >> ch >> d; | ||
| s >> octets[0] >> sep[0] >> octets[1] >> sep[1] >> octets[2] >> sep[2] >> octets[3]; | ||
|
|
||
| if (!is_valid_octet(a) || !is_valid_octet(b) || !is_valid_octet(c) || !is_valid_octet(d)) | ||
| if (!std::ranges::all_of(octets, is_valid_octet) || | ||
| !std::ranges::all_of(sep, [](char c) { return c == '.'; })) |
| TEST_F(HyperV_ComponentIntegrationTests, alpine_vm_gets_permanent_neighbor_on_ics_dhcp_network) | ||
| { | ||
| hyperv::hcs::HcsSystemHandle handle{nullptr}; | ||
| // 10.0. 0.0 to 10.255. 255.255. |
| std::optional<std::string> neighbor_address; | ||
| for (auto attempts = 0; attempts < 120 && !neighbor_address; ++attempts) | ||
| { | ||
| neighbor_address = windows_network_utils().permanent_ipv4_neighbor( | ||
| *endpoint_info.mac_address); | ||
| if (!neighbor_address) | ||
| std::this_thread::sleep_for(500ms); | ||
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5097 +/- ##
==========================================
- Coverage 73.36% 73.36% -0.00%
==========================================
Files 332 332
Lines 17937 17936 -1
==========================================
- Hits 13158 13157 -1
Misses 4779 4779 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The existing hostname resolution approach for getting the VM IP is unreliable since the hosts.ics file tends to break on its own due to a longstanding Windows bug. Using the HCN API for querying the info will make the IP resolution much more robust than what we already have. Also introduced a
GetIpNetTable2based fallback lookup when the HCN API yields no IP addresses. This is to overcome HCN API's shortcomings for querying IP addresses for the ICS networks.