From 69f6695253170fa4e09738c6ca0b31b0a95b7aae Mon Sep 17 00:00:00 2001 From: Soumik Dutta Date: Sun, 5 Mar 2023 19:27:32 +0530 Subject: [PATCH 1/8] Add support for healthchecks "log" feature #628 Signed-off-by: Soumik Dutta --- borgmatic/commands/borgmatic.py | 18 ++++++++++++++++++ borgmatic/config/schema.yaml | 1 + borgmatic/hooks/cronhub.py | 1 + borgmatic/hooks/cronitor.py | 1 + borgmatic/hooks/healthchecks.py | 3 ++- borgmatic/hooks/monitor.py | 1 + borgmatic/hooks/ntfy.py | 1 + 7 files changed, 25 insertions(+), 1 deletion(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index e08df97..b5e8664 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -152,6 +152,24 @@ def run_configuration(config_filename, config, arguments): encountered_error = error error_repository = repository_path + try: + # send logs irrespective of error + dispatch.call_hooks( + 'ping_monitor', + hooks, + config_filename, + monitor.MONITOR_HOOK_NAMES, + monitor.State.LOG, + monitoring_log_level, + global_arguments.dry_run, + ) + except (OSError, CalledProcessError) as error: + if command.considered_soft_failure(config_filename, error): + return + + encountered_error = error + yield from log_error_records('{}: Error pinging monitor'.format(config_filename), error) + if not encountered_error: try: if using_primary_action: diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 2f873b7..0b2a378 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -1199,6 +1199,7 @@ properties: - start - finish - fail + - log uniqueItems: true description: | List of one or more monitoring states to ping for: diff --git a/borgmatic/hooks/cronhub.py b/borgmatic/hooks/cronhub.py index b93788e..b3a3521 100644 --- a/borgmatic/hooks/cronhub.py +++ b/borgmatic/hooks/cronhub.py @@ -10,6 +10,7 @@ MONITOR_STATE_TO_CRONHUB = { monitor.State.START: 'start', monitor.State.FINISH: 'finish', monitor.State.FAIL: 'fail', + monitor.State.LOG: 'log', } diff --git a/borgmatic/hooks/cronitor.py b/borgmatic/hooks/cronitor.py index 8866a6a..c01d5f6 100644 --- a/borgmatic/hooks/cronitor.py +++ b/borgmatic/hooks/cronitor.py @@ -10,6 +10,7 @@ MONITOR_STATE_TO_CRONITOR = { monitor.State.START: 'run', monitor.State.FINISH: 'complete', monitor.State.FAIL: 'fail', + monitor.State.LOG: 'log', } diff --git a/borgmatic/hooks/healthchecks.py b/borgmatic/hooks/healthchecks.py index 03d012a..6ad8449 100644 --- a/borgmatic/hooks/healthchecks.py +++ b/borgmatic/hooks/healthchecks.py @@ -10,6 +10,7 @@ MONITOR_STATE_TO_HEALTHCHECKS = { monitor.State.START: 'start', monitor.State.FINISH: None, # Healthchecks doesn't append to the URL for the finished state. monitor.State.FAIL: 'fail', + monitor.State.LOG: 'log', } PAYLOAD_TRUNCATION_INDICATOR = '...\n' @@ -117,7 +118,7 @@ def ping_monitor(hook_config, config_filename, state, monitoring_log_level, dry_ ) logger.debug('{}: Using Healthchecks ping URL {}'.format(config_filename, ping_url)) - if state in (monitor.State.FINISH, monitor.State.FAIL): + if state in (monitor.State.FINISH, monitor.State.FAIL, monitor.State.LOG): payload = format_buffered_logs_for_payload() else: payload = '' diff --git a/borgmatic/hooks/monitor.py b/borgmatic/hooks/monitor.py index 846fca1..c016817 100644 --- a/borgmatic/hooks/monitor.py +++ b/borgmatic/hooks/monitor.py @@ -7,3 +7,4 @@ class State(Enum): START = 1 FINISH = 2 FAIL = 3 + LOG = 4 diff --git a/borgmatic/hooks/ntfy.py b/borgmatic/hooks/ntfy.py index 99ed254..fda912c 100644 --- a/borgmatic/hooks/ntfy.py +++ b/borgmatic/hooks/ntfy.py @@ -10,6 +10,7 @@ MONITOR_STATE_TO_NTFY = { monitor.State.START: None, monitor.State.FINISH: None, monitor.State.FAIL: None, + monitor.State.LOG: None, } From 1573d68fe243d030006271f7c17a6515e2fec407 Mon Sep 17 00:00:00 2001 From: Soumik Dutta Date: Sun, 5 Mar 2023 21:57:13 +0530 Subject: [PATCH 2/8] update schema.yaml description also add monitor.State.LOG to cronitor. Signed-off-by: Soumik Dutta --- borgmatic/config/schema.yaml | 8 ++++---- borgmatic/hooks/cronitor.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 0b2a378..9242390 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -951,9 +951,9 @@ properties: name: type: string description: | - This is used to tag the database dump file - with a name. It is not the path to the database - file itself. The name "all" has no special + This is used to tag the database dump file + with a name. It is not the path to the database + file itself. The name "all" has no special meaning for SQLite databases. example: users path: @@ -1168,7 +1168,7 @@ properties: type: string description: | Healthchecks ping URL or UUID to notify when a - backup begins, ends, or errors. + backup begins, ends, errors or just to send logs. example: https://hc-ping.com/your-uuid-here verify_tls: type: boolean diff --git a/borgmatic/hooks/cronitor.py b/borgmatic/hooks/cronitor.py index c01d5f6..f3ab13b 100644 --- a/borgmatic/hooks/cronitor.py +++ b/borgmatic/hooks/cronitor.py @@ -10,7 +10,7 @@ MONITOR_STATE_TO_CRONITOR = { monitor.State.START: 'run', monitor.State.FINISH: 'complete', monitor.State.FAIL: 'fail', - monitor.State.LOG: 'log', + monitor.State.LOG: 'ok', } From 45256ae33f39c40481a402b5b8dbf0ba7b34ec6f Mon Sep 17 00:00:00 2001 From: Soumik Dutta Date: Mon, 6 Mar 2023 03:38:08 +0530 Subject: [PATCH 3/8] add test for healthchecks Signed-off-by: Soumik Dutta --- borgmatic/commands/borgmatic.py | 21 +++++++++++---------- borgmatic/hooks/cronhub.py | 1 - borgmatic/hooks/cronitor.py | 1 - borgmatic/hooks/ntfy.py | 7 ------- tests/unit/hooks/test_healthchecks.py | 17 +++++++++++++++++ 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index b5e8664..6bc2456 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -153,16 +153,17 @@ def run_configuration(config_filename, config, arguments): error_repository = repository_path try: - # send logs irrespective of error - dispatch.call_hooks( - 'ping_monitor', - hooks, - config_filename, - monitor.MONITOR_HOOK_NAMES, - monitor.State.LOG, - monitoring_log_level, - global_arguments.dry_run, - ) + if using_primary_action: + # send logs irrespective of error + dispatch.call_hooks( + 'ping_monitor', + hooks, + config_filename, + monitor.MONITOR_HOOK_NAMES, + monitor.State.LOG, + monitoring_log_level, + global_arguments.dry_run, + ) except (OSError, CalledProcessError) as error: if command.considered_soft_failure(config_filename, error): return diff --git a/borgmatic/hooks/cronhub.py b/borgmatic/hooks/cronhub.py index b3a3521..b93788e 100644 --- a/borgmatic/hooks/cronhub.py +++ b/borgmatic/hooks/cronhub.py @@ -10,7 +10,6 @@ MONITOR_STATE_TO_CRONHUB = { monitor.State.START: 'start', monitor.State.FINISH: 'finish', monitor.State.FAIL: 'fail', - monitor.State.LOG: 'log', } diff --git a/borgmatic/hooks/cronitor.py b/borgmatic/hooks/cronitor.py index f3ab13b..8866a6a 100644 --- a/borgmatic/hooks/cronitor.py +++ b/borgmatic/hooks/cronitor.py @@ -10,7 +10,6 @@ MONITOR_STATE_TO_CRONITOR = { monitor.State.START: 'run', monitor.State.FINISH: 'complete', monitor.State.FAIL: 'fail', - monitor.State.LOG: 'ok', } diff --git a/borgmatic/hooks/ntfy.py b/borgmatic/hooks/ntfy.py index fda912c..3f897aa 100644 --- a/borgmatic/hooks/ntfy.py +++ b/borgmatic/hooks/ntfy.py @@ -6,13 +6,6 @@ from borgmatic.hooks import monitor logger = logging.getLogger(__name__) -MONITOR_STATE_TO_NTFY = { - monitor.State.START: None, - monitor.State.FINISH: None, - monitor.State.FAIL: None, - monitor.State.LOG: None, -} - def initialize_monitor( ping_url, config_filename, monitoring_log_level, dry_run diff --git a/tests/unit/hooks/test_healthchecks.py b/tests/unit/hooks/test_healthchecks.py index ee78e52..d577953 100644 --- a/tests/unit/hooks/test_healthchecks.py +++ b/tests/unit/hooks/test_healthchecks.py @@ -184,6 +184,23 @@ def test_ping_monitor_hits_ping_url_for_fail_state(): ) +def test_ping_monitor_hits_ping_url_for_log_state(): + hook_config = {'ping_url': 'https://example.com'} + payload = 'data' + flexmock(module).should_receive('format_buffered_logs_for_payload').and_return(payload) + flexmock(module.requests).should_receive('post').with_args( + 'https://example.com/log', data=payload.encode('utf'), verify=True + ).and_return(flexmock(ok=True)) + + module.ping_monitor( + hook_config, + 'config.yaml', + state=module.monitor.State.LOG, + monitoring_log_level=1, + dry_run=False, + ) + + def test_ping_monitor_with_ping_uuid_hits_corresponding_url(): hook_config = {'ping_url': 'abcd-efgh-ijkl-mnop'} payload = 'data' From e211863cba14bdd3f8f63cdc0712843694d47a4e Mon Sep 17 00:00:00 2001 From: Soumik Dutta Date: Mon, 6 Mar 2023 05:12:24 +0530 Subject: [PATCH 4/8] update test_borgmatic.py Signed-off-by: Soumik Dutta --- tests/unit/commands/test_borgmatic.py | 45 ++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 38b28fd..e2da3b8 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -41,8 +41,10 @@ def test_run_configuration_logs_monitor_start_error(): flexmock(module.dispatch).should_receive('call_hooks').and_raise(OSError).and_return( None ).and_return(None) - expected_results = [flexmock()] - flexmock(module).should_receive('log_error_records').and_return(expected_results) + expected_results = [flexmock(), flexmock()] + flexmock(module).should_receive('log_error_records').and_return( + [expected_results[0]] + ).and_return([expected_results[1]]) flexmock(module).should_receive('run_actions').never() config = {'location': {'repositories': ['foo']}} arguments = {'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock()} @@ -99,7 +101,7 @@ def test_run_configuration_bails_for_actions_soft_failure(): assert results == [] -def test_run_configuration_logs_monitor_finish_error(): +def test_run_configuration_logs_monitor_log_error(): flexmock(module).should_receive('verbosity_to_log_level').and_return(logging.INFO) flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock()) flexmock(module.dispatch).should_receive('call_hooks').and_return(None).and_return( @@ -116,13 +118,48 @@ def test_run_configuration_logs_monitor_finish_error(): assert results == expected_results +def test_run_configuration_bails_for_monitor_log_soft_failure(): + flexmock(module).should_receive('verbosity_to_log_level').and_return(logging.INFO) + flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock()) + error = subprocess.CalledProcessError(borgmatic.hooks.command.SOFT_FAIL_EXIT_CODE, 'try again') + flexmock(module.dispatch).should_receive('call_hooks').and_return(None).and_return( + None + ).and_raise(error) + flexmock(module).should_receive('log_error_records').never() + flexmock(module).should_receive('run_actions').and_return([]) + flexmock(module.command).should_receive('considered_soft_failure').and_return(True) + config = {'location': {'repositories': ['foo']}} + arguments = {'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock()} + + results = list(module.run_configuration('test.yaml', config, arguments)) + + assert results == [] + + +def test_run_configuration_logs_monitor_finish_error(): + flexmock(module).should_receive('verbosity_to_log_level').and_return(logging.INFO) + flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock()) + flexmock(module.dispatch).should_receive('call_hooks').and_return(None).and_return( + None + ).and_return(None).and_raise(OSError) + expected_results = [flexmock()] + flexmock(module).should_receive('log_error_records').and_return(expected_results) + flexmock(module).should_receive('run_actions').and_return([]) + config = {'location': {'repositories': ['foo']}} + arguments = {'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock()} + + results = list(module.run_configuration('test.yaml', config, arguments)) + + assert results == expected_results + + def test_run_configuration_bails_for_monitor_finish_soft_failure(): flexmock(module).should_receive('verbosity_to_log_level').and_return(logging.INFO) flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock()) error = subprocess.CalledProcessError(borgmatic.hooks.command.SOFT_FAIL_EXIT_CODE, 'try again') flexmock(module.dispatch).should_receive('call_hooks').and_return(None).and_return( None - ).and_raise(error) + ).and_raise(None).and_raise(error) flexmock(module).should_receive('log_error_records').never() flexmock(module).should_receive('run_actions').and_return([]) flexmock(module.command).should_receive('considered_soft_failure').and_return(True) From f442aeae9c930aa2d2d4c482861823462f01c877 Mon Sep 17 00:00:00 2001 From: Soumik Dutta Date: Mon, 6 Mar 2023 05:21:56 +0530 Subject: [PATCH 5/8] fix logs_monitor_start_error() Signed-off-by: Soumik Dutta --- tests/unit/commands/test_borgmatic.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index e2da3b8..fc3f34d 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -40,11 +40,9 @@ def test_run_configuration_logs_monitor_start_error(): flexmock(module.borg_version).should_receive('local_borg_version').and_return(flexmock()) flexmock(module.dispatch).should_receive('call_hooks').and_raise(OSError).and_return( None - ).and_return(None) - expected_results = [flexmock(), flexmock()] - flexmock(module).should_receive('log_error_records').and_return( - [expected_results[0]] - ).and_return([expected_results[1]]) + ).and_return(None).and_return(None) + expected_results = [flexmock()] + flexmock(module).should_receive('log_error_records').and_return(expected_results) flexmock(module).should_receive('run_actions').never() config = {'location': {'repositories': ['foo']}} arguments = {'global': flexmock(monitoring_verbosity=1, dry_run=False), 'create': flexmock()} From 4fcfddbe0804431be8419d5c7db7635741665c3b Mon Sep 17 00:00:00 2001 From: Soumik Dutta Date: Mon, 6 Mar 2023 19:58:57 +0530 Subject: [PATCH 6/8] return early if unsupported state is passed Signed-off-by: Soumik Dutta --- borgmatic/hooks/cronhub.py | 8 ++++++++ borgmatic/hooks/cronitor.py | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/borgmatic/hooks/cronhub.py b/borgmatic/hooks/cronhub.py index b93788e..8ec5aac 100644 --- a/borgmatic/hooks/cronhub.py +++ b/borgmatic/hooks/cronhub.py @@ -27,6 +27,14 @@ def ping_monitor(hook_config, config_filename, state, monitoring_log_level, dry_ Ping the configured Cronhub URL, modified with the monitor.State. Use the given configuration filename in any log entries. If this is a dry run, then don't actually ping anything. ''' + if state not in MONITOR_STATE_TO_CRONHUB: + logger.debug( + '{}: Ignoring unsupported monitoring {} in Cronhub hook'.format( + config_filename, state.name.lower() + ) + ) + return + dry_run_label = ' (dry run; not actually pinging)' if dry_run else '' formatted_state = '/{}/'.format(MONITOR_STATE_TO_CRONHUB[state]) ping_url = ( diff --git a/borgmatic/hooks/cronitor.py b/borgmatic/hooks/cronitor.py index 8866a6a..f5164e4 100644 --- a/borgmatic/hooks/cronitor.py +++ b/borgmatic/hooks/cronitor.py @@ -27,6 +27,14 @@ def ping_monitor(hook_config, config_filename, state, monitoring_log_level, dry_ Ping the configured Cronitor URL, modified with the monitor.State. Use the given configuration filename in any log entries. If this is a dry run, then don't actually ping anything. ''' + if state not in MONITOR_STATE_TO_CRONITOR: + logger.debug( + '{}: Ignoring unsupported monitoring {} in Cronitor hook'.format( + config_filename, state.name.lower() + ) + ) + return + dry_run_label = ' (dry run; not actually pinging)' if dry_run else '' ping_url = '{}/{}'.format(hook_config['ping_url'], MONITOR_STATE_TO_CRONITOR[state]) From 98e429594e33cadf39579b3d271be0b2efb0ccb9 Mon Sep 17 00:00:00 2001 From: Soumik Dutta Date: Mon, 6 Mar 2023 20:31:00 +0530 Subject: [PATCH 7/8] added tests to make sure unsupported log states are detected Signed-off-by: Soumik Dutta --- tests/unit/hooks/test_cronhub.py | 10 ++++++++++ tests/unit/hooks/test_cronitor.py | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/tests/unit/hooks/test_cronhub.py b/tests/unit/hooks/test_cronhub.py index 14e8eb2..9007d2c 100644 --- a/tests/unit/hooks/test_cronhub.py +++ b/tests/unit/hooks/test_cronhub.py @@ -102,3 +102,13 @@ def test_ping_monitor_with_other_error_logs_warning(): monitoring_log_level=1, dry_run=False, ) + + +def test_ping_monitor_with_unsupported_monitoring_state(): + hook_config = {'ping_url': 'https://example.com'} + flexmock(module.logger).should_receive("debug").once().with_args( + '{}: Ignoring unsupported monitoring {} in Cronhub hook'.format("config.yaml", "log") + ) + module.ping_monitor( + hook_config, 'config.yaml', module.monitor.State.LOG, monitoring_log_level=1, dry_run=False, + ) diff --git a/tests/unit/hooks/test_cronitor.py b/tests/unit/hooks/test_cronitor.py index 4b762d8..51fba98 100644 --- a/tests/unit/hooks/test_cronitor.py +++ b/tests/unit/hooks/test_cronitor.py @@ -87,3 +87,13 @@ def test_ping_monitor_with_other_error_logs_warning(): monitoring_log_level=1, dry_run=False, ) + + +def test_ping_monitor_with_unsupported_monitoring_state(): + hook_config = {'ping_url': 'https://example.com'} + flexmock(module.logger).should_receive("debug").once().with_args( + '{}: Ignoring unsupported monitoring {} in Cronitor hook'.format("config.yaml", "log") + ) + module.ping_monitor( + hook_config, 'config.yaml', module.monitor.State.LOG, monitoring_log_level=1, dry_run=False, + ) From 044ae7869af4457e017e117abe57ebe998edab5e Mon Sep 17 00:00:00 2001 From: Soumik Dutta Date: Wed, 8 Mar 2023 03:30:12 +0530 Subject: [PATCH 8/8] fix tests Signed-off-by: Soumik Dutta --- borgmatic/hooks/cronhub.py | 4 +--- borgmatic/hooks/cronitor.py | 4 +--- tests/unit/hooks/test_cronhub.py | 4 +--- tests/unit/hooks/test_cronitor.py | 4 +--- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/borgmatic/hooks/cronhub.py b/borgmatic/hooks/cronhub.py index 8ec5aac..cd0ffa5 100644 --- a/borgmatic/hooks/cronhub.py +++ b/borgmatic/hooks/cronhub.py @@ -29,9 +29,7 @@ def ping_monitor(hook_config, config_filename, state, monitoring_log_level, dry_ ''' if state not in MONITOR_STATE_TO_CRONHUB: logger.debug( - '{}: Ignoring unsupported monitoring {} in Cronhub hook'.format( - config_filename, state.name.lower() - ) + f'{config_filename}: Ignoring unsupported monitoring {state.name.lower()} in Cronhub hook' ) return diff --git a/borgmatic/hooks/cronitor.py b/borgmatic/hooks/cronitor.py index f5164e4..633b4c3 100644 --- a/borgmatic/hooks/cronitor.py +++ b/borgmatic/hooks/cronitor.py @@ -29,9 +29,7 @@ def ping_monitor(hook_config, config_filename, state, monitoring_log_level, dry_ ''' if state not in MONITOR_STATE_TO_CRONITOR: logger.debug( - '{}: Ignoring unsupported monitoring {} in Cronitor hook'.format( - config_filename, state.name.lower() - ) + f'{config_filename}: Ignoring unsupported monitoring {state.name.lower()} in Cronitor hook' ) return diff --git a/tests/unit/hooks/test_cronhub.py b/tests/unit/hooks/test_cronhub.py index 9007d2c..f470b88 100644 --- a/tests/unit/hooks/test_cronhub.py +++ b/tests/unit/hooks/test_cronhub.py @@ -106,9 +106,7 @@ def test_ping_monitor_with_other_error_logs_warning(): def test_ping_monitor_with_unsupported_monitoring_state(): hook_config = {'ping_url': 'https://example.com'} - flexmock(module.logger).should_receive("debug").once().with_args( - '{}: Ignoring unsupported monitoring {} in Cronhub hook'.format("config.yaml", "log") - ) + flexmock(module.requests).should_receive('get').never() module.ping_monitor( hook_config, 'config.yaml', module.monitor.State.LOG, monitoring_log_level=1, dry_run=False, ) diff --git a/tests/unit/hooks/test_cronitor.py b/tests/unit/hooks/test_cronitor.py index 51fba98..7ec1e2e 100644 --- a/tests/unit/hooks/test_cronitor.py +++ b/tests/unit/hooks/test_cronitor.py @@ -91,9 +91,7 @@ def test_ping_monitor_with_other_error_logs_warning(): def test_ping_monitor_with_unsupported_monitoring_state(): hook_config = {'ping_url': 'https://example.com'} - flexmock(module.logger).should_receive("debug").once().with_args( - '{}: Ignoring unsupported monitoring {} in Cronitor hook'.format("config.yaml", "log") - ) + flexmock(module.requests).should_receive('get').never() module.ping_monitor( hook_config, 'config.yaml', module.monitor.State.LOG, monitoring_log_level=1, dry_run=False, )