build-extensions-container: Pass variant as a container build argument #3274
Conversation
|
Not tested by the CI here. Will do full manual testing. |
|
I tested this, works! |
|
Thanks! |
| if [[ -f "${workdir}/src/config.json" ]]; then | ||
| variant="$(jq --raw-output '."coreos-assembler.config-variant"' "${workdir}/src/config.json")" | ||
| fi |
There was a problem hiding this comment.
Shouldn't this be using the variant info from the build we're extending instead?
I think it's confusing to have both src/config.json and the variant in the build metadata. Can we delete the latter former (i.e. src/config.json) and just make it a cosa build switch instead? It only matters for the initial build anyway since everything else should be operating on an existing build, in which case we'd want it to care about the variant of the build itself.
There was a problem hiding this comment.
OK, now I understand the idea. That would indeed likely simplify things and remove the config.json file.
But this requires changes in CI scripts and several places in COSA so not a simple change.
There was a problem hiding this comment.
Right, let's not block on this for coreos/fedora-coreos-pipeline#787, but I think it'd be a nice conceptual cleanup to do soon-ish before more things try to consume src/config.json.
There was a problem hiding this comment.
But this also means that we change the semantics of what we do in that we'll always have to pass the variant option to cosa build instead of it being fixed at repo init time.
There was a problem hiding this comment.
You'll have to pay the cost of "manually specify variant" at least once regardless. This would move it from cosa init time to cosa build time. We could have cosa build default to the variant it last built if there's a previous build locally so that in the dev path, it doesn't have to be repeated each time.
There was a problem hiding this comment.
OK, see #3276 for a proposal with pending questions.
|
/cherry-pick rhcos-4.12 |
|
Cherry-pick for 4.12 in #3287 |
Fixes: openshift/os#1080
Needs: openshift/os#1082