flyingcircusio / backy

3 stars 1 forks source link

Support for multiple Ceph RBD version's output format #28

Closed osnyx closed 1 year ago

osnyx commented 1 year ago

Follow-up to #23.

This makes backy compatible to the Ceph Nautilus client tooling: rbd showmapped has now a different json output. It now returns a list of objects (dicts) decribing mappings, instead of an object (dictionary) already at the top level. Keeps compatibility to previous Ceph versions by checking the Ceph version at runtime and differing in behaviour upon that.

Unit tests check behaviour based on outputs from Ceph Jewel, Luminous, and Nautilus.

Manually tested with #26 and #27 already merged. Formatting issues have been (partially) neglected, as changes are going to be enforced by black through pre-commit rules anyways after merging #22.

Release process

Impact: None, just wider compatibility.

Changelog:

Security implications

osnyx commented 1 year ago

This currently fails due to an issue in consulate:

root@patty .../backy/test72 # backy backup daily
New revision 4QnDN32iM2adKKbQPLSk4d [daily]
Loaded 4153 known chunks.
Consul: requesting consistent snapshot of test72@backy-4QnDN32iM2adKKbQPLSk4d via snapshot/8398456e-9727-452e-9842-5f26ab2cac80
Error: string indices must be integers
string indices must be integers
Traceback (most recent call last):
  File "/nix/store/0k80kqmnk3qi869g1nchpzq1a7nwqm8x-python3.10-backy-2.5.0dev/lib/python3.10/site-packages/backy/main.py", line 349, in main
    func(**func_args)
  File "/nix/store/0k80kqmnk3qi869g1nchpzq1a7nwqm8x-python3.10-backy-2.5.0dev/lib/python3.10/site-packages/backy/main.py", line 90, in backup
    b.backup(tags)
  File "/nix/store/0k80kqmnk3qi869g1nchpzq1a7nwqm8x-python3.10-backy-2.5.0dev/lib/python3.10/site-packages/backy/backup.py", line 59, in locked_function
    return f(self, *args, **kw)
  File "/nix/store/0k80kqmnk3qi869g1nchpzq1a7nwqm8x-python3.10-backy-2.5.0dev/lib/python3.10/site-packages/backy/backup.py", line 59, in locked_function
    return f(self, *args, **kw)
  File "/nix/store/0k80kqmnk3qi869g1nchpzq1a7nwqm8x-python3.10-backy-2.5.0dev/lib/python3.10/site-packages/backy/backup.py", line 194, in backup
    with self.source(new_revision) as source:
  File "/nix/store/0k80kqmnk3qi869g1nchpzq1a7nwqm8x-python3.10-backy-2.5.0dev/lib/python3.10/site-packages/backy/sources/ceph/source.py", line 53, in __enter__
    self.create_snapshot(snapname)
  File "/nix/store/0k80kqmnk3qi869g1nchpzq1a7nwqm8x-python3.10-backy-2.5.0dev/lib/python3.10/site-packages/backy/sources/flyingcircus/source.py", line 66, in create_snapshot
    for key in list(consul.kv.find('snapshot/')):
  File "/nix/store/cxahpk7wksr39haxb4xxszw8xcxgyw1q-python3.10-consulate-0.6.0/lib/python3.10/site-packages/consulate/api/kv.py", line 165, in find
    results[row['Key']] = row['Value']
TypeError: string indices must be integers
Backy operation failed.

As this PR got #26 merged in for being able to test it on fc-nixos-21.05-dev, it suddenly runs on python-3.10. I recall that consulate had already issued the warning DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.10 it will stop working in earlier releases, this might be related.

osnyx commented 1 year ago

All tests are fixed now, ready for review.

One possible blocker: I see 1 backup failure in dev, verly likely due to corrupt parent revision data. We need to look into this before merging. PL-131339

osnyx commented 1 year ago

@dhnasa and I think that the mentioned backup failure is not a new regression, but an extended case of #17 and the old revisions not having been distrusted despite failing their verifications in older backy versions.