Create /etc files, symlinks and folders if they don't exist

[pothos]

• Create /etc files, symlinks and folders if they don't exist

  The existing tmpfile logic took care of folders that the ebuild keepdir
  directive wanted to exist on the OS. However, files and symlinks were
  not created, causing them to be missing if we didn't explicitly modify
  the ebuild files in coreos-overlay to find a solution with tmpfiles or
  patching the paths to /usr.
  Add logic to create missing files and symlinks through tmpfile
  directives and preserve any directory, not only the ones with the
  keepdir ebuild directive. Also remove any state from the rootfs to make
  sure that we don't rely on it when testing our images before the
  release. To create the files the final /etc folder is moved to
  /usr/share/flatcar/etc and in the future this can be used for a better logic
  that could take care of updating files the user didn't modify, deleting
  those that aren't needed anymore, and even reconciling changed files
  through a 3-way merge, instead of using simple tmpfile logic.

• build_library/build_image_util.sh: move image modification before trim

  The trim should be the last action before the image is finalized,
  otherwise it may not result in the minimal size.

How to use

Testing done

Done here
using flatcar/mantle#316 should fix the failing tests.

Also did some manual tests of the python script and checked the generated entries in the image and the resulting files.

• Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)

[krnowak]

@pothos: Thanks for the draft PR. I didn't know that we already had a tmpfile generator, that's why I wanted to leverage what we already had (that is - existing tmpfiles in coreos-overlay packages). But since the generator is already there and it required only a minor patching, then yeah - I think your solution sounds good too.

Do you want me to take it over or you will finish it?

[pothos]

Do you want me to take it over or you will finish it?

Happy to collaborate, feel free to push to this branch. Currently it's not working and some debugging lays ahead ;)

[pothos]

Currently it's not working and some debugging lays ahead ;)

Ok, tmpfiles with f have only inline content, C is for copying a file

[krnowak]

Why removing var if we are not backing it up? Or maybe - why not backing up var above, like we do with etc?

[jepio]

/var is backed up on line 656

[pothos]

(without files/symlinks)

[jepio]

I reviewed this, and I worry about the long term effects of shipping something like this now. If long term we want to do 3-way merge (not a fan, lots could go wrong) or the overlay solution, then there will be very different behaviors around /etc in different releases.

The other concern is: if a user wants to remove a config file, the systemd-tmpfiles definition will bring it back on next boot, which could be bad. So I feel like we should either be doing this only on first boot (completely empty filesystem), or replace the individual file definitions with:

C /etc - - - - /usr/share/flatcar/etc

(will recursively copy without dereferencing symlinks, and only if etc is missing).

[jepio]

This flag does two things. How about splitting into two, like:
--process-files and --file-src.

[pothos]

Both need to be present, or it's a no-op, that's why I bound them together into one

[jepio]

Another question: the tmpfiles stuff runs in sysroot right? Is there anything in initramfs that needs to look at /etc (i know there is a parse-etc.service), or some service that needs config files before tmpfiles runs? Anything selinux related?

[pothos]

We already support running from an empty rootfs that gets created by Ignition, and this is tested in the cl.ignition.*.*root tests to some extend.

The other concern is: if a user wants to remove a config file, the systemd-tmpfiles definition will bring it back on next boot, which could be bad. So I feel like we should either be doing this only on first boot (completely empty filesystem), or replace the individual file definitions with

Only doing it on first boot won't work, the goal we address here is that required files get created not only on first boot but also after updates. Yes, currently a user can't remove the file, we would need the 3-way merge for that to have the old factory /etc as reference. I think depending on the file in question there are ways to disable it without removal (e.g., an empty file or a symlink to /dev/null) - in general this use case already wasn't possible anyway for the files we hardcoded to be under /usr instead of /etc (or those where a manual tmpfile rule for /etc was added).
Edit: called this out in the changelog entry now

[jepio]

Only doing it on first boot won't work, the goal we address here is that required files get created not only on first boot but also after updates.

But the way tmpfiles works is it will only copy files over if the file is missing, not if its updated. So with updates we will only ever accumulate new files and keep all old contents. This is different to the way we currently work with read-only config files in /usr and also different to the way an overlay would work (with an overlay, config files the user doesn't modify would be updated together with the overlay).

Yes, currently a user can't remove the file, we would need the 3-way merge for that to have the old factory /etc as reference. I think depending on the file in question there are ways to disable it without removal (e.g., an empty file or a symlink to /dev/null) - in general this use case already wasn't possible anyway for the files we hardcoded to be under /usr instead of /etc (or those where a manual tmpfile rule for /etc was added).

I thought about empty files or symlinks to /dev/null, but are we sure every application will handle that correctly and not error out saying "configuration unreadable"?

So this feels like we need to have a plan for where we're going first. I like the idea behind this PR but wouldn't want it to lock us into a solution that we would have to break in a couple of months again.

[pothos]

Correct, the update under /usr is what we would lack when removing our downstream patches at the moment.
Yes, I agree that instead of using tmpfiles we should have a more complex logic as outlined in the referenced issue.
E.g., only copies the file if it wasn't deleted by the user (compared to the existence in the previous factory version), and updates the file if it exists only if it wasn't modified (again, compared to the previous factory version), plus it can remove a file if it existed in the previous version but not in the new version and wasn't modified. The prerequisite for comparing is backing up the current factory /etc to the rootfs on the first boot or the reboot after an update. I think this scheme is simple (because it doesn't deal with file contents yet) but already covers most cases we care about and brings us on par with what package managers like dpkg can do.

I think of this here as an interim solution that is not perfect but could help already. If we think it causes more trouble, I'm fine to skip it and go straight to a custom logic that is executed at a similar stage.

[jepio]

I have my reservations but they are not strong enough to attempt to block this.

For config file merging gentoo has two built-in tools to look at: etc-update and dispatch-conf. They allow merging files if the new config file is present at /etc/(dir/).cfgXXXX_name, which we could easily integrate with. I believe a check of the checksum of the old config file is also performed when emerge decides whether to remove or create the .cfg.... file. So this could be done in flatcar-postinst.

[jepio]

Soft approve

[pothos]

I'm really ok to wait and go straight to the solution that covers updating files under /etc, not only creating missing files. More opinions on this @krnowak? Do you think we should take this and start dropping downstream patches? Or take it only as a safety measure and keep most downstream patches until we have a comprehensive solution?

[pothos]

@krnowak I didn't move the write_contents here with the idea that for the image contents /etc is the path that makes more sense. I'm totally fine moving this below to have the real image contents with /usr/share/…etc.

[pothos]

Thinking about this again I would say that using an overlayfs is great to sidestep the requirement of doing a 3-way comparison and operating on files but it gives us the same benefits (the content merging is probably something out of reach anyway…).
So I would say we do the move to /usr/share/flatcar/etc as done here but set up an overlayfs mount. The gen_tmpfiles.py change can stay if we don't make use of it by not passing --files.
Since the overlayfs mount would prefer the rootfs files on existing installs, I think it would be beneficial to have a job in the initrd that removes duplicates, i.e., for every file in /usr/share/flatcar/etc we can delete it from the rootfs if it is identical. This way we can make sure that future /usr updates will actually update the files because otherwise they would be still frozen despite having no modifications.

[krnowak]

Thinking about this again I would say that using an overlayfs is great to sidestep the requirement of doing a 3-way comparison and operating on files but it gives us the same benefits (the content merging is probably something out of reach anyway…). So I would say we do the move to /usr/share/flatcar/etc as done here but set up an overlayfs mount. The gen_tmpfiles.py change can stay if we don't make use of it by not passing --files. Since the overlayfs mount would prefer the rootfs files on existing installs, I think it would be beneficial to have a job in the initrd that removes duplicates, i.e., for every file in /usr/share/flatcar/etc we can delete it from the rootfs if it is identical. This way we can make sure that future /usr updates will actually update the files because otherwise they would be still frozen despite having no modifications.

I admit that the message is not entirely clear to me. To set up the overlayfs we will need the read-only lower directory, read-write upper directory and the merged directory. As I understand it, /usr/share/flatcar/etc would be the lower directory, /etc would be the merged directory. Not sure where to put the upper directory, /var/flatcar/etc? Some more thought needs to be put into the version update process, because changing the lower directory (for example from <USR-A>/share/flatcar/etc to <USR-B>/share/flatcar/etc will result in an undefined behavior.

[pothos]

I admit that the message is not entirely clear to me. To set up the overlayfs we will need the read-only lower directory, read-write upper directory and the merged directory. As I understand it, /usr/share/flatcar/etc would be the lower directory, /etc would be the merged directory. Not sure where to put the upper directory, /var/flatcar/etc? Some more thought needs to be put into the version update process, because changing the lower directory (for example from <USR-A>/share/flatcar/etc to <USR-B>/share/flatcar/etc will result in an undefined behavior.

/etc can be both the upper and merged directory (the only drawback is that 'wiped' files will be in there as special chardevs when the overlay is not set up). While I try to get an upstream solution in systemd-syscfg with a mutable /etc that we could use for this I think we can already try it out in Flatcar. I'll open a new PR for that.