osbuild / images

Image builder image definition library
Apache License 2.0
21 stars 49 forks source link

osbuild-built images often get wrong 'isfinal' in buildstamp (so pre-release warnings are shown when they should not be, or vice versa) #515

Open AdamWill opened 7 months ago

AdamWill commented 7 months ago

Fedora pre-release images are meant to show some warnings of their pre-release status. They should show a red "PRE-RELEASE / TESTING" text at top right, under the release version. Like this, from the livemedia-creator-created image:

pr

But the osbuild-built image does not show that warning:

nopr

There's some subtlety to how exactly anaconda decides whether to show the warning that I'm too tired to remember right now. I'll try and remember it tomorrow and add it as a comment. CCing @nirik for the releng angle here (I know for at least one case we have to call a tool with a special argument to mark images as pre-release or not...)

supakeen commented 7 months ago

Thanks, I'll also cc @jkonecny12 on this as they might know how to set the pre-release.

I'd have expected that to live in fedora-release maybe?

AdamWill commented 7 months ago

well, yes, one of the paths is that it gets shown if fedora-release has a release (in RPM terms) of less than 1. But I don't remember if that's the one that applies to live images, or exactly how that works in detail. There must be something more tricky to cause it to show on the livemedia-created image but not on the osbuild-created one. I'll try and figure it tomorrow if I get time (I'm at a conference ATM).

AdamWill commented 7 months ago

OK, so I looked it up again. The logic happens here, where we look for a buildstamp file in a couple of places and fall back on environment variables if we can't find it.

The path that runs through the fedora-release version is here - we find the thing that provides system-release (this is an abstraction mechanism for rebuilds of Fedora, so they can have a (somethingelse)-release package that provides system-release), and set ANACONDA_ISFINAL - one of the environment vars that the logic code reads - based on it.

This is making me remember that, the other day, I noticed that Cloud images built with Kiwi didn't have fedora-release on them at all (IIRC). I think nothing actually requires system-release very hard any more, and it's entirely possible to build an image which doesn't have it. That might be what's going on here - we have no fedora-release on the osbuild-built live, so this breaks down. I'll see if I can find a manifest to check.

jkonecny12 commented 6 months ago

Yes, it's in buildstamp file created by Lorax or on Live as mentioned above by Adam it's decided in the liveinst script.

AdamWill commented 6 months ago

Live images don't have that file, though, which is why we have that other path in liveinst.

AdamWill commented 6 months ago

EDIT EDIT okay, this is more mysterious than I thought, because:

liveuser@localhost-live:~$ rpm -q --whatprovides system-release
fedora-release-workstation-40-0.36.noarch
liveuser@localhost-live:~$ 

So, digging deeper. But keeping the comment for the note below.

I'm also a bit confused, because the image has no fedora-release package:

liveuser@localhost-live:~$ rpm -q fedora-release
package fedora-release is not installed

But if I go to the task that built that image and click on x86_64-live-installer.manifest.json, the list there does include the fedora-release package. Similarly the list under metadata->anaconda-tree->org.osbuild.rpm->packages in x86_64-live-installer.log.json does include fedora-release.

Do neither of those actually indicate what packages are included in the live image? If so, is there anywhere that does tell me what packages are included in the built image? livemedia-creator does provide this information.

AdamWill commented 6 months ago

Oh, okay, so the problem is quite "simple": it looks like osbuild does write a buildstamp file into live images, unlike livemedia-creator, so we never fall back on the environment variable.

liveuser@localhost-live:~$ cat /.buildstamp 
[Main]
product = Fedora
version = 40
isfinal = True
uuid = 202403170615.x86_64
variant = Workstation

[Compose]
osbuild = devel

we just need to make that say isfinal = False for builds before the Final RCs. However, this seems to be hardcoded into osbuild as true, here and here. I'll see if I can do anything about that...

supakeen commented 6 months ago

If you want me to I can pick this up as well @AdamWill just let me know :)

AdamWill commented 6 months ago

well, you'd probably do it faster but I'd learn more...:P

AdamWill commented 6 months ago

well, I may know more about the other end of this, if it helps with the design. Pungi has the concept of whether the compose is "supported", which can be manually set but which in fact we rely (I think) on being set automatically, by this bit of magic. Basically it gets set for composes with a label like RC-x.y, which is Final candidate composes.

It then uses that property in various paths. e.g. in the buildinstall and ostree_installer phases, it passes a --isfinal arg to lorax if it's True, which causes lorax to set this value as True in the buildstamp file. On the createiso and extra_isos phases, it winds up passing a --supported-iso arg to /usr/bin/implantisomd5 (I'm not sure what that does, but it doesn't really matter).

We probably want to follow that same logic here, if possible; the osbuild phase should pass that compose.supported property to osbuild somehow - via the opts passed to koji.koji_proxy.osbuildImage I guess? - and osbuild should use that to decide what to write into the buildstamp file.

AdamWill commented 6 months ago

So, there's a lot of 'figuring out what sends what to what' to do if you don't already know it, but so far I think I want to add this to koji-osbuild:

diff --git a/plugins/builder/osbuild.py b/plugins/builder/osbuild.py
index db5be9f..c9c752a 100644
--- a/plugins/builder/osbuild.py
+++ b/plugins/builder/osbuild.py
@@ -112,10 +112,11 @@ class Repository:

 class ImageRequest:
-    def __init__(self, arch: str, image_type: str, repos: List):
+    def __init__(self, arch: str, image_type: str, repos: List, final: bool):
         self.architecture = arch
         self.image_type = image_type
         self.repositories = repos
+        self.final = final
         self.ostree: Optional[OSTreeOptions] = None
         self.upload_options: Optional[Dict] = None

@@ -126,7 +127,8 @@ class ImageRequest:
             "image_type": self.image_type,
             "repositories": [
                 repo.as_dict(arch) for repo in self.repositories
-            ]
+            ],
+            "final": self.final
         }
         if self.ostree:
             res["ostree"] = self.ostree.as_dict(self.architecture)

then figure out what bit of osbuild picks up those requests and ultimately get that passed through as a property of the types defined in osbuild-images pkg/image/*.go (e.g. AnacondaLiveInstaller)...

AdamWill commented 6 months ago

Good grief, I don't mean to be rude but the chain of image types creating other image types creating other image types is a bit of a stretch to wrap your head around here, trying to implement this...

The list of open files in my text editor trying to work this out:

image

AdamWill commented 6 months ago

So I got some way through the maze here but I think I'm gonna wave a white flag and ask @supakeen to do this, it's probably better for me to learn by looking at what he does. Sorry, Simon :)

supakeen commented 6 months ago

It's OK; as we discussed on Matrix I'll likely initially do a similar thing to what I did for ISOLabel's and then we can figure out after how we're going to get this from koji into images through osbuild-composer's APIs.

AdamWill commented 6 months ago

So, to recap from discussion in https://github.com/osbuild/images/pull/522 and elsewhere, this is a hard problem to solve at the osbuild level. Let me recap just the different cases I can remember in Fedora:

The way this works for lorax-built installer images is that pungi has a heuristic which decides that, if the compose has a label that starts 'RC-', 'Update-', or 'SecurityFix-', then it's a "supported" compose, and it passes --isfinal to lorax. This produces the correct behaviour for mainline Fedora composes (and I guess RHEL composes, I don't know), but not for IoT, because IoT composes always have 'RC-' labels, so they never get pre-release warnings even if they clearly are pre-releases.

I think the best thing for osbuild to do here would be to provide a convenient way to declare an isfinal value at "run time", like lorax does. Then we can pass it in from pungi however we deem most appropriate.

My best idea to fix the IoT problem at pungi level is to make it possible to override the heuristic in pungi config (maybe this is already possible, I haven't checked). Then IoT could just explicitly set it to the appropriate value in its pungi config. We could do the same for the main compose, or just keep relying on the heuristic there, as it does work.

CC @nullr0ute @pcdubs for the IoT angle here.