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

nixos/btrbk: add snapshotOnly option #369480

Merged
merged 1 commit into from
Jan 5, 2025
Merged

nixos/btrbk: add snapshotOnly option #369480

merged 1 commit into from
Jan 5, 2025

Conversation

cything
Copy link
Contributor

@cything cything commented Dec 30, 2024

Things done

Added snapshotOnly option that is useful to people who use btrbk to send incremental snapshot backups to an external drive. Upstream has this use case documented here.

Using snapshot subcommand instead of run makes btrbk skip backup creation and deletion steps which can later be run manually with btrbk resume. With run, the service would fail if the external drive is not mounted.

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 30, 2024
@cything cything requested a review from oxalica January 1, 2025 04:37
Copy link
Member

@rhendric rhendric left a comment

Choose a reason for hiding this comment

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

Would you please add btrbk(1) to doc/manpage-urls.json? Otherwise, looks good to me!

@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label Jan 3, 2025
@cything
Copy link
Contributor Author

cything commented Jan 3, 2025

Would you please add btrbk(1) to doc/manpage-urls.json? Otherwise, looks good to me!

Done!
Upstream links to this URL here

@github-actions github-actions bot added 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Jan 3, 2025
@rhendric
Copy link
Member

rhendric commented Jan 4, 2025

Going to wait for @oxalica to look at this, but if they don't get back to you in a week or so you can ping me for a merge.

Copy link
Contributor

@oxalica oxalica left a comment

Choose a reason for hiding this comment

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

Going to wait for @oxalica to look at this

I don't have a strong opinion on this. I have a dedicated .conf for snapshot-only purpose (with a different cleanup policy) so I don't "need" this change personally. But I think it's worth to have option changing the btrbk verb.

@@ -185,6 +185,17 @@ in
Setting it to null disables the timer, thus this instance can only be started manually.
'';
};
snapshotOnly = mkOption {
Copy link
Contributor

Choose a reason for hiding this comment

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

This option toggles between "snapshot" and "run" subcommand, thus preventing us from adding/using subcommand other than these two. Will "resume" or "prune" also be helpful in some cases? If so, we should add a "subcommand" option which defaults to "run" instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't think of an example where someone might want to run resume or prune as a service. resume will prune according to the configured retention policy after it's done backing up just like snapshot. You can't only use resume on its own either since btrbk compares against snapshots instead of source during the backup stage meaning snapshot has to have run before to create snapshots that haven't been backed up yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I'm convinced.

@wegank wegank added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Jan 5, 2025
@rhendric rhendric merged commit 289a4c6 into NixOS:master Jan 5, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants