DaanHoogland commented on a change in pull request #4953:
URL: https://github.com/apache/cloudstack/pull/4953#discussion_r623872505
##########
File path: systemvm/debian/opt/cloud/bin/ipsectunnel.sh
##########
@@ -163,6 +170,20 @@ ipsec_tunnel_add() {
sudo echo " dpdaction=restart" >> $vpnconffile
fi
+ if [ $splitconnections -eq 1 ]
+ then
+ # Split out all but the first right subnet into their own statements
+ subnetidx=2
+ for rsubnet in ${rsubnetarr[@]:1}; do
+ sudo echo "" >> $vpnconffile &&
+ sudo echo "conn vpn-$rightpeer-$subnetidx" >> $vpnconffile &&
+ sudo echo " also=vpn-$rightpeer" >> $vpnconffile &&
+ sudo echo " auto=route" >> $vpnconffile &&
+ sudo echo " rightsubnet=$rsubnet" >> $vpnconffile
+ ((++subnetidx))
+ done
+ fi
Review comment:
separate method, please?
##########
File path: ui/public/locales/pl.json
##########
@@ -828,6 +828,7 @@
"label.icmpcode": "ICMP Code",
"label.icmptype": "ICMP Type",
"label.id": "ID",
+"label.ike.version": "IKE Version",
Review comment:
polish version contains mostly english. I'm not sure if we should add to
that.
##########
File path: systemvm/debian/opt/cloud/bin/ipsectunnel.sh
##########
@@ -139,14 +139,21 @@ ipsec_tunnel_add() {
check_and_enable_iptables
+ rsubnets=" rightsubnets={$rightnets}"
+ if [ $splitconnections -eq 1 ]
+ then
+ rsubnetarr=(${rightnets})
+ rsubnets=" rightsubnet=${rsubnetarr[0]}"
+ fi
Review comment:
separate method please?
##########
File path: systemvm/debian/opt/cloud/bin/configure.py
##########
@@ -582,6 +590,14 @@ def configure_ipsec(self, obj):
file.addeq(" dpddelay=30")
file.addeq(" dpdtimeout=120")
file.addeq(" dpdaction=restart")
+ if splitconnections and peerlistarr.count > 1:
+ logging.debug('Splitting connections for rightsubnets %s' %
peerlistarr)
+ for peeridx in range(1, len(peerlistarr)):
+ logging.debug('Adding split connection -%d for subnet %s' %
(peeridx + 1, peerlistarr[peeridx]))
+ file.append('')
+ file.search('conn vpn-.*-%d' % (peeridx + 1), "conn vpn-%s-%d"
% (rightpeer, peeridx + 1))
+ file.append(' also=vpn-%s' % rightpeer)
+ file.append(' rightsubnet=%s' % peerlistarr[peeridx])
Review comment:
extra method, please?
##########
File path: systemvm/debian/opt/cloud/bin/configure.py
##########
@@ -556,10 +556,18 @@ def configure_ipsec(self, obj):
vpnsecretsfile = "%s/ipsec.vpn-%s.secrets" % (self.VPNCONFDIR,
rightpeer)
ikepolicy = obj['ike_policy'].replace(';', '-')
esppolicy = obj['esp_policy'].replace(';', '-')
+ splitconnections = obj['split_connections'] if 'split_connections' in
obj else False
+ ikeversion = obj['ike_version'] if 'ike_version' in obj and
obj['ike_version'].lower() in ('ike', 'ikev1', 'ikev2') else 'ike'
+
+ peerlistarr = peerlist.split(',')
+ if splitconnections:
+ logging.debug('Splitting rightsubnets %s' % peerlistarr)
+ peerlist = peerlistarr[0]
Review comment:
separate method
##########
File path: systemvm/debian/opt/cloud/bin/configure.py
##########
@@ -595,14 +611,25 @@ def configure_ipsec(self, obj):
os.chmod(vpnsecretsfile, 0400)
for i in xrange(3):
- result = CsHelper.execute('ipsec status vpn-%s | grep "%s"' %
(rightpeer, peerlist.split(",", 1)[0]))
- if len(result) > 0:
+ done = True
+ for peeridx in range(0, len(peerlistarr)):
+ # Check for the proper connection and subnet
+ conn = rightpeer if not splitconnections else rightpeer if
peeridx == 0 else '%s-%d' % (rightpeer, peeridx + 1)
+ result = CsHelper.execute('ipsec status vpn-%s | grep "%s"' %
(conn, peerlistarr[peeridx]))
+ # If any of the peers hasn't yet finished, continue the outer
loop
+ if len(result) == 0:
+ done = False
+ if done:
Review comment:
separate out please?
##########
File path: systemvm/debian/opt/cloud/bin/configure.py
##########
@@ -595,14 +611,25 @@ def configure_ipsec(self, obj):
os.chmod(vpnsecretsfile, 0400)
for i in xrange(3):
- result = CsHelper.execute('ipsec status vpn-%s | grep "%s"' %
(rightpeer, peerlist.split(",", 1)[0]))
- if len(result) > 0:
+ done = True
+ for peeridx in range(0, len(peerlistarr)):
+ # Check for the proper connection and subnet
+ conn = rightpeer if not splitconnections else rightpeer if
peeridx == 0 else '%s-%d' % (rightpeer, peeridx + 1)
+ result = CsHelper.execute('ipsec status vpn-%s | grep "%s"' %
(conn, peerlistarr[peeridx]))
+ # If any of the peers hasn't yet finished, continue the outer
loop
+ if len(result) == 0:
+ done = False
+ if done:
break
time.sleep(1)
# With 'auto=route', connections are established on an attempt to
# communicate over the S2S VPN. This uses ping to initialize the
connection.
- CsHelper.execute("timeout 5 ping -c 3 %s" % (peerlist.split("/",
1)[0].replace(".0", ".1")))
+ for peer in peerlistarr:
+ octets = peer.split('/', 1)[0].split('.')
+ octets[3] = str((int(octets[3]) + 1))
+ ipinsubnet = '.'.join(octets)
+ CsHelper.execute("timeout 5 ping -c 3 %s" % ipinsubnet)
Review comment:
factor out in a separate methos, please?
##########
File path:
server/src/main/java/com/cloud/network/vpn/Site2SiteVpnManagerImpl.java
##########
@@ -494,7 +501,40 @@ public Site2SiteCustomerGateway
updateCustomerGateway(UpdateVpnCustomerGatewayCm
gw.setEspLifetime(espLifetime);
gw.setDpd(dpd);
gw.setEncap(encap);
+ gw.setSplitConnections(splitConnections);
+ if (ikeVersion != null) {
+ gw.setIkeVersion(ikeVersion);
+ }
_customerGatewayDao.persist(gw);
+
+ List<Site2SiteVpnConnectionVO> conns =
_vpnConnectionDao.listByCustomerGatewayId(id);
+ if (conns != null) {
+ for (Site2SiteVpnConnection conn : conns) {
+ try {
+ _accountMgr.checkAccess(caller, null, false, conn);
+ } catch (PermissionDeniedException e) {
+ // Just don't restart this connection, as the user has no
rights to it
+ // Maybe should issue a notification to the system?
+ s_logger.info("Site2SiteVpnManager:updateCustomerGateway()
Not resetting VPN connection " + conn.getId() + " as user lacks permission");
+ continue;
+ }
+
+ if (conn.getState() == State.Pending) {
+ // Vpn connection cannot be reset when the state is Pending
+ continue;
+ }
+ try {
+ if (conn.getState() == State.Connected || conn.getState()
== State.Error) {
+ stopVpnConnection(conn.getId());
+ }
+ startVpnConnection(conn.getId());
+ } catch (ResourceUnavailableException e) {
+ // Should never get here, as we are looping on the actual
connections, but we must handle it regardless
+ continue;
+ }
+ }
+ }
Review comment:
extra nested methods
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
