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

Add ability to add multiple tags with docker build #15780

Merged
merged 1 commit into from Oct 23, 2015

Conversation

mountkin
Copy link
Contributor

Closes #863

docker build -t tag1 -t tag2:v1 -t tagN .

TODO:

  • Reach a consensus on whether we need this feature
  • Complete the test cases

Signed-off-by: Shijiang Wei mountkin@gmail.com

@cpuguy83
Copy link
Member

I don't see a good reason to -1 this, so +1.

@thaJeztah
Copy link
Member

I'm +1, and it was already approved on in #863 (comment), but a very long time ago 😄

@@ -50,7 +50,7 @@ var validCommitCommands = map[string]bool{
type Config struct {
DockerfileName string
RemoteURL string
RepoName string
RepoName interface{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to use an interface{} here.
Entrpoiny/Cmd have some special parsing you can look at (in runconfig/config.go) to handle this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! I think we can abstract the parser in runconfig to "opts" package so that we can reuse the code here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpuguy83 this will depend on #15913
I found change the RepoName field to []string is okay. No BC break introduced.

@mountkin mountkin force-pushed the build-multi-tags branch 2 times, most recently from e0a9033 to b206b2c Compare August 30, 2015 14:02
@mountkin mountkin changed the title [WIP] Add ability to add multiple tags with docker build Add ability to add multiple tags with docker build Aug 30, 2015
@mountkin
Copy link
Contributor Author

Test added. The failure of the experimental test is not caused by this PR.

@@ -40,7 +40,7 @@ var (
func() bool {
// Set a timeout on the GET at 15s
var timeout = time.Duration(15 * time.Second)
var url = "https://hub.docker.com"
var url = "http://www.baidu.com"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure this should be changed? (also this changes from a https:// to a http:// domain)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry. It's quite slow to connect to docker hub in China so I changed the url temporarily but forgot to change it back before pushing to github.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I thought that would probably be the case; given that we have quite some contributors from China; do you think we should have some alternatives?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's okay to leave it as it is. But I have to be more careful if I change it again 😄
If in the future other contributors run into the same problem as mine, I think we can allow the user to pass the URL to check from an environment variable when running the tests.

@thaJeztah
Copy link
Member

@mountkin I see you updated the PR; is this ready for another review?

@mountkin
Copy link
Contributor Author

mountkin commented Sep 1, 2015

@thaJeztah I think it's ready for code review now.

@tiborvass
Copy link
Contributor

@mountkin what's the usecase? I don't feel strongly either way, don't mind if it gets in, don't mind if it doesn't.

@mountkin
Copy link
Contributor Author

mountkin commented Sep 4, 2015

@tiborvass it's useful when I want to build an image and push it to multiple registries. Without this feature I have to run "docker tag" after the build.
Other usecases can be found in #863

@tiborvass
Copy link
Contributor

Fair enough, weak +1 for design

@calavera
Copy link
Contributor

It looks useful. Let's move it forward.

@calavera
Copy link
Contributor

@mountkin do you mind rebase and making sure we have proper tests and documentation for this?

@mountkin mountkin force-pushed the build-multi-tags branch 2 times, most recently from f046fe5 to edc2518 Compare September 22, 2015 14:39
@mountkin
Copy link
Contributor Author

@calavera rebased and updated docs. Thanks for helping to move this forward.

@calavera
Copy link
Contributor

LGMT

@thaJeztah
Copy link
Member

Thanks @mountkin, docs LGTM, but unfortunately this needs a rebase 😢

ping @moxiegirl PTAL

@mountkin
Copy link
Contributor Author

@thaJeztah rebased 😃

@thaJeztah
Copy link
Member

Thanks @mountkin !

ping @jamtur01 @moxiegirl @SvenDowideit ptal

the resulting image in case of success.
- **t** – A name and optional tag to apply to the image in the `name:tag` format.
If you omit the `tag` the default `latest` value is assumed.
You can provide one or more **t** parameters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs ticks not bold around the t so t

Signed-off-by: Shijiang Wei <mountkin@gmail.com>
@mountkin
Copy link
Contributor Author

Updated the docs per @moxiegirl's suggestions.
Thanks all the reviewers for the patience. 😃

@thaJeztah
Copy link
Member

Thanks @mountkin, docs LGTM!

ping @SvenDowideit @moxiegirl PTAL

@moxiegirl
Copy link
Contributor

LGTM! Thank you @mountkin.

@vdemeester
Copy link
Member

yay 🎉

@thaJeztah
Copy link
Member

Al green! Merging. Thanks @mountkin !

thaJeztah added a commit that referenced this pull request Oct 23, 2015
Add ability to add multiple tags with docker build
@thaJeztah thaJeztah merged commit 448398c into moby:master Oct 23, 2015
@liyaka
Copy link

liyaka commented Oct 26, 2015

What about pushing multiple tags in one command as well?
For example I have an image with 2 tags xx:1.3-latest and xx:1.3.58. At the moment I have to push the same image twice which adds a few minutes to my build time...

@thaJeztah
Copy link
Member

@liyaka there are still some outstanding issues / improvements to be made to push/pull, it has been decided to uphold new features in that until those improvements are made.

Having said that; yes it would be nice to have :-)

@mountkin mountkin deleted the build-multi-tags branch November 10, 2015 11:30
@thaJeztah thaJeztah added this to the 1.10.0 milestone Jan 16, 2016
@SimenB
Copy link

SimenB commented Sep 1, 2016

@thaJeztah Any news on multiple tags in single push? 😄

@stevvooe
Copy link
Contributor

stevvooe commented Sep 1, 2016

@SimenB We want to reserve the syntax in docker push to allow for specifying alternate remotes. For example, we'd like to be able to say docker push myregistry:5000 image. I could see adding a -t flag, as well, but there is no active proposal that covers these two use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet