Add support for Windows named pipes

[samuelkarp]

Fixes #531

This series of commits adds support for communicating with Docker over a Windows named pipe.

There are a few things to note:

• I've only tested this with toy programs so far.
• All the unit/integ tests in this package are passing on both Linux and Windows.
• I haven't ported all the tests in container_unix_test.go to Windows; I started running into some issues with Accept() on a net.Listener for a named pipe and haven't spent time troubleshooting it yet. If this is a blocker to accepting the pull request I can spend more time here.
• I did not add TLS support for named pipes. I don't plan to do this since I don't expect to have a use-case for this.
• A few tests that were otherwise platform-independent made some assumptions about running on Linux that don't appear to always work on Windows; I've fixed them to ensure that they run on Linux and Windows.

All the commits here are contributed under the terms of the BSD 2-clause license.

Let me know if you need anything else!

[fsouza]

@samuelkarp thanks for you awesome work :-D

I'll try to get a Windows CI to make sure we run the tests. Do you know if we can run integration tests on AppVeyor (i.e. have a Docker daemon running in the box)?

Cheers!

[samuelkarp]

@fsouza I missed the integration target in the Makefile; I haven't ported that test to Windows yet. I haven't used AppVeyor, but the other tests don't require a running Docker daemon and should (theoretically) work.

Looking at TestIntegrationPullCreateStartLogs, this appears to assume a Linux daemon (it uses a Linux image and runs cat). Since a Windows client might be used to run either Linux or Windows containers (Linux containers through "Docker for Windows" and Windows containers through "Docker on Windows") but the daemon would be different, should the test assume that it can use a Windows container on Windows or should it do something like query docker info to find the OSType and use that as switching logic?

[fsouza]

@samuelkarp hm, I'm not entirely sure. Ideally it would test that it can communicate with named pipes, so we would need a whole new integration test. Here's what I'm going to do: I'll setup the appveyor to run make test and ensure that everything works, along with reviewing the code itself, and then I'll drop another issue for running integration tests on Windows (or simply add a note to #353). What do you think?

[samuelkarp]

@fsouza That sounds like a good plan.

Testing whether it can communicate over named pipes should be easy (the other tests already do that and we can either pass the named pipe location to NewClient or call NewClientFromEnv to get the named pipe automatically), but both "Docker for Windows" and "Docker on Windows" use named pipes so the question of what the test should be is still an open one.

I can't promise that I'll have time to get the integration test done, but I'll see if I can get something quickly today.

[zramsay]

yay! thanks for doing this @samuelkarp. very helpful 👍

@fsouza "Do you know if we can run integration tests on AppVeyor" => we're currently using docker-machine for CI on windows, but we'll definitely be investigating getting Docker for Windows up and running on AppVeyor.

[zramsay]

does the package need the name declaration? client_windows.go doesn't have it

[samuelkarp]

No, not necessary. I think goimports did it for me here and I must have manually written the import in the other file.

[fsouza]

Nice catch @zramsay, thanks!

@samuelkarp can you remove the aliasing from this line and make this just "github.com/Microsoft/go-winio".

[fsouza]

@samuelkarp changes look mostly good to me. Sorry for the long delay in getting back to you.

I have appveyor setup in a branch with this PR merged, the test suite passes, but the race detector detected some data races: https://ci.appveyor.com/project/fsouza/go-dockerclient/build/6.

Can you take a look? It looks like calling close concurrently to any write/read is not safe :-(

You can copy the appveyor.yml file to your branch to have app veyor running your changes in the PR.

[samuelkarp]

@fsouza Thank you for taking a look! I've addressed the aliasing comment and opened microsoft/go-winio#31 for the race condition in the go-winio library.

[samuelkarp]

(I've also rebased on top of the current tip of master since there were some merge conflicts that needed to be resolved.)

[fsouza]

@samuelkarp awesome, thanks! I'll just wait for travis and merge this. Before pushing to master, I'll disable -race in app veyor, and keep an eye on microsoft/go-winio#31.

Thank you very much for working on that!