nixos/restic: Add rclone options #91424

i077 · 2020-06-24T17:26:55Z

Motivation for this change

Restic provides the ability to backup to rclone remotes, but options to specify rclone configuration for restic are not available here. Rclone allows global options (bandwidth limits, use checksums, etc.) to be set via environment variables, and remote-specific config to be set via either environment variables or a config file, the latter of which takes precedence.

I'm adding 3 new options to the restic module here, rcloneOptions, rcloneConfig, and rcloneConfigFile, which allows the user to specify the above options respectively.

I also modified the existing restic test to backup to a new local repository just using rclone. It backs up the exact same stuff in the same way. Unfortunately I can't actually run this test because I'm running into a weird kernel panic error no matter what test I run. If someone could either help me diagnose this error (here is the log, the error seems to happen at line 366) or just run the restic.nix test themselves, I'd be grateful.

I do know that this works: I tested it on my own config with a OneDrive rclone remote, which produces the exact same systemd service file.

Things done

flokli · 2020-06-24T19:09:25Z

@i077 I didn't get the kernel panic. Wild shot into the blue: Try booting into a more recent kernel ;-)

However, the test fails, as rclone isn't in $PATH:

server: must succeed: /nix/store/m7zgfvbqs31bnzadpgxiqh088zfmfx28-restic-0.9.6/bin/restic -r /tmp/restic-backup -p /nix/store/9qx02d6lfmfp3jkfbnylgx9r4ir5yl44-password snapshots -c | grep -e "^1 snapshot"
(0.73 seconds)
server: must succeed: /nix/store/m7zgfvbqs31bnzadpgxiqh088zfmfx28-restic-0.9.6/bin/restic -r rclone:local:/tmp/restic-rclone-backup -p /nix/store/9qx02d6lfmfp3jkfbnylgx9r4ir5yl44-password snapshots -c | grep -e "^1 snapshot"
server # Fatal: unable to open repo at rclone:local:/tmp/restic-rclone-backup: cmd.Start: exec: "rclone": executable file not found in $PATH
server: output:
error: command `/nix/store/m7zgfvbqs31bnzadpgxiqh088zfmfx28-restic-0.9.6/bin/restic -r rclone:local:/tmp/restic-rclone-backup -p /nix/store/9qx02d6lfmfp3jkfbnylgx9r4ir5yl44-password snapshots -c | grep -e "^1 snapshot"` failed (exit code 1)
cleaning up
killing server (pid 9)

I believe we want to prefix PATH with ${rclone}/bin, like we do with borg and ${openssh}/bin.

i077 · 2020-06-24T19:20:56Z

Wild shot into the blue: Try booting into a more recent kernel ;-)

I'm actually on kernel 5.7.4 right now. Maybe that's too new?

However, the test fails, as rclone isn't in $PATH:
...
I believe we want to prefix PATH with ${rclone}/bin, like we do with borg and ${openssh}/bin.

Oh yeah, that makes sense, since restic doesn't magically know where rclone is outside of the systemd service. Should I just add pkgs.rclone to environment.systemPackages in the test server?

EDIT: Booted into a previous generation with kernel 5.6.18 and tests there work fine.

i077 · 2020-06-24T20:56:13Z

Okay, I've updated the test so rclone is in $PATH. I also had to add a session variable to tell rclone that local was a local provider outside of the systemd service. The test now passes on my end.

flokli · 2020-06-25T09:04:02Z

I believe we want to prefix PATH with ${rclone}/bin, like we do with borg and ${openssh}/bin.

I'd prefer that approach, as it wouldn't require users to also have rclone in their $PATH to be able to mount or restore things with restic.

i077 · 2020-06-25T12:42:46Z

Rclone is already in the PATH of the systemd service (see line 242 of the restic module), if that's what you're asking. Users don't need to install rclone separately to start a backup service.

I'm not sure how to add it outside other than exporting a new PATH or just adding it to environment.systemPackages (which is what I did in the test). You'll need rclone outside of restic to set up the remote separately anyways.

i077 · 2020-07-02T20:19:27Z

Is this PR ready to be merged? Everything works on my end.

flokli · 2020-07-02T21:42:25Z

As written in #91424 (comment), I think wrapping restic to always find rclone seems to be less error-prone (and is consistent to what we do with borg and ssh).

