[Feature] Add Uptime Kuma hook #885
witten
Loadingβ¦
x
Reference in New Issue
Block a user
No description provided.
Delete Branch "pswilde/borgmatic:main"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Hi,
I've created a basic hook for working with Uptime Kuma.
Uptime Kuma is a self-hosted uptime monitoring service that, traditionally, is used for performing minutely checks on services via pings, curls, tcp sockets, and so on for servers. However, it can also be used in a similar way to healthchecks.io, whereby it can await a get request from a host instead of making a request to it.
It just expects a simple get request to an API endpoint with a few query parameters - nothing too in depth, no authentication, etc.
I've been using it for backup monitoring in this way with some scripts and it works very well.
I've recently started using borg and borgmatic and though the ability to have hooks in borgmatic is fantastic, and as similar hooks are already available I thought it a good idea to get Uptime Kuma working as well.
Apologies for not writing any tests, as far as I'm aware there are no public Uptime Kuma hosts so I need to work on this. If you would prefer tests to be in place then please stand by and I will create them, otherwise I have tested this working with my Uptime Kuma server and it works correctly.
Thanks for all your hard work creating borgmatic!
Add Uptime Kuma hookto [Feature] Add Uptime Kuma hookWow, thank you so much for implementing this hook! I will say that Uptime Kuma integration for borgmatic was vaguely on my radar, so this is a great addition IMO.
I haven't had a chance to look at the code yet, but in terms of tests, I would like to see coverage. But the good news is that it doesn't need to (shouldn't) actually hit any Uptime Kuma server. The tests just need to assert that the basic logic is solid. For instance, take a look at the existing
tests/unit/hooks/test_healthchecks.pyor the tests for any of the other monitoring hooks. They mock out any network calls (and dependencies on units other than the one under test) and only test the code on a per-function basis.If you run into any difficulties when adding tests, I'd be happy to answer questions.. While the mocking library in use is pretty powerful, it's not always intuitive.
Thank you!
Thanks!
I'll copy that
test_healthchecks.pytests and try to get them working for Uptime Kuma.I probably won't have loads of time for the next few days to do it, but I'll do my best then get back in touch.
Thanks,
[Feature] Add Uptime Kuma hookto WIP: [Feature] Add Uptime Kuma hookActually, scrap that, turns out that wasn't so hard to implement at all! (I hope!)
So some basic start, finish, fail tests are in place now, I've run tox which is also coming back clear after some further adjustments.
Thanks,
WIP: [Feature] Add Uptime Kuma hookto [Feature] Add Uptime Kuma hookThis looks great, including the tests! Most of my comments are on super minor stuff. One thing I see missing though is an entry for Uptime Kuma in the docs at https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/ ... I'd be happy to add this myself, but I thought I'd mention it in case you wanted to take a stab at it as part of this PR. Either way is fine with me!
@ -62,6 +62,7 @@ borgmatic is powered by [Borg Backup](https://www.borgbackup.org/).<a href="https://www.mongodb.com/"><img src="docs/static/mongodb.png" alt="MongoDB" height="60px" style="margin-bottom:20px; margin-right:20px;"></a><a href="https://sqlite.org/"><img src="docs/static/sqlite.png" alt="SQLite" height="60px" style="margin-bottom:20px; margin-right:20px;"></a><a href="https://healthchecks.io/"><img src="docs/static/healthchecks.png" alt="Healthchecks" height="60px" style="margin-bottom:20px; margin-right:20px;"></a><a href="https://uptime.kuma.pet/"><img src="docs/static/uptimekuma.png" alt="Uptime Kuma" height="60px" style="margin-bottom:20px; margin-right:20px;"></a>Thank you for even adding the logo to the docs!
@ -1679,0 +1681,4 @@required: ['server', 'push_code']additionalProperties: falseproperties:server:What do you think of calling this
ping_urlorserver_urlfor consistency with the other monitoring hooks? (I realize it can be a UUID as well, similar to the Healthchecks hook.)@ -1679,0 +1685,4 @@type: stringdescription: |Uptime Kuma base URL or UUID to notify when a backupbegins, ends, or errorsMinor nit: Convention is periods a the end of sentences / fragments in schema descriptions. (Here and elsewhere.)
This says this can be a UUID, but I don't see any code below to handle that case. So is this accurate or is this maybe just a copy-paste error?
@ -1679,0 +1691,4 @@type: stringdescription: |Uptime Kuma "Push Code" from the push URL you have beengiven. For example, the push code forI'm a little unclear on "the push URL you have been given." Is this referring to the
servervalue above.. or is it a separate URL? (I've never used Uptime Kuma.) Would it be clearer to just say "the server URL" or similar?@ -30,6 +31,7 @@ HOOK_NAME_TO_MODULE = {'postgresql_databases': postgresql,'sqlite_databases': sqlite,'loki': loki,'uptimekuma': uptimekuma,For this dict and also for
MONITOR_HOOK_NAMES, it might be nice to alpha order the contents. Do not feel strongly.@ -0,0 +22,4 @@run_states = hook_config.get('states', ['start', 'finish', 'fail'])if state.name.lower() in run_states:You can lose a level of indention on most of this function if you "early out" here. Example:
Do not feel strongly.
@ -0,0 +23,4 @@run_states = hook_config.get('states', ['start', 'finish', 'fail'])if state.name.lower() in run_states:Nit: Remove newline.
@ -0,0 +28,4 @@status = 'up'if state.name.lower() == 'fail':status = 'down'This could be shortened to:
Do not feel strongly.
@ -0,0 +34,4 @@push_code = hook_config.get('push_code')logger.info(f'{config_filename}: Pinging Uptime Kuma push_code {push_code}{dry_run_label}')logger.debug(f'{config_filename}: Using Uptime Kuma ping URL {base_url}/{push_code}')Seems like this log entry is unnecessary given that the next one is the same except with the addition of query parameters. (And they're both at the same log level.)
@ -0,0 +36,4 @@logger.info(f'{config_filename}: Pinging Uptime Kuma push_code {push_code}{dry_run_label}')logger.debug(f'{config_filename}: Using Uptime Kuma ping URL {base_url}/{push_code}')logger.debug(f'{config_filename}: Full Uptime Kuma state URL {base_url}/{push_code}?status={status}&msg={state.name.lower()}&ping='You could save the full URL into a variable for use both here and below in the
requests.get()call. That way, you don't run the risk of a change being made in one place and not the other.@ -0,0 +39,4 @@f'{config_filename}: Full Uptime Kuma state URL {base_url}/{push_code}?status={status}&msg={state.name.lower()}&ping=')if not dry_run:You could also early out here.
@ -0,0 +5,4 @@default_base_url = 'https://example.uptime.kuma'custom_base_url = 'https://uptime.example.com'push_code = 'abcd1234'Code convention: All of these variables should be all-capitals since they're actually global constants. π
@ -0,0 +146,4 @@)def test_ping_monitor_does_not_hit_custom_uptimekuma_on_start_dry_run():Personally, I don't think you have to test every conceivable combination of inputs (custom / not custom, each state, dry run / not dry run) as long as you test the major combinations to exercise the code as it's written or where things could reasonably go wrong in the future. I do not feel strongly though, so if you want this level of extensive coverage, that's fine with me!
@ -0,0 +223,4 @@borgmatic.hooks.monitor.State.FAIL,monitoring_log_level=1,dry_run=False,)The one test I see missing though is a test that a given state that's not in
run_statesresults in the function not pinging anything.Hi,
Thanks for all your pointers there, really helpful.
I've refactored a bit a changed a couple of things.
The main change I've made is to drop entirely the "server" and "push_code" variables and replace them with a "push_url" variable. This is because Uptime Kuma basically just gives you a URL named "Push" to make calls to with a query string, as below:
Having slept on it, I felt it's easier for someone to understand just to paste in that URL (without the query string) instead of splitting it up. This resolves a few of your points i.e. the UUID (copy-paste error), server, push_code, etc. confusion, and I think makes it much easier to set up in the first place.
I've also removed the "&ping=" query parameter from the code as it is not required and is actually there for a reverse uptime speed test solution (i.e. we can ping out at 10ms) so not really necessary for backups.
I've removed many of the tests calling to the "default" URL, as ultimately these would never be used anyway. We should only be interested in the custom URL. I've added a test for an invalid run state as you have suggested.
Otherwise I've just changed things as you've advised - many of them are because I haven't written any python in a few years so I'm a bit rusty! π
I'm happy to take a look at the docs as well, and will get to those shortly.
Thanks,
Just another quick note, I've changed the
ping_monitorname topush_monitoras it then matches the variable names a bit better, do you think this is OK?Basically all that's happening is a GET request to the Uptime Kuma URL, so "ping" doesn't really suit it (I copied a lot from the ntfy and healthchecks hooks). I think "push" is better as that's what Uptime Kuma calls it, but let me know if you feel otherwise.
Thanks,
Thanks for making all of these changes! I'll have a look when I get a chance.
Makes sense to me. Good call!
It's totally fine everywhere except at the top level functions of the hook module (
def ping_monitor(): ..., etc.). That's because those functions are called by name from the borgmatic monitoring dispatch code, and if those names don't exist as expected, the hook can't be called. However inside those functions and in the configuration schema you can call things whatever makes the most sense for Uptime Kuma.FWIW, many of the existing monitoring hooks just do a GET as well.
Thanks for the note on this, I've changed the top level function back to ping_monitor π
Thanks for making these changes.. This all looks good! There are a few minor formatting/whitespace things, but rather than doing another PR go-round, I'll just make those changes after merging.
FYI I'm also renaming the configuration option from
uptimekumatouptime_kuma!Also, I really appreciate you taking the time to write up docs too.
All good, thanks again for making a great piece of software!
Sorry, just a quick note to say the renaming of the config option has stopped calls being made - looks like it needs to be updated in both monitor.py and dispatch.py as well.
Sent a new PR for this, not that it's a big one to fix π€£
Released in borgmatic 1.8.13. Thanks again!