Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display created tags on successful build #32077

Merged
merged 1 commit into from Mar 27, 2017

Conversation

@zigarn
Copy link
Contributor

commented Mar 24, 2017

- What I did

Display created tags on successful build:

$ docker build -t test -t zigarn/test -t test:1.0 -t zigarn/test:1.0 -t localhost:5000/test -t localhost:5000/test:1.0 .
Sending build context to Docker daemon  2.048kB
Step 1/2 : FROM alpine
 ---> 4a415e366388
Step 2/2 : CMD sh
 ---> Using cache
 ---> dc67a808676c
Successfully built dc67a808676c
Successfully tagged test:latest
Successfully tagged zigarn/test:latest
Successfully tagged test:1.0
Successfully tagged zigarn/test:1.0
Successfully tagged localhost:5000/test:latest
Successfully tagged localhost:5000/test:1.0

- How I did it

By adapting the code?

- How to verify it

Launch a build with one or more --tag options and see the output at the end of the build showing a "Successfully tagged ".

- Description for the changelog

Display created tags on successful build.

- A picture of a cute animal (not mandatory but encouraged)

@zigarn zigarn force-pushed the zigarn:build-output branch from 338cbd3 to 7bc472b Mar 24, 2017
@GordonTheTurtle GordonTheTurtle removed the dco/no label Mar 24, 2017
@zigarn zigarn force-pushed the zigarn:build-output branch from 7bc472b to 4a3372a Mar 24, 2017
@thaJeztah

This comment has been minimized.

Copy link
Member

commented Mar 24, 2017

Nice! Design, code LGTM ping @vdemeester

Copy link
Member

left a comment

Nice @zigarn 👍
LGTM (design & code) 🦁
/cc @tonistiigi (and @thaJeztah for docs 👼)

@@ -324,6 +324,9 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
}

fmt.Fprintf(b.Stdout, "Successfully built %s\n", shortImgID)
for _, rt := range repoAndTags {

This comment has been minimized.

Copy link
@tonistiigi

tonistiigi Mar 24, 2017

Member

nit: I think the error handling makes more sense if we move the "Successfully built" before the tagging and combine the loops. In the future builder shouldn't do the tagging at all and should leave it to the API layer(or client).

This comment has been minimized.

Copy link
@zigarn

zigarn Mar 24, 2017

Author Contributor

I thought about it but wanted to minimize the modifications (as I'm a newbie in golang).
Moving the "Successfully built" before and displaying the created tags when they are created can be more relevant in case of error in one tagging: the image was successfully built and the first tags were created.

E.g. if there is an error while creating tag 'localhost:5000/test:latest', this:

[...]
 ---> Using cache
 ---> dc67a808676c
Successfully built dc67a808676c
Successfully tagged test:latest
Successfully tagged zigarn/test:latest
Successfully tagged test:1.0
Successfully tagged zigarn/test:1.0
ERROR

Would be better than this:

[...]
 ---> Using cache
 ---> dc67a808676c
ERROR

I can adapt the PR accordingly.

@zigarn zigarn force-pushed the zigarn:build-output branch from 4a3372a to 391a96d Mar 25, 2017
Signed-off-by: Alexandre Garnier <alexandre.garnier@zenika.com>
Signed-off-by: Alexandre Garnier <zigarn@gmail.com>
@zigarn zigarn force-pushed the zigarn:build-output branch from 391a96d to d005219 Mar 25, 2017
Copy link
Member

left a comment

LGTM

@tonistiigi

This comment has been minimized.

Copy link
Member

commented Mar 27, 2017

LGTM

@tonistiigi tonistiigi merged commit c5f178d into moby:master Mar 27, 2017
6 checks passed
6 checks passed
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 32090 has succeeded
Details
janky Jenkins build Docker-PRs 40706 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 832 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 11783 has succeeded
Details
z Jenkins build Docker-PRs-s390x 693 has succeeded
Details
@GordonTheTurtle GordonTheTurtle added this to the 17.05.0 milestone Mar 27, 2017
gesellix added a commit to gesellix/docker-client that referenced this pull request Apr 18, 2017
Necessary when using Docker above 17.05, due to moby/moby#32077.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.