-
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
Add hidden coreos-rootfs seal
command
#1911
Conversation
use libc; | ||
|
||
/// A reference to a *directory* file descriptor. Unfortunately, | ||
/// the openat crate always uses O_PATH which doesn't support ioctl(). |
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.
And yeah, this turned out to be a lot more code in Rust than it would have been in C particularly if we just copy-pasted the libostree code but I doggedly kept at it 😉
319874f
to
a538b85
Compare
☔ The latest upstream changes (presumably 553be6e) made this pull request unmergeable. Please resolve the merge conflicts. |
a538b85
to
310e663
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.
I'm conflicted on this. On one hand, it's definitely convenient. On the other hand, putting this here feels a bit forced. It's not the first time and I'm sure not the last that we'll want/need something better than shell to run in the initrd.
I'm not sure of a better place for it though. Guess cosa
could grow a compile-on-the-fly
option? (And then having this code live in fedora-coreos-config
). Would probably tie in with the move to make local dev easier too.
Anyway, open to trying this out!
|
||
#[derive(Debug, StructOpt)] | ||
#[structopt(name = "coreos-rootfs")] | ||
#[structopt(rename_all = "kebab-case")] |
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.
kebab-case is best case
Yeah I was too. But...when I narrowed down to:
That's how I got here. |
310e663
to
4accb1e
Compare
4accb1e
to
1a25638
Compare
/approve |
1a25638
to
6d0f8af
Compare
All this does is put the immutable bit on the target directory. The intention is to replace this bit to start: https://github.com/coreos/coreos-assembler/blob/8b205bfbb971707382ace76bbb39e46ed3fc560d/src/create_disk.sh#L229 However, the real goal here is to add code in this file to handle redeploying the rootfs for Fedora CoreOS which combines OSTree+Ignition: coreos/fedora-coreos-tracker#94 Basically doing this in proper Rust is going to be a lot nicer than shell script in dracut modules. Among other details, coreutils `mv` doesn't seem to do the right thing for SELinux labels when policy isn't loaded. Closes: #1911 Approved by: jlebon
💔 Test failed - status-papr |
PAPR is failing because OpenStack no longer enables nested virt by default. Requested access again for our tenant. |
bot, retest this please |
1 similar comment
bot, retest this please |
6d0f8af
to
0b8887a
Compare
/lgtm |
Hmm weird. The PR status page shows it's "Good to be merged". Not sure what tide is waiting for here. |
0b8887a
to
5fb47e6
Compare
5fb47e6
to
db0622e
Compare
Ah, I think this was a race condition because we added the new Prow job. The tide status showed waiting for Pushed a rebase on master. |
bot, retest this please |
/lgtm What's going to be tricky here is retrying jobs that Prow doesn't control. That was a nice thing with Homu's model of just pushing to a branch and letting different CI services pick up the event naturally. |
All this does is put the immutable bit on the target directory. The intention is to replace this bit to start: https://github.com/coreos/coreos-assembler/blob/8b205bfbb971707382ace76bbb39e46ed3fc560d/src/create_disk.sh#L229 However, the real goal here is to add code in this file to handle redeploying the rootfs for Fedora CoreOS which combines OSTree+Ignition: coreos/fedora-coreos-tracker#94 Basically doing this in proper Rust is going to be a lot nicer than shell script in dracut modules. Among other details, coreutils `mv` doesn't seem to do the right thing for SELinux labels when policy isn't loaded.
db0622e
to
3c26cc3
Compare
Rebased 🏄♂️ also to retrigger CI. |
/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 |
/override f29-primary
but that can't break from this. |
@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. |
Hmm although misc-1 also failed with an unusual error. |
Hmm, I had fixed this in this PR, but it got lost somehow? |
All this does is put the immutable bit on the target directory.
The intention is to replace this bit to start:
https://github.com/coreos/coreos-assembler/blob/8b205bfbb971707382ace76bbb39e46ed3fc560d/src/create_disk.sh#L229
However, the real goal here is to add code in this file
to handle redeploying the rootfs for Fedora CoreOS which
combines OSTree+Ignition:
coreos/fedora-coreos-tracker#94
Basically doing this in proper Rust is going to be a lot
nicer than shell script in dracut modules. Among other
details, coreutils
mv
doesn't seem to do the right thingfor SELinux labels when policy isn't loaded.