Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add healthcheck URL to protect against silent failures #160

Open
wants to merge 6 commits into
base: testing
Choose a base branch
from

Conversation

shukon
Copy link

@shukon shukon commented Jun 9, 2024

This PR continues #142. If there is anything I can do to help merging, just tell me.

Problem

There is no easy solution for external monitoring of the backups. Since there's nothing worse than setting up a backup to discover after 6 months (or years) that it stopped working after a couple of days.
Yunohost sending mails alleviates the problem somewhat, but it doesn't protect against silent failures.
This PR fixes #117 - it can be extended later on by multiple options for backup monitoring, something like the backup-hooks in borgmatic, but it doesn't have to.

Solution

Introduce an optional config-variable called healthchecks_url that is called upon success or failure (in the event of failure it includes the logs). You can send this either to your own hosted version of healthchecks.io or to the official one.

PR Status

  • Code finished and ready to be reviewed/tested
  • The fix/enhancement were manually tested (if applicable)
    • -> I tested install with and without URL, with and without failures as well as changing the URL.

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

@Limezy
Copy link

Limezy commented Jun 22, 2024

Wow, that's great, I'll use it with my Ntfy instance

@alexAubin alexAubin deleted the branch YunoHost-Apps:testing August 5, 2024 16:37
@alexAubin alexAubin closed this Aug 5, 2024
@alexAubin alexAubin reopened this Aug 5, 2024
@shukon
Copy link
Author

shukon commented Nov 20, 2024

I rebased to reflect changes to testing. Is it likely this would be accepted into the package?

@alexAubin
Copy link
Member

(smol poke @zamentur , you may have an opinion on this, didnt look at the diff but sounds relevant idk)

Copy link
Contributor

@zamentur zamentur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, however note that if /etc/yunohost.app/borg/settings.yml is empty (by error), this will sadly fail silently... Same if the cron is stopped, but if you use borgserver_ynh you should be warned that the repo hasn't been refresh since 1 day (week or what ever you configure during the install).

@shukon
Copy link
Author

shukon commented Nov 27, 2024

Thank you!

however note that if /etc/yunohost.app/borg/settings.yml is empty (by error), this will sadly fail silently... Same if the cron is stopped, but if you use borgserver_ynh you should be warned that the repo hasn't been refresh

But then the whole backup command is not triggered, right? Or - if it's triggered, but there is no config, there will be no URL to ping either?
So even if you don't have the borgserver_ynh reminder, the healthchecks will work the same way, alerting after a defined time without ping.
Either way, thanks for approving!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants