From 76b34fb30cd6579301ba3d1dbdee703a291a1b29 Mon Sep 17 00:00:00 2001 From: "Glover, Rene (rg9975)" Date: Wed, 12 Feb 2025 09:02:48 -0600 Subject: [PATCH 1/4] add use of virsh domifaddr to get VM external DHCP IP --- .../LibvirtGetVmIpAddressCommandWrapper.java | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java index d65b6907eeb5..3c25ba0a19ec 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java @@ -47,7 +47,6 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin } String sanitizedVmName = sanitizeBashCommandArgument(vmName); String networkCidr = command.getVmNetworkCidr(); - List commands = new ArrayList<>(); final String virt_ls_path = Script.getExecutableAbsolutePath("virt-ls"); final String virt_cat_path = Script.getExecutableAbsolutePath("virt-cat"); final String virt_win_reg_path = Script.getExecutableAbsolutePath("virt-win-reg"); @@ -55,7 +54,26 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin final String grep_path = Script.getExecutableAbsolutePath("grep"); final String awk_path = Script.getExecutableAbsolutePath("awk"); final String sed_path = Script.getExecutableAbsolutePath("sed"); - if(!command.isWindows()) { + final String virsh_path = Script.getExecutableAbsolutePath("virsh"); + + // first run virsh domiflist to get the network interface name from libvirt. This is set if qemu guest agent is running in the VM + // and is the most reliable and least intrusive + List commands = new ArrayList<>(); + commands.add(new String[]{virsh_path, "domifaddr", sanitizedVmName, "--source", "agent"}); + String output = Script.executePipedCommands(commands, 0).second(); + if (output != null) { + String[] lines = output.split("\n"); + for (String line : lines) { + ip = parseDomIfListOutput(line, networkCidr); + if (ip != null) { + break; + } + } + } + + commands.clear(); + commands.add(new String[]{grep_path, ".*\\*"}); + if(ip == null && !command.isWindows()) { //List all dhcp lease files inside guestVm commands.add(new String[]{virt_ls_path, sanitizedVmName, "/var/lib/dhclient/"}); commands.add(new String[]{grep_path, ".*\\*.leases"}); @@ -106,4 +124,20 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin } return new Answer(command, result, ip); } + + private String parseDomIfListOutput(String line, String networkCidr) { + String ip = null; + if (line.contains("ipv4")) { + String[] parts = line.split(" "); + if (parts.length > 2) { + String[] ipParts = parts[2].split("/"); + if (ipParts.length > 0) { + if (NetUtils.isIpWithInCidrRange(ipParts[0], networkCidr)) { + ip = ipParts[0]; + } + } + } + } + return ip; + } } From 1e3cfbd3db3769388717af71ada08e3284d23657 Mon Sep 17 00:00:00 2001 From: "Glover, Rene (rg9975)" Date: Wed, 26 Feb 2025 09:00:33 -0600 Subject: [PATCH 2/4] updates to modularize LibvirtGetVmIpAddressCommandWrapper per comments; added test cases to cover 90%+ scenarios --- .../LibvirtGetVmIpAddressCommandWrapper.java | 181 ++++++----- ...bvirtGetVmIpAddressCommandWrapperTest.java | 301 ++++++++++++++++++ 2 files changed, 409 insertions(+), 73 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapperTest.java diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java index 3c25ba0a19ec..0dd52ddfb105 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java @@ -29,6 +29,7 @@ import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.Pair; import com.cloud.utils.net.NetUtils; import com.cloud.utils.script.Script; @@ -37,6 +38,26 @@ public final class LibvirtGetVmIpAddressCommandWrapper extends CommandWrapper commands = new ArrayList<>(); commands.add(new String[]{virsh_path, "domifaddr", sanitizedVmName, "--source", "agent"}); - String output = Script.executePipedCommands(commands, 0).second(); - if (output != null) { + Pair response = executePipedCommands(commands, 0); + if (response != null) { + String output = response.second(); String[] lines = output.split("\n"); for (String line : lines) { - ip = parseDomIfListOutput(line, networkCidr); - if (ip != null) { - break; + if (line.contains("ipv4")) { + String[] parts = line.split(" "); + String[] ipParts = parts[parts.length-1].split("/"); + if (ipParts.length > 1) { + if (NetUtils.isIpWithInCidrRange(ipParts[0], networkCidr)) { + ip = ipParts[0]; + break; + } + } } } + } else { + s_logger.error("ipFromDomIf: Command execution failed for VM: " + sanitizedVmName); } + return ip; + } - commands.clear(); - commands.add(new String[]{grep_path, ".*\\*"}); - if(ip == null && !command.isWindows()) { - //List all dhcp lease files inside guestVm - commands.add(new String[]{virt_ls_path, sanitizedVmName, "/var/lib/dhclient/"}); - commands.add(new String[]{grep_path, ".*\\*.leases"}); - String leasesList = Script.executePipedCommands(commands, 0).second(); - if(leasesList != null) { - String[] leasesFiles = leasesList.split("\n"); - for(String leaseFile : leasesFiles){ - //Read from each dhclient lease file inside guest Vm using virt-cat libguestfs utility - commands = new ArrayList<>(); - commands.add(new String[]{virt_cat_path, sanitizedVmName, "/var/lib/dhclient/" + leaseFile}); - commands.add(new String[]{tail_path, "-16"}); - commands.add(new String[]{grep_path, "fixed-address"}); - commands.add(new String[]{awk_path, "{print $2}"}); - commands.add(new String[]{sed_path, "-e", "s/;//"}); - String ipAddr = Script.executePipedCommands(commands, 0).second(); - // Check if the IP belongs to the network - if((ipAddr != null) && NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)) { - ip = ipAddr; - break; - } - s_logger.debug("GetVmIp: "+ vmName + " Ip: "+ipAddr+" does not belong to network "+networkCidr); + private String ipFromDhcpLeaseFile(String sanitizedVmName, String networkCidr) { + String ip = null; + List commands = new ArrayList<>(); + commands.add(new String[]{virt_ls_path, sanitizedVmName, "/var/lib/dhclient/"}); + commands.add(new String[]{grep_path, ".*\\*.leases"}); + Pair response = executePipedCommands(commands, 0); + + if(response != null && response.second() != null) { + String leasesList = response.second(); + String[] leasesFiles = leasesList.split("\n"); + for(String leaseFile : leasesFiles){ + commands = new ArrayList<>(); + commands.add(new String[]{virt_cat_path, sanitizedVmName, "/var/lib/dhclient/" + leaseFile}); + commands.add(new String[]{tail_path, "-16"}); + commands.add(new String[]{grep_path, "fixed-address"}); + commands.add(new String[]{awk_path, "{print $2}"}); + commands.add(new String[]{sed_path, "-e", "s/;//"}); + String ipAddr = executePipedCommands(commands, 0).second(); + if((ipAddr != null) && NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)) { + ip = ipAddr; + break; } + s_logger.debug("GetVmIp: "+ sanitizedVmName + " Ip: "+ipAddr+" does not belong to network "+networkCidr); } } else { - // For windows, read from guest Vm registry using virt-win-reg libguestfs ulitiy. Registry Path: HKEY_LOCAL_MACHINE\SYSTEM\ControlSet001\Services\Tcpip\Parameters\Interfaces\\DhcpIPAddress - commands = new ArrayList<>(); - commands.add(new String[]{virt_win_reg_path, "--unsafe-printable-strings", sanitizedVmName, "HKEY_LOCAL_MACHINE\\SYSTEM\\ControlSet001\\Services\\Tcpip\\Parameters\\Interfaces"}); - commands.add(new String[]{grep_path, "DhcpIPAddress"}); - commands.add(new String[]{awk_path, "-F", ":", "{print $2}"}); - commands.add(new String[]{sed_path, "-e", "s/^\"//", "-e", "s/\"$//"}); - String ipList = Script.executePipedCommands(commands, 0).second(); - if(ipList != null) { - s_logger.debug("GetVmIp: "+ vmName + "Ips: "+ipList); - String[] ips = ipList.split("\n"); - for (String ipAddr : ips){ - // Check if the IP belongs to the network - if((ipAddr != null) && NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)){ - ip = ipAddr; - break; - } - s_logger.debug("GetVmIp: "+ vmName + " Ip: "+ipAddr+" does not belong to network "+networkCidr); - } - } - } - if(ip != null){ - result = true; - s_logger.debug("GetVmIp: "+ vmName + " Found Ip: "+ip); + s_logger.error("ipFromDhcpLeaseFile: Command execution failed for VM: " + sanitizedVmName); } - return new Answer(command, result, ip); + return ip; } - private String parseDomIfListOutput(String line, String networkCidr) { + private String ipFromWindowsRegistry(String sanitizedVmName, String networkCidr) { String ip = null; - if (line.contains("ipv4")) { - String[] parts = line.split(" "); - if (parts.length > 2) { - String[] ipParts = parts[2].split("/"); - if (ipParts.length > 0) { - if (NetUtils.isIpWithInCidrRange(ipParts[0], networkCidr)) { - ip = ipParts[0]; - } + List commands = new ArrayList<>(); + commands.add(new String[]{virt_win_reg_path, "--unsafe-printable-strings", sanitizedVmName, "HKEY_LOCAL_MACHINE\\SYSTEM\\ControlSet001\\Services\\Tcpip\\Parameters\\Interfaces"}); + commands.add(new String[]{grep_path, "DhcpIPAddress"}); + commands.add(new String[]{awk_path, "-F", ":", "{print $2}"}); + commands.add(new String[]{sed_path, "-e", "s/^\"//", "-e", "s/\"$//"}); + Pair pair = executePipedCommands(commands, 0); + if(pair != null && pair.second() != null) { + String ipList = pair.second(); + ipList = ipList.replaceAll("\"", ""); + s_logger.debug("GetVmIp: "+ sanitizedVmName + "Ips: "+ipList); + String[] ips = ipList.split("\n"); + for (String ipAddr : ips){ + if((ipAddr != null) && NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)){ + ip = ipAddr; + break; } + s_logger.debug("GetVmIp: "+ sanitizedVmName + " Ip: "+ipAddr+" does not belong to network "+networkCidr); } + } else { + s_logger.error("ipFromWindowsRegistry: Command execution failed for VM: " + sanitizedVmName); } return ip; } + + static Pair executePipedCommands(List commands, long timeout) { + return Script.executePipedCommands(commands, timeout); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapperTest.java new file mode 100644 index 000000000000..697144c31b55 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapperTest.java @@ -0,0 +1,301 @@ +package com.cloud.hypervisor.kvm.resource.wrapper; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.MockitoAnnotations; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.GetVmIpAddressCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.utils.Pair; +import com.cloud.utils.script.Script; + +public class LibvirtGetVmIpAddressCommandWrapperTest { + + private static String VIRSH_DOMIF_OUTPUT = " Name MAC address Protocol Address\n" + // + "-------------------------------------------------------------------------------\n" + // + " lo 00:00:00:00:00:70 ipv4 127.0.0.1/8\n" + // + " eth0 02:0c:02:f9:00:80 ipv4 192.168.0.10/24\n" + // + " net1 b2:41:19:69:a4:90 N/A N/A\n" + // + " net2 52:a2:36:cf:d1:50 ipv4 10.244.6.93/32\n" + // + " net3 a6:1d:d3:52:d3:40 N/A N/A\n" + // + " net4 2e:9b:60:dc:49:30 N/A N/A\n" + // + " lxc5b7327203b6f 92:b2:77:0b:a9:20 N/A N/A\n"; + + @Before + public void setUp() { + MockitoAnnotations.openMocks(this); + } + + @Test + public void testExecuteWithValidVmName() { + LibvirtComputingResource libvirtComputingResource = mock(LibvirtComputingResource.class); + GetVmIpAddressCommand getVmIpAddressCommand = mock(GetVmIpAddressCommand.class); + LibvirtGetVmIpAddressCommandWrapper commandWrapper = new LibvirtGetVmIpAddressCommandWrapper(); + MockedStatic