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

Support WinRM As A Communicator #451

Closed
joefitzgerald opened this issue Sep 23, 2013 · 51 comments
Closed

Support WinRM As A Communicator #451

joefitzgerald opened this issue Sep 23, 2013 · 51 comments

Comments

@joefitzgerald
Copy link

@joefitzgerald joefitzgerald commented Sep 23, 2013

Currently when building a Packer template for Windows (e.g. https://github.com/joefitzgerald/packer-windows/), we need to install Cygwin to allow Packer's Provisioners to work and to allow the shutdown command to be run. Ideally, Packer could use WinRM to perform these actions, allowing us to avoid bundling Cygwin with our Windows boxes.

See here for related conversation.

@mitchellh You and I discussed using a Ruby command line utility to provide this functionality, because Go has no WinRM / WS-Management / WS-* / SOAP libraries to build upon.

@jeffjensen
Copy link

@jeffjensen jeffjensen commented Oct 5, 2013

If the solution is a Ruby script, then isn't that just substituting one big dep (Ruby environment) for another (Cygwin environment) [or perhaps I misunderstand]? Is there a solution that won't require these big deps?

@mitchellh
Copy link
Member

@mitchellh mitchellh commented Oct 5, 2013

@jeffjensen The idea of substituting with a Ruby script is on the host side, not the VM. And it is a shim until Go has native WinRM support, but there are a lot of blockers in the way of that.

@dylanmei
Copy link

@dylanmei dylanmei commented Nov 25, 2013

I believe that a communicator plugin that implements packer.Communicator, without implementing all of WS-Management, is very possible in Go.

However, Packer is currently hard-wired to use the SSH Communicator plugin. There is no plugin.ServeCommunicator, although its easy to add since there is an rpc.RegisterCommunicator.

@joefitzgerald
Copy link
Author

@joefitzgerald joefitzgerald commented Nov 25, 2013

@mitchellh so @dylanmei and I have discussed this at length and are interested in collaborating to get a winrm communicator for packer written in go. Dylan has even roughed this out here: https://github.com/dylanmei/packer-communicator-winrm

I'm interested in establishing a few things before we invest a ton of effort:

  • You're open to making the default communicator more pluggable / configurable (per Dylan's comments' above) so that it is not hard coded as the communicator.
  • You're open to us submitting a PR to get this new communicator included in the packer repo, once it's ready for use (are there any specific criteria you need for that)?
  • You're ok with the direction we're headed in (implementing only those pieces needed to make packer work with winrm).

Thoughts?

@mitchellh
Copy link
Member

@mitchellh mitchellh commented Nov 25, 2013

@joefitzgerald yes yes yes.

@sneal
Copy link
Contributor

@sneal sneal commented Dec 11, 2013

I may start working on this, however there are some issues with WinRM's security model. For instance some installers will not work over WinRM because of the security model. With SSH they actually work just fine.

Vagrant-windows gets around this by monkey patching the builtin provisioners creating a scheduled task on the guest to kick off the provisioner. These monkey patches are brittle and probably not even an option with Packer. I've been threatening to create Windows specific shell, chef, and puppet provisioners in vagrant-windows - this is one possible approach for Packer.

The other option is leave the provisioners pretty much as-is and push the complication down to the script/cookbook authors. I still think the provisioners need to be partially aware of the guest OS. Currently you need to overwrite most of the provisioner templates and turn off install and sudo; for example a working Windows chef-solo config block:

      "type": "chef-solo",
      "cookbook_paths": ["./cookbooks"],
      "execute_command": "c:/opscode/chef/bin/chef-solo --no-color -c {{.ConfigPath}} -j {{.JsonPath}} -l debug",
      "staging_directory": "c:/windows/temp",
      "prevent_sudo": true,
      "skip_install": true,
      "run_list": ["dotnetframework", "sqlce"]

One could make an argument either way in regards to the scheduled task support. On one hand its simpler to write your scripts/cookbooks but on the other hand they might not work when you try to use knife to run them. It really depends on how you intend on using it.

I really just want the tooling to just work... like it does on Linux.

@joefitzgerald
Copy link
Author

@joefitzgerald joefitzgerald commented Dec 11, 2013

Perhaps a good first step is to get things working as cleanly as possible - excluding the category of installers that have issues with the WinRM security model. At that point most of the core problem is solved, and it will give us the luxury of time to discuss solutions to the security issue.

Agreed on having platform specific defaults so that people fall into the pit of success rather than the pit of failure.

@masterzen
Copy link

@masterzen masterzen commented Jan 20, 2014

Perhaps it's too late, but I implemented a WinRM client library in Go, available here: https://github.com/masterzen/winrm

It has a comprehensive test suite and is based on code I wrote in Java, and I'm using it in production since about 3 weeks or so with great success :)
It's API models the Go ssh library, so it might be easy to integrate into packer.

Feel free to use it for this issue.

@joefitzgerald
Copy link
Author

@joefitzgerald joefitzgerald commented Jan 20, 2014

@masterzen I don't want to speak for @dylanmei but it seems like you've knocked over the majority of the task. All that's remaining is to implement the communicator, and abstract any SSH-coupled elements of packer. I'll fork the packer repo and try to put together a pull request. Would you be open to collaborating on that?

@masterzen
Copy link

@masterzen masterzen commented Jan 20, 2014

@joefitzgerald of course I'm open to collaborate on this work (one of the reason I wrote the lib beside learning Go was to try to integrate it into packer). If you find anything that doesn't work or if the API doesn't suit packer's needs, let me know.

Still there's one piece that is not covered by WinRM: file uploading.
One solution is to use use SMB over TCP which is quite effective, but I'm not aware of any Go port of this (and it's a different matter than writing the WinRM client. :) We could still resort to use the packer host mount command for this, but that feels a bit clumsy...

I'm open to suggestions to tackle this specific issue.

@dylanmei
Copy link

@dylanmei dylanmei commented Jan 21, 2014

@masterzen Very cool! It will be fun to play with this. As for my work on a communicator, I did not attempt to implement all of WinRM and so therefore did not have to pick up the Gokogiri dependency. I'm pretty much also at the point where I can run commands all day long but I have to find a way to handle file uploads. After much research, I was leaning towards the vagrant-windows method as well ;)

I've lost some momentum over the holidays and with the day job. But do have a look: https://github.com/dylanmei/packer-communicator-winrm

@joefitzgerald
Copy link
Author

@joefitzgerald joefitzgerald commented Jan 21, 2014

@dylanmei @masterzen Gokogiri depends on libxml; does this introduce potential compatibility issues for Packer when supporting running Packer on Windows?

@dylanmei
Copy link

@dylanmei dylanmei commented Jan 21, 2014

I haven't tried tbh. This thread shows it can be done but it also scared me away: moovweb/gokogiri#49

@masterzen
Copy link

@masterzen masterzen commented Jan 21, 2014

@joefitzgerald I admit I didn't think about running packer on windows, as my shop is pretty much linux only (besides those pesky windows images I'm trying to build).
One solution to this problem is to ship the libxm2 dll with the windows version (see http://www.zlatkovic.com/libxml.en.html for a build), or as a dependency when building.

It's certainly possible to rewrite my winrm client implementation without gokogiri, but the reason I chose it, was that the other xpath alternative weren't complete for my needs. I might revisit this if there's better xpath handling nowadays. (it's too bad that Go xml support wasn't build on libxml2 :)

The other solution is to build on top of @dylanmei work if it's ready for showtime, because at first glance it has no native dependencies.

For the shortest time to market, I'd suggest we go the gokogiri/native deps route first, then when @dylanmei solution gets ready (and converge to the ssh API if it doesn't) then we can switch the dependency in a breeze. Does that sound like a plan?

@joefitzgerald
Copy link
Author

@joefitzgerald joefitzgerald commented Jan 21, 2014

@masterzen Do you think your dependence on Gokogiri could be substituted with encoding/xml + xmlpath?

I'm going to go out on a limb and say that anything requiring the distribution of .dlls for the Windows release of Packer probably won't fly. It complicates the build process and the ability of Windows folks to contribute to Packer. Not that it matters to me (I'm a Mac user) but given the primary use case of this is WinRM – and the primary users are more likely to run Packer on Windows than other use cases – I think we should probably go out of our way to rely on native go dependencies rather than C wrappers.

@masterzen
Copy link

@masterzen masterzen commented Jan 21, 2014

@joefitzgerald when I looked at encoding/xml back when I started working on the client, I think it wasn't easy to model the needed subset of SOAP into the encoding/xml model. That's why I went with the gokogiri solution.
Now that I have more experience in Go, I might revisit this to see if encoding/xml and xpath can support what we need, otherwise, then too bad :)

