The Wayback Machine - https://web.archive.org/web/20251014201011/https://github.com/moby/moby/pull/32881
Skip to content

Conversation

coolljt0725
Copy link
Contributor

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}

- 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)

@GordonTheTurtle GordonTheTurtle added dco/no Automatically set by a bot when one of the commits lacks proper signature status/0-triage labels Apr 27, 2017
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Apr 27, 2017
Copy link
Contributor

Choose a reason for hiding this comment

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

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)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's better, done

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 <leijitang@huawei.com>
Copy link
Contributor

@boaz0 boaz0 left a comment

Choose a reason for hiding this comment

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

LGTM.
Another way I could think of is to handle the error coming from getExecConfig but that's much cleaner!

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 53a75ee into moby:master Jul 13, 2017
@coolljt0725 coolljt0725 deleted the fix_exec_faild branch July 13, 2017 02:33
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 <leijitang@huawei.com>
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.

9 participants