From 95a9ddd388b5bb65f2d472b6c19985c4da366a83 Mon Sep 17 00:00:00 2001 From: Adam Dyess Date: Thu, 27 Oct 2022 14:20:12 -0500 Subject: [PATCH] Issue#743: fetch.ubuntu._run_with_retries raises on unexpected exitcode (#744) * Issue#743: fetch.ubuntu._run_with_retries raises on unexpected exitcode * Address `test_check_restart_timestamps` issues in py3.10 * apply appropriate mocks during test_service unit tests (cherry picked from commit 0f7ca5396edd6a932aada59385b90bd95739b30c) --- charmhelpers/fetch/ubuntu.py | 8 +++++-- .../contrib/openstack/test_deferred_events.py | 10 +++++---- tests/core/test_services.py | 21 +++++++++++++------ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/charmhelpers/fetch/ubuntu.py b/charmhelpers/fetch/ubuntu.py index fcf096751..27565d694 100644 --- a/charmhelpers/fetch/ubuntu.py +++ b/charmhelpers/fetch/ubuntu.py @@ -945,10 +945,14 @@ def _run_with_retries(cmd, max_retries=CMD_RETRY_COUNT, retry_exitcodes=(1,), try: result = subprocess.check_call(cmd, env=env, **kwargs) except subprocess.CalledProcessError as e: - retry_count = retry_count + 1 + result = e.returncode + if result not in retry_results: + # a non-retriable exitcode was produced + raise + retry_count += 1 if retry_count > max_retries: + # a retriable exitcode was produced more than {max_retries} times raise - result = e.returncode log(retry_message) time.sleep(CMD_RETRY_DELAY) diff --git a/tests/contrib/openstack/test_deferred_events.py b/tests/contrib/openstack/test_deferred_events.py index 7af848164..43ed3a08d 100644 --- a/tests/contrib/openstack/test_deferred_events.py +++ b/tests/contrib/openstack/test_deferred_events.py @@ -304,10 +304,13 @@ def test_get_service_start_time(self, check_output): def test_check_restart_timestamps(self, get_service_start_time, log, clear_deferred_restarts, get_deferred_restarts): + request_time = '2021-02-02 10:19:55' + timestamp = datetime.datetime.strptime( + 'Tue ' + request_time + ' UTC', + '%a %Y-%m-%d %H:%M:%S %Z').timestamp() deferred_restarts = [ - # 'Tue 2021-02-02 10:19:55 UTC' deferred_events.ServiceEvent( - timestamp=1612261195.0, + timestamp=timestamp, service='svcA', reason='ReasonA', action='restart')] @@ -326,8 +329,7 @@ def test_check_restart_timestamps(self, get_service_start_time, log, self.assertFalse(clear_deferred_restarts.called) log.assert_called_once_with( ('Restart still required, svcA was started at 2021-02-02 10:10:55,' - ' restart was requested after that at {}'.format( - datetime.datetime.fromtimestamp(1612261195.0))), + ' restart was requested after that at {}'.format(request_time)), level='DEBUG') def test_set_deferred_hook(self): diff --git a/tests/core/test_services.py b/tests/core/test_services.py index bacea5526..9acca33ea 100644 --- a/tests/core/test_services.py +++ b/tests/core/test_services.py @@ -46,11 +46,13 @@ def test_register_preserves_order(self): @mock.patch.object(services.ServiceManager, 'reconfigure_services') @mock.patch.object(services.ServiceManager, 'stop_services') @mock.patch.object(hookenv, 'hook_name') - @mock.patch.object(hookenv, 'config') - def test_manage_stop(self, config, hook_name, stop_services, reconfigure_services): + @mock.patch.object(hookenv, '_run_atstart') + @mock.patch.object(hookenv, 'config', mock.Mock()) + def test_manage_stop(self, _run_atstart, hook_name, stop_services, reconfigure_services): manager = services.ServiceManager() hook_name.return_value = 'stop' manager.manage() + _run_atstart.assert_called_once_with() stop_services.assert_called_once_with() assert not reconfigure_services.called @@ -58,15 +60,18 @@ def test_manage_stop(self, config, hook_name, stop_services, reconfigure_service @mock.patch.object(services.ServiceManager, 'reconfigure_services') @mock.patch.object(services.ServiceManager, 'stop_services') @mock.patch.object(hookenv, 'hook_name') - @mock.patch.object(hookenv, 'config') - def test_manage_other(self, config, hook_name, stop_services, reconfigure_services, provide_data): + @mock.patch.object(hookenv, '_run_atstart') + @mock.patch.object(hookenv, 'config', mock.Mock()) + def test_manage_other(self, _run_atstart, hook_name, stop_services, reconfigure_services, provide_data): manager = services.ServiceManager() hook_name.return_value = 'config-changed' manager.manage() + _run_atstart.assert_called_once_with() assert not stop_services.called reconfigure_services.assert_called_once_with() provide_data.assert_called_once_with() + @mock.patch.object(hookenv, '_atstart', []) def test_manage_calls_atstart(self): cb = mock.MagicMock() hookenv.atstart(cb) @@ -74,19 +79,23 @@ def test_manage_calls_atstart(self): manager.manage() self.assertTrue(cb.called) - def test_manage_calls_atexit(self): + @mock.patch.object(hookenv, '_run_atstart') + def test_manage_calls_atexit(self, _run_atstart): cb = mock.MagicMock() hookenv.atexit(cb) manager = services.ServiceManager() manager.manage() + _run_atstart.assert_called_once_with() self.assertTrue(cb.called) @mock.patch.object(hookenv, 'config') - def test_manage_config_not_saved(self, config): + @mock.patch.object(hookenv, '_run_atstart') + def test_manage_config_not_saved(self, _run_atstart, config): config = config.return_value config.implicit_save = False manager = services.ServiceManager() manager.manage() + _run_atstart.assert_called_once_with() self.assertFalse(config.save.called) @mock.patch.object(services.ServiceManager, 'save_ready')