Enable inspect exec if container is pause/restarting/non-running#32881
Merged
Conversation
GordonTheTurtle
added
dco/no
coolljt0725
force-pushed
the
fix_exec_faild
branch
from
April 27, 2017 08:14
c290202 to
5719e7b
Compare
GordonTheTurtle
removed
the
dco/no
allencloud
reviewed
Apr 27, 2017
Contributor
There was a problem hiding this comment.
I am not sure that the design is ok. While I think here we can simplify the code like format:
e := daemon.execCommands.Get(id)
if e == nil {
return nil, errExecNotFound(id)
}
if container := daemon.containers.Get(e.ContainerID); container == nil {
return nil, errExecNotFound(id)
}
Contributor
Author
There was a problem hiding this comment.
That's better, done
coolljt0725
force-pushed
the
fix_exec_faild
branch
from
April 28, 2017 01:45
5719e7b to
0e87588
Compare
if docker exec exit and at the same the the container is pause,
there could be a chance the `docker exec exit` will fail
```
$ docker exec -ti 388c7f47a06c sh
/ # exit
Error response from daemon: Container 388c7f47a06cce0856266ffd56a2ce2901689ca7a6b9cd741b37652418448f2b is paused, unpause the container before exec
```
To reproduce this error easilly, we can add a sleep in `containerPause`
```
--- a/daemon/pause.go
+++ b/daemon/pause.go
@@ -2,6 +2,7 @@ package daemon
import (
"fmt"
+ "time"
"github.com/docker/docker/container"
)
@@ -25,7 +26,7 @@ func (daemon *Daemon) ContainerPause(name string) error {
func (daemon *Daemon) containerPause(container *container.Container) error {
container.Lock()
defer container.Unlock()
-
+ time.Sleep(time.Second * 5)
// We cannot Pause the container which is not running
if !container.Running {
return errNotRunning{container.ID}
```
Signed-off-by: Lei Jitang <[email protected]>
boaz0
approved these changes
May 16, 2017
boaz0
left a comment
boaz0
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
Another way I could think of is to handle the error coming from getExecConfig but that's much cleaner!
liusdu
pushed a commit
to liusdu/moby
that referenced
this pull request
Oct 30, 2017
if docker exec exit and at the same the the container is pause,
there could be a chance the `docker exec exit` will fail
```
$ docker exec -ti 388c7f47a06c sh
/ # exit
Error response from daemon: Container 388c7f47a06cce0856266ffd56a2ce2901689ca7a6b9cd741b37652418448f2b is paused, unpause the container before exec
```
To reproduce this error easilly, we can add a sleep in `containerPause`
```
--- a/daemon/pause.go
+++ b/daemon/pause.go
@@ -2,6 +2,7 @@ package daemon
import (
"fmt"
+ "time"
"github.com/docker/docker/container"
)
@@ -25,7 +26,7 @@ func (daemon *Daemon) ContainerPause(name string) error {
func (daemon *Daemon) containerPause(container *container.Container) error {
container.Lock()
defer container.Unlock()
-
+ time.Sleep(time.Second * 5)
// We cannot Pause the container which is not running
if !container.Running {
return errNotRunning{container.ID}
```
upstream pr moby#32881
Signed-off-by: Lei Jitang <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
if docker exec exit and at the same the the container is pause,
there could be a chance the
docker exec exitwill failTo reproduce this error easilly, we can add a sleep in
containerPause- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)