Throw error in Arista JSON API calls on error status code

Error returns from JSON API calls are currently handled
identically to successful returns. We should differentiate
these cases by raising an exception on error return codes.

Change-Id: I703cb0cfa82f6586a2961fca3cd4b9a64c1c468e

networking_arista/ml2/rpc/arista_eapi.py

@@ -609,6 +609,10 @@ class AristaRPCWrapperEapi(AristaRPCWrapperBase):
                     port_name = 'name "%s"' % neutron_port['name']
 
                 device_owner = neutron_port['device_owner']
+
+                if port_id not in port_profiles:
+                    continue
+
                 vnic_type = port_profiles[port_id]['vnic_type']
                 network_id = neutron_port['network_id']
                 segments = []

networking_arista/ml2/rpc/arista_json.py

@@ -78,8 +78,14 @@ class AristaRPCWrapperJSON(AristaRPCWrapperBase):
 
             resp = func(url, timeout=self.conn_timeout, verify=False,
                         data=data, headers=request_headers)
-            LOG.info(_LI('JSON response contains: %s'), resp.json())
-            return resp.json()
+            msg = (_LI('JSON response contains: %(code)s %(resp)s') %
+                   {'code': resp.status_code,
+                   'resp': resp.json()})
+            LOG.info(msg)
+            if resp.ok:
+                return resp.json()
+            else:
+                raise arista_exc.AristaRpcError(msg=resp.json().get('error'))
         except requests.exceptions.ConnectionError:
             msg = (_('Error connecting to %(url)s') % {'url': url})
             LOG.warning(msg)
@@ -149,8 +155,11 @@ class AristaRPCWrapperJSON(AristaRPCWrapperBase):
 
     def get_region_updated_time(self):
         path = 'agent/'
-        data = self._send_api_request(path, 'GET')
-        return {'regionTimestamp': data['uuid']}
+        try:
+            data = self._send_api_request(path, 'GET')
+            return {'regionTimestamp': data.get('uuid', '')}
+        except arista_exc.AristaRpcError:
+            return {'regionTimestamp': ''}
 
     def create_region(self, region):
         path = 'region/'
@@ -166,11 +175,14 @@ class AristaRPCWrapperJSON(AristaRPCWrapperBase):
         return self.delete_region(self.region)
 
     def get_region(self, name):
-        path = 'region/'
-        regions = self._send_api_request(path, 'GET')
-        for region in regions:
-            if region['name'] == name:
-                return region
+        path = 'region/%s' % name
+        try:
+            regions = self._send_api_request(path, 'GET')
+            for region in regions:
+                if region['name'] == name:
+                    return region
+        except arista_exc.AristaRpcError:
+            pass
         return None
 
     def sync_supported(self):
@@ -182,6 +194,13 @@ class AristaRPCWrapperJSON(AristaRPCWrapperBase):
     def sync_start(self):
         try:
             region = self.get_region(self.region)
+
+            # If the region doesn't exist, we may need to create
+            # it in order for POSTs to the sync endpoint to succeed
+            if not region:
+                self.register_with_eos()
+                return False
+
             if region and region['syncStatus'] == 'syncInProgress':
                 LOG.info('Sync in progress, not syncing')
                 return False
@@ -425,6 +444,10 @@ class AristaRPCWrapperJSON(AristaRPCWrapperBase):
                 instance = self._create_instance_data(inst_id, inst_host)
 
                 device_owner = neutron_port['device_owner']
+
+                if port_id not in port_profiles:
+                    continue
+
                 vnic_type = port_profiles[port_id]['vnic_type']
                 if device_owner == n_const.DEVICE_OWNER_DHCP:
                     instance_type = const.InstanceType.DHCP

networking_arista/tests/unit/ml2/rpc/test_arista_json_rpc_wrapper.py

@@ -113,13 +113,28 @@ class TestAristaJSONRPCWrapper(testlib_api.SqlTestCase):
         ]
         self._verify_send_api_request_call(mock_send_api_req, calls)
 
+    @patch('requests.Response')
+    def test_sync_start_exception(self, mock_response):
+        mock_response.ok.return_value = False
+        self.assertFalse(self.drv.sync_start())
+
+    @patch(JSON_SEND_FUNC)
+    def test_sync_start_no_region(self, mock_send_api_req):
+        mock_send_api_req.return_value = {}
+        self.assertFalse(self.drv.sync_start())
+        calls = [
+            ('region/RegionOne', 'GET'),
+            ('region/', 'POST', [{'name': 'RegionOne'}])
+        ]
+        self._verify_send_api_request_call(mock_send_api_req, calls)
+
     @patch(JSON_SEND_FUNC)
     @patch(RAND_FUNC, _get_random_name)
     def test_sync_end(self, mock_send_api_req):
         mock_send_api_req.return_value = [{'requester':
                                            self._get_random_name()}]
         self.drv.current_sync_name = self._get_random_name()
-        assert self.drv.sync_end()
+        self.assertTrue(self.drv.sync_end())
         calls = [
             ('region/RegionOne/sync', 'DELETE')
         ]
@@ -131,12 +146,23 @@ class TestAristaJSONRPCWrapper(testlib_api.SqlTestCase):
         calls = [('region/', 'POST', [{'name': 'foo'}])]
         self._verify_send_api_request_call(mock_send_api_req, calls)
 
+    @patch('requests.Response')
+    def test_get_region_exception(self, mock_response):
+        mock_response.ok.return_value = False
+        self.assertIsNone(self.drv.get_region('foo'))
+
     @patch(JSON_SEND_FUNC)
     def test_delete_region(self, mock_send_api_req):
         self.drv.delete_region('foo')
         calls = [('region/', 'DELETE', [{'name': 'foo'}])]
         self._verify_send_api_request_call(mock_send_api_req, calls)
 
+    @patch('requests.Response')
+    def test_get_region__updated_exception(self, mock_response):
+        mock_response.ok.return_value = False
+        self.assertEqual(self.drv.get_region_updated_time(),
+                         {'regionTimestamp': ''})
+
     @patch(JSON_SEND_FUNC)
     def test_get_tenants(self, mock_send_api_req):
         self.drv.get_tenants()

networking_arista/tests/unit/ml2/test_arista_sync.py

@@ -111,6 +111,24 @@ class SyncServiceTest(testlib_api.SqlTestCase):
         db_lib.forget_network_segment(tenant_id, network_id)
         db_lib.forget_tenant(tenant_id)
 
+    def test_sync_start_failure(self):
+        """Tests that we force another sync when sync_start fails.
+
+           The failure could be because a region does not exist or
+           because another controller has the sync lock.
+        """
+        self.sync_service.synchronize = mock.MagicMock()
+        region_updated_time = {
+            'regionName': 'RegionOne',
+            'regionTimestamp': '424242'
+        }
+        self.rpc.get_region_updated_time.return_value = region_updated_time
+        self.rpc.check_cvx_availability.return_value = True
+        self.rpc.sync_start.return_value = False
+        self.sync_service.do_synchronize()
+        self.assertFalse(self.sync_service.synchronize.called)
+        self.assertTrue(self.sync_service._force_sync)
+
     def test_synchronize_not_required(self):
         """Tests whether synchronize() sends the right commands.