Container services #49

marcosnils · 2015-07-06T06:45:22Z

This PR adds docker container services to telegraf so complex services can be tested in a local environment.

In addition I've also fixed some tests issues I found on the road while implementing this.

I've tried to add CI (circle) to the project but due to a bug in Go I was having some issues and decided to postpone it. You can read more about this in this commit: 6355228

This commit initializes the needed services which are not mocked so tests can be executed in any environment with docker. Some default modifications (i.e: connection strings) were also made to current tests to accomodate them for this setup. A docker-compose.yml file is provided with all the necessary parameters for this services to be initialized. Future services can be added easily by extending this configuration file In addition a makefile has been introduced to simplify command execution

provided services Update test users to use circleci default services

I've tried to set-up circleci but I came across a Golang issue that was preventing CPU tests to PASS. Instead of ignoring those tests I've submitted an issue to Go (golang/go#11609) hoping that this gets fixed soon.

toddboom · 2015-07-06T23:14:58Z

@marcosnils this is a great idea - thank you! i'm going to pull this down and do a little testing before merging.

do you have any specific guidance on getting this up and running, or does the make file pretty much do everything that's needed?

marcosnils · 2015-07-06T23:19:35Z

The requirements are:

Docker (I assume pretty much everyone knows about how to install this)
Docker compose (I've added the link in the README file)

The makefile takes care about the rest.

marcosnils · 2015-07-07T00:19:16Z

@toddboom sometimes when you run the make test the first time you "might" get an error depending on how much time the containers (specially the mysql) take to initialize.

This has been a known issue? in docker / docker-compose and the community still haven't reached a consensus about how to handle this situations. When this happens running make test again should be enough.

Further info can be read here:
moby/moby#7445
docker-library/mysql#81

This allows to run tests in environments where DOCKER_HOST is used. This is extremely helpful when using boot2docker to run docker

brocaar · 2015-07-07T15:04:32Z

I like the idea about using docker-compose for setting up different services (for a few of my projects I'm using it too for local dev and functional testing). Would it be an idea to add a Dockerfile for the project itself, so instead of docker-compose up before running the tests (prepare), you could run something like docker-compose run telegraf make test? This has the advantage that the Go environment will be the same for everybody running the tests with docker-compose. An other advantage is that you can link the containers without exposing their ports and run multiple docker-compose environments in parallel (when you start the second docker-compose environment, it will fail because it can't bind the ports to the host). Then as well you're invoking the Makefile through docker-compose, instead the other way around (so people preferring to run make test without using docker-compose are able to do so).

However, since many services will be added to telegraf in the future (I assume), doesn't this add the risk that the list of containers to fetch and start is going to grow and grow (and the tests will be slower and slower)?

Therefore, wouldn't it be better to implement mocks where possible? E.g. for testing databases something like https://github.com/DATA-DOG/go-sqlmock could be used.

marcosnils · 2015-07-07T16:17:25Z

@brocaar I do agree that mocking / stubbing should be used when possible. I've encouraged the use of mocks in the README file (https://github.com/influxdb/telegraf/pull/49/files#diff-04c6e90faac2675aa89e2176d2eec7d8R178) to prevent the dependency growth problem you are mentioning.

Regarding running the tests inside another container through compose, I've tried it before and IMHO I agree with you that even though it gives you better isolation and environment consistency, it takes away the flexibility of just going into the code locally and run go test inside any package you prefer.

The Makefile is just an alias to run all tests but a common use-case while coding is to run only the desired package / file tests. Also I usually find useful when working with services to connect from my local computer using a cli program to understand what might be not working.

In my experience every time I've tried to automate by doing everything in containers (even running tests) I ended up exposing ports and running tests locally in favor of flexibility.

brocaar · 2015-07-08T14:21:44Z

What I mean with my comment about the Makefile is, isn't it better to invoke the make command through docker-compose run, instead of invoking docker-compose through the make command. That way you can run it in- and outside a container and is the Makefile decoupled from docker-compose :-) Eventually you could make a docker-compose-test rule in your Makefile, just as a wrapper.

marcosnils · 2015-07-08T16:50:39Z

@brocaar your idea is only viable when all services are mocked and you don't need docker containers as a dependency to run them. Meanwhile, I don't see any benefit of decoupling docker-compose from the makefile.

evanphx · 2015-07-21T20:39:40Z

plugins/postgresql/postgresql_test.go

@@ -12,7 +13,7 @@ func TestPostgresqlGeneratesMetrics(t *testing.T) {
 	p := &Postgresql{
 		Servers: []*Server{
 			{
-				Address:   "sslmode=disable",
+				Address:   fmt.Sprintf("host=%s user=postgres sslmode=disable", testutil.GetLocalHost()),


This and the changes below it mean that the tests won't run as the current user any longer, which breaks the non-docker test setup.

@evanphx AFAIK there's no non-docker test setup in telegraf. The reason why this PR exists is because if you need to run telegraf tests as today you need to install almost all the services (which are not mocked) in your local machine and expect them to be set-up with the proper/default? (is there really a proper?) configuration.

This PR provides a docker based deterministic way to set-up all the services and run tests against them, so changing the default DB test user (BTW, I've used the default in the official postgres docker image) seems trivial as long as the tests run in a deterministic way.

For the record, I had to change the user because the postgres docker image uses it to initialize the DB configuration. If I wanted to use some other user (like the default/session one) then I had to make a custom docker image which I didn't want to do.

toddboom · 2015-07-30T23:29:39Z

amidst a bit of controversy, i'm going to merge this in. someone on our team will be taking this over officially on Monday, and i'm going to try to get this into a locally-testable state before then. thanks, @marcosnils!

Container services

marcosnils · 2015-07-30T23:32:00Z

@toddboom if someone on your team needs help with this just ping me. BTW, I've noticed that some other services were added to telegraf, it might be a good idea to containerize those which can't be mocked.

toddboom · 2015-07-30T23:33:51Z

@marcosnils will do - i'll probably be running through a bunch of the open PRs tomorrow, so i'll ping you if i get stuck!

marcosnils added 5 commits July 6, 2015 01:46

Add cirleci script

0db5500

Remove unnecessary circleci configuration as we're using default

ae385b3

provided services Update test users to use circleci default services

Remove circle ci implementation due to Golang bug.

6355228

I've tried to set-up circleci but I came across a Golang issue that was preventing CPU tests to PASS. Instead of ignoring those tests I've submitted an issue to Go (golang/go#11609) hoping that this gets fixed soon.

Use postgres default configuration

4471e2b

Add --no-recreate option to prepare target

aa86c16

marcosnils added 2 commits July 6, 2015 22:18

Add DOCKER_HOST support for tests

d2810dd

This allows to run tests in environments where DOCKER_HOST is used. This is extremely helpful when using boot2docker to run docker

Add missing files

ef335d9

evanphx reviewed Jul 21, 2015
View reviewed changes

toddboom added a commit that referenced this pull request Jul 30, 2015

Merge pull request #49 from marcosnils/container_services

a4d0c47

Container services

toddboom merged commit a4d0c47 into influxdata:master Jul 30, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Container services #49

Container services #49

marcosnils commented Jul 6, 2015

toddboom commented Jul 6, 2015

marcosnils commented Jul 6, 2015

marcosnils commented Jul 7, 2015

brocaar commented Jul 7, 2015

marcosnils commented Jul 7, 2015

brocaar commented Jul 8, 2015

marcosnils commented Jul 8, 2015

evanphx Jul 21, 2015

marcosnils Jul 21, 2015

marcosnils Jul 21, 2015

toddboom commented Jul 30, 2015

marcosnils commented Jul 30, 2015

toddboom commented Jul 30, 2015

Container services #49

Container services #49

Conversation

marcosnils commented Jul 6, 2015

toddboom commented Jul 6, 2015

marcosnils commented Jul 6, 2015

marcosnils commented Jul 7, 2015

brocaar commented Jul 7, 2015

marcosnils commented Jul 7, 2015

brocaar commented Jul 8, 2015

marcosnils commented Jul 8, 2015

evanphx Jul 21, 2015

Choose a reason for hiding this comment

marcosnils Jul 21, 2015

Choose a reason for hiding this comment

marcosnils Jul 21, 2015

Choose a reason for hiding this comment

toddboom commented Jul 30, 2015

marcosnils commented Jul 30, 2015

toddboom commented Jul 30, 2015