Add running wait condition to container and exec#36147
Add running wait condition to container and exec#36147mlaventure wants to merge 2 commits intomoby:masterfrom
Conversation
Signed-off-by: Kenfe-Mickael Laventure <mickael.laventure@gmail.com>
|
|
This design looks good, and I was just thinking we needed a wait condition for running. |
|
The lint in the api are generated swagger files, can't find where to tell the linter to ignore it. |
599880f to
184e478
Compare
|
|
Signed-off-by: Kenfe-Mickael Laventure <mickael.laventure@gmail.com>
184e478 to
36da0ce
Compare
|
thanks @dnephin! updated. |
|
Discussing this in the maintainers meeting, and we'd prefer the fix for the issue to be implemented without introducing a new API endpoint (that's a new feature, so should be looked at separately). We could fix the issue by making the "resize" endpoint wait for the exec to be started, and then perform the actual resize (as a separate PR so that we can keep the discussion on the "wait" endpoint separate) |
|
To be fair, this PR doesn't fix anything, the fix is in the cli by having
it use the new endpoint to avoid the race 😉.
But sure a different PR for he resize endpoint can be made and count as a
direct fix (although it'd be a behavioral change).
…On Thu, Feb 8, 2018, 12:06 PM Sebastiaan van Stijn ***@***.***> wrote:
Discussing this in the maintainers meeting, and we'd prefer the fix for
the issue to be implemented without introducing a new API endpoint (that's
a new feature, so should be looked at separately).
We could fix the issue by making the "resize" endpoint wait for the exec
to be started, and then perform the actual resize (as a separate PR so that
we can keep the discussion on the "wait" endpoint separate)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#36147 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/APUifl8Fr4yCAv1sA3jWoCnBlAcHF6L5ks5tS1PXgaJpZM4RyrpD>
.
--
Sent from my phone.
|
|
Looking at the possible Resize solution instead, I realize that neither my current 2 PRs or a potential change would really fix the issue. I had forgotten, but contrary to the initial process, Maybe if we modify |
|
I think it would be fair to block resize while the exec is in a "starting" (this would be new) state.... alternatively it could send back a typed error to know if the client should retry... The former requires no client changes. The exec start endpoint is hijacking the connection way before it's needed and as such sending a response back too soon. We could minimize this... probably would need to anyway to set a starting state before the http handler responds. |
| post: | ||
| summary: "Wait for a container" | ||
| description: "Block until a container stops, then returns the exit code." | ||
| description: "Block until a container reach a given condition, then returns the exit code." |
|
ping @tonistiigi @cpuguy83 PTAL |
|
Discussing in the maintainers meeting with @cpuguy83 and while this may not be the solution for the exec resize issue, having a wait for exec in general is a useful thing to have, so I'm moving this to code review |
…earing wonky on start due to container resize issue. resizing the terminal window fixes the issue (workaround). waiting for fix to moby/moby#36147
|
@mlaventure @tonistiigi guess we can close this one, now that #37172 was merged? |
|
Yep, closing |
- What I did
- How I did it
- How to verify it
- Description for the changelog
Add running condition as an option to contanier wait endpoint
Add exec wait endpoint to wait on exec running or exit state
- A picture of a cute animal (not mandatory but encouraged)
Fixes #35407