From 6c4f476b517101e2debc6c9515851ba0bf3e1112 Mon Sep 17 00:00:00 2001 From: Mitchell Jameson Date: Tue, 10 Oct 2017 16:35:09 -0700 Subject: [PATCH] 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 | 4 ++ networking_arista/ml2/rpc/arista_json.py | 41 +++++++++++++++---- .../ml2/rpc/test_arista_json_rpc_wrapper.py | 28 ++++++++++++- .../tests/unit/ml2/test_arista_sync.py | 18 ++++++++ 4 files changed, 81 insertions(+), 10 deletions(-) diff --git a/networking_arista/ml2/rpc/arista_eapi.py b/networking_arista/ml2/rpc/arista_eapi.py index f730d55d..6aa5d637 100644 --- a/networking_arista/ml2/rpc/arista_eapi.py +++ b/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 = [] diff --git a/networking_arista/ml2/rpc/arista_json.py b/networking_arista/ml2/rpc/arista_json.py index ca789a44..b7b0ec18 100644 --- a/networking_arista/ml2/rpc/arista_json.py +++ b/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 diff --git a/networking_arista/tests/unit/ml2/rpc/test_arista_json_rpc_wrapper.py b/networking_arista/tests/unit/ml2/rpc/test_arista_json_rpc_wrapper.py index 0423e400..82e43643 100644 --- a/networking_arista/tests/unit/ml2/rpc/test_arista_json_rpc_wrapper.py +++ b/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() diff --git a/networking_arista/tests/unit/ml2/test_arista_sync.py b/networking_arista/tests/unit/ml2/test_arista_sync.py index 8d95a064..3cb49c10 100644 --- a/networking_arista/tests/unit/ml2/test_arista_sync.py +++ b/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.