Ensure that exec sends resize events

We previously tried to send resize events only after the exec
session successfully started, which makes sense (we might drop an
event or two that came in before the exec session started
otherwise). However, the start function blocks, so waiting
actually means we send no resize events at all, which is
obviously worse than losing a few.. Sending resizes before attach
starts seems to work fine in my testing, so let's do that until we
get bug reports that it doesn't work.

Fixes #5584

Signed-off-by: Matthew Heon <mheon@redhat.com>
This commit is contained in:
Matthew Heon
2020-03-25 15:33:52 -04:00
parent ff0124aee1
commit 1313f8a450
2 changed files with 6 additions and 5 deletions

View File

@ -575,7 +575,7 @@ func (c *Container) ExecResize(sessionID string, newSize remotecommand.TerminalS
return errors.Wrapf(define.ErrNoSuchExecSession, "container %s has no exec session with ID %s", c.ID(), sessionID) return errors.Wrapf(define.ErrNoSuchExecSession, "container %s has no exec session with ID %s", c.ID(), sessionID)
} }
logrus.Infof("Removing container %s exec session %s", c.ID(), session.ID()) logrus.Infof("Resizing container %s exec session %s to %+v", c.ID(), session.ID(), newSize)
if session.State != define.ExecStateRunning { if session.State != define.ExecStateRunning {
return errors.Wrapf(define.ErrExecSessionStateInvalid, "cannot resize container %s exec session %s as it is not running", c.ID(), session.ID()) return errors.Wrapf(define.ErrExecSessionStateInvalid, "cannot resize container %s exec session %s as it is not running", c.ID(), session.ID())
@ -592,9 +592,6 @@ func (c *Container) Exec(config *ExecConfig, streams *AttachStreams, resize <-ch
if err != nil { if err != nil {
return -1, err return -1, err
} }
if err := c.ExecStartAndAttach(sessionID, streams); err != nil {
return -1, err
}
// Start resizing if we have a resize channel. // Start resizing if we have a resize channel.
// This goroutine may likely leak, given that we cannot close it here. // This goroutine may likely leak, given that we cannot close it here.
@ -605,6 +602,7 @@ func (c *Container) Exec(config *ExecConfig, streams *AttachStreams, resize <-ch
// session. // session.
if resize != nil { if resize != nil {
go func() { go func() {
logrus.Debugf("Sending resize events to exec session %s", sessionID)
for resizeRequest := range resize { for resizeRequest := range resize {
if err := c.ExecResize(sessionID, resizeRequest); err != nil { if err := c.ExecResize(sessionID, resizeRequest); err != nil {
// Assume the exec session went down. // Assume the exec session went down.
@ -615,6 +613,10 @@ func (c *Container) Exec(config *ExecConfig, streams *AttachStreams, resize <-ch
}() }()
} }
if err := c.ExecStartAndAttach(sessionID, streams); err != nil {
return -1, err
}
session, err := c.ExecSession(sessionID) session, err := c.ExecSession(sessionID)
if err != nil { if err != nil {
return -1, err return -1, err

View File

@ -793,7 +793,6 @@ func (r *ConmonOCIRuntime) ExecAttachResize(ctr *Container, sessionID string, ne
} }
defer controlFile.Close() defer controlFile.Close()
logrus.Debugf("Received a resize event for container %s exec session %s: %+v", ctr.ID(), sessionID, newSize)
if _, err = fmt.Fprintf(controlFile, "%d %d %d\n", 1, newSize.Height, newSize.Width); err != nil { if _, err = fmt.Fprintf(controlFile, "%d %d %d\n", 1, newSize.Height, newSize.Width); err != nil {
return errors.Wrapf(err, "failed to write to ctl file to resize terminal") return errors.Wrapf(err, "failed to write to ctl file to resize terminal")
} }