From 7a29dfffe17670e20cee39c996e8f34eec69fc70 Mon Sep 17 00:00:00 2001 From: Bharat Kumar Date: Thu, 6 Aug 2015 14:15:48 +0530 Subject: [PATCH 01/10] CLOUDSTACK-8751 Minimise network downtime during network updates when redundant VR is being used. database schema changes Made changes to the updateNetwork API. --- api/src/com/cloud/network/Network.java | 2 + api/src/com/cloud/network/NetworkService.java | 2 +- .../network/element/RedundantResource.java | 11 + .../cloud/network/router/VirtualRouter.java | 4 + .../apache/cloudstack/api/ApiConstants.java | 1 + .../network/UpdateNetworkCmdByAdmin.java | 2 +- .../user/network/UpdateNetworkCmd.java | 12 +- .../service/NetworkOrchestrationService.java | 6 + .../orchestration/NetworkOrchestrator.java | 53 +++- .../src/com/cloud/vm/DomainRouterVO.java | 14 + .../com/cloud/network/NetworkServiceImpl.java | 252 ++++++++++-------- .../network/element/VirtualRouterElement.java | 98 ++++++- .../VirtualNetworkApplianceManagerImpl.java | 5 + .../RouterDeploymentDefinition.java | 12 +- .../RouterDeploymentDefinitionBuilder.java | 4 + .../com/cloud/vpc/MockNetworkManagerImpl.java | 17 +- .../RouterDeploymentDefinitionTest.java | 2 +- .../RouterDeploymentDefinitionTestBase.java | 3 + setup/db/db/schema-452to460.sql | 2 + 19 files changed, 377 insertions(+), 125 deletions(-) create mode 100644 api/src/com/cloud/network/element/RedundantResource.java diff --git a/api/src/com/cloud/network/Network.java b/api/src/com/cloud/network/Network.java index 7cc5441603ae..81447d6457c5 100644 --- a/api/src/com/cloud/network/Network.java +++ b/api/src/com/cloud/network/Network.java @@ -41,6 +41,8 @@ public enum GuestType { Shared, Isolated } + public String updatingInSequence ="updatingInSequence"; + public static class Service { private static List supportedServices = new ArrayList(); diff --git a/api/src/com/cloud/network/NetworkService.java b/api/src/com/cloud/network/NetworkService.java index c1b68eb3a463..e26db340ff4f 100644 --- a/api/src/com/cloud/network/NetworkService.java +++ b/api/src/com/cloud/network/NetworkService.java @@ -77,7 +77,7 @@ IpAddress allocatePortableIP(Account ipOwner, int regionId, Long zoneId, Long ne IpAddress getIp(long id); Network updateGuestNetwork(long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, Long networkOfferingId, - Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String newUUID); + Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String newUUID, boolean updateInSequence); PhysicalNetwork createPhysicalNetwork(Long zoneId, String vnetRange, String networkSpeed, List isolationMethods, String broadcastDomainRange, Long domainId, List tags, String name); diff --git a/api/src/com/cloud/network/element/RedundantResource.java b/api/src/com/cloud/network/element/RedundantResource.java new file mode 100644 index 000000000000..863c9cd330c5 --- /dev/null +++ b/api/src/com/cloud/network/element/RedundantResource.java @@ -0,0 +1,11 @@ +package com.cloud.network.element; + +import com.cloud.network.Network; + +/** + * Created by bharat on 11/08/15. + */ +public interface RedundantResource { + public void configureResource(Network network); + public int getResourceCount(Network network); +} diff --git a/api/src/com/cloud/network/router/VirtualRouter.java b/api/src/com/cloud/network/router/VirtualRouter.java index 0114a962146f..060ef0fa1416 100644 --- a/api/src/com/cloud/network/router/VirtualRouter.java +++ b/api/src/com/cloud/network/router/VirtualRouter.java @@ -26,6 +26,10 @@ public enum Role { VIRTUAL_ROUTER, LB, INTERNAL_LB_VM } + public enum UpdateState { + UPDATE_NEEDED, UPDATE_IN_PROGRESS, UPDATE_COMPLETE, UPDATE_FAILED + } + Role getRole(); boolean getIsRedundantRouter(); diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java index 66ab13a03ed7..f8260a056f6b 100644 --- a/api/src/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/org/apache/cloudstack/api/ApiConstants.java @@ -267,6 +267,7 @@ public class ApiConstants { public static final String USERNAME = "username"; public static final String USER_SECURITY_GROUP_LIST = "usersecuritygrouplist"; public static final String USE_VIRTUAL_NETWORK = "usevirtualnetwork"; + public static final String Update_IN_SEQUENCE ="updateinsequence"; public static final String VALUE = "value"; public static final String VIRTUAL_MACHINE_ID = "virtualmachineid"; public static final String VIRTUAL_MACHINE_IDS = "virtualmachineids"; diff --git a/api/src/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java b/api/src/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java index 269f43ec9596..f2c5119466cc 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java +++ b/api/src/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java @@ -49,7 +49,7 @@ public void execute() throws InsufficientCapacityException, ConcurrentOperationE } Network result = _networkService.updateGuestNetwork(getId(), getNetworkName(), getDisplayText(), callerAccount, - callerUser, getNetworkDomain(), getNetworkOfferingId(), getChangeCidr(), getGuestVmCidr(), getDisplayNetwork(), getCustomId()); + callerUser, getNetworkDomain(), getNetworkOfferingId(), getChangeCidr(), getGuestVmCidr(), getDisplayNetwork(), getCustomId(), getUpdateInSequence()); if (result != null) { diff --git a/api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java b/api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java index 921e74b67b2c..8ef9251e0475 100644 --- a/api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java @@ -75,6 +75,9 @@ public class UpdateNetworkCmd extends BaseAsyncCustomIdCmd { @Parameter(name = ApiConstants.GUEST_VM_CIDR, type = CommandType.STRING, description = "CIDR for guest VMs, CloudStack allocates IPs to guest VMs only from this CIDR") private String guestVmCidr; + @Parameter(name =ApiConstants.Update_IN_SEQUENCE, type=CommandType.BOOLEAN, description = "if true, we will update the routers one after the other. applicable only for redundant router based networks using virtual router as provider") + private Boolean updateInSequence; + @Parameter(name = ApiConstants.DISPLAY_NETWORK, type = CommandType.BOOLEAN, description = "an optional field, whether to the display the network to the end user or not.", authorized = {RoleType.Admin}) @@ -119,6 +122,13 @@ public Boolean getDisplayNetwork() { return displayNetwork; } + public Boolean getUpdateInSequence(){ + if(updateInSequence ==null) + return false; + else + return updateInSequence; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -149,7 +159,7 @@ public void execute() throws InsufficientCapacityException, ConcurrentOperationE Network result = _networkService.updateGuestNetwork(getId(), getNetworkName(), getDisplayText(), callerAccount, callerUser, getNetworkDomain(), getNetworkOfferingId(), - getChangeCidr(), getGuestVmCidr(), getDisplayNetwork(), getCustomId()); + getChangeCidr(), getGuestVmCidr(), getDisplayNetwork(), getCustomId(), getUpdateInSequence()); if (result != null) { NetworkResponse response = _responseGenerator.createNetworkResponse(ResponseView.Restricted, result); diff --git a/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java b/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java index b71aa96d5ccc..1e2761f274a8 100644 --- a/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java +++ b/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java @@ -224,4 +224,10 @@ void implementNetworkElementsAndResources(DeployDestination dest, ReservationCon boolean resourceCountNeedsUpdate(NetworkOffering ntwkOff, ACLType aclType); void prepareAllNicsForMigration(VirtualMachineProfile vm, DeployDestination dest); + + boolean canUpdateInSequence(Network network); + + void configureUpdateInSequence(Network network); + + int getResourceCount(Network network); } diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 95cf45df8ff8..420d1ca32334 100644 --- a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -36,6 +36,12 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.network.Networks; + +import com.cloud.network.dao.NetworkDetailsDao; +import com.cloud.network.element.RedundantResource; +import com.cloud.vm.dao.DomainRouterDao; +import org.apache.log4j.Logger; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.cloud.entity.api.db.VMNetworkMapVO; @@ -50,7 +56,6 @@ import org.apache.cloudstack.framework.messagebus.PublishScope; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.region.PortableIpDao; -import org.apache.log4j.Logger; import com.cloud.agent.AgentManager; import com.cloud.agent.Listener; @@ -107,7 +112,6 @@ import com.cloud.network.NetworkModel; import com.cloud.network.NetworkProfile; import com.cloud.network.NetworkStateListener; -import com.cloud.network.Networks; import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.Networks.TrafficType; import com.cloud.network.PhysicalNetwork; @@ -265,9 +269,12 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra MessageBus _messageBus; @Inject VMNetworkMapDao _vmNetworkMapDao; + @Inject + DomainRouterDao _rotuerDao; List networkGurus; + public List getNetworkGurus() { return networkGurus; } @@ -350,6 +357,8 @@ public void setDhcpProviders(final List dhcpProviders) { PortableIpDao _portableIpDao; @Inject ConfigDepot _configDepot; + @Inject + NetworkDetailsDao _networkDetailsDao; protected StateMachine2 _stateMachine; ScheduledExecutorService _executor; @@ -1271,6 +1280,46 @@ protected boolean prepareElement(final NetworkElement element, final Network net return true; } + @Override + public boolean canUpdateInSequence(Network network){ + List providers = getNetworkProviders(network.getId()); + //check if the there are no service provider other than virtualrouter. + for(Provider provider :providers){ + if(provider!=Provider.VirtualRouter) + throw new UnsupportedOperationException("Cannot update the network resources in sequence when providers other than virtualrouter are used"); + } + return true; + } + + @Override + public void configureUpdateInSequence(Network network) { + List providers = getNetworkProviders(network.getId()); + for (NetworkElement element : networkElements) { + if (providers.contains(element.getProvider())) { + if (element instanceof RedundantResource) { + ((RedundantResource) element).configureResource(network); + } + } + } + } + + @Override + public int getResourceCount(Network network){ + List providers = getNetworkProviders(network.getId()); + int resourceCount=0; + for (NetworkElement element : networkElements) { + if (providers.contains(element.getProvider())) { + //currently only one element implements the redundant resource interface + if (element instanceof RedundantResource) { + resourceCount= ((RedundantResource) element).getResourceCount(network); + break; + } + } + } + return resourceCount; + } + + @DB protected void updateNic(final NicVO nic, final long networkId, final int count) { Transaction.execute(new TransactionCallbackNoReturn() { diff --git a/engine/schema/src/com/cloud/vm/DomainRouterVO.java b/engine/schema/src/com/cloud/vm/DomainRouterVO.java index 2596d24a5fb5..2a7aa49b6ed4 100644 --- a/engine/schema/src/com/cloud/vm/DomainRouterVO.java +++ b/engine/schema/src/com/cloud/vm/DomainRouterVO.java @@ -69,6 +69,11 @@ public class DomainRouterVO extends VMInstanceVO implements VirtualRouter { @Column(name = "vpc_id") private Long vpcId; + + @Column(name= "update_state") + @Enumerated(EnumType.STRING) + private UpdateState updateState; + public DomainRouterVO(final long id, final long serviceOfferingId, final long elementId, final String name, final long templateId, final HypervisorType hypervisorType, final long guestOSId, final long domainId, final long accountId, final long userId, final boolean isRedundantRouter, final RedundantState redundantState, final boolean haEnabled, final boolean stopPending, final Long vpcId) { @@ -193,4 +198,13 @@ public Long getVpcId() { return vpcId; } + public UpdateState getUpdateState() { + return updateState; + } + + public void setUpdateState(UpdateState updateState) { + this.updateState = updateState; + } + + } diff --git a/server/src/com/cloud/network/NetworkServiceImpl.java b/server/src/com/cloud/network/NetworkServiceImpl.java index b6dac872f30a..bb573decde28 100644 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -108,6 +108,8 @@ import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.LoadBalancerVMMapDao; import com.cloud.network.dao.NetworkDao; +import com.cloud.network.dao.NetworkDetailVO; +import com.cloud.network.dao.NetworkDetailsDao; import com.cloud.network.dao.NetworkDomainDao; import com.cloud.network.dao.NetworkDomainVO; import com.cloud.network.dao.NetworkServiceMapDao; @@ -178,6 +180,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.exception.ExceptionUtil; import com.cloud.utils.net.NetUtils; +import com.cloud.vm.DomainRouterVO; import com.cloud.vm.Nic; import com.cloud.vm.NicSecondaryIp; import com.cloud.vm.NicVO; @@ -187,6 +190,7 @@ import com.cloud.vm.UserVmVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; +import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.NicSecondaryIpDao; import com.cloud.vm.dao.NicSecondaryIpVO; @@ -324,6 +328,12 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { @Inject MessageBus _messageBus; + @Inject + DomainRouterDao _routerDao; + + @Inject + NetworkDetailsDao _networkDetailsDao; + int _cidrLimit; boolean _allowSubdomainNetworkAccess; @@ -1992,8 +2002,7 @@ private boolean checkForNonStoppedVmInNetwork(long networkId) { @DB @ActionEvent(eventType = EventTypes.EVENT_NETWORK_UPDATE, eventDescription = "updating network", async = true) public Network updateGuestNetwork(final long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, - final Long networkOfferingId, Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String customId) { - + final Long networkOfferingId, Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String customId, boolean updateInSequence) { boolean restartNetwork = false; // verify input parameters @@ -2239,130 +2248,153 @@ public Network updateGuestNetwork(final long networkId, String name, String disp ReservationContext context = new ReservationContextImpl(null, null, callerUser, callerAccount); // 1) Shutdown all the elements and cleanup all the rules. Don't allow to shutdown network in intermediate // states - Shutdown and Implementing + List routers=null; + int resourceCount=1; + if(updateInSequence && restartNetwork && _networkOfferingDao.findById(network.getNetworkOfferingId()).getRedundantRouter() && networkOfferingId!=null && _networkOfferingDao.findById(networkOfferingId).getRedundantRouter() && network.getVpcId()==null) { + _networkMgr.canUpdateInSequence(network); + NetworkDetailVO networkDetail =new NetworkDetailVO(network.getId(),Network.updatingInSequence,"true",true); + _networkDetailsDao.persist(networkDetail); + _networkMgr.configureUpdateInSequence(network); + resourceCount=_networkMgr.getResourceCount(network); + } + boolean validStateToShutdown = (network.getState() == Network.State.Implemented || network.getState() == Network.State.Setup || network.getState() == Network.State.Allocated); - if (restartNetwork) { - if (validStateToShutdown) { - if (!changeCidr) { - s_logger.debug("Shutting down elements and resources for network id=" + networkId + " as a part of network update"); - - if (!_networkMgr.shutdownNetworkElementsAndResources(context, true, network)) { - s_logger.warn("Failed to shutdown the network elements and resources as a part of network restart: " + network); - CloudRuntimeException ex = new CloudRuntimeException("Failed to shutdown the network elements and resources as a part of update to network of specified id"); - ex.addProxyObject(network.getUuid(), "networkId"); - throw ex; - } - } else { - // We need to shutdown the network, since we want to re-implement the network. - s_logger.debug("Shutting down network id=" + networkId + " as a part of network update"); - - //check if network has reservation - if (NetUtils.isNetworkAWithinNetworkB(network.getCidr(), network.getNetworkCidr())) { - s_logger.warn("Existing IP reservation will become ineffective for the network with id = " + networkId - + " You need to reapply reservation after network reimplementation."); - //set cidr to the newtork cidr - network.setCidr(network.getNetworkCidr()); - //set networkCidr to null to bring network back to no IP reservation state - network.setNetworkCidr(null); - } + try { - if (!_networkMgr.shutdownNetwork(network.getId(), context, true)) { - s_logger.warn("Failed to shutdown the network as a part of update to network with specified id"); - CloudRuntimeException ex = new CloudRuntimeException("Failed to shutdown the network as a part of update of specified network id"); + do { + if (restartNetwork) { + if (validStateToShutdown) { + if (!changeCidr) { + s_logger.debug("Shutting down elements and resources for network id=" + networkId + " as a part of network update"); + + if (!_networkMgr.shutdownNetworkElementsAndResources(context, true, network)) { + s_logger.warn("Failed to shutdown the network elements and resources as a part of network restart: " + network); + CloudRuntimeException ex = new CloudRuntimeException("Failed to shutdown the network elements and resources as a part of update to network of specified id"); + ex.addProxyObject(network.getUuid(), "networkId"); + throw ex; + } + } else { + // We need to shutdown the network, since we want to re-implement the network. + s_logger.debug("Shutting down network id=" + networkId + " as a part of network update"); + + //check if network has reservation + if (NetUtils.isNetworkAWithinNetworkB(network.getCidr(), network.getNetworkCidr())) { + s_logger.warn("Existing IP reservation will become ineffective for the network with id = " + networkId + + " You need to reapply reservation after network reimplementation."); + //set cidr to the newtork cidr + network.setCidr(network.getNetworkCidr()); + //set networkCidr to null to bring network back to no IP reservation state + network.setNetworkCidr(null); + } + + if (!_networkMgr.shutdownNetwork(network.getId(), context, true)) { + s_logger.warn("Failed to shutdown the network as a part of update to network with specified id"); + CloudRuntimeException ex = new CloudRuntimeException("Failed to shutdown the network as a part of update of specified network id"); + ex.addProxyObject(network.getUuid(), "networkId"); + throw ex; + } + } + } else { + CloudRuntimeException ex = new CloudRuntimeException( + "Failed to shutdown the network elements and resources as a part of update to network with specified id; network is in wrong state: " + network.getState()); ex.addProxyObject(network.getUuid(), "networkId"); throw ex; } } - } else { - CloudRuntimeException ex = new CloudRuntimeException( - "Failed to shutdown the network elements and resources as a part of update to network with specified id; network is in wrong state: " + network.getState()); - ex.addProxyObject(network.getUuid(), "networkId"); - throw ex; - } - } - // 2) Only after all the elements and rules are shutdown properly, update the network VO - // get updated network - Network.State networkState = _networksDao.findById(networkId).getState(); - boolean validStateToImplement = (networkState == Network.State.Implemented || networkState == Network.State.Setup || networkState == Network.State.Allocated); - if (restartNetwork && !validStateToImplement) { - CloudRuntimeException ex = new CloudRuntimeException( - "Failed to implement the network elements and resources as a part of update to network with specified id; network is in wrong state: " + networkState); - ex.addProxyObject(network.getUuid(), "networkId"); - throw ex; - } + // 2) Only after all the elements and rules are shutdown properly, update the network VO + // get updated network + Network.State networkState = _networksDao.findById(networkId).getState(); + boolean validStateToImplement = (networkState == Network.State.Implemented || networkState == Network.State.Setup || networkState == Network.State.Allocated); + if (restartNetwork && !validStateToImplement) { + CloudRuntimeException ex = new CloudRuntimeException( + "Failed to implement the network elements and resources as a part of update to network with specified id; network is in wrong state: " + networkState); + ex.addProxyObject(network.getUuid(), "networkId"); + throw ex; + } - if (networkOfferingId != null) { - if (networkOfferingChanged) { - Transaction.execute(new TransactionCallbackNoReturn() { - @Override - public void doInTransactionWithoutResult(TransactionStatus status) { - network.setNetworkOfferingId(networkOfferingId); - _networksDao.update(networkId, network, newSvcProviders); - // get all nics using this network - // log remove usage events for old offering - // log assign usage events for new offering - List nics = _nicDao.listByNetworkId(networkId); - for (NicVO nic : nics) { - long vmId = nic.getInstanceId(); - VMInstanceVO vm = _vmDao.findById(vmId); - if (vm == null) { - s_logger.error("Vm for nic " + nic.getId() + " not found with Vm Id:" + vmId); - continue; + if (networkOfferingId != null) { + if (networkOfferingChanged) { + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + network.setNetworkOfferingId(networkOfferingId); + _networksDao.update(networkId, network, newSvcProviders); + // get all nics using this network + // log remove usage events for old offering + // log assign usage events for new offering + List nics = _nicDao.listByNetworkId(networkId); + for (NicVO nic : nics) { + long vmId = nic.getInstanceId(); + VMInstanceVO vm = _vmDao.findById(vmId); + if (vm == null) { + s_logger.error("Vm for nic " + nic.getId() + " not found with Vm Id:" + vmId); + continue; + } + long isDefault = (nic.isDefaultNic()) ? 1 : 0; + String nicIdString = Long.toString(nic.getId()); + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NETWORK_OFFERING_REMOVE, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), nicIdString, + oldNetworkOfferingId, null, isDefault, VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplay()); + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NETWORK_OFFERING_ASSIGN, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), nicIdString, + networkOfferingId, null, isDefault, VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplay()); + } + } + }); + } else { + network.setNetworkOfferingId(networkOfferingId); + _networksDao.update(networkId, network, + _networkMgr.finalizeServicesAndProvidersForNetwork(_entityMgr.findById(NetworkOffering.class, networkOfferingId), network.getPhysicalNetworkId())); } - long isDefault = (nic.isDefaultNic()) ? 1 : 0; - String nicIdString = Long.toString(nic.getId()); - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NETWORK_OFFERING_REMOVE, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), nicIdString, - oldNetworkOfferingId, null, isDefault, VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplay()); - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NETWORK_OFFERING_ASSIGN, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), nicIdString, - networkOfferingId, null, isDefault, VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplay()); + } else { + _networksDao.update(networkId, network); } + + // 3) Implement the elements and rules again + if (restartNetwork) { + if (network.getState() != Network.State.Allocated) { + DeployDestination dest = new DeployDestination(_dcDao.findById(network.getDataCenterId()), null, null, null); + s_logger.debug("Implementing the network " + network + " elements and resources as a part of network update"); + try { + if (!changeCidr) { + _networkMgr.implementNetworkElementsAndResources(dest, context, network, _networkOfferingDao.findById(network.getNetworkOfferingId())); + } else { + _networkMgr.implementNetwork(network.getId(), dest, context); + } + } catch (Exception ex) { + s_logger.warn("Failed to implement network " + network + " elements and resources as a part of network update due to ", ex); + CloudRuntimeException e = new CloudRuntimeException("Failed to implement network (with specified id) elements and resources as a part of network update"); + e.addProxyObject(network.getUuid(), "networkId"); + throw e; + } } - }); - } else { - network.setNetworkOfferingId(networkOfferingId); - _networksDao.update(networkId, network, - _networkMgr.finalizeServicesAndProvidersForNetwork(_entityMgr.findById(NetworkOffering.class, networkOfferingId), network.getPhysicalNetworkId())); - } - } else { - _networksDao.update(networkId, network); - } - - // 3) Implement the elements and rules again - if (restartNetwork) { - if (network.getState() != Network.State.Allocated) { - DeployDestination dest = new DeployDestination(_dcDao.findById(network.getDataCenterId()), null, null, null); - s_logger.debug("Implementing the network " + network + " elements and resources as a part of network update"); - try { - if (!changeCidr) { - _networkMgr.implementNetworkElementsAndResources(dest, context, network, _networkOfferingDao.findById(network.getNetworkOfferingId())); - } else { - _networkMgr.implementNetwork(network.getId(), dest, context); - } - } catch (Exception ex) { - s_logger.warn("Failed to implement network " + network + " elements and resources as a part of network update due to ", ex); - CloudRuntimeException e = new CloudRuntimeException("Failed to implement network (with specified id) elements and resources as a part of network update"); - e.addProxyObject(network.getUuid(), "networkId"); - throw e; } - } - } - // 4) if network has been upgraded from a non persistent ntwk offering to a persistent ntwk offering, - // implement the network if its not already - if (networkOfferingChanged && !oldNtwkOff.getIsPersistent() && networkOffering.getIsPersistent()) { - if (network.getState() == Network.State.Allocated) { - try { - DeployDestination dest = new DeployDestination(_dcDao.findById(network.getDataCenterId()), null, null, null); - _networkMgr.implementNetwork(network.getId(), dest, context); - } catch (Exception ex) { - s_logger.warn("Failed to implement network " + network + " elements and resources as a part o" + "f network update due to ", ex); - CloudRuntimeException e = new CloudRuntimeException("Failed to implement network (with specified" + " id) elements and resources as a part of network update"); - e.addProxyObject(network.getUuid(), "networkId"); - throw e; + // 4) if network has been upgraded from a non persistent ntwk offering to a persistent ntwk offering, + // implement the network if its not already + if (networkOfferingChanged && !oldNtwkOff.getIsPersistent() && networkOffering.getIsPersistent()) { + if (network.getState() == Network.State.Allocated) { + try { + DeployDestination dest = new DeployDestination(_dcDao.findById(network.getDataCenterId()), null, null, null); + _networkMgr.implementNetwork(network.getId(), dest, context); + } catch (Exception ex) { + s_logger.warn("Failed to implement network " + network + " elements and resources as a part o" + "f network update due to ", ex); + CloudRuntimeException e = new CloudRuntimeException("Failed to implement network (with specified" + " id) elements and resources as a part of network update"); + e.addProxyObject(network.getUuid(), "networkId"); + throw e; + } + } + } + resourceCount--; + } while(updateInSequence && resourceCount>0); + }catch (Exception exception){ + throw new CloudRuntimeException("failed to update network "+network.getUuid()+"due to "+exception.getMessage()); + }finally { + if(updateInSequence){ + if( _networkDetailsDao.findDetail(networkId,Network.updatingInSequence)!=null){ + _networkDetailsDao.removeDetail(networkId,Network.updatingInSequence); } } } - return getNetwork(network.getId()); } diff --git a/server/src/com/cloud/network/element/VirtualRouterElement.java b/server/src/com/cloud/network/element/VirtualRouterElement.java index d802188e4c4b..03d19588af3c 100644 --- a/server/src/com/cloud/network/element/VirtualRouterElement.java +++ b/server/src/com/cloud/network/element/VirtualRouterElement.java @@ -24,6 +24,9 @@ import javax.inject.Inject; +import com.cloud.network.dao.NetworkDetailVO; +import com.cloud.network.dao.NetworkDetailsDao; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.agent.api.to.LoadBalancerTO; import com.cloud.configuration.ConfigurationManager; import com.cloud.dc.DataCenter; @@ -109,7 +112,7 @@ public class VirtualRouterElement extends AdapterBase implements VirtualRouterElementService, DhcpServiceProvider, UserDataServiceProvider, SourceNatServiceProvider, StaticNatServiceProvider, FirewallServiceProvider, LoadBalancingServiceProvider, PortForwardingServiceProvider, RemoteAccessVPNServiceProvider, IpDeployer, -NetworkMigrationResponder, AggregatedCommandExecutor { +NetworkMigrationResponder, AggregatedCommandExecutor, RedundantResource { private static final Logger s_logger = Logger.getLogger(VirtualRouterElement.class); public static final AutoScaleCounterType AutoScaleCounterCpu = new AutoScaleCounterType("cpu"); public static final AutoScaleCounterType AutoScaleCounterMemory = new AutoScaleCounterType("memory"); @@ -159,6 +162,9 @@ public class VirtualRouterElement extends AdapterBase implements VirtualRouterEl @Inject NetworkTopologyContext networkTopologyContext; + @Inject + NetworkDetailsDao _networkDetailsDao; + @Inject protected RouterDeploymentDefinitionBuilder routerDeploymentDefinitionBuilder; @@ -262,7 +268,7 @@ public boolean prepare(final Network network, final NicProfile nic, final Virtua public boolean applyFWRules(final Network network, final List rules) throws ResourceUnavailableException { boolean result = true; if (canHandle(network, Service.Firewall)) { - final List routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); + final List routers = getRouters(network); if (routers == null || routers.isEmpty()) { s_logger.debug("Virtual router elemnt doesn't need to apply firewall rules on the backend; virtual " + "router doesn't exist in the network " + network.getId()); return true; @@ -407,7 +413,7 @@ public boolean applyLBRules(final Network network, final List return false; } - final List routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); + final List routers = getRouters(network); if (routers == null || routers.isEmpty()) { s_logger.debug("Virtual router elemnt doesn't need to apply lb rules on the backend; virtual " + "router doesn't exist in the network " + network.getId()); return true; @@ -498,7 +504,7 @@ public boolean applyIps(final Network network, final List routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); + final List routers = getRouters(network); if (routers == null || routers.isEmpty()) { s_logger.debug("Virtual router elemnt doesn't need to associate ip addresses on the backend; virtual " + "router doesn't exist in the network " + network.getId()); return true; @@ -657,7 +663,7 @@ private static Map> setCapabilities() { public boolean applyStaticNats(final Network network, final List rules) throws ResourceUnavailableException { boolean result = true; if (canHandle(network, Service.StaticNat)) { - final List routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); + final List routers = getRouters(network); if (routers == null || routers.isEmpty()) { s_logger.debug("Virtual router elemnt doesn't need to apply static nat on the backend; virtual " + "router doesn't exist in the network " + network.getId()); return true; @@ -673,6 +679,46 @@ public boolean applyStaticNats(final Network network, final List getRouters(Network network){ + List routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); + if (routers !=null && routers.isEmpty()) { + return null; + } + NetworkDetailVO updateInSequence=_networkDetailsDao.findDetail(network.getId(), Network.updatingInSequence); + if(network.isRedundant() && updateInSequence!=null && "true".equalsIgnoreCase(updateInSequence.getValue())){ + List masterRouters=new ArrayList(); + int noOfrouters=routers.size(); + while (noOfrouters>0){ + DomainRouterVO router = routers.get(0); + if(router.getUpdateState()== VirtualRouter.UpdateState.UPDATE_IN_PROGRESS){ + ArrayList routerList = new ArrayList(); + routerList.add(router); + return routerList; + } + if(router.getUpdateState()== VirtualRouter.UpdateState.UPDATE_COMPLETE) { + routers.remove(router); + noOfrouters--; + continue; + } + if(router.getRedundantState()!=VirtualRouter.RedundantState.BACKUP) { + masterRouters.add(router); + routers.remove(router); + } + noOfrouters--; + } + if(routers.size()==0 && masterRouters.size()==0){ + return null; + } + if(routers.size()==0 && masterRouters.size()!=0){ + routers=masterRouters; + } + routers=routers.subList(0,1); + routers.get(0).setUpdateState(VirtualRouter.UpdateState.UPDATE_IN_PROGRESS); + _routerDao.persist(routers.get(0)); + } + return routers; + } + @Override public boolean shutdown(final Network network, final ReservationContext context, final boolean cleanup) throws ConcurrentOperationException, ResourceUnavailableException { final List routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); @@ -1031,7 +1077,7 @@ protected List getRouters(final Network network, final DeployDes List routers; if (publicNetwork) { - routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); + routers = getRouters(network); } else { if (isPodBased && dest.getPod() != null) { final Long podId = dest.getPod().getId(); @@ -1228,7 +1274,27 @@ public boolean completeAggregatedExecution(final Network network, final DeployDe throw new ResourceUnavailableException("Can't find at least one router!", DataCenter.class, network.getDataCenterId()); } - return _routerMgr.completeAggregatedExecution(network, routers); + NetworkDetailVO networkDetail=_networkDetailsDao.findDetail(network.getId(), Network.updatingInSequence); + boolean updateInSequence= "true".equalsIgnoreCase((networkDetail!=null ? networkDetail.getValue() : null)); + if(updateInSequence){ + DomainRouterVO router=routers.get(0); + router.setUpdateState(VirtualRouter.UpdateState.UPDATE_COMPLETE); + _routerDao.persist(router); + } + boolean result=false; + try{ + result=_routerMgr.completeAggregatedExecution(network, routers); + } finally { + if(!result && updateInSequence) { + //fail the network update. even if one router fails we fail the network update. + List routerList = _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER); + for (DomainRouterVO router : routerList) { + router.setUpdateState(VirtualRouter.UpdateState.UPDATE_FAILED); + _routerDao.persist(router); + } + } + } + return result; } @Override @@ -1237,4 +1303,22 @@ public boolean cleanupAggregatedExecution(final Network network, final DeployDes // lets not waste another command return true; } + + @Override + public void configureResource(Network network) { + NetworkDetailVO networkDetail=_networkDetailsDao.findDetail(network.getId(), Network.updatingInSequence); + if(networkDetail==null || !"true".equalsIgnoreCase(networkDetail.getValue())) + throw new CloudRuntimeException("failed to configure the resource, network update is not in progress."); + Listrouters = _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER); + for(DomainRouterVO router : routers){ + router.setUpdateState(VirtualRouter.UpdateState.UPDATE_NEEDED); + _routerDao.persist(router); + } + } + + @Override + public int getResourceCount(Network network) { + return _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER).size(); + } + } diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index f7947d5c54b2..68899e947d46 100644 --- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -2282,6 +2282,11 @@ public VirtualRouter startRouter(final long routerId, final boolean reprogramNet // verify parameters DomainRouterVO router = _routerDao.findById(routerId); + //clean up the update_state feild + if(router.getUpdateState()== VirtualRouter.UpdateState.UPDATE_FAILED){ + router.setUpdateState(null); + _routerDao.update(router.getId(),router); + } if (router == null) { throw new InvalidParameterValueException("Unable to find router by id " + routerId + "."); } diff --git a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java index 2d04a7e633f1..16c947231aa8 100644 --- a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java +++ b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java @@ -20,6 +20,9 @@ import java.util.List; import java.util.Map; +import com.cloud.network.dao.NetworkDetailVO; +import com.cloud.network.dao.NetworkDetailsDao; +import com.cloud.network.router.VirtualRouter; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.log4j.Logger; @@ -106,6 +109,7 @@ public class RouterDeploymentDefinition { protected Long tableLockId; protected boolean isPublicNetwork; protected PublicIp sourceNatIp; + protected NetworkDetailsDao networkDetailsDao; protected RouterDeploymentDefinition(final Network guestNetwork, final DeployDestination dest, final Account owner, final Map params) { @@ -393,7 +397,13 @@ protected void deployAllVirtualRouters() throws ConcurrentOperationException, In // Don't start the router as we are holding the network lock that // needs to be released at the end of router allocation final DomainRouterVO router = nwHelper.deployRouter(this, false); - + //check if the network update is in progress. + //if update is in progress add the update_pending flag to DomainRouterVO. + NetworkDetailVO detail =networkDetailsDao.findDetail(guestNetwork.getId(),Network.updatingInSequence); + if("true".equalsIgnoreCase(detail!=null ? detail.getValue() : null)) { + router.setUpdateState(VirtualRouter.UpdateState.UPDATE_IN_PROGRESS); + routerDao.persist(router); + } if (router != null) { routerDao.addRouterToGuestNetwork(router, guestNetwork); //Fix according to changes by Sheng Yang in commit ID cb4513379996b262ae378daf00c6388c6b7313cf diff --git a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java index 3ba4fad77de8..3765537a1481 100644 --- a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java +++ b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java @@ -22,6 +22,7 @@ import javax.inject.Inject; +import com.cloud.network.dao.NetworkDetailsDao; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; @@ -96,6 +97,8 @@ public class RouterDeploymentDefinitionBuilder { private VpcManager vpcMgr; @Inject private VlanDao vlanDao; + @Inject + private NetworkDetailsDao networkDetailsDao; @Autowired @Qualifier("networkHelper") @@ -133,6 +136,7 @@ protected RouterDeploymentDefinition injectDependencies( routerDeploymentDefinition.ipv6Dao = ipv6Dao; routerDeploymentDefinition.ipAddressDao = ipAddressDao; routerDeploymentDefinition.serviceOfferingId = offeringId; + routerDeploymentDefinition.networkDetailsDao = networkDetailsDao; routerDeploymentDefinition.nwHelper = nwHelper; diff --git a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java index a5d8a1ab584a..3e80865572b6 100644 --- a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java @@ -247,7 +247,7 @@ public IpAddress getIp(long id) { */ @Override public Network updateGuestNetwork(long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, - Long networkOfferingId, Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String newUUID) { + Long networkOfferingId, Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String newUUID,boolean updateInSequence) { // TODO Auto-generated method stub return null; } @@ -841,6 +841,21 @@ public void prepareAllNicsForMigration(VirtualMachineProfile vm, DeployDestinati return; } + @Override + public boolean canUpdateInSequence(Network network) { + return false; + } + + @Override + public void configureUpdateInSequence(Network network) { + return; + } + + @Override + public int getResourceCount(Network network) { + return 0; + } + @Override public void prepareNicForMigration(VirtualMachineProfile vm, DeployDestination dest) { // TODO Auto-generated method stub diff --git a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java index 1570a2e15a94..07d11e1fceec 100644 --- a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java +++ b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java @@ -684,7 +684,7 @@ public void testDeployAllVirtualRouters() final DomainRouterVO routerVO2 = mock(DomainRouterVO.class); when(mockNetworkHelper.deployRouter(deploymentUT, false)) .thenReturn(routerVO1).thenReturn(routerVO2); - + when(networkDetailsDao.findById(anyLong())).thenReturn(null); // Execute deploymentUT.deployAllVirtualRouters(); diff --git a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java index 4225083ca2bd..626c2d7acc67 100644 --- a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java +++ b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; +import com.cloud.network.dao.NetworkDetailsDao; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -79,6 +80,8 @@ public class RouterDeploymentDefinitionTestBase { @Mock protected NetworkHelper mockNetworkHelper; @Mock + protected NetworkDetailsDao networkDetailsDao; + @Mock protected VpcNetworkHelperImpl vpcNwHelper; @Mock protected VMInstanceDao mockVmDao; diff --git a/setup/db/db/schema-452to460.sql b/setup/db/db/schema-452to460.sql index dfb629f95a0f..e05ad6d255db 100644 --- a/setup/db/db/schema-452to460.sql +++ b/setup/db/db/schema-452to460.sql @@ -420,3 +420,5 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(),'KVM', 'default', 'CentOS 7', 246, utc_timestamp(), 0); UPDATE `cloud`.`hypervisor_capabilities` SET `max_data_volumes_limit` = '32' WHERE `hypervisor_capabilities`.`hypervisor_type` = 'KVM'; +ALTER TABLE `cloud`.`domain_router` ADD COLUMN update_state varchar(64) DEFAULT NULL; + From 466c8f1a12d39c273f02f64d22cb9a3a8a4937f3 Mon Sep 17 00:00:00 2001 From: Bharat Kumar Date: Thu, 20 Aug 2015 17:21:20 +0530 Subject: [PATCH 02/10] CLOUDSTACK-8751 Added tests --- .../element/VirtualRouterElementTest.java | 151 +++++++++++++++++- 1 file changed, 150 insertions(+), 1 deletion(-) diff --git a/server/test/com/cloud/network/element/VirtualRouterElementTest.java b/server/test/com/cloud/network/element/VirtualRouterElementTest.java index 659277824ea3..4fbc28ee4cb8 100644 --- a/server/test/com/cloud/network/element/VirtualRouterElementTest.java +++ b/server/test/com/cloud/network/element/VirtualRouterElementTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyList; import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; @@ -26,6 +27,11 @@ import java.util.ArrayList; import java.util.List; +import com.cloud.exception.AgentUnavailableException; +import com.cloud.network.dao.NetworkDetailVO; +import com.cloud.network.dao.NetworkDetailsDao; +import com.cloud.network.router.VirtualRouter; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder; @@ -35,6 +41,8 @@ import org.mockito.InjectMocks; import org.mockito.Matchers; import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; import com.cloud.cluster.dao.ManagementServerHostDao; @@ -111,6 +119,7 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; +import org.mockito.stubbing.Answer; @RunWith(MockitoJUnitRunner.class) public class VirtualRouterElementTest { @@ -127,6 +136,7 @@ public class VirtualRouterElementTest { @Mock private ManagementServerHostDao _msHostDao; @Mock private NetworkDao _networkDao; @Mock private NetworkOfferingDao _networkOfferingDao; + @Mock private NetworkDetailsDao _networkDetailsDao; @Mock private NicDao _nicDao; @Mock private NicIpAliasDao _nicIpAliasDao; @Mock private OpRouterMonitorServiceDao _opRouterMonitorServiceDao; @@ -225,6 +235,62 @@ public void testPrepare() { } + @Test + public void testGetRouters1(){ + Network networkUpdateInprogress=new NetworkVO(1l,null,null,null,1l,1l,1l,1l,"d","d","d",null,1l,1l,null,true,null,true); + mockDAOs((NetworkVO)networkUpdateInprogress,testOffering); + //getRoutes should always return the router that is updating. + List routers=virtualRouterElement.getRouters(networkUpdateInprogress); + assertTrue(routers.size()==1); + assertTrue(routers.get(0).getUpdateState()== VirtualRouter.UpdateState.UPDATE_IN_PROGRESS); + } + + @Test + public void testGetRouters2(){ + Network networkUpdateInprogress=new NetworkVO(2l,null,null,null,1l,1l,1l,1l,"d","d","d",null,1l,1l,null,true,null,true); + mockDAOs((NetworkVO)networkUpdateInprogress,testOffering); + //alwyas return backup routers first when both master and backup need update. + List routers=virtualRouterElement.getRouters(networkUpdateInprogress); + assertTrue(routers.size()==1); + assertTrue(routers.get(0).getRedundantState()==RedundantState.BACKUP && routers.get(0).getUpdateState()==VirtualRouter.UpdateState.UPDATE_IN_PROGRESS); + } + + @Test + public void testGetRouters3(){ + Network network=new NetworkVO(3l,null,null,null,1l,1l,1l,1l,"d","d","d",null,1l,1l,null,true,null,true); + mockDAOs((NetworkVO)network,testOffering); + //alwyas return backup routers first when both master and backup need update. + List routers=virtualRouterElement.getRouters(network); + assertTrue(routers.size()==4); + } + + @Test + public void getResourceCountTest(){ + Network network=new NetworkVO(3l,null,null,null,1l,1l,1l,1l,"d","d","d",null,1l,1l,null,true,null,true); + mockDAOs((NetworkVO)network,testOffering); + int routers=virtualRouterElement.getResourceCount(network); + assertTrue(routers==4); + } + + @Test + public void completeAggregationCommandTest1() throws AgentUnavailableException,ResourceUnavailableException { + virtualRouterElement._routerMgr = Mockito.mock(VpcVirtualNetworkApplianceManagerImpl.class); + virtualRouterElement.routerDeploymentDefinitionBuilder = routerDeploymentDefinitionBuilder; + Network network = new NetworkVO(6l, null, null, null, 1l, 1l, 1l, 1l, "d", "d", "d", null, 1l, 1l, null, true, null, true); + when(virtualRouterElement._routerMgr.completeAggregatedExecution(any(Network.class), anyList())).thenReturn(true); + mockDAOs((NetworkVO) network, testOffering); + when(virtualRouterElement._routerDao.persist(any(DomainRouterVO.class))).thenAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocationOnMock) throws Throwable { + Object args[] = invocationOnMock.getArguments(); + DomainRouterVO router = (DomainRouterVO) args[0]; + if (router.getUpdateState() != VirtualRouter.UpdateState.UPDATE_COMPLETE) { + throw new CloudRuntimeException("TestFailed: completeAggregationCommandTest1 failed"); + } else return null; + } + }); + virtualRouterElement.completeAggregatedExecution(network, testDestination); + } /** * @param networks * @param offerings @@ -293,11 +359,94 @@ private void mockDAOs(final NetworkVO network, final NetworkOfferingVO offering) /* haEnabled */ false, /* stopPending */ false, /* vpcId */ null); - + final DomainRouterVO routerNeedUpdateBackup = new DomainRouterVO(/* id */ 2L, + /* serviceOfferingId */ 1L, + /* elementId */ 0L, + "name", + /* templateId */0L, + HypervisorType.XenServer, + /* guestOSId */ 0L, + /* domainId */ 0L, + /* accountId */ 1L, + /* userId */ 1L, + /* isRedundantRouter */ false, + RedundantState.BACKUP, + /* haEnabled */ false, + /* stopPending */ false, + /* vpcId */ null); + routerNeedUpdateBackup.setUpdateState(VirtualRouter.UpdateState.UPDATE_NEEDED); + final DomainRouterVO routerNeedUpdateMaster = new DomainRouterVO(/* id */ 3L, + /* serviceOfferingId */ 1L, + /* elementId */ 0L, + "name", + /* templateId */0L, + HypervisorType.XenServer, + /* guestOSId */ 0L, + /* domainId */ 0L, + /* accountId */ 1L, + /* userId */ 1L, + /* isRedundantRouter */ false, + RedundantState.MASTER, + /* haEnabled */ false, + /* stopPending */ false, + /* vpcId */ null); + routerNeedUpdateMaster.setUpdateState(VirtualRouter.UpdateState.UPDATE_NEEDED); + final DomainRouterVO routerUpdateComplete = new DomainRouterVO(/* id */ 4L, + /* serviceOfferingId */ 1L, + /* elementId */ 0L, + "name", + /* templateId */0L, + HypervisorType.XenServer, + /* guestOSId */ 0L, + /* domainId */ 0L, + /* accountId */ 1L, + /* userId */ 1L, + /* isRedundantRouter */ false, + RedundantState.UNKNOWN, + /* haEnabled */ false, + /* stopPending */ false, + /* vpcId */ null); + routerUpdateComplete.setUpdateState(VirtualRouter.UpdateState.UPDATE_COMPLETE); + final DomainRouterVO routerUpdateInProgress = new DomainRouterVO(/* id */ 5L, + /* serviceOfferingId */ 1L, + /* elementId */ 0L, + "name", + /* templateId */0L, + HypervisorType.XenServer, + /* guestOSId */ 0L, + /* domainId */ 0L, + /* accountId */ 1L, + /* userId */ 1L, + /* isRedundantRouter */ false, + RedundantState.UNKNOWN, + /* haEnabled */ false, + /* stopPending */ false, + /* vpcId */ null); + routerUpdateInProgress.setUpdateState(VirtualRouter.UpdateState.UPDATE_IN_PROGRESS); + List routerList1=new ArrayList<>(); + routerList1.add(routerUpdateComplete); + routerList1.add(routerNeedUpdateBackup); + routerList1.add(routerNeedUpdateMaster); + routerList1.add(routerUpdateInProgress); + List routerList2=new ArrayList<>(); + routerList2.add(routerUpdateComplete); + routerList2.add(routerNeedUpdateBackup); + routerList2.add(routerNeedUpdateMaster); + List routerList3=new ArrayList<>(); + routerList3.add(routerUpdateComplete); + routerList3.add(routerUpdateInProgress); when(_routerDao.getNextInSequence(Long.class, "id")).thenReturn(1L); when(_templateDao.findRoutingTemplate(HypervisorType.XenServer, "SystemVM Template (XenServer)")).thenReturn(new VMTemplateVO()); when(_routerDao.persist(any(DomainRouterVO.class))).thenReturn(router); when(_routerDao.findById(router.getId())).thenReturn(router); + when(_routerDao.listByNetworkAndRole(1l, VirtualRouter.Role.VIRTUAL_ROUTER)).thenReturn(routerList1); + when(_routerDao.listByNetworkAndRole(2l, VirtualRouter.Role.VIRTUAL_ROUTER)).thenReturn(routerList2); + when(_routerDao.listByNetworkAndRole(3l, VirtualRouter.Role.VIRTUAL_ROUTER)).thenReturn(routerList1); + when(_routerDao.listByNetworkAndRole(6l, VirtualRouter.Role.VIRTUAL_ROUTER)).thenReturn(routerList3); + when(_networkDetailsDao.findDetail(1l, Network.updatingInSequence)).thenReturn(new NetworkDetailVO(1l,Network.updatingInSequence,"true",true)); + when(_networkDetailsDao.findDetail(2l, Network.updatingInSequence)).thenReturn(new NetworkDetailVO(2l,Network.updatingInSequence,"true",true)); + when(_networkDetailsDao.findDetail(6l, Network.updatingInSequence)).thenReturn(new NetworkDetailVO(2l,Network.updatingInSequence,"true",true)); + when(_routerDao.persist(any(DomainRouterVO.class))).thenReturn(router); } } From 067fabd758babd3d0c6d605e0adc7015f3139fb6 Mon Sep 17 00:00:00 2001 From: Bharat Kumar Date: Thu, 26 Nov 2015 15:30:06 +0530 Subject: [PATCH 03/10] Do not update network if one of the router's state is unknown Added checks to prevent netwrok update when router state is unknown or when the new offering removes a service that is in use. Added a new param forced to the updateNetwork API. The network will undergo a forced update when this param is set to true. CLOUDSTACK-8751 Clean network config like firewall rules etc, when network services are removed during network update. --- api/src/com/cloud/network/NetworkService.java | 2 +- .../network/vpn/RemoteAccessVpnService.java | 2 +- .../network/UpdateNetworkCmdByAdmin.java | 2 +- .../user/network/UpdateNetworkCmd.java | 11 +- .../user/vpn/DeleteRemoteAccessVpnCmd.java | 2 +- .../service/NetworkOrchestrationService.java | 4 + .../orchestration/NetworkOrchestrator.java | 113 ++++++++++++++++++ .../cloud/network/IpAddressManagerImpl.java | 2 +- .../com/cloud/network/NetworkServiceImpl.java | 32 ++++- ...VpcVirtualNetworkApplianceManagerImpl.java | 10 ++ .../vpn/RemoteAccessVpnManagerImpl.java | 6 +- .../com/cloud/user/AccountManagerImpl.java | 2 +- .../com/cloud/vpc/MockNetworkManagerImpl.java | 12 +- 13 files changed, 186 insertions(+), 14 deletions(-) diff --git a/api/src/com/cloud/network/NetworkService.java b/api/src/com/cloud/network/NetworkService.java index e26db340ff4f..7a8a94987807 100644 --- a/api/src/com/cloud/network/NetworkService.java +++ b/api/src/com/cloud/network/NetworkService.java @@ -77,7 +77,7 @@ IpAddress allocatePortableIP(Account ipOwner, int regionId, Long zoneId, Long ne IpAddress getIp(long id); Network updateGuestNetwork(long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, Long networkOfferingId, - Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String newUUID, boolean updateInSequence); + Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String newUUID, boolean updateInSequence, boolean forced); PhysicalNetwork createPhysicalNetwork(Long zoneId, String vnetRange, String networkSpeed, List isolationMethods, String broadcastDomainRange, Long domainId, List tags, String name); diff --git a/api/src/com/cloud/network/vpn/RemoteAccessVpnService.java b/api/src/com/cloud/network/vpn/RemoteAccessVpnService.java index decf8c437330..d089b8524497 100644 --- a/api/src/com/cloud/network/vpn/RemoteAccessVpnService.java +++ b/api/src/com/cloud/network/vpn/RemoteAccessVpnService.java @@ -33,7 +33,7 @@ public interface RemoteAccessVpnService { RemoteAccessVpn createRemoteAccessVpn(long vpnServerAddressId, String ipRange, boolean openFirewall, Boolean forDisplay) throws NetworkRuleConflictException; - boolean destroyRemoteAccessVpnForIp(long ipId, Account caller) throws ResourceUnavailableException; + boolean destroyRemoteAccessVpnForIp(long ipId, Account caller, boolean forceCleanup) throws ResourceUnavailableException; RemoteAccessVpn startRemoteAccessVpn(long vpnServerAddressId, boolean openFirewall) throws ResourceUnavailableException; diff --git a/api/src/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java b/api/src/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java index f2c5119466cc..388348c592c1 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java +++ b/api/src/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java @@ -49,7 +49,7 @@ public void execute() throws InsufficientCapacityException, ConcurrentOperationE } Network result = _networkService.updateGuestNetwork(getId(), getNetworkName(), getDisplayText(), callerAccount, - callerUser, getNetworkDomain(), getNetworkOfferingId(), getChangeCidr(), getGuestVmCidr(), getDisplayNetwork(), getCustomId(), getUpdateInSequence()); + callerUser, getNetworkDomain(), getNetworkOfferingId(), getChangeCidr(), getGuestVmCidr(), getDisplayNetwork(), getCustomId(), getUpdateInSequence(),getForced()); if (result != null) { diff --git a/api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java b/api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java index 8ef9251e0475..c313f369b0e2 100644 --- a/api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java @@ -83,6 +83,9 @@ public class UpdateNetworkCmd extends BaseAsyncCustomIdCmd { description = "an optional field, whether to the display the network to the end user or not.", authorized = {RoleType.Admin}) private Boolean displayNetwork; + @Parameter(name= ApiConstants.FORCED, type = CommandType.BOOLEAN, description = "Setting this to true will cause a forced network update,", authorized = {RoleType.Admin}) + private Boolean forced; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -129,6 +132,12 @@ public Boolean getUpdateInSequence(){ return updateInSequence; } + public boolean getForced(){ + if(forced==null){ + return false; + } + return forced; + } ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -159,7 +168,7 @@ public void execute() throws InsufficientCapacityException, ConcurrentOperationE Network result = _networkService.updateGuestNetwork(getId(), getNetworkName(), getDisplayText(), callerAccount, callerUser, getNetworkDomain(), getNetworkOfferingId(), - getChangeCidr(), getGuestVmCidr(), getDisplayNetwork(), getCustomId(), getUpdateInSequence()); + getChangeCidr(), getGuestVmCidr(), getDisplayNetwork(), getCustomId(), getUpdateInSequence(), getForced()); if (result != null) { NetworkResponse response = _responseGenerator.createNetworkResponse(ResponseView.Restricted, result); diff --git a/api/src/org/apache/cloudstack/api/command/user/vpn/DeleteRemoteAccessVpnCmd.java b/api/src/org/apache/cloudstack/api/command/user/vpn/DeleteRemoteAccessVpnCmd.java index 37b7b5aaa3e7..12ab531375f1 100644 --- a/api/src/org/apache/cloudstack/api/command/user/vpn/DeleteRemoteAccessVpnCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/vpn/DeleteRemoteAccessVpnCmd.java @@ -93,7 +93,7 @@ public String getEventType() { @Override public void execute() throws ResourceUnavailableException { - if (! _ravService.destroyRemoteAccessVpnForIp(publicIpId, CallContext.current().getCallingAccount())) { + if (! _ravService.destroyRemoteAccessVpnForIp(publicIpId, CallContext.current().getCallingAccount(), false)) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete remote access vpn"); } } diff --git a/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java b/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java index 1e2761f274a8..89bec1783f61 100644 --- a/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java +++ b/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java @@ -227,6 +227,10 @@ void implementNetworkElementsAndResources(DeployDestination dest, ReservationCon boolean canUpdateInSequence(Network network); + List getServicesNotSupportedInNewOffering(Network network, long newNetworkOfferingId); + + void cleanupConfigForServicesInNetwork(List services, Network network); + void configureUpdateInSequence(Network network); int getResourceCount(Network network); diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 420d1ca32334..966975b424d8 100644 --- a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -39,6 +39,9 @@ import com.cloud.network.Networks; import com.cloud.network.dao.NetworkDetailsDao; +import com.cloud.network.dao.RemoteAccessVpnDao; +import com.cloud.network.dao.RemoteAccessVpnVO; +import com.cloud.network.dao.VpnUserDao; import com.cloud.network.element.RedundantResource; import com.cloud.vm.dao.DomainRouterDao; import org.apache.log4j.Logger; @@ -271,6 +274,10 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra VMNetworkMapDao _vmNetworkMapDao; @Inject DomainRouterDao _rotuerDao; + @Inject + RemoteAccessVpnDao _remoteAccessVpnDao; + @Inject + VpnUserDao _vpnUserDao; List networkGurus; @@ -1283,6 +1290,7 @@ protected boolean prepareElement(final NetworkElement element, final Network net @Override public boolean canUpdateInSequence(Network network){ List providers = getNetworkProviders(network.getId()); + //check if the there are no service provider other than virtualrouter. for(Provider provider :providers){ if(provider!=Provider.VirtualRouter) @@ -1291,6 +1299,111 @@ public boolean canUpdateInSequence(Network network){ return true; } + @Override + public List getServicesNotSupportedInNewOffering(Network network,long newNetworkOfferingId){ + NetworkOffering offering =_networkOfferingDao.findById(newNetworkOfferingId); + List services=_ntwkOfferingSrvcDao.listServicesForNetworkOffering(offering.getId()); + List serviceMap= _ntwkSrvcDao.getServicesInNetwork(network.getId()); + List servicesNotInNewOffering=new ArrayList<>(); + for(NetworkServiceMapVO serviceVO :serviceMap){ + boolean inlist=false; + for(String service: services){ + if(serviceVO.getService().equalsIgnoreCase(service)){ + inlist=true; + break; + } + } + if(!inlist){ + //ignore Gateway service as this has no effect on the + //behaviour of network. + if(!serviceVO.getService().equalsIgnoreCase(Service.Gateway.getName())) + servicesNotInNewOffering.add(serviceVO.getService()); + } + } + return servicesNotInNewOffering; + } + + @Override + public void cleanupConfigForServicesInNetwork(List services, final Network network){ + long networkId=network.getId(); + Account caller=_accountDao.findById(Account.ACCOUNT_ID_SYSTEM); + long userId=User.UID_SYSTEM; + //remove all PF/Static Nat rules for the network + s_logger.info("Services:"+services+" are no longer supported in network:"+network.getUuid()+ + " after applying new network offering:"+network.getNetworkOfferingId()+" removing the related configuration"); + if(services.contains(Service.StaticNat.getName())|| services.contains(Service.PortForwarding.getName())) { + try { + if (_rulesMgr.revokeAllPFStaticNatRulesForNetwork(networkId, userId, caller)) { + s_logger.debug("Successfully cleaned up portForwarding/staticNat rules for network id=" + networkId); + } else { + s_logger.warn("Failed to release portForwarding/StaticNat rules as a part of network id=" + networkId + " cleanup"); + } + if(services.contains(Service.StaticNat.getName())){ + //removing static nat configured on ips. + //optimizing the db operations using transaction. + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + List ips = _ipAddressDao.listStaticNatPublicIps(network.getId()); + for (IPAddressVO ip : ips) { + ip.setOneToOneNat(false); + ip.setAssociatedWithVmId(null); + ip.setVmIp(null); + _ipAddressDao.update(ip.getId(),ip); + } + } + }); + } + } catch (ResourceUnavailableException ex) { + s_logger.warn("Failed to release portForwarding/StaticNat rules as a part of network id=" + networkId + " cleanup due to resourceUnavailable ", ex); + } + } + if(services.contains(Service.SourceNat.getName())){ + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + List ips = _ipAddressDao.listByAssociatedNetwork(network.getId(),true); + //removing static nat configured on ips. + for (IPAddressVO ip : ips) { + ip.setSourceNat(false); + _ipAddressDao.update(ip.getId(),ip); + } + } + }); + } + if(services.contains(Service.Lb.getName())){ + //remove all LB rules for the network + if (_lbMgr.removeAllLoadBalanacersForNetwork(networkId, caller, userId)) { + s_logger.debug("Successfully cleaned up load balancing rules for network id=" + networkId); + } else { + s_logger.warn("Failed to cleanup LB rules as a part of network id=" + networkId + " cleanup"); + } + } + + if(services.contains(Service.Firewall.getName())){ + //revoke all firewall rules for the network + try { + if (_firewallMgr.revokeAllFirewallRulesForNetwork(networkId, userId, caller)) { + s_logger.debug("Successfully cleaned up firewallRules rules for network id=" + networkId); + } else { + s_logger.warn("Failed to cleanup Firewall rules as a part of network id=" + networkId + " cleanup"); + } + } catch (ResourceUnavailableException ex) { + s_logger.warn("Failed to cleanup Firewall rules as a part of network id=" + networkId + " cleanup due to resourceUnavailable ", ex); + } + } + + //do not remove vpn service for vpc networks. + if(services.contains(Service.Vpn.getName()) && network.getVpcId()==null){ + RemoteAccessVpnVO vpn = _remoteAccessVpnDao.findByAccountAndNetwork(network.getAccountId(),networkId); + try { + _vpnMgr.destroyRemoteAccessVpnForIp(vpn.getServerAddressId(), caller, true); + } catch (ResourceUnavailableException ex) { + s_logger.warn("Failed to cleanup remote access vpn resources of network:"+network.getUuid() + " due to Exception: ", ex); + } + } + } + @Override public void configureUpdateInSequence(Network network) { List providers = getNetworkProviders(network.getId()); diff --git a/server/src/com/cloud/network/IpAddressManagerImpl.java b/server/src/com/cloud/network/IpAddressManagerImpl.java index bd59c66df3ab..3745433289e5 100644 --- a/server/src/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/com/cloud/network/IpAddressManagerImpl.java @@ -562,7 +562,7 @@ protected boolean cleanupIpResources(long ipId, long userId, Account caller) { // the code would be triggered s_logger.debug("Cleaning up remote access vpns as a part of public IP id=" + ipId + " release..."); try { - _vpnMgr.destroyRemoteAccessVpnForIp(ipId, caller); + _vpnMgr.destroyRemoteAccessVpnForIp(ipId, caller,false); } catch (ResourceUnavailableException e) { s_logger.warn("Unable to destroy remote access vpn for ip id=" + ipId + " as a part of ip release", e); success = false; diff --git a/server/src/com/cloud/network/NetworkServiceImpl.java b/server/src/com/cloud/network/NetworkServiceImpl.java index bb573decde28..cade54f088c4 100644 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -39,6 +39,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.network.router.VirtualRouter; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.ApiConstants; @@ -2002,7 +2003,7 @@ private boolean checkForNonStoppedVmInNetwork(long networkId) { @DB @ActionEvent(eventType = EventTypes.EVENT_NETWORK_UPDATE, eventDescription = "updating network", async = true) public Network updateGuestNetwork(final long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, - final Long networkOfferingId, Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String customId, boolean updateInSequence) { + final Long networkOfferingId, Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String customId, boolean updateInSequence, boolean forced) { boolean restartNetwork = false; // verify input parameters @@ -2248,14 +2249,39 @@ public Network updateGuestNetwork(final long networkId, String name, String disp ReservationContext context = new ReservationContextImpl(null, null, callerUser, callerAccount); // 1) Shutdown all the elements and cleanup all the rules. Don't allow to shutdown network in intermediate // states - Shutdown and Implementing - List routers=null; int resourceCount=1; - if(updateInSequence && restartNetwork && _networkOfferingDao.findById(network.getNetworkOfferingId()).getRedundantRouter() && networkOfferingId!=null && _networkOfferingDao.findById(networkOfferingId).getRedundantRouter() && network.getVpcId()==null) { + if(updateInSequence && restartNetwork && _networkOfferingDao.findById(network.getNetworkOfferingId()).getRedundantRouter() + && (networkOfferingId==null || _networkOfferingDao.findById(networkOfferingId).getRedundantRouter()) && network.getVpcId()==null) { _networkMgr.canUpdateInSequence(network); NetworkDetailVO networkDetail =new NetworkDetailVO(network.getId(),Network.updatingInSequence,"true",true); _networkDetailsDao.persist(networkDetail); _networkMgr.configureUpdateInSequence(network); resourceCount=_networkMgr.getResourceCount(network); + //check if routers are in correct state before proceeding with the update + List routers=_routerDao.listByNetworkAndRole(networkId, VirtualRouter.Role.VIRTUAL_ROUTER); + for(DomainRouterVO router :routers){ + if(router.getRedundantState()== VirtualRouter.RedundantState.UNKNOWN){ + if(!forced){ + throw new CloudRuntimeException("Domain router: "+router.getInstanceName()+" is in unknown state, Cannot update network. set parameter forced to true for forcing an update"); + } + } + } + } + List servicesNotInNewOffering = null; + if(networkOfferingId != null) + servicesNotInNewOffering = _networkMgr.getServicesNotSupportedInNewOffering(network,networkOfferingId); + if(!forced && servicesNotInNewOffering != null && !servicesNotInNewOffering.isEmpty()){ + NetworkOfferingVO newOffering = _networkOfferingDao.findById(networkOfferingId); + throw new CloudRuntimeException("The new offering:"+newOffering.getUniqueName() + +" will remove the following services "+servicesNotInNewOffering +"along with all the related configuration currently in use. will not proceed with the network update." + + "set forced parameter to true for forcing an update."); + } + try{ + if(servicesNotInNewOffering!=null && !servicesNotInNewOffering.isEmpty()){ + _networkMgr.cleanupConfigForServicesInNetwork(servicesNotInNewOffering,network); + } + }catch (Throwable e){ + s_logger.debug("failed to cleanup config related to unused services error:"+e.getMessage()); } boolean validStateToShutdown = (network.getState() == Network.State.Implemented || network.getState() == Network.State.Setup || network.getState() == Network.State.Allocated); diff --git a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java index c2d923cb9512..7b82125dccfb 100644 --- a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java @@ -696,6 +696,16 @@ public List getVpcRouters(final long vpcId) { return _routerDao.listByVpcId(vpcId); } + @Override + public boolean start() { + return true; + } + + @Override + public boolean stop() { + return true; + } + @Override public boolean startRemoteAccessVpn(final RemoteAccessVpn vpn, final VirtualRouter router) throws ResourceUnavailableException { if (router.getState() != State.Running) { diff --git a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java index b473f050e040..065c097f0f8b 100644 --- a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java +++ b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java @@ -281,7 +281,7 @@ private void validateRemoteAccessVpnConfiguration() throws ConfigurationExceptio @Override @DB @ActionEvent(eventType = EventTypes.EVENT_REMOTE_ACCESS_VPN_DESTROY, eventDescription = "removing remote access vpn", async = true) - public boolean destroyRemoteAccessVpnForIp(long ipId, Account caller) throws ResourceUnavailableException { + public boolean destroyRemoteAccessVpnForIp(long ipId, Account caller, final boolean forceCleanup) throws ResourceUnavailableException { final RemoteAccessVpnVO vpn = _remoteAccessVpnDao.findByPublicIpAddress(ipId); if (vpn == null) { s_logger.debug("there are no Remote access vpns for public ip address id=" + ipId); @@ -309,7 +309,7 @@ public boolean destroyRemoteAccessVpnForIp(long ipId, Account caller) throws Res RemoteAccessVpn.State.Running); success = false; } finally { - if (success) { + if (success|| forceCleanup) { //Cleanup corresponding ports final List vpnFwRules = _rulesDao.listByIpAndPurpose(ipId, Purpose.Vpn); @@ -339,7 +339,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) { success = _firewallMgr.applyIngressFirewallRules(ipId, caller); } - if (success) { + if (success|| forceCleanup) { try { Transaction.execute(new TransactionCallbackNoReturn() { @Override diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index d3eb40c7db69..b0d7d4b0ad87 100644 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -786,7 +786,7 @@ protected boolean cleanupAccount(AccountVO account, long callerUserId, Account c try { for (RemoteAccessVpnVO vpn : remoteAccessVpns) { - _remoteAccessVpnMgr.destroyRemoteAccessVpnForIp(vpn.getServerAddressId(), caller); + _remoteAccessVpnMgr.destroyRemoteAccessVpnForIp(vpn.getServerAddressId(), caller, false); } } catch (ResourceUnavailableException ex) { s_logger.warn("Failed to cleanup remote access vpn resources as a part of account id=" + accountId + " cleanup due to Exception: ", ex); diff --git a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java index 3e80865572b6..6d2348f97d85 100644 --- a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java @@ -247,7 +247,7 @@ public IpAddress getIp(long id) { */ @Override public Network updateGuestNetwork(long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, - Long networkOfferingId, Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String newUUID,boolean updateInSequence) { + Long networkOfferingId, Boolean changeCidr, String guestVmCidr, Boolean displayNetwork, String newUUID,boolean updateInSequence, boolean forced) { // TODO Auto-generated method stub return null; } @@ -846,6 +846,16 @@ public boolean canUpdateInSequence(Network network) { return false; } + @Override + public List getServicesNotSupportedInNewOffering(Network network, long newNetworkOfferingId) { + return null; + } + + @Override + public void cleanupConfigForServicesInNetwork(List services, Network network) { + return; + } + @Override public void configureUpdateInSequence(Network network) { return; From ecb3b2cf2daaff4909b0c72c4eddaf9b37fd4cdb Mon Sep 17 00:00:00 2001 From: Bharat Kumar Date: Tue, 22 Dec 2015 23:51:31 +0530 Subject: [PATCH 04/10] Added a test to test update router in sequence --- .../component/maint/test_redundant_router.py | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) diff --git a/test/integration/component/maint/test_redundant_router.py b/test/integration/component/maint/test_redundant_router.py index 98b0bae1ba09..cad1d341d55b 100644 --- a/test/integration/component/maint/test_redundant_router.py +++ b/test/integration/component/maint/test_redundant_router.py @@ -29,6 +29,7 @@ get_zone, get_process_status) import time +import multiprocessing # Import Local Modules from marvin.cloudstackTestCase import cloudstackTestCase @@ -872,9 +873,16 @@ def setUpClass(cls): cls.testdata["nw_off_isolated_RVR"], conservemode=True ) + cls.network_offering_for_update=NetworkOffering.create( + cls.api_client, + cls.testdata["nw_off_isolated_RVR"], + conservemode=True + ) + cls._cleanup.append(cls.network_offering_for_update) cls._cleanup.append(cls.network_offering) # Enable Network offering cls.network_offering.update(cls.api_client, state='Enabled') + cls.network_offering_for_update.update(cls.api_client, state='Enabled') return @classmethod @@ -1511,3 +1519,149 @@ def test_05_stopBackupRvR_startInstance(self): "Redundant state of the router should be BACKUP but is %s" % routers[0].redundantstate) return + + def updateNetwork(self, conn): + try: + self.network.update( + self.api_client, + networkofferingid=self.network_offering_for_update.id, + updateinsequence=True, + forced=True, + changecidr=False + ) + except Exception as e: + conn.send("Failed to update network: %s due to %s"%(self.network.name, e)) + conn.send("update Network Complete") + return + + + + def get_master_and_backupRouter(self): + retry = 4 + master_router = backup_router=None + while retry > 0: + routers = Router.list( + self.apiclient, + networkid=self.network.id, + listall=True + ) + retry = retry-1 + if not (routers[0].redundantstate == 'MASTER' or routers[1].redundantstate == 'MASTER'): + continue; + if routers[0].redundantstate == 'MASTER': + master_router = routers[0] + backup_router = routers[1] + break + else: + master_router = routers[1] + backup_router = routers[0] + break + return master_router, backup_router + + + def chek_for_new_backupRouter(self,old_backup_router): + master_router, backup_router = self.get_master_and_backupRouter() + retry = 4 + self.info("Checking if new router is getting created.") + self.info("old_backup_router:"+old_backup_router.name+" new_backup_router:"+backup_router.name) + while old_backup_router.name == backup_router.name: + self.debug("waiting for new router old router:"+backup_router.name) + retry = retry-1 + if retry == 0: + break; + time.sleep(self.testdata["sleep"]) + master_router, backup_router = self.get_master_and_backupRouter() + if retry == 0: + self.fail("New router creation taking too long, timed out") + + def wait_untill_router_stabilises(self): + retry=4 + while retry > 0: + routers = Router.list( + self.apiclient, + networkid=self.network.id, + listall=True + ) + retry = retry-1 + self.info("waiting untill state of the routers is stable") + if routers[0].redundantstate != 'UNKNOWN' and routers[1].redundantstate != 'UNKNOWN': + return + elif retry==0: + self.fail("timedout while waiting for routers to stabilise") + return + time.sleep(self.testdata["sleep"]) + + @attr(tags=["bharat"]) + def test_06_updateVRs_in_sequence(self): + """Test update network and check if VRs are updated in sequence + """ + + # Steps to validate + # update network to a new offering + # check if the master router is running while backup is starting. + # check if the backup is running while master is starting. + # check if both the routers are running after the update is complete. + + #clean up the network to make sure it is in proper state. + self.network.restart(self.apiclient,cleanup=True) + time.sleep(self.testdata["sleep"]) + self.wait_untill_router_stabilises() + old_master_router, old_backup_router = self.get_master_and_backupRouter() + self.info("old_master_router:"+old_master_router.name+" old_backup_router"+old_backup_router.name) + #chek if the network is in correct state + self.assertEqual(old_master_router.state, "Running", "The master router is not running, network is not in a correct state to start the test") + self.assertEqual(old_backup_router.state, "Running", "The backup router is not running, network is not in a correct state to start the test") + + worker, monitor = multiprocessing.Pipe() + worker_process = multiprocessing.Process(target=self.updateNetwork, args=(worker,)) + worker_process.start() + if not worker_process.is_alive(): + message = monitor.recv() + if "Complete" not in message: + self.fail(message) + + self.info("Network update Started, the old backup router will get destroyed and a new router will be created") + + self.chek_for_new_backupRouter(old_backup_router) + master_router, new_backup_router=self.get_master_and_backupRouter() + #the state of the master router should be running. while backup is being updated + self.assertEqual(master_router.state, "Running", "State of the master router is not running") + self.assertEqual(master_router.redundantstate, 'MASTER', "Redundant state of the master router should be MASTER, but it is %s"%master_router.redundantstate) + self.info("Old backup router:"+old_backup_router.name+" is destroyed and new router:"+new_backup_router.name+" got created") + + #wait for the new backup to become master. + retry = 4 + while new_backup_router.name != master_router.name: + retry = retry-1 + if retry == 0: + break + time.sleep(self.testdata["sleep"]) + self.info("wating for backup router to become master router name:"+new_backup_router.name) + master_router, backup_router = self.get_master_and_backupRouter() + if retry == 0: + self.fail("timed out while waiting for new backup router to change state to MASTER.") + + #new backup router has become master. + self.info("newly created router:"+new_backup_router.name+" has changed state to Master") + self.info("old master router:"+old_master_router.name+"is destroyed") + #old master will get destroyed and a new backup will be created. + #wait until new backup changes state from unknown to backup + master_router, backup_router = self.get_master_and_backupRouter() + retry = 4 + while backup_router.redundantstate != 'BACKUP': + retry = retry-1 + self.info("waiting for router:"+backup_router.name+" to change state to Backup") + if retry == 0: + break + time.sleep(self.testdata["sleep"]) + master_router, backup_router = self.get_master_and_backupRouter() + self.assertEqual(master_router.state, "Running", "State of the master router is not running") + self.assertEqual(master_router.redundantstate, 'MASTER', "Redundant state of the master router should be MASTER, but it is %s"%master_router.redundantstate) + if retry == 0: + self.fail("timed out while waiting for new backup rotuer to change state to MASTER.") + + #the network update is complete.finally both the router should be running. + new_master_router, new_backup_router=self.get_master_and_backupRouter() + self.assertEqual(new_master_router.state, "Running", "State of the master router:"+new_master_router.name+" is not running") + self.assertEqual(new_backup_router.state, "Running", "State of the backup router:"+new_backup_router.name+" is not running") + worker_process.join() From e45b30b7ad6afbf82a5150006c66a1854d044ef9 Mon Sep 17 00:00:00 2001 From: Bharat Kumar Date: Mon, 12 Sep 2016 15:34:07 +0530 Subject: [PATCH 05/10] Added license headders --- .../cloud/network/element/RedundantResource.java | 16 ++++++++++++++++ setup/db/db/schema-452to460.sql | 2 +- setup/db/db/schema-481to482.sql | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/api/src/com/cloud/network/element/RedundantResource.java b/api/src/com/cloud/network/element/RedundantResource.java index 863c9cd330c5..39b6b97d73a6 100644 --- a/api/src/com/cloud/network/element/RedundantResource.java +++ b/api/src/com/cloud/network/element/RedundantResource.java @@ -1,3 +1,19 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. package com.cloud.network.element; import com.cloud.network.Network; diff --git a/setup/db/db/schema-452to460.sql b/setup/db/db/schema-452to460.sql index e05ad6d255db..3b380f397e66 100644 --- a/setup/db/db/schema-452to460.sql +++ b/setup/db/db/schema-452to460.sql @@ -420,5 +420,5 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(),'KVM', 'default', 'CentOS 7', 246, utc_timestamp(), 0); UPDATE `cloud`.`hypervisor_capabilities` SET `max_data_volumes_limit` = '32' WHERE `hypervisor_capabilities`.`hypervisor_type` = 'KVM'; -ALTER TABLE `cloud`.`domain_router` ADD COLUMN update_state varchar(64) DEFAULT NULL; + diff --git a/setup/db/db/schema-481to482.sql b/setup/db/db/schema-481to482.sql index 4cebaf1542eb..274509707c72 100644 --- a/setup/db/db/schema-481to482.sql +++ b/setup/db/db/schema-481to482.sql @@ -21,3 +21,4 @@ INSERT IGNORE INTO `cloud`.`guest_os` (id, uuid, category_id, display_name, created) VALUES (275, UUID(), 6, 'Other PV Virtio-SCSI (64-bit)', now()); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid, hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created) VALUES (UUID(), 'KVM', 'default', 'Other PV Virtio-SCSI (64-bit)', 275, now()); +ALTER TABLE `cloud`.`domain_router` ADD COLUMN update_state varchar(64) DEFAULT NULL; From 07d22c5608201536de89fed8d42ba776c884c5ac Mon Sep 17 00:00:00 2001 From: Syed Date: Tue, 23 Feb 2016 10:14:33 -0500 Subject: [PATCH 06/10] CLOUDSTACK-9296: Start ipsec for client VPN --- .../debian/config/opt/cloud/bin/configure.py | 1 + .../integration/component/test_vpn_service.py | 212 ++++++++++++++++++ 2 files changed, 213 insertions(+) create mode 100644 test/integration/component/test_vpn_service.py diff --git a/systemvm/patches/debian/config/opt/cloud/bin/configure.py b/systemvm/patches/debian/config/opt/cloud/bin/configure.py index b5f65e733cb6..595b071773c1 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/configure.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/configure.py @@ -612,6 +612,7 @@ def process(self): #Enable remote access vpn if vpnconfig['create']: logging.debug("Enabling remote access vpn on "+ public_ip) + CsHelper.start_if_stopped("ipsec") self.configure_l2tpIpsec(public_ip, self.dbag[public_ip]) logging.debug("Remote accessvpn data bag %s", self.dbag) self.remoteaccessvpn_iptables(public_ip, self.dbag[public_ip]) diff --git a/test/integration/component/test_vpn_service.py b/test/integration/component/test_vpn_service.py new file mode 100644 index 000000000000..8d27624b766c --- /dev/null +++ b/test/integration/component/test_vpn_service.py @@ -0,0 +1,212 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" P1 tests for VPN service +""" +# Import Local Modules +from nose.plugins.attrib import attr +from marvin.cloudstackException import CloudstackAPIException +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import ( + Account, + ServiceOffering, + VirtualMachine, + PublicIPAddress, + Vpn, + VpnUser, + Configurations, + NATRule + ) +from marvin.lib.common import (get_domain, + get_zone, + get_template + ) +from marvin.lib.utils import cleanup_resources + + +class Services: + """Test VPN Service + """ + + def __init__(self): + self.services = { + "account": { + "email": "test@test.com", + "firstname": "Test", + "lastname": "User", + "username": "test", + # Random characters are appended for unique + # username + "password": "password", + }, + "service_offering": { + "name": "Tiny Instance", + "displaytext": "Tiny Instance", + "cpunumber": 1, + "cpuspeed": 100, # in MHz + "memory": 128, # In MBs + }, + "disk_offering": { + "displaytext": "Small Disk Offering", + "name": "Small Disk Offering", + "disksize": 1 + }, + "virtual_machine": { + "displayname": "TestVM", + "username": "root", + "password": "password", + "ssh_port": 22, + "hypervisor": 'KVM', + "privateport": 22, + "publicport": 22, + "protocol": 'TCP', + }, + "vpn_user": { + "username": "test", + "password": "test", + }, + "natrule": { + "privateport": 1701, + "publicport": 1701, + "protocol": "UDP" + }, + "ostype": 'CentOS 5.5 (64-bit)', + "sleep": 60, + "timeout": 10, + # Networking mode: Advanced, Basic + } + + +class TestVPNService(cloudstackTestCase): + @classmethod + def setUpClass(cls): + cls.testClient = super(TestVPNService, cls).getClsTestClient() + cls.api_client = cls.testClient.getApiClient() + + cls.services = Services().services + # Get Zone, Domain and templates + cls.domain = get_domain(cls.api_client) + cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) + + cls.services["mode"] = cls.zone.networktype + + cls.template = get_template( + cls.api_client, + cls.zone.id, + cls.services["ostype"] + ) + + cls.services["virtual_machine"]["zoneid"] = cls.zone.id + cls.service_offering = ServiceOffering.create( + cls.api_client, + cls.services["service_offering"] + ) + + cls._cleanup = [cls.service_offering, ] + return + + @classmethod + def tearDownClass(cls): + try: + # Cleanup resources used + cleanup_resources(cls.api_client, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + try: + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.account = Account.create( + self.apiclient, + self.services["account"], + domainid=self.domain.id + ) + self.cleanup = [ + self.account, + ] + self.virtual_machine = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + templateid=self.template.id, + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id + ) + self.public_ip = PublicIPAddress.create( + self.apiclient, + accountid=self.virtual_machine.account, + zoneid=self.virtual_machine.zoneid, + domainid=self.virtual_machine.domainid, + services=self.services["virtual_machine"] + ) + return + except CloudstackAPIException as e: + self.tearDown() + raise e + + def tearDown(self): + try: + # Clean up, terminate the created instance, volumes and snapshots + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def create_VPN(self, public_ip): + """Creates VPN for the network""" + + self.debug("Creating VPN with public IP: %s" % public_ip.ipaddress.id) + try: + # Assign VPN to Public IP + vpn = Vpn.create(self.apiclient, + self.public_ip.ipaddress.id, + account=self.account.name, + domainid=self.account.domainid) + + self.debug("Verifying the remote VPN access") + vpns = Vpn.list(self.apiclient, + publicipid=public_ip.ipaddress.id, + listall=True) + self.assertEqual( + isinstance(vpns, list), + True, + "List VPNs shall return a valid response" + ) + return vpn + except Exception as e: + self.fail("Failed to create remote VPN access: %s" % e) + + + @attr(tags=["advanced", "advancedns"]) + def test_01_VPN_service(self): + """Tests if VPN service is running""" + + # Validate if IPSEC is running on the public + # IP by using ike-scan + + self.create_VPN(self.public_ip) + + cmd = ['ike-scan', self.public_ip, '-s', '4534'] # Random port + + stdout = subprocess.check_output(cmd) + + if "1 returned handshake" not in stdout: + self.fail("Unable to connect to VPN service") + + return From 6874a8e26aeba604f9a682127e1239ac59a4b3f4 Mon Sep 17 00:00:00 2001 From: Slair1 Date: Thu, 23 Feb 2017 18:35:41 -0600 Subject: [PATCH 07/10] CLOUDSTACK-9801 Make sure IP address is active --- systemvm/patches/debian/config/opt/cloud/bin/configure.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/configure.py b/systemvm/patches/debian/config/opt/cloud/bin/configure.py index 595b071773c1..9f38caaff6e7 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/configure.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/configure.py @@ -612,6 +612,12 @@ def process(self): #Enable remote access vpn if vpnconfig['create']: logging.debug("Enabling remote access vpn on "+ public_ip) + + dev = CsHelper.get_device(public_ip) + if dev == "": + logging.error("Request for ipsec to %s not possible because ip is not configured", public_ip) + continue + CsHelper.start_if_stopped("ipsec") self.configure_l2tpIpsec(public_ip, self.dbag[public_ip]) logging.debug("Remote accessvpn data bag %s", self.dbag) From 837e16d20f7159fd1a526d3a4e7f3011e33ec7a8 Mon Sep 17 00:00:00 2001 From: Nathan Johnson Date: Tue, 18 Apr 2017 10:49:52 -0500 Subject: [PATCH 08/10] CLOUDSTACK-9882 removing blanket catch of CloudRuntimeException --- .../cloudstack/engine/orchestration/NetworkOrchestrator.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 966975b424d8..dfb519aec053 100644 --- a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -1048,9 +1048,6 @@ public Pair implementNetwork(final long networkId, final } catch (final NoTransitionException e) { s_logger.error(e.getMessage()); return null; - } catch (final CloudRuntimeException e) { - s_logger.error("Caught exception: " + e.getMessage()); - return null; } finally { if (implemented.first() == null) { s_logger.debug("Cleaning up because we're unable to implement the network " + network); From b483274735d9e4f1373f7233c49294ebfdd4585b Mon Sep 17 00:00:00 2001 From: Nathan Johnson Date: Tue, 18 Apr 2017 15:38:07 -0500 Subject: [PATCH 09/10] fixing cs_ip.py from master -- this rolls in 2003 --- .../debian/config/opt/cloud/bin/cs_ip.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs_ip.py b/systemvm/patches/debian/config/opt/cloud/bin/cs_ip.py index 050a8dc71e68..f83bf298c4d1 100644 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs_ip.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs_ip.py @@ -19,27 +19,33 @@ from netaddr import * def merge(dbag, ip): - index = -1 # a non-valid array index + nic_dev_id = None for dev in dbag: if dev == "id": continue - for i, address in enumerate(dbag[dev]): + for address in dbag[dev]: if address['public_ip'] == ip['public_ip']: - index = i + if 'nic_dev_id' in address: + nic_dev_id = address['nic_dev_id'] + dbag[dev].remove(address) ipo = IPNetwork(ip['public_ip'] + '/' + ip['netmask']) - ip['device'] = 'eth' + str(ip['nic_dev_id']) + if 'nic_dev_id' in ip: + nic_dev_id = ip['nic_dev_id'] + ip['device'] = 'eth' + str(nic_dev_id) ip['broadcast'] = str(ipo.broadcast) ip['cidr'] = str(ipo.ip) + '/' + str(ipo.prefixlen) ip['size'] = str(ipo.prefixlen) ip['network'] = str(ipo.network) + '/' + str(ipo.prefixlen) if 'nw_type' not in ip.keys(): ip['nw_type'] = 'public' + else: + ip['nw_type'] = ip['nw_type'].lower() if ip['nw_type'] == 'control': dbag[ip['device']] = [ip] else: - if index != -1: - dbag[ip['device']][index] = ip + if 'source_nat' in ip and ip['source_nat'] and ip['device'] in dbag and len(dbag[ip['device']]) > 0: + dbag[ip['device']].insert(0, ip) # make sure the source_nat ip is first (primary) on the device else: dbag.setdefault(ip['device'], []).append(ip) From ab5b5952167098411b0c36a0ea72c529d0ea0859 Mon Sep 17 00:00:00 2001 From: Nathan Johnson Date: Tue, 6 Jun 2017 16:33:02 -0500 Subject: [PATCH 10/10] CLOUDSTACK-9949 adding ability to specify mac addresses Added macaddress optional parameter for deployVirtualMachine Added validation to NetUtil for mac addresses Added test coverage for NetUtil mac address validation Modified IpAddresses class to contain a mac address Modified networkfalseiptonetworklistip parameter to accept mac Updated addNicToVirtualMachineAdds to accept macaddress optional updating description for iptonetworklist Make sure mac is unique among the network when specifying for user vm. adding macADdress to AllFieldsSearch for NicDaoImpl Added NetUtils.standardizeMac() to ensure colon separator. Updated test_nic smoke test to cover specifying macaddress. add isUnicastMac() in NetUtils to ensure unicast macs. --- api/src/com/cloud/network/Network.java | 14 +++++++ api/src/com/cloud/network/NetworkModel.java | 3 +- api/src/com/cloud/vm/NicProfile.java | 5 +++ .../apache/cloudstack/api/ApiConstants.java | 1 + .../api/command/user/vm/AddNicToVMCmd.java | 17 +++++++++ .../api/command/user/vm/DeployVMCmd.java | 31 +++++++++++++-- .../schema/src/com/cloud/vm/dao/NicDao.java | 2 + .../src/com/cloud/vm/dao/NicDaoImpl.java | 9 +++++ .../cloud/network/IpAddressManagerImpl.java | 9 +++-- .../com/cloud/network/NetworkModelImpl.java | 15 +++++++- .../src/com/cloud/vm/UserVmManagerImpl.java | 18 ++++++--- .../cloud/network/MockNetworkModelImpl.java | 3 +- .../com/cloud/vpc/MockNetworkModelImpl.java | 3 +- test/integration/smoke/test_nic.py | 38 +++++++++++++++++++ tools/marvin/marvin/lib/base.py | 12 +++++- .../java/com/cloud/utils/net/NetUtils.java | 27 +++++++++++++ .../com/cloud/utils/net/NetUtilsTest.java | 18 +++++++++ 17 files changed, 208 insertions(+), 17 deletions(-) diff --git a/api/src/com/cloud/network/Network.java b/api/src/com/cloud/network/Network.java index 81447d6457c5..2e14f63cb3d5 100644 --- a/api/src/com/cloud/network/Network.java +++ b/api/src/com/cloud/network/Network.java @@ -274,12 +274,26 @@ private State(String description) { public class IpAddresses { private String ip4Address; private String ip6Address; + private String macAddress; + + public String getMacAddress() { + return macAddress; + } + + public void setMacAddress(String macAddress) { + this.macAddress = macAddress; + } public IpAddresses(String ip4Address, String ip6Address) { setIp4Address(ip4Address); setIp6Address(ip6Address); } + public IpAddresses(String ipAddress, String ip6Address, String macAddress) { + this(ipAddress, ip6Address); + setMacAddress(macAddress); + } + public String getIp4Address() { return ip4Address; } diff --git a/api/src/com/cloud/network/NetworkModel.java b/api/src/com/cloud/network/NetworkModel.java index 780f97d22f4e..cada36eaaa3d 100644 --- a/api/src/com/cloud/network/NetworkModel.java +++ b/api/src/com/cloud/network/NetworkModel.java @@ -27,6 +27,7 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.network.Network.Capability; +import com.cloud.network.Network.IpAddresses; import com.cloud.network.Network.Provider; import com.cloud.network.Network.Service; import com.cloud.network.Networks.TrafficType; @@ -254,7 +255,7 @@ public interface NetworkModel { void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gateway, String ip6Cidr) throws InvalidParameterValueException; - void checkRequestedIpAddresses(long networkId, String ip4, String ip6) throws InvalidParameterValueException; + void checkRequestedIpAddresses(long networkId, IpAddresses ips) throws InvalidParameterValueException; String getStartIpv6Address(long id); diff --git a/api/src/com/cloud/vm/NicProfile.java b/api/src/com/cloud/vm/NicProfile.java index 58e05124c89f..6ef9cfe5db16 100644 --- a/api/src/com/cloud/vm/NicProfile.java +++ b/api/src/com/cloud/vm/NicProfile.java @@ -114,6 +114,11 @@ public NicProfile(String requestedIPv4, String requestedIPv6) { this.requestedIPv6 = requestedIPv6; } + public NicProfile(String requestedIPv4, String requestedIPv6, String requestedMacAddress) { + this(requestedIPv4, requestedIPv6); + this.macAddress = requestedMacAddress; + } + public NicProfile(ReservationStrategy strategy, String iPv4Address, String macAddress, String iPv4gateway, String iPv4netmask) { format = AddressFormat.Ip4; this.iPv4Address = iPv4Address; diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java index f8260a056f6b..5f30d6dfaf23 100644 --- a/api/src/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/org/apache/cloudstack/api/ApiConstants.java @@ -159,6 +159,7 @@ public class ApiConstants { public static final String LUN = "lun"; public static final String LBID = "lbruleid"; public static final String MAX = "max"; + public static final String MAC_ADDRESS = "macaddress"; public static final String MAX_SNAPS = "maxsnaps"; public static final String MEMORY = "memory"; public static final String MODE = "mode"; diff --git a/api/src/org/apache/cloudstack/api/command/user/vm/AddNicToVMCmd.java b/api/src/org/apache/cloudstack/api/command/user/vm/AddNicToVMCmd.java index f265ecf236a0..ef03e78bea57 100644 --- a/api/src/org/apache/cloudstack/api/command/user/vm/AddNicToVMCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/vm/AddNicToVMCmd.java @@ -36,8 +36,10 @@ import org.apache.cloudstack.context.CallContext; import com.cloud.event.EventTypes; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.user.Account; import com.cloud.uservm.UserVm; +import com.cloud.utils.net.NetUtils; import com.cloud.vm.VirtualMachine; @APICommand(name = "addNicToVirtualMachine", description = "Adds VM to specified network by creating a NIC", responseObject = UserVmResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, @@ -60,6 +62,9 @@ public class AddNicToVMCmd extends BaseAsyncCmd { @Parameter(name = ApiConstants.IP_ADDRESS, type = CommandType.STRING, description = "IP Address for the new network") private String ipaddr; + @Parameter(name = ApiConstants.MAC_ADDRESS, type = CommandType.STRING, description = "Mac Address for the new network") + private String macaddr; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -76,6 +81,18 @@ public String getIpAddress() { return ipaddr; } + public String getMacAddress() { + if (macaddr == null) { + return null; + } + if(!NetUtils.isValidMac(macaddr)) { + throw new InvalidParameterValueException("Mac address is not valid: " + macaddr); + } else if(!NetUtils.isUnicastMac(macaddr)) { + throw new InvalidParameterValueException("Mac address is not unicast: " + macaddr); + } + return NetUtils.standardizeMacAddress(macaddr); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java b/api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java index 7769ff8573b1..67c33511be1d 100644 --- a/api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java @@ -152,7 +152,7 @@ public class DeployVMCmd extends BaseAsyncCreateCustomIdCmd { private List securityGroupNameList; @Parameter(name = ApiConstants.IP_NETWORK_LIST, type = CommandType.MAP, description = "ip to network mapping. Can't be specified with networkIds parameter." - + " Example: iptonetworklist[0].ip=10.10.10.11&iptonetworklist[0].ipv6=fc00:1234:5678::abcd&iptonetworklist[0].networkid=uuid - requests to use ip 10.10.10.11 in network id=uuid") + + " Example: iptonetworklist[0].ip=10.10.10.11&iptonetworklist[0].ipv6=fc00:1234:5678::abcd&iptonetworklist[0].networkid=uuid&iptonetworklist[0].mac=aa:bb:cc:dd:ee::ff - requests to use ip 10.10.10.11 in network id=uuid") private Map ipToNetworkList; @Parameter(name = ApiConstants.IP_ADDRESS, type = CommandType.STRING, description = "the ip address for default vm's network") @@ -161,6 +161,9 @@ public class DeployVMCmd extends BaseAsyncCreateCustomIdCmd { @Parameter(name = ApiConstants.IP6_ADDRESS, type = CommandType.STRING, description = "the ipv6 address for default vm's network") private String ip6Address; + @Parameter(name = ApiConstants.MAC_ADDRESS, type = CommandType.STRING, description = "the mac address for default vm's network") + private String macAddress; + @Parameter(name = ApiConstants.KEYBOARD, type = CommandType.STRING, description = "an optional keyboard device type for the virtual machine. valid value can be one of de,de-ch,es,fi,fr,fr-be,fr-ch,is,it,jp,nl-be,no,pt,uk,us") private String keyboard; @@ -352,10 +355,19 @@ private Map getIpToNetworkMap() { } String requestedIp = ips.get("ip"); String requestedIpv6 = ips.get("ipv6"); + String requestedMac = ips.get("mac"); if (requestedIpv6 != null) { requestedIpv6 = NetUtils.standardizeIp6Address(requestedIpv6); } - IpAddresses addrs = new IpAddresses(requestedIp, requestedIpv6); + if (requestedMac != null) { + if(!NetUtils.isValidMac(requestedMac)) { + throw new InvalidParameterValueException("Mac address is not valid: " + requestedMac); + } else if(!NetUtils.isUnicastMac(requestedMac)) { + throw new InvalidParameterValueException("Mac address is not unicast: " + requestedMac); + } + requestedMac = NetUtils.standardizeMacAddress(requestedMac); + } + IpAddresses addrs = new IpAddresses(requestedIp, requestedIpv6, requestedMac); ipToNetworkMap.put(networkId, addrs); } } @@ -370,6 +382,19 @@ public String getIp6Address() { return NetUtils.standardizeIp6Address(ip6Address); } + + public String getMacAddress() { + if (macAddress == null) { + return null; + } + if(!NetUtils.isValidMac(macAddress)) { + throw new InvalidParameterValueException("Mac address is not valid: " + macAddress); + } else if(!NetUtils.isUnicastMac(macAddress)) { + throw new InvalidParameterValueException("Mac address is not unicast: " + macAddress); + } + return NetUtils.standardizeMacAddress(macAddress); + } + public List getAffinityGroupIdList() { if (affinityGroupNameList != null && affinityGroupIdList != null) { throw new InvalidParameterValueException("affinitygroupids parameter is mutually exclusive with affinitygroupnames parameter"); @@ -577,7 +602,7 @@ public void create() throws ResourceAllocationException { } UserVm vm = null; - IpAddresses addrs = new IpAddresses(ipAddress, getIp6Address()); + IpAddresses addrs = new IpAddresses(ipAddress, getIp6Address(), getMacAddress()); if (zone.getNetworkType() == NetworkType.Basic) { if (getNetworkIds() != null) { throw new InvalidParameterValueException("Can't specify network Ids in Basic zone"); diff --git a/engine/schema/src/com/cloud/vm/dao/NicDao.java b/engine/schema/src/com/cloud/vm/dao/NicDao.java index 2c7895a5a557..9105a0d5c8d3 100644 --- a/engine/schema/src/com/cloud/vm/dao/NicDao.java +++ b/engine/schema/src/com/cloud/vm/dao/NicDao.java @@ -44,6 +44,8 @@ public interface NicDao extends GenericDao { NicVO findByIp4AddressAndNetworkId(String ip4Address, long networkId); + NicVO findByNetworkIdAndMacAddress(long networkId, String mac); + NicVO findDefaultNicForVM(long instanceId); /** diff --git a/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java b/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java index f27088a44570..a860a8dfe4c9 100644 --- a/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java +++ b/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java @@ -66,6 +66,7 @@ protected void init() { AllFieldsSearch.and("secondaryip", AllFieldsSearch.entity().getSecondaryIp(), Op.EQ); AllFieldsSearch.and("nicid", AllFieldsSearch.entity().getId(), Op.EQ); AllFieldsSearch.and("strategy", AllFieldsSearch.entity().getReservationStrategy(), Op.EQ); + AllFieldsSearch.and("macAddress", AllFieldsSearch.entity().getMacAddress(), Op.EQ); AllFieldsSearch.done(); IpSearch = createSearchBuilder(String.class); @@ -190,6 +191,14 @@ public NicVO findByIp4AddressAndNetworkId(String ip4Address, long networkId) { return findOneBy(sc); } + @Override + public NicVO findByNetworkIdAndMacAddress(long networkId, String mac) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("network", networkId); + sc.setParameters("macAddress", mac); + return findOneBy(sc); + } + @Override public NicVO findDefaultNicForVM(long instanceId) { SearchCriteria sc = AllFieldsSearch.create(); diff --git a/server/src/com/cloud/network/IpAddressManagerImpl.java b/server/src/com/cloud/network/IpAddressManagerImpl.java index 3745433289e5..2dc04d5e823c 100644 --- a/server/src/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/com/cloud/network/IpAddressManagerImpl.java @@ -1897,7 +1897,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Insuff nic.setBroadcastUri(BroadcastDomainType.Vlan.toUri(ip.getVlanTag())); nic.setFormat(AddressFormat.Ip4); nic.setReservationId(String.valueOf(ip.getVlanTag())); - nic.setMacAddress(ip.getMacAddress()); + if (nic.getMacAddress() == null) { + nic.setMacAddress(ip.getMacAddress()); + } } nic.setIPv4Dns1(dc.getDns1()); nic.setIPv4Dns2(dc.getDns2()); @@ -1969,8 +1971,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Insuff nic.setBroadcastUri(network.getBroadcastUri()); nic.setFormat(AddressFormat.Ip4); - - nic.setMacAddress(_networkModel.getNextAvailableMacAddressInNetwork(network.getId())); + if(nic.getMacAddress() == null) { + nic.setMacAddress(_networkModel.getNextAvailableMacAddressInNetwork(network.getId())); + } } nic.setIPv4Dns1(dc.getDns1()); nic.setIPv4Dns2(dc.getDns2()); diff --git a/server/src/com/cloud/network/NetworkModelImpl.java b/server/src/com/cloud/network/NetworkModelImpl.java index f3e25e8695b5..5dfc02fbd450 100644 --- a/server/src/com/cloud/network/NetworkModelImpl.java +++ b/server/src/com/cloud/network/NetworkModelImpl.java @@ -60,6 +60,7 @@ import com.cloud.network.IpAddress.State; import com.cloud.network.Network.Capability; import com.cloud.network.Network.GuestType; +import com.cloud.network.Network.IpAddresses; import com.cloud.network.Network.Provider; import com.cloud.network.Network.Service; import com.cloud.network.Networks.TrafficType; @@ -2140,7 +2141,10 @@ public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gatew } @Override - public void checkRequestedIpAddresses(long networkId, String ip4, String ip6) throws InvalidParameterValueException { + public void checkRequestedIpAddresses(long networkId, IpAddresses ips) throws InvalidParameterValueException { + String ip4 = ips.getIp4Address(); + String ip6 = ips.getIp6Address(); + String mac = ips.getMacAddress(); if (ip4 != null) { if (!NetUtils.isValidIp(ip4)) { throw new InvalidParameterValueException("Invalid specified IPv4 address " + ip4); @@ -2169,6 +2173,15 @@ public void checkRequestedIpAddresses(long networkId, String ip4, String ip6) th throw new InvalidParameterValueException("Requested IPv6 is not in the predefined range!"); } } + if (mac != null) { + if(!NetUtils.isValidMac(mac)) { + throw new InvalidParameterValueException("Invalid specified MAC address " + mac); + } + if (_nicDao.findByNetworkIdAndMacAddress(networkId, mac) != null) { + throw new InvalidParameterValueException("The requested Mac address is already taken! " + mac); + } + + } } @Override diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 208899e49270..cadbe1273596 100644 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -1122,6 +1122,7 @@ public UserVm addNicToVirtualMachine(AddNicToVMCmd cmd) throws InvalidParameterV Long vmId = cmd.getVmId(); Long networkId = cmd.getNetworkId(); String ipAddress = cmd.getIpAddress(); + String macAddress = cmd.getMacAddress(); Account caller = CallContext.current().getCallingAccount(); UserVmVO vmInstance = _vmDao.findById(vmId); @@ -1152,12 +1153,17 @@ public UserVm addNicToVirtualMachine(AddNicToVMCmd cmd) throws InvalidParameterV throw new CloudRuntimeException("A NIC already exists for VM:" + vmInstance.getInstanceName() + " in network: " + network.getUuid()); } - NicProfile profile = new NicProfile(null, null); + NicProfile profile = new NicProfile(ipAddress, null, macAddress); if (ipAddress != null) { if (!(NetUtils.isValidIp(ipAddress) || NetUtils.isValidIpv6(ipAddress))) { throw new InvalidParameterValueException("Invalid format for IP address parameter: " + ipAddress); } - profile = new NicProfile(ipAddress, null); + } + + if(macAddress != null) { + if(!NetUtils.isValidMac(macAddress)) { + throw new InvalidParameterValueException("Invalid format for MAC address parameter: " + macAddress); + } } // Perform permission check on VM @@ -3249,17 +3255,19 @@ protected UserVm createVirtualMachine(DataCenter zone, ServiceOffering serviceOf if (requestedIpPair == null) { requestedIpPair = new IpAddresses(null, null); } else { - _networkModel.checkRequestedIpAddresses(network.getId(), requestedIpPair.getIp4Address(), requestedIpPair.getIp6Address()); + _networkModel.checkRequestedIpAddresses(network.getId(), requestedIpPair); } - NicProfile profile = new NicProfile(requestedIpPair.getIp4Address(), requestedIpPair.getIp6Address()); + NicProfile profile = new NicProfile(requestedIpPair.getIp4Address(), requestedIpPair.getIp6Address(), requestedIpPair.getMacAddress()); if (defaultNetworkNumber == 0) { defaultNetworkNumber++; // if user requested specific ip for default network, add it if (defaultIps.getIp4Address() != null || defaultIps.getIp6Address() != null) { - _networkModel.checkRequestedIpAddresses(network.getId(), defaultIps.getIp4Address(), defaultIps.getIp6Address()); + _networkModel.checkRequestedIpAddresses(network.getId(), defaultIps); profile = new NicProfile(defaultIps.getIp4Address(), defaultIps.getIp6Address()); + } else if (defaultIps.getMacAddress() != null) { + profile = new NicProfile(null, null, defaultIps.getMacAddress()); } profile.setDefaultNic(true); diff --git a/server/test/com/cloud/network/MockNetworkModelImpl.java b/server/test/com/cloud/network/MockNetworkModelImpl.java index c00d83f72651..04861d311d89 100644 --- a/server/test/com/cloud/network/MockNetworkModelImpl.java +++ b/server/test/com/cloud/network/MockNetworkModelImpl.java @@ -30,6 +30,7 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.network.Network.Capability; import com.cloud.network.Network.GuestType; +import com.cloud.network.Network.IpAddresses; import com.cloud.network.Network.Provider; import com.cloud.network.Network.Service; import com.cloud.network.Networks.IsolationType; @@ -825,7 +826,7 @@ public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gatew } @Override - public void checkRequestedIpAddresses(long networkId, String ip4, String ip6) throws InvalidParameterValueException { + public void checkRequestedIpAddresses(long networkId, IpAddresses ips) throws InvalidParameterValueException { // TODO Auto-generated method stub } diff --git a/server/test/com/cloud/vpc/MockNetworkModelImpl.java b/server/test/com/cloud/vpc/MockNetworkModelImpl.java index 93b31e95f9ce..50100014cb77 100644 --- a/server/test/com/cloud/vpc/MockNetworkModelImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkModelImpl.java @@ -33,6 +33,7 @@ import com.cloud.network.Network; import com.cloud.network.Network.Capability; import com.cloud.network.Network.GuestType; +import com.cloud.network.Network.IpAddresses; import com.cloud.network.Network.Provider; import com.cloud.network.Network.Service; import com.cloud.network.NetworkModel; @@ -840,7 +841,7 @@ public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gatew } @Override - public void checkRequestedIpAddresses(long networkId, String ip4, String ip6) throws InvalidParameterValueException { + public void checkRequestedIpAddresses(long networkId, IpAddresses ips) throws InvalidParameterValueException { // TODO Auto-generated method stub } diff --git a/test/integration/smoke/test_nic.py b/test/integration/smoke/test_nic.py index 7067074c78b5..9dc385c5432e 100644 --- a/test/integration/smoke/test_nic.py +++ b/test/integration/smoke/test_nic.py @@ -30,6 +30,7 @@ import signal import sys +import logging import time @@ -37,6 +38,11 @@ class TestNic(cloudstackTestCase): def setUp(self): self.cleanup = [] + self.logger = logging.getLogger('TestNIC') + self.stream_handler = logging.StreamHandler() + self.logger.setLevel(logging.DEBUG) + self.logger.addHandler(self.stream_handler) + def signal_handler(signal, frame): self.tearDown() @@ -278,6 +284,38 @@ def test_01_nic(self): return + def test_02_nic_with_mac(self): + """Test to add and update added nic to a virtual machine with specific mac""" + + hypervisorIsVmware = False + isVmwareToolInstalled = False + if self.hypervisor.lower() == "vmware": + hypervisorIsVmware = True + + self.virtual_machine2 = VirtualMachine.create( + self.apiclient, + self.services["small"], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + networkids=[self.test_network.id], + macaddress="aa:bb:cc:dd:ee:ff", + mode=self.zone.networktype if hypervisorIsVmware else "default" + ) + self.cleanup.insert(0, self.virtual_machine2) + self.assertEqual(self.virtual_machine2.nic[0].macaddress, "aa:bb:cc:dd:ee:ff", "Mac address not honored") + vmdata = self.virtual_machine2.add_nic( + self.apiclient, + self.test_network2.id, + macaddress="ee:ee:dd:cc:bb:aa") + found = False + for n in vmdata.nic: + if n.macaddress == "ee:ee:dd:cc:bb:aa": + found = True + break + + self.assertTrue(found, "Nic not successfully added with specified mac address") + def tearDown(self): try: for obj in self.cleanup: diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index 1881589a18ec..977ad66072a7 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -361,7 +361,7 @@ def create(cls, apiclient, services, templateid=None, accountid=None, hostid=None, keypair=None, ipaddress=None, mode='default', method='GET', hypervisor=None, customcpunumber=None, customcpuspeed=None, custommemory=None, rootdisksize=None, - rootdiskcontroller=None): + rootdiskcontroller=None, macaddress=None): """Create the instance""" cmd = deployVirtualMachine.deployVirtualMachineCmd() @@ -475,6 +475,11 @@ def create(cls, apiclient, services, templateid=None, accountid=None, if mode.lower() == 'basic': cls.ssh_access_group(apiclient, cmd) + if macaddress: + cmd.macaddress = macaddress + elif macaddress in services: + cmd.macaddress = services["macaddress"] + virtual_machine = apiclient.deployVirtualMachine(cmd, method=method) virtual_machine.ssh_ip = virtual_machine.nic[0].ipaddress @@ -687,7 +692,7 @@ def detach_volume(self, apiclient, volume): cmd.id = volume.id return apiclient.detachVolume(cmd) - def add_nic(self, apiclient, networkId, ipaddress=None): + def add_nic(self, apiclient, networkId, ipaddress=None, macaddress=None): """Add a NIC to a VM""" cmd = addNicToVirtualMachine.addNicToVirtualMachineCmd() cmd.virtualmachineid = self.id @@ -696,6 +701,9 @@ def add_nic(self, apiclient, networkId, ipaddress=None): if ipaddress: cmd.ipaddress = ipaddress + if macaddress: + cmd.macaddress = macaddress + return apiclient.addNicToVirtualMachine(cmd) def remove_nic(self, apiclient, nicId): diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java index a4e569c59356..a049efc4ae95 100644 --- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java +++ b/utils/src/main/java/com/cloud/utils/net/NetUtils.java @@ -44,6 +44,7 @@ import org.apache.commons.lang.SystemUtils; import org.apache.commons.net.util.SubnetUtils; import org.apache.commons.validator.routines.InetAddressValidator; +import org.apache.commons.validator.routines.RegexValidator; import org.apache.log4j.Logger; import com.cloud.utils.IteratorUtil; @@ -1163,6 +1164,23 @@ public static boolean validateGuestCidr(final String cidr) { return false; } + public static boolean isValidMac(final String macAddr) { + RegexValidator mv = new RegexValidator("^(?:[0-9a-f]{1,2}([-:\\.]))(?:[0-9a-f]{1,2}\\1){4}[0-9a-f]{1,2}$", false); + return mv.isValid(macAddr); + } + + public static boolean isUnicastMac(final String macAddr) { + String std = standardizeMacAddress(macAddr); + if(std == null) { + return false; + } + long stdl = mac2Long(std); + // libvirt says that the last bit of first octet of mac address + // must be zero or this is a multicast MAC. + long mask = 0x1l << 40l; + return ((stdl & mask) == mask) ? false : true; + } + public static boolean verifyInstanceName(final String instanceName) { //instance name for cloudstack vms shouldn't contain - and spaces if (instanceName.contains("-") || instanceName.contains(" ") || instanceName.contains("+")) { @@ -1434,6 +1452,15 @@ public static String standardizeIp6Address(final String ip6Addr) { } } + public static String standardizeMacAddress(final String macAddr) { + if (!isValidMac(macAddr)) { + return null; + } + String norm = macAddr.replace('.', ':'); + norm = norm.replace('-', ':'); + return long2Mac(mac2Long(norm)); + } + public static String standardizeIp6Cidr(final String ip6Cidr){ try { return IPv6Network.fromString(ip6Cidr).toString(); diff --git a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java index 490d0df4eb09..fab0b9bcf7e7 100644 --- a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java +++ b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java @@ -171,6 +171,24 @@ public void testIsValidIp6Cidr() { assertFalse(NetUtils.isValidIp6Cidr("1234:5678::1")); } + @Test + public void testIsValidMacAddr() { + assertTrue(NetUtils.isValidMac("ee:12:34:5:32:ff")); + assertTrue(NetUtils.isValidMac("ee.12.34.5.32.ff")); + assertTrue(NetUtils.isValidMac("ee-12-34-5-32-ff")); + assertFalse(NetUtils.isValidMac("aa.12:34:5:32:ff")); + assertFalse(NetUtils.isValidMac("gg.gg:gg:gg:gg:gg")); + } + + @Test + public void testIsUnicastMac() { + + assertTrue(NetUtils.isUnicastMac("ee:12:34:5:32:ff")); + assertFalse(NetUtils.isUnicastMac("ff:12:34:5:32:ff")); + assertFalse(NetUtils.isUnicastMac("01:12:34:5:32:ff")); + assertTrue(NetUtils.isUnicastMac("00:ff:ff:ff:ff:ff")); + } + @Test public void testIsValidIpv6() { assertTrue(NetUtils.isValidIpv6("fc00::1"));