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

Supporting tagging images during build #457

Closed
wants to merge 3 commits into from

Conversation

dnephin
Copy link

@dnephin dnephin commented Aug 30, 2014

Resolves #213

Includes:

  • docs
  • validation for tags as list not string or dict
  • unit tests and an integration test

The first commit was some small cleanup in preparation for this change. _build_tag_name() would be confusing since it's not related to tags exactly. I believe full_name is more clear here because it's the name used by fig to uniquely identify this service, not just the image name.

The new unit tests also found a bug where BuildError() was not being called correctly.

@bencevans
Copy link

+1

1 similar comment
@yaojialyu
Copy link

👍

- 'tag-without-version'
- 'tag-with-version:v3'
- 'user/tag-with-user'
- 'user/tag-with-user-and-version:v4'
Copy link
Member

Choose a reason for hiding this comment

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

Should there be tests with the same repo/image name, but different tags? e.g.

- 'user/tag-with-user-and-version:v4'
- 'user/tag-with-user-and-version:latest'

(Not sure if those tests are useful, but it is something that may be used)

Copy link
Author

Choose a reason for hiding this comment

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

Ya, I suspect I will be using it that way myself. I'll add a user/tag-with-user-and-version:latest

Copy link
Member

Choose a reason for hiding this comment

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

Great! Thanks for writing this feature btw!

@dnephin
Copy link
Author

dnephin commented Sep 5, 2014

Rebase, and added the extra tag to the test fixture

move service._build_tag_name() to service.full_name in preparation for adding additional tags.

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
…n they are built.

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
@dnephin
Copy link
Author

dnephin commented Sep 26, 2014

After using this branch for a bit, I realized that doing the tagging implicitly during build is probably not ideal. On a shared host which builds and tests many images (jenkins), you probably don't want to tag things until after the tests have completed.

I've created a new fig tag command and removed the tagging from fig build. That way it can be done separately after any testing has completed.

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
@bfirsh
Copy link

bfirsh commented Sep 30, 2014

Hmm – I quite like the simplicity of tagging after the build. Then it behaves a bit like the -t flag to docker build.

On Jenkins, if you always build with a Git SHA tag, then surely they won't collide with other builds?

@bfirsh bfirsh added this to the 1.0.0 milestone Sep 30, 2014
@dnephin
Copy link
Author

dnephin commented Sep 30, 2014

True (this latest change also added support for environment variable expansion in the tags, which would be required to tag with a git sha).

I think the case where it doesn't work is when you want to tag latest as well as a git sha. You'd always want to tag latest at the end of the test suite.

What about adding a --add-tags (or something like that) to fig build, that way options are supported?

@bfirsh
Copy link

bfirsh commented Sep 30, 2014

What do you need to tag latest for?

@aanand what do you think?

@dnephin
Copy link
Author

dnephin commented Sep 30, 2014

In production we would never use latest, but in testing, when one service depends on the image of another, we'll always want to test against the latest image. Every time a new service version is pushed, we can't go changing every other downstream build to use the latest sha.

So to support that style of testing, we need to build a latest image for every service.

@aanand
Copy link

aanand commented Sep 30, 2014

I understand the argument for the tag feature as currently implemented, but I find it a bit weird that the tags specified in fig.yml won't be applied until fig tag is called - that's definitely going to be surprising for people.

@dnephin
Copy link
Author

dnephin commented Sep 30, 2014

Cool, so what about inverting it then, the default could be to apply tags during build, and fig build --no-tags would skip that step. Keeping fig tag for that case when you want to tag later.

@bfirsh
Copy link

bfirsh commented Sep 30, 2014

I think as long as you don't push at the end of an unsuccessful Jenkins build, your latest tag is going to be correct, right?

Alternatively if you want the latest tag on your builds to only be applied on success, you could have only tag: "foo:${GIT_SHA}" in your fig.yml, then at the end of your build run docker tag foo:${GIT_SHA} foo:latest.

@dnephin
Copy link
Author

dnephin commented Sep 30, 2014

Not necessarily. Other test runs on that jenkins host will use the local docker, which already has latest. So you can potentially break or pollute other test suites that run around the same time.

I guess the docker tag seems reasonable enough to me. It sounds like the consensus is to revert to the older behaviour, so I'll do that.

@bfirsh
Copy link

bfirsh commented Sep 30, 2014

I can understand where you're coming from, but I'm just trying to figure out the minimum we need to add to Fig to make it work. Will this still work for you without the separate fig tag command?

Aren't your builds going to collide anyway? fig up and fig run both still use service.full_name.

@dnephin
Copy link
Author

dnephin commented Sep 30, 2014

They wouldn't collide because of how we treat these tags. The default tags created by fig <project>_<service> I consider "internal tags" that only the local build should ever reference. Every build has a unique FIG_PROJECT_NAME so they will never conflict.

The tags in tags are the "external tags", so they can be referenced from other builds.

@bfirsh
Copy link

bfirsh commented Sep 30, 2014

Ah, understood. I think keeping this as part of fig build instead of having a separate command makes sense.

In theory, you could achieve this with docker tag entirely, right? fig build; docker tag <project>_<service> foo.

@dnephin
Copy link
Author

dnephin commented Sep 30, 2014

At this point, yes docker tag would basically do the same thing, but I have plans to re-use these tags (which is another reason that I want to include the latest tag in this list and not have it external).

For #318 there is one blocker. You can't use this setup and include remote services that use a build target. A build wouldn't ever succeed because you don't have the source/docker context to build it. So you need some way to say "use the docker image that was created as part of this build".

One way to do that would be to use the <project>_<service> tag, but as discussed, I see these as internal only (and also temporary, they don't get pushed to a registry, so they can be removed by cleanup scripts). Using the internal tags would mean conflicting with and polluting other builds. So I want to use the tags as the image name. A tag with a git sha wouldn't work here because we don't have access to the git sha either.

So that still leaves me with the requirement of having to specify the image name in a way that is safe for external use, and supports a latest tag.

FWIW, I don't see #318 as a unique requirement. Others have described a very similar system in #489, #428, and #159 (which has a bunch of +1s). While #159 (and #428) only cover the simple "single level" of dependencies, I feel the more general solution (of n-levels of dependencies) will cover both, and is more of a requirement for larger systems, with larger dependency trees.

@bfirsh
Copy link

bfirsh commented Sep 30, 2014

It is still an open question as to whether we're going to do something like #318. We are intentionally limiting the scope of Fig because we are working on a much better solution to this problem built into Docker itself.

Ignoring #318 for now, if we can achieve what this pull request is doing by using docker tag, I'm not sure it needs to be included in Fig. @aanand – thoughts?

@aanand
Copy link

aanand commented Sep 30, 2014

Yeah, seems like a no-new-code solution would be to use docker tag to create an externally-suitable tag as Ben suggests:

$ fig build
$ fig run or whatever
$ docker tag <project>_<service> <external tag>

@dnephin
Copy link
Author

dnephin commented Sep 30, 2014

That's fine, I can wait on this PR and see what gets added to Docker.

@bfirsh bfirsh removed this from the 1.0.0 milestone Sep 30, 2014
@bfirsh
Copy link

bfirsh commented Sep 30, 2014

Let's revisit this after 1.0.0. :)

@dnephin
Copy link
Author

dnephin commented Nov 7, 2014

Since this doesn't need to be in fig itself I've created a separate tool to do the tag+push step. I'm going to close this PR for now.

@dnephin dnephin closed this Nov 7, 2014
@dnephin dnephin mentioned this pull request Nov 24, 2014
@sandric
Copy link

sandric commented Feb 2, 2015

@dnephin Can you point to this tool please? Came here from #467 and there you suggested to wait till this will be resolved. I just new to fig and want to know if pushing to registry from fig will be implemented anyhow, is there any plans for this?

@dnephin
Copy link
Author

dnephin commented Feb 3, 2015

The tool I'm using isn't open sourced yet, but it's basically something like this

fig build
fig up -d
# run some tests
fig stop

If the tests pass, the tool does:

docker tag project_service_1 new_image_name
docker push new_image_name

I think fig (now docker-compose) will probably support push eventually, I'm not sure when that will be though.

@sandric
Copy link

sandric commented Feb 3, 2015

@dnephin yeah, I understood you can just tag all images, its just there can be more than one docker image to push, I mean some fig.yml parsing for images that have build entry in it, figuring out the name of image from it and pushing all of them at once.

Anyway, thanks for answer. I love fig's 'simplicity sweetness' and even think now that it can be damaging in some way - may be should do separate simple deploy tool.

@dnephin
Copy link
Author

dnephin commented Feb 3, 2015

Ya, I think there could be value in having some companion tools for complex build scenarios, and deployment scenarios. That way fig can mostly focus on composition, with very simple build, and the companion tools can focus on the more complex scenarios for each of those.

@anthony-o
Copy link

We still can't specify an image name of each build of compose... see use case here http://stackoverflow.com/q/30339950/535203

Neither tags: nor image_name: are supported by docker-compose...

@dnephin
Copy link
Author

dnephin commented Oct 2, 2015

See #2092 for the plan to support this

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

Successfully merging this pull request may close these issues.

Ability to add extra tag builds
8 participants