docker-compose and reuse it in crossdock #493

pavolloffay · 2017-10-25T15:45:31Z

Docker-compose yaml with cassandra deployment.
The compose file is reused in crossdock.
test-driver is not starting jaeger services

Tests are not passing because xdock containers send tracing data to test-driver and not jaeger-*.

If we can use docker compose 3.3 (it requires docker engine 17.06.0+) then we can make this even more generic -- split this compose file and support multiple storages:

one compose file to define only jaeger services with enabled --config-file=
one compose file per storage. compose file would define config map for all jaeger services.

This is related to #469 #405

coveralls · 2017-10-25T15:54:33Z

Coverage remained the same at 100.0% when pulling 7e7a9d8 on pavolloffay:generic-integration-tests into a7f203b on jaegertracing:master.

pavolloffay · 2017-10-25T16:13:13Z

Makefile

@@ -31,7 +31,7 @@ THRIFT_GEN_DIR=thrift-gen
 PASS=$(shell printf "\033[32mPASS\033[0m")
 FAIL=$(shell printf "\033[31mFAIL\033[0m")
 COLORIZE=$(SED) ''/PASS/s//$(PASS)/'' | $(SED) ''/FAIL/s//$(FAIL)/''
-DOCKER_NAMESPACE?=$(USER)
+DOCKER_NAMESPACE?=jaegertracing


Setting the repository to the correct one. I added user input to docker push to avoid unwanted pushes.

This was on purpose, so that we don't push accidentally to the official repository. Just pass DOCKER_NAMESPACE before the make command to override it:

DOCKER_NAMESPACE=jaegertracing make

Or, like this:

jaeger/travis/build-docker-images.sh

Line 23 in f0193c6

export DOCKER_NAMESPACE=jaegertracing

This was on purpose, so that we don't push accidentally to the official repository.

I know, because of this I have added user input/verification before docker push.

This change is because now xdock uses only docker images so it has to build. We could use different image names/repository in docker-compose but that seems more odd.

This should work CONTINUE=y make docker-push

yurishkuro · 2017-10-26T02:37:20Z

test-driver is not starting jaeger services

awesome!

jpkrohling · 2017-10-26T09:46:52Z

Makefile

 	docker build -t $(DOCKER_NAMESPACE)/jaeger-cassandra-schema:${DOCKER_TAG} plugin/storage/cassandra/
 	@echo "Finished building jaeger-cassandra-schema =============="
+	cp -r jaeger-ui-build/build/ cmd/query/jaeger-ui-build


It's easier for this command to fail than the jaeger-cassandra-schema, so, keeping it as the first will make it fail "faster". Or were you having problems with this order of actions?

It makes sense, I just moved it closer to the place where it is used.

jpkrohling · 2017-10-26T09:47:36Z

Makefile


 .PHONY: docker-push
 docker-push:
+	@while [ -z "$$CONTINUE" ]; do \


Will this work in non-interactive mode? Like, on Travis?

I have to find out :).

This target at the moment is not used on travis. However I would like to simplify travis config a bit and use travis stages and have deploy stage which would consistently deploy all images (potentially using this command)

coveralls · 2017-10-26T12:32:29Z

Coverage remained the same at 100.0% when pulling 672aae4 on pavolloffay:generic-integration-tests into a7f203b on jaegertracing:master.

coveralls · 2017-10-26T12:36:18Z

Coverage remained the same at 100.0% when pulling 672aae4 on pavolloffay:generic-integration-tests into a7f203b on jaegertracing:master.

pavolloffay · 2017-10-26T16:54:11Z

The question also is if we want to have compose file in this repository or move it to a separate.

coveralls · 2017-10-26T17:12:02Z

Coverage remained the same at 100.0% when pulling d1e3fb5 on pavolloffay:generic-integration-tests into a7f203b on jaegertracing:master.

coveralls · 2017-10-26T17:28:21Z

