Add more information to meta.json in buildah path for backwards compatibility
#4281
Conversation
…} labels These labels contain information about the git repo that was built.
That key traditionally represented git information about the source config repo that was built. But now in the buidah path, the source of truth for this information has moved to the standard OCI labels. But for convenience and because a bunch of things check for this in the old place (notably the build browser and the release job), just proxy that info under the same key into `meta.json` as before. This should make the "FCOS commit" key show up in the build browser.
That key contains information about the cosa container image itself being used. There's some value in capturing this even just for knowing what `cosa import` code was executed, but it's also much more important when we talk about the disk images, which still heavily rely on cosa code. This should make the "COSA commit" key show up in the build browser.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds git source and revision information as OCI labels during the buildah build process and then reads these labels during import to populate meta.json for backward compatibility. The changes are logical and well-implemented. I have a couple of suggestions to improve code clarity and maintainability.
| # For the source: check if there's only one remote, if so use it with get-url | ||
| # For revision: rev-parse |
There was a problem hiding this comment.
These comments are a bit confusing as they describe the logic within prepare_git_artifacts, which has already been executed. They don't seem to apply to the buildah build command that follows. Removing them would improve the script's clarity.
| source = metadata.get('Labels', {}).get('org.opencontainers.image.source') | ||
| commit = metadata.get('Labels', {}).get('org.opencontainers.image.revision') |
There was a problem hiding this comment.
To avoid calling metadata.get('Labels', {}) twice and to improve readability, you can store the Labels dictionary in a variable first.
| source = metadata.get('Labels', {}).get('org.opencontainers.image.source') | |
| commit = metadata.get('Labels', {}).get('org.opencontainers.image.revision') | |
| labels = metadata.get('Labels', {}) | |
| source, commit = labels.get('org.opencontainers.image.source'), labels.get('org.opencontainers.image.revision') |
We need the stream name there. It was previously getting that from the git branch name, but we don't populate that anymore in the buildah path, only the git URL and commit. But we do have the more explicit stream name as a label anyway, so let's use that.
dustymabe
left a comment
dustymabe
left a comment
There was a problem hiding this comment.
LGTM.
I'd like to also restore being able to see diffs and package list s in the release browser..
Also, just saw this today: coreos/fedora-coreos-tracker#2014
| 'commit': commit, | ||
| } | ||
|
|
||
| # add a reference to ourselves as well |
There was a problem hiding this comment.
I like the comment about this from cmd-build. For some reason it makes it a little more clear to me where the information is being sourced from:
# And the build information about our container, if we are executing
# from a container.
There was a problem hiding this comment.
Will tweak in the next patch set.
Yeah agree. I guess that's https://gitlab.com/fedora/bootc/tracker/-/issues/65 for the more generalized issue. But we could carry the CoreOS-specific implementation of this for now. |
2 participants
See individual commit messages.