server: keep networks order and ips while move a vm with multiple networks (#4602)

This PR fixes an issue when move a vm from an account to another account.

Steps to reproduce the issue
(1) create a vm with multiple shared networks (in advanced zone, or advanced zone with security groups)
(2) create another account (in same domain who can also access the shared networks)
(3) move vm to new account, with a list of networkid

expected result: the vm has nics on the networks in same order as specified in API request, and nics have the same ips as before actual result: network order is not same as specified, ips are changed.
This commit is contained in:
Wei Zhou 2021-02-01 09:44:20 +01:00 committed by GitHub
parent 890e84777c
commit 1913c6854e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 69 additions and 39 deletions

View File

@ -129,7 +129,7 @@ public interface NetworkOrchestrationService {
void cleanupNics(VirtualMachineProfile vm);
void expungeNics(VirtualMachineProfile vm);
void removeNics(VirtualMachineProfile vm);
List<NicProfile> getNicProfiles(VirtualMachine vm);

View File

@ -2168,10 +2168,10 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra
}
@Override
public void expungeNics(final VirtualMachineProfile vm) {
final List<NicVO> nics = _nicDao.listByVmIdIncludingRemoved(vm.getId());
public void removeNics(final VirtualMachineProfile vm) {
final List<NicVO> nics = _nicDao.listByVmId(vm.getId());
for (final NicVO nic : nics) {
_nicDao.expunge(nic.getId());
_nicDao.remove(nic.getId());
}
}

View File

@ -24,8 +24,8 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@ -6221,7 +6221,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
_securityGroupMgr.removeInstanceFromGroups(cmd.getVmId());
// cleanup the network for the oldOwner
_networkMgr.cleanupNics(vmOldProfile);
_networkMgr.expungeNics(vmOldProfile);
_networkMgr.removeNics(vmOldProfile);
// security groups will be recreated for the new account, when the
// VM is started
List<NetworkVO> networkList = new ArrayList<NetworkVO>();
@ -6283,34 +6283,25 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
s_logger.debug("AssignVM: Basic zone, adding security groups no " + securityGroupIdList.size() + " to " + vm.getInstanceName());
} else {
Set<NetworkVO> applicableNetworks = new LinkedHashSet<>();
Map<Long, String> requestedIPv4ForNics = new HashMap<>();
Map<Long, String> requestedIPv6ForNics = new HashMap<>();
if (zone.isSecurityGroupEnabled()) { // advanced zone with security groups
// cleanup the old security groups
_securityGroupMgr.removeInstanceFromGroups(cmd.getVmId());
Set<NetworkVO> applicableNetworks = new HashSet<NetworkVO>();
String requestedIPv4ForDefaultNic = null;
String requestedIPv6ForDefaultNic = null;
// if networkIdList is null and the first network of vm is shared network, then keep it if possible
if (networkIdList == null || networkIdList.isEmpty()) {
NicVO defaultNicOld = _nicDao.findDefaultNicForVM(vm.getId());
if (defaultNicOld != null) {
NetworkVO defaultNetworkOld = _networkDao.findById(defaultNicOld.getNetworkId());
if (defaultNetworkOld != null && defaultNetworkOld.getGuestType() == Network.GuestType.Shared && defaultNetworkOld.getAclType() == ACLType.Domain) {
try {
_networkModel.checkNetworkPermissions(newAccount, defaultNetworkOld);
applicableNetworks.add(defaultNetworkOld);
requestedIPv4ForDefaultNic = defaultNicOld.getIPv4Address();
requestedIPv6ForDefaultNic = defaultNicOld.getIPv6Address();
s_logger.debug("AssignVM: use old shared network " + defaultNetworkOld.getName() + " with old ip " + requestedIPv4ForDefaultNic + " on default nic of vm:" + vm.getInstanceName());
} catch (PermissionDeniedException e) {
s_logger.debug("AssignVM: the shared network on old default nic can not be applied to new account");
}
if (canAccountUseNetwork(newAccount, defaultNetworkOld)) {
applicableNetworks.add(defaultNetworkOld);
requestedIPv4ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv4Address());
requestedIPv6ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv6Address());
s_logger.debug("AssignVM: use old shared network " + defaultNetworkOld.getName() + " with old ip " + defaultNicOld.getIPv4Address() + " on default nic of vm:" + vm.getInstanceName());
}
}
}
// cleanup the network for the oldOwner
_networkMgr.cleanupNics(vmOldProfile);
_networkMgr.expungeNics(vmOldProfile);
if (networkIdList != null && !networkIdList.isEmpty()) {
// add any additional networks
@ -6333,10 +6324,24 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
ex.addProxyObject(network.getUuid(), "networkId");
throw ex;
}
if (network.getGuestType() == Network.GuestType.Shared && network.getAclType() == ACLType.Domain) {
NicVO nicOld = _nicDao.findByNtwkIdAndInstanceId(network.getId(), vm.getId());
if (nicOld != null) {
requestedIPv4ForNics.put(network.getId(), nicOld.getIPv4Address());
requestedIPv6ForNics.put(network.getId(), nicOld.getIPv6Address());
s_logger.debug("AssignVM: use old shared network " + network.getName() + " with old ip " + nicOld.getIPv4Address() + " on nic of vm:" + vm.getInstanceName());
}
}
s_logger.debug("AssignVM: Added network " + network.getName() + " to vm " + vm.getId());
applicableNetworks.add(network);
}
}
// cleanup the network for the oldOwner
_networkMgr.cleanupNics(vmOldProfile);
_networkMgr.removeNics(vmOldProfile);
// add the new nics
LinkedHashMap<Network, List<? extends NicProfile>> networks = new LinkedHashMap<Network, List<? extends NicProfile>>();
int toggle = 0;
@ -6345,11 +6350,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
NicProfile defaultNic = new NicProfile();
if (toggle == 0) {
defaultNic.setDefaultNic(true);
defaultNic.setRequestedIPv4(requestedIPv4ForDefaultNic);
defaultNic.setRequestedIPv6(requestedIPv6ForDefaultNic);
defaultNetwork = appNet;
toggle++;
}
defaultNic.setRequestedIPv4(requestedIPv4ForNics.get(appNet.getId()));
defaultNic.setRequestedIPv6(requestedIPv6ForNics.get(appNet.getId()));
networks.put(appNet, new ArrayList<NicProfile>(Arrays.asList(defaultNic)));
}
@ -6412,27 +6418,20 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
if (securityGroupIdList != null && !securityGroupIdList.isEmpty()) {
throw new InvalidParameterValueException("Can't move vm with security groups; security group feature is not enabled in this zone");
}
Set<NetworkVO> applicableNetworks = new HashSet<NetworkVO>();
// if networkIdList is null and the first network of vm is shared network, then keep it if possible
if (networkIdList == null || networkIdList.isEmpty()) {
NicVO defaultNicOld = _nicDao.findDefaultNicForVM(vm.getId());
if (defaultNicOld != null) {
NetworkVO defaultNetworkOld = _networkDao.findById(defaultNicOld.getNetworkId());
if (defaultNetworkOld != null && defaultNetworkOld.getGuestType() == Network.GuestType.Shared && defaultNetworkOld.getAclType() == ACLType.Domain) {
try {
_networkModel.checkNetworkPermissions(newAccount, defaultNetworkOld);
applicableNetworks.add(defaultNetworkOld);
} catch (PermissionDeniedException e) {
s_logger.debug("AssignVM: the shared network on old default nic can not be applied to new account");
}
if (canAccountUseNetwork(newAccount, defaultNetworkOld)) {
applicableNetworks.add(defaultNetworkOld);
requestedIPv4ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv4Address());
requestedIPv6ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv6Address());
s_logger.debug("AssignVM: use old shared network " + defaultNetworkOld.getName() + " with old ip " + defaultNicOld.getIPv4Address() + " on default nic of vm:" + vm.getInstanceName());
}
}
}
// cleanup the network for the oldOwner
_networkMgr.cleanupNics(vmOldProfile);
_networkMgr.expungeNics(vmOldProfile);
if (networkIdList != null && !networkIdList.isEmpty()) {
// add any additional networks
for (Long networkId : networkIdList) {
@ -6452,6 +6451,16 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
ex.addProxyObject(network.getUuid(), "networkId");
throw ex;
}
if (network.getGuestType() == Network.GuestType.Shared && network.getAclType() == ACLType.Domain) {
NicVO nicOld = _nicDao.findByNtwkIdAndInstanceId(network.getId(), vm.getId());
if (nicOld != null) {
requestedIPv4ForNics.put(network.getId(), nicOld.getIPv4Address());
requestedIPv6ForNics.put(network.getId(), nicOld.getIPv6Address());
s_logger.debug("AssignVM: use old shared network " + network.getName() + " with old ip " + nicOld.getIPv4Address() + " on nic of vm:" + vm.getInstanceName());
}
}
s_logger.debug("AssignVM: Added network " + network.getName() + " to vm " + vm.getId());
applicableNetworks.add(network);
}
} else if (applicableNetworks.isEmpty()) {
@ -6515,6 +6524,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
applicableNetworks.add(defaultNetwork);
}
// cleanup the network for the oldOwner
_networkMgr.cleanupNics(vmOldProfile);
_networkMgr.removeNics(vmOldProfile);
// add the new nics
LinkedHashMap<Network, List<? extends NicProfile>> networks = new LinkedHashMap<Network, List<? extends NicProfile>>();
int toggle = 0;
@ -6524,6 +6537,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
defaultNic.setDefaultNic(true);
toggle++;
}
defaultNic.setRequestedIPv4(requestedIPv4ForNics.get(appNet.getId()));
defaultNic.setRequestedIPv6(requestedIPv6ForNics.get(appNet.getId()));
networks.put(appNet, new ArrayList<NicProfile>(Arrays.asList(defaultNic)));
}
VirtualMachine vmi = _itMgr.findById(vm.getId());
@ -6536,6 +6551,21 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
return vm;
}
private boolean canAccountUseNetwork(Account newAccount, Network network) {
if (network != null && network.getAclType() == ACLType.Domain
&& (network.getGuestType() == Network.GuestType.Shared
|| network.getGuestType() == Network.GuestType.L2)) {
try {
_networkModel.checkNetworkPermissions(newAccount, network);
return true;
} catch (PermissionDeniedException e) {
s_logger.debug(String.format("AssignVM: %s network %s can not be used by new account %s", network.getGuestType(), network.getName(), newAccount.getAccountName()));
return false;
}
}
return false;
}
@Override
public UserVm restoreVM(RestoreVMCmd cmd) throws InsufficientCapacityException, ResourceUnavailableException {
// Input validation

View File

@ -580,10 +580,10 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches
}
/* (non-Javadoc)
* @see com.cloud.network.NetworkManager#expungeNics(com.cloud.vm.VirtualMachineProfile)
* @see com.cloud.network.NetworkManager#removeNics(com.cloud.vm.VirtualMachineProfile)
*/
@Override
public void expungeNics(VirtualMachineProfile vm) {
public void removeNics(VirtualMachineProfile vm) {
// TODO Auto-generated method stub
}