-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OSTree task improvements #713
OSTree task improvements #713
Conversation
The environment variable OSTREE_BOOT_PARTITION is only used when using GRUB. Move the export into the if statement. Also add a comment why manually adding /boot/loader{.0} directory is necessary. Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
The current code clears tmp and then creates a symlink inside it to /sysroot/tmp: tmp └── tmp -> sysroot/tmp This is likely a mistake and the root tmp should have pointed to sysroot/tmp. However, since /tmp is mounted as a tmpfs anyways, we can get rid of all this logic. Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Instead of copying the files to be commited to the ostree just use a hardlink tree. This improves performance and wasts less diskspace. When using this method the root directory has already the correct permission bits set. Also get rid of the unnecessary sync. This halfs the execution time of the do_image_ostree tasks in my measurments. Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
The home directory currently are commited to the OSTree, presumably to then use it for the deployment. However, we do have access to the original rootfs in the OSTree deployment tasks (do_image_ota) hence transferring the files "via OSTree" is not necessary. We do already carry over some files from the original OE rootfs to /var/sota. Follow this approach for /var/local and /home as well. The home will still be stored in the sysroot as documented in https://ostree.readthedocs.io/en/latest/manual/adapting-existing/. Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Instead of using the double indirection mode /home -> /var/rootdirs/home -> /sysroot/home move the home directory physically into /var/rootdirs. This allows to use the --modern flag when initializing the file system. The "old" style is still supported, and does make sense in case the home directories need to be shared between multiple deployments. Since multiple deployments is not a use case in meta-updater use the /var approach. See also: ostreedev/ostree#2085. The modern flag also gets rid of dev, proc, root, run, sys and tmp. All of them have been empty and unused. Note: This change cannot be pushed through updates as this is an initial deployment setting. Only devices provisioned with images built with this change applied will use the new layout. Updates will continue to work on both systems as the symlink from the deployment stays the same (first indirection is still /home -> /var/rootdirs/home). Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Thanks for putting this together! It looks like a lot of good work. I'm reviewing it now and testing it in CI, but it might take just a bit of time to make sure that everything works like we expect. |
CI is unfortunately broken apparently because Yocto's certs expired, which also means I can't test manually. Once that is resolved we'll retry. I've reviewed the logic, though, and I don't see any obvious concerns. Everything looks like an improvement and update to modern OSTree usage. BTW, I agree that the tmp handling logic we had in place was just a mistake. The code was committed in the earliest days of this project and has probably never been looked at too closely again until now. Thanks for catching that! |
Hehe, yeah our CI got tripped by it today too :-)
Yeah I was assuming that. Thanks for confirming. |
@patrickvacek added two more commits, one of which addresses more or less what @eu-smirnov in #687 tried to achieve. If you prefer to have those two separate still I can pop them and create a separate merge request. |
Again, looks like good stuff! I probably would prefer if the new stuff were in a separate PR, though. It's a lot to test, and all of this has to be tested manually, since CI doesn't really cover the interesting parts here. Does your change to |
The changes look good and well motivated in their commit messages, thanks! |
f3f634d
to
fb058bb
Compare
@patrickvacek ok, dropped off the last two patches! Will create a separate pull request for those. |
@patrickvacek and no, it does not address #412. |
Looks good to me! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the following:
- Update from old layout to new
- Update from new layout to old
- Update from new to new
And looked around the affected directories to make sure I didn't see anything unexpected. Everything looks good, just as I anticipated. Thanks again for this!
Just wanted to point out that using the new |
Hi, this are smaller improvements to the OSTree image tasks. Most notably it changes how /home folder is stored to align with what Fedora CoreOS is doing and what upstream OSTree today uses as the modern approach.