diff --git a/neutron/plugins/bigswitch/db/consistency_db.py b/neutron/plugins/bigswitch/db/consistency_db.py index e5f1cc9150c..7f0faff0d39 100644 --- a/neutron/plugins/bigswitch/db/consistency_db.py +++ b/neutron/plugins/bigswitch/db/consistency_db.py @@ -76,7 +76,12 @@ class HashHandler(object): else: conhash = ConsistencyHash(hash_id=self.hash_id, hash=hash) self.session.merge(conhash) - self.transaction.commit() - self.transaction = None + self.close_update_session() LOG.debug(_("Consistency hash for group %(hash_id)s updated " "to %(hash)s"), {'hash_id': self.hash_id, 'hash': hash}) + + def close_update_session(self): + if not self.transaction: + return + self.transaction.commit() + self.transaction = None diff --git a/neutron/plugins/bigswitch/servermanager.py b/neutron/plugins/bigswitch/servermanager.py index fdce8a48984..487e0f3442a 100644 --- a/neutron/plugins/bigswitch/servermanager.py +++ b/neutron/plugins/bigswitch/servermanager.py @@ -184,15 +184,20 @@ class ServerProxy(object): try: self.currentconn.request(action, uri, body, headers) response = self.currentconn.getresponse() - hash_handler.put_hash(response.getheader(HASH_MATCH_HEADER)) respstr = response.read() respdata = respstr if response.status in self.success_codes: + hash_value = response.getheader(HASH_MATCH_HEADER) + # don't clear hash from DB if a hash header wasn't present + if hash_value is not None: + hash_handler.put_hash(hash_value) try: respdata = json.loads(respstr) except ValueError: # response was not JSON, ignore the exception pass + else: + hash_handler.close_update_session() ret = (response.status, response.reason, respstr, respdata) except httplib.HTTPException: # If we were using a cached connection, try again with a new one. diff --git a/neutron/tests/unit/bigswitch/test_servermanager.py b/neutron/tests/unit/bigswitch/test_servermanager.py index 85583ed5381..ef9e4af2395 100644 --- a/neutron/tests/unit/bigswitch/test_servermanager.py +++ b/neutron/tests/unit/bigswitch/test_servermanager.py @@ -114,6 +114,8 @@ class ServerManagerTests(test_rp.BigSwitchProxyPluginV2TestCase): with mock.patch(HTTPCON) as conmock: rv = conmock.return_value rv.getresponse.return_value.getheader.return_value = 'HASHHEADER' + rv.getresponse.return_value.status = 200 + rv.getresponse.return_value.read.return_value = '' with self.network(): callheaders = rv.request.mock_calls[0][1][3] self.assertIn('X-BSN-BVS-HASH-MATCH', callheaders) @@ -133,6 +135,25 @@ class ServerManagerTests(test_rp.BigSwitchProxyPluginV2TestCase): self.assertEqual(callheaders['X-BSN-BVS-HASH-MATCH'], 'HASH2') + def test_consistency_hash_header_no_update_on_bad_response(self): + # mock HTTP class instead of rest_call so we can see headers + with mock.patch(HTTPCON) as conmock: + rv = conmock.return_value + rv.getresponse.return_value.getheader.return_value = 'HASHHEADER' + rv.getresponse.return_value.status = 200 + rv.getresponse.return_value.read.return_value = '' + with self.network(): + # change the header that will be received on delete call + rv.getresponse.return_value.getheader.return_value = 'EVIL' + rv.getresponse.return_value.status = 'GARBAGE' + + # create again should not use header from delete call + with self.network(): + callheaders = rv.request.mock_calls[2][1][3] + self.assertIn('X-BSN-BVS-HASH-MATCH', callheaders) + self.assertEqual(callheaders['X-BSN-BVS-HASH-MATCH'], + 'HASHHEADER') + def test_file_put_contents(self): pl = manager.NeutronManager.get_plugin() with mock.patch(SERVERMANAGER + '.open', create=True) as omock: