-
Notifications
You must be signed in to change notification settings - Fork 91
Add batch file clones of the unix shell scripts #58
Add batch file clones of the unix shell scripts #58
Conversation
@mavogel I made some more minor changes to the scripts and most of the tests pass on Windows now. The remaining errors look like this:
I'm not sure why all the other tests successfully contact Docker, but some of them fail complaining that |
@AndreLouisCaron thx for the update.
|
No offense if I still try and fix this, I'm learning and enjoying the experience :-)
I reset my environment by running the following commands:
Then, Finally, I ran Note that after the tests finish, I see some dangling container:
I'm guessing that some error handling path does not clean up correctly when tests fail. I'll keep you posted if I find any relevant info about the issue with the named pipe. |
I spent some time instrumenting the system calls in
Sorry if the messages are in French :-) It seems like we're exhausting the named pipe server and then our attempt to wait until a pipe is available fails, saying the path is invalid. I'll keep you posted if I find anything else. |
I wrote a stress test for the Win32 named pipes API and managed to reproduce the Long story short: there is a race condition in the Win32 named pipe API where the Anyways, I'm pretty sure the client needs to handle the While looking at the traces, I noticed that the Docker client seems to open a new named pipe connection for each request (presumably because keep alive is dangerous when the server can't handle loads of concurrent connections). I guess that this is causing the test suite to fail so reliably -- at least on my machine. Seems like the code that needs to be fixed is directly in Microsoft/go-winio, which is used by |
call:print "### removed auth and certs ###" | ||
for %%r in ("container" "volume") do ( | ||
call docker %%r ls -f "name=tftest-" -q | ||
for /F %%i in ('docker %%r ls -f "name=tf-test" -q') do ( |
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.
@mavogel While running this script repeatedly on my machine, I noticed two things:
- A container named
tf-test
was left around after each run. This container's named does not match thetftest-
pattern that is used to find dangling containers to delete. - The
docker rm
commands don't use-v
. This causes some volumes to stick around and it sometimes causes an issue where Docker will refuse to create new mount points because "/host_mnt/c
is already mounted".
I fixed them in these batch scripts so I can now repeat the test runs, but since they are basically a transliteration of the .sh
scripts, it's likely the cleanup bugs exist there too.
The issue with the named pipe connection errors were already reported in microsoft/go-winio#67. I submitted a tentative fix for this in microsoft/go-winio#75. When I include this patch in |
@AndreLouisCaron I found that Windows has ready VMs :) so I can help testing now as well. |
replaced by #90 |
After discussion with @mavogel in issue #52, I prepared some batch files to run the acceptance tests on Windows.
I manage to run the test suite using these batch files. However, the tests fail because they reference Unix sockets, so I guess there is more work to do :-)
Edit:
I'm not super familiar with AppVeyor, but maybe I can give that a try tomorrow.I submitted PR #59 with a AppVeyor support. Assuming that gets merged, I could have that run these scripts.Note:
docker
,go
andopenssl
must be in your%PATH%
for these scripts to work. I run the tests using cmder, which seems to provideopenssl
out of the box.Note: the first time you mount a volume inside a container, Docker for Windows will prompt you for a request to allow the mount. I'm not sure what this will do when you try to run it under AppVeyor.