Add DESCRIPTION to build instructions, presented in the JSON result of

[erikh]

This adds a new instruction to the dockerfile, DESCRIPTION, which was proposed in #2167. I figured this would be a fun way to learn about how Dockerfiles are handled, so here you go. bonus feature: tests!

It also adds Config.Description to the inspect output which should make the hub people happy.

[tianon]

So this is just for the short description, then? Also, what will the "Hub people" show on an image with multiple tags? Just the DESCRIPTION off :latest?

[tiborvass]

If DESCRIPTION is for build instructions, I'm afraid we would need to support multiline.

Also would this tag be taken into account for the caching? I guess it should only update the description irrespective to its position so as to not invalidate the remaining Dockerfile commands.

[erikh]

On Jun 25, 2014, at 9:27 AM, Tibor Vass notifications@github.com wrote:

If DESCRIPTION is for build instructions, I'm afraid we would need to support multiline.

Also would this tag be taken into account for the caching? I guess it should only update the description irrespective to its position so as to not invalidate the remaining Dockerfile commands.

re: caching, I think it will with the current patch. I can fix that if it’s a concern.

multiline is supported, albeit in a weird way; see my other PR about escaping and swallowing spaces, which was meant to improve this functionality. We could probably go further using quotes and whatnot.

Let me know how you’d like to proceed.

[tiborvass]

@erikh I think it'd be better to not invalidate the cache (except for DESCRIPTION).

[tiborvass]

@erikh Actually, we have the same problem with MAINTAINER.
ping @vieux @unclejack @crosbymichael what do you think about not invalidating the cache of the commands following MAINTAINER or DESCRIPTION?

[erikh]

Happy to fix MAINTAINER in this patch too; I’m pretty sure I know what’s involved in fixing it.

On Jun 25, 2014, at 2:33 PM, Tibor Vass notifications@github.com wrote:

@erikh Actually, we have the same problem with MAINTAINER.
ping @vieux @unclejack @crosbymichael what do you think about not invalidating the cache of the commands following MAINTAINER or DESCRIPTION?

—
Reply to this email directly or view it on GitHub.

[tiborvass]

@erikh My bad: apparently the cache is still very simple, I'm not sure we want to change caching right now. (Could be a separate issue/PR).

However, I will put my DESCRIPTION at the end of my Dockerfiles :)

[SvenDowideit]

please remember the documentation changes for this :)

[erikh]

Ah! I will get to it this week.

-Erik

On Jun 29, 2014, at 6:42 PM, Sven Dowideit notifications@github.com wrote:

please remember the documentation changes for this :)

—
Reply to this email directly or view it on GitHub.

[SvenDowideit]

ping @erikh :)

[erikh]

@SvenDowideit gah! Remind me in the morning please. :)

[erikh]

@SvenDowideit docs should be done now.

[rhatdan]

I just submitted a pull request for a simple solution to this problem using the COMMENT field that docker inspect already looks at.

#6900

Here is the type of dockerfile that I used to test my patch

more Dockerfile

Based on the Fedora image

FROM fedora

MAINTAINER "Dan Walsh"

COMMENT {
"docs" : "http://readthedocs.org/",\
"license" : "file://LICENSE",
"product" : "ID123456789",
"policy" : "Publication or distribution policy statement",
"foo" : "bar"
}

CMD ["/usr/bin/sh"]

[{
"Architecture": "amd64",
"Author": "Dan Walsh",
"Comment": {
"docs": "\u003chttp://readthedocs.org/\u003e",
"foo": "bar",
"license": "\u003cfile://LICENSE\u003e",
"policy": "Publication or distribution policy statement",
"product": "ID123456789"
},
...

[jamtur01]

Dockerfile.

[erikh]

@jamtur01 resolved -- I also corrected the usage in the DESCRIPTION example, too.

[jamtur01]

Docs LGTM

[rhatdan]

Could someone explain what the difference is between "COMMENT" and "DESCRIPTION"? We already have "COMMENT" stored in images, although without my patch not populated form docker build.

[tiborvass]

@rhatdan Valid point.

Either @erikh can update his PR to use COMMENT instead of DESCRIPTION (with the already existing comment struct field), or @rhatdan can add an example in the docs, and a test like in erik's PR.

Closes #6900

[erikh]

Dan, I have no preference here; if you wish to have you patch merged, just let me know. If you are too busy, I can do the work required.

On Jul 11, 2014, at 4:06 PM, Tibor Vass notifications@github.com wrote:

@rhatdan Valid point.

Either @erikh can update his PR to use COMMENT instead of DESCRIPTION (with the already existing comment struct field), or @rhatdan can add an example in the docs, and a test like in erik's PR.

Closes #6900

—
Reply to this email directly or view it on GitHub.

[rhatdan]

I can do the work, What do I need to do to get the pull request merged?

[SvenDowideit]

@tiborvass this PR has a test, has docs and works - The only extra touch might be to add to /docs/man/docker.5.md ?

[tianon]

I'd certainly appreciate if we got some love in contrib/syntax for new instructions, too 👼

[erikh]

I don’t want to be trouble, but can we decide on which one to accept before we add anything to them? I’m sure @rhatdan will appreciate it as well.

On Jul 12, 2014, at 6:34 PM, Tianon Gravi notifications@github.com wrote:

I'd certainly appreciate if we got some love in contrib/syntax for new instructions, too

—
Reply to this email directly or view it on GitHub.

[tianon]

Yeah absolutely, sorry I wasn't more clear; I just meant that whichever one
goes forward ought to include syntax highlighting updates. :)

[rhatdan]

I added to my pull a syntax/highlighting patch to match MAINTAINER

[SvenDowideit]

@rhatdan did you need to push? (can you throw me an ssh key to grab it from your notebook :))

[SvenDowideit]

I personally prefer DESCRIPTION over COMMENT - as # is a comment already

[rhatdan]

If we wanted to change to Description from Comment, that would be fine with me. I just don't think we should support both within the image JSON, since they have big overlap. We have people at Red Hat who would like to have a "Vendor" flag for similar types of information.

I am preparing another patch set that will allow you to apply comments/descriptions to an Image at docker import time.

cat IMAGE.tag | docker import -c "This is a RHEL7.0 Image" - TAG

So we can get something better then "imported from -" as the comment describing the base images.

Or better yet

cat IMAGE.tag | docker import -c ''{"RELEASE"="RHEL", "VERSION"=7, "BUILDTAG": "..." }' - TAG

[tiborvass]

@SvenDowideit I agree with you. The problem is that image == layer in Docker. And a layer already has a comment field which becomes exposed as Comment in the inspect API for an image. So the question is do we want to duplicate Comment and Description in the API? I don't think so.

[erikh]

I agree with this, but I will admit that @rhatdan’s patch has different goals — one to encapsulate json in the response if I understand correctly.

Rubygems has a similar struct element called “metadata” that allows users to embed arbitrary metadata in the gem. Perhaps renaming ‘COMMENT’ to ‘METADATA’ would solve the use-case for both situations and be more semantically appropriate.

This might be a @shykes call, though.

-Erik

On Jul 15, 2014, at 7:00 AM, Tibor Vass notifications@github.com wrote:

@SvenDowideit I agree with you. The problem is that image == layer in Docker. And a layer already has a comment field which becomes exposed as Comment in the inspect API for an image. So the question is do we want to duplicate Comment and Description in the API? I don't think so.

—
Reply to this email directly or view it on GitHub.

[aweiteka]

@erikh +1. Call it what it is: METADATA. However I think the @rhatdan patch is backward compatible since COMMENT was partially implemented.

[tiborvass]

Commenting on a layer, comes from the git workflow with docker commit. Hence the name "comment". Its purpose is not to hold JSON. That should be solved with something else; whether METADATA is the answer, I don't know.

All I know is that we need to maintain backwards compatibility with Comment in the API.

[rhatdan]

And just for consistency it is called --message :^)
man docker-commit
...
-m, --message="" Commit message

I have no problem using my patches to fix the missing parts of setting the "Comment" message to free text and then adding the "Description/MetaData/Vendor" Patch to allow us to write extended messages into the images json file.

[tiborvass]

@rhatdan lol you're right about that. Let's stick with Comment wdyt @SvenDowideit ?

[SvenDowideit]

@tiborvass mostly, I'm waiting impatiently for one of them to get merged :)

[rhatdan]

I pushed a new "Comment" patch which adds the ability for docker import to have a --message flag which updates the comment flag. This functionality matches the docker commit --message flag.

I believe the way forward then is to add the Comment patch and then redo a patch to implement MetaData.

[SvenDowideit]

+1

[tiborvass]

@erikh I'm closing this in favor of #6900. We can continue the discussion on that one.

[erikh]

Sure

On Sep 23, 2014, at 5:52 PM, Tibor Vass notifications@github.com wrote:

@erikh I'm closing this in favor of #6900. We can continue the discussion on that one.

—
Reply to this email directly or view it on GitHub.