openstack/neutron-vpnaas
Code Issues Proposed changes

Improve the reconciling for VPNaaS

Browse Source 
* Update the router (holding RouterInfo objects) with current data
   on update / sync. Since the sync method should reconcile out of
   any state, we need to update the RouterInfo we store locally in
   the driver to ensure we have not missed e.g. a ha_state_change.

 * Consistently use RouterInfo instead of some mix of dict and Router
   and RouterInfo.

 * Ensure NAT rules are current by using a tag to clean them all and
   then re-create the currently required rules before applying them via
   iptables manager. This ensures there are no dangling rules or duplicates.

Co-Authored-By: Niklas Schwarz <niklas.schwarz@inovex.de>
Closes-Bug: https://bugs.launchpad.net/neutron/+bug/1943449

Change-Id: I378ba5a0b500110ce5f9293a885730c0a62578b0
This commit is contained in:
Christian Rohmann 2022-11-08 10:41:05 +01:00 committed by Niklas Schwarz
parent 6e40758925
commit 7c2018b6fb
7 changed files with 200 additions and 212 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
neutron_vpnaas/services/vpn/agent.py
View File

@ -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"""

162
neutron_vpnaas/services/vpn/device_drivers/ipsec.py
View File

@ -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
@ -834,7 +836,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]
@ -854,16 +856,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
@ -877,61 +879,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.
@ -939,13 +908,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):
@ -980,28 +969,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:
@ -1009,7 +998,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):
@ -1104,11 +1093,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.
@ -1123,7 +1112,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)
@ -1140,13 +1144,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()

2
neutron_vpnaas/services/vpn/device_drivers/libreswan_ipsec.py
View File

@ -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

7
neutron_vpnaas/tests/functional/common/test_scenario.py
View File

@ -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:

205
neutron_vpnaas/tests/unit/services/vpn/device_drivers/test_ipsec.py
View File

@ -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):
@ -1293,7 +1272,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)
@ -1303,7 +1282,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)

14
neutron_vpnaas/tests/unit/services/vpn/device_drivers/test_ovn_ipsec.py
View File

@ -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
View 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.