-
-
Notifications
You must be signed in to change notification settings - Fork 607
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
Create /home/vagrant/trellis bindfs mount with proper permissions #705
Conversation
@@ -67,11 +67,13 @@ Vagrant.configure('2') do |config| | |||
fail_with_message "vagrant-hostmanager missing, please install the plugin with this command:\nvagrant plugin install vagrant-hostmanager" | |||
end | |||
|
|||
bin_path = File.join(ANSIBLE_PATH.sub(__dir__, '/vagrant'), 'bin') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line after this please 👼
89cbab0
to
06ae36c
Compare
Hi @fullyint! I just tried implementing the changes from this pull request (was on 8453c53). I am on a Windows host and using Winnfsd. On 8453c53 everything works fine and Winnfsd is able to mount the shared folders that I have. But when running Running
Just let me know if you need more information from me to help you. |
Some more information:
I looked at the repo and found this commit gael-ian/vagrant-bindfs@f07ab7c. Especially this line is interesting
|
@arashohadi Thank you for testing. Your information is very helpful. After updating my If - bin_path = File.join(ANSIBLE_PATH.sub(__dir__, '/vagrant'), 'bin')
+ bin_path = File.join(ANSIBLE_PATH.sub(__dir__, '/home/vagrant/trellis'), 'bin')
if Vagrant::Util::Platform.windows? and !Vagrant.has_plugin? 'vagrant-winnfsd'
wordpress_sites.each_pair do |name, site|
config.vm.synced_folder local_site_path(site), remote_site_path(name, site), owner: 'vagrant', group: 'www-data', mount_options: ['dmode=776', 'fmode=775']
end
- config.vm.synced_folder '.', '/vagrant', mount_options: ['dmode=755', 'fmode=644']
+ config.vm.synced_folder '.', '/home/vagrant/trellis', mount_options: ['dmode=755', 'fmode=644']
config.vm.synced_folder File.join(ANSIBLE_PATH, 'bin'), bin_path, mount_options: ['dmode=755', 'fmode=755']
else
if !Vagrant.has_plugin? 'vagrant-bindfs'
fail_with_message "vagrant-bindfs missing, please install the plugin with this command:\nvagrant plugin install vagrant-bindfs"
else
wordpress_sites.each_pair do |name, site|
config.vm.synced_folder local_site_path(site), nfs_path(name), type: 'nfs'
config.bindfs.bind_folder nfs_path(name), remote_site_path(name, site), u: 'vagrant', g: 'www-data', o: 'nonempty'
end
- config.vm.synced_folder '.', '/vagrant-nfs', type: 'nfs'
+ config.vm.synced_folder '.', '/vagrant', type: 'nfs'
- config.bindfs.bind_folder '/vagrant-nfs', '/vagrant', o: 'nonempty', p: '0644,a+D'
+ config.bindfs.bind_folder '/vagrant', '/home/vagrant/trellis', o: 'nonempty', p: '0644,a+D'
config.bindfs.bind_folder bin_path, bin_path, perms: '0755'
end
end
provisioner = Vagrant::Util::Platform.windows? ? :ansible_local : :ansible
- provisioning_path = Vagrant::Util::Platform.windows? ? ANSIBLE_PATH.sub(__dir__, '/vagrant') : ANSIBLE_PATH
+ provisioning_path = Vagrant::Util::Platform.windows? ? ANSIBLE_PATH.sub(__dir__, '/home/vagrant/trellis') : ANSIBLE_PATH This change would require users to I don't know if that will resolve the |
I added a commit based on the diff above. It works on my Windows 10 with encrypted On the VM, the Windows users would switch to running Ansible commands from Does anyone see any problem with this change to Note that if the I think only the Windows docs would need a few minor updates. |
f8003ae
to
c3a8549
Compare
c3a8549
to
a1c0899
Compare
Fixes an issue where Ansible, running in a VM on a Windows host, throws an error while interpreting a
.vault_pass
file as executable.This PR implements a variation of
@aoe
's approach, using vagrant-bindfs to mount the/vagrant
share with non-executable permissions. This PR makes a separate bindfs mount for/vagrant/bin
executable, the only items that need to be executable, as far as I know.Punishing details below.
NFS performance
I could have wrapped the new vagrant-bindfs mounts (lines 85-87) in a conditional for Windows only, but it seems simpler without. I compared times from 20 trials of a test playbook that runs various Ansible
local_action
tasks on the VM from a regular VirtualBox share (current Trellis) and from an NFS share (result of this PR). The NFS mean time of 23.245 seconds (SD = 2.018) was slightly faster than the VB share mean of 23.603 seconds (SD = 2.401), but the difference was not statistically significant, p = 0.613. This suggests that changing to NFS doesn't incur any performance hit.Pipelining
Performance tests while preparing this PR also examined setting
pipelining = True
inansible.cfg
.The Trellis Ubuntu Xenial distro does not have
requiretty
in/etc/sudoers
, so 👍 pipelining.Repeating the performance tests mentioned above, but with pipelining enabled, the NFS mean time of 18.238 seconds (SD = 0.323) was again slightly faster than the VB share mean of 18.618 seconds (SD = 0.188), and the difference statistically significant, p < 0.001 (very small difference nonetheless). This again suggests that changing to NFS doesn't incur any performance hit.
Mount options
I tested various combinations of
mount_options
: ['rw', 'vers=3', 'tcp', fsc', 'actimeo=1']
that appear in various internet reports of how to speed up NFS performance on Vagrant. According tonfsstat -m
(command run on the VM) our nfs shares automatically apply therw
andvers=3
. The other options occasionally achieved tiny improvements that were statistically significant, but their effect size was so small they seemed negligible and not worth including.The
fsc
would require installing and enablingcachefilesd
on the VM, then remounting the NFS shares, which would complicate thevagrant up
process, unless we start using a vagrant box withcachefilesd
already installed/enabled.The
tcp
option is said to be slower, but helpful if NFS is used over "lossy networks." This didn't feel applicable, and testing results didn't show a clear benefit.The
actimeo
option to set file attribute cache times would perhaps improvegulp watch
response times ifgulp
were being run on the VM (example), but the Roots recommendation is to rungulp
on the host machine.I also tested these mount options on the NFS share with the
site
files. The 20 iterations oftouch site/web/app/themes/sage/assets
then measuring thegulp watch
build times and Browsersync page load times with a VM running roots/sage 8.5.0 again occasionally showed statistically significant differences, but again they had negligibly small effect sizes, so I omitted all mount options forsite
. Perhaps someone will want to revisit these mount options with Sage 9 when it's released.Permissions on .vault_pass
A nice thing about
@aoe
's approach of.vault_pass
into a newvault/
dir is thep: '0000,u=rD'
sog
ando
have no permissions for.vault_pass
. However, for Trellis to sync avault/
directory by default, the directory would have to always be present, whether added to Trellis core, or created by every user, which seems impractical. Users who want tighter permissions on just.vault_pass
can create their own directory and sync it as@aoe
suggested. Even then,.vault_pass
would still be readable byvagrant
user, the user that I imagine will be used by all accessing the VM.