-
Notifications
You must be signed in to change notification settings - Fork 195
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
vmcheck: Work around read-only /sysroot #2023
vmcheck: Work around read-only /sysroot #2023
Conversation
There's definitely more bits that need to be adjusted. Will add on to this. |
51fcdd2
to
6b617ed
Compare
I'm totally fine with this, but a shortcut would mount it writable at the top of the code in all tests for now except the one or two that explicitly test it. |
6b617ed
to
c9ebe98
Compare
Yeah, I considered that, but I'm worried it'd make it too easy for us to regress on read-only handling in some random paths where we weren't even trying to test it. I think we're at least past the halfway mark now; let's see how it goes! |
c9ebe98
to
bbd90bf
Compare
Classic shell gotcha. We don't want to run `vm_kola_spawn` as part of the if-statement or otherwise we lose the `set -e` behaviour.
bbd90bf
to
e38813c
Compare
OK, this is ready to go! |
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.
👍
We need to adapt some of our tests here which assume that `/sysroot` is writable. However, in FCOS this is no longer the case now that we enable `sysroot.readonly`. We only remount rw for the couple of operations that need it so that we still retain coverage for the ro path everywhere else.
e38813c
to
55206db
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, 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 |
We need to adapt some of our tests here which assume that
/sysroot
iswritable. However, in FCOS this is no longer the case now that we enable
sysroot.readonly
.We only remount rw for the couple of operations that need it so that we
still retain coverage for the ro path everywhere else.