Univention Bugzilla – Full Text Bug Listing |
Summary: | Limit number of backups in /var/univention-backup | ||
---|---|---|---|
Product: | UCS | Reporter: | Tim Petersen <petersen> |
Component: | General | Assignee: | Lukas Oyen <oyen> |
Status: | CLOSED FIXED | QA Contact: | Arvid Requate <requate> |
Severity: | enhancement | ||
Priority: | P5 | CC: | andree.hingst, best, grandjean, hinrichs, requate, stephan.hendl, stoeckigt |
Version: | UCS 4.0 | Flags: | oyen:
Patch_Available+
|
Target Milestone: | UCS 4.2-2-errata | ||
Hardware: | Other | ||
OS: | Linux | ||
What kind of report is it?: | --- | What type of bug is this?: | --- |
Who will be affected by this bug?: | --- | How will those affected feel about the bug?: | --- |
User Pain: | Enterprise Customer affected?: | Yes | |
School Customer affected?: | ISV affected?: | ||
Waiting Support: | Flags outvoted (downgraded) after PO Review: | ||
Ticket number: | 2015110221000417, 2016041321000837, 2016072721000191 | Bug group (optional): | External feedback |
Max CVSS v3 score: | |||
Bug Depends on: | |||
Bug Blocks: | 45408 | ||
Attachments: |
Suggested patch for /usr/sbin/univention-ldap-backup
/usr/share/ucs-test/10_ldap/101_test_univention_ldap_backup_limit 38554-backup-cleanup-420.patch quoting.patch |
Description
Tim Petersen
2015-05-18 15:50:21 CEST
Are there any updates? Maybe upcoming UCS-4.1 is a good place in case it is not possible within 4.0-x? Requested again at 2015110221000417 Requested during workshop. 2016041321000837 dearly requested at Ticket#2016072721000191 Created attachment 7995 [details]
Suggested patch for /usr/sbin/univention-ldap-backup
This is a suggested patch for /usr/sbin/univention-ldap-backup
A maximum number of backups can be set like this:
ucr set ucr/univention-ldap-backup/limit='n'
After a new backup has been created, the oldest backups will be deleted until only the newest n remain.
Created attachment 8291 [details]
/usr/share/ucs-test/10_ldap/101_test_univention_ldap_backup_limit
This is a test case for the attachment above.
Are there any new investigations on this? Created attachment 8891 [details]
38554-backup-cleanup-420.patch
The attached patches implement `clean_old_backups` in univention-lib. This
functions expects a pattern and an optional `max_age` and deletes files in
/var/univention-backup older than max_age.
Two new UCR variables control this function: backup/clean/max_age and
backup/clean/min_backups. If the former is unset no cleanup is performed. The
latter acts as a safeguard of a minimal amount of backups to keep, even if older
than `max_age` (10 is the default if unset).
This changes the behaviour of univention-samba4-backup, as per default no
cleanup of samba4-backups is performed (u-s4-backup deleted backups after 356
days). univention-samba4-backup overrides backup/clean/max_age locally if called
with `--days`. This is possible via the UCR variable samba4/backup/cron/options.
This is a more general solution, in contrast to Julius' cleanup of LDAP-backups.
We should bump the dependencies on `shell-univention-lib` on the following
packages if this is commited: univention-base-files, univention-ldap,
univention-samba4
There is a Customer ID set so I set the flag "Enterprise Customer affected". Ok, please keep the syntax /bin/sh compatible if possible, so the library can be used by dash scripts too. Also doesn't it make sense to keep the base path out of the function to make it more usable? like clean_old_backups '/var/univention-backup/samba/\(samba4_\|sysvol\).*.bz2' \ "$DAYS" (In reply to Arvid Requate from comment #11) > Also doesn't it make sense to keep the base > path out of the function to make it more usable? Yes, but such a general solution would also allow the user to pass `(/var/univention-backup/)|(/home/peter/backup)` as the pattern. This makes it difficult to extract a base-dir to pass to find. So fixing the directory to /var/univention-backup keeps the implementation simple and spares us from running `find / -regex ...` which would be a performance problem. A possible solution would be an optional third parameter which defaults to /var/univention-backup. Ok, I understand, then just commit it. AFAIK dash doesn't support assigning a value to a variable during the "local" declaration. implementation in univention-lib: r81720-r81722, advisory r81723 changes in univention-base-files: r81726, advisory r81727 changes in univention-samba4: r81728, advisory r81729 changes in univention-ldap: r81730, advisory r81731 I'd appreciate an (erratum) package :-) We need this feature to avoid filesystem messages from Nagios/Icinga! > The latter acts as a safeguard of a minimal amount of backups to keep, even if older than `max_age` (10 is the default if unset).
From reading the code I think it would remove all files aged over max_age if at least backup/clean/min/backups files are found. So assuming I have 9 files with age > max_age and then I add one new backup, I think the code would just keep the one new file. Please also declare the shell variables in the function as local.
(In reply to Arvid Requate from comment #16) > From reading the code I think it would remove all files aged over max_age if > at least backup/clean/min/backups files are found. So assuming I have 9 > files with age > max_age and then I add one new backup, I think the code > would just keep the one new file. No, if you have 9 old files and one new files, `count` would still be 9 and no files will be removed. Or I don't understand you correctly. But the second `find` incorrectly uses `backup_clean_max_age` instead of `max_age`. > Please also declare the shell variables in the function as local. This contradicts comment 11, after which I removed the `local` for variables. Added `local` back in and fixed the `max_age` typo. 4.2-1: r82597/r82598, YAML: r82601 4.2-2: r82599/r82600, YAML: r82607 Ok, works. Created attachment 9178 [details]
quoting.patch
Just a minor quoting patch, simplifies debugging.
Applied in 04a60cdb, YAML 9520d7. Ok, works. |