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

Btrfs snapshotter without snapper #2220

Merged

Conversation

davidcassany
Copy link
Contributor

@davidcassany davidcassany commented Oct 31, 2024

This pull requests provides a new subvolumebackend interface to handle btrfs subvolumes and snapshots. The motivation is to not make snapper strict requirement of the btrfs snapshotter to better suite distributions where snapper is not that much adopted.

In any case (even if snapper is not in use) the snapshotter is based and built around common snapper conventions, this is easier from a maintenance perspective and allows to eventually consider switching to snapper at a later point in time.

This PR adds a new config parameter snapper for the btrfs snapshotter:

snapshotter:
  type: btrfs
  max-snaps: 4
  config:
    snapper: true

If snapper set to true (this is the default value) the snapshotter will attempt to use snapper, otherwise it will relay entirely on btrfs utility calls.

Snapper can't perform alone all the uses cases that the Elemental Snapshotter interface requires (specially corner cases related to first snapshot), for those cases the snapperBackend implementation makes use of the btrfsBacked implementation, hopefully this can be improved in up comming snapper releases (openSUSE/snapper#944).

This PR includes a couple minor behavior changes (on the snapperBackend) that are actually reflected in unit tests:

  • The snapper configuration is applied directly over the snapshot (before it was applied over the working directory)
  • Snapper is used to define the read only and default snapshot for all snapshots except for the first one

@davidcassany davidcassany requested a review from a team as a code owner October 31, 2024 08:07
@davidcassany davidcassany marked this pull request as draft October 31, 2024 08:07
@davidcassany
Copy link
Contributor Author

I consider this PR ready to be reviewed despite being in draft. Probably minor changes could be applied on top of the current logic to make public subvolumeBackend interface and its implementations in order to include specific unit tests.

Copy link
Contributor

@anmazzotti anmazzotti left a comment

Choose a reason for hiding this comment

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

This looks good to me. I found it confusing that the snapper backend is using the (plain) btrfs one, but I agree that this should be addressed on the snapper side instead with the other issue you already linked.
Nice improvement!

@davidcassany
Copy link
Contributor Author

I found it confusing that the snapper backend is using the (plain) btrfs one

Yes I agree, so far this is the simplest idea that came to my mind to not duplicate code. Opened to other ideas too 😉

@davidcassany davidcassany self-assigned this Nov 5, 2024
@davidcassany davidcassany marked this pull request as ready for review November 5, 2024 11:43
This commit adds a backend interface in btrfs snapshotter. The interface
essentially wraps snapper and btrfs utilities.

The idea is make a pure btrfs implementation of the interface and
also a snapper based implementation.

The functions that snapper can't provide are simply managed by the btrfs
implementation.

Signed-off-by: David Cassany <[email protected]>
This commit implements the btrfs backend for the btrfs snapshotter
and moves all the logic of specific btrfs client calls there.

The backend interface is also refined for that purpose.

Signed-off-by: David Cassany <[email protected]>
Signed-off-by: David Cassany <[email protected]>
Signed-off-by: David Cassany <[email protected]>
Signed-off-by: David Cassany <[email protected]>
@davidcassany davidcassany force-pushed the btrfs_snapshotter_without_snapper branch from bc86410 to a31e34a Compare November 5, 2024 11:43
@davidcassany davidcassany enabled auto-merge (squash) November 5, 2024 12:52
@davidcassany davidcassany merged commit ca0ac59 into rancher:main Nov 5, 2024
28 checks passed
@kkaempf kkaempf added kind/enhancement New feature or request area/btrfs labels Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/btrfs kind/enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants