From b95fbe9ed4d8585e72bfc4f93aefb3d3ca1ab932 Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Fri, 26 Feb 2021 14:48:40 +0100 Subject: [PATCH] Fix overriding default listener timeout values in config file The default value for timeout parameters in the BaseListenerType was not correctly set because the class was defined before reading the config file. Story 2008666 Task 41953 Change-Id: Ia4aa2047a79ad6fc3e33c7ebe2da9438914f7a88 --- octavia/api/v2/types/listener.py | 28 +++------- octavia/db/prepare.py | 13 +++++ .../tests/functional/api/v2/test_listener.py | 17 ++++++ .../functional/api/v2/test_load_balancer.py | 56 ++++++++++++++++++- ...values-for-listeners-108c8048ba8beb9a.yaml | 6 ++ 5 files changed, 99 insertions(+), 21 deletions(-) create mode 100644 releasenotes/notes/fix-default-timeout-values-for-listeners-108c8048ba8beb9a.yaml diff --git a/octavia/api/v2/types/listener.py b/octavia/api/v2/types/listener.py index 1f72456403..6ea100d455 100644 --- a/octavia/api/v2/types/listener.py +++ b/octavia/api/v2/types/listener.py @@ -12,7 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_config import cfg from wsme import types as wtypes from octavia.api.common import types @@ -20,9 +19,6 @@ from octavia.api.v2.types import l7policy from octavia.api.v2.types import pool from octavia.common import constants -CONF = cfg.CONF -CONF.import_group('haproxy_amphora', 'octavia.common.config') - class BaseListenerType(types.BaseType): _type_to_model_map = { @@ -136,20 +132,16 @@ class ListenerPOST(BaseListenerType): loadbalancer_id = wtypes.wsattr(wtypes.UuidType(), mandatory=True) timeout_client_data = wtypes.wsattr( wtypes.IntegerType(minimum=constants.MIN_TIMEOUT, - maximum=constants.MAX_TIMEOUT), - default=CONF.haproxy_amphora.timeout_client_data) + maximum=constants.MAX_TIMEOUT)) timeout_member_connect = wtypes.wsattr( wtypes.IntegerType(minimum=constants.MIN_TIMEOUT, - maximum=constants.MAX_TIMEOUT), - default=CONF.haproxy_amphora.timeout_member_connect) + maximum=constants.MAX_TIMEOUT)) timeout_member_data = wtypes.wsattr( wtypes.IntegerType(minimum=constants.MIN_TIMEOUT, - maximum=constants.MAX_TIMEOUT), - default=CONF.haproxy_amphora.timeout_member_data) + maximum=constants.MAX_TIMEOUT)) timeout_tcp_inspect = wtypes.wsattr( wtypes.IntegerType(minimum=constants.MIN_TIMEOUT, - maximum=constants.MAX_TIMEOUT), - default=CONF.haproxy_amphora.timeout_tcp_inspect) + maximum=constants.MAX_TIMEOUT)) tags = wtypes.wsattr(wtypes.ArrayType(wtypes.StringType(max_length=255))) client_ca_tls_container_ref = wtypes.StringType(max_length=255) client_authentication = wtypes.wsattr( @@ -231,20 +223,16 @@ class ListenerSingleCreate(BaseListenerType): wtypes.DictType(str, wtypes.StringType(max_length=255))) timeout_client_data = wtypes.wsattr( wtypes.IntegerType(minimum=constants.MIN_TIMEOUT, - maximum=constants.MAX_TIMEOUT), - default=CONF.haproxy_amphora.timeout_client_data) + maximum=constants.MAX_TIMEOUT)) timeout_member_connect = wtypes.wsattr( wtypes.IntegerType(minimum=constants.MIN_TIMEOUT, - maximum=constants.MAX_TIMEOUT), - default=CONF.haproxy_amphora.timeout_member_connect) + maximum=constants.MAX_TIMEOUT)) timeout_member_data = wtypes.wsattr( wtypes.IntegerType(minimum=constants.MIN_TIMEOUT, - maximum=constants.MAX_TIMEOUT), - default=CONF.haproxy_amphora.timeout_member_data) + maximum=constants.MAX_TIMEOUT)) timeout_tcp_inspect = wtypes.wsattr( wtypes.IntegerType(minimum=constants.MIN_TIMEOUT, - maximum=constants.MAX_TIMEOUT), - default=CONF.haproxy_amphora.timeout_tcp_inspect) + maximum=constants.MAX_TIMEOUT)) tags = wtypes.wsattr(wtypes.ArrayType(wtypes.StringType(max_length=255))) client_ca_tls_container_ref = wtypes.StringType(max_length=255) client_authentication = wtypes.wsattr( diff --git a/octavia/db/prepare.py b/octavia/db/prepare.py index 617c4c75e6..9dbc291343 100644 --- a/octavia/db/prepare.py +++ b/octavia/db/prepare.py @@ -118,6 +118,19 @@ def create_listener(listener_dict, lb_id): listener_dict['alpn_protocols'] = ( CONF.api_settings.default_listener_alpn_protocols) + if listener_dict.get('timeout_client_data') is None: + listener_dict['timeout_client_data'] = ( + CONF.haproxy_amphora.timeout_client_data) + if listener_dict.get('timeout_member_connect') is None: + listener_dict['timeout_member_connect'] = ( + CONF.haproxy_amphora.timeout_member_connect) + if listener_dict.get('timeout_member_data') is None: + listener_dict['timeout_member_data'] = ( + CONF.haproxy_amphora.timeout_member_data) + if listener_dict.get('timeout_tcp_inspect') is None: + listener_dict['timeout_tcp_inspect'] = ( + CONF.haproxy_amphora.timeout_tcp_inspect) + return listener_dict diff --git a/octavia/tests/functional/api/v2/test_listener.py b/octavia/tests/functional/api/v2/test_listener.py index 14070bb896..ec6d0f46f5 100644 --- a/octavia/tests/functional/api/v2/test_listener.py +++ b/octavia/tests/functional/api/v2/test_listener.py @@ -663,6 +663,23 @@ class TestListener(base.BaseAPITest): self.assertEqual(constants.MAX_TIMEOUT, listener_api.get('timeout_tcp_inspect')) + def test_create_with_default_timeouts(self): + self.conf = self.useFixture(oslo_fixture.Config(cfg.CONF)) + self.conf.config(group='haproxy_amphora', timeout_client_data=20) + self.conf.config(group='haproxy_amphora', timeout_member_connect=21) + self.conf.config(group='haproxy_amphora', + timeout_member_data=constants.MIN_TIMEOUT) + self.conf.config(group='haproxy_amphora', + timeout_tcp_inspect=constants.MAX_TIMEOUT) + + listener_api = self.test_create() + self.assertEqual(20, listener_api.get('timeout_client_data')) + self.assertEqual(21, listener_api.get('timeout_member_connect')) + self.assertEqual(constants.MIN_TIMEOUT, + listener_api.get('timeout_member_data')) + self.assertEqual(constants.MAX_TIMEOUT, + listener_api.get('timeout_tcp_inspect')) + def test_create_with_timeouts_too_high(self): optionals = { 'timeout_client_data': 1, diff --git a/octavia/tests/functional/api/v2/test_load_balancer.py b/octavia/tests/functional/api/v2/test_load_balancer.py index 26437e7af9..76a720296e 100644 --- a/octavia/tests/functional/api/v2/test_load_balancer.py +++ b/octavia/tests/functional/api/v2/test_load_balancer.py @@ -2708,7 +2708,15 @@ class TestLoadBalancerGraph(base.BaseAPITest): create_client_crl_container=None, expected_client_crl_container=None, create_allowed_cidrs=None, - expected_allowed_cidrs=None): + expected_allowed_cidrs=None, + create_timeout_client_data=None, + expected_timeout_client_data=None, + create_timeout_member_connect=None, + expected_timeout_member_connect=None, + create_timeout_member_data=None, + expected_timeout_member_data=None, + create_timeout_tcp_inspect=None, + expected_timeout_tcp_inspect=None): create_listener = { 'name': name, 'protocol_port': protocol_port, @@ -2789,6 +2797,31 @@ class TestLoadBalancerGraph(base.BaseAPITest): [lib_consts.ALPN_PROTOCOL_HTTP_1_1, lib_consts.ALPN_PROTOCOL_HTTP_1_0]) + if create_timeout_client_data is not None: + create_listener['timeout_client_data'] = ( + create_timeout_client_data) + if expected_timeout_client_data is not None: + expected_listener['timeout_client_data'] = ( + expected_timeout_client_data) + if create_timeout_member_connect is not None: + create_listener['timeout_member_connect'] = ( + create_timeout_member_connect) + if expected_timeout_member_connect is not None: + expected_listener['timeout_member_connect'] = ( + expected_timeout_member_connect) + if create_timeout_member_data is not None: + create_listener['timeout_member_data'] = ( + create_timeout_member_data) + if expected_timeout_member_data is not None: + expected_listener['timeout_member_data'] = ( + expected_timeout_member_data) + if create_timeout_tcp_inspect is not None: + create_listener['timeout_tcp_inspect'] = ( + create_timeout_tcp_inspect) + if expected_timeout_tcp_inspect is not None: + expected_listener['timeout_tcp_inspect'] = ( + expected_timeout_tcp_inspect) + return create_listener, expected_listener def _get_pool_bodies(self, name='pool1', create_members=None, @@ -2974,6 +3007,27 @@ class TestLoadBalancerGraph(base.BaseAPITest): api_lb = response.json.get(self.root_tag) self._assert_graphs_equal(expected_lb, api_lb) + def test_with_one_listener_with_default_timeouts(self): + self.conf = self.useFixture(oslo_fixture.Config(cfg.CONF)) + self.conf.config(group='haproxy_amphora', timeout_client_data=20) + self.conf.config(group='haproxy_amphora', timeout_member_connect=21) + self.conf.config(group='haproxy_amphora', + timeout_member_data=constants.MIN_TIMEOUT) + self.conf.config(group='haproxy_amphora', + timeout_tcp_inspect=constants.MAX_TIMEOUT) + + create_listener, expected_listener = self._get_listener_bodies( + expected_timeout_client_data=20, + expected_timeout_member_connect=21, + expected_timeout_member_data=constants.MIN_TIMEOUT, + expected_timeout_tcp_inspect=constants.MAX_TIMEOUT) + create_lb, expected_lb = self._get_lb_bodies([create_listener], + [expected_listener]) + body = self._build_body(create_lb) + response = self.post(self.LBS_PATH, body) + api_lb = response.json.get(self.root_tag) + self._assert_graphs_equal(expected_lb, api_lb) + def test_with_many_listeners(self): create_listener1, expected_listener1 = self._get_listener_bodies() create_listener2, expected_listener2 = self._get_listener_bodies( diff --git a/releasenotes/notes/fix-default-timeout-values-for-listeners-108c8048ba8beb9a.yaml b/releasenotes/notes/fix-default-timeout-values-for-listeners-108c8048ba8beb9a.yaml new file mode 100644 index 0000000000..cab6766799 --- /dev/null +++ b/releasenotes/notes/fix-default-timeout-values-for-listeners-108c8048ba8beb9a.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fix default value override for timeout values for listeners. Changing the + default timeouts in the configuration file wasn't correctly applied in the + default listener parameters.