Add standalone Dockerfile that runs agent, collector, and query #69

badiib · 2017-03-29T18:41:54Z

UI is not part of this docker instance

yurishkuro · 2017-03-29T18:49:19Z

cmd/standalone/Dockerfile

+EXPOSE 5775 6831 6832 14267 16686
+
+#example build command from github.com/uber/jaeger directory: docker build -f cmd/standalone/Dockerfile -t standalone .
+#example docker run command: docker run --publish 5755:5775 --publish 6831:6831 --publish 6832:6832 --publish 14267:14267 --publish 16686:16686 --name standalone --rm standalone


what are these?

can you explain what you mean by "these", please?

yurishkuro · 2017-03-29T18:52:51Z

cmd/standalone/Dockerfile

+
+ENTRYPOINT go run /go/src/github.com/uber/jaeger/cmd/standalone/main.go --span-storage.type=memory
+
+EXPOSE 5775 6831 6832 14267 16686


I made a comment on earlier PRs that we need to define a single ports.go file and consolidate all ports used by the backend in one place. The docker image should be able to expose all ports with N-M range.

I can't find this comment. Our port definitions are all over the place, including work that was done in the agent. Is this something we want to tackle in this PR or is this a request that should be encapsulated in a separate ticket?

it's a pre-requisite

yurishkuro · 2017-03-29T18:56:42Z

let's keep this open and define a proper task (#70) with all requirements.

coveralls · 2017-03-29T19:52:56Z

Coverage remained the same at 98.365% when pulling f783410 on rooz/dockerize-single-application into 00435d3 on master.

vprithvi · 2017-04-06T17:58:04Z

cmd/standalone/Dockerfile

+RUN node -v
+RUN npm -v
+
+RUN git clone https://github.com/uber/jaeger-ui


Do you think it's worthwhile to publish to npmjs and depend on that instead?

@saminzadeh ^ what do these words mean? Is it possible? we're trying to build a docker container that contains the UI, query service and hotrod.

My preference is to use a git submodule. Publishing to NPM is unnecessary overhead & release friction.

Yea, not worth it publish to NPM.

yurishkuro · 2017-04-06T18:18:59Z

cmd/standalone/Dockerfile

+FROM golang:1.7
+EXPOSE 5775 6831 6832 14267 3000 3001 8080 8081 8082 8083
+
+ADD . /go/src/github.com/uber/jaeger


the crossdock tests do the build outside of container and only copy the binary. Why would we do this differently?

I didn't like what I for crossdock, I first wanted to build everything inside the container but couldn't figure out how. I prefer building everything inside the container instead of doing it in travis and passing the binaries. I see no benefit of having travis do it.

building outside of the container is faster than inside it, which is especially useful for npm install which takes a long time.

this is a docker image we are going to publish and people are going to download. It makes no sense to have all the source files in that image. You could do the build and remove the sources, but then see (1). Also, the base image can be a lot smaller if we only need to put a binary in there, as opposed to having both Go and Node toolchains installed

okey..... ill do it that way

yurishkuro · 2017-04-06T18:19:51Z

cmd/standalone/Dockerfile

+
+RUN git clone https://github.com/uber/jaeger-ui
+
+RUN cd jaeger-ui && npm install


similar to Go binary, the npm build should happen outside of container, we only need to copy the results of the build to container

why though? when we can encapsulate everything in one place.

yurishkuro · 2017-04-06T18:20:21Z

I assume this also needs a travis change

coveralls · 2017-04-06T18:43:07Z

Coverage remained the same at 98.31% when pulling 0f4ac46 on rooz/dockerize-single-application into 2b99d9d on master.

coveralls · 2017-04-06T20:37:52Z

Coverage remained the same at 98.31% when pulling b5141eb on rooz/dockerize-single-application into 2b99d9d on master.

vprithvi · 2017-04-06T20:38:33Z

cmd/standalone/Dockerfile

+
+ENTRYPOINT /bin/hotrod-demo-linux all & /bin/standalone-linux --span-storage.type=memory --query.port=3001
+
+#example build command from github.com/uber/jaeger directory: docker build -f cmd/standalone/Dockerfile -t standalone .


I prefer that the examples go into a separate Makefile or some such, so that we can simply ask users to make docker_build or make docker_run.

right now, docker_build command depends on the user having jaeger-ui and npm, etc. For now I'm not going to expose it in the makefile. Will do in the future when we make jaeger-ui a submodule.

yurishkuro · 2017-04-06T20:38:56Z

.travis.yml

+  - secure: cnVh5hOEa/UIBP1dJlZGpY7yyQlV+X3dXDuC7atePUVUTRNiRXtG96BGkz3l1mOb6R2s1We1e4SDoADP8NQfsn0TABXrAUu2ZuDW7NwMT5xoJ53aehpmXfRqO55Wau2m8X/kHl3V7eECsdekCXWua8ijmMfqYySUXwIZvuyT1tObqEpv4aItDIUFuv8app30dIZF1cE9hM91gl+0gu5N+FuwO7AAVt+Q0a3B1Ntv8mTb/rmz/lYkBBSaClgrBMQ2sjAuDDPOWfZla0SVMrymCI/F4JtR2eo82Enn6Z+JBwHRFZgMyc05EDoNh9m7hvQCqDVsvMExuf/qSVXTAlL1Tqzk6uOCyLoB1XM6SydEUIMukiNFN3h+v0WVczK9MyRaFuhN3CS6BAn1jwW9BAQKAXC4DZ8bQTCKzWT1Jd8+9x32HJrhB8ylBXA/ER8xghiIMv/Wk8bFW6tfnTYU/uiCtoFfSk5PI9cLLVhuSUxgyX/9KWBPGnA1/PhC2W3H4vs+uVHTisE5+LPBTWoSII7Z82u+UZTLH/wmAgOroNEHCULdSd82Efn8El9e4ebl6zR5MWOtgsK1zKd4wdq00oc19x0vLgAUeznTc+XwhLfSN7WwpR0mYEiZnOubvscc/rTo01FnPlIbn1vKdwUKw2GfXafqq2Vsa0p/q1Eo4pZfOzE=
+  - secure: BMpuCCm+z/sOAMZ5BDX7BDzI5y/H32KpwfJaVNL7womV5Mzxz9yPe1muQG+Z9cL6aCBQwToRTbvuFLdSzjPT9A+EBQuDuWUfVC3vkpbkXk1vdN73D1LeWkAsjI7OZ32fim3+xksHnGMs6VsWsObZrmq6QXeHl50MmXeeYqMnc+4g3TkhIyFnYeVCHUuiidMmVqikJm7ujLvoN89tXP3QZ4khw74nHd3IiT45h06GPkoYRs8euT9x8wNBkhm2RVNBkTeClbiqlhfywDbU2/O6otQ5+6A21QNBAk0rKH4OCc+lJ39hRV2bmPnqbw0vFhGk9YdIN3JmydpmOQW3vWr7eiBOrI8eMBXbsNzddYCISOzEtgApJnaK8tQIeEY8QvoqdgVwk2mEXUKn9EJii0BhkAwv3KF5Obi3x5jZcEo8YP5dn05gXzpjk2cFJB8npBlSSlkKcKYQKX605UYwEbs3XxOU90Mg03nwxNKwk26aFx0T1mUIzYcx0h/a99OLMFr/mK5BKRLQaFh03kp6HCS0q5j/fotN4ZW41QYPKIjvv+FyuMcGNtx5C5qTlEWZcECVJoM4dKxMa/z3REeKVqtQYq0azCSjG5Tje8cslUbLNF4N7NvNOJux6GHVGtmE29atzrw3Wsra4hARFm+kOAE9b/1Qfjnz3C3XDmFe07Sxntg=
+  - secure: tlWX/wnfuyvTTEBG6UwIrAugCLKKYuEZvcwrOYgaO27Q0SQmHOevGhsCOVDDJ2e01no60IR6Cu67O+JlRrZqOuZIkl9uvnf60SZWDbqCMhZraVh5b0iZD1li0XHXo56zj+ptDXc3fOmnUTT2PYnZ+4+sR76jBw2PnZKE/u6NNSm3KkKvEQGfFkFdId/17z6Lob++BwXqmIw1n2DG7Ts+VxyCVf5HBfe8RfDPWDlXfqQUE5x9hbl+VbwCR+4LDAAJmh9h7JTItX7nvNG91EzEGedKea8IWn7fkLcQSZWuMalpGADqMx0nsPyJk8fzc0X17w595P54ktatwcWWgVv+ZhzozoO3FPWOom7c35gx+WAlMeIBtpxYQtixP2xPpK1i4HJXjW4KS+zjTEkoqeGQrFXcIKHUM0YXQvRIp5rfRky/INDzm2AYTReGoLX+ZQ7O/tSVDaBfdvTd96hRP6M1t4zCGULaglQGdE7/RLQrz7S6j2ys0fhF5gFBmOSodGt5gG+YFjzKiu+5FYMICwpUaOO+h4mLTsP0t63vFKGfXt79eWFJ+pRaMyWb68geZqB5u+P64sTnEIa8LjP/Lhrgnh0vab0O0m2KgnRMKBVlc4modKLiTXbYn1KZUyk0dfKYR2lCQHxAZW94nBOXzynDu8qZE0nXemVokFkEETm1FB0=
+  - secure: aR7jrdGh3cXncHlIJ6FgVf5jqMiUHSWiGON3tocCUIAB3GyRoT4lFsHg4y86c4qDkpiTrFZT2mY7Iyqs/Y+Hi64V2pGP2FouN9cZK8b5IuBDboa//9nebAvTVuzsuUBLmRwtaQ2qvpS+Jg7ApXyLjF+Cdvfh4WRm4IcwxbvmMr/+zVxkPbEBNnnALx7h1kNiAtQVrCjiHJ1Q+p2RzER+OSTtdvJRqMzkwI741ttj7pvxElbexdPwiU+9lLC9IoW/lSOSwx29Ph8WAK2aF2VrxxrUbNiTd8o2U11Zc2ic44FhOadTrGk6bJBLHOerFnDt9ieoGpbocinviZJ2OGvSoXWaI9XNPaxCNSuA3woRRX4+j9SK/AFunSxUtC2gyUhCvMKLcz1WbVtto9kAXZKq+N1zBrA5zeFCArbKCLOidTdrv1iY5eC/mV6RPhvy2knpDRzFtk6UP/7oiTiUu1yI1CGDRrWRQaaguRcLdhSKFKfgjfRNksfO1QQ6rokZWZdc5kUchLEcb2QtCRWOAShfuvyr+j38l8Rgx5DwBAnd7zov0H4cXUZ1oBOzbHmejHQF7KMs6cnGgCvDT2mQYEwvMc9cueD8S3qWrkaLIm78pQesKLuKarYHeAF8pWivQPr9hwSSPnY944E9YT0odMbNmNrWruwF2YuyFKTMdIVhy3Q=


