From c1ccc585bda70925a4cffe617743ef6472fe60c4 Mon Sep 17 00:00:00 2001 From: Paul Michali Date: Fri, 21 Mar 2014 13:14:07 +0000 Subject: [PATCH] Cisco VPN driver correct reporting for admin state chg Depends on reference implementation change (81124 review) that will pass VPN service admin up/down changes to the service driver (for subsequent passing to the device driver). This change will save the runtime state of the IPSec connections that have been removed due to a VPN service down change, so that this can be reported to the plugin properly. Otherwise, without the change, there is no info on the downed connection and no change report so the plugin thinks the connection is still active. In addition, the status for the VPN service will reflect whether there are any IPSec connections ACTIVE. If one or more are acive, the service will be active, otherwise it will be DOWN. Updated UT to add tests for admin state and status reporting. Also changed some IPSec create UTs because they were not cleaning up correctly upon test failures (only seen with a live CSR). In the future, when the Cisco CSR REST API supports admin up/down support, the IPSec connections will not be deleted, but instead will be shut down, in response to an admin down event (and then brought up, for admin up). During the down time, the state will be reported correctly and no run-time state recording needed. Change-Id: I294bfb400c31ef36dfe5d9e85b34845e5aef8515 Closes-Bug: 1291619 --- .../vpn/device_drivers/cisco_ipsec.py | 136 +++++---- .../vpn/device_drivers/cisco_csr_mock.py | 11 + .../device_drivers/notest_cisco_csr_rest.py | 32 ++- .../vpn/device_drivers/test_cisco_ipsec.py | 271 ++++++++++++++++-- 4 files changed, 359 insertions(+), 91 deletions(-) diff --git a/neutron/services/vpn/device_drivers/cisco_ipsec.py b/neutron/services/vpn/device_drivers/cisco_ipsec.py index 5bbe5628b..117849696 100644 --- a/neutron/services/vpn/device_drivers/cisco_ipsec.py +++ b/neutron/services/vpn/device_drivers/cisco_ipsec.py @@ -237,29 +237,42 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver): """Handle notification for a single IPSec connection.""" vpn_service = self.service_state[vpn_service_id] conn_id = conn_data['id'] - is_admin_up = conn_data[u'admin_state_up'] + conn_is_admin_up = conn_data[u'admin_state_up'] + if conn_id in vpn_service.conn_state: ipsec_conn = vpn_service.conn_state[conn_id] - ipsec_conn.last_status = conn_data['status'] - if is_admin_up: - ipsec_conn.is_dirty = False - LOG.debug(_("Update: IPSec connection %s unchanged - " - "marking clean"), conn_id) - # TODO(pcm) FUTURE - Handle update requests (delete/create?) - # will need to detect what has changed. For now assume no - # change (it is blocked in service driver). + if ipsec_conn.forced_down: + if vpn_service.is_admin_up and conn_is_admin_up: + LOG.debug(_("Update: Connection %s no longer admin down"), + conn_id) + # TODO(pcm) Do no shut on tunnel, once CSR supports + ipsec_conn.forced_down = False + ipsec_conn.create_ipsec_site_connection(context, conn_data) else: - LOG.debug(_("Update: IPSec connection %s is admin down - " - "will be removed in sweep phase"), conn_id) - else: - if not is_admin_up: - LOG.debug(_("Update: Unknown IPSec connection %s is admin " - "down - ignoring"), conn_id) - return - LOG.debug(_("Update: New IPSec connection %s - marking clean"), - conn_id) + if not vpn_service.is_admin_up or not conn_is_admin_up: + LOG.debug(_("Update: Connection %s forced to admin down"), + conn_id) + # TODO(pcm) Do shut on tunnel, once CSR supports + ipsec_conn.forced_down = True + ipsec_conn.delete_ipsec_site_connection(context, conn_id) + else: + # TODO(pcm) FUTURE handle connection update + LOG.debug(_("Update: Ignoring existing connection %s"), + conn_id) + else: # New connection... ipsec_conn = vpn_service.create_connection(conn_data) - ipsec_conn.create_ipsec_site_connection(context, conn_data) + if not vpn_service.is_admin_up or not conn_is_admin_up: + # TODO(pcm) Create, but set tunnel down, once CSR supports + LOG.debug(_("Update: Created new connection %s in admin down " + "state"), conn_id) + ipsec_conn.forced_down = True + else: + LOG.debug(_("Update: Created new connection %s"), conn_id) + ipsec_conn.create_ipsec_site_connection(context, conn_data) + + ipsec_conn.is_dirty = False + ipsec_conn.last_status = conn_data['status'] + ipsec_conn.is_admin_up = conn_is_admin_up return ipsec_conn def update_service(self, context, service_data): @@ -271,27 +284,21 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver): "router (%(csr_id)s is not associated with a Cisco " "CSR"), {'service': vpn_service_id, 'csr_id': csr_id}) return - is_admin_up = service_data[u'admin_state_up'] + if vpn_service_id in self.service_state: + LOG.debug(_("Update: Existing VPN service %s detected"), + vpn_service_id) vpn_service = self.service_state[vpn_service_id] - vpn_service.last_status = service_data['status'] - if is_admin_up: - vpn_service.is_dirty = False - else: - LOG.debug(_("Update: VPN service %s is admin down - will " - "be removed in sweep phase"), vpn_service_id) - return vpn_service else: - if not is_admin_up: - LOG.debug(_("Update: Unknown VPN service %s is admin down - " - "ignoring"), vpn_service_id) - return + LOG.debug(_("Update: New VPN service %s detected"), vpn_service_id) vpn_service = self.create_vpn_service(service_data) - # Handle all the IPSec connection notifications in the data - LOG.debug(_("Update: Processing IPSec connections for VPN service %s"), - vpn_service_id) + + vpn_service.is_dirty = False + vpn_service.connections_removed = False + vpn_service.last_status = service_data['status'] + vpn_service.is_admin_up = service_data[u'admin_state_up'] for conn_data in service_data['ipsec_conns']: - self.update_connection(context, service_data['id'], conn_data) + self.update_connection(context, vpn_service_id, conn_data) LOG.debug(_("Update: Completed update processing")) return vpn_service @@ -333,6 +340,7 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver): for vpn_service_id, vpn_service in self.service_state.items(): dirty = [c_id for c_id, c in vpn_service.conn_state.items() if c.is_dirty] + vpn_service.connections_removed = len(dirty) > 0 for conn_id in dirty: conn_state = vpn_service.conn_state[conn_id] conn_state.delete_ipsec_site_connection(context, conn_id) @@ -341,6 +349,8 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver): if vpn_service.is_dirty: service_count += 1 del self.service_state[vpn_service_id] + elif dirty: + self.connections_removed = True LOG.debug(_("Sweep: Removed %(service)d dirty VPN service%(splural)s " "and %(conn)d dirty IPSec connection%(cplural)s"), {'service': service_count, 'conn': connection_count, @@ -361,8 +371,15 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver): tunnels = vpn_service.get_ipsec_connections_status() report = {} for connection in vpn_service.conn_state.values(): - current_status = connection.find_current_status_in(tunnels) - frag = connection.build_report_based_on_status(current_status) + if connection.forced_down: + LOG.debug(_("Connection %s forced down"), connection.conn_id) + current_status = constants.DOWN + else: + current_status = connection.find_current_status_in(tunnels) + LOG.debug(_("Connection %(conn)s reported %(status)s"), + {'conn': connection.conn_id, + 'status': current_status}) + frag = connection.update_status_and_build_report(current_status) if frag: LOG.debug(_("Report: Adding info for IPSec connection %s"), connection.conn_id) @@ -380,14 +397,10 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver): as DOWN). """ conn_report = self.build_report_for_connections_on(vpn_service) - if conn_report: + if conn_report or vpn_service.connections_removed: pending_handled = plugin_utils.in_pending_status( vpn_service.last_status) - if (len(conn_report) == 1 and - conn_report.values()[0]['status'] != constants.ACTIVE): - vpn_service.last_status = constants.DOWN - else: - vpn_service.last_status = constants.ACTIVE + vpn_service.update_last_status() LOG.debug(_("Report: Adding info for VPN service %s"), vpn_service.service_id) return {u'id': vpn_service.service_id, @@ -397,15 +410,20 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver): else: return {} + @lockutils.synchronized('vpn-agent', 'neutron-') def report_status(self, context): """Report status of all VPN services and IPSec connections to plugin. - This is called periodically by the agent, to push up status changes, - and at the end of any sync operation to reflect the changes due to a - sync or change notification. + This is called periodically by the agent, to push up changes in + status. Use a lock to serialize access to (and changing of) + running state. """ + return self.report_status_internal(context) + + def report_status_internal(self, context): + """Generate report and send to plugin, if anything changed.""" service_report = [] - LOG.debug(_("Report: Starting status report")) + LOG.debug(_("Report: Starting status report processing")) for vpn_service_id, vpn_service in self.service_state.items(): LOG.debug(_("Report: Collecting status for VPN service %s"), vpn_service_id) @@ -428,11 +446,14 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver): Called when update/delete a service or create/update/delete a connection (vpnservice_updated message), or router change (_process_routers). + + Use lock to serialize access (and changes) to running state for VPN + service and IPsec connections. """ self.mark_existing_connections_as_dirty() self.update_all_services_and_connections(context) self.remove_unknown_connections(context) - self.report_status(context) + self.report_status_internal(context) def create_router(self, process_id): """Actions taken when router created.""" @@ -451,10 +472,9 @@ class CiscoCsrVpnService(object): def __init__(self, service_data, csr): self.service_id = service_data['id'] - self.last_status = service_data['status'] self.conn_state = {} - self.is_dirty = False self.csr = csr + self.is_admin_up = True # TODO(pcm) FUTURE - handle sharing of policies def create_connection(self, conn_data): @@ -502,6 +522,16 @@ class CiscoCsrVpnService(object): if connection.tunnel == tunnel_id: return connection.conn_id + def no_connections_up(self): + return not any(c.last_status == 'ACTIVE' + for c in self.conn_state.values()) + + def update_last_status(self): + if not self.is_admin_up or self.no_connections_up(): + self.last_status = constants.DOWN + else: + self.last_status = constants.ACTIVE + class CiscoCsrIPSecConnection(object): @@ -511,8 +541,8 @@ class CiscoCsrIPSecConnection(object): self.conn_id = conn_info['id'] self.csr = csr self.steps = [] - self.is_dirty = False - self.last_status = conn_info['status'] + self.forced_down = False + self.is_admin_up = conn_info[u'admin_state_up'] self.tunnel = conn_info['cisco']['site_conn_id'] def find_current_status_in(self, statuses): @@ -521,7 +551,7 @@ class CiscoCsrIPSecConnection(object): else: return constants.ERROR - def build_report_based_on_status(self, current_status): + def update_status_and_build_report(self, current_status): if current_status != self.last_status: pending_handled = plugin_utils.in_pending_status(self.last_status) self.last_status = current_status 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 84f5d4ca5..31626689f 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 @@ -378,6 +378,17 @@ def get_none(url, request): return httmock.response(requests.codes.OK, content=content) +@filter_request(['get'], 'interfaces/GigabitEthernet3') +@httmock.urlmatch(netloc=r'localhost') +def get_local_ip(url, request): + if not request.headers.get('X-auth-token', None): + return {'status_code': requests.codes.UNAUTHORIZED} + content = {u'kind': u'object#interface', + u'subnet-mask': u'255.255.255.0', + u'ip-address': u'10.5.0.2'} + return httmock.response(requests.codes.OK, content=content) + + @httmock.urlmatch(netloc=r'localhost') def post(url, request): if request.method != 'POST': 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 776d4da46..db8063dea 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 @@ -874,10 +874,10 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase): u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'} } location = self.csr.create_ipsec_connection(connection_info) - self.assertEqual(requests.codes.CREATED, self.csr.status) self.addCleanup(self._remove_resource_for_test, self.csr.delete_ipsec_connection, 'Tunnel%d' % tunnel_id) + self.assertEqual(requests.codes.CREATED, self.csr.status) self.assertIn('vpn-svc/site-to-site/Tunnel%d' % tunnel_id, location) # Check the hard-coded items that get set as well... @@ -911,10 +911,10 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase): u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'} } location = self.csr.create_ipsec_connection(connection_info) - self.assertEqual(requests.codes.CREATED, self.csr.status) self.addCleanup(self._remove_resource_for_test, self.csr.delete_ipsec_connection, 'Tunnel%d' % tunnel_id) + self.assertEqual(requests.codes.CREATED, self.csr.status) self.assertIn('vpn-svc/site-to-site/Tunnel%d' % tunnel_id, location) # Check the hard-coded items that get set as well... @@ -947,10 +947,10 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase): u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'} } location = self.csr.create_ipsec_connection(connection_info) - self.assertEqual(requests.codes.CREATED, self.csr.status) self.addCleanup(self._remove_resource_for_test, self.csr.delete_ipsec_connection, 'Tunnel%d' % tunnel_id) + self.assertEqual(requests.codes.CREATED, self.csr.status) self.assertIn('vpn-svc/site-to-site/Tunnel%d' % tunnel_id, location) # Check the hard-coded items that get set as well... @@ -983,10 +983,10 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase): u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'} } location = self.csr.create_ipsec_connection(connection_info) - self.assertEqual(requests.codes.CREATED, self.csr.status) self.addCleanup(self._remove_resource_for_test, self.csr.delete_ipsec_connection, 'Tunnel%d' % tunnel_id) + self.assertEqual(requests.codes.CREATED, self.csr.status) self.assertIn('vpn-svc/site-to-site/Tunnel%d' % tunnel_id, location) # Check the hard-coded items that get set as well... @@ -1013,21 +1013,35 @@ 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 _determine_conflicting_ip(self): + with httmock.HTTMock(csr_request.token, csr_request.get_local_ip): + details = self.csr.get_request('interfaces/GigabitEthernet3') + if self.csr.status != requests.codes.OK: + self.fail("Unable to obtain interface GigabitEthernet3's IP") + if_ip = details.get('ip-address') + if not if_ip: + self.fail("No IP address for GigabitEthernet3 interface") + return '.'.join(if_ip.split('.')[:3]) + '.10' + def test_create_ipsec_connection_conficting_tunnel_ip(self): """Negative test of connection create with conflicting tunnel IP. - The GigabitEthernet3 interface has an IP of 10.2.0.6. This will - try a connection create with an IP that is on the same subnet. + Find out the IP of a local interface (GigabitEthernet3) and create an + IP that is on the same subnet. Note: this interface needs to be up. """ + conflicting_ip = self._determine_conflicting_ip() tunnel_id, ipsec_policy_id = self._prepare_for_site_conn_create() with httmock.HTTMock(csr_request.token, csr_request.post_bad_ip): connection_info = { u'vpn-interface-name': u'Tunnel%d' % tunnel_id, u'ipsec-policy-id': u'%d' % ipsec_policy_id, - u'local-device': {u'ip-address': u'10.2.0.10/24', + u'local-device': {u'ip-address': u'%s/24' % conflicting_ip, u'tunnel-ip-address': u'10.10.10.10'}, u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'} } @@ -1051,10 +1065,10 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase): u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'} } location = self.csr.create_ipsec_connection(connection_info) - self.assertEqual(requests.codes.CREATED, self.csr.status) self.addCleanup(self._remove_resource_for_test, self.csr.delete_ipsec_connection, 'Tunnel%d' % tunnel_id) + self.assertEqual(requests.codes.CREATED, self.csr.status) self.assertIn('vpn-svc/site-to-site/Tunnel%d' % tunnel_id, location) # Check the hard-coded items that get set as well... @@ -1106,10 +1120,10 @@ class TestCsrRestIPSecConnectionCreate(base.BaseTestCase): u'remote-device': {u'tunnel-ip-address': u'10.10.10.20'} } location = self.csr.create_ipsec_connection(connection_info) - self.assertEqual(requests.codes.CREATED, self.csr.status) self.addCleanup(self._remove_resource_for_test, self.csr.delete_ipsec_connection, u'Tunnel123') + self.assertEqual(requests.codes.CREATED, self.csr.status) self.assertIn('vpn-svc/site-to-site/Tunnel%d' % tunnel_id, location) with httmock.HTTMock(csr_request.token, csr_request.normal_get): 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 78a0bd84f..2261e0442 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 @@ -471,8 +471,8 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): self.assertFalse(self.conn_create.called) def test_update_connection_admin_down(self): - """Connection updated to admin down state - dirty.""" - # Make existing service, and conenction that was active + """Connection updated to admin down state - force down.""" + # Make existing service, and connection that was active vpn_service = self.driver.create_vpn_service(self.service123_data) conn_data = {u'id': '1', u'status': constants.ACTIVE, u'admin_state_up': True, @@ -485,7 +485,8 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): connection = self.driver.update_connection(self.context, u'123', conn_data) - self.assertTrue(connection.is_dirty) + self.assertFalse(connection.is_dirty) + self.assertTrue(connection.forced_down) self.assertEqual(u'Tunnel0', connection.tunnel) self.assertEqual(constants.DOWN, connection.last_status) self.assertFalse(self.conn_create.called) @@ -494,8 +495,8 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): """Connection not present is in admin down state - nop. If the agent has restarted, and a sync notification occurs with - a connection that is in admin down state, ignore the connection - versus creating and marking dirty and then deleting. + a connection that is in admin down state, create the structures, + but indicate that the connection is down. """ # Make existing service, but no connection self.driver.create_vpn_service(self.service123_data) @@ -505,9 +506,37 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): connection = self.driver.update_connection(self.context, u'123', conn_data) - self.assertIsNone(connection) + self.assertIsNotNone(connection) + self.assertFalse(connection.is_dirty) + self.assertFalse(connection.is_admin_up) + self.assertTrue(connection.forced_down) self.assertFalse(self.conn_create.called) + def test_update_connection_admin_up(self): + """Connection updated to admin up state - record.""" + # Make existing service, and connection that was admin down + conn_data = {u'id': '1', u'status': constants.DOWN, + u'admin_state_up': False, + u'cisco': {u'site_conn_id': u'Tunnel0'}} + service_data = {u'id': u'123', + u'status': constants.DOWN, + u'external_ip': u'1.1.1.1', + u'admin_state_up': True, + u'ipsec_conns': [conn_data]} + self.driver.update_service(self.context, service_data) + self.driver.mark_existing_connections_as_dirty() + # Now simulate that the notification shows the connection admin up + conn_data[u'admin_state_up'] = True + conn_data[u'status'] = constants.DOWN + + connection = self.driver.update_connection(self.context, + u'123', conn_data) + self.assertFalse(connection.is_dirty) + self.assertFalse(connection.forced_down) + self.assertEqual(u'Tunnel0', connection.tunnel) + self.assertEqual(constants.DOWN, connection.last_status) + self.assertEqual(1, self.conn_create.call_count) + def test_update_for_vpn_service_create(self): """Creation of new IPSec connection on new VPN service - create. @@ -589,10 +618,9 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): self.assertFalse(self.conn_create.called) def test_update_service_admin_down(self): - """VPN service updated to admin down state - dirty. + """VPN service updated to admin down state - force all down. - Mark service dirty and do not process any notfications for - connections using the service. + If service is down, then all connections are forced down. """ # Create an "existing" service, prior to notification prev_vpn_service = self.driver.create_vpn_service(self.service123_data) @@ -607,24 +635,76 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): u'ipsec_conns': [conn_data]} vpn_service = self.driver.update_service(self.context, service_data) self.assertEqual(prev_vpn_service, vpn_service) - self.assertTrue(vpn_service.is_dirty) + self.assertFalse(vpn_service.is_dirty) + self.assertFalse(vpn_service.is_admin_up) self.assertEqual(constants.DOWN, vpn_service.last_status) - self.assertIsNone(vpn_service.get_connection(u'1')) + conn = vpn_service.get_connection(u'1') + self.assertIsNotNone(conn) + self.assertFalse(conn.is_dirty) + self.assertTrue(conn.forced_down) + self.assertTrue(conn.is_admin_up) - def test_update_unknown_service_admin_down(self): - """Unknown VPN service uodated to admin down state - nop. + def test_update_new_service_admin_down(self): + """Unknown VPN service updated to admin down state - nop. Can happen if agent restarts and then gets its first notificaiton - of a service that is in the admin down state. Will not do anything, - versus creating, marking dirty, and then deleting the VPN service. + of a service that is in the admin down state. Structures will be + created, but forced down. """ + conn_data = {u'id': u'1', u'status': constants.ACTIVE, + u'admin_state_up': True, + u'cisco': {u'site_conn_id': u'Tunnel0'}} service_data = {u'id': u'123', u'status': constants.DOWN, u'external_ip': u'1.1.1.1', u'admin_state_up': False, - u'ipsec_conns': []} + u'ipsec_conns': [conn_data]} vpn_service = self.driver.update_service(self.context, service_data) - self.assertIsNone(vpn_service) + self.assertIsNotNone(vpn_service) + self.assertFalse(vpn_service.is_dirty) + self.assertFalse(vpn_service.is_admin_up) + self.assertEqual(constants.DOWN, vpn_service.last_status) + conn = vpn_service.get_connection(u'1') + self.assertIsNotNone(conn) + self.assertFalse(conn.is_dirty) + self.assertTrue(conn.forced_down) + self.assertTrue(conn.is_admin_up) + + def test_update_service_admin_up(self): + """VPN service updated to admin up state - restore. + + If service is up now, then connections that are admin up will come + up and connections that are admin down, will remain down. + """ + # Create an "existing" service, prior to notification + prev_vpn_service = self.driver.create_vpn_service(self.service123_data) + self.driver.mark_existing_connections_as_dirty() + conn_data1 = {u'id': u'1', u'status': constants.DOWN, + u'admin_state_up': False, + u'cisco': {u'site_conn_id': u'Tunnel0'}} + conn_data2 = {u'id': u'2', u'status': constants.ACTIVE, + u'admin_state_up': True, + u'cisco': {u'site_conn_id': u'Tunnel1'}} + service_data = {u'id': u'123', + u'status': constants.DOWN, + u'external_ip': u'1.1.1.1', + u'admin_state_up': True, + u'ipsec_conns': [conn_data1, conn_data2]} + vpn_service = self.driver.update_service(self.context, service_data) + self.assertEqual(prev_vpn_service, vpn_service) + self.assertFalse(vpn_service.is_dirty) + self.assertTrue(vpn_service.is_admin_up) + self.assertEqual(constants.DOWN, vpn_service.last_status) + conn1 = vpn_service.get_connection(u'1') + self.assertIsNotNone(conn1) + self.assertFalse(conn1.is_dirty) + self.assertTrue(conn1.forced_down) + self.assertFalse(conn1.is_admin_up) + conn2 = vpn_service.get_connection(u'2') + self.assertIsNotNone(conn2) + self.assertFalse(conn2.is_dirty) + self.assertFalse(conn2.forced_down) + self.assertTrue(conn2.is_admin_up) def test_update_of_unknown_service_create(self): """Create of VPN service that is currently unknown - record. @@ -800,7 +880,12 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): def simulate_mark_update_sweep_for_service_with_conn(self, service_state, connection_state): - """Create internal structures for single service with connection.""" + """Create internal structures for single service with connection. + + The service and connection will be marked as clean, and since + none are being deleted, the service's connections_removed + attribute will remain false. + """ # Simulate that we have done mark, update, and sweep. conn_data = {u'id': u'1', u'status': connection_state, u'admin_state_up': True, @@ -833,7 +918,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): # Create report fragment due to change self.assertNotEqual(connection.last_status, current_status) - report_frag = connection.build_report_based_on_status(current_status) + report_frag = connection.update_status_and_build_report(current_status) self.assertEqual(current_status, connection.last_status) expected = {'1': {'status': constants.ACTIVE, 'updated_pending_status': True}} @@ -860,7 +945,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): # Should be no report, as no change self.assertEqual(connection.last_status, current_status) - report_frag = connection.build_report_based_on_status(current_status) + report_frag = connection.update_status_and_build_report(current_status) self.assertEqual(current_status, connection.last_status) self.assertEqual({}, report_frag) @@ -885,7 +970,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): # Create report fragment due to change self.assertNotEqual(connection.last_status, current_status) - report_frag = connection.build_report_based_on_status(current_status) + report_frag = connection.update_status_and_build_report(current_status) self.assertEqual(current_status, connection.last_status) expected = {'1': {'status': constants.ACTIVE, 'updated_pending_status': False}} @@ -915,12 +1000,40 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): # Create report fragment due to change self.assertNotEqual(connection.last_status, current_status) - report_frag = connection.build_report_based_on_status(current_status) + report_frag = connection.update_status_and_build_report(current_status) self.assertEqual(current_status, connection.last_status) expected = {'1': {'status': constants.ERROR, 'updated_pending_status': True}} self.assertEqual(expected, report_frag) + def test_report_fragment_connection_admin_down(self): + """Report for a connection that is in admin down state.""" + # Prepare service and connection with previous status ACTIVE, but + # with connection admin down + conn_data = {u'id': u'1', u'status': constants.ACTIVE, + u'admin_state_up': False, + u'cisco': {u'site_conn_id': u'Tunnel0'}} + service_data = {u'id': u'123', + u'status': constants.ACTIVE, + u'external_ip': u'1.1.1.1', + u'admin_state_up': True, + u'ipsec_conns': [conn_data]} + vpn_service = self.driver.update_service(self.context, service_data) + # Tunnel would have been deleted, so simulate no status + self.csr.read_tunnel_statuses.return_value = [] + + connection = vpn_service.get_connection(u'1') + self.assertIsNotNone(connection) + self.assertTrue(connection.forced_down) + self.assertEqual(constants.ACTIVE, connection.last_status) + + # Create report fragment due to change + report_frag = self.driver.build_report_for_connections_on(vpn_service) + self.assertEqual(constants.DOWN, connection.last_status) + expected = {'1': {'status': constants.DOWN, + 'updated_pending_status': False}} + self.assertEqual(expected, report_frag) + def test_report_fragment_two_connections(self): """Generate report fragment for two connections on a service.""" # Prepare service with two connections, one ACTIVE, one DOWN @@ -1032,7 +1145,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): Note: No report will be generated if the last connection on the service is deleted. The service (and connection) objects will - have been reoved by the sweep operation and thus not reported. + have been removed by the sweep operation and thus not reported. On the plugin, the service should be changed to DOWN. Likewise, if the service goes to admin down state. """ @@ -1055,7 +1168,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): In addition to reporting the status change and recording the new state for the IPSec connection, the VPN service status will be - ACTIVE. + DOWN. """ # Simulate an existing service and connection that are ACTIVE vpn_service = self.simulate_mark_update_sweep_for_service_with_conn( @@ -1085,7 +1198,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): In addition to reporting the status change and recording the new state for the IPSec connection, the VPN service status will be - DOWN. + ACTIVE. """ # Simulate an existing service and connection that are DOWN vpn_service = self.simulate_mark_update_sweep_for_service_with_conn( @@ -1113,9 +1226,8 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): def test_report_service_with_two_connections_gone_down(self): """One service with two connections that went down - report. - If there is more than one IPSec connection on a VPN service, the - service will always report as being ACTIVE (whereas, if there is - only one connection, the service will reflect the connection status. + Shows the case where all the connections are down, so that the + service should report as DOWN, as well. """ # Simulated one service with two ACTIVE connections conn1_data = {u'id': u'1', u'status': constants.ACTIVE, @@ -1138,7 +1250,7 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): expected_report = { u'id': u'123', u'updated_pending_status': False, - u'status': constants.ACTIVE, + u'status': constants.DOWN, u'ipsec_site_connections': { u'1': {u'status': constants.DOWN, u'updated_pending_status': False}, @@ -1147,7 +1259,108 @@ class TestCiscoCsrIPsecDeviceDriverSyncStatuses(base.BaseTestCase): } self.assertEqual(expected_report, report) # Check that service and connection statuses are updated + self.assertEqual(constants.DOWN, vpn_service.last_status) + self.assertEqual(constants.DOWN, + vpn_service.get_connection(u'1').last_status) + self.assertEqual(constants.DOWN, + vpn_service.get_connection(u'2').last_status) + + def test_report_service_with_connection_removed(self): + """One service with two connections where one is removed - report. + + With a connection removed and the other connection unchanged, + normally there would be nothing to report for the connections, but + we need to report any possible change to the service state. In this + case, the service was ACTIVE, but since the only ACTIVE connection + is deleted and the remaining connection is DOWN, the service will + indicate as DOWN. + """ + # Simulated one service with one connection up, one down + conn1_data = {u'id': u'1', u'status': constants.ACTIVE, + u'admin_state_up': True, + u'cisco': {u'site_conn_id': u'Tunnel1'}} + conn2_data = {u'id': u'2', u'status': constants.DOWN, + u'admin_state_up': True, + u'cisco': {u'site_conn_id': u'Tunnel2'}} + service_data = {u'id': u'123', + u'status': constants.ACTIVE, + u'external_ip': u'1.1.1.1', + u'admin_state_up': True, + u'ipsec_conns': [conn1_data, conn2_data]} + vpn_service = self.driver.update_service(self.context, service_data) self.assertEqual(constants.ACTIVE, vpn_service.last_status) + self.assertEqual(constants.ACTIVE, + vpn_service.get_connection(u'1').last_status) + self.assertEqual(constants.DOWN, + vpn_service.get_connection(u'2').last_status) + + # Simulate that one is deleted + self.driver.mark_existing_connections_as_dirty() + service_data = {u'id': u'123', + u'status': constants.ACTIVE, + u'external_ip': u'1.1.1.1', + u'admin_state_up': True, + u'ipsec_conns': [conn2_data]} + vpn_service = self.driver.update_service(self.context, service_data) + self.driver.remove_unknown_connections(self.context) + self.assertTrue(vpn_service.connections_removed) + self.assertEqual(constants.ACTIVE, vpn_service.last_status) + self.assertIsNone(vpn_service.get_connection(u'1')) + self.assertEqual(constants.DOWN, + vpn_service.get_connection(u'2').last_status) + + # Simulate that only one connection reports and status is unchanged, + # so there will be NO connection info to report. + self.csr.read_tunnel_statuses.return_value = [(u'Tunnel2', u'DOWN')] + report = self.driver.build_report_for_service(vpn_service) + expected_report = { + u'id': u'123', + u'updated_pending_status': False, + u'status': constants.DOWN, + u'ipsec_site_connections': {} + } + self.assertEqual(expected_report, report) + # Check that service and connection statuses are updated + self.assertEqual(constants.DOWN, vpn_service.last_status) + self.assertEqual(constants.DOWN, + vpn_service.get_connection(u'2').last_status) + + def test_report_service_admin_down_with_two_connections(self): + """One service admin down, with two connections - report. + + When the service is admin down, all the connections will report + as DOWN. + """ + # Simulated one service (admin down) with two ACTIVE connections + conn1_data = {u'id': u'1', u'status': constants.ACTIVE, + u'admin_state_up': True, + u'cisco': {u'site_conn_id': u'Tunnel1'}} + conn2_data = {u'id': u'2', u'status': constants.ACTIVE, + u'admin_state_up': True, + u'cisco': {u'site_conn_id': u'Tunnel2'}} + service_data = {u'id': u'123', + u'status': constants.ACTIVE, + u'external_ip': u'1.1.1.1', + u'admin_state_up': False, + u'ipsec_conns': [conn1_data, conn2_data]} + vpn_service = self.driver.update_service(self.context, service_data) + # Since service admin down, connections will have been deleted + self.csr.read_tunnel_statuses.return_value = [] + + report = self.driver.build_report_for_service(vpn_service) + expected_report = { + u'id': u'123', + u'updated_pending_status': False, + u'status': constants.DOWN, + u'ipsec_site_connections': { + u'1': {u'status': constants.DOWN, + u'updated_pending_status': False}, + u'2': {u'status': constants.DOWN, + u'updated_pending_status': False}} + } + self.assertEqual(expected_report, report) + # Check that service and connection statuses are updated + self.assertEqual(constants.DOWN, vpn_service.last_status) self.assertEqual(constants.DOWN, vpn_service.get_connection(u'1').last_status) self.assertEqual(constants.DOWN, -- 2.45.2