borgmatic-collective/borgmatic
borgmatic-collective
/
borgmatic
15
56
Fork 39
Code Issues 52 Pull Requests 1 Actions Packages Releases 121 Activity

Order of operations between normalise and override #561

New Issue
Closed
opened 2022-07-05 21:05:55 +00:00 by g-a-c · 4 comments
g-a-c commented 2022-07-05 21:05:55 +00:00
Contributor
Copy Link

What I'm trying to do and why

Since commit 02781662f85bcdbab795bf67fdecf28f85cb50d7 the healthchecks configuration changed from a simple string to an object. This is backwards compatible within the configuration file because the normalize function will upgrade the configuration on-the-fly.

However this normalisation happens before overrides are applied (borgmatic/config/validate.py), so if you have an override to ping a different Healthcheck (which I currently do before I switch to ntfy) then this will fail "silently" without sending a failure ping because the Healthchecks config is invalid

Maybe it would be better to override before normalising so that these dynamic changes can apply in both cases instead of silently failing in one of them?

[borgmatic/config/validate.py](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/master/borgmatic/config/validate.py#L101-L102)

Steps to reproduce (if a bug)

hooks:
  healthchecks: https://hc-ping.com/UUID1

borgmatic check --config /etc/borgmatic/config.yaml -v 1 --override hooks.healthchecks=https://hc-ping.com/UUID2

Actual behavior (if a bug)

Jul 05 21:10:18 server borgmatic[8923]: CRITICAL An error occurred while parsing a configuration file at /etc/borgmatic/config.yaml:
    At 'hooks.healthchecks': 'https://hc-ping.com/UUID2' is not of type 'object'
Jul 05 21:10:18 server pipenv[8923]: /etc/borgmatic/config.yaml: No valid configuration files found
Jul 05 21:10:18 server borgmatic[8923]: CRITICAL /etc/borgmatic/config.yaml: No valid configuration files found       
Jul 05 21:10:18 server borgmatic[8923]: CRITICAL
Jul 05 21:10:18 server pipenv[8923]: Need some help? https://torsion.org/borgmatic/#issues
Jul 05 21:10:18 server borgmatic[8923]: CRITICAL Need some help? https://torsion.org/borgmatic/#issues
Jul 05 21:10:18 server systemd[1]: borgmatic.service: Main process exited, code=exited, status=1/FAILURE
Jul 05 21:10:18 server systemd[1]: borgmatic.service: Failed with result 'exit-code'.
Jul 05 21:10:18 server systemd[1]: Failed to start Run system-wide Borgmatic backups.

Expected behavior (if a bug)

That the command line --override is also normalised in the same way, which makes it compatible with hooks.healthchecks being an object, not a string

Other notes / implementation ideas

Environment

borgmatic version: 1.6.3

Use sudo borgmatic --version or sudo pip show borgmatic | grep ^Version

borgmatic installation method: pipenv

Borg version: 1.2.1

Use sudo borg --version

Python version: 3.9.2

Use python3 --version

Database version (if applicable): n/a

Use psql --version or mysql --version on client and server.

operating system and version: Debian 11

#### What I'm trying to do and why Since commit `02781662f85bcdbab795bf67fdecf28f85cb50d7` the `healthchecks` configuration changed from a simple string to an object. This is backwards compatible within the configuration file because the `normalize` function will upgrade the configuration on-the-fly. However this normalisation happens before overrides are applied ([`borgmatic/config/validate.py`](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/master/borgmatic/config/validate.py#L101-L102)), so if you have an override to ping a different Healthcheck (which I currently do before I switch to `ntfy`) then this will fail "silently" without sending a failure ping because the Healthchecks config is invalid Maybe it would be better to override _before_ normalising so that these dynamic changes can apply in both cases instead of silently failing in one of them? `[borgmatic/config/validate.py](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/master/borgmatic/config/validate.py#L101-L102)` #### Steps to reproduce (if a bug) ```yaml hooks: healthchecks: https://hc-ping.com/UUID1 ``` ```sh borgmatic check --config /etc/borgmatic/config.yaml -v 1 --override hooks.healthchecks=https://hc-ping.com/UUID2 ``` #### Actual behavior (if a bug) ``` Jul 05 21:10:18 server borgmatic[8923]: CRITICAL An error occurred while parsing a configuration file at /etc/borgmatic/config.yaml: At 'hooks.healthchecks': 'https://hc-ping.com/UUID2' is not of type 'object' Jul 05 21:10:18 server pipenv[8923]: /etc/borgmatic/config.yaml: No valid configuration files found Jul 05 21:10:18 server borgmatic[8923]: CRITICAL /etc/borgmatic/config.yaml: No valid configuration files found Jul 05 21:10:18 server borgmatic[8923]: CRITICAL Jul 05 21:10:18 server pipenv[8923]: Need some help? https://torsion.org/borgmatic/#issues Jul 05 21:10:18 server borgmatic[8923]: CRITICAL Need some help? https://torsion.org/borgmatic/#issues Jul 05 21:10:18 server systemd[1]: borgmatic.service: Main process exited, code=exited, status=1/FAILURE Jul 05 21:10:18 server systemd[1]: borgmatic.service: Failed with result 'exit-code'. Jul 05 21:10:18 server systemd[1]: Failed to start Run system-wide Borgmatic backups. ``` #### Expected behavior (if a bug) That the command line `--override` is also normalised in the same way, which makes it compatible with `hooks.healthchecks` being an object, not a string #### Other notes / implementation ideas #### Environment **borgmatic version:** 1.6.3 Use `sudo borgmatic --version` or `sudo pip show borgmatic | grep ^Version` **borgmatic installation method:** `pipenv` **Borg version:** 1.2.1 Use `sudo borg --version` **Python version:** 3.9.2 Use `python3 --version` **Database version (if applicable):** n/a Use `psql --version` or `mysql --version` on client and server. **operating system and version:** Debian 11
witten commented 2022-07-05 23:06:26 +00:00
Owner
Copy Link

Good catch! I apologize for the breakage. I think the solution you suggest (apply overrides before normalization) makes sense. I'd just have to make sure that if you pass in old-style overrides using a new-style configuration file, the override would still replace the existing configuration.

Good catch! I apologize for the breakage. I think the solution you suggest (apply overrides before normalization) makes sense. I'd just have to make sure that if you pass in old-style overrides using a new-style configuration file, the override would still replace the existing configuration.
witten added the
bug
 label 2022-07-05 23:06:34 +00:00
g-a-c commented 2022-07-06 09:06:51 +00:00
Author
Contributor
Copy Link

No need to apologise, it's a breakage of my own making since I probably didn't bother reading the release notes last time I upgraded :)

