-
Notifications
You must be signed in to change notification settings - Fork 198
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
daemon: Use MountFlags=slave and opt-in to OSTree read-only /sysroot #1896
Conversation
bot, retest this please |
Requires: ostreedev/ostree#1767 |
fc352aa
to
353ac4c
Compare
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.
LGTM, though... I'd feel better if we had tests for this. Can you add a trivial vmcheck
test that e.g. sets the flag, reboots, then does some basic operations?
(Once we switch FCOS over to it and merge #1949, that'll be the default, but I think at that point we can modify the test to sanity-check the opposite trivial case of /sysroot
rw still working... and we can drop it if we turn it on by default in the future).
Of course, this would be more meaningful if CI wasn't actually broken right now... 😢 |
353ac4c
to
c44c5cf
Compare
I toggled this on in the kernel args tests. Also tested this locally with FCOS and things seem good! |
OK yup, tested /lgtm And of course, this won't actually pass CI until we build a newer ostree and tag it into the continuous tag. Basic pipeline tests did pass though. /override continuous-integration/jenkins/pr-merge f29 tests appear to be blocking on some Rust issue:
Not sure what's going on there. It did pass on f31 in the pipeline tests though. /override f29-compose1 |
@jlebon: Overrode contexts on behalf of jlebon: continuous-integration/jenkins/pr-merge, f29-compose1, f29-compose2, f29-primary In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/retest |
c44c5cf
to
a518341
Compare
OK, I reworked this to be compile-time conditional. What we really want again is a FCOS-continuous stream, testing the latest git master of everything; distinct from the main FCOS builds. We'll get there... Anyways on this, what we'll end up with is that only people building ostree from git master will get the ro-sysroot, and with this they'll also need to build rpm-ostree git master too. Which personally I always do. |
Being probably excessively conservative, I don't want to stick ostree git master in anything that main Fedora users might see right now...though I'd of course be more confident of all this once we get our CI going again. |
Ahh heh, FAHC rdgo is actually still going strong. So we did pull in git master ostree and turned on the flag. Though now tests are failiing with:
Which yup, of course, is because VMs are re-used. Anyway, I'm gonna context switch to try to get #1949 working, though meanwhile it looks like our CI is co-operating... let's just turn off that flag again at the end of the test (or in |
a518341
to
2a1540c
Compare
Done! |
Alrighty, let's do this! /lgtm |
Ahh OK, there seems to be a real test failure here.
That should've worked, right? |
But I tested this test on FCOS and it had passed. But that was with coreos/coreos-assembler#736. Maybe there's a subtle difference there between doing it at creation time than turning it on live and rebooting? |
2a1540c
to
66525a9
Compare
This is all we need to tell libostree that we support a read-only `/sysroot` and `/boot`. See ostreedev/ostree#1265 PR in ostreedev/ostree#1767
OK, it definitely works to turn on the bit live and reboot in FCOS, so this might be something funny going on with f29. Anyway, I'm not really interested in debugging f29, so for now I just commented out the vmcheck additions. (We can uncomment once we get #1949 in and flip it to /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
bot, retest this please |
/override f29-primary |
@cgwalters: Overrode contexts on behalf of cgwalters: f29-primary In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/override f29-compose1 |
@cgwalters: Overrode contexts on behalf of cgwalters: f29-compose1 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
This is all we need to tell libostree that we support a read-only
/sysroot
and/boot
.See ostreedev/ostree#1265
PR in ostreedev/ostree#1767