diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java index 1e8135460ab..183606d5cc6 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java @@ -129,7 +129,7 @@ public interface NetworkOrchestrationService { void cleanupNics(VirtualMachineProfile vm); - void expungeNics(VirtualMachineProfile vm); + void removeNics(VirtualMachineProfile vm); List getNicProfiles(VirtualMachine vm); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 5cf195ea0b9..363d38eebd0 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -2168,10 +2168,10 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } @Override - public void expungeNics(final VirtualMachineProfile vm) { - final List nics = _nicDao.listByVmIdIncludingRemoved(vm.getId()); + public void removeNics(final VirtualMachineProfile vm) { + final List nics = _nicDao.listByVmId(vm.getId()); for (final NicVO nic : nics) { - _nicDao.expunge(nic.getId()); + _nicDao.remove(nic.getId()); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 45782d9d4cc..adf00ba1935 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -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 networkList = new ArrayList(); @@ -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 applicableNetworks = new LinkedHashSet<>(); + Map requestedIPv4ForNics = new HashMap<>(); + Map requestedIPv6ForNics = new HashMap<>(); if (zone.isSecurityGroupEnabled()) { // advanced zone with security groups // cleanup the old security groups _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId()); - - Set applicableNetworks = new HashSet(); - 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> networks = new LinkedHashMap>(); 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(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 applicableNetworks = new HashSet(); // 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> networks = new LinkedHashMap>(); 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(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 diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index c962d90d2dd..0cc10ea7bb5 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -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 }