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

kvm stage1 should return pod exit status #2777

Open
lucab opened this Issue Jun 10, 2016 · 11 comments

Comments

Projects
None yet
8 participants
@lucab
Member

lucab commented Jun 10, 2016

While writing test cases, I realized that kvm flavors always return 0 even if the application exit with some other exit codes. Behavior should be aligned with non-kvm flavors, where user/caller gets a meaningful exit status upon termination.
I think @alban and others were already aware of this, but I didn't find any report tracking it.

/cc @jellonek

@tmrts

This comment has been minimized.

Contributor

tmrts commented Jun 23, 2016

@pskrzyns can you take a look at this one?

@pskrzyns

This comment has been minimized.

Contributor

pskrzyns commented Jun 23, 2016

I'm on vacation till monday :)
@jjlakis @lukasredynk could you look at it ?

@pskrzyns

This comment has been minimized.

Contributor

pskrzyns commented Jun 23, 2016

@tmrts @alban can we create team kvm on this repo ?

@tmrts

This comment has been minimized.

Contributor

tmrts commented Jun 24, 2016

@pskrzyns 👍

@tmrts tmrts added this to the v1.10.0 milestone Jun 24, 2016

@iaguis

This comment has been minimized.

Member

iaguis commented Jul 5, 2016

@coreos/rkt-kvm-maintainers can you set a proper milestone here?

@iaguis

This comment has been minimized.

Member

iaguis commented Jul 6, 2016

I'll move it to v1.12.0

@iaguis iaguis modified the milestones: v1.12.0, v1.10.0 Jul 6, 2016

@jjlakis

This comment has been minimized.

Contributor

jjlakis commented Jul 6, 2016

@iaguis Thanks

@lucab

This comment has been minimized.

Member

lucab commented Aug 3, 2016

Bump, @jjlakis any chance you can have a look at this?

@lucab lucab modified the milestones: v1.13.0, v1.12.0 Aug 3, 2016

@jellonek

This comment has been minimized.

Contributor

jellonek commented Aug 3, 2016

I see no other way to support this than running hypervisor engine (lkvm, qemu) by other process, and then collect status from remains of pod as this is already done by rkt status. So rkt binary would not exec directly to hypervisor engine, but to this overlapping helper binary. This looks annoying but seems to be very simple in implementing...
Imo this binary should be statically linked C program, small binary like enter, prepare-app because of golang overhead...

@lucab

This comment has been minimized.

Member

lucab commented Aug 15, 2016

I see. I was probably too optimistic asking for stage1 to just bubble-up the return value from systemd, but skimming a bit I realized lkvm/bzimage have no concept of a return value (uml has, though).

Thinking a bit more on this, the binary wrapper may not even be such a great solution because we don't want to introduce another long-running component, and it will miss anyway errors from failed stage1 setup (while stage2 status can be already retrieved the way you mentioned).

The original context for this was that we have some test units using this feature on the coreos flavor, and we end up not running some tests on kvm due to this. At this point, however, I think it is better to adjust the testsuite to be aware of this, instead of introducing other moving parts here.

De-milestoning but keeping this around in case we get better support from the intermediate layers in the future.

@grahamwhaley

This comment has been minimized.

grahamwhaley commented Jan 6, 2017

Just as an update/note here - the same situation exists for other VM container systems. In Clear Containers we have gone with the 'intermediate binary' approach, where we have an extra process that reaps the exit code and returns it to the higher level.
A fundamental issue is that (for instance) qemu does not return the exit code of its payload, but of itself - thus, directly running qemu will never get you the payload exit code, unless we could add a feature to qemu.
When we come to re-visit this issue we can look at re-using the Clear Containers code.
CC @jodh-intel

@alban alban referenced this issue Feb 6, 2017

Merged

Add semaphore.sh #3

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment