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

Handle enabled=off with recursion in place #625

Merged
merged 17 commits into from
Mar 11, 2024

Conversation

jimklimov
Copy link
Contributor

@jimklimov jimklimov commented Jan 15, 2024

(Includes and) Follows up from #623 with a practical feature: if the same intermediate dataset that declares enabled=off also says recursive=on, the disablement takes place for descendants as well.

As discussed among ideas in #623, this PR adds recognition for datasets (descendants of a "backupSet") which declare not only an org.znapzend:enabled value but optionally also an org.znapzend:recursive one (so only these one or two ZFS properties are defined in a local source dataset under the one which defines a full retention schedule).

It also somewhat reasonably defines behavior for such dataset trees where a backupSet with a full retention schedule has some descendant which defines an enabled=off behavior (individual or recursive), and some of its descendants re-enable the backup (individually or recursively). I'm sure this can be tested with mocks, but out of time at least today, maybe longer, to pursue testing. Worked as expected in a real-life setup (which however did not try all edge cases) :)

This is also seen in znapzendzetup list outputs:

...
*** backup plan: rpool/home ***
           dst_0 = znapzend:pond/export/DUMP/NUTCI/znapzend/ci-deb/rpool/home
      dst_0_plan = 1week=>1day,1month=>1week,1year=>1month,10years=>6months
         enabled = on
         mbuffer = off
    mbuffer_size = 128M
   post_znap_cmd = off
    pre_znap_cmd = off
       recursive = on
             src = rpool/home
        src_plan = 1week=>1day,1month=>1week,1year=>1month,10years=>6months
        tsformat = znapzend-auto-%Y-%m-%dT%H:%M:%SZ
      zend_delay = 0

*** backup plan: rpool/home/abuild/.ccache ***
         enabled = off
             src = rpool/home/abuild/.ccache

*** backup plan: rpool/home/abuild/jenkins-nut-altroots ***
         enabled = off
       recursive = on
             src = rpool/home/abuild/jenkins-nut-altroots

...

…of org.znapzend:enabled" for sub-datasets

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
… cleanup of snapshots on enabled=off sub-datasets

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
…teSnapshot() so it can also be used in sendRecvCleanup()

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
… done in refreshBackupPlans() once

Track the list of names as @{$backupSet->{srcDisabledDescendants}}

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
…ledSourceDescendants(): recognize sub-datasets that are both enabled(=off|on) and recursive(=on)

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
Copy link

@check-spelling-bot Report jimklimov/znapzend: enabled-off-with-recursion into -> oetiker/znapzend: 100c9f6

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (5)

hashmaps
idx
LRds
rindex
substr

Previously acknowledged words that are now absent aix Balert Bcreate Bdebug Bdelete Bedit Berr Bexport Bimport Binfo Bnoaction Bnot Bpidfile Bpost Bpre Bsyslog Bwarning Bzfs Bznapzend Bznapzendzetup Bznapzendztatz CBuilder cpanfile cpanm cpanmin CPANSNAPV crt Cwd cygwin DBD DESTDIR distdir DTDs endif EXTRADIST Fcntl forkcall Icommand Icommon Icreate Idataset Idestroy Idocuments Iexport Ifacility Ifeature Ifilepath Ihome Ilimited imandir Inumber Ioptions Ipath Ipictures Irecursive Isend Iskip Isnapshots Isnapsuffix Isources Itank Ithirdparty Itimeout Iusbbackup Iuser Ivalue Iznapzendzetup JBERGER LEONT lpr Mkbootstrap nobase notest nroff ODBC Pipely RCAPUTO SUBDIRS svcdir troff unicode utf VOS vroff xargs 🫥
To accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands

... in a clone of the https://github.com/oetiker/znapzend repository
on the master branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.22/apply.pl' |
perl - 'https://github.com/oetiker/znapzend/actions/runs/7533184333/attempts/1'
Available 📚 dictionaries could cover words (expected and unrecognized) not in the 📘 dictionary

This includes both expected items (649) from .github/workflows//spelling/expect.txt and unrecognized words (5)

Dictionary Entries Covers Uniquely
cspell:software-terms/dict/softwareTerms.txt 1288 82 18
cspell:php/dict/php.txt 1689 59 6
cspell:python/src/python/python-lib.txt 2417 54 6
cspell:node/dict/node.txt 891 56 5
cspell:filetypes/filetypes.txt 264 19 4

Consider adding them (in .github/workflows/spelling.yml):

      with:
        extra_dictionaries:
          cspell:software-terms/dict/softwareTerms.txt
          cspell:php/dict/php.txt
          cspell:python/src/python/python-lib.txt
          cspell:node/dict/node.txt
          cspell:filetypes/filetypes.txt

To stop checking additional dictionaries, add (in .github/workflows/spelling.yml):

check_extra_dictionaries: ''

Copy link
Owner

@oetiker oetiker left a comment

Choose a reason for hiding this comment

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

removing the extra snapshots quickly seems like a good move!

lib/ZnapZend.pm Outdated Show resolved Hide resolved
lib/ZnapZend.pm Outdated Show resolved Hide resolved
lib/ZnapZend.pm Outdated Show resolved Hide resolved
lib/ZnapZend.pm Outdated Show resolved Hide resolved
CHANGES Outdated Show resolved Hide resolved
@jimklimov
Copy link
Contributor Author

jimklimov commented Jan 16, 2024

Thanks for the review, and the master-branch bump (docker fix), nice to see it all claim to be green again :)

removing the extra snapshots quickly seems like a good move!

I don't think this PR tackles it directly - this is part of createSnapshot() and was for a while (since #154 at least?); this change here (and started by "externalization" of code in #623) modifies how users can state more easily which datasets are not-enabled for znapzend handling so their snapshots would be removed earlier. Also #626 takes care of skipping zfs send for whatever datasets are in the not-enabled lists (also builds upon #623, but is independent of the change in this PR -- so if that one is merged alone, it would be about skipping exactly the datasets with enabled=off and no care for recursions).

@jimklimov
Copy link
Contributor Author

jimklimov commented Jan 16, 2024

Hm, I don't actually see any docs about the enabled=off handling :\

UPDATE: Actually, README.md starts out to say how this is not supported :) I'll claim it as "experimental" instead and document as such.

@jimklimov jimklimov mentioned this pull request Jan 16, 2024
Signed-off-by: Jim Klimov <jimklimov@gmail.com>
…figurations

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
…r a recursive backup schedule

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
…tree pruning [oetiker#625]

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
… type for raw and parsed cmd output [oetiker#625 review]

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
…d" [oetiker#625 review]

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
@jimklimov
Copy link
Contributor Author

Ok, so after the merge of groundwork from #623 the specific changes of this PR are much better visible :)

@oetiker oetiker merged commit 95c2bd8 into oetiker:master Mar 11, 2024
4 checks passed
@jimklimov jimklimov deleted the enabled-off-with-recursion branch March 12, 2024 08:49
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.

2 participants