please add comments showing which env var is defined by each line

yurishkuro · 2017-04-06T20:40:29Z

cmd/standalone/Dockerfile

+COPY ./examples/hotrod/hotrod-demo-linux /bin/
+COPY ./cmd/standalone/standalone-linux /bin/
+
+ENTRYPOINT /bin/hotrod-demo-linux all & /bin/standalone-linux --span-storage.type=memory --query.port=3001


Q: what's the reason we need to change query port?

the ui looks for 3001 specifically

I think you're running it incorrectly. There's no 3001 in our internal build of query + ui.

well yeah, internally we build the static files and move them into the query service. We haven't exposed that in oss. Do you want me to do that too for this?

yurishkuro · 2017-04-06T20:43:56Z

cmd/standalone/Dockerfile

+FROM golang:1.7
+EXPOSE 5775 6831 6832 14267 3000 3001 8080 8081 8082 8083
+
+COPY ./examples/hotrod/hotrod-demo-linux /bin/


I don't think we want to do that. This image is meant to run jaeger-backend only. The demo can be run from source, especially since for the purpose of actual demos it requires changing the source.

yurishkuro · 2017-04-06T20:46:13Z

cmd/standalone/Dockerfile

@@ -0,0 +1,10 @@
+FROM golang:1.7


there may be a lighter base image than the one with full go tool chain (add TODO)

The alpine image should work because we don't depend on git, etc
We can simply use golang:1.7-alpine

coveralls · 2017-04-06T21:54:34Z

Coverage increased (+0.07%) to 98.378% when pulling 6df44b7 on rooz/dockerize-single-application into 2b99d9d on master.

coveralls · 2017-04-07T14:29:32Z

Coverage increased (+0.07%) to 98.378% when pulling 1740b79 on rooz/dockerize-single-application into 2b99d9d on master.

…ce (with no UI)

coveralls · 2017-04-07T20:37:14Z

Coverage decreased (-0.1%) to 98.227% when pulling 2bb5afc on rooz/dockerize-single-application into 527cde4 on master.

coveralls · 2017-04-07T21:18:16Z

Coverage decreased (-0.1%) to 98.227% when pulling b4bfb0e on rooz/dockerize-single-application into 527cde4 on master.

coveralls · 2017-04-07T21:28:49Z

Coverage increased (+0.0007%) to 98.339% when pulling b4bfb0e on rooz/dockerize-single-application into 527cde4 on master.

yurishkuro · 2017-04-07T21:55:21Z

.travis.yml

  - make install_ci

 script:
  - make test_ci
  - travis_retry goveralls -coverprofile=cover.out -service=travis-ci || true

