-
Notifications
You must be signed in to change notification settings - Fork 426
COS-3023: pkg/cli/admin/release/info: support generating RPM diffs #1966
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
Conversation
|
/uncc @deads2k @atiratree @Prashanth684 |
wking
left a comment
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.
I left a bunch of thoughts inline, and this kind of thing is complicated enough that there's probably quite a bit of room for polish and hardening and all of that. But having this functionality land at all, even if it has some rough edges, would provide a big release-controller UX win. And it seems niche enough that I'm not expecting customers to be poking around with it and complaining about rough edges (but if they do, that's at least giving us trackers to work on polishing). So I'm approving as you have it, and also holding in case you want to do any more polishing now. Feel free to lift the hold when you're ready and ping me for a fresh LGTM if necessary.
/lgtm
/hold
f4f6685 to
830353e
Compare
Capture the image tags which had component version information in them. Prep for future patch.
This is mostly a code move. No functional change otherwise. Prep for a future patch where we'll want to call this bit from a different place in the code.
By default, we fetch all images. But support passing a set of only some tags we want fetched. Prep for future patch.
It's often useful when looking up release images to know the list of RPM packages that shipped in the node image. Add new switches for this: - `oc adm release info --rpmdb $IMG` will list all the packages in the node image for the given release image payload - `oc adm release info --rpmdb-diff $IMG1 $IMG2` will diff the set of packages in the node image for the given release image payloads The code is generic over the actual target image. By default, the node image is used, but `--rpmdb-image` can be used to select a different one. The primary motivation for this is openshift/enhancements#1637, in which the node image will no longer be built within the CoreOS pipeline as a base image. Instead, it will be a layered image built in OpenShift CI/Konflux. As a result, all layered packages will not show up in the CoreOS release browser differ. With this functionality, the release controller will be able to render RPM diffs in the web UI, greatly de-emphasize the CoreOS differ and effectively dropping the requirement for having VPN access. Some notes on the implementation: - The rpmdb for a given image is cached, keyed by the image digest. - The new layered node image in specific supports a "metadata layer" semantic in which the last layer of the image is a metadata-only layer describing the RPM contents. If we detect this, we use it. This saves us from having to download the whole image. - Otherwise, we don't try to be smart here and e.g. only download some layers. There are some issues with doing that. We literally do download the full image, _but_ we only cache the rpmdb content and throw away the rest. That said, the high cost isn't an issue in practice because the release controller can nicely represent operations which take time so it didn't feel worth the effort of trying to optimize this further. Once we have SBOMs available for all our images, this should be a more canonical way to cheaply query the RPM contents of an image. Additionally/alternatively, for the node image specifically, if we ever end up with lockfiles in the git repo, this would effectively mean that the git changelog _is_ the RPM changelog also, meshing nicely with the existing infrastructure around that.
|
OK updated this now! The biggest change is that we now detect if the image implements the new "metalayer semantic" now in use in layered node images (though the mechanism is generic and could be implemented in other images). This makes retrieving the package list for a node image vastly cheaper and in a cross-platform way (i.e. without an RPM dependency). That said, we still also keep the rpmdb fallback to handle the case where a node image had more layers added and also to allow the release-controller to provide diffs between older releases and 4.19. (But also because I still believe in having this code work across all container images, and not just the node image.) |
|
@jlebon: This pull request references COS-3023 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/hold cancel |
|
/retest |
1 similar comment
|
/retest |
petr-muller
left a comment
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.
Let's move this forward, Trevor reviewed and was fine to merge this, Brad tested, if we subtly break something it's more likely to be discovered after merge than by further human scrutiny before that, and we can always revert or fix forward.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlebon, petr-muller, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@jlebon: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1719de8
into
openshift:master
|
[ART PR BUILD NOTIFIER] Distgit: ose-tools |
|
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-cli |
|
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-deployer |
|
[ART PR BUILD NOTIFIER] Distgit: ose-cli-artifacts |
It's often useful when looking up release images to know the list of RPM packages that shipped in the node image. Add new switches for this:
oc adm release info --rpmdb $IMGwill list all the packages in the node image for the given release image payloadoc adm release info --rpmdb-diff $IMG1 $IMG2will diff the set of packages in the node image for the given release image payloadsThe code is generic over the actual target image. By default, the node image is used, but
--rpmdb-imagecan be used to select a different one.The primary motivation for this is
openshift/enhancements#1637, in which the node image will no longer be built within the CoreOS pipeline as a base image. Instead, it will be a layered image built in OpenShift CI/Konflux. As a result, all layered packages will not show up in the CoreOS release browser differ.
With this functionality, the release controller will be able to render RPM diffs in the web UI, greatly de-emphasize the CoreOS differ and effectively dropping the requirement for having VPN access.
Some notes on the implementation:
Once we have SBOMs available for all our images, this should be a much cheaper way to query its RPM contents. Additionally/alternatively, for the node image specifically, if we ever end up with lockfiles in the git repo, this would effectively mean that the git changelog is the RPM changelog also, meshing nicely with the existing infrastructure around that.