openstack
/
trove
Code Issues Proposed changes

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:
Petr Malik 2015-04-24 15:36:06 -04:00
parent 0ba5aeea2d
commit d68ecc03cb
3 changed files with 7 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
trove/guestagent/strategies/backup/base.py
View File

@ -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

2
trove/tests/unittests/backup/test_backupagent.py
View File

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

6
trove/tests/unittests/guestagent/test_backups.py
View File

@ -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()