Skip to content

Conversation

@cgwalters
Copy link
Member

Pairs with coreos/ignition-dracut#146

What we really want is to use this in kola, will do as a
separate followup.

@cgwalters
Copy link
Member Author

To recap the benefit here, we can make it so that cosa kola run basic dies quickly with a clear error if something is wrong in the initramfs (in qemu) instead of timing out after 5 minutes.

@cgwalters
Copy link
Member Author

Hah, that last CI failure was because in the PXE testiso case we end up doing a hard poweroff from inside the system, so we'd get EOF from the channel before qemu exited. Fixed.


// WaitAll wraps the process exit as well as WaitIgnitionError,
// returning an error if either fail.
func (inst *QemuInstance) WaitAll() error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, do we actually need this new interface? Can we have it part of Wait() directly and have that return ErrInitramfsEmergency if Ignition failed? That way we automatically get this check everywhere we use and will use Wait().

Copy link
Member Author

@cgwalters cgwalters Apr 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny you ask. I actually did it that way first. And then I went to go test out a FCOS build failing in the initramfs (which btw if you want to conveniently test, use e.g.
cosa run --kargs ignition.config.url=blah://) and qemu started, printed an error about the initramfs failure and then exited...so I couldn't debug it.

After that I made it a separate interface, because for that reason we don't always want to die if there's a failure in the initramfs. (Now, theoretically the caller could distinguish and explicitly call Wait() again but that's racy because there's already another goroutine doing that...)

@cgwalters
Copy link
Member Author

Rebased 🏄‍♂️

@jlebon
Copy link
Member

jlebon commented Apr 3, 2020

Cool!
/lgtm

Pairs with coreos/ignition-dracut#146

This way, we error out fast if something went wrong in the initramfs
rather than timing out.  And further, we get the journal as JSON,
so we can do something intelligent in the future to analyze it.

And add a test case for this.
@cgwalters
Copy link
Member Author

Rebased 🏄‍♂️

@jlebon
Copy link
Member

jlebon commented Apr 3, 2020

/lgtm

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit e8d2893 into coreos:master Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants