Bugzilla (bugzilla.redhat.com) will be under maintenance for infrastructure upgrades and will not be available on July 31st between 12:30 AM - 05:30 AM UTC. We appreciate your understanding and patience. You can follow status.redhat.com for details.
Bug 1576413 - Review Request: boom-boot - boot manager
Summary: Review Request: boom-boot - boot manager
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-05-09 12:01 UTC by Marian Csontos
Modified: 2018-07-17 18:09 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-07-17 18:09:42 UTC
Type: ---
ngompa13: fedora-review+


Attachments (Terms of Use)

Description Marian Csontos 2018-05-09 12:01:59 UTC
Spec URL: https://raw.githubusercontent.com/csonto/fedpkg-boom-boot/master/boom-boot.spec
SRPM URL: https://mcsontos.fedorapeople.org/boom-boot/boom-boot-0.8.5-6.fc29.src.rpm
Description: A set of libraries and tools for managing boot loader entries
Fedora Account System Username: mcsontos

- wouldbe fedpkg git repo is here: https://github.com/csonto/fedpkg-boom-boot
- the fresh srpm and rpm results: https://mcsontos.fedorapeople.org/boom-boot

NOTES:

Original project name is boom, but boom package have been in use already in fedora. The package was entirely removed from F27, but IIUC we can not reuse the name.

rpmlint does not run cleanly:

> python3-boom.noarch: W: spelling-error %description -l en_US systemd -> systems, system, system d

Yes, systemd should be in the used vocabulary

> python3-boom.noarch: W: spelling-error %description -l en_US bls -> lbs, bl, ls

BLS is an acronym, and this is a name of patch for grub

> python3-boom.noarch: E: non-standard-dir-perm /boot/boom/profiles 700
> python3-boom.noarch: E: non-standard-dir-perm /boot/loader/entries 700

The author used 0700 for permissions, and this is compatible with grub2 permissions for its entries.

> 1 packages and 0 specfiles checked; 2 errors, 2 warnings.

Comment 1 Marian Csontos 2018-05-09 12:11:00 UTC
Benefit to Fedora:

This package handles entries in /boot for both multiple linux distributions and for snapshots of root filesystem.

This is for those old fashioned folks who do not run containers, atomic and ostree...

Comment 2 Neal Gompa 2018-05-09 17:35:27 UTC
Taking this review.

Comment 3 Marian Csontos 2018-05-11 12:04:28 UTC
I have made the package slightly more non-compliant, adding these:

python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/boom.conf
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/profiles/21e37c8002f33c177524192b15d91dc9612343a3-ubuntu16.04.profile
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/profiles/3fc389bba581e5b20c6a46c7fc31b04be465e973-rhel7.2.profile
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/profiles/9736c347ccb724368be04e51bb25687a361e535c-fedora25.profile
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/profiles/98c3edb94b7b3c8c95cb7d93f75693d2b25f764d-rhel6.profile
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/profiles/9cb53ddda889d6285fd9ab985a4c47025884999f-fedora24.profile
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/profiles/c0b921ea84dbc5259477cbff7a15b000dd222671-rhel7.profile
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/profiles/d4439b7d2f928c39f1160c0b0291407e5990b9e0-fedora26.profile
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/profiles/efd6d41ee868310fec02d25925688e4840a7869a-rhel4.profile

Files in /boot must be treated as config files. These are supposed to be edited by user, and must be in /boot to ensure they are shared among all snapshots and/or operating systems.

The precedent for this is grub2, which has the same reasons for placing files in /boot and using %config for those.

I have also added Conflict, and dependencies (Requires: and Suggests:)

Comment 5 Neal Gompa 2018-05-11 20:21:26 UTC
Marian,

Is there a solid reason for these things being in /boot as content managed by the package? Can they not be installed in more normal locations?

My understanding is that boom doesn't do any _real_ boot management functions, and instead just generates configuration for loaders like GRUB2, rEFInd, or sd-boot to use instead.

Thus, I don't see why I should allow non-compliant stuff like this like we do for GRUB 2.

Comment 6 Alasdair Kergon 2018-05-11 21:03:58 UTC
Have you tried out using the package to swap between different environments or snapshots? 

Whichever environment you choose to boot into - with different instances of root volumes and data etc. - you need to be able to see the current version of all the boom data.  So there isn't really anywhere else to put /boot/boom unless you're going to start imposing filesystem layouts on people which would defeat part of the reason for using boom.

Comment 7 Alasdair Kergon 2018-05-11 21:41:13 UTC
From the docs https://github.com/bmr-cymru/boom :

> Boom configuration data is stored in the /boot file system to permit
> the tool to be run from any booted instance of any installed operating system.

Comment 8 Neal Gompa 2018-05-12 20:40:53 UTC
Alasdair,

This is fundamentally flawed. My own setup has /boot as part of the OS disk that is snapshotted with the rest of the OS. The reason being is precisely because we install files into /boot.

If Boom cannot handle this, then it needs to do something to make that work.

Comment 9 Marian Csontos 2018-05-14 10:09:46 UTC
I think snpshots of /boot has its own set of problems. One of snapper's flaws when using BTRFS is exactly that - it keeps information about snapshots inside the filesystem which is "snapshotted".

I wonder, if you have multiple snapshots of /boot, where do you keep information about those snapshots and grub.cfg to pick the snapshot+kernel+initrd+cmdline to boot?

And if you boot into a snapshot, and want to make an update to that config, where to save that information?

Either you still keep data inside "master" snapshot of /boot, or you need to keep it up to date somehow, don't you?

Or do you move the problem to EFI boot partition or elsewhere? And than one can not "snapshot" that, and files are saved/installed there too.

And should the FS get dirty, how does one fsck it? Of course /boot can get dirty itself too, but it is rather low traffic, compared to /.

Comment 10 Bryn M. Reeves 2018-05-14 11:04:15 UTC
> My own setup has /boot as part of the OS disk that is snapshotted with the rest 
> of the OS

We're aware of this limitation and are trying to find long-term solutions, however, even before this there's a bigger problem with this configuration: the grub2 BLS support is fundamentally broken in this scenario (since it is unable to recognise that it must search within the boot/ directory contained within the root device).

Comment 11 Bryn M. Reeves 2018-05-14 11:06:10 UTC
It's also worth noting that the boot-as-directory-of-root scenario is somewhat incompatible with the aims of BLS: to provide a single, system-wide boot volume. If it's a directory of one particular OS instance it can never fulfil this role.

It's possible that we may simply need to rule this configuration out as supportable and to make that clear in our documentation and guidance.

Comment 12 Alasdair Kergon 2018-05-14 11:46:30 UTC
1) document more clearly?
2) have the tool recognise this situation and provide helpful messages?

Comment 13 Bryn M. Reeves 2018-05-14 12:18:42 UTC
> If Boom cannot handle this, then it needs to do something to make that work.

I don't think that it's reasonable to expect boom to make things work that are explicitly outside the scope of the standard (BLS) that we are following. We can try to find ways to accommodate it to the best extent possible, and will look at doing so, but fundamentally we will always be limited by the applicability of the standard that we are adopting.

> My understanding is that boom doesn't do any _real_ boot management functions, 
> and instead just generates configuration for loaders like GRUB2, rEFInd, or sd-
> boot to use instead.

Define "real boot management" ;-)

The purpose of this is as documented: to allow *any* booted instance of the OS (including snapshots, which may have been taken prior to the creation of other managed instances) to manage *any other*. This can only be done if there is a single, shared system-wide source for this configuration (which is exactly what the BLS sets out to create).

Comment 14 Marian Csontos 2018-05-16 10:46:18 UTC
Bryn, after all, boom packaging could use different locations for the files in /boot:

*/boot/boom/boom.conf* => /etc/boom.conf || /etc/boom/boom.conf: This specifies boot_root, which is where the BLS entries will be written, and it may be different from /boot in the OS instance - e.g. user may want to create BLS entries into different path than /boot - e.g. IMO this could solve the BTRFS usecase where /boot is part of the /, and it would use different partition/path for BLS entries.

