initrd-setup-root: Improve migration for OEM systemd-sysext images #68
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The filename should have a version suffix but didn't in the fallback download case when it's not the "initial" MVP sysext. Reuse the path we have prepared for the symlink target when moving to ensure that the file has the version suffix.
The retry logic in the loop relied on the if statement after it. Move the failure into the retry loop.
The list of old OEM files to clean up when migrating to an OEM sysext can have many files which might have changing names but since they are part of a common subfolder, can be removed easily through removing the whole folder. Support deleting folders in the migration logic.
The old OEM setup was creating and enabling a cloud-init service unit from the default.ign file on the rootfs. When deleting this, we need to ensure that cloud-init is still enabled by enabling the inbuilt service unit from the base image. Special-case the removal of the cloud-init enablement symlink to create it again with the new target.
First the plan was to request that the migration list should use /sysroot/ as prefix but if someone forgets that and writes regular paths it would remove things in the initrd instead (or nothing). Simplify the interface by expecting regular paths and prepending /sysroot on removal.
A simple list of extensions to enable is not enough in case we want to move parts of the base image to an extension because then we would have to enable it by default. This requires an option for the user to explicity disable it. Add support for reading the OS default extensions from /usr and allow that the user disables extensions through writing "-NAME" in the extension config file in /etc.
krnowak
reviewed
Jul 31, 2023
| for NAME in $(grep -o '^[^#]*' /sysroot/etc/flatcar/enabled-sysext.conf || true); do | ||
| # The enabled-sysext.conf file is read from /etc and /usr and contains one name per line, | ||
| # and when the name is prefixed with a "-" it means that the extension should be disabled if enabled by default in the file from /usr. | ||
| # It may contain comments starting with "#" at the beginning of a line or after a name. |
There was a problem hiding this comment.
I wouldn't bother with allowing comments after the name, it only complicates things. Unless the file format is not something we have a control over.
| # It may contain comments starting with "#" at the beginning of a line or after a name. | ||
| # The file is also used in update-engine to know which extensions to download. | ||
| # Note that we don't need "{ grep || true ; }" to suppress the match return code because in for _ in $(grep...) return codes are ignored | ||
| for NAME in $(grep -h -o '^[^#]*' /sysroot/etc/flatcar/enabled-sysext.conf /sysroot/usr/share/flatcar/enabled-sysext.conf | grep -v -x -f <(grep '^-' /sysroot/etc/flatcar/enabled-sysext.conf | cut -d - -f 2-) | grep -v -P '^(-).*'); do |
There was a problem hiding this comment.
If we don't allow the comments after the name, then this could be simpler, like I suggested in the init PR.
There was a problem hiding this comment.
That would simplify '^[^#]*' but not the rest, I think it's good to follow the https://en.wikipedia.org/wiki/Robustness_principle
There was a problem hiding this comment.
That is probably a good principle to follow, in principle. :) So let's keep it like this.
But personally, for cases where I am the sole producer and consumer of some protocol/file/whatever, I go strict/conservative in both ways.
To have one place where the list of old OEM files to clean up is specified it is now read from /usr.
2 tasks
krnowak
approved these changes
Sep 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Combined with flatcar/update_engine#24 these changes improve the migration to OEM systemd-sysext images and the handling of extra Flatcar extensions.
How to use
Testing done
Tested with update-engine PR