+after_success:
+  - git clone https://github.com/uber/jaeger-ui


I would like this to be a submodule, not an explicit clone. Submodule allows precise version control of the UI project when we're building query service, but clone always pulls latest that may be incompatible.

yurishkuro · 2017-04-07T21:56:30Z

.travis.yml

  - make install_ci

 script:
  - make test_ci
  - travis_retry goveralls -coverprofile=cover.out -service=travis-ci || true

+after_success:
+  - git clone https://github.com/uber/jaeger-ui
+  - cd jaeger-ui && npm install && npm run build && cd ..


I think it's better to do take this in parentheses, then you don't need cd .. (which incidentally may fail)

yurishkuro · 2017-04-07T21:57:09Z

.travis.yml

  - make install_ci

 script:
  - make test_ci
  - travis_retry goveralls -coverprofile=cover.out -service=travis-ci || true

+after_success:
+  - git clone https://github.com/uber/jaeger-ui
+  - cd jaeger-ui && npm install && npm run build && cd ..


Also, not sure if yarn is installed by default on travis, if it is, the yarn install is a lot faster than npm install

actually, why don't we have a make target for this? it will be needed when building / testing query service locally anyway, less noise in travis file.

yurishkuro · 2017-04-07T21:57:51Z

.travis.yml

 install:
+  - nvm version
+  - node -v


we need a minimum version, I would suggest nvm use 4 somewhere in this file - @saminzadeh thoughts?

yurishkuro · 2017-04-07T22:01:30Z

.travis.yml

+  - docker build -f cmd/standalone/Dockerfile -t $REPO:$COMMIT .
+#  - docker tag $REPO:$COMMIT $REPO:$TAG TODO docker doesn't like tags with \ need to omit them
+  - docker tag $REPO:$COMMIT $REPO:travis-$TRAVIS_BUILD_NUMBER
+  - docker push $REPO


what happens if some of these commands fails? I think the may execution still continues, i.e. we may fail to build the UI yet we'd continue creating the invalid docker image. We don't have to address it in this diff, but need a better strategy. Perhaps take a look at how zipkin-docker travis files are setup.

yurishkuro · 2017-04-07T22:02:09Z

.travis.yml

+  - echo "TRAVIS_BRANCH=$TRAVIS_BRANCH, REPO=$REPO, PR=$PR, BRANCH=$BRANCH, TAG=$TAG"
+  - docker login -u $DOCKER_USER -p $DOCKER_PASS
+  - docker build -f cmd/standalone/Dockerfile -t $REPO:$COMMIT .
+#  - docker tag $REPO:$COMMIT $REPO:$TAG TODO docker doesn't like tags with \ need to omit them


maybe you just need to include it in quotes? "$REPO:$TAG"
without this line we are never going to mark something latest

yurishkuro · 2017-04-07T22:03:09Z

Makefile

@@ -82,6 +82,12 @@ install_examples: install
 build_examples:
 	go build -o ./examples/hotrod/hotrod-demo ./examples/hotrod/main.go

+build_standalone:


we don't really need this, let's not pollute makefile

yurishkuro · 2017-04-07T22:10:20Z

.travis.yml

+  - git clone https://github.com/uber/jaeger-ui
+  - cd jaeger-ui && npm install && npm run build && cd ..
+  - mkdir jaeger-ui-build && cp -r jaeger-ui/build jaeger-ui-build/
+  - make build_standalone_linux


can we call it build-all-in-one-linux (with dash, not underscore - we should fix all targets with underscore, bad practice)

yurishkuro · 2017-04-07T22:11:30Z

cmd/standalone/Dockerfile

+ENTRYPOINT /go/bin/standalone-linux --span-storage.type=memory --query.static-files=/go/src/jaeger-ui-build/build/
+
+#example build command from github.com/uber/jaeger directory: docker build -f cmd/standalone/Dockerfile -t standalone .
+#example docker run command: docker run --publish 5755:5775 --publish 6831:6832 --publish 14267:14267 --publish 16686:16686 --name standalone --rm standalone


remove these?

yurishkuro · 2017-04-07T22:51:24Z

Makefile

+build-all-in-one-linux:
+	cd jaeger-ui && npm install && npm run build
+	rm -rf jaeger-ui-build && mkdir jaeger-ui-build
+	cp -r jaeger-ui/build jaeger-ui-build/


nit: move the above 3 lines to build-ui target and make this one depend on it (or even not depend)

yurishkuro · 2017-04-07T23:09:52Z

don't forget to move back to after_script:

