Merge "Big Switch: Only update hash header on success"

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

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.

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: