Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Add batch file clones of the unix shell scripts #58

Closed
wants to merge 3 commits into from
Closed

Add batch file clones of the unix shell scripts #58

wants to merge 3 commits into from

Conversation

AndreLouisCaron
Copy link
Contributor

@AndreLouisCaron AndreLouisCaron commented Apr 20, 2018

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 and openssl must be in your %PATH% for these scripts to work. I run the tests using cmder, which seems to provide openssl 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.

@AndreLouisCaron
Copy link
Contributor Author

@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:

--- FAIL: TestAccDockerContainer_volume (1.62s)
        testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:

                * docker_container.foo: 1 error(s) occurred:

                * docker_container.foo: Unable to create container: container already exists
        testing.go:494: Error destroying resource! WARNING: Dangling resources
                may exist. The full state and error is shown below.

                Error: Error applying: 1 error(s) occurred:

                * docker_image.foo (destroy): 1 error(s) occurred:

                * docker_image.foo: Unable to remove Docker image: Unable to list Docker images: Get http://unix.sock/images/json?: open //.//pipe//docker_engine: Le chemin d’accès spécifié n’est pas valide.

                State: docker_image.foo:
                  ID = sha256:b175e7467d666648e836f666d762be92a56938efe16c874a73bab31be5f99a3bnginx:latest
                  latest = sha256:b175e7467d666648e836f666d762be92a56938efe16c874a73bab31be5f99a3b
                  name = nginx:latest

I'm not sure why all the other tests successfully contact Docker, but some of them fail complaining that //.//pipe//docker_engine is an invalid path. I clearly set DOCKER_HOST=npipe:////.//pipe//docker_engine, so there is some transformation going on somewhere, but I'm getting a bit out of my league here.

@mavogel
Copy link
Contributor

mavogel commented Apr 20, 2018

@AndreLouisCaron thx for the update.

  • regarding the failure: check with docker ps if there are some containers running before you run the tests. Remove them an re-run the tests.
  • regarding the pipe: could you provide me a log of the failure please. This would help me. Nevertheless, I'll check next week with the windows machine if I can find the issue or the point where it breaks.

@AndreLouisCaron
Copy link
Contributor Author

Nevertheless, I'll check next week with the windows machine if I can find the issue or the point where it breaks.

No offense if I still try and fix this, I'm learning and enjoying the experience :-)

check with docker ps if there are some containers running before you run the tests. Remove them an re-run the tests.

I reset my environment by running the following commands:

$ docker container prune -f
...
$ docker image prune -f
...
$ docker network prune -f
...

Then, docker ps -a shows nothing.

Finally, I ran scripts\runAccTests.bat 2>&1 | tee runAccTests.log to capture the output. You can find the result in this gist.

Note that after the tests finish, I see some dangling container:

$ docker ps -a
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS
     NAMES
2fc10b3ae639        b175e7467d66        "nginx -g 'daemon of…"   4 minutes ago       Up 4 minutes        80/tcp
     tf-test

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.

@AndreLouisCaron
Copy link
Contributor Author

I spent some time instrumenting the system calls in Microsoft/go-winio and this came up:

=== RUN   TestAccDockerContainer_customized
dial: "//.//pipe//docker_engine"
createFile("//.//pipe//docker_engine") -> %!s(<nil>)
dial: "//.//pipe//docker_engine"
createFile("//.//pipe//docker_engine") -> %!s(<nil>)
dial: "//.//pipe//docker_engine"
dial: "//.//pipe//docker_engine"
createFile("//.//pipe//docker_engine") -> %!s(<nil>)
createFile("//.//pipe//docker_engine") -> Toutes les instances des canaux de communication sont occupées.
waitNamedPipe("//.//pipe//docker_engine", 2000) -> Le chemin d’accès spécifié n’est pas valide.
dial: "//.//pipe//docker_engine"
createFile("//.//pipe//docker_engine") -> %!s(<nil>)
dial: "//.//pipe//docker_engine"
createFile("//.//pipe//docker_engine") -> %!s(<nil>)
dial: "//.//pipe//docker_engine"
createFile("//.//pipe//docker_engine") -> %!s(<nil>)
dial: "//.//pipe//docker_engine"
createFile("//.//pipe//docker_engine") -> %!s(<nil>)
--- FAIL: TestAccDockerContainer_customized (1.04s)
	testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:
		
		* docker_image.foo: 1 error(s) occurred:
		
		* docker_image.foo: Unable to read Docker image into resource: Unable to list Docker images: Get http://unix.sock/images/json?: open //.//pipe//docker_engine: Le chemin d’accès spécifié n’est pas valide.

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.

@AndreLouisCaron
Copy link
Contributor Author

I wrote a stress test for the Win32 named pipes API and managed to reproduce the FILE_NOT_FOUND error in the call to WaitNamedPipe(). See this gist for the source code.

Long story short: there is a race condition in the Win32 named pipe API where the CreateFile() and WaitNamedPipe() calls on the client can be invoked while the server has no outstanding calls on ConnectNamedPipe(). The BSD-style sockets API solved this using a listen backlog, but there doesn't seem to be an equivalent in the Win32 named pipes API.

Anyways, I'm pretty sure the client needs to handle the FILE_NOT_FOUND error with a short (ideally randomized) pause and a retry to stay resilient to that race condition.

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 go-dockerclient. I'll report the issue there and we'll see where that takes us :-)

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 (
Copy link
Contributor Author

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:

  1. A container named tf-test was left around after each run. This container's named does not match the tftest- pattern that is used to find dangling containers to delete.
  2. 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.

@AndreLouisCaron
Copy link
Contributor Author

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 vendor/github.com/Microsoft/go-winio, the acceptance tests pass on my Windows machine.

@mavogel
Copy link
Contributor

mavogel commented Apr 23, 2018

@AndreLouisCaron I found that Windows has ready VMs :) so I can help testing now as well.

@mavogel
Copy link
Contributor

mavogel commented Sep 25, 2018

replaced by #90

@mavogel mavogel closed this Sep 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants