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

Add option to set working directory for restic backup #354

Merged
merged 7 commits into from
Apr 3, 2024

Conversation

seiuneko
Copy link
Contributor

This PR introduces the change-working-dir option within the resticprofile configuration, enabling users to set a specific working directory for the restic backup command. This enhancement is particularly beneficial for users backing up Btrfs snapshots, where the ability to use relative paths is crucial. By utilizing relative paths, we can omit the snapshot path prefix in the restic repository, which significantly simplifies the restoration process from snapshots.

Prior to this change, resticprofile would always expand relative paths to absolute paths (e.g., converting source: . to its absolute path), and the base-dir setting was only modifiable at the profile level, affecting path resolution across different sections.

With the addition of the change-working-dir option, users now have the flexibility to ensure that the source paths are not expanded into absolute paths. This is achieved by changing the restic backup command's working directory to the one specified by source-base.

I'm relatively new to Golang, so if there's any aspect of my implementation that's incorrect or if I've overlooked anything, please do let me know. I appreciate your feedback and thank you in advance!

@jkellerer
Copy link
Collaborator

@seiuneko , thanks a lot for your contribution 👍 at first sight it looks correct to me but I need to test it. So far there was the assumption that restic doesn't store relative paths but looking at it again it looks that in fact it does store the information internally (though not visible when listing snapshots) and it does make a difference for restore. Thanks for pointing it out.

Speaking of the config option, since what we really want is to enable relative source dir, I imagine the property could be named SourceRelative (the fact that it changes CWD for restic is a technical requirement so it may be documented but the main point is that source paths are kept relative).

Copy link

codecov bot commented Mar 29, 2024

Codecov Report

Attention: Patch coverage is 84.00000% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 71.50%. Comparing base (8297f50) to head (582757d).
Report is 1 commits behind head on master.

Files Patch % Lines
config/profile.go 88.24% 1 Missing and 1 partial ⚠️
wrapper.go 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #354      +/-   ##
==========================================
- Coverage   71.51%   71.50%   -0.01%     
==========================================
  Files         121      121              
  Lines       12659    12671      +12     
==========================================
+ Hits         9053     9060       +7     
- Misses       3201     3204       +3     
- Partials      405      407       +2     
Flag Coverage Δ
unittests 71.50% <84.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

shell/command_test.go Outdated Show resolved Hide resolved
shell/command_test.go Outdated Show resolved Hide resolved
shell/command_test.go Outdated Show resolved Hide resolved
config/profile.go Outdated Show resolved Hide resolved
@seiuneko
Copy link
Contributor Author

@jkellerer Thank you for your thorough review. I have revised the code according to your suggestions. Are there any other issues?

@jkellerer jkellerer added the enhancement New feature or request label Mar 30, 2024
@jkellerer jkellerer added this to the v0.27.0 milestone Mar 30, 2024
config/profile.go Outdated Show resolved Hide resolved
@jkellerer
Copy link
Collaborator

LGTM

Thanks a lot for the updates, just tried it with the following profile and it works perfectly fine (including error reporting for missing base paths):

relative:
  inherit: default
  base-dir: /opt/containers

  backup:
    source-relative: true
    source-base: /opt/snapshot_containers
    source: 
      - test-container
    run-before:
      - 'btrfs subvolume snapshot -r /opt/containers /opt/snapshot_containers'
    run-finally:
      - 'btrfs subvolume delete /opt/snapshot_containers'

  ls:
    path: true

  restore:
    path: true
    target: '.'

In detail:

  • resticprofile relative.backup - create a backup from btrfs snapshopt
  • resticprofile relative.ls latest - lists the files (with relative paths)
  • resticprofile relative.restore latest - restores files into the original base-dir

Only remaining issue I found is that restic resolves paths differently than resticprofile but I'll take care of it in a separate PR.

@jkellerer
Copy link
Collaborator

@creativeprojects , I'm good with the contribution, do you have any concerns?

@creativeprojects
Copy link
Owner

Thanks for the PR, I'll give it a try on my zfs snapshots 👍🏻

@creativeprojects
Copy link
Owner

I guess it doesn't really change much in the end, but it can simplify the configuration.

It looks good to me 👍🏻

@jkellerer jkellerer merged commit f9b5dac into creativeprojects:master Apr 3, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants