PR#1707: Improve autodetection of productmd image type for osbuild images - pungi

pungi

#1707 Improve autodetection of productmd image type for osbuild images

Merged 5 months ago by lsedlar. Opened 5 months ago by adamwill.

adamwill/pungi osbuild-productmdtype-autodetect-improve into master

Improve autodetection of productmd image type for osbuild images

Adam Williamson • 5 months ago

aa7fcc1

pungi/phases/osbuild.py

file modified

+14 -4

		`@@ -252,10 +252,20 @@`

		`# Get the manifest type from the config if supplied, otherwise we`
		`# determine the manifest type based on the koji output`
		`- img.type = config.get(`
		`- "manifest_type",`
		`- archive["type_name"] if archive["type_name"] != "iso" else "dvd",`
		`- )`
		`+ img.type = config.get("manifest_type")`
		`+ if not img.type:`
		`+ if archive["type_name"] != "iso":`
		`+ img.type = archive["type_name"]`
		`+ else:`
		`+ fn = archive["filename"].lower()`
		`+ if "ostree" in fn:`
		`+ img.type = "dvd-ostree-osbuild"`
		`+ elif "live" in fn:`
		`+ img.type = "live-osbuild"`
		`+ elif "netinst" in fn or "boot" in fn:`
		`+ img.type = "boot"`
		`+ else:`
		`+ img.type = "dvd"`

		`img.format = suffix`
		`img.path = os.path.join(rel_image_dir, archive["filename"])`

adamwill commented 5 months ago

I don't love inferring the type from the filename like this -
it's kinda backwards - but it's an improvement on the current
logic (I don't think 'dvd' is ever currently the correct value
here, I don't think osbuild can currently build the type of
image that 'dvd' is meant to indicate). I can't immediately see
any better source of data here (we could use the 'name' or
'package_name' from 'build_info', but those are pretty much
just inputs to the filenames anyway).

Types that are possible in productmd but not covered here are
'cd' (never likely to be used again in Fedora at least, not sure
about RHEL), 'dvd-debuginfo' (again not used in Fedora, may be
used in RHEL), 'ec2', 'kvm' (not sure about those), 'netinst'
(this is a synonym for 'boot', we use 'boot' in practice in
Fedora metadata), 'p2v' and 'rescue' (not sure).

Signed-off-by: Adam Williamson awilliam@redhat.com

adamwill commented 5 months ago

Note, I think ideally we'd add 'boot-osbuild' and 'dvd-osbuild' types to productmd and autodetect those instead of the 'canonical' types, for safety. In a glorious future where everything is built by osbuild and it should own the 'canonical' types, we can easily change this autodetection. (Assuming osbuild is ever intended to build traditional installer images, of course).

adamwill commented 5 months ago

@supakeen for info

pbrobinson commented 5 months ago

It can build anaconda installer ISOs, is that a DVD class device?

adamwill commented 5 months ago

'dvd' is the style of installer image which contains an RPM repository on the image itself, and defaults to that as the installation source. Like the Server DVD or the old Everything DVD.

lsedlar commented on line 19 of pungi/phases/osbuild.py 5 months ago

The latter two ifs need to be elif.

lsedlar commented 5 months ago

Getting the correct type is rather difficult here. I don't know if this heuristic is bullet proof, but it certainly seems better than the previous one.

It's possible to specify the type via the config file (manifest_type), which may be a better approach.

adamwill commented 5 months ago

The latter two ifs need to be elif.

well, they don't need to be - I don't think any of the strings actually occur together in real lif - but it would probably be safer, yeah.

I know about manifest_type, I sent PRs for that at the same time - https://pagure.io/fedora-iot/pungi-iot/pull-request/32 and https://pagure.io/fedora-iot/pungi-iot/pull-request/33 . Honestly, though, neither of these quite...'feels' right? Like, it doesn't seem like someone should have to explicitly enter the correct productmd image type in the pungi config, but inferring it from the filename also doesn't seem right. I kinda want to look at how this works for the existing workflow. I feel like there must be a better place for this to happen, somehow. Logically speaking there must be a point somewhere in the process where something knows "this thing I am building is an ostree installer image", and that should be the point where this gets set (if it's not manually overridden...)