diff --git a/neutron/services/vpn/device_drivers/cisco_csr_rest_client.py b/neutron/services/vpn/device_drivers/cisco_csr_rest_client.py index 3cc3c435f2..2e55992836 100644 --- a/neutron/services/vpn/device_drivers/cisco_csr_rest_client.py +++ b/neutron/services/vpn/device_drivers/cisco_csr_rest_client.py @@ -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): " diff --git a/neutron/services/vpn/device_drivers/cisco_ipsec.py b/neutron/services/vpn/device_drivers/cisco_ipsec.py index c518491d3e..5bbe5628b8 100644 --- a/neutron/services/vpn/device_drivers/cisco_ipsec.py +++ b/neutron/services/vpn/device_drivers/cisco_ipsec.py @@ -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: diff --git a/neutron/services/vpn/service_drivers/__init__.py b/neutron/services/vpn/service_drivers/__init__.py index c932de7163..9e7484ed58 100644 --- a/neutron/services/vpn/service_drivers/__init__.py +++ b/neutron/services/vpn/service_drivers/__init__.py @@ -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) diff --git a/neutron/services/vpn/service_drivers/cisco_ipsec.py b/neutron/services/vpn/service_drivers/cisco_ipsec.py index 625fea9369..4afd71b64e 100644 --- a/neutron/services/vpn/service_drivers/cisco_ipsec.py +++ b/neutron/services/vpn/service_drivers/cisco_ipsec.py @@ -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.""" diff --git a/neutron/tests/unit/services/vpn/device_drivers/cisco_csr_mock.py b/neutron/tests/unit/services/vpn/device_drivers/cisco_csr_mock.py index 3830303030..84f5d4ca5c 100644 --- a/neutron/tests/unit/services/vpn/device_drivers/cisco_csr_mock.py +++ b/neutron/tests/unit/services/vpn/device_drivers/cisco_csr_mock.py @@ -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') diff --git a/neutron/tests/unit/services/vpn/device_drivers/notest_cisco_csr_rest.py b/neutron/tests/unit/services/vpn/device_drivers/notest_cisco_csr_rest.py index 920b9e403b..776d4da464 100644 --- a/neutron/tests/unit/services/vpn/device_drivers/notest_cisco_csr_rest.py +++ b/neutron/tests/unit/services/vpn/device_drivers/notest_cisco_csr_rest.py @@ -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): diff --git a/neutron/tests/unit/services/vpn/device_drivers/test_cisco_ipsec.py b/neutron/tests/unit/services/vpn/device_drivers/test_cisco_ipsec.py index d2ff8796e9..78a0bd84fa 100644 --- a/neutron/tests/unit/services/vpn/device_drivers/test_cisco_ipsec.py +++ b/neutron/tests/unit/services/vpn/device_drivers/test_cisco_ipsec.py @@ -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. diff --git a/neutron/tests/unit/services/vpn/service_drivers/test_cisco_ipsec.py b/neutron/tests/unit/services/vpn/service_drivers/test_cisco_ipsec.py index 7c53e6fa7a..50f3bc2c33 100644 --- a/neutron/tests/unit/services/vpn/service_drivers/test_cisco_ipsec.py +++ b/neutron/tests/unit/services/vpn/service_drivers/test_cisco_ipsec.py @@ -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'})