diff --git a/octavia/amphorae/drivers/haproxy/rest_api_driver.py b/octavia/amphorae/drivers/haproxy/rest_api_driver.py index 60d4a0b2e7..10fcf5b06f 100644 --- a/octavia/amphorae/drivers/haproxy/rest_api_driver.py +++ b/octavia/amphorae/drivers/haproxy/rest_api_driver.py @@ -78,15 +78,17 @@ class HaproxyAmphoraLoadBalancerDriver( connection_logging=CONF.haproxy_amphora.connection_logging) self.lvs_jinja = jinja_udp_cfg.LvsJinjaTemplater() - def _get_haproxy_versions(self, amphora): + def _get_haproxy_versions(self, amphora, timeout_dict=None): """Get major and minor version number from haproxy Example: ['1', '6'] :returns version_list: A list with the major and minor numbers """ - self._populate_amphora_api_version(amphora) - amp_info = self.clients[amphora.api_version].get_info(amphora) + self._populate_amphora_api_version( + amphora, timeout_dict=timeout_dict) + amp_info = self.clients[amphora.api_version].get_info( + amphora, timeout_dict=timeout_dict) haproxy_version_string = amp_info['haproxy_version'] return haproxy_version_string.split('.')[:2] @@ -136,7 +138,8 @@ class HaproxyAmphoraLoadBalancerDriver( return # Check which HAProxy version is on the amp - haproxy_versions = self._get_haproxy_versions(amphora) + haproxy_versions = self._get_haproxy_versions( + amphora, timeout_dict=timeout_dict) # Check which config style to use api_version = self._populate_amphora_api_version(amphora) if api_version[0] == 0 and api_version[1] <= 5: # 0.5 or earlier @@ -259,15 +262,18 @@ class HaproxyAmphoraLoadBalancerDriver( self._populate_amphora_api_version(amp) self.clients[amp.api_version].update_cert_for_rotation(amp, pem) - def _apply(self, func_name, loadbalancer, amphora=None, *args): + def _apply(self, func_name, loadbalancer, amphora=None, *args, **kwargs): if amphora is None: amphorae = loadbalancer.amphorae else: amphorae = [amphora] + timeout_dict = args[0] + for amp in amphorae: if amp.status != consts.DELETED: - api_version = self._populate_amphora_api_version(amp) + api_version = self._populate_amphora_api_version( + amp, timeout_dict=timeout_dict) # Check which config style to use if api_version[0] == 0 and api_version[1] <= 5: # 0.5 or earlier @@ -369,11 +375,14 @@ class HaproxyAmphoraLoadBalancerDriver( self.clients[amphora.api_version].delete_listener( amphora, listener.load_balancer.id) - def get_info(self, amphora, raise_retry_exception=False): + def get_info(self, amphora, raise_retry_exception=False, + timeout_dict=None): self._populate_amphora_api_version( - amphora, raise_retry_exception=raise_retry_exception) + amphora, raise_retry_exception=raise_retry_exception, + timeout_dict=timeout_dict) return self.clients[amphora.api_version].get_info( - amphora, raise_retry_exception=raise_retry_exception) + amphora, raise_retry_exception=raise_retry_exception, + timeout_dict=timeout_dict) def get_diagnostics(self, amphora): pass @@ -811,8 +820,10 @@ class AmphoraAPIClient0_5(AmphoraAPIClientBase): amp, 'listeners/{listener_id}'.format(listener_id=listener_id)) return exc.check_exception(r, (404,)) - def get_info(self, amp, raise_retry_exception=False): - r = self.get(amp, "info", raise_retry_exception=raise_retry_exception) + def get_info(self, amp, raise_retry_exception=False, + timeout_dict=None): + r = self.get(amp, "info", raise_retry_exception=raise_retry_exception, + timeout_dict=timeout_dict) if exc.check_exception(r): return r.json() return None @@ -940,8 +951,10 @@ class AmphoraAPIClient1_0(AmphoraAPIClientBase): amp, 'listeners/{object_id}'.format(object_id=object_id)) return exc.check_exception(r, (404,)) - def get_info(self, amp, raise_retry_exception=False): - r = self.get(amp, "info", raise_retry_exception=raise_retry_exception) + def get_info(self, amp, raise_retry_exception=False, + timeout_dict=None): + r = self.get(amp, "info", raise_retry_exception=raise_retry_exception, + timeout_dict=timeout_dict) if exc.check_exception(r): return r.json() return None diff --git a/octavia/amphorae/drivers/keepalived/vrrp_rest_driver.py b/octavia/amphorae/drivers/keepalived/vrrp_rest_driver.py index 7d21c31aef..da5b9af499 100644 --- a/octavia/amphorae/drivers/keepalived/vrrp_rest_driver.py +++ b/octavia/amphorae/drivers/keepalived/vrrp_rest_driver.py @@ -50,7 +50,8 @@ class KeepalivedAmphoraDriverMixin(driver_base.VRRPDriverMixin): LOG.debug("Update amphora %s VRRP configuration.", amphora.id) - self._populate_amphora_api_version(amphora) + self._populate_amphora_api_version(amphora, + timeout_dict=timeout_dict) # Get the VIP subnet prefix for the amphora # For amphorav2 amphorae_network_config will be list of dicts try: diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py index 5e592bc288..72a6f28285 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py @@ -488,6 +488,16 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): API_VERSION].reload_listener.assert_called_once_with( amp1, listener.id, None) + self.driver.clients[ + API_VERSION].reload_listener.reset_mock() + timeout_dict = { + 'elem1': 1000 + } + self.driver.reload(loadbalancer, timeout_dict=timeout_dict) + self.driver.clients[ + API_VERSION].reload_listener.assert_called_once_with( + amp1, listener.id, timeout_dict) + def test_start_with_amphora(self): # Execute driver method amp = mock.MagicMock() @@ -657,7 +667,19 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): ref_haproxy_versions = ['1', '6'] result = self.driver._get_haproxy_versions(self.amp) self.driver.clients[API_VERSION].get_info.assert_called_once_with( - self.amp) + self.amp, timeout_dict=None) + self.assertEqual(ref_haproxy_versions, result) + + def test_get_haproxy_versions_with_timeout_dict(self): + ref_haproxy_versions = ['1', '6'] + timeout_dict = { + constants.CONN_MAX_RETRIES: 100, + constants.CONN_RETRY_INTERVAL: 1 + } + result = self.driver._get_haproxy_versions(self.amp, + timeout_dict=timeout_dict) + self.driver.clients[API_VERSION].get_info.assert_called_once_with( + self.amp, timeout_dict=timeout_dict) self.assertEqual(ref_haproxy_versions, result) def test_populate_amphora_api_version(self): @@ -751,6 +773,19 @@ class TestAmphoraAPIClientTest(base.TestCase): information = self.driver.get_info(self.amp) self.assertEqual(info, information) + @requests_mock.mock() + def test_get_info_with_timeout_dict(self, m): + info = {"hostname": "some_hostname", "version": "some_version", + "api_version": "0.5", "uuid": FAKE_UUID_1} + m.get("{base}/info".format(base=self.base_url_ver), + json=info) + timeout_dict = { + constants.CONN_MAX_RETRIES: 100, + constants.CONN_RETRY_INTERVAL: 1 + } + information = self.driver.get_info(self.amp, timeout_dict=timeout_dict) + self.assertEqual(info, information) + @requests_mock.mock() def test_get_info_unauthorized(self, m): m.get("{base}/info".format(base=self.base_url_ver), diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py index 60ee82bb0d..980617c803 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py @@ -490,6 +490,16 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): API_VERSION].reload_listener.assert_called_once_with( amp1, loadbalancer.id, None) + self.driver.clients[ + API_VERSION].reload_listener.reset_mock() + timeout_dict = { + 'elem1': 1000 + } + self.driver.reload(loadbalancer, timeout_dict=timeout_dict) + self.driver.clients[ + API_VERSION].reload_listener.assert_called_once_with( + amp1, loadbalancer.id, timeout_dict) + def test_start_with_amphora(self): # Execute driver method amp = mock.MagicMock() @@ -752,7 +762,19 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): ref_haproxy_versions = ['1', '6'] result = self.driver._get_haproxy_versions(self.amp) self.driver.clients[API_VERSION].get_info.assert_called_once_with( - self.amp) + self.amp, timeout_dict=None) + self.assertEqual(ref_haproxy_versions, result) + + def test_get_haproxy_versions_with_timeout_dict(self): + ref_haproxy_versions = ['1', '6'] + timeout_dict = { + constants.CONN_MAX_RETRIES: 100, + constants.CONN_RETRY_INTERVAL: 1 + } + result = self.driver._get_haproxy_versions(self.amp, + timeout_dict=timeout_dict) + self.driver.clients[API_VERSION].get_info.assert_called_once_with( + self.amp, timeout_dict=timeout_dict) self.assertEqual(ref_haproxy_versions, result) def test_populate_amphora_api_version(self): @@ -840,6 +862,19 @@ class TestAmphoraAPIClientTest(base.TestCase): information = self.driver.get_info(self.amp) self.assertEqual(info, information) + @requests_mock.mock() + def test_get_info_with_timeout_dict(self, m): + info = {"hostname": "some_hostname", "version": "some_version", + "api_version": "0.5", "uuid": FAKE_UUID_1} + m.get("{base}/info".format(base=self.base_url_ver), + json=info) + timeout_dict = { + constants.CONN_MAX_RETRIES: 100, + constants.CONN_RETRY_INTERVAL: 1 + } + information = self.driver.get_info(self.amp, timeout_dict=timeout_dict) + self.assertEqual(info, information) + @requests_mock.mock() def test_get_info_unauthorized(self, m): m.get("{base}/info".format(base=self.base_url_ver), diff --git a/releasenotes/notes/fix-timeout-dict-in-failover-tasks-537456e0fe1d7cb8.yaml b/releasenotes/notes/fix-timeout-dict-in-failover-tasks-537456e0fe1d7cb8.yaml new file mode 100644 index 0000000000..c343620438 --- /dev/null +++ b/releasenotes/notes/fix-timeout-dict-in-failover-tasks-537456e0fe1d7cb8.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fix an issue when Octavia performs a failover of an ACTIVE-STANDBY load + balancer that has both amphorae missing. + Some tasks in the controller took too much time to timeout because the + timeout value defined in + ``[haproxy_amphora].active_connection_max_retries`` and + ``[haproxy_amphora].active_connection_rety_interval`` was not used.