*/boot/boom/profiles/*.profile* => /dev/null: Profiles go to %(boom_root)s/profiles/, which is configurable, so IMO these should be created on demand only. Also these will be outdated most of the time (and already are). Let's keep them as examples only in doc dir.

What do you think?

Comment 15 Neal Gompa 2018-05-16 12:01:18 UTC
(In reply to Marian Csontos from comment #9)
> I think snpshots of /boot has its own set of problems. One of snapper's
> flaws when using BTRFS is exactly that - it keeps information about
> snapshots inside the filesystem which is "snapshotted".
> 

I wouldn't be snapshotting /boot at the same time as / if it weren't for the fact that we deliberately install crap there. The reason I do it is because everything from EFI files (shim and grub2), GRUB module files, to kernel images, to stuff like this is being installed there, and the rpmdb isn't happy when those files are "mysteriously missing" or "wrong".

> I wonder, if you have multiple snapshots of /boot, where do you keep
> information about those snapshots and grub.cfg to pick the
> snapshot+kernel+initrd+cmdline to boot?
> 

You're right that this is a problem. I currently have the information on ESP, but I don't like it there. It'd be cleaner if stuff wasn't getting installed into /boot directly and everything in there was something that was recoverable from the main filesystem. Then I could leave /boot alone.

Comment 16 Bryn M. Reeves 2018-05-16 13:18:14 UTC
> The reason I do it is because everything from EFI files (shim and grub2), GRUB 
> module files, to kernel images, to stuff like this is being installed there, 
> and the rpmdb isn't happy when those files are "mysteriously missing" or "wrong".

This isn't something that will happen soon, so it's really out of scope for this review request, but in the future we would like to see a situation where /boot is treated as a cache, and the corresponding packages install their files to another location under the root file system.

There's actually some progress toward this in other distros, as well as other boot management tools, but it's not something that the majority of distributions are going to be able to switch over to in short order.

When /boot is viewed in this way the management of files there becomes much easier and with less potential for conflict between multiple instances that are sharing the same volume.

Comment 17 Marian Csontos 2018-06-06 15:00:39 UTC
Let's get this to users.

I have removed profiles from /boot/boom/profiles/ - these will be created by CLI on demand. These are data, and after all should not be packaged.

But /boot/boom/boom.conf is supposed to be shared among distributions and kept outside of snapshots. If there were a better destination for the machine wide config file, as there is not, we would be happy to use that.

Precedent is grub2 doing the same.

Also where grub2 fails, is where it is not doing the same - as each OS is rewriting the file on its own. boom config file should be created once and changed by user, not by some generator.

Is this a show stopper?

Comment 18 Bryn M. Reeves 2018-06-06 15:14:04 UTC
Right: the profiles can be added by the user (although we do need the user to tell us the utsname pattern in that case, since it cannot be guessed from os-release data).

For boom.conf itself, if it causes such a headache, we can just make it optional: the only options a regular user would currently want to set there is enabling the legacy grub1 support (this is principally aimed at users migrating from distributions like RHEL6).

The only other options are to control the location of /boot and /boot/boom themselves: these are principally intended for debugging and testing the software and are used in conjunction with -c/--config.

Given that Fedora is well past grub1 at this point we aren't really gaining anything by shipping a default config file (this does not need any change to boom itself).

Comment 19 Neal Gompa 2018-06-08 14:37:26 UTC
For what it's worth, I'm okay with boom.conf being shipped in /boot for now. We already do this in GRUB 2, and everyone here knows that both GRUB 2 and Boom will eventually need to be fixed.

Comment 20 Neal Gompa 2018-06-08 14:38:16 UTC
I'm okay with the changes proposed so far, so please get up an updated spec and SRPM so I can review it.

Comment 21 Marian Csontos 2018-06-08 15:17:22 UTC
Hi Neal, I updated the spec file, made a scratch build, and uploaded rpm and srpm to below locations:

Spec URL: https://raw.githubusercontent.com/csonto/fedpkg-boom-boot/master/boom-boot.spec
SRPM URL: https://mcsontos.fedorapeople.org/boom-boot/boom-boot-0.8.5-6.2.fc29.src.rpm

Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=27490751

- wouldbe fedpkg git repo is here: https://github.com/csonto/fedpkg-boom-boot
- the fresh srpm and rpm results: https://mcsontos.fedorapeople.org/boom-boot

complete rpmlint output:

python3-boom.noarch: W: spelling-error %description -l en_US systemd -> systems, system, system d
python3-boom.noarch: W: spelling-error %description -l en_US bls -> lbs, bl, ls
python3-boom.noarch: W: non-etc-or-var-file-marked-as-conffile /boot/boom/boom.conf
python3-boom.noarch: E: non-standard-dir-perm /boot/boom/profiles 700
python3-boom.noarch: E: non-standard-dir-perm /boot/loader/entries 700
1 packages and 0 specfiles checked; 2 errors, 3 warnings.

Comment 22 Marian Csontos 2018-06-27 10:04:14 UTC
Hi Neal, have you found some time to look at this?

NOTE: There is a new boom version 0.9 - this is the same as the previously built version except the version and one small patch.

Spec URL: https://raw.githubusercontent.com/csonto/fedpkg-boom-boot/master/boom-boot.spec
SRPM URL: https://mcsontos.fedorapeople.org/boom-boot/boom-boot-0.9-1.fc29.src.rpm

Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=27894771

Comment 23 Neal Gompa 2018-06-28 14:56:22 UTC
Yeah, I've got a couple of concerns:

* Why are we stuffing everything into a "python3-boom" subpackage? Why can't most of the "bootloader content" be in the main "boom-boot" package?

* Since boom can be used with grub2 or sd-boot, shouldn't the grub2 files be in their own subpackage that requires grub2 to be installed? Then you can use a rich Supplements statement to install it automatically as needed.

For example: "Supplements: (boom-boot and grub2)" on "grub2-boom" (or "grub2-boom-boot" or "boom-boot-grub2") will ensure it's auto-installed in the event grub2 is on the system.

Comment 24 Marian Csontos 2018-06-29 06:32:57 UTC
Splitting it into more subpackages and using rich dependencies was actually one of the earlier ideas[1], but then I decided to go with plain 1 RPM, as I thought the simpler spec would be easier to review and maintain...

[1]: https://github.com/csonto/boom/blob/master/boom.spec

Do you see any other issues with the package?

I will update spec with subpackages to see how that looks like

Comment 25 Marian Csontos 2018-06-29 14:51:03 UTC
...except now I remember another reason why I made single package - I am not sure how much sense it makes to stuff anything into the main boom-boot package.

It is the python library which make use of the configuration, profiles and creates BLS entries - and my original solution had only two packages.

I think that split is rather artificial - it would be only the "script" and man page - and these are, when provided, commonly packaged with python libraries.

I can spin off the grub2 part, but that would be only the grub.d and default/boom file - but it does not make much sense to have them on system without grub2.

Comment 27 Marian Csontos 2018-07-11 14:33:00 UTC
Neal, could you please check the changes? Is there anything else what needs addressing?

Comment 28 Neal Gompa 2018-07-12 11:01:23 UTC
Please don't call it python3-boom unless it actually can be used as a Python module. If it is supposed to be able to be used as a Python module, then only the module sources should be in python3-boom, and the rest in boom-boot, which would depend on python3-boom.

Comment 29 Marian Csontos 2018-07-12 11:22:41 UTC
It can be used as python module. The issue is the configuration is required by both the module and the wrapper. The split just does not make sense.

And it is rather common for python apps to include the launcher, it is even covered in packaging guidelines - https://fedoraproject.org/wiki/Packaging:Python#Executables_in_.2Fusr.2Fbin

Comment 30 Marian Csontos 2018-07-12 11:58:39 UTC
I have made a search, and have not found single package which would made such an artificial splitting of launcher and configuration into a separate subpackage.

OTOH there is a bunch of packages shipping launcher and configuration:

python-arc
python-bucky
python-carbon
python-fedmsg-genacls
python-flower
python-gencpp
python-genlisp
python-genpy
python-ldaptor
python-pycadf
python-sippy
python-tahrir
python-virtualenvwrapper
python-wstool

I think the most prominent of them are python-virtualenvwrapper and fedora own python-fedmsg-genacls.

(Actually python-pycadf does split config files into a -common subpackage, required by both python2 and python3 modules)

Comment 31 Marian Csontos 2018-07-12 12:01:13 UTC
(Actually, I searched only packages with python in name, and with config files. I admit there may be a package using python and having the launcher in a separate subpackage...)

Comment 32 Marian Csontos 2018-07-12 12:26:24 UTC
...and quickly looking at dozens of those without python in name, I still have to find one which would do such a split, so it looks like what you are suggesting is an antipattern.

Comment 33 Marian Csontos 2018-07-12 12:46:03 UTC
The only one I found is dnf, where the split could be driven by the fact both python 2 and 3 are supported.

Either almost everyone is doing it wrong, or the split was never meant to be mandatory.

Comment 34 Neal Gompa 2018-07-12 14:30:06 UTC
kiwi and yum are examples of this pattern that you call an antipattern, too.

But it's not worth fighting this, since I think it doesn't matter what I say anyway..

PACKAGE APPROVED.

Comment 35 Marian Csontos 2018-07-12 15:48:34 UTC
We must be looking at different yum.spec files, as the one I have opened has everything in the package %{name}.lang, including %config, launcher, and python modules.

I think dnf is the closest to what you have proposed and fitting boom, but IMHO that split is partially driven by having support for both python2 and python3, and in our case does not make that much sense.

Also main package in dnf is only the launcher (and man page and bash-completion), this requiring either python2 or python3 module, and both those modules require the common conf subpackage - so conf is required by python module, and not the other way.

May be antipattern was not the best choice. But in this case that pattern does not fit - the configuration is required by python module, so should be either included, or python module should require the configuration subpackage, just like dnf does.

Big thanks for all your time!

Comment 36 Neal Gompa 2018-07-13 10:34:55 UTC
(In reply to Marian Csontos from comment #35)
> We must be looking at different yum.spec files, as the one I have opened has
> everything in the package %{name}.lang, including %config, launcher, and
> python modules.
> 
> I think dnf is the closest to what you have proposed and fitting boom, but
> IMHO that split is partially driven by having support for both python2 and
> python3, and in our case does not make that much sense.
> 
> Also main package in dnf is only the launcher (and man page and
> bash-completion), this requiring either python2 or python3 module, and both
> those modules require the common conf subpackage - so conf is required by
> python module, and not the other way.
> 
> May be antipattern was not the best choice. But in this case that pattern
> does not fit - the configuration is required by python module, so should be
> either included, or python module should require the configuration
> subpackage, just like dnf does.
> 
> Big thanks for all your time!

I just think that it's "surprising" for people and they're not likely to find the boom boot manager if it's called "python3-boom". Our rules say python modules that other people are supposed to be able to use should be `python3-<foo>`, but they are specifically silent on what you should do with the rest of it.

Whatever...

Comment 37 Marian Csontos 2018-07-13 15:02:19 UTC
Now that's actually a valid concern and something which is bugging me too. After reviewing all those tools with modules, it is quite common to have a package name not start with python. I will give it some more thought over weekend while fedpkg repo is ready.

Comment 38 Jason Tibbitts 2018-07-13 17:39:34 UTC
Our rules are pretty clear, though.  You don't name standalone applications with a language-specific prefix.  If you have something that's both an application and a module then you are encouraged to split the module and the application into separate (sub)packages.

https://fedoraproject.org/wiki/Packaging:Guidelines#Libraries_and_Applications

Comment 39 Mohan Boddu 2018-07-13 19:23:13 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/boom-boot

Comment 40 Neal Gompa 2018-07-15 15:23:40 UTC
(In reply to Jason Tibbitts from comment #38)
> Our rules are pretty clear, though.  You don't name standalone applications
> with a language-specific prefix.  If you have something that's both an
> application and a module then you are encouraged to split the module and the
> application into separate (sub)packages.
> 
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#Libraries_and_Applications

Unfortunately, a lot of people have been doing the opposite and claiming this is supported by the guidelines. Hell, even I doubted myself and I don't do it because it doesn't even make sense...

Comment 41 Marian Csontos 2018-07-16 11:29:16 UTC
So the final version with separate boom-boot and boom-boot-conf.
Neal, could you look at it and ack, so I can push to the repo.

Downside is now we have 4 RPMs instead of 2, and repo metadata are growing... :-(

Spec URL: https://raw.githubusercontent.com/csonto/fedpkg-boom-boot/split/boom-boot.spec
SRPM URL: https://mcsontos.fedorapeople.org/boom-boot/boom-boot-0.9-3.fc29.src.rpm

Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=28333649

Comment 42 Neal Gompa 2018-07-16 14:23:55 UTC
Looks good except for one bit:

> Supplements: (grub2 and python3-boom)

Change this to "Supplements: (grub2 and boom-boot)".

You can fix this on import.

Comment 43 Marian Csontos 2018-07-17 18:09:25 UTC
Thanks! Everything is done, pushed to master, f27 and f28, built. I did a smoke test - F28 works fine, Rawhide is broken ATM.


Note You need to log in before you can comment on or make changes to this bug.