It's just an unfortunate one because the config being invalid means that borgmatic never runs, so it won't even send a failure ping. Also that second UUID I set up with Healthchecks doesn't nag every day if the maintenance (check, prune) phases don't run so I hadn't noticed they weren't running for a short while.

No need to apologise, it's a breakage of my own making since I probably didn't bother reading the release notes last time I upgraded :) It's just an unfortunate one because the config being invalid means that `borgmatic` never runs, so it won't even send a failure ping. Also that second UUID I set up with Healthchecks doesn't nag every day if the maintenance (check, prune) phases don't run so I hadn't noticed they weren't running for a short while.
witten commented 2022-07-07 01:21:29 +00:00
Owner
Copy Link

This is fixed in master and will be part of the next release. Thanks again for reporting it!

This is fixed in master and will be part of the next release. Thanks again for reporting it!
witten commented 2022-07-20 03:35:52 +00:00
Owner
Copy Link

This has been released in borgmatic 1.6.6!

This has been released in borgmatic 1.6.6!
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
main
spot-check
1.8.10
1.8.9
1.8.8
1.8.7
1.8.6
1.8.5
1.8.4
1.8.3
1.8.2
1.8.1
1.8.0
1.7.15
1.7.14
1.7.13
1.7.12
1.7.11
1.7.10
1.7.9
1.7.8
1.7.7
1.7.6
1.7.5
1.7.4
1.7.3
1.7.2
1.7.1
1.7.0
1.6.6
1.6.5
1.6.4
1.6.3
1.6.2
1.6.1
1.6.0
1.5.24
1.5.23
1.5.22
1.5.21
1.5.20
1.5.19
1.5.18
1.5.17
1.5.16
1.5.15
1.5.14
1.5.13
1.5.12
1.5.11
1.5.10
1.5.9
1.5.8
1.5.7
1.5.7.dev0
1.5.6
1.5.5
1.5.4
1.5.3
1.5.2
1.5.1
1.5.0
1.4.22
1.4.21
1.4.20
1.4.19
1.4.18
1.4.17
1.4.16
1.4.15
1.4.14
1.4.13
1.4.12
1.4.11
1.4.10
1.4.9
1.4.8
1.4.7
1.4.6
1.4.5
1.4.4
1.4.3
1.4.2
1.4.1
1.4.0
1.3.26
1.3.25
1.3.24
1.3.23
1.3.22
1.3.21
1.3.20
1.3.19
1.3.18
1.3.17
1.3.16
1.3.15
1.3.14
1.3.13
1.3.12
1.3.11
1.3.10
1.3.9
1.3.8
1.3.7
1.3.6
1.3.5
1.3.4
1.3.3
1.3.2
1.3.1
1.3.0
1.2.18
1.2.17
1.2.16
1.2.15
1.2.14
1.2.13
1.2.12
1.2.11
1.2.10
1.2.9
1.2.8
1.2.7
1.2.6
1.2.5
1.2.4
1.2.3
1.2.2
1.2.1
1.2.0
1.1.15
1.1.14
1.1.13
1.1.12
1.1.11
1.1.10
1.1.9
1.1.7
1.1.6
1.1.5
1.1.4
1.1.3
1.1.2
1.1.1
1.1.0
1.0.3
1.0.2
1.0.1
1.0.0
0.1.8
0.1.7
0.1.6
0.1.5
0.1.4
0.1.3
0.1.2
0.1.1
0.1.0
0.0.7
0.0.6
0.0.5
0.0.4
0.0.3
0.0.2
Labels
Clear labels   
bug

   
data loss

   
design finalized

   
good first issue

   
new feature area

   
question / support

   
security

   
waiting for response
No Label
bug
data loss
design finalized
good first issue
new feature area
question / support
security
waiting for response
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: borgmatic-collective/borgmatic#561
No description provided.
Delete Branch "%!s(<nil>)"

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?