kernel-install: support the case /etc/machine-id is missing or empty

[yuwata]

Some plugins does not require non-empty /etc/machine-id such as
20-grubby.install for Fedora and 50-depmod.install.
To support such plugins to run without valid machine-id, let's remove to check
if MACHINE_ID variable is empty or not.
This may be useful for installing kernel for e.g. stateless systems
which initialize machine-id at booting the systems.

[poettering]

Supporting stateless systems definitely makes sense to me, but this patch is too simple. For example, for BOOT_DIR_ABS we should carefully come up with alternative paths if machine ID is not set.

Moreover 90-loaderentry.install should really be changed accordingly, too.

[yuwata]

Thank you for the review. In the updated version, if machine ID is not set or ESP is not detected, BOOT_DIR_ABS is set to /boot. And 90-loaderentry.install does nothing if BOOT_DIR_ABS is /boot.
What do you think this approach?

[keszybz]

I don't think this is going to fly. There are external scripts, even possibly custom scripts that admins added, and we cannot just set BOOT_DIR_ABS to an existing directory and hope for the best. Something that is backwards compatible must be invented.

I'd propose the following:

• run the .install helpers with an environment variable set (KERNEL_INSTALL_MACHINE_ID=...), that'd be empty if /etc/machine-id is missing.
• create a temporary directory to call the scripts with (mktemp -d ...) when /etc/machine-id is missing, and use that as BOOT_DIR_ABS argument.

Then any script could check for defined-but-empty $KERNEL_INSTALL_MACHINE_ID (using [[ -z ${KERNEL_INSTALL_MACHINE_ID+x} ]] to differentiate older kernel-install versions that simply do not set the variable), and in that case assume that BOOT_DIR_ABS is fake, and skip the parts that require BOOT_DIR_ABS.

Then 90-loaderentry.install would be updated along the lines you proposed, but with the updated condition.

This way we would keep existing scripts happy, and allow updated scripts to be smart. Unupdated scripts would do some pointless work, but at least they would not wreck any existing directory.

[yuwata]

@keszybz Thank you for your comments. I've updated the patch along with your suggestion. Please take a look.

[keszybz]

Do we need to read machine-id here? kernel-install already did that for us, so the elif can be dropped.

[keszybz]

So this will "leak" the temporary directory on each premature exit, e.g. when kernel-install add is called without the proper arguments. Please install the cleanup function in a trap, see https://github.com/systemd/systemd/blob/master/test/hwdb-test.sh#L32.

[yuwata]

@keszybz Thank you for the review. Updated.

[poettering]

@haraldh can you comment as well?

[haraldh]

looks ok.. although the mktemp is ugly

[keszybz]

I think we should announce that requiring BOOT_DIR to be present is deprecated for scripts, then wait two releases, then warn if the scripts put anything in BOOT_DIR, and then stop setting after two more releases. This should give us the new semantics without breaking compatibility too quickly.

[yuwata]

Updated. I've added a warning message if the temporary directory is not empty and a news entry.

[keszybz]

Looks nice.