From 0b7a7687309e8c740ccf79fbbbcabbc64b6f5fd1 Mon Sep 17 00:00:00 2001 From: Aaron Rosen Date: Fri, 11 Jul 2014 13:56:15 -0700 Subject: [PATCH] NSX: Correct default timeout params Previously, req_timeout and http_timeout were set to the same value which is not correct. req_timeout is the total time limit for a cluster request and http_timeout is the time allowed before aborting a request on an unresponsive controller. Since the default configuration allows 2 retries req_timeout should be double that of http_timeout because of this this patch goes ahead and removes req_timeout as this should just be http_timeout * retries. Because prevouly req_timeout and http_timeout were the same this exposed a corner case that when the nsx controller returned a 307 we would issue the request against the redirected controller but in the case where the session cookie had expire when the request was issued we would get a 401 response back and never retry the request. Now that the default values are corrected this issue should no longer occur as the next time time we issue the request we'll fetch a new auth cookie for the redirected controller. This patch also bumps the timeout values to be higher. We've seen more and more timeouts occur in our CI system largely because our cloud is overloaded so increasing the default timeouts will *hopefully* help reduce test failures. DocImpact Closes-bug: 1340969 Closes-bug: 1338846 Change-Id: Id7244cd4d9316931f4f7df1c3b41b3a894f2909a --- etc/neutron/plugins/vmware/nsx.ini | 8 ++------ neutron/plugins/vmware/api_client/client.py | 8 ++------ neutron/plugins/vmware/api_client/eventlet_request.py | 8 +++----- neutron/plugins/vmware/api_client/request.py | 1 - neutron/plugins/vmware/check_nsx_config.py | 1 - neutron/plugins/vmware/common/config.py | 5 +---- neutron/plugins/vmware/common/nsx_utils.py | 1 - .../unit/vmware/apiclient/test_api_eventlet_request.py | 2 +- neutron/tests/unit/vmware/etc/nsx.ini.agentless.test | 1 - neutron/tests/unit/vmware/etc/nsx.ini.combined.test | 1 - neutron/tests/unit/vmware/etc/nsx.ini.full.test | 1 - neutron/tests/unit/vmware/etc/nvp.ini.full.test | 1 - neutron/tests/unit/vmware/nsxlib/base.py | 4 ++-- neutron/tests/unit/vmware/test_nsx_opts.py | 6 +----- neutron/tests/unit/vmware/test_nsx_sync.py | 1 - 15 files changed, 12 insertions(+), 37 deletions(-) diff --git a/etc/neutron/plugins/vmware/nsx.ini b/etc/neutron/plugins/vmware/nsx.ini index 6ce36c04bf6..1ddb8b4516e 100644 --- a/etc/neutron/plugins/vmware/nsx.ini +++ b/etc/neutron/plugins/vmware/nsx.ini @@ -5,12 +5,8 @@ # Password for NSX controller # nsx_password = admin -# Total time limit for a cluster request -# (including retries across different controllers) -# req_timeout = 30 - -# Time before aborting a request on an unresponsive controller -# http_timeout = 30 +# Time before aborting a request on an unresponsive controller (Seconds) +# http_timeout = 75 # Maximum number of times a particular request should be retried # retries = 2 diff --git a/neutron/plugins/vmware/api_client/client.py b/neutron/plugins/vmware/api_client/client.py index a6981a8533f..103943ada61 100644 --- a/neutron/plugins/vmware/api_client/client.py +++ b/neutron/plugins/vmware/api_client/client.py @@ -35,12 +35,9 @@ class NsxApiClient(eventlet_client.EventletApiClient): gen_timeout=base.GENERATION_ID_TIMEOUT, use_https=True, connect_timeout=base.DEFAULT_CONNECT_TIMEOUT, - request_timeout=30, http_timeout=10, retries=2, redirects=2): + http_timeout=75, retries=2, redirects=2): '''Constructor. Adds the following: - :param request_timeout: all operations (including retries, redirects - from unresponsive controllers, etc) should finish within this - timeout. :param http_timeout: how long to wait before aborting an unresponsive controller (and allow for retries to another controller in the cluster) @@ -53,7 +50,7 @@ class NsxApiClient(eventlet_client.EventletApiClient): gen_timeout=gen_timeout, use_https=use_https, connect_timeout=connect_timeout) - self._request_timeout = request_timeout + self._request_timeout = http_timeout * retries self._http_timeout = http_timeout self._retries = retries self._redirects = redirects @@ -85,7 +82,6 @@ class NsxApiClient(eventlet_client.EventletApiClient): g = eventlet_request.GenericRequestEventlet( self, method, url, body, content_type, auto_login=True, - request_timeout=self._request_timeout, http_timeout=self._http_timeout, retries=self._retries, redirects=self._redirects) g.start() diff --git a/neutron/plugins/vmware/api_client/eventlet_request.py b/neutron/plugins/vmware/api_client/eventlet_request.py index 26c378e0c41..0d42dbe1f35 100644 --- a/neutron/plugins/vmware/api_client/eventlet_request.py +++ b/neutron/plugins/vmware/api_client/eventlet_request.py @@ -48,7 +48,6 @@ class EventletApiRequest(request.ApiRequest): def __init__(self, client_obj, url, method="GET", body=None, headers=None, - request_timeout=request.DEFAULT_REQUEST_TIMEOUT, retries=request.DEFAULT_RETRIES, auto_login=True, redirects=request.DEFAULT_REDIRECTS, @@ -59,7 +58,7 @@ class EventletApiRequest(request.ApiRequest): self._method = method self._body = body self._headers = headers or {} - self._request_timeout = request_timeout + self._request_timeout = http_timeout * retries self._retries = retries self._auto_login = auto_login self._redirects = redirects @@ -109,7 +108,7 @@ class EventletApiRequest(request.ApiRequest): '''Return a copy of this request instance.''' return EventletApiRequest( self._api_client, self._url, self._method, self._body, - self._headers, self._request_timeout, self._retries, + self._headers, self._retries, self._auto_login, self._redirects, self._http_timeout) def _run(self): @@ -220,14 +219,13 @@ class GenericRequestEventlet(EventletApiRequest): def __init__(self, client_obj, method, url, body, content_type, auto_login=False, - request_timeout=request.DEFAULT_REQUEST_TIMEOUT, http_timeout=request.DEFAULT_HTTP_TIMEOUT, retries=request.DEFAULT_RETRIES, redirects=request.DEFAULT_REDIRECTS): headers = {"Content-Type": content_type} super(GenericRequestEventlet, self).__init__( client_obj, url, method, body, headers, - request_timeout=request_timeout, retries=retries, + retries=retries, auto_login=auto_login, redirects=redirects, http_timeout=http_timeout) diff --git a/neutron/plugins/vmware/api_client/request.py b/neutron/plugins/vmware/api_client/request.py index 70e7dcef495..aacd926f6f8 100644 --- a/neutron/plugins/vmware/api_client/request.py +++ b/neutron/plugins/vmware/api_client/request.py @@ -30,7 +30,6 @@ from neutron.plugins.vmware import api_client LOG = logging.getLogger(__name__) -DEFAULT_REQUEST_TIMEOUT = 30 DEFAULT_HTTP_TIMEOUT = 30 DEFAULT_RETRIES = 2 DEFAULT_REDIRECTS = 2 diff --git a/neutron/plugins/vmware/check_nsx_config.py b/neutron/plugins/vmware/check_nsx_config.py index 8607f833424..cb0721edbf6 100644 --- a/neutron/plugins/vmware/check_nsx_config.py +++ b/neutron/plugins/vmware/check_nsx_config.py @@ -100,7 +100,6 @@ def main(): print("\tmax_lp_per_bridged_ls: %s" % cfg.CONF.NSX.max_lp_per_bridged_ls) print("\tmax_lp_per_overlay_ls: %s" % cfg.CONF.NSX.max_lp_per_overlay_ls) print("----------------------- Cluster Options -----------------------") - print("\trequested_timeout: %s" % cfg.CONF.req_timeout) print("\tretries: %s" % cfg.CONF.retries) print("\tredirects: %s" % cfg.CONF.redirects) print("\thttp_timeout: %s" % cfg.CONF.http_timeout) diff --git a/neutron/plugins/vmware/common/config.py b/neutron/plugins/vmware/common/config.py index fe93cedf7dd..5fcc52d16ad 100644 --- a/neutron/plugins/vmware/common/config.py +++ b/neutron/plugins/vmware/common/config.py @@ -110,11 +110,8 @@ connection_opts = [ deprecated_name='nvp_password', secret=True, help=_('Password for NSX controllers in this cluster')), - cfg.IntOpt('req_timeout', - default=30, - help=_('Total time limit for a cluster request')), cfg.IntOpt('http_timeout', - default=30, + default=75, help=_('Time before aborting a request')), cfg.IntOpt('retries', default=2, diff --git a/neutron/plugins/vmware/common/nsx_utils.py b/neutron/plugins/vmware/common/nsx_utils.py index e77e3f28d63..267467edef3 100644 --- a/neutron/plugins/vmware/common/nsx_utils.py +++ b/neutron/plugins/vmware/common/nsx_utils.py @@ -210,7 +210,6 @@ def create_nsx_cluster(cluster_opts, concurrent_connections, gen_timeout): for ctrl in cluster.nsx_controllers] cluster.api_client = client.NsxApiClient( api_providers, cluster.nsx_user, cluster.nsx_password, - request_timeout=cluster.req_timeout, http_timeout=cluster.http_timeout, retries=cluster.retries, redirects=cluster.redirects, diff --git a/neutron/tests/unit/vmware/apiclient/test_api_eventlet_request.py b/neutron/tests/unit/vmware/apiclient/test_api_eventlet_request.py index b3e0369093b..c77b855687b 100644 --- a/neutron/tests/unit/vmware/apiclient/test_api_eventlet_request.py +++ b/neutron/tests/unit/vmware/apiclient/test_api_eventlet_request.py @@ -68,7 +68,7 @@ class ApiRequestEventletTest(base.BaseTestCase): def test_apirequest_start(self): for i in range(10): a = request.EventletApiRequest( - self.client, self.url, request_timeout=0.1) + self.client, self.url) a._handle_request = mock.Mock() a.start() eventlet.greenthread.sleep(0.1) diff --git a/neutron/tests/unit/vmware/etc/nsx.ini.agentless.test b/neutron/tests/unit/vmware/etc/nsx.ini.agentless.test index ee129f1d1f1..d69df7275db 100644 --- a/neutron/tests/unit/vmware/etc/nsx.ini.agentless.test +++ b/neutron/tests/unit/vmware/etc/nsx.ini.agentless.test @@ -8,7 +8,6 @@ default_l3_gw_service_uuid = whatever default_l2_gw_service_uuid = whatever default_service_cluster_uuid = whatever default_interface_name = whatever -req_timeout = 14 http_timeout = 13 redirects = 12 retries = 11 diff --git a/neutron/tests/unit/vmware/etc/nsx.ini.combined.test b/neutron/tests/unit/vmware/etc/nsx.ini.combined.test index 2a6f8307cb3..079a738656e 100644 --- a/neutron/tests/unit/vmware/etc/nsx.ini.combined.test +++ b/neutron/tests/unit/vmware/etc/nsx.ini.combined.test @@ -8,7 +8,6 @@ default_l3_gw_service_uuid = whatever default_l2_gw_service_uuid = whatever default_service_cluster_uuid = whatever default_interface_name = whatever -req_timeout = 14 http_timeout = 13 redirects = 12 retries = 11 diff --git a/neutron/tests/unit/vmware/etc/nsx.ini.full.test b/neutron/tests/unit/vmware/etc/nsx.ini.full.test index 7ca29bd42cc..a11a86df2a1 100644 --- a/neutron/tests/unit/vmware/etc/nsx.ini.full.test +++ b/neutron/tests/unit/vmware/etc/nsx.ini.full.test @@ -7,7 +7,6 @@ nsx_password = bar default_l3_gw_service_uuid = whatever default_l2_gw_service_uuid = whatever default_interface_name = whatever -req_timeout = 14 http_timeout = 13 redirects = 12 retries = 11 diff --git a/neutron/tests/unit/vmware/etc/nvp.ini.full.test b/neutron/tests/unit/vmware/etc/nvp.ini.full.test index 500cc0eedf5..83de5930970 100644 --- a/neutron/tests/unit/vmware/etc/nvp.ini.full.test +++ b/neutron/tests/unit/vmware/etc/nvp.ini.full.test @@ -7,7 +7,6 @@ nvp_password = bar default_l3_gw_service_uuid = whatever default_l2_gw_service_uuid = whatever default_interface_name = whatever -req_timeout = 4 http_timeout = 3 redirects = 2 retries = 2 diff --git a/neutron/tests/unit/vmware/nsxlib/base.py b/neutron/tests/unit/vmware/nsxlib/base.py index 8856c00c376..baff3e9c63a 100644 --- a/neutron/tests/unit/vmware/nsxlib/base.py +++ b/neutron/tests/unit/vmware/nsxlib/base.py @@ -47,7 +47,7 @@ class NsxlibTestCase(base.BaseTestCase): self.fake_cluster.api_client = client.NsxApiClient( ('1.1.1.1', '999', True), self.fake_cluster.nsx_user, self.fake_cluster.nsx_password, - self.fake_cluster.req_timeout, self.fake_cluster.http_timeout, + self.fake_cluster.http_timeout, self.fake_cluster.retries, self.fake_cluster.redirects) super(NsxlibTestCase, self).setUp() @@ -81,7 +81,7 @@ class NsxlibNegativeBaseTestCase(base.BaseTestCase): self.fake_cluster.api_client = client.NsxApiClient( ('1.1.1.1', '999', True), self.fake_cluster.nsx_user, self.fake_cluster.nsx_password, - self.fake_cluster.req_timeout, self.fake_cluster.http_timeout, + self.fake_cluster.http_timeout, self.fake_cluster.retries, self.fake_cluster.redirects) super(NsxlibNegativeBaseTestCase, self).setUp() diff --git a/neutron/tests/unit/vmware/test_nsx_opts.py b/neutron/tests/unit/vmware/test_nsx_opts.py index 6bdfc340879..2b15373845b 100644 --- a/neutron/tests/unit/vmware/test_nsx_opts.py +++ b/neutron/tests/unit/vmware/test_nsx_opts.py @@ -45,7 +45,6 @@ class NSXClusterTest(base.BaseTestCase): 'default_l2_gw_service_uuid': uuidutils.generate_uuid(), 'nsx_user': 'foo', 'nsx_password': 'bar', - 'req_timeout': 45, 'http_timeout': 25, 'retries': 7, 'redirects': 23, @@ -89,7 +88,6 @@ class ConfigurationTest(base.BaseTestCase): self.assertEqual(cluster.nsx_password, 'bar') def _assert_extra_options(self, cluster): - self.assertEqual(14, cluster.req_timeout) self.assertEqual(13, cluster.http_timeout) self.assertEqual(12, cluster.redirects) self.assertEqual(11, cluster.retries) @@ -124,8 +122,7 @@ class ConfigurationTest(base.BaseTestCase): self.assertIsNone(cfg.CONF.default_tz_uuid) self.assertEqual('admin', cfg.CONF.nsx_user) self.assertEqual('admin', cfg.CONF.nsx_password) - self.assertEqual(30, cfg.CONF.req_timeout) - self.assertEqual(30, cfg.CONF.http_timeout) + self.assertEqual(75, cfg.CONF.http_timeout) self.assertEqual(2, cfg.CONF.retries) self.assertEqual(2, cfg.CONF.redirects) self.assertIsNone(cfg.CONF.nsx_controllers) @@ -247,7 +244,6 @@ class OldNVPConfigurationTest(base.BaseTestCase): cluster = plugin.cluster # Verify old nvp_* params have been fully parsed self._assert_required_options(cluster) - self.assertEqual(4, cluster.req_timeout) self.assertEqual(3, cluster.http_timeout) self.assertEqual(2, cluster.retries) self.assertEqual(2, cluster.redirects) diff --git a/neutron/tests/unit/vmware/test_nsx_sync.py b/neutron/tests/unit/vmware/test_nsx_sync.py index 0c5e782ee36..ec3f092f363 100644 --- a/neutron/tests/unit/vmware/test_nsx_sync.py +++ b/neutron/tests/unit/vmware/test_nsx_sync.py @@ -282,7 +282,6 @@ class SyncTestCase(base.BaseTestCase): self.fake_cluster.api_client = client.NsxApiClient( ('1.1.1.1', '999', True), self.fake_cluster.nsx_user, self.fake_cluster.nsx_password, - request_timeout=self.fake_cluster.req_timeout, http_timeout=self.fake_cluster.http_timeout, retries=self.fake_cluster.retries, redirects=self.fake_cluster.redirects)