1089457 - [PulseGuardian] Use docker for tests

Status: RESOLVED FIXED

(Webtools :: Pulse, defect, P2)

Description

[Mark Côté [:mcote]]

gps has done some really nice stuff with docker for external dependencies in the version-control-tools tests.  We already have a vagrant box in the mozillapulse package, so we should try to do something similar, but with docker so we can automate it even better.

Comment 1

[Ricardo Castro]

Picking this one up.

Comment 2

[Ricardo Castro]

I'm making some progress on this.

So far, I managed to create Dockerfile that will create an image with Ubuntu 14.04 and RabbitMQ. I also managed to create a daemonized container running rabbitmq-server.

Next I'll configure ports, vhost and user.

Comment 3

[Mark Côté [:mcote]]

Awesome!  Remember that you need to enable the management plugin too; see the docs in the mozillapulse package or the .travis.yml file in pulseguardian.

Comment 4

[Ricardo Castro]

(In reply to Mark Côté [:mcote] from comment #3)
> Awesome!  Remember that you need to enable the management plugin too; see
> the docs in the mozillapulse package or the .travis.yml file in
> pulseguardian.

Exactly. We'll also need to extend the README file (or add a more convenient one for the purpose) to explain and help the setup of the container.

Comment 5

[Ricardo Castro]

I still have some testing to do on this, but I think I got this working. At least, I can now run PulseGuardian and RabitMQ server is running on a docker container :)

Do you want add anything else to the container?

Comment 6

[Ricardo Castro]

Another question:

Want me to add the Dockerfile along side the other files? Or do you think it's preferable to create a tests folder like in mozillapulse and put the Dockerfile in there? Maybe even put the runtests.py file in there too?

What are your thoughts on that?

Comment 7

[Mark Côté [:mcote]]

(In reply to Ricardo Castro from comment #5)
> Do you want add anything else to the container?

I don't think so.  We really just need a working RabbitMQ (with management plugin) to develop/test PulseGuardian.

(In reply to Ricardo Castro from comment #6)
> Another question:
> 
> Want me to add the Dockerfile along side the other files? Or do you think
> it's preferable to create a tests folder like in mozillapulse and put the
> Dockerfile in there? Maybe even put the runtests.py file in there too?
> 
> What are your thoughts on that?

Hmmm good question.  Since this can be used for development as well as testing, perhaps it can be left in the root.  But seeing as how you've been working with PulseGuardian for a bit now, what's your opinion? :)

Comment 8

[Ricardo Castro]

I'm with you on this one. Since we're going to use it for development, the root seems to be a more adequate place.

Comment 9

[Ricardo Castro]

Attachment: Pul request.

Comment 10

[Ricardo Castro]

Hey Mark,

Give it a look and tell me what you think.

Comment 11

[Mark Côté [:mcote]]

Comment on attachment 8518992 [details] [review]
Pul request.

Thanks, this is a good start.

I managed to get docker up on my Mac, but I could not execute the tests even after modifying the rabbit_host variable in config.py and running runtests.py and passing the boot2docker ip to --host.  It still keeps trying to connect to localhost.  Do you have a Mac to test on?

Also as noted in the pull request comments, we should automate this even more--automatically set up the docker container, and tear it down at the end.  I admit this might be tricky, but I think it's important if we expect people to run the tests themselves.

Comment 12

[Ricardo Castro]

Hey Mark,

I'll test it on a Mac and figure out what's wrong. 

You make a good point. It might be a little bit tricky but I think it's worth it. It will make the test suite a lot cleaner easier to use.

Comment 13

[Ricardo Castro]

Starting to look it doesn't work on a Mac. This might be interesting:

> INFO:requests.packages.urllib3.connectionpool:Starting new HTTP connection (1): 192.168.59.103
> INFO:dbinit:Finished initializing database.
> INFO:requests.packages.urllib3.connectionpool:Starting new HTTP connection (1): localhost
> ERROR:root:Failed to connect to the RabbitMQ server.

Comment 14

[Ricardo Castro]

>85: self.management_api = PulseManagementAPI()

I believe this defaults to 'localhost'.

From 'managment.py':

> DEFAULT_RABBIT_HOST = 'localhost'
> def __init__(self, host=DEFAULT_RABBIT_HOST,
>              management_port=DEFAULT_RABBIT_MANAGEMENT_PORT,
>              user=DEFAULT_RABBIT_USER,
>              password=DEFAULT_RABBIT_PASSWORD):
>        self.host = host

I'll test it :)

Comment 15

[Ricardo Castro]

That was it. Just to test, changed to something like:

> self.management_api = PulseManagementAPI(host=DEFAULT_RABBIT_HOST,
>                                       user=DEFAULT_RABBIT_USER,
>                                       password=DEFAULT_RABBIT_PASSWORD)

And it worked: tests are now running. Have to wait until they complete, but looks good.

'runtests.py' just needs some small adjustments :)

Comment 16

[Ricardo Castro]

Got it setup and running in Ubuntu but having some trouble with OS X. boot2docker seems to be blocking connections and although I've forwarded ports, still not working. I'll dig deeper.

Comment 17

[Ricardo Castro]

Attachment: Pull request.

Just a few points:

1) an image and a container are being crated each time a test is run. The image is only built the first time since subsequent builds identify that the image already exist and use it to build the container. On the other a container is built and destroyed each time. I decided not to destroy the image every time because building it takes some time (download and actually building it)and because we might end up removing the Ubuntu image that could happen to be used somewhere else. Nonetheless there's a function for that effect;

2) I added an extra argument to the runtests.py to invoke the use of docker. That way we can easily use the same script in Travis CI (I've updated .travis.yml and the tests are passing)

Comment 18

[Mark Côté [:mcote]]

Comment on attachment 8522533 [details] [review]
Pull request.

This is great!  My first run failed, but only because I had a pulse container left over from earlier; the second run (and third) ran great. \o/

I have a few comments, all related to making this even smoother, but this is pretty fantastic right now. :)

Comment 19

[Mark Côté [:mcote]]

I'm going to merge your branch over to the official mozilla repo (as a branch with the same name) just to get test results there.

Comment 20

[Ricardo Castro]

Cool :) 

I was struggling with the config file myself. I was thinking it was a bit unproductive to have to set the host every time we needed to run the test with the docker container.

The only issue here, is that the developer might already have a config.py set for it's own development.  My suggestion would be to check if that file exists and if it does, rename it to something line config.dev. Then have a config.docker ready to go, rename it config.py and after the tests run revert these changes.

What do you think?

Comment 21

[Ricardo Castro]

Actually, the config file is already being imported in runtests.py. We can simply just update the rabbit_host with the docker ip.

Comment 22

[Ricardo Castro]

One other thing I just noticed: we're binding the contianer ports to the rabbitmq ports, using the default rabbitmq values . What that means is that if a developer is running it's own local instance of rabbitmq (most likely using the defaults) the container will fail because it wont be able to bind it's own rabbitmq instance to those ports.

In order to make this even more self contained, we should use a different set of ports, or we will have to force the developer to stop the local instance (in case it's running in those ports).

Comment 23

[Ricardo Castro]

In 'dbinit.py' we have:

> pulse_management = PulseManagementAPI(host=config.rabbit_host,
>                                      user=config.rabbit_user,
>                                      password=config.rabbit_password)

This makes that code tightly coupled with 'config.py' values. We either make the above option of renaming (and renaming back) config files for tests (probably not a good idea). Or we can decouple that code, by passing a PulseManagementAPI object to each function of 'dbinit.py'.

Comment 24

[Ricardo Castro]

Maybe alternatively, and to avoid big changes, we can import that 'dbinit.py' pulse_management object, assign it our PulseManagementAPI 'self.management_api' and call 'init_and_clear_db()' only after that.

Comment 25

[Ricardo Castro]

Attachment: Pull request.

The default now it's to use docker. A new argument '--use-local' was added to allow the developer to run tests against it's local setup.

Also an option for removing the image was added.

The test suite will try to connect to rabbitmq inside the container for a configurable  amount of time (20s as default) failing if it can't.

Checking if a $DOCKER_HOST env variable exists and using it as a host.

Comment 26

[Mark Côté [:mcote]]

Comment on attachment 8524891 [details] [review]
Pull request.

This is awesome!  I just made a few comments on the PR (one of which rambly as I realized I misunderstood something; I'm also used to Bugzilla where you can't edit nor delete a comment after submitting it :).  Really minor points.  It ran perfectly on my OS X machine here.

Comment 27

[Ricardo Castro]

Attachment: Pull request.

Changes:

- Fixed spelling errors
- Fixed new argument format
- Using urlparse to get DOCKER_HOST
- Removed unnecessary calls to 'init_db' and 'dbinit.init_and_clear_db'

I think we're almost there. I think those 'init_db' and 'dbinit.init_and_clear_db' I removed are not really necessary. I tested on both Ubuntu and OS X and tests ran smoothly :)

Comment 28

[Mark Côté [:mcote]]

Comment on attachment 8525517 [details] [review]
Pull request.

Great!  There was one tiny spelling mistake and an incorrect link; I fixed them, squashed your commits into one, and merged it into master.  Thanks!

Comment 29

[Mark Côté [:mcote]]

https://github.com/mozilla/pulseguardian/commit/cf8cb7a7c290f43bd4d855e85439622791f37791