Fix process attribute check in BackupRunner
Base 'BackupRunner' checks for the 'process' attribute on __exit__ and terminates the runner process if any. It however always initializes the process to 'None' in the __init__. That check is therefore always True. It should change the condition to check for 'None' value instead. I also made the 'run' method 'protected' to keep it consistent with the 'pre' and 'post' methods (and also the restore runner which has the process handling method protected as well). Change-Id: I6e98a47d33de7da95d884fb67b2cfab918183b3b Closes-Bug: 1448279
This commit is contained in:
parent
0ba5aeea2d
commit
d68ecc03cb
@ -57,7 +57,7 @@ class BackupRunner(Strategy):
|
||||
def backup_type(self):
|
||||
return type(self).__name__
|
||||
|
||||
def run(self):
|
||||
def _run(self):
|
||||
LOG.debug("BackupRunner running cmd: %s", self.command)
|
||||
self.process = subprocess.Popen(self.command, shell=True,
|
||||
stdout=subprocess.PIPE,
|
||||
@ -68,7 +68,7 @@ class BackupRunner(Strategy):
|
||||
def __enter__(self):
|
||||
"""Start up the process."""
|
||||
self._run_pre_backup()
|
||||
self.run()
|
||||
self._run()
|
||||
return self
|
||||
|
||||
def __exit__(self, exc_type, exc_value, traceback):
|
||||
@ -76,7 +76,7 @@ class BackupRunner(Strategy):
|
||||
if exc_type is not None:
|
||||
return False
|
||||
|
||||
if hasattr(self, 'process'):
|
||||
if getattr(self, 'process', None):
|
||||
try:
|
||||
# Send a sigterm to the session leader, so that all
|
||||
# child processes are killed and cleaned up on terminate
|
||||
|
@ -397,7 +397,7 @@ class BackupAgentTest(testtools.TestCase):
|
||||
|
||||
mysql_impl.InnoBackupExIncremental.metadata = MagicMock(
|
||||
return_value=meta)
|
||||
mysql_impl.InnoBackupExIncremental.run = MagicMock(
|
||||
mysql_impl.InnoBackupExIncremental._run = MagicMock(
|
||||
return_value=True)
|
||||
mysql_impl.InnoBackupExIncremental.__exit__ = MagicMock(
|
||||
return_value=True)
|
||||
|
@ -328,18 +328,18 @@ class CouchbaseBackupTests(testtools.TestCase):
|
||||
|
||||
def test_backup_success(self):
|
||||
self.backup_runner.__exit__ = mock.Mock()
|
||||
self.backup_runner.run = mock.Mock()
|
||||
self.backup_runner._run = mock.Mock()
|
||||
self.backup_runner._run_pre_backup = mock.Mock()
|
||||
self.backup_runner._run_post_backup = mock.Mock()
|
||||
utils.execute_with_timeout = mock.Mock(return_value=None)
|
||||
with self.backup_runner(12345):
|
||||
pass
|
||||
self.assertTrue(self.backup_runner.run)
|
||||
self.assertTrue(self.backup_runner._run)
|
||||
self.assertTrue(self.backup_runner._run_pre_backup)
|
||||
self.assertTrue(self.backup_runner._run_post_backup)
|
||||
|
||||
def test_backup_failed_due_to_run_backup(self):
|
||||
self.backup_runner.run = mock.Mock(
|
||||
self.backup_runner._run = mock.Mock(
|
||||
side_effect=exception.ProcessExecutionError('test'))
|
||||
self.backup_runner._run_pre_backup = mock.Mock()
|
||||
self.backup_runner._run_post_backup = mock.Mock()
|
||||
|
Loading…
Reference in New Issue
Block a user