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

Built image digest is stored in the build status #3533

Closed
wants to merge 1 commit into from

Conversation

mmilata
Copy link

@mmilata mmilata commented Jan 19, 2017

Please merge after openshift/origin#12407 is merged.

@mmilata mmilata changed the title [do not merge] Built image digest is stored in the build status Built image digest is stored in the build status Jan 23, 2017
@mmilata
Copy link
Author

mmilata commented Jan 23, 2017

@openshift/team-documentation @bparees PTAL

@vikram-redhat
Copy link
Contributor

@ahardin-rh - ptal.

@ahardin-rh ahardin-rh added this to the Future Release milestone Jan 23, 2017
=== Output Image Digest

Built images can be uniquely identified by their
link:++https://docs.docker.com/registry/spec/api/#/content-digests++[digest], which can
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the ++ before and after the URL

Copy link
Author

Choose a reason for hiding this comment

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

Without ++ asciibinder mis-parses the URL, probably because it contains #. Is there another way to escape it?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mmilata Interesting. I've not seen that before. @adellape @vikram-redhat Do you know a workaround, or do you think the ++ can be accepted? Not sure if this will impact how the docs build. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

@ahardin-rh I'm not sure of another workaround. I think only the Customer Portal build would potentially have an issue; worth checking with Lee on. @vikram-redhat Can you look into that?

I'd be inclined to merge this but mark for follow-up before it gets downstream.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vikram-redhat bump ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

@ahardin-rh - interesting. I haven't seen this issue before either. But from memory, if asciibinder is miss-parsing the URL due to the hash, you could just take the 'link' off. Can you try that and see if it fixes it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ignore my previous comment. Discussed with Lee. He has suggested using $$ instead of ++.

Built images can be uniquely identified by their
link:++https://docs.docker.com/registry/spec/api/#/content-digests++[digest], which can
later be used to
link:++https://docs.docker.com/engine/reference/commandline/pull/#/pull-an-image-by-digest-immutable-identifier++[pull the image]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Please remove the ++ before and after the URL.

link:++https://docs.docker.com/engine/reference/commandline/pull/#/pull-an-image-by-digest-immutable-identifier++[pull the image]
regardless of its current tag.

Docker and Source builds store the digest in
Copy link
Contributor

Choose a reason for hiding this comment

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

make source lowercase

Copy link
Author

Choose a reason for hiding this comment

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

There are three other instances of this in the document, I'll change them all.


Docker and Source builds store the digest in
`Build.status.output.to.imageDigest` after the image is pushed to registry.
Because the digest is computed by the registry it might not be always present,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested revision:
The digest is computed by the registry. Therefore, it may not always be present (for example, when the registry did not return a digest, or when the builder image did not understand its format).

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that sounds better:)

understand its format.

.Built image digest after successful push to registry
====
Copy link
Contributor

Choose a reason for hiding this comment

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

.Built Image Digest After a Successful Push to the Registry

@ahardin-rh
Copy link
Contributor

@mmilata Just a few comments from me. Once these changes are made and they are squashed into one commit, we'll be ready to go. Thanks!

@bparees
Copy link
Contributor

bparees commented Jan 23, 2017

technical content lgtm.

@mmilata
Copy link
Author

mmilata commented Jan 25, 2017

The ++ syntax is documented here: http://asciidoctor.org/docs/asciidoc-syntax-quick-reference/#links though I can't find it in the official asciidoc manual.

@adellape adellape mentioned this pull request Feb 14, 2017
1 task
@adellape
Copy link
Contributor

@mmilata I'm breaking up builds.adoc into multiple topics, so I'll consolidate your changes here into #3739.

@adellape
Copy link
Contributor

Closing in favor of #3772 now that #3739 has merged.

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.

None yet

5 participants