Consolidation of winrm-transport into winrm-fs

[mwrock]

This is a work in progress of a merging of the winrm-transport gem into winrm-fs. See test-kitchen/winrm-transport#16

Both gems set out to solve the same problem and both have strengths and weaknesses. This PR aims to remove the maintenance of 2 codebases and merge the best of both.

[mwrock]

As things stand now, this code is passing all functional winrm tests.

Things that need to happen here:

1. Test against Test-Kitchen and Vagrant to ensure functional parity
2. Benchmark - Make sure this does not introduce perf regressions and provide some data as to how this compares with the current master.
3. Possibly bring back the zip file factories. For simplicity, I stripped the shell implementation. The benchmarking (mentioned above) should prove if its worth adding back.
4. Move the CommandExecutor and ShellCloser into the WinRM gem and possibly refactor how ShellCloser is interacted with.
5. Rebase on Master

Once the above is done and this is released, Test-Kitchen and Train should be adjusted to use this gem and winrm-transport can be depricated.

[sneal]

@mwrock This is exciting to see finally happen, nice job!

• Should we use the logging library instead of stdout for debug output?
• RuboCop is very angry about the interpolated strings. Fix or disable that?

[mwrock]

@sneal I think its using the logging library now and I don't see a reason to change that. Maybe you are seeing the commented lines where I was directing the logging to stdout which I'll definitely clean out and maybe I missed some stray puts which I'll also check. All in all, things are not exacly pristine right now :)

I personally prefer the default rubocop interpolation rules and was gonna cleanup style before release.

[mwrock]

I have validated that both Test-Kitchen (with some slight rewiring) and Vagrant work when pointed to this branch. However, one finding this turned up is that winrm-fs copies as src/* -> dest while winrm-transport is src -> dest. (actually I think I remember seeing this in November when I did the initial merge and forgot about it).

Ultimately I think winrm-transport's strategy is the correct one since it is the same strategy as xcopy, copy-item, cp, etc. , but that's a breaking change for winrm-fs. I think the initial release should be non-breaking and there are only 2 lines in test-kitchen needed to be changed to compensate.

I also did some benchmarking. I benchmarked this branch against master and winrm-transport. I used the test-kitchen repo as a directory to copy. Its total size (including .git folder) is 6.6MB and 1578 files. Over several iterations, the times were nearly identical among the three tests (~25s). This branch was about a half second slower before I made a change to avoid an initial round trip to create the winrm-upload directory.

A couple interesting findings:

• I initially used a much larger directory to benchmark (51.6MB) and master consistently crashed on exceeding number of commands. Thats on a 2012R2 node so it must be exceeding the higher threshold supported in PS v3+. This branch succeeded.
• winrm-transport's dirty check always returns True on a Windows host because of how windows inserts timestamps into the zip entries.

I noticed that master does not actually use the shell zip factory so I'm gonna keep that out.

So that completes items 1 - 3 listed above.

[sneal]

I think I'd rather see us make a breaking change to the copy behavior we want. The sooner we do it the easier it'll be.

We can always add the shell zip factory in later if we need it, but zipping is still a lot faster than the file transfer.

[mwrock]

Yeah I was going back and forth on breaking earlier. I think you are right. It will likely mean chaning vagrant to adapt but I'm guessing that will be straight forward.

[mwrock]

I have rebased this PR with current master. Here is an updated todo list:

1. Add support for yielding progress - this came up in the rebase adding the test we added that adequately tests this. Its failing here now.
2. Make rubocop happy
3. Port over unit tests for file_transporter and tmp_zip from winrm-transport making them rspec (currently mini test) and rubocop compliant
4. Sanity check code
5. Release (pending release of winrm 1.5)
6. Retrofit vagrant, test-kitchen and Train

[mwrock]

1 - 3 above are complete.

[mwrock]

I've removed WIP from the title of this pr and its ready for comment. I'll squash it before it gets merged. I've tested against test-kitchen and it works with some tweaks that I will submit after this is merged. I am curious @sneal if you could suggest a good test case for vagrant. I have lightly pursued its usage there and it seems like vagrant only copies files, not directories or am I wrong.I'm hoping that the changes here will be non breaking for vagrant.

[mwrock]

I took another pass through the vagrant code that touches communicator.upload. From what I can tell the only thing there that could have potentially broken compatibility is the file provisioner and only if the source is a directory and the destination directory already exists. The current winrm-fs would simply copy the containing files and directories to the destination directory while this branch would create the source directory and its contents as a new directory under the destination directory.

The good thing is that now the winrm and ssh directories now share the same behavior.

It looks like all other provisioners (at least those that ship inside of vagrant) are completely unafected since they either only copy files or they delete the target directory before the copy.

I think this is an acceptable change as long as vagrant makes clear mention of this in its release doc which I'll make sure to submit promptly after this is released.

@sneal let me know if you are comfortable with this PR (no rush) and I'll release. Thanks!

[sneal]

WRT Vagrant, it should be fine as long as the file provisioner works. Vagrant typically shares files out from the host rather than uploading them.

I do wonder if the changes will affect the https://github.com/Cimpress-MCP/vagrant-winrm-syncedfolders plugin. I've used that for spinning up AWS Windows instances from Vagrant before.

All that being said, I think we can and bump the Vagrant dependency on winrm-fs. Once that is done perhaps we try out the synced folder plugin and fix any issues we may find.

[mwrock]

cool. I have overlayed latest winrm/winrm-fs into a vagrant omnibus and the file provisioner is working.I'll give the syncedfolders a whirl before merging. Thanks!