From 471666905c92dc96a4170b9378315ab48c70e37d Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 1 Sep 2020 11:48:12 +0200 Subject: [PATCH] Replace oslo's loopingcall with tenacity The latter has a more natural API and does not have a hard requirement of eventlet. It is already a dependency of ironic-lib. Change-Id: I68de9e989af137b34c19bbaf9b7c0a5ba6e1d4e3 --- ironic_python_agent/ironic_api_client.py | 28 ++++---- .../tests/unit/test_ironic_api_client.py | 70 ++++++++++++------- lower-constraints.txt | 1 + requirements.txt | 1 + 4 files changed, 59 insertions(+), 41 deletions(-) diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index 4738f22d1..617a5e700 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -18,8 +18,8 @@ from distutils.version import StrictVersion from oslo_config import cfg from oslo_log import log from oslo_serialization import jsonutils -from oslo_service import loopingcall import requests +import tenacity from ironic_python_agent import encoding from ironic_python_agent import errors @@ -138,24 +138,22 @@ class APIClient(object): raise errors.HeartbeatError(msg) def lookup_node(self, hardware_info, timeout, starting_interval, - node_uuid=None): - timer = loopingcall.BackOffLoopingCall( - self._do_lookup, - hardware_info=hardware_info, - node_uuid=node_uuid) + node_uuid=None, max_interval=30): + retry = tenacity.retry( + retry=tenacity.retry_if_result(lambda r: r is False), + stop=tenacity.stop_after_delay(timeout), + wait=tenacity.wait_random_exponential(min=starting_interval, + max=max_interval), + reraise=True) try: - node_content = timer.start(starting_interval=starting_interval, - timeout=timeout).wait() - except loopingcall.LoopingCallTimeOut: + return retry(self._do_lookup)(hardware_info=hardware_info, + node_uuid=node_uuid) + except tenacity.RetryError: raise errors.LookupNodeError('Could not look up node info. Check ' 'logs for details.') - return node_content def _do_lookup(self, hardware_info, node_uuid): - """The actual call to lookup a node. - - Should be called as a `loopingcall.BackOffLoopingCall`. - """ + """The actual call to lookup a node.""" params = { 'addresses': ','.join(iface.mac_address for iface in hardware_info['interfaces'] @@ -241,7 +239,7 @@ class APIClient(object): return False # Got valid content - raise loopingcall.LoopingCallDone(retvalue=content) + return content def _get_agent_url(self, advertise_address, advertise_protocol='http'): return '{}://{}:{}'.format(advertise_protocol, diff --git a/ironic_python_agent/tests/unit/test_ironic_api_client.py b/ironic_python_agent/tests/unit/test_ironic_api_client.py index bd710e77f..86a8e865c 100644 --- a/ironic_python_agent/tests/unit/test_ironic_api_client.py +++ b/ironic_python_agent/tests/unit/test_ironic_api_client.py @@ -15,7 +15,6 @@ from unittest import mock from oslo_serialization import jsonutils -from oslo_service import loopingcall import requests from ironic_python_agent import errors @@ -244,7 +243,7 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): uuid='meow', advertise_address=('192.0.2.1', '9999')) - @mock.patch('eventlet.greenthread.sleep', autospec=True) + @mock.patch('time.sleep', autospec=True) @mock.patch('ironic_python_agent.ironic_api_client.APIClient._do_lookup', autospec=True) def test_lookup_node(self, lookup_mock, sleep_mock): @@ -256,8 +255,7 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): 'heartbeat_timeout': 300 } } - lookup_mock.side_effect = loopingcall.LoopingCallDone( - retvalue=content) + lookup_mock.return_value = content returned_content = self.api_client.lookup_node( hardware_info=self.hardware_info, timeout=300, @@ -265,34 +263,54 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): self.assertEqual(content, returned_content) - @mock.patch('eventlet.greenthread.sleep', autospec=True) + @mock.patch('time.sleep', autospec=True) @mock.patch('ironic_python_agent.ironic_api_client.APIClient._do_lookup', autospec=True) - def test_lookup_timeout(self, lookup_mock, sleep_mock): - lookup_mock.side_effect = loopingcall.LoopingCallTimeOut() - self.assertRaises(errors.LookupNodeError, - self.api_client.lookup_node, - hardware_info=self.hardware_info, - timeout=300, - starting_interval=1) - - def test_do_lookup(self): - response = FakeResponse(status_code=200, content={ + def test_lookup_node_retries(self, lookup_mock, sleep_mock): + content = { 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, 'config': { 'heartbeat_timeout': 300 } - }) + } + lookup_mock.side_effect = [False, content] + returned_content = self.api_client.lookup_node( + hardware_info=self.hardware_info, + timeout=300, + starting_interval=0.001) + + self.assertEqual(content, returned_content) + + @mock.patch('time.sleep', autospec=True) + @mock.patch('ironic_python_agent.ironic_api_client.APIClient._do_lookup', + autospec=True) + def test_lookup_timeout(self, lookup_mock, sleep_mock): + lookup_mock.return_value = False + self.assertRaises(errors.LookupNodeError, + self.api_client.lookup_node, + hardware_info=self.hardware_info, + timeout=0.1, + starting_interval=0.001) + + def test_do_lookup(self): + content = { + 'node': { + 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' + }, + 'config': { + 'heartbeat_timeout': 300 + } + } + response = FakeResponse(status_code=200, content=content) self.api_client.session.request = mock.Mock() self.api_client.session.request.return_value = response - self.assertRaises(loopingcall.LoopingCallDone, - self.api_client._do_lookup, - hardware_info=self.hardware_info, - node_uuid=None) + self.assertEqual(content, self.api_client._do_lookup( + hardware_info=self.hardware_info, + node_uuid=None)) url = '{api_url}v1/lookup'.format(api_url=API_URL) request_args = self.api_client.session.request.call_args[0] @@ -303,22 +321,22 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): params) def test_do_lookup_with_uuid(self): - response = FakeResponse(status_code=200, content={ + content = { 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, 'config': { 'heartbeat_timeout': 300 } - }) + } + response = FakeResponse(status_code=200, content=content) self.api_client.session.request = mock.Mock() self.api_client.session.request.return_value = response - self.assertRaises(loopingcall.LoopingCallDone, - self.api_client._do_lookup, - hardware_info=self.hardware_info, - node_uuid='someuuid') + self.assertEqual(content, self.api_client._do_lookup( + hardware_info=self.hardware_info, + node_uuid='someuuid')) url = '{api_url}v1/lookup'.format(api_url=API_URL) request_args = self.api_client.session.request.call_args[0] diff --git a/lower-constraints.txt b/lower-constraints.txt index 914e3be6e..a31099f89 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -78,6 +78,7 @@ Sphinx==2.0.0 sphinxcontrib-websupport==1.0.1 stestr==1.0.0 stevedore==1.20.0 +tenacity==6.2.0 testrepository==0.0.20 testtools==2.2.0 traceback2==1.4.0 diff --git a/requirements.txt b/requirements.txt index 28ea8ace7..8a857be38 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,5 +16,6 @@ pyudev>=0.18 # LGPLv2.1+ requests>=2.14.2 # Apache-2.0 rtslib-fb>=2.1.65 # Apache-2.0 stevedore>=1.20.0 # Apache-2.0 +tenacity>=6.2.0 # Apache-2.0 ironic-lib>=4.1.0 # Apache-2.0 Werkzeug>=0.15.0 # BSD License