From ba9dcba16df604d8d4b84084bc24c04cc27fb9ac Mon Sep 17 00:00:00 2001 From: Bharat Kumar Date: Thu, 26 Nov 2015 15:30:06 +0530 Subject: [PATCH] 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 e26db340ff4..7a8a9498780 100644 --- a/api/src/com/cloud/network/NetworkService.java +++ b/api/src/com/cloud/network/NetworkService.java @@ -77,7 +77,7 @@ public interface NetworkService { 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 decf8c43733..d089b852449 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 f2c5119466c..388348c592c 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 class UpdateNetworkCmdByAdmin extends UpdateNetworkCmd { } 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 8ef9251e047..c313f369b0e 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 class UpdateNetworkCmd extends BaseAsyncCustomIdCmd { return updateInSequence; } + public boolean getForced(){ + if(forced==null){ + return false; + } + return forced; + } ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -159,7 +168,7 @@ public class UpdateNetworkCmd extends BaseAsyncCustomIdCmd { 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 37b7b5aaa3e..12ab531375f 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 class DeleteRemoteAccessVpnCmd extends BaseAsyncCmd { @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 1e2761f274a..89bec1783f6 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 @@ public interface NetworkOrchestrationService { 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 c0ea2f9b635..5a89dac7810 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 javax.naming.ConfigurationException; 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 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra @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 class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra 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 e65adb60f7d..8a2c3fd7b1f 100644 --- a/server/src/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/com/cloud/network/IpAddressManagerImpl.java @@ -562,7 +562,7 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage // 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 bb573decde2..cade54f088c 100644 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -39,6 +39,7 @@ import java.util.UUID; 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 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { @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 class NetworkServiceImpl extends ManagerBase implements NetworkService { 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 c2d923cb951..7b82125dccf 100644 --- a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java @@ -696,6 +696,16 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian 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 b473f050e04..065c097f0f8 100644 --- a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java +++ b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java @@ -281,7 +281,7 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc @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 class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc 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 class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc 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 7e80681caa2..880d363b8e6 100644 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -786,7 +786,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M 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 3e80865572b..6d2348f97d8 100644 --- a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java @@ -247,7 +247,7 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches */ @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 class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches 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;