Big Switch: Only update hash header on success

This patch moves the hash update call into the success block
of the server manager so the database isn't updated with
a hash header from an error response. Additionally, it prevents
the hash from being updated to an empty value if the hash header
is not present in the response.

Closes-Bug: #1348766
Change-Id: I512d01f9bb91b208dd58883d2464951ecc6748e1

diff --git a/neutron/plugins/bigswitch/db/consistency_db.py b/neutron/plugins/bigswitch/db/consistency_db.py
index e5f1cc9150c8343069cec012dc235c02fe444b0e..7f0faff0d392bd739670b64411f942f783d37ef6 100644 (file)
--- 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 fdce8a48984d0d78e9070e3dc9b231c82cb6a17b..487e0f3442a05c1d931554aead5e58e51ef6d610 100644 (file)
--- 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 85583ed5381e2201ecf5d520d6cae5d879418dc3..ef9e4af23950d85ebfe5e777282ca08bab11a16c 100644 (file)
--- 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: