openstack
/
neutron
Code Issues Proposed changes

Cisco VPN device driver post-merge cleanup 

During review for I-3 there were some minor comments regarding log
level on message and asserts for mock calls.

In addition, some debug logging enhancements were made between the
service and device driver to better indicate the source of the
shared "vpnservice_updated" RPC.

Lastly, unit tests were updated, based on a newer Cisco CSR image,
which had REST API fixes and behavior changes.

Change-Id: I22277462270df0b2cd642ec576e8652c9df146b5
Closes-bug: 1288387
This commit is contained in:
Paul Michali 2014-03-12 14:04:30 +00:00
parent 30342f608e
commit 7f096512bb
8 changed files with 66 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
neutron/services/vpn/device_drivers/cisco_csr_rest_client.py
View File

@ -102,10 +102,9 @@ class CsrRestClient(object):
if isinstance(te, r_exc.SSLError) else '',
'host': self.host})
self.status = requests.codes.REQUEST_TIMEOUT
except r_exc.ConnectionError as ce:
LOG.error(_("%(method)s: Unable to connect to CSR(%(host)s): "
"%(error)s"),
{'method': method, 'host': self.host, 'error': ce})
except r_exc.ConnectionError:
LOG.exception(_("%(method)s: Unable to connect to CSR(%(host)s)"),
{'method': method, 'host': self.host})
self.status = requests.codes.NOT_FOUND
except Exception as e:
LOG.error(_("%(method)s: Unexpected error for CSR (%(host)s): "

9
neutron/services/vpn/device_drivers/cisco_ipsec.py
View File

@ -16,7 +16,7 @@
import abc
from collections import namedtuple
import httplib
import requests
import netaddr
from oslo.config import cfg
@ -221,7 +221,8 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver):
def vpnservice_updated(self, context, **kwargs):
"""Handle VPNaaS service driver change notifications."""
LOG.debug(_("Handling VPN service update notification"))
LOG.debug(_("Handling VPN service update notification '%s'"),
kwargs.get('reason', ''))
self.sync(context, [])
def create_vpn_service(self, service_data):
@ -675,7 +676,7 @@ class CiscoCsrIPSecConnection(object):
def _check_create(self, resource, which):
"""Determine if REST create request was successful."""
if self.csr.status == httplib.CREATED:
if self.csr.status == requests.codes.CREATED:
LOG.debug("%(resource)s %(which)s is configured",
{'resource': resource, 'which': which})
return
@ -701,7 +702,7 @@ class CiscoCsrIPSecConnection(object):
def _verify_deleted(self, status, resource, which):
"""Determine if REST delete request was successful."""
if status in (httplib.NO_CONTENT, httplib.NOT_FOUND):
if status in (requests.codes.NO_CONTENT, requests.codes.NOT_FOUND):
LOG.debug("%(resource)s configuration %(which)s was removed",
{'resource': resource, 'which': which})
else:

7
neutron/services/vpn/service_drivers/__init__.py
View File

@ -76,7 +76,7 @@ class BaseIPsecVpnAgentApi(proxy.RpcProxy):
active=True)
for l3_agent in l3_agents:
LOG.debug(_('Notify agent at %(topic)s.%(host)s the message '
'%(method)s'),
'%(method)s %(args)s'),
{'topic': self.to_agent_topic,
'host': l3_agent.host,
'method': method,
@ -86,6 +86,7 @@ class BaseIPsecVpnAgentApi(proxy.RpcProxy):
version=version,
topic='%s.%s' % (self.to_agent_topic, l3_agent.host))
def vpnservice_updated(self, context, router_id):
def vpnservice_updated(self, context, router_id, **kwargs):
"""Send update event of vpnservices."""
self._agent_notification(context, 'vpnservice_updated', router_id)
self._agent_notification(context, 'vpnservice_updated', router_id,
**kwargs)

12
neutron/services/vpn/service_drivers/cisco_ipsec.py
View File

