-
Notifications
You must be signed in to change notification settings - Fork 12
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
Heroku-CNB Image Relationships in Mermaid #437
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
```mermaid | ||
--- | ||
title: Heroku-CNB Image Relationships | ||
--- | ||
graph TB | ||
subgraph HIF["Heroku 22 Image Family"] | ||
direction LR | ||
HIF_CNB_BUILDER("CNB Builder Image\n heroku/builder:22") --> HIF_CNB_BUILD | ||
HIF_CNB_BUILD("CNB Build Stack Image\n heroku/heroku:22-cnb-build") --> HIF_H_BUILD | ||
HIF_H_BUILD("Heroku Build Stack Image\n heroku/heroku:22-build") --> HIF_H_RUN | ||
HIF_CNB_RUN("CNB Runtime Stack Image\n heroku/heroku:22-cnb") --> HIF_H_RUN | ||
HIF_H_RUN("Heroku Runtime Stack Image\n heroku/heroku:22") --> HIF_BASE | ||
HIF_BASE("Ubuntu Base Image\n ubuntu:22.04") | ||
end | ||
|
||
subgraph CNB["Cloud Native Buildpack Concepts"] | ||
direction LR | ||
subgraph CNB_BUILDER_IMG["builder image"] | ||
direction TB | ||
CNB_BUILDER_IMG_BP1("buildpack 1") --> CNB_BUILDER_IMG_BP2 | ||
CNB_BUILDER_IMG_BP2("buildpack 2") --> CNB_BUILDER_IMG_LC | ||
CNB_BUILDER_IMG_LC("lifecycle") --> CNB_BUILDER_IMG_BUILD | ||
CNB_BUILDER_IMG_BUILD("build image") | ||
end | ||
|
||
subgraph CNB_APP_IMG["app image"] | ||
direction TB | ||
CNB_APP_IMG_BP1("bp1-provided layers") --> CNB_APP_IMG_BP2 | ||
CNB_APP_IMG_BP2("bp2-provided layers") --> CNB_APP_IMG_APP | ||
CNB_APP_IMG_APP("app layers") --> CNB_APP_IMG_RUN | ||
CNB_APP_IMG_RUN("run image") | ||
end | ||
end | ||
|
||
HIF_CNB_BUILDER o-.-o CNB_BUILDER_IMG | ||
HIF_CNB_BUILD o-.-o CNB_BUILDER_IMG_BUILD | ||
HIF_CNB_RUN o-.-o CNB_APP_IMG_RUN | ||
``` |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dzuelke I kept it linked out to a separate doc. Wasn't sure if you wanted to inlined right into the top-level readme or as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends whether we want to link to it from elsewhere... thoughts, @edmorley ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the just-landed #440, I've overhauled the README somewhat. Whilst that change was primarily motivated by the builder image deprecations (I wanted to make it easier to understand the difference between the shimmed and native image variants), it's also improved the overall explanations of the images, what run time base images they contain (previously only the build time images were listed), plus the lifecycle versions used for each.
In addition, the README now gives a brief explanation about what a builder image is, and links to the upstream docs for the rest (IMO we should leverage upstream as much as possible for our CNB documentation strategy in general).
The "What is a builder" docs the README on
main
now links to, already contains a diagram (a much prettier one than we could create in mermaid I imagine):https://buildpacks.io/docs/concepts/#what-is-a-builder
As such, I think adding another diagram here might now be redundant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, that was the original PR (#434) and was asked to switch it to Mermaid.
As someone new to CNB and the various Heroku stack images, it took some time for me to connect the dots and understand how they were all related. I made the diagram mainly for my own understanding. I got feedback from my team that it was helpful and to share it. I'll leave it up to y'all if you want to include it or not and it what form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the mixed messages - I didn't know in advance about the review that ended up being left on #434.
I agree that there's a lot to learn with CNBs - and a lot we'll need to educate users and collaborators about. The documentation parts of Heroku's migration to CNBs has not yet begun - which is why there aren't many resources at present.
We'll also need to re-evaluate our overall docs/features/bugs strategy now that we're using open standards (CNB spec, OCI images etc) instead of proprietary Heroku technologies/APIs (such as the classic buildpack API and slugs). IMO part of that will be encouraging an "upstream first" mentality - whether that be reporting bugs upstream, upstreaming any of our custom features by working with upstream via their RFC process, or helping upstream make their docs and ecosystem (eg https://registry.buildpacks.io) first class.
Now clearly, Heroku is still going to need its own CNB docs, but IMO those docs should focus on the basics, and link to upstream for the more advanced topics / concepts / specs, rather than duplicating everything from upstream and increasing maintenance burden, chance of error/drift etc.
In addition, I suspect most of Heroku's own docs will want to live on Dev Center rather than in say the README of individual repos. This is why I'm leaning towards saying it's out of scope for this repo to explain exactly what a builder is - beyond a sentence or two and a link to the upstream docs.
I guess I'm curious to know more about what was the underlying learning friction that prompted the opening of the PR?
There was a definite omission from the README until recently (the README didn't mention the build vs run image tags, and also didn't link to the upstream concepts page), which I was hoping has now been solved by #440.
One thing I can see that's not currently in the README builder table, but is in the diagram in this PR, is the mention of the base Ubuntu OS version - I actually had it in the table in an earlier draft of #440, but removed it since the table was getting too wide, and instead added the following prose:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you added in #440 was the main chunk of it, so that is definitely a welcome change. I agree we should not duplicate the CNB docs, but it was the mapping of CNB concepts to the Heroku artifacts where I felt the friction. The other piece, as you mention, was the connection between the CNB Heroku images and the non-CNB Heroku stack images (specifically because I was doing some experimentation with CNB images on the non-CNB stack image). The latter part is probably less important to end users, so if you want to close this out, that is fine with me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're planning on removing the
-cnb
base image variants with the introduction of Heroku-24, which will simplify things further.As such between that planned change, the changes already made to the README, and future docs plans, I think we should close this out for now. Happy to revisit later if needed :-)