-
Notifications
You must be signed in to change notification settings - Fork 5
Add support for parallel testing #1
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
Conversation
…e methods and add a helper to get a services IPAdddress
Looking at Travis the build was initially failing because go vet got removed (it included by default now). Now it is failing because
which I do not have a clue about. |
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.
Thanks for the PR!
-
Feel free to try bumping the Go version to 1.8 in Travis, I don't care about supporting older versions it's just that I haven't touched this in a while.
-
Would you mind adding a test that's similar to
TestMustConnectWithDefaults
but starts two parallel instances?
|
||
// StartParallel starts a Docker Compose configuration and is suitable for concurrent usage. | ||
// Note that the services should not bind to localhost ports. | ||
func StartParallel(dockerComposeYML string, forcePull bool) (*Compose, error) { |
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.
Would you mind renaming this to StartProject and letting clients pass in the project name? I'm thinking it would be more versatile if it let clients choose their strategy for generating project names. Also I'd preserve the rmFirst parameter.
func StartProject(dockerComposeYML string, forcePull, rmFirst bool, projectName string) (*Compose, error)
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.
NP - added
} | ||
|
||
// GetPublicIPAddressForService returns the IPAddress of a service | ||
func (c *Compose) GetPublicIPAddressForService(serviceName string) (bool, string) { |
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.
Is this actually needed? What I've traditionally been doing is assuming that the IP address for the service is the one returned by InferDockerHost(), so you could for example find the IP:port of a container by doing this:
mockServerURL := fmt.Sprintf("%v:%v", MustInferDockerHost(), compose.Containers["container-name"].MustGetFirstPublicPort(1080, "tcp"))
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.
I'll tell you what I was thinking -
InferDockerHost gives you the IPAddress for the machine running the Docker daemon - so for me on Ubuntu it is localhost but if I was running a Mac and using the Docker toolbox (or Docker Machine or whatever it is called now) it would be the IPAddress of the virtual machine running the Docker daemon.
GetPublicIPAddressForService should give you the IPAddress the docker daemon has allocated for the container for the service defined in the docker-compose file.
InferDockerHost is really useful if your docker-compose file maps container ports to "where ever the docker daemon is running". When you are running simultaneous copies of the docker-compose file you really need to know the IPAddress of the container to call into from your test and that was why I added the method.
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.
Sorry I'm still not sure I'm clear on this one - my understanding is that if you want to access a container from a test (i.e. from outside the virtual Docker network) your only option is to ask Docker to bind an exposed port from a container to a port on the host network, because the internal IP address of the container won't be reachable from the host otherwise.
You can do that using the "ports" section in the compose file. You won't be able to specify the port in the "hostport:containerport" format as the second container will fail to start because the port on the host is already taken. If you only specify the "containerport" port, Docker will automatically allocate a free port on the host network, and you can find out which one it is using MustGetFirstPublicPort
.
Maybe I'm missing something though - is there a way to route from the host network to an IP address in the virtual Docker network?
Note that if you are instead trying to connect to a container from another one, you can just specify the dependency in the "links" section and use the container name as hostname, as the virtual Docker network has a DNS server which will resolve it to the internal IP address of the container.
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.
I've added a test - TestParallelMustConnectWithDefaults which shows the GetPublicIPAddressForService method being used to address the containers.
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.
So for example on my machine, if I do:
$ docker run -d jamesdbloom/mockserver
$ docker ps
I get
279eff516fcb jamesdbloom/mockserver "/opt/mockserver/run_" About a minute ago Up About a minute 1080/tcp, 1090/tcp festive_hopper
Then
$ docker inspect festive_hopper
I get
...
"NetworkSettings": {
"Bridge": "",
"SandboxID": "200a8403eeae9d7e40c68a7db142cf5a24a103c24e7ad5690bb64ca79b5cd271",
"HairpinMode": false,
"LinkLocalIPv6Address": "",
"LinkLocalIPv6PrefixLen": 0,
"Ports": {
"1080/tcp": null,
"1090/tcp": null
},
"SandboxKey": "/var/run/docker/netns/200a8403eeae",
"SecondaryIPAddresses": null,
"SecondaryIPv6Addresses": null,
"EndpointID": "b26b58b68f82f0bbee026b4673359260fab8912f113050b4c74d70de9c784873",
"Gateway": "172.17.0.1",
"GlobalIPv6Address": "",
"GlobalIPv6PrefixLen": 0,
"IPAddress": "172.17.0.4",
"IPPrefixLen": 16,
"IPv6Gateway": "",
"MacAddress": "02:42:ac:11:00:04",
"Networks": {
Then
$ curl -v http://172.17.0.4:1080
It times out. But:
$ docker run -d -p 1080 jamesdbloom/mockserver
$ docker ps
Gives me
d2f56406d942 jamesdbloom/mockserver "/opt/mockserver/run_" 3 seconds ago Up 2 seconds 1090/tcp, 0.0.0.0:32769->1080/tcp vigilant_torvalds
Which tells me that I can connect to this container using port 32769 on the Docker host. In my case, since I'm using boot2docker, it would be:
$ curl -v http://192.168.99.100:32769
Which works! In your case it'd be "localhost:32769". If I did:
$ docker run -d -p 1080:1080 jamesdbloom/mockserver
$ docker run -d -p 1080:1080 jamesdbloom/mockserver
The first one would succeed and I would be able to connect using curl -v http://192.168.99.100:1080
, the second one would fail with this error message:
docker: Error response from daemon: driver failed programming external connectivity on endpoint relaxed_saha (fd8eeb6e8c75baeda60698af4205595efb7765565568080b81464236a852315f): Bind for 0.0.0.0:1080 failed: port is already allocated.
If I did:
$ docker run -d -p 1080 jamesdbloom/mockserver
$ docker run -d -p 1080 jamesdbloom/mockserver
$ docker ps
Then I'd get:
483f78f46127 jamesdbloom/mockserver "/opt/mockserver/run_" 1 seconds ago Up 1 second 1090/tcp, 0.0.0.0:32771->1080/tcp musing_shaw
78db85c9228a jamesdbloom/mockserver "/opt/mockserver/run_" 40 seconds ago Up 40 seconds 1090/tcp, 0.0.0.0:32770->1080/tcp trusting_nobel
Which means I'd still be able to reach both containers using different ports.
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.
Ahh I see what you mean now by just specifying a "containerport" port.
I think that makes sense to me - I've always just used the IPAddress (probably too much as I've spent a lot of time with K8s)
I'll have a little think...
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.
Okay,
Removing the GetPublicIPAddressForService and using the InferDockerHost method has one issue and that is getting the access to the correct container by name.
I hadn't noticed before that in the tests and examples you were using the "container_name" attribute to fix the name to a known value. Understandably Docker just barfs if you try to run multiple instances with the same name.
I propose a fix where I ensure that the container has the same name as the service in the returned collection of containers. This means it is possible to reliably get to the container/service you want. It is however I fear a breaking change.
To illustrate -
test_mockserver:
image: jamesdbloom/mockserver
ports:
- "10000:1080"
- "1090"
test_postgres:
image: postgres
ports:
- "5432"
would return two containers in the collection called 'test_mockserver' and 'test_postgres'. I can pick out the originally specified name from the labels written by docker-compose. In both cases the actual name of the docker container would just be the usual unreliable junk.
Do you have any thoughts?
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.
Sounds good, would you mind coding that up?
Hey @ibrt I think I've addressed all of the comments - can you check if you have any other concerns? Thanks for your time, Mark |
Hey,
For a new project I wanted to add an ability to run tests against a docker-compose file in parallel.
To that aim I've added to the public api
// starts a compose with a random project name
func StartParallel(dockerComposeYML string, forcePull bool) (*Compose, error)
func MustStartParallel(dockerComposeYML string, forcePull bool) (*Compose, error)
// for a Compose gets the public IPAddress for a service
func(c *Compose) GetPublicIPAddressForService(serviceName string) (bool,string)
func(c *Compose) MustGetPublicIPAddressForService(serviceName string) string
Usage -
The existing public api methods all remain unchanged both in contract. Calling Start and/or MustStart hard code the docker-compose project to the existing name.
The only 'here be dragons' point worth mentioning is that if your docker-compose file binds to ports on the localhost, starting in parallel fails.
It would be great to get some feedback?
Thanks,
Mark