tknerr / vagrant-managed-servers

Enables Vagrant to ssh into and provision managed servers
MIT License
185 stars 30 forks source link

Smb synced folders for Windows "guests" #47

Closed chrisbaldauf closed 9 years ago

chrisbaldauf commented 9 years ago

Change default synced_folders behavior on Windows host/guest pair to use SMB file sharing, rather than the very slow uploading over WinRM.

I've tested this on a Win7 host and Windows Server 2008 managed host. You will be prompted for credentials if you don't include them in the Vagrantfile per the docs

FWIW, I'm planning on adding functionality to Vagrant Orchestrate that will prompt you once for credentials and pass them to winrm and SMB.

I assumed that this would be a big enough change to warrant a 0.7 release, but if that isn't the case, let me know and I can turn this into a 0.6.x change.

Meant to address #46

Happy to discuss if you have any feedback.

tknerr commented 9 years ago

Please help me understand:

From what I have experienced playing around with automating windows guests, I got the impression that vagrant by default uses WinRM for synced folders too? I'm not 100% sure with that, but it was my impression at least (and I never ran it from an admin shell and I was never prompted for passwords either).

Is it right, that the Vagrant::Action::Builtin::SyncedFolders determine that automatically? My assumption is that it would use winrm by default, but also allow you to switch to the smb implementation as per the docs you linked above.

I might be totally wrong, just trying to understand.

tknerr commented 9 years ago

Trying to get an example running, and I see what you mean now with winrm uploads being "painfully slow" ;-)

tknerr commented 9 years ago

However, I have some kind of weird behaviour:

May that be because I am not running from a privileged shell?

My example and debug gist follows...

chrisbaldauf commented 9 years ago

Were you running this from a Windows or non-Windows host? I also didn't see the output gist, my apologies if I missed something obvious.

tknerr commented 9 years ago

I'm running this from a windows host, using the latest tknerr/bills-kitchen version. It has rsync.exe in the PATH, maybe that makes a difference

Didn't come to uploading the gist yet, was interrupted by $dayjob ;-)

chrisbaldauf commented 9 years ago

I'm trying to repro using the Vagrantfile in your WIP branch on my Windows host and will report back what I find.

tknerr commented 9 years ago

So, here's my current state:

Haven't had the chance to analyze the log output yet, and will be afk for a while now.

Most interesting for me would be if you can reproduce it, or whether it's specific to my env...

chrisbaldauf commented 9 years ago

I'll check the gist and see if it differs from what I'm seeing. I was using a "real" managed server in my testing, so I'll try to control for that as well. Thanks for taking a look!

On Tue, May 19, 2015 at 10:48 AM, Torben Knerr notifications@github.com wrote:

So, here's my current state:

Haven't had the chance to analyze the log output yet, and will be afk for a while now.

Most interesting for me would be if you can reproduce it, or whether it's specific to my env...

— Reply to this email directly or view it on GitHub https://github.com/tknerr/vagrant-managed-servers/pull/47#issuecomment-103532482 .

chrisbaldauf commented 9 years ago

Update: while trying to debug another, semi-unrelated issue, I was able to repro your case. I'm digging in further now.

chrisbaldauf commented 9 years ago

I think I've figured out what was going on - the SMB synced folder action was not usable? from your machine because either a) you weren't running from an admin command prompt or b) you don't have PowerShell 3 or greater installed. I've revised the implementation to check whether the SMB folder sync is usable and if not - 1) print why and 2) use WinRM. Please consider the following Proof of Concept code

      def self.action_provision
        Vagrant::Action::Builder.new.tap do |b|
          b.use ConfigValidate
          b.use WarnNetworks
          b.use Call, IsLinked do |env, b2|
            unless env[:result]
              b2.use MessageNotLinked
              next
            end

            b2.use Call, IsReachable do |env, b3|
              unless env[:result]
                b3.use MessageNotReachable
                next
              end

             b3.use Provision
              if env[:machine].config.vm.communicator == :winrm
                env[:ui].info("Checking if SMB folder sync is usable")
                smb_usable = false
                begin
                  smb_synced_folder_class = Vagrant.plugin("2").manager.synced_folders[:smb][0]
                  smb_usable = smb_synced_folder_class.new.usable?(machine: nil, raise_error: true)
                rescue => ex
                  env[:ui].warn("Unable to use SMB folder sync, falling back to WinRM")
                  env[:ui].warn(ex.message)
                end

                if smb_usable
                  b3.use Vagrant::Action::Builtin::SyncedFolders
                else
                  b3.use SyncFolders
                end
              else
                # Vagrant managed servers custom implementation
                b3.use SyncFolders
              end
            end
          end
        end
      end

I still need to handle a few other things, but it would be good to validate this new approach works for you before I invest more time.

TODO:

tknerr commented 9 years ago

@chrisbaldauf let me do some further investigation. I just noted that the initial gist I posted was completely useless, as the remote windows server was not running. Just updated it with the more meaningful log output. Can not see from the logs why it thinks that rsync would be the best option though....

tknerr commented 9 years ago

After removing rsync.exe from the PATH I now get Vagrant::Errors::NoDefaultSyncedFolderImpl

...
 INFO interface: error: Vagrant::Errors::NoDefaultSyncedFolderImpl: No synced folder implementation is available for your synced folders!
Please consult the documentation to learn why this may be the case.
You may force a synced folder implementation by specifying a "type:"
option for the synced folders. Available synced folder implementations
are listed below.

docker, nfs, rsync, smb, virtualbox
Vagrant::Errors::NoDefaultSyncedFolderImpl: No synced folder implementation is available for your synced folders!
Please consult the documentation to learn why this may be the case.
You may force a synced folder implementation by specifying a "type:"
option for the synced folders. Available synced folder implementations
are listed below.

Also updated the gist with debug log output from the without-rsync case

tknerr commented 9 years ago

After telling vagrant explicitly to use "smb":

    ms_config.vm.synced_folder ".", "/vagrant", type: "smb"

I now get a clear error message:

...
VagrantPlugins::SyncedFolderSMB::Errors::WindowsAdminRequired: SMB shared folders require running Vagrant with administrative
privileges. This is a limitation of Windows, since creating new
network shares requires admin privileges. Please try again in a
console with proper permissions or use another synced folder type.
tknerr commented 9 years ago

Once I run the above from an elevated command prompt to make smb work, vagrant seems to hang after checking the powershell version. Also looks like I have powershell v2 installed, so I would need v3 anyway...

Here's where it stalls:

...
 INFO provision: Checking provisioner sentinel file...
 INFO warden: Calling IN action: #<Berkshelf::Vagrant::Action::Install:0x5e2fd68>
 INFO interface: warn: Berkshelf plugin is disabled but a Berksfile was found at your configured path: C:/Repos/_github/vagrant-managed-servers/Berksfile Berkshelf plugin is disabled but a Berksfile was found at your configured path: C:/Repos/_github/vagrant-managed-servers/Berksfile
 INFO interface: warn: Enable the Berkshelf plugin by setting 'config.berkshelf.enabled = true' in your vagrant config
Enable the Berkshelf plugin by setting 'config.berkshelf.enabled = true' in your vagrant config
 INFO warden: Calling IN action: #<Berkshelf::Vagrant::Action::Upload:0x5e2fd20>
 INFO warden: Calling IN action: #<VagrantPlugins::Omnibus::Action::InstallChef:0x5e2fd08>
 INFO warden: Calling IN action: #<Vagrant::Action::Builtin::SyncedFolders:0x5e14c18>
 INFO subprocess: Starting process: ["C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\/powershell.EXE", "-NoProfile", "-ExecutionPolicy", "Bypass", "$PSVersionTable.PSVersion.Major"]
DEBUG subprocess: Selecting on IO
DEBUG subprocess: stdout: 2
tknerr commented 9 years ago

Also, once I am in an elevated shell, I no longer need this line in the managed server config:

 ms_config.vm.synced_folder ".", "/vagrant", type: "smb"

Even without the config above, it will try to use smb when run from an elevated shell. Still it stalls at the same place as mentioned above

chrisbaldauf commented 9 years ago

I was able to reproduce on a windows machine running powershell 2.0. I think I've got enough feedback for now and I'll follow up when I've got something ready. Thanks!

On May 19, 2015, at 5:53 PM, Torben Knerr notifications@github.com wrote:

Once I run the above from an elevated command prompt to make smb work, vagrant seems to hang after checking the powershell version. Also looks like I have powershell v2 installed, so I would need v3 anyway...

Here's where it stalls:

... INFO provision: Checking provisioner sentinel file... INFO warden: Calling IN action: #Berkshelf::Vagrant::Action::Install:0x5e2fd68 INFO interface: warn: Berkshelf plugin is disabled but a Berksfile was found at your configured path: C:/Repos/_github/vagrant-managed-servers/Berksfile Berkshelf plugin is disabled but a Berksfile was found at your configured path: C:/Repos/_github/vagrant-managed-servers/Berksfile INFO interface: warn: Enable the Berkshelf plugin by setting 'config.berkshelf.enabled = true' in your vagrant config Enable the Berkshelf plugin by setting 'config.berkshelf.enabled = true' in your vagrant config INFO warden: Calling IN action: #Berkshelf::Vagrant::Action::Upload:0x5e2fd20 INFO warden: Calling IN action: #VagrantPlugins::Omnibus::Action::InstallChef:0x5e2fd08 INFO warden: Calling IN action: #Vagrant::Action::Builtin::SyncedFolders:0x5e14c18 INFO subprocess: Starting process: ["C:\Windows\System32\WindowsPowerShell\v1.0\/powershell.EXE", "-NoProfile", "-ExecutionPolicy", "Bypass", "$PSVersionTable.PSVersion.Major"] DEBUG subprocess: Selecting on IO DEBUG subprocess: stdout: 2 — Reply to this email directly or view it on GitHub.

tknerr commented 9 years ago

@chrisbaldauf thanks, too!

I think we'll need both: the existing slow (but most compatible) winrm upload, and if usable? the better smb support for hosts that support it.

We can try to autodetect what's better, but it would be also nice to have config option to override.

I would also be 100% fine with:

That way we would not need any auto-detection at all (but also have no auto-detection ;-)) and would not need to introduce an additional config option.

If you say that auto-detection is useful for you, let's add it. You care more about the windows / smb support than I do currently. And you contributed the whole windows support. So the decision should definitely be up to you

tknerr commented 9 years ago

For the sake of documentation and completeness:

Without an elevated prompt: not usable? in my environment

C:\Repos\_github\vagrant-managed-servers>bundle exec irb
Your Gemfile lists the gem vagrant-managed-servers (>= 0) more than once.
You should probably keep only one of them.
While it's not a problem now, it could cause errors if you change the version of just one of them later.
irb(main):001:0> require 'vagrant'
=> true
irb(main):002:0> x = Vagrant.plugin("2").manager.synced_folders[:smb][0]
=> VagrantPlugins::SyncedFolderSMB::SyncedFolder
irb(main):003:0> x.new.usable?(machine: nil, raise_error: true)
=> false

Within an elevated prompt: aksing if usable? is stalling and never returns

C:\Repos\_github\vagrant-managed-servers>bundle exec irb
Your Gemfile lists the gem vagrant-managed-servers (>= 0) more than once.
You should probably keep only one of them.
While it's not a problem now, it could cause errors if you change the version of just one of them later.
irb(main):001:0> require 'vagrant'
=> true
irb(main):002:0>  x = Vagrant.plugin("2").manager.synced_folders[:smb][0]
=> VagrantPlugins::SyncedFolderSMB::SyncedFolder
irb(main):003:0> x.new.usable?(machine: nil, raise_error: true)
tknerr commented 9 years ago

@chrisbaldauf if we wanted to do it really clean and in line with the vagrant philosophy, this would probably mean:

This would be the best / cleanest solution imho, but probably the most effort too. I don't have time to invest for this right now, though :-/. If you have, feel free to go ahead, I'll happily support you with testing. If not, let's make it work first and then make it nice (later, in a second step).

tknerr commented 9 years ago

Thinking again... maybe the above clean approach would not be so much extra work though, guess it could save us some work afterwards, e.g.:

tknerr commented 9 years ago

On reusing the existing rsync plugin:

Just tested: works fine for me on my win7 host, yet I still need to work around mitchellh/vagrant#4073 by appending the "/cygpath" prefix here (which I needed to do anyway if I ever wanted to use rsync shared folders from my win7 host...)

chrisbaldauf commented 9 years ago

The problem that I have with setting the type: "smb" for synced folders is some provisioners (Puppet in particular) create synced folders dynamically. Adding the WinRM folder sync to Vagrant core feels wrong to me, since the only reason that I implemented that in the first place was really as a test to see if this was a valid approach on Windows - it turns out it is. From where I stand, I think the best balance is to let the default remain as is - WinRM folder synching with the managed-server specific sync_folders action. Add a configuration option that allows you to force override synced_folders type for all managed servers and allow the user to set that to smb if they like. If that is set, we try to use SMB and error otherwise.

I do like the idea of using the built in SyncFolders action, but then we lose the WinRM upload, which seems to be fast enough and pretty reliable for small numbers of files / folders. Do you think a WinRM synced_folder type would have general value to Windows users? Do you think they'd choose it over SMB? Do you have any idea on the turnaround time for submitting features to Vagrant core?

tknerr commented 9 years ago

Hey @chrisbaldauf, I didn't meant to contribute the winrm synced folder to vagrant core. I rather meant to construct it ike the other synced folder plugin implementations, i.e.:

I see the problem with plugins creating synced folders dynamically. And I believe all these plugins (same for Chef I guess) rely on Vagrant to decide which of the available synced folder implementations is the best (if we are using Vagrant::Action::Builtin::SyncedFolders at least).

One way to fix it would be to register the "winrm" synced folder implementation with a priority of 6:

Putting the winrm plugin inbetween would mean it would try smb.usable? first, and if not winrm.usable?. That would pretty much reflect what we want (use smb if usable, otherwise fall back to winrm before trying rsync) as a generalized default behaviour for everyone who uses Vagrant::Action::Builtin::SyncedFolders. What do you think?

Btw: I didn't mean to contribute the winrm synced folder to vagrant core, but instead just use the synced folder plugin API as (I believe is) intended. It could still live here in this plugin, but since it's a generally useful addition, it would also justify for a dedicated "vagrant-winrm-syncedfolder" plugin.

While my current feeling says this is the right way to do it, I would still be okay with keeping our own SyncFolders action for now and adding a config option for forcing smb, so that we can quickly release something that fulfills your need without doing the extra effort now. I would create a separate issue for this refactoring then.

chrisbaldauf commented 9 years ago

Ah, I see - not putting WinRM synced folders into the core makes it a much easier sell. I think your proposal is the right way to go and I'll work on pulling something together. I had noticed that there was room to squeeze the WinRM with a priority of 6 in there, but I incorrectly figured it should go into Vagrant core.

I think the biggest blocker at this point is the hanging Powershell version check when run on Powershell 2.0. I'm able to reproduce that regularly and I believe we need an answer to that before any of this works, since if you're running PowerShell 2.0, it won't give you an error message about upgrading, it will just hang forever.

tknerr commented 9 years ago

Ah right, the powershell 2.0 issue is indeed annoying. I usually don't run into it because I'm never running inside a privileged shell usually...

Would be much better if vagrant would just elevate for that specific purpose rather than expecting you to run from an elevated shell up front. Am 20.05.2015 14:57 schrieb "Chris Baldauf" notifications@github.com:

Ah, I see - not putting WinRM synced folders into the core makes it a much easier sell. I think your proposal is the right way to go and I'll work on pulling something together. I had noticed that there was room to squeeze the WinRM with a priority of 6 in there, but I incorrectly figured it should go into Vagrant core.

I think the biggest blocker at this point is the hanging Powershell version check when run on Powershell 2.0. I'm able to reproduce that regularly and I believe we need an answer to that before any of this works, since if you're running PowerShell 2.0, it won't give you an error message about upgrading, it will just hang forever.

— Reply to this email directly or view it on GitHub https://github.com/tknerr/vagrant-managed-servers/pull/47#issuecomment-103875242 .

chrisbaldauf commented 9 years ago

Just pushed something that I believe should work (save the powershell version check hanging bug). You can test that the new WinRM synced folder plugin works by patching the SMB usable to return false immediately here.

My intention is to move the winrm folder sync to another repo and publish it as its own plugin, just wanted to check that it worked integrated first. There is still more work to do in cleaning up SMB folders, but it certainly feels cleaner and as if we're making progress.

I left the non-winrm implementation as is, since it seems like there is real risk that the behavior could differ between builtin Rsync synced folders and the VMS implementation. If you feel that this is something that is required, let me know.

tknerr commented 9 years ago

The WinRM synced folder plugin looks good to me, and works as advertised:

:+1: for publishing it as a separate plugin, so that one can benefit from it independently of the VMS plugin

As for the VMS specific rsync implementation: yes, let's keep it in for now until we have a clean workaround for mitchellh/vagrant#4073 (which the VMS implementation does not suffer from). My idea was to publish this via a "vagrant-rsync-on-windows-monkey-patch" plugin (probably need to find a better name ;-)) first

tknerr commented 9 years ago

To summarize

When provisioning managed Windows servers (more precisely: with config.vm.communicator == :winrm):

There are still open bugs with SMB, but these are actually vagrant core smb plugin bugs:

I'm fine with releasing the current state, it's not perfect but definitely an improvement.

Remaining question about the externalizing the winrm plugin: once we have this as a separate plugin, can we depend on it, so that it always gets installed along with VMS if not present? Are inter-plugin dependencies in Vagrant a good idea or bad idea?

chrisbaldauf commented 9 years ago

I've made solid progress in creating the vagrant-winrm-syncedfolders plugin and should have a candidate release today. I agree with your summary above and was just about to ask the question to make sure that you were ok with making the syncedfolders plugin a dependency of vagrant-managed-servers. I found at least one example of inter-vagrant-plugin dependencies, so I believe that it should work without issue, since vagrant plugins are just gems. I think adding a dependency is the right way to go and I'll test it out to make sure it works.

chrisbaldauf commented 9 years ago

I've pushed something that I think lines up with your summary above. Please review and let me know if there is anything else that you think I can do. Thanks so much for all of your help and feedback! One user that is using this for an internal project said that (the patched SMB support that I gave him) took the provisioning time on Windows from 20 minutes to 90 seconds.

:+1:

tknerr commented 9 years ago

That is awesome :+1: :+1: :+1:. 20 mins to 90 secs deserves a triple-thumbs-up at mininum! He should definitely buy you some :beers: for that :-)

Thanks for doing all the work and making vagrant a happier place for windows users!

I will be giving it a quick test again and probably push out a new VMS release tomorrow.

chrisbaldauf commented 9 years ago

Thanks. Before that, I'll add some more info to the readme on the Windows folder sync.

On Thu, May 21, 2015 at 5:43 PM, Torben Knerr notifications@github.com wrote:

That is awesome [image: :+1:] [image: :+1:] [image: :+1:]. 20 mins to 90 secs deserves a triple-thumbs-up at mininum! He should definitely buy you some [image: :beers:] for that :-)

Thanks for doing all the work and making vagrant a happier place for windows users!

I will be giving it a quick test again and probably push out a new VMS release tomorrow.

— Reply to this email directly or view it on GitHub https://github.com/tknerr/vagrant-managed-servers/pull/47#issuecomment-104429921 .

tknerr commented 9 years ago

perfect :+1:, will hold on for that

chrisbaldauf commented 9 years ago

Readme updated.