@ -179,7 +179,8 @@ class CiscoCsrIPsecVPNDriver(service_drivers.VpnDriver):
self.service_plugin.update_ipsec_site_conn_status(
context, ipsec_site_connection['id'], constants.ERROR)
csr_id_map.create_tunnel_mapping(context, ipsec_site_connection)
self.agent_rpc.vpnservice_updated(context, vpnservice['router_id'])
self.agent_rpc.vpnservice_updated(context, vpnservice['router_id'],
reason='ipsec-conn-create')
def update_ipsec_site_connection(
self, context, old_ipsec_site_connection, ipsec_site_connection):
@ -190,7 +191,8 @@ class CiscoCsrIPsecVPNDriver(service_drivers.VpnDriver):
def delete_ipsec_site_connection(self, context, ipsec_site_connection):
vpnservice = self.service_plugin._get_vpnservice(
context, ipsec_site_connection['vpnservice_id'])
self.agent_rpc.vpnservice_updated(context, vpnservice['router_id'])
self.agent_rpc.vpnservice_updated(context, vpnservice['router_id'],
reason='ipsec-conn-delete')
def create_ikepolicy(self, context, ikepolicy):
pass
@ -214,10 +216,12 @@ class CiscoCsrIPsecVPNDriver(service_drivers.VpnDriver):
pass
def update_vpnservice(self, context, old_vpnservice, vpnservice):
self.agent_rpc.vpnservice_updated(context, vpnservice['router_id'])
self.agent_rpc.vpnservice_updated(context, vpnservice['router_id'],
reason='vpn-service-update')
def delete_vpnservice(self, context, vpnservice):
self.agent_rpc.vpnservice_updated(context, vpnservice['router_id'])
self.agent_rpc.vpnservice_updated(context, vpnservice['router_id'],
reason='vpn-service-delete')
def get_cisco_connection_mappings(self, conn_id, context):
"""Obtain persisted mappings for IDs related to connection."""

40
neutron/tests/unit/services/vpn/device_drivers/cisco_csr_mock.py
View File

@ -29,7 +29,6 @@ from neutron.openstack.common import log as logging
from neutron.tests.unit.services.vpn.device_drivers import httmock
# TODO(pcm) Remove, once verified these have been fixed
FIXED_CSCum50512 = False
FIXED_CSCum35484 = False
FIXED_CSCul82396 = False
FIXED_CSCum10324 = False
@ -236,7 +235,7 @@ def normal_get(url, request):
u'outgoing-interface': u'GigabitEthernet1',
u'admin-distance': 1}
return httmock.response(requests.codes.OK, content=content)
if 'vpn-svc/site-to-site/active/sessions':
if 'vpn-svc/site-to-site/active/sessions' in url.path:
# Only including needed fields for mock
content = {u'kind': u'collection#vpn-active-sessions',
u'items': [{u'status': u'DOWN-NEGOTIATING',
@ -318,26 +317,23 @@ def get_defaults(url, request):
def get_unnumbered(url, request):
if not request.headers.get('X-auth-token', None):
return {'status_code': requests.codes.UNAUTHORIZED}
if FIXED_CSCum50512:
tunnel = url.path.split('/')[-1]
ipsec_policy_id = tunnel[6:]
content = {u'kind': u'object#vpn-site-to-site',
u'vpn-interface-name': u'%s' % tunnel,
u'ip-version': u'ipv4',
u'vpn-type': u'site-to-site',
u'ipsec-policy-id': u'%s' % ipsec_policy_id,
u'ike-profile-id': None,
u'mtu': 1500,
u'local-device': {
u'ip-address': u'unnumbered GigabitEthernet3',
u'tunnel-ip-address': u'10.10.10.10'
},
u'remote-device': {
u'tunnel-ip-address': u'10.10.10.20'
}}
return httmock.response(requests.codes.OK, content=content)
else:
return httmock.response(requests.codes.INTERNAL_SERVER_ERROR)
tunnel = url.path.split('/')[-1]
ipsec_policy_id = tunnel[6:]
content = {u'kind': u'object#vpn-site-to-site',
u'vpn-interface-name': u'%s' % tunnel,
u'ip-version': u'ipv4',
u'vpn-type': u'site-to-site',
u'ipsec-policy-id': u'%s' % ipsec_policy_id,
u'ike-profile-id': None,
u'mtu': 1500,
u'local-device': {
u'ip-address': u'GigabitEthernet3',
u'tunnel-ip-address': u'10.10.10.10'
},
u'remote-device': {
u'tunnel-ip-address': u'10.10.10.20'
}}
return httmock.response(requests.codes.OK, content=content)
@filter_request(['get'], 'vpn-svc/site-to-site')

24
neutron/tests/unit/services/vpn/device_drivers/notest_cisco_csr_rest.py
View File

@ -919,17 +919,13 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase):
location)
# Check the hard-coded items that get set as well...
content = self.csr.get_request(location, full_url=True)
if csr_request.FIXED_CSCum50512:
self.assertEqual(requests.codes.OK, self.csr.status)
expected_connection = {u'kind': u'object#vpn-site-to-site',
u'ip-version': u'ipv4'}
expected_connection.update(connection_info)
expected_connection[u'local-device'][u'ip-address'] = (
u'unnumbered GigabitEthernet3')
self.assertEqual(expected_connection, content)
else:
self.assertEqual(requests.codes.INTERNAL_SERVER_ERROR,
self.csr.status)
self.assertEqual(requests.codes.OK, self.csr.status)
expected_connection = {u'kind': u'object#vpn-site-to-site',
u'ike-profile-id': None,
u'mtu': 1500,
u'ip-version': u'ipv4'}
expected_connection.update(connection_info)
self.assertEqual(expected_connection, content)
def test_create_ipsec_connection_no_pre_shared_key(self):
"""Test of connection create without associated pre-shared key.
@ -1036,6 +1032,9 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase):
u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'}
}
self.csr.create_ipsec_connection(connection_info)
self.addCleanup(self._remove_resource_for_test,
self.csr.delete_ipsec_connection,
'Tunnel%d' % tunnel_id)
self.assertEqual(requests.codes.BAD_REQUEST, self.csr.status)
def test_create_ipsec_connection_with_max_mtu(self):
@ -1080,6 +1079,9 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase):
u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'}
}
self.csr.create_ipsec_connection(connection_info)
self.addCleanup(self._remove_resource_for_test,
self.csr.delete_ipsec_connection,
'Tunnel%d' % tunnel_id)
self.assertEqual(requests.codes.BAD_REQUEST, self.csr.status)
def test_status_when_no_tunnels_exist(self):

10
neutron/tests/unit/services/vpn/device_drivers/test_cisco_ipsec.py
View File

@ -426,7 +426,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase):
self.assertFalse(connection.is_dirty)
self.assertEqual(u'Tunnel0', connection.tunnel)
self.assertEqual(constants.ACTIVE, connection.last_status)
self.assertEqual(0, self.conn_create.call_count)
self.assertFalse(self.conn_create.called)
# TODO(pcm) FUTURE - handling for update (delete/create?)
def test_update_of_unknown_ipsec_connection(self):
@ -468,7 +468,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase):
self.assertFalse(connection.is_dirty)
self.assertEqual(u'Tunnel0', connection.tunnel)
self.assertEqual(constants.ACTIVE, connection.last_status)
self.assertEqual(0, self.conn_create.call_count)
self.assertFalse(self.conn_create.called)
def test_update_connection_admin_down(self):
"""Connection updated to admin down state - dirty."""
@ -488,7 +488,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase):
self.assertTrue(connection.is_dirty)
self.assertEqual(u'Tunnel0', connection.tunnel)
self.assertEqual(constants.DOWN, connection.last_status)
self.assertEqual(0, self.conn_create.call_count)
self.assertFalse(self.conn_create.called)
def test_update_missing_connection_admin_down(self):
"""Connection not present is in admin down state - nop.
@ -506,7 +506,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase):
connection = self.driver.update_connection(self.context,
u'123', conn_data)
self.assertIsNone(connection)
self.assertEqual(0, self.conn_create.call_count)
self.assertFalse(self.conn_create.called)
def test_update_for_vpn_service_create(self):
"""Creation of new IPSec connection on new VPN service - create.
@ -586,7 +586,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase):
self.assertFalse(connection.is_dirty)
self.assertEqual(u'Tunnel0', connection.tunnel)
self.assertEqual(constants.ACTIVE, connection.last_status)
self.assertEqual(0, self.conn_create.call_count)
self.assertFalse(self.conn_create.called)
def test_update_service_admin_down(self):
"""VPN service updated to admin down state - dirty.

16
neutron/tests/unit/services/vpn/service_drivers/test_cisco_ipsec.py
View File

@ -309,12 +309,12 @@ class TestCiscoIPsecDriver(base.BaseTestCase):
mock.patch.object(csr_db, 'create_tunnel_mapping').start()
self.context = n_ctx.Context('some_user', 'some_tenant')
def _test_update(self, func, args):
def _test_update(self, func, args, reason=None):
with mock.patch.object(self.driver.agent_rpc, 'cast') as cast:
func(self.context, *args)
cast.assert_called_once_with(
self.context,
{'args': {},
{'args': reason,
'namespace': None,
'method': 'vpnservice_updated'},
version='1.0',
@ -322,7 +322,8 @@ class TestCiscoIPsecDriver(base.BaseTestCase):
def test_create_ipsec_site_connection(self):
self._test_update(self.driver.create_ipsec_site_connection,
[FAKE_VPN_CONNECTION])
[FAKE_VPN_CONNECTION],
{'reason': 'ipsec-conn-create'})
def test_failure_validation_ipsec_connection(self):
"""Failure test of validation during IPSec site connection create.
@ -352,12 +353,15 @@ class TestCiscoIPsecDriver(base.BaseTestCase):
def test_delete_ipsec_site_connection(self):
self._test_update(self.driver.delete_ipsec_site_connection,
[FAKE_VPN_CONNECTION])
[FAKE_VPN_CONNECTION],
{'reason': 'ipsec-conn-delete'})
def test_update_vpnservice(self):
self._test_update(self.driver.update_vpnservice,
[FAKE_VPN_SERVICE, FAKE_VPN_SERVICE])
[FAKE_VPN_SERVICE, FAKE_VPN_SERVICE],
{'reason': 'vpn-service-update'})
def test_delete_vpnservice(self):
self._test_update(self.driver.delete_vpnservice,
[FAKE_VPN_SERVICE])
[FAKE_VPN_SERVICE],
{'reason': 'vpn-service-delete'})