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
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,20 @@ 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 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;
if (NetUtils.isLinkLocalIp6(ipPart)) {
logger.debug(String.format("Ignoring link-local ipv6 %s with mac address %s", ipPart, macAddress));
} else {
ipv6 = ipPart;
}
Comment on lines +133 to +146
}
}
logger.debug(String.format("Found ipv4: %s and ipv6: %s with mac address %s", ipv4, ipv6, macAddress));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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 +61 to +77

@Before
public void setUp() {
MockitoAnnotations.openMocks(this);
Expand Down Expand Up @@ -324,6 +342,38 @@ 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());
}

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

assertTrue(answer.getResult());
assertEquals("172.25.16.236", answer.getDetails());
}

@Test
public void testInit() {
MockedStatic<Script> scriptMock = null;
Expand Down
18 changes: 18 additions & 0 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,17 @@ protected void runInContext() {
nic.setIPv4Address(null);
_nicDao.update(nicId, nic);
}
} 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 +811 to +821
} else if (NetUtils.isValidIp4(vmIp)) {
// set this vm ip addr in vm nic.
if (nic != null) {
Expand Down Expand Up @@ -2522,6 +2533,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 +2536 to +2542
if (nic.getIPv4Address() == null) {
long nicId = nic.getId();
long vmId = nic.getInstanceId();
Expand Down
28 changes: 28 additions & 0 deletions utils/src/main/java/com/cloud/utils/net/NetUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,34 @@ public static String getLinkLocalCIDR() {
return "169.254.0.0/16";
}

/**
* Returns true if the given address is a valid IPv4 link-local (APIPA, RFC 3927) address, i.e. within 169.254.0.0/16.
* Such an address is self-assigned by a guest when no DHCP lease has been obtained.
*/
public static boolean isLinkLocalIp4(final String ip) {
return isValidIp4(ip) && isIpWithInCidrRange(ip, getLinkLocalCIDR());
}

/**
* Returns true if the given address is a valid IPv6 link-local address, i.e. within fe80::/10.
* A zone index suffix (e.g. "fe80::1%6") is accepted and ignored.
*/
public static boolean isLinkLocalIp6(final String ip) {
if (ip == null) {
return false;
}
final int zoneIndex = ip.indexOf('%');
final String address = zoneIndex >= 0 ? ip.substring(0, zoneIndex) : ip;
if (!isValidIp6(address)) {
return false;
}
try {
return InetAddress.getByName(address).isLinkLocalAddress();
} catch (final UnknownHostException e) {
return false;
}
}

public static String getLinkLocalFirstAddressFromCIDR(final String cidr) {
SubnetUtils subnetUtils = new SubnetUtils(cidr);
return subnetUtils.getInfo().getLowAddress();
Expand Down
25 changes: 25 additions & 0 deletions utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -955,4 +955,29 @@ public void testVpnIpRange() {
String cidr = NetUtils.transformCidr(startIp + "/" + cidrSize);
Assert.assertEquals("10.1.2.0/28", cidr);
}

@Test
public void testIsLinkLocalIp4() {
Assert.assertTrue(NetUtils.isLinkLocalIp4("169.254.149.120"));
Assert.assertTrue(NetUtils.isLinkLocalIp4("169.254.0.1"));
Assert.assertFalse(NetUtils.isLinkLocalIp4("172.25.16.236"));
Assert.assertFalse(NetUtils.isLinkLocalIp4("169.255.0.1"));
Assert.assertFalse(NetUtils.isLinkLocalIp4("fe80::1"));
Assert.assertFalse(NetUtils.isLinkLocalIp4("not-an-ip"));
Assert.assertFalse(NetUtils.isLinkLocalIp4(null));
}

@Test
public void testIsLinkLocalIp6() {
Assert.assertTrue(NetUtils.isLinkLocalIp6("fe80::df84:2984:e396:8ac3"));
Assert.assertTrue(NetUtils.isLinkLocalIp6("fe80::df84:2984:e396:8ac3%6"));
Assert.assertTrue(NetUtils.isLinkLocalIp6("febf::1"));
Assert.assertFalse(NetUtils.isLinkLocalIp6("fd00:ac19:0:1910:0:d3fc:4177:2c32"));
Assert.assertFalse(NetUtils.isLinkLocalIp6("::1"));
Assert.assertFalse(NetUtils.isLinkLocalIp6("169.254.149.120"));
Assert.assertFalse(NetUtils.isLinkLocalIp6(null));
Assert.assertFalse(NetUtils.isLinkLocalIp6("%"));
Assert.assertFalse(NetUtils.isLinkLocalIp6("%6"));
Assert.assertFalse(NetUtils.isLinkLocalIp6(""));
}
}
Loading