Merge "Improve the reconciling for VPNaaS"
This commit is contained in:
commit
58d8ce0546
@ -64,15 +64,20 @@ class VPNAgent(l3_extension.L3AgentExtension):
|
||||
if ri is not None:
|
||||
for device_driver in self.device_drivers:
|
||||
device_driver.create_router(ri)
|
||||
device_driver.sync(context, [ri.router])
|
||||
device_driver.sync(context, [ri])
|
||||
else:
|
||||
LOG.debug("Router %s was concurrently deleted while "
|
||||
"creating VPN for it", data['id'])
|
||||
|
||||
def update_router(self, context, data):
|
||||
"""Handles router update event"""
|
||||
for device_driver in self.device_drivers:
|
||||
device_driver.sync(context, [data])
|
||||
ri = self.agent_api.get_router_info(data['id'])
|
||||
if ri is not None:
|
||||
for device_driver in self.device_drivers:
|
||||
device_driver.sync(context, [ri])
|
||||
else:
|
||||
LOG.debug("Router %s was concurrently deleted while "
|
||||
"updating VPN for it", data['id'])
|
||||
|
||||
def delete_router(self, context, data):
|
||||
"""Handles router delete event"""
|
||||
|
@ -21,10 +21,12 @@ import re
|
||||
import shutil
|
||||
import socket
|
||||
import sys
|
||||
import typing as ty
|
||||
|
||||
import eventlet
|
||||
import jinja2
|
||||
import netaddr
|
||||
from neutron.agent.l3.router_info import RouterInfo
|
||||
from neutron.agent.linux import ip_lib
|
||||
from neutron.agent.linux import utils as agent_utils
|
||||
from neutron_lib.api import validators
|
||||
@ -839,7 +841,7 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
node_topic = '%s.%s' % (self.topic, self.host)
|
||||
|
||||
self.processes = {}
|
||||
self.routers = {}
|
||||
self.routers: ty.Dict[str, ty.Any] = {}
|
||||
self.process_status_cache = {}
|
||||
|
||||
self.endpoints = [self]
|
||||
@ -859,16 +861,16 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
Note: If the router is a DVR, then the SNAT namespace will be
|
||||
provided. If the router does not exist, return None.
|
||||
"""
|
||||
router = self.routers.get(router_id)
|
||||
if not router:
|
||||
router_info = self.routers.get(router_id)
|
||||
if not router_info:
|
||||
return
|
||||
# For DVR, use SNAT namespace
|
||||
# TODO(pcm): Use router object method to tell if DVR, when available
|
||||
if router.router['distributed']:
|
||||
return router.snat_namespace.name
|
||||
return router.ns_name
|
||||
if router_info.router['distributed']:
|
||||
return router_info.snat_namespace.name
|
||||
return router_info.ns_name
|
||||
|
||||
def get_router_based_iptables_manager(self, router):
|
||||
def get_router_based_iptables_manager(self, ri):
|
||||
"""Returns router based iptables manager
|
||||
|
||||
In DVR routers the IPsec VPN service should run inside
|
||||
@ -882,61 +884,28 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
return the legacy iptables_manager.
|
||||
"""
|
||||
# TODO(pcm): Use router object method to tell if DVR, when available
|
||||
if router.router['distributed']:
|
||||
return router.snat_iptables_manager
|
||||
return router.iptables_manager
|
||||
if ri.router['distributed']:
|
||||
return ri.snat_iptables_manager
|
||||
return ri.iptables_manager
|
||||
|
||||
def add_nat_rule(self, router_id, chain, rule, top=False):
|
||||
"""Add nat rule in namespace.
|
||||
def ensure_nat_rules(self, vpnservice):
|
||||
"""Ensure the required nat rules for ipsec exist in iptables.
|
||||
|
||||
:param router_id: router_id
|
||||
:param chain: a string of chain name
|
||||
:param rule: a string of rule
|
||||
:param top: if top is true, the rule
|
||||
will be placed on the top of chain
|
||||
Note if there is no router, this method does nothing
|
||||
"""
|
||||
router = self.routers.get(router_id)
|
||||
if not router:
|
||||
return
|
||||
iptables_manager = self.get_router_based_iptables_manager(router)
|
||||
iptables_manager.ipv4['nat'].add_rule(chain, rule, top=top)
|
||||
|
||||
def remove_nat_rule(self, router_id, chain, rule, top=False):
|
||||
"""Remove nat rule in namespace.
|
||||
|
||||
:param router_id: router_id
|
||||
:param chain: a string of chain name
|
||||
:param rule: a string of rule
|
||||
:param top: unused
|
||||
needed to have same argument with add_nat_rule
|
||||
"""
|
||||
router = self.routers.get(router_id)
|
||||
if not router:
|
||||
return
|
||||
iptables_manager = self.get_router_based_iptables_manager(router)
|
||||
iptables_manager.ipv4['nat'].remove_rule(chain, rule, top=top)
|
||||
|
||||
def iptables_apply(self, router_id):
|
||||
"""Apply IPtables.
|
||||
|
||||
:param router_id: router_id
|
||||
This method do nothing if there is no router
|
||||
"""
|
||||
router = self.routers.get(router_id)
|
||||
if not router:
|
||||
return
|
||||
iptables_manager = self.get_router_based_iptables_manager(router)
|
||||
iptables_manager.apply()
|
||||
|
||||
def _update_nat(self, vpnservice, func):
|
||||
"""Setting up nat rule in iptables.
|
||||
|
||||
We need to setup nat rule for ipsec packet.
|
||||
:param vpnservice: vpnservices
|
||||
:param func: self.add_nat_rule or self.remove_nat_rule
|
||||
"""
|
||||
router_id = vpnservice['router_id']
|
||||
LOG.debug("ensure_nat_rules called for router %s",
|
||||
vpnservice['router_id'])
|
||||
ri = self.routers.get(vpnservice['router_id'])
|
||||
if not ri:
|
||||
LOG.debug("No router info for router %s", vpnservice['router_id'])
|
||||
return
|
||||
|
||||
iptables_manager = self.get_router_based_iptables_manager(ri)
|
||||
# clear all existing rules first
|
||||
LOG.debug("Clearing vpnaas tagged NAT rules for router %s",
|
||||
ri.router_id)
|
||||
iptables_manager.ipv4['nat'].clear_rules_by_tag('vpnaas')
|
||||
|
||||
for ipsec_site_connection in vpnservice['ipsec_site_connections']:
|
||||
for local_cidr in ipsec_site_connection['local_cidrs']:
|
||||
# This ipsec rule is not needed for ipv6.
|
||||
@ -944,13 +913,33 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
continue
|
||||
|
||||
for peer_cidr in ipsec_site_connection['peer_cidrs']:
|
||||
func(router_id,
|
||||
'POSTROUTING',
|
||||
'-s %s -d %s -m policy '
|
||||
'--dir out --pol ipsec '
|
||||
'-j ACCEPT ' % (local_cidr, peer_cidr),
|
||||
top=True)
|
||||
self.iptables_apply(router_id)
|
||||
LOG.debug("Adding an ipsec policy NAT rule"
|
||||
"%s <-> %s to router id %s",
|
||||
peer_cidr, local_cidr, vpnservice['router_id'])
|
||||
|
||||
iptables_manager.ipv4['nat'].add_rule(
|
||||
'POSTROUTING',
|
||||
'-s %s -d %s -m policy '
|
||||
'--dir out --pol ipsec '
|
||||
'-j ACCEPT ' % (local_cidr, peer_cidr),
|
||||
top=True, tag='vpnaas')
|
||||
|
||||
LOG.debug("Applying iptables for router id %s",
|
||||
vpnservice['router_id'])
|
||||
iptables_manager.apply()
|
||||
|
||||
@log_helpers.log_method_call
|
||||
def remove_nat_rules(self, router_id):
|
||||
"""Remove all our iptables rules in namespace.
|
||||
|
||||
:param router_id: router_id
|
||||
"""
|
||||
router = self.routers.get(router_id)
|
||||
if not router:
|
||||
return
|
||||
iptables_manager = self.get_router_based_iptables_manager(router)
|
||||
iptables_manager.ipv4['nat'].clear_rules_by_tag('vpnaas')
|
||||
iptables_manager.apply()
|
||||
|
||||
@log_helpers.log_method_call
|
||||
def vpnservice_updated(self, context, **kwargs):
|
||||
@ -985,28 +974,28 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
process.update_vpnservice(vpnservice)
|
||||
return process
|
||||
|
||||
def create_router(self, router):
|
||||
def create_router(self, router_info: RouterInfo):
|
||||
"""Handling create router event.
|
||||
|
||||
Agent calls this method, when the process namespace is ready.
|
||||
Note: process_id == router_id == vpnservice_id
|
||||
"""
|
||||
process_id = router.router_id
|
||||
self.routers[process_id] = router
|
||||
process_id = router_info.router_id
|
||||
self.routers[process_id] = router_info
|
||||
if process_id in self.processes:
|
||||
# In case of vpnservice is created
|
||||
# before router's namespace
|
||||
process = self.processes[process_id]
|
||||
self._update_nat(process.vpnservice, self.add_nat_rule)
|
||||
self.ensure_nat_rules(process.vpnservice)
|
||||
# Don't run ipsec process for backup HA router
|
||||
if router.router['ha'] and router.ha_state == 'backup':
|
||||
if router_info.router['ha'] and router_info.ha_state == 'backup':
|
||||
return
|
||||
process.enable()
|
||||
|
||||
def destroy_process(self, process_id):
|
||||
"""Destroy process.
|
||||
|
||||
Disable the process, remove the nat rule, and remove the process
|
||||
Disable the process, remove the iptables rules, and remove the process
|
||||
manager for the processes that no longer are running vpn service.
|
||||
"""
|
||||
if process_id in self.processes:
|
||||
@ -1014,7 +1003,7 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
process.disable()
|
||||
vpnservice = process.vpnservice
|
||||
if vpnservice:
|
||||
self._update_nat(vpnservice, self.remove_nat_rule)
|
||||
self.remove_nat_rules(process_id)
|
||||
del self.processes[process_id]
|
||||
|
||||
def destroy_router(self, process_id):
|
||||
@ -1109,11 +1098,11 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
|
||||
@log_helpers.log_method_call
|
||||
@lockutils.synchronized('vpn-agent', 'neutron-')
|
||||
def sync(self, context, routers):
|
||||
def sync(self, context, router_information: ty.List):
|
||||
"""Sync status with server side.
|
||||
|
||||
:param context: context object for RPC call
|
||||
:param routers: Router objects which is created in this sync event
|
||||
:param router_information: RouterInfo objects with updated state
|
||||
|
||||
There could be many failure cases should be
|
||||
considered including the followings.
|
||||
@ -1128,7 +1117,22 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
vpnservices = self.agent_rpc.get_vpn_services_on_host(
|
||||
context, self.host)
|
||||
router_ids = [vpnservice['router_id'] for vpnservice in vpnservices]
|
||||
sync_router_ids = [router['id'] for router in routers]
|
||||
sync_router_ids = []
|
||||
|
||||
for ri in router_information:
|
||||
# Update our router_info with the updated one
|
||||
if isinstance(ri, RouterInfo):
|
||||
router_id = ri.router_id if ri.router_id else None
|
||||
elif isinstance(ri, dict):
|
||||
router_id = ri.get("router_id")
|
||||
else:
|
||||
router_id = None
|
||||
|
||||
if not router_id:
|
||||
continue
|
||||
|
||||
self.routers[router_id] = ri
|
||||
sync_router_ids.append(router_id)
|
||||
|
||||
self._sync_vpn_processes(vpnservices, sync_router_ids)
|
||||
self._delete_vpn_processes(sync_router_ids, router_ids)
|
||||
@ -1145,13 +1149,13 @@ class IPsecDriver(device_drivers.DeviceDriver, metaclass=abc.ABCMeta):
|
||||
vpnservice['router_id'] in sync_router_ids):
|
||||
process = self.ensure_process(vpnservice['router_id'],
|
||||
vpnservice=vpnservice)
|
||||
self._update_nat(vpnservice, self.add_nat_rule)
|
||||
router = self.routers.get(vpnservice['router_id'])
|
||||
if not router:
|
||||
self.ensure_nat_rules(vpnservice)
|
||||
ri = self.routers.get(vpnservice['router_id'])
|
||||
if not ri:
|
||||
continue
|
||||
# For HA router, spawn vpn process on master router
|
||||
# and terminate vpn process on backup router
|
||||
if router.router['ha'] and router.ha_state == 'backup':
|
||||
if ri.router['ha'] and ri.ha_state == 'backup':
|
||||
process.disable()
|
||||
else:
|
||||
process.update()
|
||||
|
@ -13,10 +13,8 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
import os
|
||||
import os.path
|
||||
|
||||
from neutron.agent.linux import ip_lib
|
||||
|
||||
from neutron_vpnaas.services.vpn.device_drivers import ipsec
|
||||
|
||||
|
||||
|
@ -523,8 +523,8 @@ class TestIPSecBase(framework.L3AgentTestFramework):
|
||||
|
||||
local_router_id = site1.router.router_id
|
||||
peer_router_id = site2.router.router_id
|
||||
self.driver.sync(mock.Mock(), [{'id': local_router_id},
|
||||
{'id': peer_router_id}])
|
||||
self.driver.sync(mock.Mock(), [site1.router,
|
||||
site2.router])
|
||||
self.agent._process_updated_router(site1.router.router)
|
||||
self.agent._process_updated_router(site2.router.router)
|
||||
self.addCleanup(self.driver._delete_vpn_processes,
|
||||
@ -534,8 +534,7 @@ class TestIPSecBase(framework.L3AgentTestFramework):
|
||||
"""Perform a sync on failover agent associated w/backup router."""
|
||||
self.failover_driver.agent_rpc.get_vpn_services_on_host = mock.Mock(
|
||||
return_value=[site.vpn_service])
|
||||
self.failover_driver.sync(mock.Mock(),
|
||||
[{'id': site.backup_router.router_id}])
|
||||
self.failover_driver.sync(mock.Mock(), [site.router])
|
||||
|
||||
def check_ping(self, from_site, to_site, instance=0, success=True):
|
||||
if success:
|
||||
|
@ -35,12 +35,16 @@ from neutron_vpnaas.services.vpn.device_drivers import libreswan_ipsec
|
||||
from neutron_vpnaas.services.vpn.device_drivers import strongswan_ipsec
|
||||
from neutron_vpnaas.tests import base
|
||||
|
||||
# Note: process_id == router_id == vpnservice_id
|
||||
|
||||
_uuid = uuidutils.generate_uuid
|
||||
FAKE_UUID = _uuid()
|
||||
FAKE_HOST = 'fake_host'
|
||||
FAKE_ROUTER_ID = _uuid()
|
||||
FAKE_ROUTER_ID = FAKE_UUID
|
||||
FAKE_VPNSERVICE_ID = FAKE_UUID
|
||||
FAKE_PROCESS_ID = FAKE_UUID
|
||||
FAKE_IPSEC_SITE_CONNECTION1_ID = _uuid()
|
||||
FAKE_IPSEC_SITE_CONNECTION2_ID = _uuid()
|
||||
FAKE_VPNSERVICE_ID = _uuid()
|
||||
FAKE_IKE_POLICY = {
|
||||
'ike_version': 'v1',
|
||||
'encryption_algorithm': 'aes-128',
|
||||
@ -431,13 +435,13 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
self._make_router_info_for_test()
|
||||
|
||||
def _make_router_info_for_test(self):
|
||||
self.router = legacy_router.LegacyRouter(router_id=FAKE_ROUTER_ID,
|
||||
self.router_info = legacy_router.LegacyRouter(router_id=FAKE_ROUTER_ID,
|
||||
agent=self.agent,
|
||||
**self.ri_kwargs)
|
||||
self.router.router['distributed'] = False
|
||||
self.router.iptables_manager.ipv4['nat'] = self.iptables
|
||||
self.router.iptables_manager.apply = self.apply_mock
|
||||
self.driver.routers[FAKE_ROUTER_ID] = self.router
|
||||
self.router_info.router['distributed'] = False
|
||||
self.router_info.iptables_manager.ipv4['nat'] = self.iptables
|
||||
self.router_info.iptables_manager.apply = self.apply_mock
|
||||
self.driver.routers[FAKE_ROUTER_ID] = self.router_info
|
||||
|
||||
def _test_vpnservice_updated(self, expected_param, **kwargs):
|
||||
with mock.patch.object(self.driver, 'sync') as sync:
|
||||
@ -449,17 +453,16 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
self._test_vpnservice_updated([])
|
||||
|
||||
def test_vpnservice_updated_with_router_info(self):
|
||||
router_info = {'id': FAKE_ROUTER_ID, 'ha': False}
|
||||
kwargs = {'router': router_info}
|
||||
self._test_vpnservice_updated([router_info], **kwargs)
|
||||
kwargs = {'router': self.router_info}
|
||||
self._test_vpnservice_updated([self.router_info], **kwargs)
|
||||
|
||||
def test_create_router(self):
|
||||
process = mock.Mock(openswan_ipsec.OpenSwanProcess)
|
||||
process.vpnservice = self.vpnservice
|
||||
self.driver.processes = {
|
||||
FAKE_ROUTER_ID: process}
|
||||
self.driver.create_router(self.router)
|
||||
self._test_add_nat_rule()
|
||||
self.driver.create_router(self.router_info)
|
||||
self._test_ensure_nat_rules()
|
||||
process.enable.assert_called_once_with()
|
||||
|
||||
def test_destroy_router(self):
|
||||
@ -472,75 +475,89 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
process.disable.assert_called_once_with()
|
||||
self.assertNotIn(process_id, self.driver.processes)
|
||||
|
||||
def _test_add_nat_rule(self):
|
||||
self.router.iptables_manager.ipv4['nat'].assert_has_calls([
|
||||
def _test_ensure_nat_rules(self):
|
||||
self.router_info.iptables_manager.ipv4['nat'].assert_has_calls([
|
||||
mock.call.clear_rules_by_tag('vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 10.0.0.0/24 -d 20.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 10.0.0.0/24 -d 30.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 11.0.0.0/24 -d 40.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 11.0.0.0/24 -d 50.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True)
|
||||
top=True,
|
||||
tag='vpnaas')
|
||||
])
|
||||
self.router.iptables_manager.apply.assert_called_once_with()
|
||||
self.router_info.iptables_manager.apply.assert_called_once_with()
|
||||
|
||||
def _test_add_nat_rule_with_multiple_locals(self):
|
||||
self.router.iptables_manager.ipv4['nat'].assert_has_calls([
|
||||
def _test_ensure_nat_rules_with_multiple_locals(self):
|
||||
self.router_info.iptables_manager.ipv4['nat'].assert_has_calls([
|
||||
mock.call.clear_rules_by_tag('vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 10.0.0.0/24 -d 20.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 10.0.0.0/24 -d 30.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 11.0.0.0/24 -d 20.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 11.0.0.0/24 -d 30.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 12.0.0.0/24 -d 40.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 12.0.0.0/24 -d 50.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 13.0.0.0/24 -d 40.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True),
|
||||
top=True,
|
||||
tag='vpnaas'),
|
||||
mock.call.add_rule(
|
||||
'POSTROUTING',
|
||||
'-s 13.0.0.0/24 -d 50.0.0.0/24 -m policy '
|
||||
'--dir out --pol ipsec -j ACCEPT ',
|
||||
top=True)
|
||||
top=True,
|
||||
tag='vpnaas')
|
||||
])
|
||||
self.router.iptables_manager.apply.assert_called_once_with()
|
||||
self.router_info.iptables_manager.apply.assert_called_once_with()
|
||||
|
||||
def test_sync(self):
|
||||
fake_vpn_service = FAKE_VPN_SERVICE
|
||||
@ -550,9 +567,8 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
self.driver._sync_vpn_processes = mock.Mock()
|
||||
self.driver._delete_vpn_processes = mock.Mock()
|
||||
self.driver._cleanup_stale_vpn_processes = mock.Mock()
|
||||
sync_routers = [{'id': fake_vpn_service['router_id']}]
|
||||
sync_router_ids = [fake_vpn_service['router_id']]
|
||||
self.driver.sync(context, sync_routers)
|
||||
self.driver.sync(context, [self.router_info])
|
||||
self.driver._sync_vpn_processes.assert_called_once_with(
|
||||
[fake_vpn_service], sync_router_ids)
|
||||
self.driver._delete_vpn_processes.assert_called_once_with(
|
||||
@ -567,16 +583,16 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
with mock.patch.object(self.driver, 'ensure_process') as ensure_p:
|
||||
ensure_p.side_effect = self.fake_ensure_process
|
||||
self.driver._sync_vpn_processes([new_vpnservice], router_id)
|
||||
self._test_add_nat_rule()
|
||||
self._test_ensure_nat_rules()
|
||||
self.driver.processes[router_id].update.assert_called_once_with()
|
||||
|
||||
def test_add_nat_rules_with_multiple_local_subnets(self):
|
||||
def test_ensure_nat_rules_with_multiple_local_subnets(self):
|
||||
"""Ensure that add nat rule combinations are correct."""
|
||||
overrides = {'local_cidrs': [['10.0.0.0/24', '11.0.0.0/24'],
|
||||
['12.0.0.0/24', '13.0.0.0/24']]}
|
||||
self.modify_config_for_test(overrides)
|
||||
self.driver._update_nat(self.vpnservice, self.driver.add_nat_rule)
|
||||
self._test_add_nat_rule_with_multiple_locals()
|
||||
self.driver.ensure_nat_rules(self.vpnservice)
|
||||
self._test_ensure_nat_rules_with_multiple_locals()
|
||||
|
||||
def test__sync_vpn_processes_router_with_no_vpn(self):
|
||||
"""Test _sync_vpn_processes with a router not hosting vpnservice.
|
||||
@ -613,14 +629,18 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
is updated, _sync_vpn_processes restart/update the existing vpnservices
|
||||
which are not yet stored in driver.processes.
|
||||
"""
|
||||
router_id = FAKE_ROUTER_ID
|
||||
self.driver.process_status_cache = {}
|
||||
self.driver.processes = {}
|
||||
with mock.patch.object(self.driver, 'ensure_process') as ensure_p:
|
||||
ensure_p.side_effect = self.fake_ensure_process
|
||||
self.driver._sync_vpn_processes([self.vpnservice], [router_id])
|
||||
self._test_add_nat_rule()
|
||||
self.driver.processes[router_id].update.assert_called_once_with()
|
||||
self.driver._sync_vpn_processes(
|
||||
[self.vpnservice],
|
||||
[FAKE_ROUTER_ID]
|
||||
)
|
||||
self._test_ensure_nat_rules()
|
||||
self.driver.processes[
|
||||
FAKE_ROUTER_ID
|
||||
].update.assert_called_once_with()
|
||||
|
||||
def test_delete_vpn_processes(self):
|
||||
router_id_no_vpn = _uuid()
|
||||
@ -671,6 +691,8 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
if process:
|
||||
del self.driver.processes[process_id]
|
||||
|
||||
# TODO(crohmann): Add test cases for HARouter and different ha_states
|
||||
# @ddt [(False, None),(True, 'primary'), (True, 'standby')]
|
||||
def test_sync_update_vpnservice(self):
|
||||
with mock.patch.object(self.driver,
|
||||
'ensure_process') as ensure_process:
|
||||
@ -683,12 +705,12 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
self.driver.process_status_cache = {}
|
||||
self.driver.agent_rpc.get_vpn_services_on_host.return_value = [
|
||||
new_vpn_service]
|
||||
self.driver.sync(context, [{'id': FAKE_ROUTER_ID}])
|
||||
self.driver.sync(context, [self.router_info])
|
||||
process = self.driver.processes[FAKE_ROUTER_ID]
|
||||
self.assertEqual(new_vpn_service, process.vpnservice)
|
||||
self.driver.agent_rpc.get_vpn_services_on_host.return_value = [
|
||||
updated_vpn_service]
|
||||
self.driver.sync(context, [{'id': FAKE_ROUTER_ID}])
|
||||
self.driver.sync(context, [self.router_info])
|
||||
process = self.driver.processes[FAKE_ROUTER_ID]
|
||||
process.update_vpnservice.assert_called_once_with(
|
||||
updated_vpn_service)
|
||||
@ -710,7 +732,10 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
self.driver.agent_rpc.get_vpn_services_on_host.return_value = []
|
||||
context = mock.Mock()
|
||||
process_id = _uuid()
|
||||
self.driver.sync(context, [{'id': process_id}])
|
||||
ri = self.router_info
|
||||
ri.router_id = process_id
|
||||
ri.router['id'] = process_id
|
||||
self.driver.sync(context, [self.router_info])
|
||||
self.assertNotIn(process_id, self.driver.processes)
|
||||
|
||||
def test_status_updated_on_connection_admin_down(self):
|
||||
@ -781,7 +806,7 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
|
||||
def _test_status_handling_for_downed_connection(self, down_status):
|
||||
"""Test status handling for downed connection."""
|
||||
router_id = self.router.router_id
|
||||
router_id = self.router_info.router_id
|
||||
connection_id = FAKE_IPSEC_SITE_CONNECTION2_ID
|
||||
self.driver.ensure_process(router_id, self.vpnservice)
|
||||
self._execute.return_value = down_status
|
||||
@ -794,7 +819,7 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
|
||||
def _test_status_handling_for_active_connection(self, active_status):
|
||||
"""Test status handling for active connection."""
|
||||
router_id = self.router.router_id
|
||||
router_id = self.router_info.router_id
|
||||
connection_id = FAKE_IPSEC_SITE_CONNECTION2_ID
|
||||
self.driver.ensure_process(router_id, self.vpnservice)
|
||||
self._execute.return_value = active_status
|
||||
@ -809,7 +834,7 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
def _test_status_handling_for_ike_v2_active_connection(self,
|
||||
active_status):
|
||||
"""Test status handling for active connection."""
|
||||
router_id = self.router.router_id
|
||||
router_id = self.router_info.router_id
|
||||
connection_id = FAKE_IPSEC_SITE_CONNECTION2_ID
|
||||
ike_policy = {'ike_version': 'v2',
|
||||
'encryption_algorithm': 'aes-128',
|
||||
@ -832,7 +857,7 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
def _test_connection_names_handling_for_multiple_subnets(self,
|
||||
active_status):
|
||||
"""Test connection names handling for multiple subnets."""
|
||||
router_id = self.router.router_id
|
||||
router_id = self.router_info.router_id
|
||||
process = self.driver.ensure_process(router_id, self.vpnservice)
|
||||
self._execute.return_value = active_status
|
||||
names = process.get_established_connections()
|
||||
@ -841,7 +866,7 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
def _test_status_handling_for_deleted_connection(self,
|
||||
not_running_status):
|
||||
"""Test status handling for deleted connection."""
|
||||
router_id = self.router.router_id
|
||||
router_id = self.router_info.router_id
|
||||
self.driver.ensure_process(router_id, self.vpnservice)
|
||||
self._execute.return_value = not_running_status
|
||||
self.driver.report_status(mock.Mock())
|
||||
@ -853,7 +878,7 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
def _test_parse_connection_status(self, not_running_status,
|
||||
active_status, down_status):
|
||||
"""Test the status of ipsec-site-connection is parsed correctly."""
|
||||
router_id = self.router.router_id
|
||||
router_id = self.router_info.router_id
|
||||
process = self.driver.ensure_process(router_id, self.vpnservice)
|
||||
self._execute.return_value = not_running_status
|
||||
self.assertFalse(process.active)
|
||||
@ -873,41 +898,6 @@ class IPSecDeviceLegacy(BaseIPsecDeviceDriver):
|
||||
def test_fail_getting_namespace_for_unknown_router(self):
|
||||
self.assertFalse(self.driver.get_namespace('bogus_id'))
|
||||
|
||||
def test_add_nat_rule(self):
|
||||
self.driver.add_nat_rule(FAKE_ROUTER_ID, 'fake_chain',
|
||||
'fake_rule', True)
|
||||
self.iptables.add_rule.assert_called_once_with(
|
||||
'fake_chain', 'fake_rule', top=True)
|
||||
|
||||
def test_add_nat_rule_with_no_router(self):
|
||||
self.driver.add_nat_rule(
|
||||
'bogus_router_id',
|
||||
'fake_chain',
|
||||
'fake_rule',
|
||||
True)
|
||||
self.assertFalse(self.iptables.add_rule.called)
|
||||
|
||||
def test_remove_rule(self):
|
||||
self.driver.remove_nat_rule(FAKE_ROUTER_ID, 'fake_chain',
|
||||
'fake_rule', True)
|
||||
self.iptables.remove_rule.assert_called_once_with(
|
||||
'fake_chain', 'fake_rule', top=True)
|
||||
|
||||
def test_remove_rule_with_no_router(self):
|
||||
self.driver.remove_nat_rule(
|
||||
'bogus_router_id',
|
||||
'fake_chain',
|
||||
'fake_rule')
|
||||
self.assertFalse(self.iptables.remove_rule.called)
|
||||
|
||||
def test_iptables_apply(self):
|
||||
self.driver.iptables_apply(FAKE_ROUTER_ID)
|
||||
self.apply_mock.assert_called_once_with()
|
||||
|
||||
def test_iptables_apply_with_no_router(self):
|
||||
self.driver.iptables_apply('bogus_router_id')
|
||||
self.assertFalse(self.apply_mock.called)
|
||||
|
||||
|
||||
class IPSecDeviceDVR(BaseIPsecDeviceDriver):
|
||||
|
||||
@ -918,21 +908,23 @@ class IPSecDeviceDVR(BaseIPsecDeviceDriver):
|
||||
self._make_dvr_edge_router_info_for_test()
|
||||
|
||||
def _make_dvr_edge_router_info_for_test(self):
|
||||
router = dvr_edge_router.DvrEdgeRouter(mock.sentinel.agent,
|
||||
router_info = dvr_edge_router.DvrEdgeRouter(mock.sentinel.agent,
|
||||
mock.sentinel.myhost,
|
||||
FAKE_ROUTER_ID,
|
||||
**self.ri_kwargs)
|
||||
router.router['distributed'] = True
|
||||
router.snat_namespace = dvr_snat_ns.SnatNamespace(router.router['id'],
|
||||
mock.sentinel.agent,
|
||||
self.driver,
|
||||
mock.ANY)
|
||||
router.snat_namespace.create()
|
||||
router.snat_iptables_manager = iptables_manager.IptablesManager(
|
||||
router_info.router['distributed'] = True
|
||||
router_info.snat_namespace = dvr_snat_ns.SnatNamespace(
|
||||
router_info.router['id'],
|
||||
mock.sentinel.agent,
|
||||
self.driver,
|
||||
mock.ANY
|
||||
)
|
||||
router_info.snat_namespace.create()
|
||||
router_info.snat_iptables_manager = iptables_manager.IptablesManager(
|
||||
namespace='snat-' + FAKE_ROUTER_ID, use_ipv6=mock.ANY)
|
||||
router.snat_iptables_manager.ipv4['nat'] = self.iptables
|
||||
router.snat_iptables_manager.apply = self.apply_mock
|
||||
self.driver.routers[FAKE_ROUTER_ID] = router
|
||||
router_info.snat_iptables_manager.ipv4['nat'] = self.iptables
|
||||
router_info.snat_iptables_manager.apply = self.apply_mock
|
||||
self.driver.routers[FAKE_ROUTER_ID] = router_info
|
||||
|
||||
def test_sync_dvr(self):
|
||||
fake_vpn_service = FAKE_VPN_SERVICE
|
||||
@ -942,11 +934,10 @@ class IPSecDeviceDVR(BaseIPsecDeviceDriver):
|
||||
self.driver._sync_vpn_processes = mock.Mock()
|
||||
self.driver._delete_vpn_processes = mock.Mock()
|
||||
self.driver._cleanup_stale_vpn_processes = mock.Mock()
|
||||
sync_routers = [{'id': fake_vpn_service['router_id']}]
|
||||
sync_router_ids = [fake_vpn_service['router_id']]
|
||||
with mock.patch.object(self.driver,
|
||||
'get_process_status_cache') as process_status:
|
||||
self.driver.sync(context, sync_routers)
|
||||
self.driver.sync(context, [self.driver.routers[FAKE_ROUTER_ID]])
|
||||
self.driver._sync_vpn_processes.assert_called_once_with(
|
||||
[fake_vpn_service], sync_router_ids)
|
||||
self.driver._delete_vpn_processes.assert_called_once_with(
|
||||
@ -959,22 +950,10 @@ class IPSecDeviceDVR(BaseIPsecDeviceDriver):
|
||||
namespace = self.driver.get_namespace(FAKE_ROUTER_ID)
|
||||
self.assertEqual('snat-' + FAKE_ROUTER_ID, namespace)
|
||||
|
||||
def test_add_nat_rule_with_dvr_edge_router(self):
|
||||
self.driver.add_nat_rule(FAKE_ROUTER_ID, 'fake_chain',
|
||||
'fake_rule', True)
|
||||
self.iptables.add_rule.assert_called_once_with(
|
||||
'fake_chain', 'fake_rule', top=True)
|
||||
|
||||
def test_iptables_apply_with_dvr_edge_router(self):
|
||||
self.driver.iptables_apply(FAKE_ROUTER_ID)
|
||||
def test_ensure_nat_rules_with_dvr_edge_router(self):
|
||||
self.driver.ensure_nat_rules(FAKE_VPN_SERVICE)
|
||||
self.apply_mock.assert_called_once_with()
|
||||
|
||||
def test_remove_rule_with_dvr_edge_router(self):
|
||||
self.driver.remove_nat_rule(FAKE_ROUTER_ID, 'fake_chain',
|
||||
'fake_rule', True)
|
||||
self.iptables.remove_rule.assert_called_once_with(
|
||||
'fake_chain', 'fake_rule', top=True)
|
||||
|
||||
|
||||
class TestOpenSwanConfigGeneration(BaseIPsecDeviceDriver):
|
||||
|
||||
@ -1303,7 +1282,7 @@ class TestOpenSwanProcess(IPSecDeviceLegacy):
|
||||
'updated_pending_status': True}})
|
||||
|
||||
self.assertRaises(vpn_exception.VPNPeerAddressNotResolved,
|
||||
self.process._get_nexthop, 'foo.peer.addr',
|
||||
self.process._get_nexthop, 'foo.peer.addr.',
|
||||
'fake-conn-id')
|
||||
self.assertEqual(expected_connection_status_dict,
|
||||
self.process.connection_status)
|
||||
@ -1313,7 +1292,7 @@ class TestOpenSwanProcess(IPSecDeviceLegacy):
|
||||
'updated_pending_status': False}})
|
||||
|
||||
self.assertRaises(vpn_exception.VPNPeerAddressNotResolved,
|
||||
self.process._get_nexthop, 'foo.peer.addr',
|
||||
self.process._get_nexthop, 'foo.peer.addr.',
|
||||
'fake-conn-id')
|
||||
self.assertEqual(expected_connection_status_dict,
|
||||
self.process.connection_status)
|
||||
|
@ -197,7 +197,7 @@ class TestOvnStrongSwanDriver(test_ipsec.IPSecDeviceLegacy):
|
||||
'transit_gateway_ip': '192.168.1.1',
|
||||
}
|
||||
|
||||
def test_iptables_apply(self):
|
||||
def test_ensure_nat_rules(self):
|
||||
"""Not applicable for OvnIPsecDriver"""
|
||||
pass
|
||||
|
||||
@ -218,19 +218,11 @@ class TestOvnStrongSwanDriver(test_ipsec.IPSecDeviceLegacy):
|
||||
"""Not applicable for OvnIPsecDriver"""
|
||||
pass
|
||||
|
||||
def test_remove_rule(self):
|
||||
def test_ensure_nat_rules_with_multiple_local_subnets(self):
|
||||
"""Not applicable for OvnIPsecDriver"""
|
||||
pass
|
||||
|
||||
def test_add_nat_rules_with_multiple_local_subnets(self):
|
||||
"""Not applicable for OvnIPsecDriver"""
|
||||
pass
|
||||
|
||||
def _test_add_nat_rule(self):
|
||||
"""Not applicable for OvnIPsecDriver"""
|
||||
pass
|
||||
|
||||
def test_add_nat_rule(self):
|
||||
def _test_ensure_nat_rules(self):
|
||||
"""Not applicable for OvnIPsecDriver"""
|
||||
pass
|
||||
|
||||
|
11
releasenotes/notes/bug1943449-899ba4711ff3586e.yaml
Normal file
11
releasenotes/notes/bug1943449-899ba4711ff3586e.yaml
Normal file
@ -0,0 +1,11 @@
|
||||
---
|
||||
prelude: >
|
||||
Due to an change in the IPtables NAT rule format, with the tag "vpnaas"
|
||||
upgrading to this release requires either a machine reboot or a move of
|
||||
all routers from this agent to ensure there is rules of the old format left.
|
||||
fixes:
|
||||
- |
|
||||
Reconciling via the sync method has been improved to ensure no
|
||||
`ha_state_change` event was missed.
|
||||
Also all IPtables NAT rules are now tagged "vpnaas" and refreshed on sync
|
||||
to ensure they are current and there are no duplicates.
|
Loading…
Reference in New Issue
Block a user