Added support for restic --ignore-inode

[weisdd]

Issue
When backing up data, restic considers the following attributes to decide whether files have changed since the last snapshot:

• Type (file, symlink, or directory?)
• Modification time
• Size
• Inode number (internal number used to reference a file in a file system)

The last one, inode number, should not be considered in case of FUSE-based filesystems (e.g. mounted via s3-fuse like minio) as they do not support inode consistency, which basically means that inode numbers change on every remount. Otherwise, we'll get two side effects:

• since restic thinks files are new, it has to process them all over again
  => might slow down the process;
• the restic statistics gets distorted
  e.g. "files_new":109,"files_changed":6202,"files_unmodified":525 - here, in my test, actually, 109 files were added, but none of 6202 really changed, and 525 happened to have the same inode numbers - just pure luck)
  => metrics are also distorted as a consequence.

Note: fortunately, all of that doesn't lead to anything bad like data corruption, it's just some extra work and incorrect statistics.

Proposed change
We only need to add an option to ignore inode changes (--ignore-inode).

Important notes
I'm not an expert in golang nor in your package, so the way I implemented the feature might not be the best one, and maybe there are extra cases where the --ignore-inode flag should be called.
I tested the change with v0.10.0-alpha.0 for backing up Minio via s3-fuse, and I'm pretty sure it will work with any other version of the package as there's nothing version specific.

As for the stash plugins, something simple like this will bring the support wherever it's needed:
cmd.Flags().BoolVar(&opt.setupOptions.IgnoreInode, "ignore-inode", opt.setupOptions.IgnoreInode, "Specify whether restic should ignore inodes (useful for FUSE mounts)")

[weisdd]

stashed/stash#1138

[hossainemruz]

Hi @weisdd Thank you for the PR. Please sign your commit. You can follow the steps recommended by DCO bot: https://github.com/stashed/apimachinery/pull/75/checks?check_run_id=1588113497

Btw, since --ignore-inode is a property of backend, it should be a configurable field in Repository CRD. It can also go in runtimeSettings of BackupConfiguration similar to nice and ionice. I will talk with our team about where it should fit. It may take some time as we are middle of some other tasks. Please be patient. This definitely going to be part of our next major release.

[weisdd]

Hi @hossainemruz
I've just signed the commit, the DCO check is successful now.

Thanks for the extra details!
I chose the current implementation just because it lets me enable the flag dynamically through a helm chart with a Function.
Should the change go somewhere else, I'll still be happy to see that the PR served as a simple POC. :)

[hossainemruz]

Should the change go somewhere else.

No. These changes are fine. You will also need to add this here: https://github.com/weisdd/apimachinery/blob/e7025ffbf8c9029da77c75c2e209ce9bad018baa/pkg/restic/commands.go#L165

We have to decide how should we resolve the value of w.config.IgnoreInode. It can come from a field of Repository CRD or from runtimeSettings.

[weisdd]

@hossainemruz I intentionally skipped the function for stdin. My train of thought was that inode becomes an issue only when you mount an external filesystem in userspace and backup files from there. When it's something read from stdin, there should not be an intermediate inode to care about. Plus, the mtime will change anyway, so other factors will trigger restic to analyze the file. Thus, --ignore-inode should not make any difference in this context.

[hossainemruz]

You are right @weisdd. We don't need --ignore-inode for backup stdin. Thank you for pointing it out.

[CLAassistant]

All committers have signed the CLA.

[weisdd]

Any plans to merge the PR or implement the functionality differently?

[hossainemruz]

We are really sorry that it is taking too long. I am currently occupied with a different task. I will return back to Stash in a week or two. Then, I can revisit this PR.

[weisdd]

@hossainemruz Honestly, I feel like closing the PR as it's been already 8 months for a simple flag.

[hossainemruz]

Ahh. I forgot about this PR. Sorry, for the delay. I will take a look soon.

[hossainemruz]

@weisdd We have decided to introduce a generic option to pass user arguments to the restic backup/restore command. Check this PR: #121

Now, you can pass any flags (including --ignore-inode) to the restic backup/restore command. Please see the example here: stashed/stash#1384

[weisdd]

@hossainemruz that's even better than my solution, thanks!

[hossainemruz]

Sorry, for this feature request took too long. We were busy with other stuff. Closing this PR now.