coveralls · 2017-04-08T00:12:30Z

Coverage decreased (-0.07%) to 98.272% when pulling c01a660 on rooz/dockerize-single-application into 527cde4 on master.

…ertracing#69)

yurishkuro reviewed Mar 29, 2017

View reviewed changes

vprithvi reviewed Apr 6, 2017

View reviewed changes

yurishkuro reviewed Apr 6, 2017

View reviewed changes

black-adder force-pushed the rooz/dockerize-single-application branch from 0f4ac46 to f82d370 Compare April 6, 2017 18:52

vprithvi reviewed Apr 6, 2017

View reviewed changes

yurishkuro reviewed Apr 6, 2017

View reviewed changes

black-adder changed the title ~~Add standalone Dockerfile that runs agent, collector, and query~~ [WIP] Add standalone Dockerfile that runs agent, collector, and query Apr 6, 2017

badiib and others added 11 commits April 7, 2017 14:04

Add standalone Dockerfile that runs agent, collector, and query servi…

4502c56

…ce (with no UI)

build node UI

a1d7911

push docker image to docker hub

ccf50c2

pass in binary

efc52a0

testing docker hub upload

9ce8af7

before_script

cfe32f5

remove docker hub email

af1bd85

encrypt docker pass again

cc22b3b

magic?

e401b30

cry

634805b

remove repo:tag for now

089f812

black-adder added 2 commits April 7, 2017 16:09

cmd/standalone/Dockerfile

fd89d10

move docker stuff to after_success

2bb5afc

black-adder changed the title ~~[WIP] Add standalone Dockerfile that runs agent, collector, and query~~ Add standalone Dockerfile that runs agent, collector, and query Apr 7, 2017

black-adder added 2 commits April 7, 2017 17:08

fix port issue

ecb95ce

coverage

b4bfb0e

yurishkuro reviewed Apr 7, 2017

View reviewed changes

black-adder added 3 commits April 7, 2017 18:20

more testing

cc7f7bf

4 instead

9bb4299

jjjjjj

8083fcc

yurishkuro reviewed Apr 7, 2017

View reviewed changes

one last nit

6322f4e

yurishkuro approved these changes Apr 7, 2017

View reviewed changes

move docker

c01a660

black-adder merged commit 0ab671e into master Apr 8, 2017

black-adder deleted the rooz/dockerize-single-application branch April 8, 2017 00:16

ideepika pushed a commit to ideepika/jaeger that referenced this pull request Oct 22, 2017

Add standalone Dockerfile that runs agent, collector, and query (jaeg…

6e2c6fa

…ertracing#69)

jpkrohling mentioned this pull request Dec 19, 2019

Added support for dynamic queue sizes #1985

Merged


		ENTRYPOINT go run /go/src/github.com/uber/jaeger/cmd/standalone/main.go --span-storage.type=memory

		EXPOSE 5775 6831 6832 14267 16686


		RUN git clone https://github.com/uber/jaeger-ui

		RUN cd jaeger-ui && npm install


		ENTRYPOINT /bin/hotrod-demo-linux all & /bin/standalone-linux --span-storage.type=memory --query.port=3001

		#example build command from github.com/uber/jaeger directory: docker build -f cmd/standalone/Dockerfile -t standalone .

Add standalone Dockerfile that runs agent, collector, and query #69

Add standalone Dockerfile that runs agent, collector, and query #69

Conversation

badiib commented Mar 29, 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

Choose a reason for hiding this comment

yurishkuro commented Mar 29, 2017

coveralls commented Mar 29, 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

black-adder Apr 6, 2017 • edited Loading

Choose a reason for hiding this comment

yurishkuro commented Apr 6, 2017

coveralls commented Apr 6, 2017

coveralls commented Apr 6, 2017

vprithvi Apr 6, 2017 • edited Loading

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

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

Choose a reason for hiding this comment

vprithvi Apr 6, 2017 • edited Loading

Choose a reason for hiding this comment

coveralls commented Apr 6, 2017

coveralls commented Apr 7, 2017

coveralls commented Apr 7, 2017

coveralls commented Apr 7, 2017

coveralls commented Apr 7, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurishkuro Apr 7, 2017 • edited Loading

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

Choose a reason for hiding this comment

yurishkuro commented Apr 7, 2017

coveralls commented Apr 8, 2017

black-adder Apr 6, 2017 •

edited

Loading

vprithvi Apr 6, 2017 •

edited

Loading

vprithvi Apr 6, 2017 •

edited

Loading

yurishkuro Apr 7, 2017 •

edited

Loading