-
Notifications
You must be signed in to change notification settings - Fork 158
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
overlay/growfs: support CEX-encrypted LUKS volumes #2986
Conversation
Hmm, can you provide the Ignition config you're testing with? Has Ignition already resized the partition? I would think in the general case we do still need to call |
Hi, Here is the ignition config.
|
Hi @jlebon ,
|
174381b
to
03398df
Compare
03398df
to
6cc2836
Compare
6cc2836
to
7a753f0
Compare
Hi @madhu-pillai, I've reworked this to fix some issues and clean things up a bit. WDYT? If it looks good, can you also check that the CEX case still works fine? |
Hi @jlebon , |
Hi @jlebon , I've tested in the dasd and it worked both with xfs and ext4. snippet is below.
|
OK actually, there's another issue here. We're still calling Pushed another commit there. Can you retest it and confirm that the spurious output is gone? Also, can you show the output of |
Hi @jlebon,
|
Great, thanks for testing! |
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.
Does not feel great to parse luksdump output using grep but I guess if we're doing that here, it's that we don't have a better option :/
Not tested but looks reasonable.
Support for CEX LUKS volumes was added in Ignition as part of coreos/ignition#1693. Co-authored-by: Jonathan Lebon <jonathan@jlebon.com>
If the partition is on a DASD disk, then don't run growpart since it doesn't know how to handle it and it's not needed (coreos-installer already sizes it to its maximum).
It's cleaner and less prone to false positives.
Thanks, good catch! I missed that in the original review, but indeed there's a Added another commit, but also rebased while we're here to get a fresh CI run. @madhu-pillai Can you retest the DASD and CEX cases with this? |
Will do that. |
Hi @jlebon ,
Here is the verbose mode of the growfs.
Attached boot log from zKVM-Fcos.
|
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.
Not tested but LGTM. Thanks for cleaning up the luksdump output parsing.
Support for CEX LUKS volumes was added in Ignition as part of
coreos/ignition#1693.