-
Notifications
You must be signed in to change notification settings - Fork 169
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
Updated platformid to force openstack for s390x on 4.2 #1004
Updated platformid to force openstack for s390x on 4.2 #1004
Conversation
src/gf-platformid
Outdated
@@ -42,6 +42,10 @@ else | |||
coreos_gf_run_mount "${tmp_dest}" "blocksize:4096" | |||
fi | |||
|
|||
if [[ "$arch" = "s390x" && "$platformid" = "qemu" ]]; then |
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 would just pass in openstack
instead of qemu here: https://github.com/coreos/coreos-assembler/blob/rhcos-4.2-multiarch/src/cmd-build#L248 for s390x instead of this - that way at least what you're passing in is what you set it to.
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.
The reason I chose not to implement it that was was because I don't know the underlying build process. If you can guarantee that that the build command will always be run on s390x hardware, than I could do a uname -m
lookup for the arch info. Instead, I trusted the $arch variable (defined in
coreos-assembler/src/cmdlib.sh
Line 42 in d18ca36
arch=$(uname -m) |
That is not to say that it doesn't already use uname -m
to get the arch information, but I figured I should play it safe in case there is something I missed.
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.
cmdlib.sh
i executed at the beginning of cmd-build, so arch
should be exported and available. You shouldn't need to define that again. The builds are also run on the corresponding hardware that the images are generated for, so shouldn't be an issue.
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.
Ah I missed that. :)
Great - see updated PR.
d18ca36
to
fb0ec05
Compare
lgtm |
Trying to follow here. So the issue is that on s390x, you want to use config-drives like on OpenStack for OpenShift libvirt installs? I agree with Luca and Alex in coreos/ignition-dracut#145 that this should be solved in Ignition directly. I don't think the Hmm, let me check here how hard it'd be to just hack the Ignition logic to special-case s390x. |
We know we need to fix this long term in ignition for both power and s390x. This hack would just be to tide us over until the appropriate fix gets done in ignition - to get us through the CI/CD requirement for testing OpenShift. This is specific to the multiarch-4.2 branch, so it wouldn't bleed into the main CoreOS branch. |
Correct. we would need the same logic as openstack provider for qemu: https://github.com/coreos/ignition/blob/3e633de1adb2afe8edcc184e3e8a28c40b96ea6b/internal/providers/qemu/qemu.go |
s390x doesn't support the qemu fw_cfg mechanism. Let's hack around this for now by having the fetcher on s390x just use the same one as OpenStack, i.e. config drives (and technically metadata server too, which will fail... again, this is a short-term hack). For background, see: coreos/coreos-assembler#1004 coreos/ignition-dracut#145 coreos#825
OK, I threw up coreos/ignition#905 since it wasn't too hard, but of course I'm not able to test it. Now, if we want to go that way for now, getting it backported to 4.2 would indeed take some work. If you need this fixed right now, I'm good getting this hack in assuming it stays in |
@@ -242,10 +242,16 @@ img_base=tmp/${imageprefix}-base.qcow2 | |||
# forgive me for this sin | |||
checksum_location=$(find /usr/lib/coreos-assembler-anaconda/ -name '*CHECKSUM' | head -1) | |||
|
|||
# Hack to force qemu to default to config drive based install on s390x |
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.
Link to some of the PRs where discussions took place here?
48641a1
to
92e2c47
Compare
Alternative pull request to coreos/ignition-dracut#145