Skip to content

Commit 6d3fffa

Browse files
remibergsmaremibergsma
committed
Merge pull request apache#1184 from ekholabs/fix/4.6-rvpc-pvtgw-CLOUDSTACK-9106
CLOUDSTACK-9106 - As a Developer I want the Redundant VPC private gateway feature fixedThis PR contains the same fixes from PR apache#1179, which was created against the master branch. In addition, the points mentioned by @DaanHoogland were handled in this new PR: * Made the code more consistent - result = result && methodCall(), instead of throwing exceptions in some places or not checking 2 consecutive returns - in case of rVPC. * Added an unit test to cover changes in the VpcRouterElementImpl.applyVpnUsers() method. The method returns an array of String, so I had to make sure it would contain the users from 2 consecutive calls. There are 2 tests to cover negative scenarios. * pr/1184: CLOUDSTACK-9106 - Makes Enum name compliant with Java code conventions. CLOUDSTACK-9106 - Adds a test to cover the changes in the applyVpnUsers() method CLOUDSTACK-9106 - Makes the router commands call more consistent. CLOUDSTACK-9106 - Enables private gateway tests on Redundant VPCs CLOUDSTACK-9106 - Refactor the createPrivateNicProfileForGateway() method CLOUDSTACK-9106 - Reduces the amount of iterations through the routers of a VPC Signed-off-by: Remi Bergsma <github@remi.nl>
2 parents e9de865 + 14db2d3 commit 6d3fffa

15 files changed

Lines changed: 476 additions & 254 deletions

File tree

‎plugins/network-elements/ovs/src/com/cloud/network/element/OvsElement.java‎

Lines changed: 66 additions & 60 deletions
Large diffs are not rendered by default.

