Merge "Cisco VPN driver correct reporting for admin state chg"

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 ipsec_conn.forced_down:
-            if is_admin_up:
+                if vpn_service.is_admin_up and conn_is_admin_up:
-                ipsec_conn.is_dirty = False
+                    LOG.debug(_("Update: Connection %s no longer admin down"),
-                LOG.debug(_("Update: IPSec connection %s unchanged - "
+                              conn_id)
-                            "marking clean"), conn_id)
+                    # TODO(pcm) Do no shut on tunnel, once CSR supports
-                # TODO(pcm) FUTURE - Handle update requests (delete/create?)
+                    ipsec_conn.forced_down = False
-                # will need to detect what has changed. For now assume no
+                    ipsec_conn.create_ipsec_site_connection(context, conn_data)
-                # change (it is blocked in service driver).
             else:
-                LOG.debug(_("Update: IPSec connection %s is admin down - "
+                if not vpn_service.is_admin_up or not conn_is_admin_up:
-                            "will be removed in sweep phase"), conn_id)
+                    LOG.debug(_("Update: Connection %s forced to admin down"),
-        else:
+                              conn_id)
-            if not is_admin_up:
+                    # TODO(pcm) Do shut on tunnel, once CSR supports
-                LOG.debug(_("Update: Unknown IPSec connection %s is admin "
+                    ipsec_conn.forced_down = True
-                            "down - ignoring"), conn_id)
+                    ipsec_conn.delete_ipsec_site_connection(context, conn_id)
-                return
+                else:
-            LOG.debug(_("Update: New IPSec connection %s - marking clean"),
+                    # TODO(pcm) FUTURE handle connection update
-                      conn_id)
+                    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: New VPN service %s detected"), vpn_service_id)
-                LOG.debug(_("Update: Unknown VPN service %s is admin down - "
-                            "ignoring"), vpn_service_id)
-                return
             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.is_dirty = False
-                  vpn_service_id)
+        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)
+            if connection.forced_down:
-            frag = connection.build_report_based_on_status(current_status)
+                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
+            vpn_service.update_last_status()
-                conn_report.values()[0]['status'] != constants.ACTIVE):
-                vpn_service.last_status = constants.DOWN
-            else:
-                vpn_service.last_status = constants.ACTIVE
             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,
+        This is called periodically by the agent, to push up changes in
-        and at the end of any sync operation to reflect the changes due to a
+        status. Use a lock to serialize access to (and changing of)
-        sync or change notification.
+        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.forced_down = False
-        self.last_status = conn_info['status']
+        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

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

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
+        Find out the IP of a local interface (GigabitEthernet3) and create an
-        try a connection create with an IP that is on the same subnet.
+        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):

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."""
+        """Connection updated to admin down state - force down."""
-        # Make existing service, and conenction that was active
+        # 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
+        a connection that is in admin down state, create the structures,
-        versus creating and marking dirty and then deleting.
+        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
+        If service is down, then all connections are forced down.
-        connections using the service.
         """
         # 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):
+    def test_update_new_service_admin_down(self):
-        """Unknown VPN service uodated to admin down state - nop.
+        """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,
+        of a service that is in the admin down state. Structures will be
-        versus creating, marking dirty, and then deleting the VPN service.
+        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
+        Shows the case where all the connections are down, so that the
-        service will always report as being ACTIVE (whereas, if there is
+        service should report as DOWN, as well.
-        only one connection, the service will reflect the connection status.
         """
         # 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,