@masterzen
Copy link

@masterzen masterzen commented Feb 2, 2014

@joefitzgerald I've read my code again, and I think I can remove the dependency on Gokogiri, by writing a small highly targetted xml writer and the xpath lib. I'll keep this issue posted when the code will be ready (hope to have time next week-end).

@dylanmei
Copy link

@dylanmei dylanmei commented Feb 9, 2014

@joefitzgerald The Go version of the WinRb/vagrant-windows file-upload technique: https://github.com/dylanmei/packer-communicator-winrm/blob/master/upload.go. I feel ready to get packer to accept an alternate communicator.

@masterzen I would hate to duplicate effort. If you're into making a sweet, no-deps winrm package then I'll be thrilled to use it. If you're looking to craft a packer winrm communicator, let's pool our efforts!

@masterzen
Copy link

@masterzen masterzen commented Feb 9, 2014

@dylanmei I would too hate any duplicate effort.
I started writing a smallish xml library (mostly a kind of dom system) to replace the use of Gokogiri in my winrm project. For the moment it's not very far as I struggle to find a bit of spare time to work on it. I hope to be able to hack on it this week and next week-end. I'll post my progress here as soon as I can :)

@masterzen
Copy link

@masterzen masterzen commented Feb 16, 2014

I made some good progress, but I'm not yet ready to release my winrm library that doesn't use Gokogiri. The main problem for the moment is that xmlpath doesn't support xml namespaces. I'm currently contemplating contributing support for namespaces or using a different way to grok the server responses...
Hopefully I'll be able to solve all this issues by the next week-end :)

@masterzen
Copy link

@masterzen masterzen commented Feb 18, 2014

I was able to modify xmlpath to support namespaces last night. I need now to push this patch upstream and modify winrm to use this xmlpath library (which should be relatively easy).
I'm on track for a non-Gokogiri winrm release by the week-end :)

@joefitzgerald
Copy link
Author

@joefitzgerald joefitzgerald commented Feb 18, 2014

@masterzen: great!! I have done quite a bit of work getting to the point where packer can use any communicator other than SSH. With luck, our work should converge at the same time.

@masterzen
Copy link

@masterzen masterzen commented Feb 19, 2014

@joefitzgerald @dylanmei FYI I just pushed the latest version of winrm which completely removes Gokogiri. All tests are passing and my limited manual testing was successful.

@joefitzgerald
Copy link
Author

@joefitzgerald joefitzgerald commented Feb 20, 2014

@masterzen I'll try to spend some time on this over the next few days to see if I can get the Packer communicator hooked up to it. Thanks so much for making these changes!

@dylanmei
Copy link

@dylanmei dylanmei commented Feb 20, 2014

@masterzen This is real exciting. Liking the test coverage. Pragmatic xml in go is a thing. I look forward to firing this up real soon to see how it handles uploads.

@ringods
Copy link

@ringods ringods commented Feb 25, 2014

@masterzen @joefitzgerald @dylanmei Anything to play with? I'm at the point where I have to create a few Windows base boxes for Vagrant, and automating it with Packer and WinRM would be awesome!

@docwhat
Copy link

@docwhat docwhat commented May 12, 2014

Any news, now that Vagrant 1.6 is out and supporting WinRM out-of-the-box?

@StuartWhelan
Copy link

@StuartWhelan StuartWhelan commented Oct 15, 2014

Any more news on this? Very keen!

@sneal
Copy link
Contributor

@sneal sneal commented Oct 17, 2014

We've been using our various Packer patches that support Windows for over a month now with great success. I've described in a couple of blog posts how we use Packer.

The last post describes how to use our Packer branch (which pulls in all our PRs) to build out a new Windows 2012 R2 Server Vagrant base box using WinRM.

@johnjelinek
Copy link

@johnjelinek johnjelinek commented Oct 21, 2014

@sneal I'm unable to build packer running from your branch. Could you address the merge conflicts and/or publish a bin for your build?

@mefellows
Copy link
Contributor

@mefellows mefellows commented Nov 19, 2014

I'm keen to add support/dev time on this, however looking at PR #1175, the above thread and a general scan of the issues, it's not immediately obvious what direction is going to be taken and therefore where time should be spent.

If I understand correctly, we currently have:

  • 2 working WinRM libraries written in Go - @masterzen's and @sneal's (which one should be used is not clear to me either)
  • A WinRM Packer Communicator written by @dylanmei
  • A forked Packer from @sneal and a PR containing a working solution for VMWare albeit the approach requires all plugins to be aware of both SSH and WinRM communicators. This is awesome, and not taking anything away from the great work that has been done, there doesn't appear to be clear direction on what to do next and if the current implementation is the path forward. There are pros and cons of the implementation and obviously potentially wide-ranging impacts from any decisions made so I understand this.

So, as I see it there are a few high-level options:

  1. Continue on the #1175 path - fork @sneal's version and continue adding support for all builders/provisioners
  2. Fork Packer and submit a PR with an alternative design/implementation
  3. Add WinRM support via 3rd party plugins (e.g. a WinRM Shell, WinRM Virtualbox etc.)

...and probably many more. I'd be happy to discuss over IRC or another forum (FYI I'm in Australia on AEST), but if there is an accepted plan I'm happy to work with that.

So what's the next step :)

@dylanmei
Copy link

@dylanmei dylanmei commented Nov 19, 2014

I'm still highly invested in this, it's probably now second on my list of top 100 crazy things I've gotten myself into. ;)

My plan has been to study Shawn's fork to see what he's done to make packer.Communicator a realistic type of Packer plugin. But I personally want to avoid slamming a lot of unstable code into Packer all at once.

If we can add communicator to the list of installable plugins then imho it's clear how to develop this thing in a separate project, with small, low-risk/releasable PRs added to packer to get the WinRM communicator viable.

I'm sure @masterzen is open to taking in some of the tweaks @sneal has added to winrm if it means we can get this communicator off the ground.

I haven't IRCd in years but I'm open to that or a Slack group.

@masterzen
Copy link

@masterzen masterzen commented Nov 19, 2014