‎server/src/com/cloud/network/element/VirtualRouterElement.java‎

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@
9595
import com.cloud.utils.crypt.DBEncryptionUtil;
9696
import com.cloud.utils.db.QueryBuilder;
9797
import com.cloud.utils.db.SearchCriteria.Op;
98-
import com.cloud.utils.exception.CloudRuntimeException;
9998
import com.cloud.utils.net.NetUtils;
10099
import com.cloud.vm.DomainRouterVO;
101100
import com.cloud.vm.NicProfile;
@@ -262,6 +261,7 @@ public boolean prepare(final Network network, final NicProfile nic, final Virtua
262261

263262
@Override
264263
public boolean applyFWRules(final Network network, final List<? extends FirewallRule> rules) throws ResourceUnavailableException {
264+
boolean result = true;
265265
if (canHandle(network, Service.Firewall)) {
266266
final List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
267267
if (routers == null || routers.isEmpty()) {
@@ -281,14 +281,11 @@ public boolean applyFWRules(final Network network, final List<? extends Firewall
281281
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
282282
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
283283

284-
if (!networkTopology.applyFirewallRules(network, rules, routers)) {
285-
throw new CloudRuntimeException("Failed to apply firewall rules in network " + network.getId());
286-
} else {
287-
return true;
284+
for (final DomainRouterVO domainRouterVO : routers) {
285+
result = result && networkTopology.applyFirewallRules(network, rules, domainRouterVO);
288286
}
289-
} else {
290-
return true;
291287
}
288+
return result;
292289
}
293290

294291
/*
@@ -405,6 +402,7 @@ public boolean validateLBRule(final Network network, final LoadBalancingRule rul
405402

406403
@Override
407404
public boolean applyLBRules(final Network network, final List<LoadBalancingRule> rules) throws ResourceUnavailableException {
405+
boolean result = true;
408406
if (canHandle(network, Service.Lb)) {
409407
if (!canHandleLbRules(rules)) {
410408
return false;
@@ -419,14 +417,11 @@ public boolean applyLBRules(final Network network, final List<LoadBalancingRule>
419417
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
420418
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
421419

422-
if (!networkTopology.applyLoadBalancingRules(network, rules, routers)) {
423-
throw new CloudRuntimeException("Failed to apply load balancing rules in network " + network.getId());
424-
} else {
425-
return true;
420+
for (final DomainRouterVO domainRouterVO : routers) {
421+
result = result && networkTopology.applyLoadBalancingRules(network, rules, domainRouterVO);
426422
}
427-
} else {
428-
return false;
429423
}
424+
return result;
430425
}
431426

432427
@Override
@@ -502,6 +497,7 @@ public boolean applyIps(final Network network, final List<? extends PublicIpAddr
502497
break;
503498
}
504499
}
500+
boolean result = true;
505501
if (canHandle) {
506502
final List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
507503
if (routers == null || routers.isEmpty()) {
@@ -512,10 +508,11 @@ public boolean applyIps(final Network network, final List<? extends PublicIpAddr
512508
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
513509
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
514510

515-
return networkTopology.associatePublicIP(network, ipAddress, routers);
516-
} else {
517-
return false;
511+
for (final DomainRouterVO domainRouterVO : routers) {
512+
result = result && networkTopology.associatePublicIP(network, ipAddress, domainRouterVO);
513+
}
518514
}
515+
return result;
519516
}
520517

521518
@Override
@@ -659,6 +656,7 @@ private static Map<Service, Map<Capability, String>> setCapabilities() {
659656

660657
@Override
661658
public boolean applyStaticNats(final Network network, final List<? extends StaticNat> rules) throws ResourceUnavailableException {
659+
boolean result = true;
662660
if (canHandle(network, Service.StaticNat)) {
663661
final List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
664662
if (routers == null || routers.isEmpty()) {
@@ -669,10 +667,11 @@ public boolean applyStaticNats(final Network network, final List<? extends Stati
669667
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
670668
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
671669

672-
return networkTopology.applyStaticNats(network, rules, routers);
673-
} else {
674-
return true;
670+
for (final DomainRouterVO domainRouterVO : routers) {
671+
result = result && networkTopology.applyStaticNats(network, rules, domainRouterVO);
672+
}
675673
}
674+
return result;
676675
}
677676

678677
@Override
@@ -681,20 +680,21 @@ public boolean shutdown(final Network network, final ReservationContext context,
681680
if (routers == null || routers.isEmpty()) {
682681
return true;
683682
}
684-
boolean result = true;
683+
boolean stopResult = true;
684+
boolean destroyResult = true;
685685
for (final DomainRouterVO router : routers) {
686-
result = result && _routerMgr.stop(router, false, context.getCaller(), context.getAccount()) != null;
686+
stopResult = stopResult && _routerMgr.stop(router, false, context.getCaller(), context.getAccount()) != null;
687+
if (!stopResult) {
688+
s_logger.warn("Failed to stop virtual router element " + router + ", but would try to process clean up anyway.");
689+
}
687690
if (cleanup) {
688-
if (!result) {
689-
s_logger.warn("Failed to stop virtual router element " + router + ", but would try to process clean up anyway.");
690-
}
691-
result = _routerMgr.destroyRouter(router.getId(), context.getAccount(), context.getCaller().getId()) != null;
692-
if (!result) {
691+
destroyResult = destroyResult && _routerMgr.destroyRouter(router.getId(), context.getAccount(), context.getCaller().getId()) != null;
692+
if (!destroyResult) {
693693
s_logger.warn("Failed to clean up virtual router element " + router);
694694
}
695695
}
696696
}
697-
return result;
697+
return stopResult & destroyResult;
698698
}
699699

700700
@Override
@@ -735,7 +735,7 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
735735
// save the password in DB
736736
for (final VirtualRouter router : routers) {
737737
if (router.getState() == State.Running) {
738-
return networkTopology.savePasswordToRouter(network, nic, uservm, routers);
738+
return networkTopology.savePasswordToRouter(network, nic, uservm, router);
739739
}
740740
}
741741
final String password = (String) uservm.getParameter(VirtualMachineProfile.Param.VmPassword);
@@ -768,7 +768,11 @@ public boolean saveSSHKey(final Network network, final NicProfile nic, final Vir
768768
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
769769
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
770770

771-
return networkTopology.saveSSHPublicKeyToRouter(network, nic, uservm, routers, sshPublicKey);
771+
boolean result = true;
772+
for (final DomainRouterVO domainRouterVO : routers) {
773+
result = result && networkTopology.saveSSHPublicKeyToRouter(network, nic, uservm, domainRouterVO, sshPublicKey);
774+
}
775+
return result;
772776
}
773777

774778
@Override
@@ -787,7 +791,11 @@ public boolean saveUserData(final Network network, final NicProfile nic, final V
787791
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
788792
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
789793

790-
return networkTopology.saveUserDataToRouter(network, nic, uservm, routers);
794+
boolean result = true;
795+
for (final DomainRouterVO domainRouterVO : routers) {
796+
result = result && networkTopology.saveUserDataToRouter(network, nic, uservm, domainRouterVO);
797+
}
798+
return result;
791799
}
792800

793801
@Override
@@ -844,6 +852,7 @@ public VirtualRouterProvider addElement(final Long nspId, final Type providerTyp
844852

845853
@Override
846854
public boolean applyPFRules(final Network network, final List<PortForwardingRule> rules) throws ResourceUnavailableException {
855+
boolean result = true;
847856
if (canHandle(network, Service.PortForwarding)) {
848857
final List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
849858
if (routers == null || routers.isEmpty()) {
@@ -854,14 +863,11 @@ public boolean applyPFRules(final Network network, final List<PortForwardingRule
854863
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
855864
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
856865

857-
if (!networkTopology.applyFirewallRules(network, rules, routers)) {
858-
throw new CloudRuntimeException("Failed to apply firewall rules in network " + network.getId());
859-
} else {
860-
return true;
866+
for (final DomainRouterVO domainRouterVO : routers) {
867+
result = result && networkTopology.applyFirewallRules(network, rules, domainRouterVO);
861868
}
862-
} else {
863-
return true;
864869
}
870+
return result;
865871
}
866872

867873
@Override
@@ -960,13 +966,13 @@ public boolean removeDhcpSupportForSubnet(final Network network) throws Resource
960966
@Override
961967
public boolean addDhcpEntry(final Network network, final NicProfile nic, final VirtualMachineProfile vm, final DeployDestination dest, final ReservationContext context)
962968
throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
969+
boolean result = true;
963970
if (canHandle(network, Service.Dhcp)) {
964971
if (vm.getType() != VirtualMachine.Type.User) {
965972
return false;
966973
}
967974

968975
final VirtualMachineProfile uservm = vm;
969-
970976
final List<DomainRouterVO> routers = getRouters(network, dest);
971977

972978
if (routers == null || routers.size() == 0) {
@@ -976,14 +982,17 @@ public boolean addDhcpEntry(final Network network, final NicProfile nic, final V
976982
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
977983
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
978984

979-
return networkTopology.applyDhcpEntry(network, nic, uservm, dest, routers);
985+
for (final DomainRouterVO domainRouterVO : routers) {
986+
result = result && networkTopology.applyDhcpEntry(network, nic, uservm, dest, domainRouterVO);
987+
}
980988
}
981-
return false;
989+
return result;
982990
}
983991

984992
@Override
985993
public boolean addPasswordAndUserdata(final Network network, final NicProfile nic, final VirtualMachineProfile vm, final DeployDestination dest,
986994
final ReservationContext context) throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
995+
boolean result = true;
987996
if (canHandle(network, Service.UserData)) {
988997
if (vm.getType() != VirtualMachine.Type.User) {
989998
return false;
@@ -1005,9 +1014,11 @@ public boolean addPasswordAndUserdata(final Network network, final NicProfile ni
10051014
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
10061015
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
10071016

1008-
return networkTopology.applyUserData(network, nic, uservm, dest, routers);
1017+
for (final DomainRouterVO domainRouterVO : routers) {
1018+
result = result && networkTopology.applyUserData(network, nic, uservm, dest, domainRouterVO);
1019+
}
10091020
}
1010-
return false;
1021+
return result;
10111022
}
10121023

10131024
protected List<DomainRouterVO> getRouters(final Network network, final DeployDestination dest) {

0 commit comments

Comments
 (0)