Issue#743: fetch.ubuntu._run_with_retries raises on unexpected exitco…

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)
 

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):

tests/core/test_services.py

@@ -46,47 +46,56 @@ 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
 
     @mock.patch.object(services.ServiceManager, 'provide_data')
     @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)
         manager = services.ServiceManager()
         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')