@dylanmei, it's too bad @sneal didn't fork but copied my original work (without any mention of the original author, which I find not very friendly), retrofitting all the good changes he did will be a complex task.
I would have preferred a cleaner way of sending back PR to my original code :(

@sneal would you be open to submit your changes as PR to my original work?

@dylanmei
Copy link

@dylanmei dylanmei commented Nov 19, 2014

Shawn's work is largely a proof-of-concept. I'm sure we can all create something solid together!

@sneal
Copy link
Contributor

@sneal sneal commented Nov 19, 2014

I've been building Windows and Linux boxes off my various stitched together PRs for months now. I spoke to @mitchellh about this a few months ago, so its on their radar but I'm sure Packer and especially Windows support is not their top priority.

@masterzen It certainly wasn't my intention to detract from your work, I just needed to move faster and I had incorrectly assumed the library was abandoned. I can help get bug fixes back into your library.

@masterzen
Copy link

@masterzen masterzen commented Nov 19, 2014

@sneal I'm definitely interested in merging your contributions in, especially if that helps the project in the long term.
I don't have a lot of spare time, hence I might have been silent for some time (if that happens in the future , feel free to harass^W ping me until I merge the changes).

@mefellows
Copy link
Contributor

@mefellows mefellows commented Nov 20, 2014

I agree, it would certainly be preferable for the contributions to be bite-sized, manageable and with good test coverage.

Making the communicator pluggable or at least abstracted over the SSH and WinRM communication channels seems like the best approach to me and a logical starting point.

Looking at Shawn's code, that is not quiet yet achieved. As I understand it there is a WinRM communicator but all provisioners/builders/plugins etc. need to be aware of it, for instance the VMWare Builder now has a NewConnectStep func which is used by the builder. This pattern is repeated elsewhere.

Whilst it works, it's not ideal if we want to avoid violating the DRY principle, ideally the communication mechanism is transparent to the plugins and delt with by the internal Communicator.

As a first point of call I'll look into how we might go about turning the Communicator into a Plugin. Additionally, given I have some time if there are no objections I might start with porting some of Shawn's changes back into the go-winrm and packer-communicator-winrm repositories. I think this will help with the modularisation aspect.

BTW - I'm not fussed about IRC I just noted it on https://www.packer.io/community. Slack works for me. I'm happy to set it up but I think it should be something a Packer core contributor decides.

@myoung34
Copy link

@myoung34 myoung34 commented Nov 20, 2014

A plugin seems applicable in some cases, but IMO (and purely objectively although i really want/need this functionality), would be that this is a core addition. Plugins add functionality without modifying core base functionality. A "communicator" to me sounds extremely tied to the core. At the base of Packer is:

  1. Take base image
  2. Start it
  3. Go into it
  4. Configure it.
  5. Package it.

"Go into it" brings a whole lot of issues. Timeouts, retries, communication protocol, fallback, certificates/security. Packer at it's absolute core assumes and is 100% tied to ssh. This means that WinRM as a "plugin" hacks at this, and has to make packer forget about ssh and communicate differently. I'm all about this being a community driven plugin, 100%, but I think that getting this into core from Hashicorp/ @mitchellh as the sole deciding factor is the way to go.

That said I'm curious what @mitchellh has to say a year later about this functionality. As more cloud backends go for windows support, this is a HUGE win.

@dylanmei
Copy link

@dylanmei dylanmei commented Nov 20, 2014

There is design intent in packer to for packer.Communicator to be "pluggable". It seems difficult but realistic. As you say, there is much that takes for granted that ssh is the way that things are done.

My thinking is that we keep the unstable, churning winrm bits in the optional plugin, and then get it rolled into this repo alongside ./communicator/ssh when the time was right. This seems a similar path as Vagrant and WinRB/vagrant-windows.

@mefellows i've found my way onto #packer-tool

@sneal
Copy link
Contributor

@sneal sneal commented Nov 20, 2014

We'd all like to see Windows support in Packer core, but if this continues to stay in limbo the only realistic solution is a plugin approach that allows the community to move forward together. Smaller PRs which further decouple Packer from SSH might be more readily accepted. @rasa Do you want to weigh in on this?

We might also need to duplicate the provisioners in the plugin to properly support Windows unless those PRs I have are pulled in separately from the communicator - which they could be.

@mefellows
Copy link
Contributor

@mefellows mefellows commented Nov 20, 2014

Let's not forget that @sneal has created a working prototype so that should give us a level of confidence that it's possible. I'll jump IRC to continue discussing the details.

@rasa
Copy link
Collaborator

@rasa rasa commented Nov 21, 2014

@sneal Gosh, it's really hard to say, as I've been away from it for a while now. I had your commit at sneal@112ed70 working great, along with my https://github.com/rasa/packer/tree/winrm-fixes tweaks, but of course, your work is VMWare specific. A more generic solution would be preferable, but I'd rather have something working in master now, than wait for the most optimal solution. You try to bite off too much, and you may never swallow.

@mefellows
Copy link
Contributor

@mefellows mefellows commented Nov 23, 2014

Just a quick update: I've ported @sneal's changes to WinRM and the Packer WinRM Communicator and will submit a PR in the next day or two to the upstream repositories. Builds are green but I've noticed a few bugs that need to be fixed. Test coverage is also fairly poor so I'd like to see that increased before we tackle the pluggability of the Packer Communicator so we have a safety net when we inevitably hit requirements to change the APIs.

@mkuzmin
Copy link
Contributor

@mkuzmin mkuzmin commented Feb 17, 2015

@mefellows, could you tell us about current state of this feature?
I see you are developing https://github.com/packer-community/packer-windows-plugins
Are these plugins ready to use?
Why did you choose to fork base plugins, are you going to merge these changes back?

@mefellows
Copy link
Contributor

@mefellows mefellows commented Feb 17, 2015

Hi @mkuzmin,

The current state of the plugins is fairly stable, and I am using them daily in both Development and Production environments. There is one remaining issue that I feel is the last thing to close out before a 1.0.x release (it sounds worse than it is and I haven't yet encountered it in my use cases).

We chose to fork for a number of reasons (breaking change was required), but for the most part it was practicality as it enabled us to develop with greater pace. We would love to get these merged in at some point, but we would really need the support of the community before we attempt to do so to ensure we keep Packer itself stable etc. Feel free to reach out via the usual GitHub channels!

@mitchellh
Copy link
Member

@mitchellh mitchellh commented Jun 14, 2015

Done! Now in core.

@mitchellh mitchellh closed this Jun 14, 2015
@hashicorp hashicorp locked and limited conversation to collaborators Apr 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet