Skip to content

Commit

Permalink
fix: fix cri exec hang
Browse files Browse the repository at this point in the history
do not return error when execProcess.Start() fail, if exec process
started, let execExitedAndRelease do the exit hook.

Signed-off-by: Ace-Tang <[email protected]>
  • Loading branch information
Ace-Tang authored and starnop committed Sep 12, 2018
1 parent f44a8dd commit edfa142
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
20 changes: 12 additions & 8 deletions ctrd/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,18 @@ func (c *Client) execContainer(ctx context.Context, process *Process) error {
exitTime: status.ExitTime(),
}

// run hook if not got fail here
if err := <-fail; err == nil {
for _, hook := range c.hooks {
if err := hook(process.ExecID, msg); err != nil {
logrus.Errorf("failed to execute the exec exit hooks: %v", err)
break
}
if err := <-fail; err != nil {
msg.err = err
// exit code should not be zero when exec get failed.
if msg.exitCode == 0 {
msg.exitCode = 126
}
}
// XXX: if exec process get run, io should be closed in this function,
for _, hook := range c.hooks {
if err := hook(process.ExecID, msg); err != nil {
logrus.Errorf("failed to execute the exec exit hooks: %v", err)
break
}
}

Expand All @@ -141,7 +146,6 @@ func (c *Client) execContainer(ctx context.Context, process *Process) error {
// start the exec process
if err := execProcess.Start(ctx); err != nil {
fail <- err
return errors.Wrap(err, "failed to exec process")
}

return nil
Expand Down
14 changes: 10 additions & 4 deletions daemon/mgr/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package mgr
import (
"context"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"
Expand Down Expand Up @@ -34,6 +35,7 @@ import (
containerdtypes "github.com/containerd/containerd/api/types"
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/mount"
"github.com/docker/docker/pkg/stdcopy"
"github.com/docker/libnetwork"
"github.com/go-openapi/strfmt"
"github.com/imdario/mergo"
Expand Down Expand Up @@ -1855,17 +1857,21 @@ func (mgr *ContainerManager) execExitedAndRelease(id string, m *ctrd.Message) er
execConfig.Running = false
execConfig.Error = m.RawError()

io := mgr.IOs.Get(id)
if io == nil {
eio := mgr.IOs.Get(id)
if eio == nil {
return nil
}

if err := m.RawError(); err != nil {
fmt.Fprintf(io.Stdout, "%v\n", err)
var stdout io.Writer = eio.Stdout
if !execConfig.Tty && !eio.MuxDisabled {
stdout = stdcopy.NewStdWriter(stdout, stdcopy.Stdout)
}
stdout.Write([]byte(err.Error() + "\r\n"))
}

// close io
io.Close()
eio.Close()
mgr.IOs.Remove(id)

return nil
Expand Down

0 comments on commit edfa142

Please sign in to comment.