rhcos: add info box with RPM diff command if necessary #648
Conversation
This file contains hidden or 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
Found this useful in local hacking.
Just leave some more context for future git archeologists.
As part of openshift/enhancements#1637, the version string for RHCOS will change from being OCP+RHEL-based (e.g. 419.96...) to being purely RHEL-based (e.g. 9.6...). Adapt the logic for this new scheme so that it links to the right stream. We should eventually clean this up though so that the stream name is available more directly to the release controller so that it doesn't need to do any guessing.
bradmwilliams
requested changes
Jan 27, 2025
Collaborator
bradmwilliams
left a comment
bradmwilliams
left a comment
There was a problem hiding this comment.
Changes look good. Just one little nit. Go ahead and squash your commits and I'll approve.
As part of openshift/enhancements#1637, the CoreOS pipeline now only builds a RHEL-only RHCOS base image and later on, a node image is built on top of this base image to add all the OCP-specific packages. As a result, the RHCOS release browser will only display the diff of the _base_ image content, and will not have any OCP content. Often, that's sufficient. E.g. if you're just interested in kernel or systemd changes, the RHCOS release browser is enough. However, users can be confused by the lack of OCP packages in the list. Let's add an info box to the changelog page with instructions to generate a full diff. But only display it if one of the RHCOS versions is of the new RHEL-only kind. As a bonus, these instructions also conveniently serve as a way to get any diff at all without VPN access. Of course, being able to generate this diff ourselves and rendering it would be useful. And such a mechanism need not be specific to the CoreOS image; any of the many OCP images we ship which contain RPMs would benefit from being able to view package diffs. The most likely candidate for implementing this would be in `oc adm release info`, but downloading images to generate diffs is a much more expensive operation than the git changelog-based one. So a caching service might be better instead. (That said, it's possible with Konflux that we'll end up storing RPM lockfiles in git, in which case an RPM diff _is_ a git diff which matches nicely the existing semantics.)
Member
Author
|
Thanks, updated!
Hmm, each commit has a different rationale/message. I'd rather keep them all if possible. But if you'd really like, I can squash them and merge the commit messages. |
jlebon
force-pushed
the
pr/layered-node-img
branch
from
c66a3dd to
79a6133
Compare
bradmwilliams
approved these changes
Jan 27, 2025
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bradmwilliams, jlebon 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 |
openshift-ci
bot
added
the
approved
Contributor
|
@jlebon: The following tests 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. |
jlebon
added a commit
to jlebon/release-controller
that referenced
this pull request
Jan 28, 2025
This basically fixes the gap that the infobox added in openshift#648 warns about. Instead of relying on the CoreOS release browser differ, we directly query for the diff using `oc adm release info`, which recently learned to do this, and render it at the bottom of the page. This removes the VPN requirement on being able to see the RPM diff. In the case of the new layered node images, replace the diff link to instead link to this new rendered section. We could in theory always link to that, but let's be conservative. This provide a natural rollout of the feature in case it needs ironing out. Implementation-wise, we don't want to block the main changelog render on getting the RPM diff. So we structure this the same way as the changelog render itself; if the `oc` command takes too long, we display a nice message to check again later. Once the command finishes and the output is cached, we include it in the following render requests.
jlebon
added a commit
to jlebon/release-controller
that referenced
this pull request
Jan 29, 2025
This basically fixes the gap that the infobox added in openshift#648 warns about. Instead of relying on the CoreOS release browser differ, we directly query for the diff using `oc adm release info`, which recently learned to do this, and render it at the bottom of the page. This removes the VPN requirement on being able to see the RPM diff. In the case of the new layered node images, replace the diff link to instead link to this new rendered section. We could in theory always link to that, but let's be conservative. This provide a natural rollout of the feature in case it needs ironing out. Implementation-wise, we don't want to block the main changelog render on getting the RPM diff. So we structure this the same way as the changelog render itself; if the `oc` command takes too long, we display a nice message to check again later. Once the command finishes and the output is cached, we include it in the following render requests.
jlebon
added a commit
to jlebon/release-controller
that referenced
this pull request
Feb 24, 2025
This basically fixes the gap that the infobox added in openshift#648 warns about. Instead of relying on the CoreOS release browser differ, we directly query for the diff using `oc adm release info`, which recently learned to do this, and render it at the bottom of the page. The `diff` link now simply links to that section of the page. This removes the VPN requirement on being able to see the RPM diff. Only do this for the new layered node images, as well as for 4.19. We could in theory do this all the way back to e.g. 4.12 when RHCOS switched to "OSTree native containers", but let's be conservative. This provide a natural rollout of the feature in case it needs ironing out. Implementation-wise, we don't want to block the main changelog render on getting the RPM diff. So we structure this the same way as the changelog render itself; if the `oc` command takes too long, we display a nice message to check again later. Once the command finishes and the output is cached, we include it in the following render requests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
As part of openshift/enhancements#1637, the CoreOS pipeline now only builds a RHEL-only RHCOS base image and later on, a node image is built on top of this base image to add all the OCP-specific packages.
As a result, the RHCOS release browser will only display the diff of the base image content, and will not have any OCP content.
Often, that's sufficient. E.g. if you're just interested in kernel or systemd changes, the RHCOS release browser is enough. However, users can be confused by the lack of OCP packages in the list.
Let's add an info box to the changelog page with instructions to generate a full diff. But only display it if one of the RHCOS versions is of the new RHEL-only kind.
As a bonus, these instructions also conveniently serve as a way to get any diff at all without VPN access.
Of course, being able to generate this diff ourselves and rendering it would be useful. And such a mechanism need not be specific to the CoreOS image; any of the many OCP images we ship which contain RPMs would benefit from being able to view package diffs.
The most likely candidate for implementing this would be in
oc adm release info, but downloading images to generate diffs is a much more expensive operation than the git changelog-based one. So a caching service might be better instead.(That said, it's possible with Konflux that we'll end up storing RPM lockfiles in git, in which case an RPM diff is a git diff which matches nicely the existing semantics.)
For a sample of what this looks like (forcing it on even though it wouldn't normally show in this case):