restic seems to be pretty coupled with rclone. Users should just be able to use restic, also to restore from rclone remotes, and don't need to worry about also having rclone in $PATH.

That's how I'd do it:

diff --git a/pkgs/tools/backup/restic/default.nix b/pkgs/tools/backup/restic/default.nix
index f366533f9bf..e8d7ea3346a 100644
--- a/pkgs/tools/backup/restic/default.nix
+++ b/pkgs/tools/backup/restic/default.nix
@@ -1,4 +1,11 @@
-{ stdenv, lib, buildGoPackage, fetchFromGitHub, installShellFiles, nixosTests}:
+{ stdenv
+, lib
+, buildGoPackage
+, fetchFromGitHub
+, installShellFiles
+, makeWrapper
+, nixosTests
+, rclone }:
 
 buildGoPackage rec {
   pname = "restic";
@@ -15,11 +22,13 @@ buildGoPackage rec {
 
   subPackages = [ "cmd/restic" ];
 
-  nativeBuildInputs = [ installShellFiles ];
+  nativeBuildInputs = [ installShellFiles makeWrapper ];
 
   passthru.tests.restic = nixosTests.restic;
 
-  postInstall = lib.optionalString (stdenv.hostPlatform == stdenv.buildPlatform) ''
+  postInstall = ''
+    wrapProgram $out/bin/restic --prefix PATH : '${rclone}/bin'
+  '' + lib.optionalString (stdenv.hostPlatform == stdenv.buildPlatform) ''
     $out/bin/restic generate \
       --bash-completion restic.bash \
       --zsh-completion restic.zsh \

And then of course rclone doesn't need to be in $PATH of the systemd unit anymore.

i077 · 2020-07-05T20:28:00Z

Ah, I didn't realize you were talking about the actual derivations for restic (and borg and ssh). I added the wrapper you mentioned and pushed everything up again. Test is still passing.

nixos/modules/services/backup/restic.nix

flokli · 2020-07-06T06:51:54Z

nixos/modules/services/backup/restic.nix

+            Warning: Secrets set in here will be world-readable in the Nix
+            store! Consider using the <literal>rcloneConfigFile</literal>
+            option instead to specify secret values, which will take precedence
+            over options set here.


How well does using a config file and using these options play together?

The environment variables essentially set the "default" value for rclone, but if it finds that value also set in the config file, rclone will use the value in the latter.

For example, if I specified RCLONE_CONFIG_REMOTEA_TYPE=s3, but then in the config file I had

[remoteA] type=b2

rclone will treat remoteA as a B2 remote.

~~So the user can set both values, but on a conflict, the config file will win.~~

EDIT: Looks like this doesn't actually happen. I'm updating the test so that rcloneConfig.type = "ftp", which would get overridden by a config file I'd add that says the type is local, but rclone still seems to think the remote is of type ftp.

EDIT 2: Okay, looks like it's the other way around. On conflict, the env variables win. I'll update the options and test to reflect that.

Is this intended behaviour? I'd doubt. Maybe open a bug upstream and don't specify it here (or at least link to the bug).

We shouldn't mark this as a feature, so people don't rely on it.

I opened a question on their forums here to confirm whether this is intended behavior. If it turns out I'm not supposed to do this (or that we shouldn't rely on it regardless), I can either

Remove rcloneConfig and just have users use rcloneConfigFile on its own, or

Keep both options, but add an assertion to the module that only one option can be used at a time. Users can use one option or the other, but not both.

Which would be preferable?

https://rclone.org/docs/#environment-variables reads like environment variables should set the default, but not win over command line arguments or config files.

Given rclone also supports config file encryption, we should definitely keep rcloneConfigFile.

I'm not sure if we need both rcloneConfig and rcloneOptions (if they provide the same feature set). WDYT?

Yeah, that wording in their docs wasn't the best. Hopefully they'll clear things up. I agree we should keep the rcloneConfigFile option.

rcloneConfig and rcloneOptions provide different things:

rcloneOptions was meant to control global rclone behavior, i.e. options that are listed at https://rclone.org/docs/#options. This is stuff like bandwidth limits, using checksums to compare files, etc.

rcloneConfig is used to configure specific remotes, e.g. S3 access credentials, etc, i.e. stuff available in the docs for specific remote types (for example, https://rclone.org/s3/#standard-options for S3 remotes).

Looks like this is intended behavior. See linked post for the order of precedence. I think since they specified the order (and it seems like they plan on keeping that behavior around) we should keep both options and allow users to specify both simultaneously.

@flokli if you're good with keeping both options, I'm going to resolve this thread.

flokli · 2020-07-11T21:57:44Z

Thanks for your patience and the PR!

My PR in NixOS/nixpkgs#91424 was merged!

ofborg bot added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jun 24, 2020

i077 force-pushed the restic-rclone-opts branch 2 times, most recently from 67dc600 to 608dc71 Compare June 24, 2020 20:53

flokli requested review from Mic92 and Lassulus and removed request for Mic92 June 25, 2020 09:18

restic: Wrap binary with rclone

5619182

i077 force-pushed the restic-rclone-opts branch from 608dc71 to 9c75ab4 Compare July 5, 2020 20:27

ofborg bot requested a review from mbrgm July 5, 2020 20:35

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 and removed 10.rebuild-darwin: 0 10.rebuild-linux: 1 labels Jul 5, 2020

flokli reviewed Jul 6, 2020

View reviewed changes

i077 force-pushed the restic-rclone-opts branch from 9c75ab4 to 9aa3144 Compare July 6, 2020 14:05

Imran Hossain added 2 commits July 6, 2020 10:27

nixos/restic: Add options for rclone repositories

7dd656a

nixos/tests: Add rclone repository to restic test

9b80955

i077 force-pushed the restic-rclone-opts branch from 9aa3144 to 9b80955 Compare July 6, 2020 14:30

flokli merged commit 8c0708f into NixOS:master Jul 11, 2020

i077 pushed a commit to i077/system that referenced this pull request Aug 12, 2020

backup: Add rclone options to restic module

7af4fa4

My PR in NixOS/nixpkgs#91424 was merged!

eliasp mentioned this pull request Apr 10, 2021

rclone path missing from systemd unit created by services.restic.backups #76146

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/restic: Add rclone options #91424

nixos/restic: Add rclone options #91424

i077 commented Jun 24, 2020 •

edited

flokli commented Jun 24, 2020 •

edited

i077 commented Jun 24, 2020 •

edited

i077 commented Jun 24, 2020

flokli commented Jun 25, 2020

i077 commented Jun 25, 2020 •

edited

i077 commented Jul 2, 2020

flokli commented Jul 2, 2020

i077 commented Jul 5, 2020 •

edited

flokli Jul 6, 2020

i077 Jul 6, 2020 •

edited

flokli Jul 6, 2020

i077 Jul 6, 2020 •

edited

flokli Jul 6, 2020

i077 Jul 6, 2020 •

edited

i077 Jul 6, 2020 •

edited

i077 Jul 8, 2020

flokli commented Jul 11, 2020

nixos/restic: Add rclone options #91424

nixos/restic: Add rclone options #91424

Conversation

i077 commented Jun 24, 2020 • edited

Motivation for this change

Things done

flokli commented Jun 24, 2020 • edited

i077 commented Jun 24, 2020 • edited

i077 commented Jun 24, 2020

flokli commented Jun 25, 2020

i077 commented Jun 25, 2020 • edited

i077 commented Jul 2, 2020

flokli commented Jul 2, 2020

i077 commented Jul 5, 2020 • edited

flokli Jul 6, 2020

Choose a reason for hiding this comment

i077 Jul 6, 2020 • edited

Choose a reason for hiding this comment

flokli Jul 6, 2020

Choose a reason for hiding this comment

i077 Jul 6, 2020 • edited

Choose a reason for hiding this comment

flokli Jul 6, 2020

Choose a reason for hiding this comment

i077 Jul 6, 2020 • edited

Choose a reason for hiding this comment

i077 Jul 6, 2020 • edited

Choose a reason for hiding this comment

i077 Jul 8, 2020

Choose a reason for hiding this comment

flokli commented Jul 11, 2020

i077 commented Jun 24, 2020 •

edited

flokli commented Jun 24, 2020 •

edited

i077 commented Jun 24, 2020 •

edited

i077 commented Jun 25, 2020 •

edited

i077 commented Jul 5, 2020 •

edited

i077 Jul 6, 2020 •

edited

i077 Jul 6, 2020 •

edited

i077 Jul 6, 2020 •

edited

i077 Jul 6, 2020 •

edited