Docker images do not push SHA and travis build number tags #376

pavolloffay · 2017-09-04T10:36:14Z

related to #283

It also fixes all-in-one image, latest is published per commit and added release tags.

coveralls · 2017-09-04T10:46:27Z

Coverage remained the same at 100.0% when pulling dc7cfd5 on pavolloffay:docker-image-do-not-publis-travis-sha into 413942a on uber:master.

coveralls · 2017-09-04T12:30:27Z

Coverage remained the same at 100.0% when pulling 10d4c14 on pavolloffay:docker-image-do-not-publis-travis-sha into 413942a on uber:master.

yurishkuro · 2017-09-04T16:47:37Z

The title is unclear - do you mean to turn off pushing commit & travis build tags? The reason we tag with the commit sha is because sometimes in-progress changes in one project depend on in-progress changes in another one, so integration tests can run by pulling images via SHA rather than latest

pavolloffay · 2017-09-04T16:59:50Z

See your last comment in the linked issue. If you now think something different I can bring back commit SHA tags.

yurishkuro · 2017-09-04T17:14:40Z

Right, so as you notice it explicitly mentioned "remove other tags like travis build number and commit hash -- we might still want those for crossdock images"

pavolloffay · 2017-09-04T21:28:45Z

I have added back sha images for xdock

coveralls · 2017-09-04T21:38:19Z

Coverage remained the same at 100.0% when pulling 2582110 on pavolloffay:docker-image-do-not-publis-travis-sha into 413942a on uber:master.

pavolloffay · 2017-09-05T08:04:04Z

@yurishkuro is it good now?

summary:

push latest tag per commit in master
remove travis and sha tags for jaeger components (sha tags are still added for xdock test_driver)
add release tags for all-in-one

yurishkuro · 2017-09-05T17:06:47Z

crossdock/docker-compose.yml

@@ -48,6 +48,7 @@ services:

    test_driver:
        build: .
+        image: jaegertracing/test_driver:latest


what does this directive do? We want to make sure we still build from local source

It uses this name when it builds the image. If you do docker-compose pull it will pull that image, not build

yurishkuro · 2017-09-05T17:10:10Z

Makefile

@@ -29,7 +29,7 @@ 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_TAG?=local
+DOCKER_TAG?=latest


this seems unsafe

I would rather build latest than local, it's more convenient.

Note that if you are building images using this Makefile the docker namespace is $USER so not really docker hub user. I also assume that anybody who is pushing images is cautious.

The assumption that "anybody who is pushing images is cautious" doesn't comfort me :-), but since it goes under $USER namespace it should be fine.

yurishkuro · 2017-09-05T17:12:48Z

travis/upload-to-docker.sh

@@ -3,7 +3,7 @@
 set -e

 BRANCH=${BRANCH:?'missing BRANCH env var'}
-IMAGE="${REPO:?'missing REPO env var'}:${COMMIT:?'missing COMMIT env var'}"
+IMAGE="${REPO:?'missing REPO env var'}:latest"


this feels unsafe. Using COMMIT pinpoints the image precisely, while latest is kind of vague. What's the harm in keeping COMMIT? Is it because it clutters dockerhub?

Is it because it clutters dockerhub?

basically yes, But I don't have anything against it. Although I haven't seen other projects doing it.

Would somebody depend on the SHA tag? latest features will be present in the latest tag and now we have proper release tags.

yurishkuro · 2017-09-05T17:13:54Z

travis/upload-to-docker.sh

+# Do not enable echo before the `docker login` command to avoid revealing the password.
+set -x
+docker login -u $DOCKER_USER -p $DOCKER_PASS
+# push all tags, therefore push to repo


should we start the whole thing by running some sort of purge on the local repo, to avoid the possibility of some other versions handing around? Especially if this runs in development.

we can clear docker registry when starting the CI, but it seems that travis clears registry anyway.

Or we can explicitly push everything:

docker push $REPO:latest docker push $REPO:sha

, patch, minor, major would be in the if statements.

coveralls · 2017-09-07T16:11:13Z

Coverage remained the same at 100.0% when pulling fe07cc6 on pavolloffay:docker-image-do-not-publis-travis-sha into 413942a on uber:master.

tags

coveralls · 2017-09-07T16:44:39Z

Coverage remained the same at 100.0% when pulling 2adf2c7 on pavolloffay:docker-image-do-not-publis-travis-sha into 5d5a16b on uber:master.

yurishkuro · 2017-09-07T19:54:00Z

Makefile

@@ -29,7 +29,7 @@ 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_TAG?=local
+DOCKER_TAG?=latest


The assumption that "anybody who is pushing images is cautious" doesn't comfort me :-), but since it goes under $USER namespace it should be fine.

coveralls · 2017-09-08T07:49:57Z

Coverage remained the same at 100.0% when pulling 46171fc on pavolloffay:docker-image-do-not-publis-travis-sha into d17b468 on uber:master.

…rtracing#376) except for crossdock images

pavolloffay requested review from black-adder, oibe, vprithvi and yurishkuro as code owners September 4, 2017 10:36

yurishkuro reviewed Sep 5, 2017

View reviewed changes

pavolloffay added 4 commits September 7, 2017 18:24

Docker images do not push SHA and travis build number tags

154fa7c

docker: upload standalone image :latest on every commit and push release

cb62dcb

tags

use sha tag for xdock images

57e083a

improve make xdock-fresh

252a5e6

pavolloffay force-pushed the docker-image-do-not-publis-travis-sha branch from 391f9c9 to 394d07d Compare September 7, 2017 16:24

travis.yml clear local docker registry

2adf2c7

pavolloffay force-pushed the docker-image-do-not-publis-travis-sha branch from 394d07d to 2adf2c7 Compare September 7, 2017 16:33

yurishkuro approved these changes Sep 7, 2017

View reviewed changes

Merge branch 'master' into docker-image-do-not-publis-travis-sha

46171fc

yurishkuro merged commit f0193c6 into jaegertracing:master Sep 8, 2017

yurishkuro mentioned this pull request Sep 17, 2017

Tag all-in-one image #399

Closed

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

Do not tag Dicker images with SHA and travis build number tags (jaege…

8084fb9

…rtracing#376) except for crossdock images

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

Do not tag Dicker images with SHA and travis build number tags (jaege…

1e5a319

…rtracing#376) except for crossdock images

Docker images do not push SHA and travis build number tags #376

Docker images do not push SHA and travis build number tags #376

Uh oh!

Conversation

pavolloffay commented Sep 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Sep 4, 2017

Uh oh!

coveralls commented Sep 4, 2017

Uh oh!

yurishkuro commented Sep 4, 2017

Uh oh!

pavolloffay commented Sep 4, 2017

Uh oh!

yurishkuro commented Sep 4, 2017

Uh oh!

pavolloffay commented Sep 4, 2017

Uh oh!

coveralls commented Sep 4, 2017

Uh oh!

pavolloffay commented Sep 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Sep 7, 2017

Uh oh!

coveralls commented Sep 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Sep 8, 2017

Uh oh!

Uh oh!

pavolloffay commented Sep 4, 2017 •

edited

Loading