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

Don't cache sshfs directory, fixes abiosoft/colima#99 #538

Merged
merged 2 commits into from
Jan 11, 2022

Conversation

rfay
Copy link
Contributor

@rfay rfay commented Jan 9, 2022

sshfs caching can cause asynchronous behaviors that aren't very happy, as discussed in https://stackoverflow.com/questions/3671350/any-way-to-eliminate-time-lag-with-sshfs

This adds the cache=no option to sshfs mounts to prevent this issue

In my case, a ddev test was adding a bunch of files to a mounted directory before starting a container that bind-mounted them. On the way up, it tried to access those files inside the container and got interesting behaviors where the files were listable, but not yet readable. SO

file /mnt/ddev_config/.homeadditions/.composer/auth.json
/mnt/ddev_config/.homeadditions/.composer/auth.json: regular file, no read permission

but

cat /mnt/ddev_config/.homeadditions/.composer/auth.json
cat: /mnt/ddev_config/.homeadditions/.composer/auth.json: No such file or directory

and of course a cp -r would fail.

Signed-off-by: Randy Fay <randy@randyfay.com>
Signed-off-by: Randy Fay <randy@randyfay.com>
@jandubois
Copy link
Member

It is not clear to me that this is the best option. Reading the osxfuse wiki page for this claims that you need to pair -o cache=no with -o nolocalcache, and recommends -o auto_cache instead (which may have better performance while also solving/reducing the caching issue).

So I guess I would like to see some explanation why we would choose one options over the other.

@rfay
Copy link
Contributor Author

rfay commented Jan 10, 2022

Thanks for the good thinking and link. I tried out the other options, but something was wrong with my testing, so I'll go back to it again and see what I can sort out.

@rfay
Copy link
Contributor Author

rfay commented Jan 10, 2022

I tried each combination as suggested in docs

  • auto_cache does not solve the problem
  • cache=no (as in this PR) does solve the problem
  • nolocalcaches is not an sshfs option, it's "fuse for osx" option, I'm not sure. I'm not sure where that would be configured, but am certainly interested.

Bottom line is that the PR as it is is the only one that prevents this issue.

@dee-kryvenko
Copy link
Contributor

I am facing the same (or similar) issue running Terraform in a pod with hostPath volume: hashicorp/terraform#30318. This is definitely related to sshfs as I can't reproduce that issue in the container nor the host outside of any sshfs mounts.

I do not remember facing this issue on RD <0.7.0. I wonder if there was any sshfs related changes in 0.7.0 or 0.7.1?

@dee-kryvenko
Copy link
Contributor

dee-kryvenko commented Jan 10, 2022

I am able to reproduce this with a simple git clone https://github.com/foo/bar.git && (cd bar && git checkout v0.1.4). If I do git clone https://github.com/foo/bar.git && (cd bar && git diff) - it shows random files being deleted:

$ git clone https://github.com/foo/bar.git && (cd bar && echo ">>> First diff" && git status && git diff && sleep 5s && echo ">>> Second diff" && git status && git diff)
Cloning into 'bar'...
remote: Enumerating objects: 271, done.
remote: Counting objects: 100% (271/271), done.
remote: Compressing objects: 100% (133/133), done.
remote: Total 271 (delta 125), reused 251 (delta 114), pack-reused 0
Receiving objects: 100% (271/271), 59.01 KiB | 397.00 KiB/s, done.
Resolving deltas: 100% (125/125), done.
>>> First diff
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   VERSION

no changes added to commit (use "git add" and/or "git commit -a")
diff --git a/VERSION b/VERSION
index 49d5957..e69de29 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +0,0 @@
-0.1
>>> Second diff
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean

Each time it's a new set of files.

Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Bottom line is that the PR as it is is the only one that prevents this issue.

Thanks! In that case I guess this is the way we have to go, and hope that 9p support will arrive soon. Leaving final decision to merge to @AkihiroSuda

@AkihiroSuda AkihiroSuda added this to the v0.8.1 milestone Jan 11, 2022
@@ -41,7 +42,7 @@ func (a *HostAgent) setupMount(ctx context.Context, m limayaml.Mount) (*mount, e
return nil, err
}
// NOTE: allow_other requires "user_allow_other" in /etc/fuse.conf
sshfsOptions := "allow_other"
sshfsOptions := "allow_other,cache=no"
Copy link
Member

Choose a reason for hiding this comment

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

This option seems specific to sshfs v2?
https://github.com/libfuse/sshfs/blob/ef3870a99710447a1c9219c6fcfce34bc5ab9760/sshfs.1.in#L67

Doesn't seem supported in sshfs v3 (not documented, at least)
https://github.com/libfuse/sshfs/blob/master/sshfs.rst

Copy link
Member

Choose a reason for hiding this comment

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

Ubuntu dpkg has really weird versioning scheme.. not sure whether we are using v3 or v2 🤷‍♂️
https://pkgs.org/download/sshfs
sshfs_3.6.0+repack+really2.10-0ubuntu1_amd64.deb

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 356988d into lima-vm:master Jan 11, 2022
jandubois added a commit to rancher-sandbox/lima that referenced this pull request Jan 29, 2022
This was changed to `false` in lima 0.8.0:

* lima-vm#538

It has generated multiple reports of breaking PHP package manager,
unit tests, and file system access itself:

* lima-vm#556
* abiosoft/colima#129
* rancher-sandbox/rancher-desktop#1349

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants