potential kernel arguments options support for all installers (HMS-8999)#1786
Conversation
ac591b5 to
65a1bcc
Compare
|
@mvo5 Why is this draft? |
195a8b9 to
5a81a0a
Compare
Mostly because I wanted to add a test and some tiny cleanups/research. But happy to un-draft. We will need consensus if this is what we want though (I think we do but idk about the others). |
| // the kernel options from the imageConfig but we only set | ||
| // those in the rawImage.OSTreeDeploymentCustomizations and | ||
| // not in the bootTreePipeline. Its unclear if we should change | ||
| // this or not. Its inconsistent with the other installers but |
There was a problem hiding this comment.
This one is actually (even more) interesting - we allow customizations.kernel for the iot-simplified-installer image type but afaict setting it has no effect(?)
There was a problem hiding this comment.
Yep, that's funky. Maybe this is from the time that we wanted blueprints to be able to apply to multiple image types even though silently discarding stuff is bad (in my opinion).
We can probably drop this from the allowed customizations as it has no effect?
There was a problem hiding this comment.
The iot/edge-simplified-installer types create a raw image from an ostree commit and then put that on an ISO with the coreos installer.
The kernel arguments get applied to the raw image:
https://github.com/osbuild/images/blob/0a3a48f4bc8ec382d7e31f69535488cbb1e27fc2/pkg/distro/generic/images.go#L842
https://github.com/osbuild/images/blob/0a3a48f4bc8ec382d7e31f69535488cbb1e27fc2/pkg/distro/generic/images.go#L397-L399
https://github.com/osbuild/images/blob/0a3a48f4bc8ec382d7e31f69535488cbb1e27fc2/pkg/manifest/ostree_deployment.go#L264-L272
There was a problem hiding this comment.
Sorry for this blunder - I think the comment itself is (largely) correct though or do you want me to clarify further and/or fix this (could be a followup) it would change existing manifest checksums iirc.
supakeen
left a comment
There was a problem hiding this comment.
Thank you, this is useful for at least all Anaconda installers as Anaconda switches behavior based on the kernel arguments present.
In particular this resolves the linked bug once we release the network installer.
5a81a0a to
88d568b
Compare
|
Sorry, had to force push because of conflicts |
88d568b to
6709cc6
Compare
|
Sorry, conflicts again, force pushed again, this time from in checksum manifests (so trivial) |
This commit re-adds the `kernel.append` customization to
the {minimal,image}-installer and also add it to the test
manifests.
This option was supported via
osbuild#1786
but when adding the blueprint options checking via the
YAML it seems this support got lost.
Sorry, I got a bit confused and thought this PR was adding support for specifying kernel args in the blueprint for installer types. |
This commit re-adds the `kernel.append` customization to
the {minimal,image}-installer and also add it to the test
manifests.
This option was supported via
osbuild#1786
but when adding the blueprint options checking via the
YAML it seems this support got lost.
This commit re-adds the `kernel.append` customization to
the {minimal,image}-installer and also add it to the test
manifests.
This option was supported via
osbuild#1786
but when adding the blueprint options checking via the
YAML it seems this support got lost.
This commit re-adds the `kernel.append` customization to
the {minimal,image}-installer and also add it to the test
manifests.
This option was supported via
#1786
but when adding the blueprint options checking via the
YAML it seems this support got lost.
ab80310 to
cceffc7
Compare
cceffc7 to
53d3b72
Compare
|
[..]
No worries, I rebased now that #1870 is merged and this should be ready again (also fixed the typo that Thozza pointed out). |
53d3b72 to
b60d339
Compare
b60d339 to
970678d
Compare
|
Had to force push because GH claimed conflicts (which git resolved without any changes from my side) |
Support for kernel options in the iso got added to the tar installer in osbuild#1727. For consistency this commit also enables it for the other installers via the pre-existing `AdditionalKernelOpts` that is part of the `manifest.InstallerCustomizations`. A helper to read the kernel options from the customizations and default image type(s) is extracted as well.
We can drop the (now) special handling of kernel args via the OSCustomizations in the tar installer. With the previous commit we use the manifest.InstallerCustomizations.AdditionalKernelOpts to handle the kernel arguments which is enough (this is also validated by the fact that the existing TestTarInstallerKernelOpts does not need to change).
Rename the manifest.InstallerCustomizations.AdditionalKernelOpts to KernelOptionsAppend to be similar to the existing OSCustomizations that also have the same job. This makes it easier to reason that both options do the same, one for the os pipeline, one for the installer pipeline(s).
This adds a missing unit test for the kernel options extraction via the installerCustomizations.
There are some stray newlines that visually distract from the error handling. This commit removes them.
This commit makes the code similar to the other installers when handling the kernelOpts options. Ideally we would have one shared helper but one step at a time.
The fedora `iot-installer` image sets default image type kernel options that were previously not honored by the image type. With the unification of the installer kernel options it is no honored which leads to a manifest diff. It is not clear if we want these kernel options or not but this commit is conservative and keeps the old behavior by just unsetting the kernel options in the "iot-installer" image options.
The OSTreeSimplifiedInstaller is not an anaconda installer
but it still uses some of the anaconda structs. So lets
move it to use the same installer customizations.
Note that we cannot use the InstallerCustomizations.KernelAppend
with it as it will lead to a diff in the rhel-9.x
edge-simplified-installer image:
```yaml
--- /tmp/tmp07t_j3mj/ref/rhel_9.7-x86_64-edge_simplified_installer-edge_ostree_pull_device.json 2025-08-26 15:10:18.721496181 +0200
+++ /tmp/tmp07t_j3mj/new/rhel_9.7-x86_64-edge_simplified_installer-edge_ostree_pull_device.json 2025-08-26 15:10:21.201572008 +0200
@@ -1100,7 +1100,10 @@
"coreos.inst.isoroot=RHEL-9-7-0-BaseOS-x86_64",
"coreos.inst.install_dev=/dev/sda",
"coreos.inst.image_file=/run/media/iso/image.raw.xz",
- "coreos.inst.insecure"
+ "coreos.inst.insecure",
+ "modprobe.blacklist=vc4",
+ "rw",
+ "coreos.no_persist_ip"
]
},
"isolabel": "RHEL-9-7-0-BaseOS-x86_64",
@@ -1259,7 +1262,10 @@
"coreos.inst.isoroot=RHEL-9-7-0-BaseOS-x86_64",
"coreos.inst.install_dev=/dev/sda",
"coreos.inst.image_file=/run/media/iso/image.raw.xz",
- "coreos.inst.insecure"
+ "coreos.inst.insecure",
+ "modprobe.blacklist=vc4",
+ "rw",
+ "coreos.no_persist_ip"
]
}
}
```
This is because the edge-simplified-installer for rhel uses
the kernel options from the base image type only for the
deployed system but does not add it to the ISO kernel commandline.
Its unclear if we should unify this or not but this commit errs
on the side of caution.
970678d to
1e5ef98
Compare
When debugging a test failure of centos9 iso install I want to use the cool new feature by Simon to add kernel arguments to the installer (c.f. https://github.com/osbuild/images/pull/1727/files) test in bib via a blueprint kernel append.
I noticed then that its currently implemented for a subset of our installers, so I created this quick draft to see what it would take and it seems straightforward. Is there a reason not to support the kargs for the missing installers? If not I'm happy to flesh this one out more, it needs tests (obviously) but I did not want to write them before there is agreement that is what we want.
[edit/reminder: the AanacondaContainerInstaller needs a coresponding change in bib to fully work]
Note that while this enables kernel.append in the lower levels there is a blueprint options check that will prevent this for e.g. edge-installer so if we want to support this for real we will need to enable it in the
checkOptions()for blueprints code (which is currently not fine-grained enough to allow setting Kernel.append only but not kernel.name and we will need #1216 first I think.