Skip to content

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

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

Merged
rohityadavcloud merged 3 commits into from
Feb 1, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bdec7ba
server: keep networks order and ips while move a vm with multiple net…
ustcweizhou Jan 19, 2021
064ca5e
#4602: use removeNics instead of expungeNics
ustcweizhou Jan 26, 2021
dbb35b6
#4602: create separated method
ustcweizhou Jan 26, 2021
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
2 changes: 1 addition & 1 deletion .../java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ void prepare(VirtualMachineProfile profile, DeployDestination dest, ReservationC

void cleanupNics(VirtualMachineProfile vm);

void expungeNics(VirtualMachineProfile vm);
void removeNics(VirtualMachineProfile vm);

List<NicProfile> getNicProfiles(VirtualMachine vm);

Expand Down
6 changes: 3 additions & 3 deletions ...tration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2168,10 +2168,10 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}

@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());
}
}

Expand Down
96 changes: 63 additions & 33 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 @@ -24,8 +24,8 @@
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;
Expand Down Expand Up @@ -6218,7 +6218,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
_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>();
Expand Down Expand Up @@ -6280,34 +6280,25 @@ public void doInTransactionWithoutResult(TransactionStatus status) {

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
Expand All @@ -6330,10 +6321,24 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
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;
Expand All @@ -6342,11 +6347,12 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
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)));

}
Expand Down Expand Up @@ -6409,27 +6415,20 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
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) {
Expand All @@ -6449,6 +6448,16 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
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()) {
Expand Down Expand Up @@ -6512,6 +6521,10 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
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;
Expand All @@ -6521,6 +6534,8 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
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());
Expand All @@ -6533,6 +6548,21 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
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
Expand Down
4 changes: 2 additions & 2 deletions server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,10 +580,10 @@ public void cleanupNics(VirtualMachineProfile vm) {
}

/* (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

}
Expand Down