MGMT-21574: Allow rhel-coreos- as a valid volume ID prefix
#477
Conversation
RHCOS 9.6/10.1 ISOs will soon have that volume ID there. The change was done thinking it was low risk since it's mostly cosmetic but it turns out we check for this here. We could revert it, but... hopefully this patch is enough to fix things.
openshift-ci-robot
added
the
jira/valid-reference
|
@jlebon: This pull request explicitly references no jira issue. In 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. |
openshift-ci
bot
added
the
size/S
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #477 +/- ##
==========================================
+ Coverage 59.32% 59.48% +0.15%
==========================================
Files 27 27
Lines 1571 1577 +6
==========================================
+ Hits 932 938 +6
Misses 478 478
Partials 161 161
🚀 New features to boost your workflow:
|
|
If we have a published image with this prefix available can you add it to the list in integration_test/images_test.go? |
|
Also if this is something we specifically shouldn't be testing for as a way to identify these ISOs, maybe we should just remove it or switch to checking something more reliable? |
|
FWIW the original bug was https://issues.redhat.com/browse/MGMT-12740 |
Ahhh nice. We don't have images on the mirrors yet with this new prefix. But it should get there in the next bootimage bump.
Honestly it seems fine to me. Another way I guess is to check for the |
What does this mean? Is this a "real" release or some kind of prerelease? |
It will eventually land in both 4.20 and 4.19 as we bump their respective bootimage references in |
|
Okay so it sounds like we don't want to wait on this fix then until we can get an image to use in integration tests. If we want this backported (which it sounds like we really do) we'll also need a bug. I pinged the slack thread to see if the original reporter can create one. |
carbonin
changed the title
rhel-coreos- as a valid volume ID prefixrhel-coreos- as a valid volume ID prefix
|
@jlebon: This pull request references MGMT-21574 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 bug to target the "4.20.0" version, but no target version was set. In 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. |
|
/test remaining-required Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: carbonin, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/hold We're going to revert the CoreOS change that caused this issue and tackle it differently. The volume ID will be back to Should it be merged anyway? You're free to do so but don't have to. It's unlikely right now we'll change it back again to |
openshift-ci
bot
added
the
do-not-merge/hold
I think I'd rather look into a more stable way of ensuring the ISO is something we can work with. |
|
@jlebon: all tests passed! Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
In osbuild#2148, we changed the logic to generate the volume ID from the data in `/usr/lib/os-release` to sever the reliance on metadata in the embedded treefile that will no longer exist. This had no effect in FCOS, but had an effect in RHCOS, which went from a volume ID of `rhcos-...` to `rhel-coreos-...`. This was considered harmless at the time, but in fact ended up affecting the OpenShift Assisted Image Service. See e.g. openshift/assisted-image-service#477 which attempted to adapt that code. But in the end, it felt safer and less work to just revert back to the previous volume ID. So here we are. But we still don't want to go back to using the embedded treefile. Instead, we now have access to the OS name to use as a label on the container image. This label gets serialized into the aleph during the creation of the metal image (via the `org.osbuild.ostree.aleph` stage) which we have access here when mounting the metal image via loopback. So pick it up from there and use that. But in case it's missing, fallback to the previous logic rather than hard fail to make this easier to ratchet in.
This reverts commit 702c738. The live ISO in that bootimage set contains a change to the volume ID which would break the Assisted Image Service: openshift/assisted-image-service#477 We're working on pushing out new ISOs with the reverted volume ID in: coreos/coreos-assembler#4276 But for now revert this to ensure it doesn't slip into the next z-stream release.
In osbuild#2148, we changed the logic to generate the volume ID from the data in `/usr/lib/os-release` to sever the reliance on metadata in the embedded treefile that will no longer exist. This had no effect in FCOS, but had an effect in RHCOS, which went from a volume ID of `rhcos-...` to `rhel-coreos-...`. This was considered harmless at the time, but in fact ended up affecting the OpenShift Assisted Image Service. See e.g. openshift/assisted-image-service#477 which attempted to adapt that code. But in the end, it felt safer and less work to just revert back to the previous volume ID. So here we are. But we still don't want to go back to using the embedded treefile. Instead, we now have access to the OS name to use as a label on the container image. This label gets serialized into the aleph during the creation of the metal image (via the `org.osbuild.ostree.aleph` stage) which we have access here when mounting the metal image via loopback. So pick it up from there and use that. But in case it's missing, fallback to the previous logic rather than hard fail to make this easier to ratchet in.
In osbuild#2148, we changed the logic to generate the volume ID from the data in `/usr/lib/os-release` to sever the reliance on metadata in the embedded treefile that will no longer exist. This had no effect in FCOS, but had an effect in RHCOS, which went from a volume ID of `rhcos-...` to `rhel-coreos-...`. This was considered harmless at the time, but in fact ended up affecting the OpenShift Assisted Image Service. See e.g. openshift/assisted-image-service#477 which attempted to adapt that code. But in the end, it felt safer and less work to just revert back to the previous volume ID. So here we are. But we still don't want to go back to using the embedded treefile. Instead, we now have access to the OS name to use as a label on the container image. This label gets serialized into the aleph during the creation of the metal image (via the `org.osbuild.ostree.aleph` stage) which we have access here when mounting the metal image via loopback. So pick it up from there and use that. But in case it's missing, fallback to the previous logic rather than hard fail to make this easier to ratchet in.
In #2148, we changed the logic to generate the volume ID from the data in `/usr/lib/os-release` to sever the reliance on metadata in the embedded treefile that will no longer exist. This had no effect in FCOS, but had an effect in RHCOS, which went from a volume ID of `rhcos-...` to `rhel-coreos-...`. This was considered harmless at the time, but in fact ended up affecting the OpenShift Assisted Image Service. See e.g. openshift/assisted-image-service#477 which attempted to adapt that code. But in the end, it felt safer and less work to just revert back to the previous volume ID. So here we are. But we still don't want to go back to using the embedded treefile. Instead, we now have access to the OS name to use as a label on the container image. This label gets serialized into the aleph during the creation of the metal image (via the `org.osbuild.ostree.aleph` stage) which we have access here when mounting the metal image via loopback. So pick it up from there and use that. But in case it's missing, fallback to the previous logic rather than hard fail to make this easier to ratchet in.
Description
RHCOS 9.6/10.1 ISOs will soon have that volume ID there. The change was done thinking it was low risk since it's mostly cosmetic but it turns out we check for this here.
We could revert it, but... hopefully this patch is enough to fix things.
How was this code tested?
makeAssignees
/cc @carbonin
Links
Checklist
docs, README, etc)