Просмотр исходного кода

cleanup: Removes redundant retry decorators from exec_ssh_cmd callers

exec_ssh_cmd already retries up to 5 times. Functions that also have
the @retry_on_error() decorator and only call exec_ssh_cmd (or other
already-retried helpers) are compounding retries to 25x or even more.

Remove the decorator from:
- utils: create_service, restart_service, start_service, stop_service
  (all inner calls go through test_ssh_path / exec_ssh_cmd, both retried)
- replicator: _copy_replicator_cmd, _exec_replicator, _setup_replicator
  (_setup_replicator is also wrapped externally by init_replicator, making
  its own decorator triply redundant)

Updates tests that used get_wrapped_function to bypass the (now removed)
decorator. They now call the methods directly.
Claudiu Belu 4 недель назад
Родитель
Сommit
1e73dade13
3 измененных файлов с 4 добавлено и 20 удалено
  1. 0 3
      coriolis/providers/replicator.py
  2. 4 13
      coriolis/tests/providers/test_replicator.py
  3. 0 4
      coriolis/utils.py

+ 0 - 3
coriolis/providers/replicator.py

@@ -570,7 +570,6 @@ class Replicator(object):
             ssh, "sudo mv %s %s" % (tmp, remotePath), get_pty=True)
             ssh, "sudo mv %s %s" % (tmp, remotePath), get_pty=True)
         sftp.close()
         sftp.close()
 
 
-    @utils.retry_on_error()
     def _copy_replicator_cmd(self, ssh):
     def _copy_replicator_cmd(self, ssh):
         local_path = os.path.join(
         local_path = os.path.join(
             utils.get_resources_bin_dir(), 'replicator')
             utils.get_resources_bin_dir(), 'replicator')
@@ -616,7 +615,6 @@ class Replicator(object):
                 ssh, "sudo usermod -aG disk %s" % REPLICATOR_USERNAME,
                 ssh, "sudo usermod -aG disk %s" % REPLICATOR_USERNAME,
                 get_pty=True)
                 get_pty=True)
 
 
-    @utils.retry_on_error()
     def _exec_replicator(self, ssh, port, certs, state_file):
     def _exec_replicator(self, ssh, port, certs, state_file):
         cmdline = ("%(replicator_path)s run -hash-method=%(hash_method)s "
         cmdline = ("%(replicator_path)s run -hash-method=%(hash_method)s "
                    "-ignore-mounted-disks=%(ignore_mounted)s "
                    "-ignore-mounted-disks=%(ignore_mounted)s "
@@ -738,7 +736,6 @@ class Replicator(object):
             LOG.warn("Could not change SELinux context of replicator binary. "
             LOG.warn("Could not change SELinux context of replicator binary. "
                      "Error was:%s", utils.get_exception_details())
                      "Error was:%s", utils.get_exception_details())
 
 
-    @utils.retry_on_error()
     def _setup_replicator(self, ssh):
     def _setup_replicator(self, ssh):
         # copy the binary, set up the service, generate certificates,
         # copy the binary, set up the service, generate certificates,
         # start service
         # start service

+ 4 - 13
coriolis/tests/providers/test_replicator.py

@@ -807,10 +807,7 @@ class ReplicatorTestCase(test_base.CoriolisBaseTestCase):
     def test__copy_replicator_cmd(
     def test__copy_replicator_cmd(
             self, mock_exec_ssh_cmd, mock_copy_file,
             self, mock_exec_ssh_cmd, mock_copy_file,
             mock_get_resources_bin_dir, mock_join):
             mock_get_resources_bin_dir, mock_join):
-        original_copy_replicator_cmd = testutils.get_wrapped_function(
-            self.replicator._copy_replicator_cmd)
-
-        original_copy_replicator_cmd(self.replicator, self._ssh)
+        self.replicator._copy_replicator_cmd(self._ssh)
 
 
         mock_get_resources_bin_dir.assert_called_once()
         mock_get_resources_bin_dir.assert_called_once()
         mock_join.assert_called_once_with(
         mock_join.assert_called_once_with(
@@ -895,11 +892,8 @@ class ReplicatorTestCase(test_base.CoriolisBaseTestCase):
             "srv_crt": mock.sentinel.srv_crt,
             "srv_crt": mock.sentinel.srv_crt,
             "srv_key": mock.sentinel.srv_key,
             "srv_key": mock.sentinel.srv_key,
         }
         }
-        original_exec_replicator = testutils.get_wrapped_function(
-            self.replicator._exec_replicator)
-
-        original_exec_replicator(
-            self.replicator, self._ssh, self.conn_info['port'], certs,
+        self.replicator._exec_replicator(
+            self._ssh, self.conn_info['port'], certs,
             mock.sentinel.state)
             mock.sentinel.state)
 
 
         mock_create_service.assert_called_once_with(
         mock_create_service.assert_called_once_with(
@@ -997,10 +991,7 @@ class ReplicatorTestCase(test_base.CoriolisBaseTestCase):
             mock_get_replicator_state_file):
             mock_get_replicator_state_file):
         mock_setup_replicator_group.return_value = True
         mock_setup_replicator_group.return_value = True
 
 
-        original_setup_replicator = testutils.get_wrapped_function(
-            self.replicator._setup_replicator)
-
-        result = original_setup_replicator(self.replicator, self._ssh)
+        result = self.replicator._setup_replicator(self._ssh)
 
 
         mock_get_replicator_state_file.assert_called_once()
         mock_get_replicator_state_file.assert_called_once()
         mock_copy_file.assert_called_once_with(
         mock_copy_file.assert_called_once_with(

+ 0 - 4
coriolis/utils.py

@@ -873,7 +873,6 @@ def _has_systemd(ssh):
             test_ssh_path(ssh, "/usr/lib/systemd/system"))
             test_ssh_path(ssh, "/usr/lib/systemd/system"))
 
 
 
 
-@retry_on_error()
 def create_service(ssh, cmdline, svcname, run_as=None, start=True):
 def create_service(ssh, cmdline, svcname, run_as=None, start=True):
     # Simplistic check for init system. We usually use official images,
     # Simplistic check for init system. We usually use official images,
     # and none of the supported operating systems come with both upstart
     # and none of the supported operating systems come with both upstart
@@ -889,7 +888,6 @@ def create_service(ssh, cmdline, svcname, run_as=None, start=True):
             "could not determine init system")
             "could not determine init system")
 
 
 
 
-@retry_on_error()
 def restart_service(ssh, svcname):
 def restart_service(ssh, svcname):
     if _has_systemd(ssh):
     if _has_systemd(ssh):
         exec_ssh_cmd(ssh, "sudo systemctl restart %s" % svcname, get_pty=True)
         exec_ssh_cmd(ssh, "sudo systemctl restart %s" % svcname, get_pty=True)
@@ -899,7 +897,6 @@ def restart_service(ssh, svcname):
         raise exception.UnrecognizedWorkerInitSystem()
         raise exception.UnrecognizedWorkerInitSystem()
 
 
 
 
-@retry_on_error()
 def start_service(ssh, svcname):
 def start_service(ssh, svcname):
     if _has_systemd(ssh):
     if _has_systemd(ssh):
         exec_ssh_cmd(ssh, "sudo systemctl start %s" % svcname, get_pty=True)
         exec_ssh_cmd(ssh, "sudo systemctl start %s" % svcname, get_pty=True)
@@ -909,7 +906,6 @@ def start_service(ssh, svcname):
         raise exception.UnrecognizedWorkerInitSystem()
         raise exception.UnrecognizedWorkerInitSystem()
 
 
 
 
-@retry_on_error()
 def stop_service(ssh, svcname):
 def stop_service(ssh, svcname):
     if _has_systemd(ssh):
     if _has_systemd(ssh):
         exec_ssh_cmd(ssh, "sudo systemctl stop %s" % svcname, get_pty=True)
         exec_ssh_cmd(ssh, "sudo systemctl stop %s" % svcname, get_pty=True)