osbuild: more refactor to prep for calling osbuild to generate multiple artifacts #3925
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We never officially supported this and when we switched to OSBuild in d37958a and dropped create_disk.sh this became dead code.
No longer used since we dropped create_disk.sh in d37958a.
This will enable us to generate the input config for runvm-osbuild once and re-use it for all platforms. In other words, the input JSON to runvm-osbuild shouldn't change between invocations.
- Add getconfig() helper functions to reduce boilerplate
- Rename variable for config passed to runvm-osbuild to make it more clear
- Create less files
- Use variables and pipes instead of creating multiple files
- Limit the config for runvm-osbuild
- Only pass in the variables that are used there and not the entire image.json
This means we now generate the parameters for runvm-osbuild once and then use them for all subsequent calls. It removes some of the logic from cmd-buildextend-metal so that other buildextend* commands can start to call runvm-osbuild (or some variant) too.
This will help my mental model a bit.
In this case we'll just copy out the entire exported tree for a given pipeline (which we specify as --export=platform). For individual artifacts we'll just name the file the same name as the platform and then copy them into the right place in the calling script. The benefits of this can be seen immediately because now we don't have to know about or copy around qemu-secex bootfs_hash and rootfs_hash inside the supermin VM.
jlebon
approved these changes
Nov 7, 2024
Member
jlebon
left a comment
jlebon
left a comment
There was a problem hiding this comment.
One concern, but overall LGTM to me. Nice work!
| # not the last partition on the disk so we need to explicitly size it | ||
| rootfs-size: "${rootfs_size_mb}" | ||
| EOF | ||
| /usr/lib/coreos-assembler/finalize-artifact "tmp/${runvm_osbuild_config_json}" "${runvm_osbuild_config_json}" |
Member
There was a problem hiding this comment.
Hmm, though should it be in the build dir? E.g. this will end up in S3. I guess there might be value in archiving it, though I would just leave it in tmp/ to start.
Member
Author
There was a problem hiding this comment.
ok reworking this to acommodate I just went to the next step (i was heading towards anyway) and opened #3930
Member
Author
|
closing in favor of #3930 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mostly draining of cmd-buildextend-metal. Also some cleanup here and there. See individual commit messages.