-
Notifications
You must be signed in to change notification settings - Fork 73
scripts/build-image: use libosinfo to detect installer (HMS-9801) #2063
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
base: main
Are you sure you want to change the base?
Conversation
mvo5
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.
Nice!
|
Needs linter appeasement. |
|
Python test script linting isn't a required check, but let's be clean here and fix them before merging :) |
4fabbfe to
2aa9d3e
Compare
|
Fixed linter, let's see if all ISOs have correct OS labels. Oh wait, since this is a change in |
|
Aha, Fedoras are not detected, our runner is likely CentOS? Need to figure out how to fetch latest and greatest libos database. |
2aa9d3e to
a13ca29
Compare
|
Ok, added the following to # latest database for libosinfo-detect checks
OSDB_DIR=$(mktemp -d -t osinfodb.XXXXXX)/osinfo-db
cleanup() {
test "$OSDB_DIR" && rm -rf "$OSDB_DIR"
}
trap cleanup EXIT INT TERM
git clone https://gitlab.com/libosinfo/osinfo-db "$OSDB_DIR"
pushd "$OSDB_DIR"
# install into $HOME/.config/osinfo
make install -j2
popd |
|
The only ISO images that fail the test are those which are images/data/distrodefs/fedora/imagetypes.yaml Line 1912 in 9bf9d1e
Actually, our ISO label is in the form of The libosinfo library makes a heuristic guess of the OS if the label does not match exactly, but the Therefore I suggest to update our ISO labels to align with the official ISO labels so that the test is passing. Alternatively, we can find our own label that will pass the heuristic check as Fedora. |
The problem is that we do, indeed, not know what is in the ISO (in the case of Looking at that list I assume it also doesn't really know the Fedora KDE, or any Fedora spins ISOs or how does that work? Is it just about the ordering of the fields or do we need to match those exactly? |
a13ca29 to
881f162
Compare
Yeah but does it matter for the ISO label and detected OS for virtualization? Typically, this is to pickup correct drivers and find kernel location, which is always the same for all variants. Let me know to set
The heuristic behavior is a bit cryptic, I could not figure out how exactly it works from the code. |
What do you mean by this? I think we've already established that when using
I'll try out some things :) |
|
This doesn't seem to function based on volume id or application ID, e.g. if I take a Then and Since it's now entirely seen as unbootable perhaps I fiddled up my I've also fiddled with the publisher ID and some other fields. |
|
Ok, my previous comment was wrong because I was holding |
|
Right, so we're actually matching Alternatively we can set the id to something that matches Note that all of this only works for x86_64, here's (for example) the output on an official Fedora aarch64 ISO: And Fedora Server's x86 network installer 'lies' and labels itself as This is messy stuff. It also detects the aarch64 live installer as x86_64: |
|
I like the idea of relying on <?xml version="1.0"?>
<libosinfo version="0.0.1">
<!-- Licensed under the GNU General Public License version 2 or later.
See http://www.gnu.org/licenses/ for a copy of the license text -->
<os id="http://fedoraproject.org/fedora/unknown">
<short-id>fedora-unknown</short-id>
<name>Fedora</name>
<family>linux</family>
<distro>fedora</distro>
<upgrades id="http://fedoraproject.org/fedora/rawhide"/>
<derives-from id="http://fedoraproject.org/fedora/rawhide"/>
<release-status>prerelease</release-status>
<media arch="x86_64">
<iso>
<volume-id>Fedora-.*-dvd-x86_64-(4[2-9]|[5-9][0-9])</volume-id>
</iso>
<kernel>images/pxeboot/vmlinuz</kernel>
<initrd>images/pxeboot/initrd.img</initrd>
</media>
<media arch="x86_64" live="true">
<iso>
<volume-id>Fedora-.*-Live-(4[2-9]|[5-9][0-9]).*</volume-id>
</iso>
<kernel>images/pxeboot/vmlinuz</kernel>
<initrd>images/pxeboot/initrd.img</initrd>
</media>
<tree arch="x86_64">
<treeinfo>
<family>Fedora</family>
<version>(4[2-9]|[5-9][0-9])</version>
<arch>x86_64</arch>
</treeinfo>
</tree>
<resources arch="all">
<minimum>
<n-cpus>1</n-cpus>
<cpu>2000000000</cpu>
<ram>2147483648</ram>
<storage>16106127360</storage>
</minimum>
<recommended>
<ram>4294967296</ram>
<storage>21474836480</storage>
</recommended>
</resources>
</os>
</libosinfo>Edit: Looks really like an oversight on their side because RHEL has all arches. Let me do a patch into the DB. |
We'd also need to get new version in Fedora, CentOS and RHEL as they all have versions of about a year or two old which don't know any other things otherwise the end result for users is the same (but not for our tests?). |
Let's backport this into CentOS Stream 9 and 10 perhaps? I can do that. Maybe this database is subject of regular rebasing even. Let me try to use the proposed XML change and update our labels to match it to see if that actually works. |
881f162 to
3dc8293
Compare
|
How about this, reworded the commit message, simplified it quite a bit. Let's just do this for intel and add a patch that will land later to fix this on all arches. |
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.
The change to ISO labels here will need to be approved by the respective owners of those artifacts and perhaps QA and releng (I don't know if they use ISO labels anywhere for the compose or to identify media; hopefully not) since we produce these in Fedora's compose process before this can be merged. I'll request changes based on that and then you can reach out to them.
I'd much prefer this to be the other way around and to teach libosinfo (or I guess osinfo-db) to understand our labels but that might be excessively hard.
Can you elaborate on this? Not sure what you mean by requesting changes. |
I set my review status on this PR to 'request changes' which means it can't be merged without dismissing my comment which hopefully makes sure we don't accidentally change ISO labels on Fedora artifacts without informing/asking the people who own/test these iamges :) |
When using virt-install --location disk.iso the libosinfo library is used to detect operating system and path to kernel and initramdisk on the ISO image. This is done by comparing disk label against its database of known OSes. While most of the images I tested do work, to fully ensure this is the case and to prevent regressions, let's run this tool after image is built in case it is an ISO. For unknown OS versions, the database contains "unknown" entries which are still recognized as unreleased versions, so this should work even if our runners will have older version of the library. This works for RHEL, but not for Fedora where we must use "dvd" suffix. It also does not work for non-x86_64 architectures so added a workaround until this is fixed in the libosdb. Signed-off-by: Lukas Zapletal <[email protected]>
3dc8293 to
385a9bb
Compare
When using virt-install --location disk.iso the libosinfo library is used to detect operating system and path to kernel and initramdisk on the ISO image. This is done by comparing disk label against its database of known OSes.
While most of the images I tested do work, to fully ensure this is the case and to prevent regressions, let's run this tool after image is built in case it is an ISO.
For unknown OS versions, the database contains "unknown" entries which are still recognized as unreleased versions, so this should work even if our runners will have older version of the library.
Closes: https://issues.redhat.com/browse/HMS-9801