Forbid exec a restarting container #19722
Conversation
|
Before this, daemon will print error message like this when and client will silently fail. Comparing to After applying this fix, client print when And server will print similar error, which is more meaningful to user from my point of view. |
|
Integration test is so hard to write, I'll try but don't know whether I can make it. |
| // but the container is restarting. | ||
| ErrorCodeExecRestarting = errcode.Register(errGroup, errcode.ErrorDescriptor{ | ||
| Value: "EXECRESTARTING", | ||
| Message: "Container %s is restarting, wait the container to be running", |
There was a problem hiding this comment.
wait until the container is running
|
design looks good, the message needs an small improvement 😉 |
Currently if we exec a restarting container, client will fail silently, and daemon will print error that container can't be found which is not a very meaningful prompt to user. This commit will stop user from exec a restarting container and gives more explicit error message. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
|
LGTM w/ @calavera's suggestion. |
WeiZhang555
force-pushed
the
exec-restarting
branch
from
a2f464a
to
1d2208f
Compare
|
LGTM ping @cpuguy83 for the merge |
|
LGTM |
Forbid exec a restarting container
|
@thaJeztah I add a cherry-pick, but I'm not sure |
|
@coolljt0725 not sure either; I don't think this is a regression. It's a small change though, so we could consider it, but @tiborvass is already cherry-picking. I'll leave this one to @tiborvass to decide |
tiborvass
added
impact/changelog
status/6-cherry-picked
and removed
status/5-cherry-pick
labels
Currently if we exec a restarting container, client will fail silently,
and daemon will print error that container can't be found which is not a
very meaningful prompt to user.
This commit will stop user from exec a restarting container and gives
more explicit error message.
Signed-off-by: Zhang Wei zhangwei555@huawei.com