From 3732f022778a7c4a47c58029ae77e32a0a4a6ee9 Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Wed, 10 Jul 2013 16:28:12 -0700 Subject: [PATCH] Prevent possible server list damage in BigSwitch plugin The old failover logic for the BigSwitch plugin modified a list of controllers as it tested them. If the code for one thread unexpectedly raised an exception or died, the global server list could lose a server permanently. This patch addresses that by flagging servers as failed instead so the global server list is never modified. Fixes: bug #1200022 Change-Id: Id2dcb820ef9f62fd03e3215bff3345e56c78afe2 --- neutron/plugins/bigswitch/plugin.py | 15 ++++----- .../unit/bigswitch/etc/restproxy.ini.test | 2 +- .../unit/bigswitch/test_restproxy_plugin.py | 33 +++++++++++++++++-- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/neutron/plugins/bigswitch/plugin.py b/neutron/plugins/bigswitch/plugin.py index 01c29bc91..31add796e 100644 --- a/neutron/plugins/bigswitch/plugin.py +++ b/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 = [] - while self.servers: - active_server = self.servers[0] + good_first = sorted(self.servers, key=lambda x: x.failed) + for active_server in good_first: 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)}) - self.servers.extend(failed_servers) + s.port) for s in self.servers)}) return (0, None, None, None) def get(self, resource, data='', headers=None, ignore_codes=[]): diff --git a/neutron/tests/unit/bigswitch/etc/restproxy.ini.test b/neutron/tests/unit/bigswitch/etc/restproxy.ini.test index 3c1c17f94..8df78a6eb 100644 --- a/neutron/tests/unit/bigswitch/etc/restproxy.ini.test +++ b/neutron/tests/unit/bigswitch/etc/restproxy.ini.test @@ -21,7 +21,7 @@ retry_interval = 2 # serverauth : (default: no auth) # serverssl : True | False (default: False) # -servers=localhost:8899 +servers=localhost:9000,localhost:8899 serverssl=False #serverauth=username:password diff --git a/neutron/tests/unit/bigswitch/test_restproxy_plugin.py b/neutron/tests/unit/bigswitch/test_restproxy_plugin.py index b6d743191..13caebb6d 100644 --- a/neutron/tests/unit/bigswitch/test_restproxy_plugin.py +++ b/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 - pass + if port == 9000: + 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,