Skip to content

VMware: match nic mac for ip address fetch #10641

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: 4.20
Choose a base branch
from
Open

VMware: match nic mac for ip address fetch #10641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
559c011
match nic mac for ip address fetch
Mar 29, 2025
4a40b00
rename field to properly match usage
Mar 29, 2025
2726aca
rebase to 4.20 and adapt code to match other PR
Apr 25, 2025
0dd48f1
Merge branch '4.20' into fix-vmware-ip-loss
DaanHoogland May 12, 2025
323eb1f
Merge branch '4.20' into fix-vmware-ip-loss
DaanHoogland May 13, 2025
5f0e76f
Update UserVmManagerImpl.java
alexandru-bagu May 19, 2025
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
16 changes: 12 additions & 4 deletions ...hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5828,11 +5828,19 @@
if (toolsStatus == VirtualMachineToolsStatus.TOOLS_NOT_INSTALLED) {
details += "Vmware tools not installed.";
} else {
ip = guestInfo.getIpAddress();
if (ip != null) {
result = true;
var normalizedMac = cmd.getMacAddress().replaceAll("-", ":");
for(var guestInfoNic : guestInfo.getNet()) {
var normalizedNicMac = guestInfoNic.getMacAddress().replaceAll("-", ":");

Check warning on line 5833 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L5832-L5833 

Added lines #L5832 - L5833 were not covered by tests
if (!result && normalizedNicMac.equalsIgnoreCase(normalizedMac)) {
result = true;
details = null;

Check warning on line 5836 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L5835-L5836 

Added lines #L5835 - L5836 were not covered by tests
for (var ipAddr : guestInfoNic.getIpAddress()) {
if(NetUtils.isIpWithInCidrRange(ipAddr, cmd.getVmNetworkCidr())) {

Check warning on line 5838 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L5838 

Added line #L5838 was not covered by tests
details = ipAddr;
}

Check warning on line 5840 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L5840 

Added line #L5840 was not covered by tests
}
}

Check warning on line 5842 in plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L5842 

Added line #L5842 was not covered by tests
}
details = ip;
}
} else {
details += "VM " + vmName + " no longer exists on vSphere host: " + hyperHost.getHyperHostName();
Expand Down
22 changes: 12 additions & 10 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@
String networkCidr;
String macAddress;

public VmIpAddrFetchThread(long vmId, long nicId, String instanceName, boolean windows, Long hostId, String networkCidr, String macAddress) {
public VmIpAddrFetchThread(long vmId, String vmUuid, long nicId, String instanceName, boolean windows, Long hostId, String networkCidr, String macAddress) {

Check warning on line 754 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L754 

Added line #L754 was not covered by tests
this.vmId = vmId;
this.vmUuid = vmUuid;
this.nicId = nicId;
Expand All @@ -773,8 +773,13 @@
Answer answer = _agentMgr.send(hostId, cmd);
if (answer.getResult()) {
String vmIp = answer.getDetails();

if (NetUtils.isValidIp4(vmIp)) {
if (null == vmIp) {
// we got a valid response and the NIC does not have an IP assigned, as such we will update the database with null
if (nic.getIPv4Address() != null) {
nic.setIPv4Address(null);
_nicDao.update(nicId, nic);

Check warning on line 780 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L779-L780 

Added lines #L779 - L780 were not covered by tests
}
} else if (NetUtils.isValidIp4(vmIp)) {
// set this vm ip addr in vm nic.
if (nic != null) {
nic.setIPv4Address(vmIp);
Expand All @@ -789,12 +794,8 @@
}
}
} else {
//previously vm has ip and nic table has ip address. After vm restart or stop/start
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, if we cannot know if the VM still has the IP assigned or not then we should not modify the database. In other words, we should be sure that the VM's NIC does not have an IP assigned before updating the database.

//if vm doesnot get the ip then set the ip in nic table to null
if (nic.getIPv4Address() != null) {
nic.setIPv4Address(null);
_nicDao.update(nicId, nic);
}
// since no changes are being done, we should not decrement IP usage
decrementCount = false;

Check warning on line 798 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L798 

Added line #L798 was not covered by tests
if (answer.getDetails() != null) {
logger.debug("Failed to get vm ip for Vm [id: {}, uuid: {}, name: {}], details: {}",
vmId, vmUuid, vmName, answer.getDetails());
Expand Down Expand Up @@ -2688,7 +2689,8 @@
VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(userVm);
VirtualMachine vm = vmProfile.getVirtualMachine();
boolean isWindows = _guestOSCategoryDao.findById(_guestOSDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows");
_vmIpFetchThreadExecutor.execute(new VmIpAddrFetchThread(vmId, nicId, vmInstance.getInstanceName(),

_vmIpFetchThreadExecutor.execute(new VmIpAddrFetchThread(vmId, vmInstance.getUuid(), nicId, vmInstance.getInstanceName(),

Check warning on line 2693 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L2693 

Added line #L2693 was not covered by tests
isWindows, vm.getHostId(), network.getCidr(), nicVo.getMacAddress()));

}
Expand Down
Loading