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

diff --git a/etc/neutron/plugins/vmware/nsx.ini b/etc/neutron/plugins/vmware/nsx.ini
index 6ce36c04bf643bb6153e5e5e333f1552e224fc0f..1ddb8b4516efa45028b460dd86426e44d095c780 100644 (file)
--- 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 a6981a8533fe39629dc63aaf9a68c1c070992d47..103943ada616b844ee3f7c5e981b1d8820fb217e 100644 (file)
--- 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 26c378e0c41dc476ccc74ec6ceddc405c810b2b6..0d42dbe1f358651a467fe7006dc2960157a4600e 100644 (file)
--- 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 70e7dcef495988cd8ccced48c8b907948f8274a4..aacd926f6f81a78d21e801af2ef5c46176ee8979 100644 (file)
--- 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 8607f833424d86062be55b1275000201c6f04f29..cb0721edbf64883a73176cc0b573fdb177326951 100644 (file)
--- 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 fe93cedf7dd43d351a6ccd1e28e1f8bab95af2e9..5fcc52d16ad8714a77af83b7714423d5da3c3085 100644 (file)
--- 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 e77e3f28d63cffa994498779cc1cfe63df29eb12..267467edef39fde30f7c6eb2e7aad11d5b58414e 100644 (file)
--- 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 b3e0369093bbd543bdab076b2348cd70bb1367ab..c77b855687bca89c31362eec82afa2ed8dca9a2a 100644 (file)
--- 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 ee129f1d1f129463be74ba073d2b0a5c765cfb48..d69df7275dbf13322559279e47d4731a877be29d 100644 (file)
--- 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 2a6f8307cb3d76f2b0e81882f0b96d0c581dda5b..079a738656e90753d06f2542b37386f8b3429a9d 100644 (file)
--- 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 7ca29bd42ccbf425ccb73d44e7262f358320d7e5..a11a86df2a18e216abc633669703a93147e441a7 100644 (file)
--- 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 500cc0eedf5988f0c9adc44558189b4f24a81ca4..83de5930970c873af19028b485583283721669ad 100644 (file)
--- 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 8856c00c376f48d4d262cc53a624c9014b1ada20..baff3e9c63a7d9e1d83d162799e10c68cbd49ce3 100644 (file)
--- 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 6bdfc3408793031d156175955c84505e8d34adb0..2b15373845b4229b6918b205c13d9439d3994a80 100644 (file)
--- 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 0c5e782ee363c85bf7d0bf651950318225cd0e07..ec3f092f363155e35c719c3c9ddf7b1406ae9a5a 100644 (file)
--- 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)