Merge "Prevent possible server list damage in BigSwitch plugin"

neutron/plugins/bigswitch/plugin.py

@@ -184,10 +184,10 @@ class ServerProxy(object):
         self.success_codes = SUCCESS_CODES
         self.auth = None
         self.neutron_id = neutron_id
+        self.failed = False
         if auth:
             self.auth = 'Basic ' + base64.encodestring(auth).strip()
 
-    @utils.synchronized('bsn-rest-call', external=True)
     def rest_call(self, action, resource, data, headers):
         uri = self.base_uri + resource
         body = json.dumps(data)
@@ -283,13 +283,13 @@ class ServerPool(object):
         """
         return resp[0] in SUCCESS_CODES
 
+    @utils.synchronized('bsn-rest-call', external=True)
     def rest_call(self, action, resource, data, headers, ignore_codes):
-        failed_servers = []
+        good_first = sorted(self.servers, key=lambda x: x.failed)
-        while self.servers:
+        for active_server in good_first:
-            active_server = self.servers[0]
             ret = active_server.rest_call(action, resource, data, headers)
             if not self.server_failure(ret, ignore_codes):
-                self.servers.extend(failed_servers)
+                active_server.failed = False
                 return ret
             else:
                 LOG.error(_('ServerProxy: %(action)s failure for servers: '
@@ -297,15 +297,14 @@ class ServerPool(object):
                           {'action': action,
                            'server': (active_server.server,
                                       active_server.port)})
-                failed_servers.append(self.servers.pop(0))
+                active_server.failed = True
 
         # All servers failed, reset server list and try again next time
         LOG.error(_('ServerProxy: %(action)s failure for all servers: '
                     '%(server)r'),
                   {'action': action,
                    'server': tuple((s.server,
-                                    s.port) for s in failed_servers)})
+                                    s.port) for s in self.servers)})
-        self.servers.extend(failed_servers)
         return (0, None, None, None)
 
     def get(self, resource, data='', headers=None, ignore_codes=[]):

neutron/tests/unit/bigswitch/etc/restproxy.ini.test

@@ -21,7 +21,7 @@ retry_interval = 2
 #   serverauth  :   <username:password>         (default: no auth)
 #   serverssl   :   True | False                (default: False)
 #
-servers=localhost:8899
+servers=localhost:9000,localhost:8899
 serverssl=False
 #serverauth=username:password
 

neutron/tests/unit/bigswitch/test_restproxy_plugin.py

@@ -55,13 +55,33 @@ class HTTPResponseMock404():
         return "{'status': '404 Not Found'}"
 
 
+class HTTPResponseMock500():
+    status = 500
+    reason = 'Internal Server Error'
+
+    def __init__(self, sock, debuglevel=0, strict=0, method=None,
+                 buffering=False):
+        pass
+
+    def read(self):
+        return "{'status': '500 Internal Server Error'}"
+
+
 class HTTPConnectionMock():
 
     def __init__(self, server, port, timeout):
-        self.response = None
+        if port == 9000:
-        pass
+            self.response = HTTPResponseMock500(None)
+            self.broken = True
+        else:
+            self.response = HTTPResponseMock(None)
+            self.broken = False
 
     def request(self, action, uri, body, headers):
+        if self.broken:
+            if "ExceptOnBadServer" in uri:
+                raise Exception("Broken server got an unexpected request")
+            return
         if uri.endswith('attachment') and action == 'DELETE':
             self.response = HTTPResponseMock404(None)
         else:
@@ -101,7 +121,14 @@ class TestBigSwitchProxyBasicGet(test_plugin.TestBasicGet,
 class TestBigSwitchProxyV2HTTPResponse(test_plugin.TestV2HTTPResponse,
                                        BigSwitchProxyPluginV2TestCase):
 
-    pass
+    def test_failover_memory(self):
+        # first request causes failover so next shouldn't hit bad server
+        with self.network() as net:
+            kwargs = {'tenant_id': 'ExceptOnBadServer'}
+            with self.network(**kwargs) as net:
+                req = self.new_show_request('networks', net['network']['id'])
+                res = req.get_response(self.api)
+                self.assertEqual(res.status_int, 200)
 
 
 class TestBigSwitchProxyPortsV2(test_plugin.TestPortsV2,