x/vmware-nsx
Code Issues Proposed changes

NSX|V3: Fix VPN service deletion

Browse Source 
1. Also delete local endpoints if still exists
2. Delete the NSX service if there is no other vpn service on tier1
connected to the same tier0
3. Add some logging to help future debugging

Change-Id: Icb051009d858800675634e0e22a64dc59df3bd0a
This commit is contained in:
Adit Sarfaty 2019-05-29 10:41:49 +03:00
parent c696ef4d68
commit 2d68de6ce4
2 changed files with 57 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py
View File

@ -356,7 +356,7 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver):
if ep_list['results']: if ep_list['results']:
return ep_list['results'][0]['id'] return ep_list['results'][0]['id']
def _get_local_endpoint(self, context, connection, vpnservice): def _get_local_endpoint(self, context, vpnservice):
"""Get the id of the local endpoint for a service """Get the id of the local endpoint for a service
The NSX allows only one local endpoint per local address The NSX allows only one local endpoint per local address
@ -386,18 +386,22 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver):
if ports: if ports:
return ports[0] return ports[0]
def _delete_local_endpoint(self, resource, event, trigger, **kwargs): def _delete_local_endpoint_by_router(self, context, router_id):
"""Upon router deletion / gw removal delete the matching endpoint"""
router_id = kwargs.get('router_id')
# delete the local endpoint from the NSX # delete the local endpoint from the NSX
local_ep_id = self._search_local_endpint(router_id) local_ep_id = self._search_local_endpint(router_id)
if local_ep_id: if local_ep_id:
self._nsx_vpn.local_endpoint.delete(local_ep_id) self._nsx_vpn.local_endpoint.delete(local_ep_id)
# delete the neutron port with this IP # delete the neutron port with this IP
ctx = n_context.get_admin_context() port = self._find_vpn_service_port(context, router_id)
port = self._find_vpn_service_port(ctx, router_id)
if port: if port:
self.l3_plugin.delete_port(ctx, port['id'], force_delete_vpn=True) self.l3_plugin.delete_port(context, port['id'],
force_delete_vpn=True)
def _delete_local_endpoint(self, resource, event, trigger, **kwargs):
"""Upon router deletion / gw removal delete the matching endpoint"""
router_id = kwargs.get('router_id')
ctx = n_context.get_admin_context()
self._delete_local_endpoint_by_router(ctx, router_id)
def _check_subnets_overlap_with_all_conns(self, context, subnets): def _check_subnets_overlap_with_all_conns(self, context, subnets):
# find all vpn services with connections # find all vpn services with connections
@ -556,8 +560,7 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver):
LOG.debug("Created NSX peer endpoint %s", peer_ep_id) LOG.debug("Created NSX peer endpoint %s", peer_ep_id)
# create or reuse a local endpoint using the vpn service # create or reuse a local endpoint using the vpn service
local_ep_id = self._get_local_endpoint( local_ep_id = self._get_local_endpoint(context, vpnservice)
context, ipsec_site_conn, vpnservice)
# Finally: create the session with policy rules # Finally: create the session with policy rules
rules = self._get_session_rules( rules = self._get_session_rules(
@ -698,7 +701,8 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver):
return service['id'] return service['id']
def _get_tier0_uuid(self, context, router_id): def _get_tier0_uuid(self, context, vpnservice):
router_id = vpnservice['router_id']
router_db = self._core_plugin._get_router(context, router_id) router_db = self._core_plugin._get_router(context, router_id)
return self._core_plugin._get_tier0_uuid_by_router(context, router_db) return self._core_plugin._get_tier0_uuid_by_router(context, router_db)
@ -722,8 +726,7 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver):
def _create_vpn_service_if_needed(self, context, vpnservice): def _create_vpn_service_if_needed(self, context, vpnservice):
# The service is created on the TIER0 router attached to the router GW # The service is created on the TIER0 router attached to the router GW
# The NSX can keep only one service per tier0 router so we reuse it # The NSX can keep only one service per tier0 router so we reuse it
router_id = vpnservice['router_id'] tier0_uuid = self._get_tier0_uuid(context, vpnservice)
tier0_uuid = self._get_tier0_uuid(context, router_id)
if self._find_vpn_service(tier0_uuid): if self._find_vpn_service(tier0_uuid):
return return
@ -731,23 +734,44 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver):
self._create_vpn_service(tier0_uuid) self._create_vpn_service(tier0_uuid)
def _delete_vpn_service_if_needed(self, context, vpnservice): def _delete_vpn_service_if_needed(self, context, vpnservice):
# Delete the VPN service on the NSX if no other service uses it # Delete the VPN service on the NSX if no other service connected
filters = {'router_id': [vpnservice['router_id']]} # to the same tier0 use it
services = self.vpn_plugin.get_vpnservices( elev_context = context.elevated()
context.elevated(), filters=filters) tier0_uuid = self._get_tier0_uuid(elev_context, vpnservice)
if not services: all_services = self.vpn_plugin.get_vpnservices(elev_context)
srv_id = self._get_nsx_vpn_service(context, vpnservice) for srv in all_services:
if not srv_id: if (srv['id'] != vpnservice['id'] and
self._get_tier0_uuid(elev_context, srv) == tier0_uuid):
LOG.info("Not deleting vpn service from the NSX as other "
"neutron vpn services still use it.")
return return
try:
self._nsx_vpn.service.delete(srv_id) # Find the NSX-ID
except Exception as e: srv_id = self._get_nsx_vpn_service(elev_context, vpnservice)
LOG.error("Failed to delete VPN service %s: %s", if not srv_id:
srv_id, e) LOG.error("Not deleting vpn service from the NSX as the "
"service was not found on the NSX.")
return
try:
self._nsx_vpn.service.delete(srv_id)
except Exception as e:
LOG.error("Failed to delete VPN service %s: %s",
srv_id, e)
def _delete_local_endpoints_if_needed(self, context, vpnservice):
"""When deleting the last service of a logical router
delete its local endpoint
"""
router_id = vpnservice['router_id']
elev_context = context.elevated()
filters = {'router_id': [router_id]}
services = self.vpn_plugin.get_vpnservices(
elev_context, filters=filters)
if not services:
self._delete_local_endpoint_by_router(elev_context, router_id)
def _get_nsx_vpn_service(self, context, vpnservice): def _get_nsx_vpn_service(self, context, vpnservice):
router_id = vpnservice['router_id'] tier0_uuid = self._get_tier0_uuid(context, vpnservice)
tier0_uuid = self._get_tier0_uuid(context, router_id)
return self._find_vpn_service(tier0_uuid, validate=False) return self._find_vpn_service(tier0_uuid, validate=False)
def _get_service_local_address(self, context, vpnservice): def _get_service_local_address(self, context, vpnservice):
@ -817,4 +841,5 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver):
enabled=connection_enabled) enabled=connection_enabled)
def delete_vpnservice(self, context, vpnservice): def delete_vpnservice(self, context, vpnservice):
self._delete_local_endpoints_if_needed(context, vpnservice)
self._delete_vpn_service_if_needed(context, vpnservice) self._delete_vpn_service_if_needed(context, vpnservice)

6
vmware_nsx/tests/unit/services/vpnaas/test_nsxv3_vpnaas.py
View File

@ -633,6 +633,7 @@ class TestVpnaasDriver(test_plugin.NsxV3PluginTestCaseMixin):
return_value=FAKE_ROUTER),\ return_value=FAKE_ROUTER),\
mock.patch.object(self.plugin, 'get_ports', mock.patch.object(self.plugin, 'get_ports',
return_value=[dummy_port]),\ return_value=[dummy_port]),\
mock.patch.object(self.plugin, 'delete_port'),\
mock.patch.object(self.plugin.nsxlib.logical_router, 'get', mock.patch.object(self.plugin.nsxlib.logical_router, 'get',
return_value=tier0_rtr): return_value=tier0_rtr):
self.driver.create_vpnservice(self.context, FAKE_VPNSERVICE) self.driver.create_vpnservice(self.context, FAKE_VPNSERVICE)
@ -692,6 +693,7 @@ class TestVpnaasDriver(test_plugin.NsxV3PluginTestCaseMixin):
return_value=FAKE_ROUTER),\ return_value=FAKE_ROUTER),\
mock.patch.object(self.plugin, 'get_ports', mock.patch.object(self.plugin, 'get_ports',
return_value=[dummy_port]),\ return_value=[dummy_port]),\
mock.patch.object(self.plugin, 'delete_port'),\
mock.patch.object(self.plugin.nsxlib.logical_router, 'get', mock.patch.object(self.plugin.nsxlib.logical_router, 'get',
return_value=tier0_rtr): return_value=tier0_rtr):
self.driver.create_vpnservice(self.context, FAKE_VPNSERVICE) self.driver.create_vpnservice(self.context, FAKE_VPNSERVICE)
@ -703,6 +705,8 @@ class TestVpnaasDriver(test_plugin.NsxV3PluginTestCaseMixin):
with mock.patch.object( with mock.patch.object(
self.nsxlib_vpn.service, 'list', self.nsxlib_vpn.service, 'list',
return_value={'results': nsx_services}),\ return_value={'results': nsx_services}),\
mock.patch.object(self.service_plugin, 'get_vpnservices',
return_value=[{'id': 'dummy', 'router_id': 'dummy'}]),\
mock.patch.object(self.nsxlib_vpn.service, mock.patch.object(self.nsxlib_vpn.service,
'delete') as delete_service: 'delete') as delete_service:
self.driver.delete_vpnservice( self.driver.delete_vpnservice(
@ -714,6 +718,8 @@ class TestVpnaasDriver(test_plugin.NsxV3PluginTestCaseMixin):
return_value={'results': nsx_services}),\ return_value={'results': nsx_services}),\
mock.patch.object(self.service_plugin, 'get_vpnservices', mock.patch.object(self.service_plugin, 'get_vpnservices',
return_value=[]),\ return_value=[]),\
mock.patch.object(self.service_plugin, 'get_vpnservices',
return_value=[]),\
mock.patch.object(self.nsxlib_vpn.service, mock.patch.object(self.nsxlib_vpn.service,
'delete') as delete_service: 'delete') as delete_service:
self.driver.delete_vpnservice( self.driver.delete_vpnservice(