Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions core/src/main/java/com/cloud/agent/api/GetVmIpAddressAnswer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//
package com.cloud.agent.api;

/**
* Answer to {@link GetVmIpAddressCommand}. The IPv4 address is carried in {@link #getDetails()} for
* compatibility with agents answering with a plain {@link Answer}; the (global) IPv6 address, if any, is optional.
*/
public class GetVmIpAddressAnswer extends Answer {

private String ip6Address;

public GetVmIpAddressAnswer() {
}

public GetVmIpAddressAnswer(GetVmIpAddressCommand command, boolean result, String ip4Address, String ip6Address) {
super(command, result, ip4Address);
this.ip6Address = ip6Address;
}

public String getIp6Address() {
return ip6Address;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.List;

import com.cloud.agent.api.Answer;
import com.cloud.agent.api.GetVmIpAddressAnswer;
import com.cloud.agent.api.GetVmIpAddressCommand;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.resource.CommandWrapper;
Expand Down Expand Up @@ -58,10 +59,11 @@ static void init() {
@Override
public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputingResource libvirtComputingResource) {
String ip = null;
String ip6 = null;
boolean result = false;
String vmName = command.getVmName();
if (!NetUtils.verifyDomainNameLabel(vmName, true)) {
return new Answer(command, result, ip);
return new GetVmIpAddressAnswer(command, result, ip, ip6);
}

String sanitizedVmName = sanitizeBashCommandArgument(vmName);
Expand All @@ -70,7 +72,9 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin

init();

ip = ipFromDomIf(sanitizedVmName, networkCidr, macAddress);
Pair<String, String> ipAddresses = ipFromDomIf(sanitizedVmName, networkCidr, macAddress);
ip = ipAddresses.first();
ip6 = ipAddresses.second();

if (ip == null && networkCidr != null) {
if(!command.isWindows()) {
Expand All @@ -80,18 +84,21 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin
}
}

if(ip != null){
// success needs an IPv4 address or a global IPv6 address (IPv6-only guests); a link-local IPv6 address alone
// is only reported along with an IPv4 address
if (ip != null || (ip6 != null && !NetUtils.isLinkLocalIp6(ip6))) {
result = true;
logger.debug("GetVmIp: "+ vmName + " Found Ip: "+ip);
logger.debug("GetVmIp: " + vmName + " Found Ip: " + ip + ", Ipv6: " + ip6);
} else {
logger.warn("GetVmIp: "+ vmName + " IP not found.");
}

return new Answer(command, result, ip);
return new GetVmIpAddressAnswer(command, result, ip, ip6);
}

private String ipFromDomIf(String sanitizedVmName, String networkCidr, String macAddress) {
private Pair<String, String> ipFromDomIf(String sanitizedVmName, String networkCidr, String macAddress) {
String ip = null;
String ip6 = null;
List<String[]> commands = new ArrayList<>();
commands.add(new String[]{virsh_path, "domifaddr", sanitizedVmName, "--source", "agent"});
Pair<Integer,String> response = executePipedCommands(commands, 0);
Expand All @@ -102,15 +109,17 @@ private String ipFromDomIf(String sanitizedVmName, String networkCidr, String ma
if (networkCidr == null || NetUtils.isIpWithInCidrRange(ipv4, networkCidr)) {
ip = ipv4;
}
ip6 = ipAddresses.second();
} else {
logger.error("ipFromDomIf: Command execution failed for VM: " + sanitizedVmName);
}
return ip;
return new Pair<>(ip, ip6);
}

private Pair<String, String> getIpAddresses(String output, String macAddress) {
String ipv4 = null;
String ipv6 = null;
String linkLocalIpv6 = null;
boolean found = false;
String[] lines = output.split("\n");
for (String line : lines) {
Expand All @@ -130,12 +139,29 @@ private Pair<String, String> getIpAddresses(String output, String macAddress) {
found = true;
String ipFamily = parts[parts.length - 2];
String ipPart = parts[parts.length - 1].split("/")[0];
// Skip link-local IPv4 addresses (e.g. Windows APIPA 169.254.0.0/16 before a DHCP lease is obtained),
// they are not routable and must not be reported as the instance IP
if (ipFamily.equals("ipv4")) {
ipv4 = ipPart;
if (NetUtils.isLinkLocalIp4(ipPart)) {
logger.debug(String.format("Ignoring link-local ipv4 %s with mac address %s", ipPart, macAddress));
} else {
ipv4 = ipPart;
}
} else if (ipFamily.equals("ipv6")) {
ipv6 = ipPart;
// keep the first global address, and the first link-local one as a fallback
if (NetUtils.isLinkLocalIp6(ipPart)) {
if (linkLocalIpv6 == null) {
int zoneIndex = ipPart.indexOf('%');
linkLocalIpv6 = zoneIndex >= 0 ? ipPart.substring(0, zoneIndex) : ipPart;
}
} else if (ipv6 == null) {
ipv6 = ipPart;
}
}
}
if (ipv6 == null) {
ipv6 = linkLocalIpv6;
}
logger.debug(String.format("Found ipv4: %s and ipv6: %s with mac address %s", ipv4, ipv6, macAddress));
return new Pair<>(ipv4, ipv6);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.mockito.MockitoAnnotations;

import com.cloud.agent.api.Answer;
import com.cloud.agent.api.GetVmIpAddressAnswer;
import com.cloud.agent.api.GetVmIpAddressCommand;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.utils.Pair;
Expand All @@ -58,6 +59,24 @@ public class LibvirtGetVmIpAddressCommandWrapperTest {
" Loopback Pseudo-Interface 1 ipv6 ::1/128\n" + //
" - - ipv4 127.0.0.1/8\n";

// Windows guest that has not obtained a DHCP lease yet and self-assigned an APIPA address
private static String VIRSH_DOMIF_OUTPUT_WINDOWS_APIPA = " Name MAC address Protocol Address\n" + //
"-------------------------------------------------------------------------------\n" + //
" Ethernet 02:01:00:d0:00:87 ipv6 fe80::df84:2984:e396:8ac3%6/64\n" + //
" - - ipv4 169.254.149.120/16\n" + //
" Loopback Pseudo-Interface 1 ipv6 ::1/128\n" + //
" - - ipv4 127.0.0.1/8\n";

// Same Windows guest once the DHCP lease has been obtained
private static String VIRSH_DOMIF_OUTPUT_WINDOWS_DHCP = " Name MAC address Protocol Address\n" + //
"-------------------------------------------------------------------------------\n" + //
" Ethernet 02:01:00:d0:00:87 ipv6 fd00:ac19:0:1910:0:d3fc:4177:2c32/128\n" + //
" - - ipv6 fe80::df84:2984:e396:8ac3%6/64\n" + //
" - - ipv4 172.25.16.236/22\n" + //
" - - ipv4 169.254.149.120/16\n" + //
" Loopback Pseudo-Interface 1 ipv6 ::1/128\n" + //
" - - ipv4 127.0.0.1/8\n";
Comment on lines +62 to +78

@Before
public void setUp() {
MockitoAnnotations.openMocks(this);
Expand Down Expand Up @@ -324,6 +343,116 @@ public void testIpFromWindowsRegistryCommandExecutionFailure() {
}
}

private Answer executeOnL2Network(String domIfOutput, boolean windows) {
LibvirtComputingResource libvirtComputingResource = mock(LibvirtComputingResource.class);
GetVmIpAddressCommand getVmIpAddressCommand = mock(GetVmIpAddressCommand.class);
LibvirtGetVmIpAddressCommandWrapper commandWrapper = new LibvirtGetVmIpAddressCommandWrapper();
try (MockedStatic<Script> scriptMock = mockStatic(Script.class)) {
when(getVmIpAddressCommand.getVmName()).thenReturn("i-2-187-VM");
// L2 networks have no CIDR
when(getVmIpAddressCommand.getVmNetworkCidr()).thenReturn(null);
when(getVmIpAddressCommand.getMacAddress()).thenReturn("02:01:00:d0:00:87");
when(getVmIpAddressCommand.isWindows()).thenReturn(windows);
when(Script.executePipedCommands(anyList(), anyLong())).thenReturn(new Pair<>(0, domIfOutput));

return commandWrapper.execute(getVmIpAddressCommand, libvirtComputingResource);
}
}

@Test
public void testExecuteWithWindowsVmOnlyApipaAddressOnL2Network() {
Answer answer = executeOnL2Network(VIRSH_DOMIF_OUTPUT_WINDOWS_APIPA, true);

assertFalse(answer.getResult());
assertNull(answer.getDetails());
}

private void assertAddresses(Answer answer, boolean result, String ip4, String ip6) {
assertEquals(result, answer.getResult());
assertEquals(ip4, answer.getDetails());
assertEquals(ip6, ((GetVmIpAddressAnswer) answer).getIp6Address());
}

@Test
public void testExecuteWithIpv4Only() {
String output = " Name MAC address Protocol Address\n" +
"-------------------------------------------------------------------------------\n" +
" Ethernet 02:01:00:d0:00:87 ipv4 172.25.16.236/22\n";

assertAddresses(executeOnL2Network(output, false), true, "172.25.16.236", null);
}

@Test
public void testExecuteWithIpv6Only() {
String output = " Name MAC address Protocol Address\n" +
"-------------------------------------------------------------------------------\n" +
" Ethernet 02:01:00:d0:00:87 ipv6 fd00::1/128\n";

assertAddresses(executeOnL2Network(output, false), true, null, "fd00::1");
}

@Test
public void testExecuteWithIpv4AndIpv6LinkLocalFirstGlobalSecond() {
String output = " Name MAC address Protocol Address\n" +
"-------------------------------------------------------------------------------\n" +
" Ethernet 02:01:00:d0:00:87 ipv6 fe80::1:2:3:4%6/64\n" +
" - - ipv6 fd00::1/128\n" +
" - - ipv4 172.25.16.236/22\n";

assertAddresses(executeOnL2Network(output, false), true, "172.25.16.236", "fd00::1");
}

@Test
public void testExecuteWithIpv4AndIpv6LinkLocalOnly() {
String output = " Name MAC address Protocol Address\n" +
"-------------------------------------------------------------------------------\n" +
" Ethernet 02:01:00:d0:00:87 ipv6 fe80::1:2:3:4%6/64\n" +
" - - ipv4 172.25.16.236/22\n";

assertAddresses(executeOnL2Network(output, false), true, "172.25.16.236", "fe80::1:2:3:4");
}

@Test
public void testExecuteWithIpv4AndIpv6GlobalFirstLinkLocalSecond() {
String output = " Name MAC address Protocol Address\n" +
"-------------------------------------------------------------------------------\n" +
" Ethernet 02:01:00:d0:00:87 ipv6 fd00::1/128\n" +
" - - ipv6 fe80::1:2:3:4%6/64\n" +
" - - ipv4 172.25.16.236/22\n";

assertAddresses(executeOnL2Network(output, false), true, "172.25.16.236", "fd00::1");
}

@Test
public void testExecuteWithMultipleGlobalIpv6ReturnsFirst() {
String output = " Name MAC address Protocol Address\n" +
"-------------------------------------------------------------------------------\n" +
" Ethernet 02:01:00:d0:00:87 ipv6 fe80::1:2:3:4%6/64\n" +
" - - ipv6 fd00::1/128\n" +
" - - ipv6 fd00::2/128\n" +
" - - ipv4 172.25.16.236/22\n";

assertAddresses(executeOnL2Network(output, false), true, "172.25.16.236", "fd00::1");
}

@Test
public void testExecuteWithIpv6LinkLocalOnlyNoIpv4() {
String output = " Name MAC address Protocol Address\n" +
"-------------------------------------------------------------------------------\n" +
" Ethernet 02:01:00:d0:00:87 ipv6 fe80::1:2:3:4%6/64\n";

assertAddresses(executeOnL2Network(output, false), false, null, "fe80::1:2:3:4");
}

@Test
public void testExecuteWithWindowsVmDhcpAndApipaAddressOnL2Network() {
Answer answer = executeOnL2Network(VIRSH_DOMIF_OUTPUT_WINDOWS_DHCP, true);

assertTrue(answer.getResult());
assertEquals("172.25.16.236", answer.getDetails());
assertEquals("fd00:ac19:0:1910:0:d3fc:4177:2c32", ((GetVmIpAddressAnswer) answer).getIp6Address());
}

@Test
public void testInit() {
MockedStatic<Script> scriptMock = null;
Expand Down
40 changes: 39 additions & 1 deletion server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@
import com.cloud.agent.api.Command;
import com.cloud.agent.api.GetVmDiskStatsAnswer;
import com.cloud.agent.api.GetVmDiskStatsCommand;
import com.cloud.agent.api.GetVmIpAddressAnswer;
import com.cloud.agent.api.GetVmIpAddressCommand;
import com.cloud.agent.api.GetVmNetworkStatsAnswer;
import com.cloud.agent.api.GetVmNetworkStatsCommand;
Expand Down Expand Up @@ -770,6 +771,14 @@ public void decrementCount() {
}
}

private void updateNicIp6Address(NicVO nic, String vmIp6) {
if (nic == null || vmIp6 == null || !NetUtils.isValidIp6(vmIp6) || vmIp6.equals(nic.getIPv6Address())) {
return;
}
nic.setIPv6Address(vmIp6);
_nicDao.update(nic.getId(), nic);
}

private class VmIpAddrFetchThread extends ManagedContextRunnable {
long nicId;
long vmId;
Expand Down Expand Up @@ -802,16 +811,38 @@ protected void runInContext() {
Answer answer = _agentMgr.send(hostId, cmd);
if (answer.getResult()) {
String vmIp = answer.getDetails();
String vmIp6 = answer instanceof GetVmIpAddressAnswer ? ((GetVmIpAddressAnswer) answer).getIp6Address() : null;
if (vmIp == null) {
// we got a valid response and the NIC does not have an IP assigned, as such we will update the database with null
// we got a valid response and the NIC does not have an IPv4 address assigned, as such we will update the database with null
if (nic.getIPv4Address() != null) {
nic.setIPv4Address(null);
_nicDao.update(nicId, nic);
}
if (vmIp6 != null && NetUtils.isValidIp6(vmIp6)) {
// IPv6-only guest
updateNicIp6Address(nic, vmIp6);
logger.debug("Instance [ID: {}, UUID: {}, name: {}] - IPv6 {} retrieved successfully", vmId, vmUuid, vmName, vmIp6);
vmIdCountMap.remove(nicId);
decrementCount = false;
}
} else if (NetUtils.isLinkLocalIp4(vmIp)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

on xenserver this still counts as a failed try, so after about 30 min we stop checking and a lease that comes later is never picked up. should this skip the count like kvm does now?

// a link-local (APIPA) address means the guest has not obtained a DHCP lease yet; do not record it,
// clear any previously recorded link-local address and keep retrying without consuming the retry budget
// (consistent with KVM, which reports this case as a failed answer), so a late DHCP lease is still picked up
logger.debug("Instance [ID: {}, UUID: {}, name: {}] - ignoring link-local IP {}, DHCP lease not obtained yet",
vmId, vmUuid, vmName, vmIp);
decrementCount = false;
if (nic != null && NetUtils.isLinkLocalIp4(nic.getIPv4Address())) {
nic.setIPv4Address(null);
_nicDao.update(nicId, nic);
}
Comment on lines +828 to +838
} else if (NetUtils.isValidIp4(vmIp)) {
// set this vm ip addr in vm nic.
if (nic != null) {
nic.setIPv4Address(vmIp);
if (vmIp6 != null && NetUtils.isValidIp6(vmIp6)) {
nic.setIPv6Address(vmIp6);
}
_nicDao.update(nicId, nic);
logger.debug("Instance [ID: {}, UUID: {}, name: {}] - IP {} retrieved successfully", vmId, vmUuid, vmName, vmIp);
vmIdCountMap.remove(nicId);
Expand Down Expand Up @@ -2522,6 +2553,13 @@ private void loadVmDetailsInMapForExternalDhcpIp() {
List<NicVO> nics = _nicDao.listByNetworkId(network.getId());

for (NicVO nic : nics) {
// a link-local (APIPA) address recorded before the guest obtained its DHCP lease is not a valid instance IP,
// clear it so it is no longer exposed and the NIC is retried as one without an IP
if (NetUtils.isLinkLocalIp4(nic.getIPv4Address())) {
logger.debug("Clearing link-local IP {} recorded for NIC {}", nic.getIPv4Address(), nic);
nic.setIPv4Address(null);
_nicDao.update(nic.getId(), nic);
}
Comment on lines +2556 to +2562
if (nic.getIPv4Address() == null) {
long nicId = nic.getId();
long vmId = nic.getInstanceId();
Expand Down
Loading
Loading