Coverage remained the same at 100.0% when pulling 28c72ac on pavolloffay:generic-integration-tests into a7f203b on jaegertracing:master.

pavolloffay · 2017-10-26T17:29:41Z

crossdock/docker-compose.yml

@@ -27,48 +30,49 @@ services:
        image: jaegertracing/xdock-go
        ports:
            - "8080-8082"
+        links:
+          - "jaeger-agent:test_driver"


Temporary fix, xdock images expect jaeger services on this host.

coveralls · 2017-10-26T17:36:52Z

Coverage remained the same at 100.0% when pulling 8e37126 on pavolloffay:generic-integration-tests into a7f203b on jaegertracing:master.

coveralls · 2017-10-26T17:40:38Z

Coverage remained the same at 100.0% when pulling 8e37126 on pavolloffay:generic-integration-tests into a7f203b on jaegertracing:master.

pavolloffay · 2017-10-26T18:38:02Z

It should be ready for the review.

coveralls · 2017-10-28T09:04:06Z

Coverage remained the same at 100.0% when pulling 77ce3cd on pavolloffay:generic-integration-tests into 44de283 on jaegertracing:master.

coveralls · 2017-10-28T10:18:26Z

Coverage decreased (-0.03%) to 99.969% when pulling cfa2376 on pavolloffay:generic-integration-tests into 44de283 on jaegertracing:master.

yurishkuro · 2017-10-28T22:02:04Z

cmd/standalone/main.go

-	r := mux.NewRouter()
-	apiHandler.RegisterRoutes(r)
+	if staticHandler != nil {
+		apiHandler.RegisterRoutes(r)


this is confusing. Better to group related code (and use scopes to hide vars that's not needed later)

r := mux.NewRouter() { h := New... h.RegisterRoutes(r) } if h, err := queryApp.NewStaticAssetsHandler(qOpts.StaticAssets, qOpts.UIConfig); err != nil { logger.Fatal("Could not create static assets handler", zap.Error(err)) } else if h != nil { h.RegisterRoutes(r) } else { logger.Info("Static handler is not registered") }

I have added a function which handles static handler registration

coveralls · 2017-10-30T09:20:07Z

Coverage remained the same at 100.0% when pulling 84b75ff on pavolloffay:generic-integration-tests into 44de283 on jaegertracing:master.

Signed-off-by: Pavol Loffay <[email protected]>

coveralls · 2017-10-30T12:29:54Z

Coverage remained the same at 100.0% when pulling 26c3f59 on pavolloffay:generic-integration-tests into 975982b on jaegertracing:master.

yurishkuro · 2017-10-30T16:22:05Z

@pavolloffay nice! there are 3 different tickets mentioned here, can you close those that are resolved by this pr?

yurishkuro · 2017-10-31T19:17:03Z

@pavolloffay could you take a look at jaeger-client-go builds? Crossdock tests are now failing there, I suspect it might be related to this change

https://travis-ci.org/jaegertracing/jaeger-client-go/jobs/295424981

pavolloffay · 2017-11-01T08:12:18Z

Yes I will, all xdocs has to be updated. Sorry I could have been more proactive here.

…#493) * Introduce shared docker-compose and reuse it in crossdock * Fix empty static files

pavolloffay requested review from black-adder, vprithvi and yurishkuro as code owners October 25, 2017 15:45

ghost assigned pavolloffay Oct 25, 2017

ghost added the review label Oct 25, 2017

pavolloffay commented Oct 25, 2017

View reviewed changes

jpkrohling reviewed Oct 26, 2017

View reviewed changes

pavolloffay commented Oct 26, 2017

View reviewed changes

pavolloffay changed the title ~~WIP Introduce docker-compose and reuse it in crossdock~~ docker-compose and reuse it in crossdock Oct 27, 2017

pavolloffay force-pushed the generic-integration-tests branch from 8e37126 to 77ce3cd Compare October 28, 2017 08:54

pavolloffay mentioned this pull request Oct 28, 2017

Allow embedding custom UI config in index.html #490

Merged

2 tasks

yurishkuro reviewed Oct 28, 2017

View reviewed changes

This was referenced Oct 29, 2017

Docker compose for deployment of individual components #469

Closed

Query service fails without static assets #497

Closed

pavolloffay added 3 commits October 30, 2017 10:45

Introduce docker-compose and reuse it in crossdock

ff16ac3

Signed-off-by: Pavol Loffay <[email protected]>

Fix empty static files

89362ef

Signed-off-by: Pavol Loffay <[email protected]>

fix default test

d3df411

Signed-off-by: Pavol Loffay <[email protected]>

pavolloffay added 3 commits October 30, 2017 10:45

Increase test coverage

64925fd

Signed-off-by: Pavol Loffay <[email protected]>

Refactor mains

aa61979

Signed-off-by: Pavol Loffay <[email protected]>

fix

041da91

Signed-off-by: Pavol Loffay <[email protected]>

pavolloffay force-pushed the generic-integration-tests branch from ffa2cf2 to 041da91 Compare October 30, 2017 09:45

Fix build

26c3f59

Signed-off-by: Pavol Loffay <[email protected]>

yurishkuro approved these changes Oct 30, 2017

View reviewed changes

yurishkuro merged commit 2acee35 into jaegertracing:master Oct 30, 2017

ghost removed the review label Oct 30, 2017

This was referenced Oct 30, 2017

added a docker-compose file to the main repo, and an explanation regarding its usage to the docs #306

Closed

Jaegertracing with Docker #286

Closed

Docker compose multiple storages #502

Open

isaachier pushed a commit to isaachier/jaeger that referenced this pull request Nov 1, 2017

Create shared docker-compose and reuse it in crossdock (jaegertracing…

7acf537

…#493) * Introduce shared docker-compose and reuse it in crossdock * Fix empty static files

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docker-compose and reuse it in crossdock #493

docker-compose and reuse it in crossdock #493

pavolloffay commented Oct 25, 2017

coveralls commented Oct 25, 2017

pavolloffay Oct 25, 2017

jpkrohling Oct 26, 2017

pavolloffay Oct 26, 2017

pavolloffay Oct 26, 2017

yurishkuro commented Oct 26, 2017

jpkrohling Oct 26, 2017

pavolloffay Oct 26, 2017

jpkrohling Oct 26, 2017

pavolloffay Oct 26, 2017

coveralls commented Oct 26, 2017

coveralls commented Oct 26, 2017

pavolloffay commented Oct 26, 2017

coveralls commented Oct 26, 2017

coveralls commented Oct 26, 2017

pavolloffay Oct 26, 2017

coveralls commented Oct 26, 2017

coveralls commented Oct 26, 2017

pavolloffay commented Oct 26, 2017

coveralls commented Oct 28, 2017

coveralls commented Oct 28, 2017

yurishkuro Oct 28, 2017

pavolloffay Oct 30, 2017

coveralls commented Oct 30, 2017

coveralls commented Oct 30, 2017

yurishkuro commented Oct 30, 2017

yurishkuro commented Oct 31, 2017

pavolloffay commented Nov 1, 2017

docker-compose and reuse it in crossdock #493

docker-compose and reuse it in crossdock #493

Conversation

pavolloffay commented Oct 25, 2017

coveralls commented Oct 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurishkuro commented Oct 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 26, 2017

coveralls commented Oct 26, 2017

pavolloffay commented Oct 26, 2017

coveralls commented Oct 26, 2017

coveralls commented Oct 26, 2017

Choose a reason for hiding this comment

coveralls commented Oct 26, 2017

coveralls commented Oct 26, 2017

pavolloffay commented Oct 26, 2017

coveralls commented Oct 28, 2017

coveralls commented Oct 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 30, 2017

coveralls commented Oct 30, 2017

yurishkuro commented Oct 30, 2017

yurishkuro commented Oct 31, 2017

pavolloffay commented Nov 1, 2017