rpmostreepayload: Set bootloader=none

[cgwalters]

This matches
coreos/coreos-assembler@311768c

Since we're doing a new install, we're going to be installing an
updated bootloader (e.g. GRUB2) which should parse the BLS fragments
that OSTree generates. There's no reason for ostree to rerun
grub2-mkconfig.

[pep8speaks]

Hello @cgwalters! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-07-23 13:27:47 UTC

[cgwalters]

See https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/XVVAFHIK2XIOEHV4I6QOAG7DKSLI7E4V/

(Totally untested BTW, it's been a while since I've developed on Anaconda)

[poncovka]

Jenkins, it's ok to test.

[jkonecny12]

Hi, @martinezjavier I think this could be an interesting change for you. So just a heads-up.

[poncovka]

Please, use https://github.com/coreos/coreos-assembler/commit/311768c in the commit message and the comment.

[poncovka]

I wasn't able to test it with Fedora-Silverblue-ostree-x86_64-Rawhide-20200721.n.0.iso because of the bug https://bugzilla.redhat.com/show_bug.cgi?id=1857530.

[cgwalters]

Please, use coreos/coreos-assembler@311768c in the commit message and the comment.

Sure, done.

[martinezjavier]

The changes looks good to me!

[VladimirSlavik]

Code LGTM...

[jkonecny12]

Looks good to me too. Thanks for your contribution!

[poncovka]

Tested with Fedora-Silverblue-ostree-x86_64-Rawhide-20200802.n.0.iso. The installation is successful, the configuration is set up as expected, but the installed system doesn't boot (see the screenshot below).

[cgwalters]

Thanks a lot for testing this! I think the problem might be we were relying on the side effect of running grub2-mkconfig to generate the initial bootloader config. We probably need to change Anaconda to do the same thing we're doing in coreos-assembler where we just install the default GRUB config (although Anaconda may want to replace the boot device there? So perhaps we should just run grub-mkconfig...

Something like

diff --git a/pyanaconda/payload/rpmostreepayload.py b/pyanaconda/payload/rpmostreepayload.py
index b37e7081d..adfa61601 100644
--- a/pyanaconda/payload/rpmostreepayload.py
+++ b/pyanaconda/payload/rpmostreepayload.py
@@ -60,10 +60,6 @@ class RPMOSTreePayload(Payload):
         """The DBus type of the payload."""
         return PAYLOAD_TYPE_RPM_OSTREE
 
-    @property
-    def handles_bootloader_configuration(self):
-        return True
-
     @property
     def kernel_version_list(self):
         # OSTree handles bootloader configuration

?

[AdamWill]

FWIW, openQA actually relies surprisingly heavily on the 5 second grub timeout which anaconda writes to /etc/default/grub. grub's upstream default appears to be 1 second. Various openQA tests need to see the bootloader screen long enough to hit 'e' and edit the kernel command line. 1 second isn't enough, the test either doesn't see it or sees it but doesn't hit 'e' fast enough, either way it fails.

If this is merged in such a way that the installed system has a bootloader timeout of 1 second, it would be a problem for IoT testing, because several of the tests we run on IoT installs rely on this kernel command line editing. The other ostree installer image we test is Silverblue, but at a quick check, none of the tests we currently run on that image rely on this mechanism.

[poncovka]

Btw. I have tried the fix suggested in #2752 (comment), but the installed system still doesn't boot. Based on some warnings in the logs, I have tried the changes below, but it didn't help. Unfortunately, I don't understand this part well enough to be helpful.

diff --git a/pyanaconda/modules/payloads/base/utils.py b/pyanaconda/modules/payloads/base/utils.py
index 39c106989..a1865ad5f 100644
--- a/pyanaconda/modules/payloads/base/utils.py
+++ b/pyanaconda/modules/payloads/base/utils.py
@@ -95,10 +95,7 @@ def get_dir_size(directory):
 
 
 def get_kernel_version_list(root_path):
-    files = glob.glob(root_path + "/boot/vmlinuz-*")
-    files.extend(
-        glob.glob(root_path + "/boot/efi/EFI/{}/vmlinuz-*".format(conf.bootloader.efi_dir))
-    )
+    files = glob.glob(root_path + "/boot/**/vmlinuz-*", recursive=True)
 
     kernel_version_list = sorted((f.split("/")[-1][8:] for f in files
                                   if os.path.isfile(f) and "-rescue-" not in f),
diff --git a/pyanaconda/modules/storage/bootloader/grub2.py b/pyanaconda/modules/storage/bootloader/grub2.py
index e7b52aff1..d9f4c7cb5 100644
--- a/pyanaconda/modules/storage/bootloader/grub2.py
+++ b/pyanaconda/modules/storage/bootloader/grub2.py
@@ -343,7 +343,11 @@ class GRUB2(BootLoader):
             with open(machine_id_path, "r") as fd:
                 machine_id = fd.readline().strip()
 
-            default_entry = "%s-%s" % (machine_id, self.default.version)
+            if machine_id:
+                default_entry = "%s-%s" % (machine_id, self.default.version)
+            else:
+                default_entry = self.default.version
+
             rc = util.execInSysroot("grub2-set-default", [default_entry])
             if rc:
                 log.error("failed to set default menu entry to %s", productName)

diff --git a/pyanaconda/payload/rpmostreepayload.py b/pyanaconda/payload/rpmostreepayload.py
index b37e7081d..9435954c0 100644
--- a/pyanaconda/payload/rpmostreepayload.py
+++ b/pyanaconda/payload/rpmostreepayload.py
@@ -29,6 +29,7 @@ from pyanaconda.core.i18n import _
 from pyanaconda.modules.common.constants.objects import BOOTLOADER, DEVICE_TREE
 from pyanaconda.modules.common.constants.services import STORAGE
 from pyanaconda.modules.common.structures.storage import DeviceData
+from pyanaconda.modules.payloads.base.utils import get_kernel_version_list
 from pyanaconda.progress import progressQ
 from pyanaconda.payload.base import Payload
 from pyanaconda.payload import utils as payload_utils
@@ -60,14 +61,10 @@ class RPMOSTreePayload(Payload):
         """The DBus type of the payload."""
         return PAYLOAD_TYPE_RPM_OSTREE
 
-    @property
-    def handles_bootloader_configuration(self):
-        return True
-
     @property
     def kernel_version_list(self):
         # OSTree handles bootloader configuration
-        return []
+        return get_kernel_version_list(conf.target.system_root)
 
     @property
     def space_required(self):

[cgwalters]

If this is merged in such a way that the installed system has a bootloader timeout of 1 second, it would be a problem for IoT testing, because several of the tests we run on IoT installs rely on this kernel command line editing.

I think it's better for the test harness to manipulate the image offline for that; see e.g. coreos/coreos-assembler#1265

(There's also more recently work on Ignition kargs coreos/ignition#1168 )

[jkonecny12]

Hi @cgwalters any idea why the last test still won't boot? It would be great to push this a bit.

[github-actions]

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This PR was closed because it has been stalled for 30 days with no activity.