-
Notifications
You must be signed in to change notification settings - Fork 187
kola/qemu: Pass ignition via blk device on non-fw_cfg platforms by default #1484
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
Conversation
jlebon
left a comment
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.
Cool, thanks! Yes, this is a nice cleanup now that there's a better way to pass the Ignition config.
Could you include the first paragraph of your comment as the commit message body?
Hmm, this is with using |
Without. |
…fault Move from injecting ingnition directly in to the VM image on non-fw_cfg architectures to passing it via virtio-blk device. It should be more performant and should be more robust going forward. See: coreos/ignition#905
Ahh OK. Yeah, with coreos/ignition#905, Ignition will just keep waiting until the device shows up (or eventually fail). This is by design. |
IIRC there has been mentions of the "disk" ignition being inherently prone to race conditions, reason why they are discouraged nowadays, is that right? Possibly the mentioned case/log? |
| panic("Ignition specified but no primary disk") | ||
| } else { | ||
| // Alternative to fw_cfg, should be generally usable on all arches, | ||
| // especially those without fw_cfg support. |
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.
Note it's technically not usable yet on arches that do support fw_cfg (see coreos/ignition#928 (comment)). But meh... not worth a respin.
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.
Oh... sorry bad wording on my side, should have been possibly...., will keep it on back of my mind and will fix it in some future PR.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcajka, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The ambiguity/race arises when the medium is optional. coreos/ignition#905 works around that by making it required, which is why that VM is failing. :) See coreos/ignition#928 for lots more discussions around this. |
I would like to propose to switch to virtio-blk for default ignition injection for non fw_cfg platforms. It should be more performant than injecting the ignition directly in to the VM image and IMHO it will be more resilient going forward. I'm currently not aware about any significant drawbacks compared to the current solution. I have tested this on x86_64, ppc64le, aarch64 and s390x with FCOS build/tests and have not observed any regressions.
CC'ing @Prashanth684
To add, I have recently started to see random kola test failures with FCOS(usually ~5 out of all kola tests) on non-fw_cfg platforms, caused by ignition failures. IIRC there has been reason why CoreOS nowadays preferred "non disk" based ignition injection, right? This seems to prevent these from happening.
Log snippet of noted issue follows.