Skip to content
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

migrate sysroot-readonly code here #2734

Open
cgwalters opened this issue Oct 11, 2022 · 9 comments
Open

migrate sysroot-readonly code here #2734

cgwalters opened this issue Oct 11, 2022 · 9 comments
Labels
difficulty/medium medium complexity/difficutly issue reward/small This is a minor fix or cleanup triaged This issue has been evaluated and is valid

Comments

@cgwalters
Copy link
Member

Following on chat from https://lists.fedoraproject.org/archives/list/[email protected]/thread/OPSZ7I4JLFTDC65FPEOUN4CZJNEGKGDS/
I think we should have a mechanism to tell ostree core to turn on the sysroot-ro flag across upgrades.

This would involve taking the linked systemd unit and bash code from https://pagure.io/workstation-ostree-config/blob/main/f/postprocess.sh#_8 and migrating it into either C here (or we could do it in Rust in https://github.com/ostreedev/ostree-rs-ext/ ). I lean a little bit towards the latter, though it creates a new precedent for soft-requiring the Rust code for new "core" ostree features.

We'd need to bikeshed a design (API) for systems to opt-in to enabling this. In a way, really what we're doing is dealing with the fact that we don't have a mechanism for in-place updates to the ostree repository configuration itself.

While I don't want to scope creep this, I could imagine that we have e.g. /usr/lib/ostree/default-repo-config or so?

And perhaps in theory that's how we should have implemented this feature from the start, by looking at a file in the deployment root instead of the repo config.

If we're trying to avoid creating a new general "upgrade the repo config" mechanism, we could invent a "stamp file" approach like /usr/lib/ostree/features/sysroot-ro or so?

@runcom
Copy link

runcom commented Oct 17, 2022

so near term, we just package up postprocess.sh for OSes that need that

@travier
Copy link
Member

travier commented Oct 17, 2022

You would want to package the "result" of postprocess.sh, not the script itself.

@runcom
Copy link

runcom commented Oct 17, 2022

Yeah, that’s what I meant

@runcom
Copy link

runcom commented Oct 17, 2022

my main question is about which package ships the $result of postprocess.sh, assuming that's the short-term solution and this is not blocked on the high level solution Colin sketched in the first comment

@travier
Copy link
Member

travier commented Oct 20, 2022

An ostree sub-package?

@travier
Copy link
Member

travier commented Oct 20, 2022

Short term you can even add them to the ostree dist git until we add them as C/Rust to ostree/ostree-rs-ext

@cgwalters cgwalters added difficulty/medium medium complexity/difficutly issue reward/small This is a minor fix or cleanup triaged This issue has been evaluated and is valid labels May 3, 2023
@jlebon
Copy link
Member

jlebon commented Sep 16, 2024

FYI, this topic came up again recently in an Edge bug. There's a bug in the migration code where if a system upgrades to a version with libostree v2024.6+, it'll have both the migration code and the bootloader naming change. And this breaks the migration code because edit-in-place creates the new BLS entries with the new naming convention and leaves the old BLS entries as well, which sort earlier. Having it upstream and tested in CI likely would've caught this as part of #3206.

On that specific bug, I think the right fix here is to have edit-in-place always reuse the same names that currently exist. Though this is trickier than it sounds: currently OstreeSysroot doesn't keep track of the original filenames that were parsed. It'd either have to be extended to do this or we could just hack this up in ostree_sysroot_deployment_set_kargs_in_place directly to figure it out.

@cgwalters
Copy link
Member Author

Ouch. Yes that analysis sounds right to me.

On that specific bug, I think the right fix here is to have edit-in-place always reuse the same names that currently exist.

I think that makes sense yes, seems quite tractable.

@cgwalters
Copy link
Member Author

One thing that also relates to this is containers/bootc#640

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty/medium medium complexity/difficutly issue reward/small This is a minor fix or cleanup triaged This issue has been evaluated and is valid
Projects
None yet
Development

No branches or pull requests

4 participants