creativeprojects / resticprofile

Configuration profiles manager and scheduler for restic backup
https://creativeprojects.github.io/resticprofile/
GNU General Public License v3.0
604 stars 29 forks source link

Add option to set working directory for restic backup #354

Closed seiuneko closed 3 months ago

seiuneko commented 3 months ago

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 commented 3 months ago

@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).

codecov[bot] commented 3 months ago

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 :warning:
wrapper.go 50.00% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ 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](https://app.codecov.io/gh/creativeprojects/resticprofile/pull/354/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fred) | Coverage Ξ” | | |---|---|---| | [unittests](https://app.codecov.io/gh/creativeprojects/resticprofile/pull/354/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fred) | `71.50% <84.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fred#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

seiuneko commented 3 months ago

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

jkellerer commented 3 months ago

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:

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 commented 3 months ago

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

creativeprojects commented 3 months ago

Thanks for the PR, I'll give it a try on my zfs snapshots πŸ‘πŸ»

creativeprojects commented 3 months ago

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

It looks good to me πŸ‘πŸ»