From 0b1ee3a7b167eb44ecf3ba21e876a69e67835592 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor Date: Sun, 18 Jun 2023 08:59:06 -0400 Subject: [PATCH] Exit with non-zero if any job failed or was skipped Currently any errors coming the underlying tarsnap commands are logged. This is not threaded back to the entrypoint of the application though, so we end up exiting with a zero code despite one or more of the jobs having failed. This makes monitoring the status of tarsnapper impossible. One would have to know to regularly check its logs to ensure it's actually backing things up. Fixes #39 --- tarsnapper/script.py | 43 ++++++++++++++++++++++++++++++---- tests/test_script.py | 56 ++++++++++++++++++++++++++++++-------------- 2 files changed, 78 insertions(+), 21 deletions(-) diff --git a/tarsnapper/script.py b/tarsnapper/script.py index 1fed919..b056744 100644 --- a/tarsnapper/script.py +++ b/tarsnapper/script.py @@ -262,6 +262,27 @@ def timedelta_string(value): except ValueError as e: raise argparse.ArgumentTypeError('invalid delta value: %r (suffix d, s allowed)' % e) +class Result(object): + def __init__(self, success, error=None): + if success != (error is None): + raise ValueError("If success is True, error must be None") + self.success = success + self.error = error + + def __str__(self): + if self.success: + return "Success" + else: + return "Error: %s" % self.error + + @classmethod + def Failure(self, error=''): + return Result(success=False, error=error) + + @classmethod + def Success(self): + return Result(success=True) + class Command(object): @@ -300,6 +321,7 @@ def run(self, job): backups = sorted(unsorted_backups, key=lambda x: x[1], reverse=True) for backup, _ in backups: print(" %s" % backup) + return Result.Success() class ExpireCommand(Command): @@ -316,12 +338,13 @@ def setup_arg_parser(self, parser): def expire(self, job): if not job.deltas: self.log.info("Skipping '%s', does not define deltas", job.name) - return + return Result.Failure("Skipped %s" % job.name) self.backend.expire(job) + return Result.Success() def run(self, job): - self.expire(job) + return self.expire(job) class MakeCommand(ExpireCommand): @@ -357,7 +380,7 @@ def validate_args(self, args): def run(self, job): if not job.sources: self.log.info("Skipping '%s', does not define sources", job.name) - return + return Result.Failure("Skipped '%s'" % job.name) if job.exec_before: self.backend._exec_util(job.exec_before) @@ -390,6 +413,7 @@ def run(self, job): try: self.backend.make(job) except Exception: + skipped = True self.log.exception("Something went wrong with backup job: '%s'", job.name) @@ -401,6 +425,11 @@ def run(self, job): if not skipped and not self.args.no_expire: self.expire(job) + if skipped: + return Result.Failure("Skipped '%s'" % job.name) + else: + return Result.Success() + COMMANDS = { 'make': MakeCommand, @@ -541,9 +570,10 @@ def main(argv): jobs_to_run = jobs command = args.command(args, log) + results = [] try: for job in jobs_to_run.values(): - command.run(job) + results.append(command.run(job)) for plugin in PLUGINS: plugin.all_jobs_done(args, global_config, args.command) @@ -551,6 +581,11 @@ def main(argv): log.exception("tarsnap execution failed:") return 1 + if all([result.success for result in results]): + return 0 + else: + return 1 + def run(): sys.exit(main(sys.argv[1:]) or 0) diff --git a/tests/test_script.py b/tests/test_script.py index 5c6ef25..4f6d996 100644 --- a/tests/test_script.py +++ b/tests/test_script.py @@ -11,6 +11,13 @@ TarsnapBackend, MakeCommand, ListCommand, ExpireCommand, parse_args, DEFAULT_DATEFORMAT) +def assertAllSucceeded(results): + for result in results: + assert result.success + +def assertAllFailed(results): + for result in results: + assert not result.success class FakeBackend(TarsnapBackend): @@ -66,9 +73,10 @@ def run(self, jobs, archives, **args): cmd = self.command_class(argparse.Namespace(**final_args), self.log, backend_class=FakeBackend) cmd.backend.fake_archives = archives + results = [] for job in (jobs if isinstance(jobs, list) else [jobs]): - cmd.run(job) - return cmd + results.append(cmd.run(job)) + return (cmd, results) def job(self, deltas='1d 2d', name='test', **kwargs): """Make a job object. @@ -98,19 +106,23 @@ def tset_parse(self): def test_pass_along(self): # Short option - cmd = self.run(self.job(), [], tarsnap_options=(('o', '1'),)) + (cmd, results) = self.run(self.job(), [], tarsnap_options=(('o', '1'),)) + assertAllSucceeded(results) assert cmd.backend.match([('-o', '1', '--list-archives')]) # Long option - cmd = self.run(self.job(), [], tarsnap_options=(('foo', '1'),)) + (cmd, results) = self.run(self.job(), [], tarsnap_options=(('foo', '1'),)) + assertAllSucceeded(results) assert cmd.backend.match([('--foo', '1', '--list-archives')]) # No value - cmd = self.run(self.job(), [], tarsnap_options=(('foo',),)) + (cmd, results) = self.run(self.job(), [], tarsnap_options=(('foo',),)) + assertAllSucceeded(results) assert cmd.backend.match([('--foo', '--list-archives')]) # Multiple values - cmd = self.run(self.job(), [], tarsnap_options=(('foo', '1', '2'),)) + (cmd, results) = self.run(self.job(), [], tarsnap_options=(('foo', '1', '2'),)) + assertAllSucceeded(results) assert cmd.backend.match([('--foo', '1', '2', '--list-archives')]) @@ -119,7 +131,8 @@ class TestMake(BaseTest): command_class = MakeCommand def test(self): - cmd = self.run(self.job(), []) + (cmd, results) = self.run(self.job(), []) + assertAllSucceeded(results) assert cmd.backend.match([ ('-c', '-f', 'test-.*', '.*'), ('--list-archives',) @@ -127,18 +140,21 @@ def test(self): def test_no_sources(self): """If no sources are defined, the job is skipped.""" - cmd = self.run(self.job(sources=None), []) + (cmd, results) = self.run(self.job(sources=None), []) + assertAllFailed(results) assert cmd.backend.match([]) def test_excludes(self): - cmd = self.run(self.job(excludes=['foo']), []) + (cmd, results) = self.run(self.job(excludes=['foo']), []) + assertAllSucceeded(results) assert cmd.backend.match([ ('-c', '--exclude', 'foo', '-f', 'test-.*', '.*'), ('--list-archives',) ]) def test_no_expire(self): - cmd = self.run(self.job(), [], no_expire=True) + (cmd, results) = self.run(self.job(), [], no_expire=True) + assertAllSucceeded(results) assert cmd.backend.match([ ('-c', '-f', 'test-.*', '.*'), ]) @@ -146,8 +162,9 @@ def test_no_expire(self): def test_exec(self): """Test ``exec_before`` and ``exec_after`` options. """ - cmd = self.run(self.job(exec_before="echo begin", exec_after="echo end"), + (cmd, results) = self.run(self.job(exec_before="echo begin", exec_after="echo end"), [], no_expire=True) + assertAllSucceeded(results) assert cmd.backend.match([ ('echo begin'), ('-c', '-f', 'test-.*', '.*'), @@ -160,24 +177,26 @@ class TestExpire(BaseTest): command_class = ExpireCommand def test_nothing_to_do(self): - cmd = self.run(self.job(deltas='1d 10d'), [ + (cmd, results) = self.run(self.job(deltas='1d 10d'), [ self.filename('1d'), self.filename('5d'), ]) + assertAllSucceeded(results) assert cmd.backend.match([ ('--list-archives',) ]) def test_no_deltas(self): """If a job does not define deltas, we skip it.""" - cmd = self.run(self.job(deltas=None), [ + (cmd, results) = self.run(self.job(deltas=None), [ self.filename('1d'), self.filename('5d'), ]) + assertAllFailed(results) assert cmd.backend.match([]) def test_something_to_expire(self): - cmd = self.run(self.job(deltas='1d 2d'), [ + (cmd, results) = self.run(self.job(deltas='1d 2d'), [ self.filename('1d'), self.filename('5d'), ]) @@ -187,10 +206,11 @@ def test_something_to_expire(self): ]) def test_aliases(self): - cmd = self.run(self.job(deltas='1d 2d', aliases=['alias']), [ + (cmd, results) = self.run(self.job(deltas='1d 2d', aliases=['alias']), [ self.filename('1d'), self.filename('5d', name='alias'), ]) + assertAllSucceeded(results) assert cmd.backend.match([ ('--list-archives',), ('-d', '-f', 'alias-.*'), @@ -201,9 +221,10 @@ def test_date_name_mismatch(self): we won't stumble over "home-dev-$date". This can be an issue due to the way we try to parse the dates in filenames. """ - cmd = self.run(self.job(name="home"), [ + (cmd, results) = self.run(self.job(name="home"), [ self.filename('1d', name="home-dev"), ]) + assertAllSucceeded(results) class TestList(BaseTest): @@ -211,12 +232,13 @@ class TestList(BaseTest): command_class = ListCommand def test(self): - cmd = self.run([self.job(), self.job(name='foo')], [ + (cmd, results) = self.run([self.job(), self.job(name='foo')], [ self.filename('1d'), self.filename('5d'), self.filename('1d', name='foo'), self.filename('1d', name='something-else'), ]) + assertAllSucceeded(results) # We ask to list two jobs, but only one --list-archives call is # necessary. assert cmd.backend.match([