buildextend-installer: move PXE squashfs and osmet to separate initrd #1608
Conversation
|
Skipping CI for Draft Pull Request. |
src/cmd-buildextend-installer
Outdated
| # Generate initramfs stamp file which says whether it's a live or legacy | ||
| # initramfs. |
There was a problem hiding this comment.
I'll be glad when we can drop the legacy (non-live) installer.
| # Add the padding | ||
| with open(initramfs, 'ab') as fdst: | ||
| fdst.write(bytes(initrd_ignition_padding)) | ||
| # Generate initramfses |
There was a problem hiding this comment.
Say "Generate initramfses" ten times fast! 😜
|
In our discussion for this we deemed the "unify the ISO and PXE rootfs paths" out of scope. If we do unify those paths in the future do you think it would make the code in here easier? I see a lot of |
I don't think so. IMO the initrd generation is pretty clean now. The |
|
Moved initial refactor to #1612. |
|
rebase now that #1612 merged |
|
/approve |
bgilbert
changed the title
|
Ready for review. We should hold on merging this until rdcore has landed and the fedora-coreos-config PR is ready to merge. /hold |
There was a problem hiding this comment.
Cool, I like that you added tests for this already in testiso!
One missing case I noticed is appending to the initrd= PXE config instead of concatenating. OTOH, it's identical behaviour from our point of view either way; I'm not sure it's possible for that to break without concatenating also breaking and it being a bug in our code (well except maybe in our kola PXE config, but that's our problem).
src/cmd-buildextend-installer
Outdated
| # Make stream hash for `rdcore stream-hash` | ||
| def make_stream_hash(src, dest): |
There was a problem hiding this comment.
Any reason you implemented this in Python instead of rdcore as well so both reading and writing the hash file sit together? Plus, we're sure to use OpenSSL for hashing, whereas... briefly looking at the Python hashlib docs and the CPython source, it seems like it uses its own C implementation for SHA-256. How's the speed?
Cool as is too of course (and we can change it in a follow-up as well).
Though maybe once we document the format a bit, let's add a permalink to it here?
There was a problem hiding this comment.
Any reason you implemented this in Python instead of
rdcoreas well so both reading and writing the hash file sit together?
My initial instinct was to put it in rdcore, but the additional build-time dependency seemed more troublesome than the 10 lines of Python. But yeah, we can always change it later. I'll add a link to the format docs.
Plus, we're sure to use OpenSSL for hashing, whereas... briefly looking at the Python hashlib docs and the CPython source, it seems like it uses its own C implementation for SHA-256. How's the speed?
I'm seeing 347 MiB/sec for this code (using timeit) and 335 MiB/sec for rdcore stream-hash (including process startup overhead).
| if args.legacy_pxe: | ||
| tmpinitrd_pxe_or_rootfs = tmpinitrd_pxe | ||
| else: | ||
| tmpinitrd_pxe_or_rootfs = tmpinitrd_pxe_rootfs |
There was a problem hiding this comment.
Could move this down lower to where it's actually used.
There was a problem hiding this comment.
Eh, I think it's a bit cleaner to keep it outside the conditionals.
I actually did that first, and it worked fine on x86_64. I tore it out because unconditional concatenation was less intrusive. Every architecture has its own way of plumbing additional initrds to its bootloader, which I couldn't test, and s390x required concatenation anyway.
Yup, agreed. |
For live images, add PXE "rootfs" image containing osmet files and the root squashfs. This can be appended as a second initrd or fetched by the initramfs at runtime. Write the build ID into flag files in the initramfs and rootfs so the initramfs can confirm that both came from the same build. Store a chunked hash of the rootfs into the PXE initramfs so rootfs integrity can be verified after a runtime fetch. If --legacy-pxe is specified, continue as we were before, but generate a rootfs image containing only a flag file. This allows a transition period where the initramfs can check for the flag file and warn that the rootfs hasn't been provided.
Eventually, we'll default to having the initramfs fetch the rootfs via HTTP, since that's the hard case. Or, if --pxe-append-rootfs is specified, concatenate the initramfs and rootfs together into one image. But for now, we need to ratchet CI by unconditionally concatenating.
|
Updated to add link to |
|
/lgtm |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bgilbert, dustymabe, 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 |
|
/unhold |
For live images, add PXE
rootfsimage containing osmet files and the root squashfs. This can be appended as a second initrd or fetched by the initramfs at runtime. Write the build ID into flag files in the initramfs and rootfs so the initramfs can confirm that both came from the same build. Store the hash of the rootfs into the PXE initramfs so rootfs integrity can be verified after a runtime fetch.If
--legacy-pxeis specified, continue as we were before, but generate a rootfs image containing only a flag file. This allows a transition period where the initramfs can check for the flag file and warn that the rootfs hasn't been provided.For coreos/fedora-coreos-tracker#390.