From 57f1cfe3b1dd52b0e781a6b5bbd7ca6855a04d2d Mon Sep 17 00:00:00 2001
From: Zhang Wei <zhangwei555@huawei.com>
Date: Sun, 22 May 2016 22:04:39 +0800
Subject: [PATCH] Add detach event

If we attach to a running container and stream is closed afterwards, we
can never be sure if the container is stopped or detached. Adding a new
type of `detach` event can explicitly notify client that container is
detached, so client will know that there's no need to wait for its exit
code and it can move forward to next step now.

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
Upstream-commit: 83ad006d4724929ccbde4bdf768374fad0eeab44
Component: engine
---
 components/engine/container/container.go      | 22 ++++++++++++++-----
 components/engine/daemon/attach.go            | 21 +++++++++++++-----
 components/engine/daemon/exec.go              |  6 ++++-
 .../docker_cli_run_unix_test.go               |  5 +++++
 4 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/components/engine/container/container.go b/components/engine/container/container.go
index cf58ec1fe2..34bce015ad 100644
--- a/components/engine/container/container.go
+++ b/components/engine/container/container.go
@@ -48,6 +48,21 @@ var (
 	errInvalidNetwork  = fmt.Errorf("invalid network settings while building port map info")
 )
 
+// AttachError represents errors of attach
+type AttachError interface {
+	IsDetached() bool
+}
+
+type detachError struct{}
+
+func (e detachError) IsDetached() bool {
+	return true
+}
+
+func (e detachError) Error() string {
+	return "detached from container"
+}
+
 // CommonContainer holds the fields for a container which are
 // applicable across all platforms supported by the daemon.
 type CommonContainer struct {
@@ -393,7 +408,6 @@ func AttachStreams(ctx context.Context, streamConfig *runconfig.StreamConfig, op
 			_, err = copyEscapable(cStdin, stdin, keys)
 		} else {
 			_, err = io.Copy(cStdin, stdin)
-
 		}
 		if err == io.ErrClosedPipe {
 			err = nil
@@ -492,10 +506,8 @@ func copyEscapable(dst io.Writer, src io.ReadCloser, keys []byte) (written int64
 					break
 				}
 				if i == len(keys)-1 {
-					if err := src.Close(); err != nil {
-						return 0, err
-					}
-					return 0, nil
+					src.Close()
+					return 0, detachError{}
 				}
 				nr, er = src.Read(buf)
 			}
diff --git a/components/engine/daemon/attach.go b/components/engine/daemon/attach.go
index 73ae907401..dd5c6b45af 100644
--- a/components/engine/daemon/attach.go
+++ b/components/engine/daemon/attach.go
@@ -73,9 +73,9 @@ func (daemon *Daemon) ContainerAttachRaw(prefixOrName string, stdin io.ReadClose
 	return daemon.containerAttach(container, stdin, stdout, stderr, false, stream, nil)
 }
 
-func (daemon *Daemon) containerAttach(container *container.Container, stdin io.ReadCloser, stdout, stderr io.Writer, logs, stream bool, keys []byte) error {
+func (daemon *Daemon) containerAttach(c *container.Container, stdin io.ReadCloser, stdout, stderr io.Writer, logs, stream bool, keys []byte) error {
 	if logs {
-		logDriver, err := daemon.getLogger(container)
+		logDriver, err := daemon.getLogger(c)
 		if err != nil {
 			return err
 		}
@@ -105,7 +105,7 @@ func (daemon *Daemon) containerAttach(container *container.Container, stdin io.R
 		}
 	}
 
-	daemon.LogContainerEvent(container, "attach")
+	daemon.LogContainerEvent(c, "attach")
 
 	//stream
 	if stream {
@@ -119,11 +119,20 @@ func (daemon *Daemon) containerAttach(container *container.Container, stdin io.R
 			}()
 			stdinPipe = r
 		}
-		<-container.Attach(stdinPipe, stdout, stderr, keys)
+		err := <-c.Attach(stdinPipe, stdout, stderr, keys)
+		if err != nil {
+			e, ok := err.(container.AttachError)
+			if ok && e.IsDetached() {
+				daemon.LogContainerEvent(c, "detach")
+			} else {
+				logrus.Errorf("attach failed with error: %v", err)
+			}
+		}
+
 		// If we are in stdinonce mode, wait for the process to end
 		// otherwise, simply return
-		if container.Config.StdinOnce && !container.Config.Tty {
-			container.WaitStop(-1 * time.Second)
+		if c.Config.StdinOnce && !c.Config.Tty {
+			c.WaitStop(-1 * time.Second)
 		}
 	}
 	return nil
diff --git a/components/engine/daemon/exec.go b/components/engine/daemon/exec.go
index fd09fd784d..b8b1b2994f 100644
--- a/components/engine/daemon/exec.go
+++ b/components/engine/daemon/exec.go
@@ -222,7 +222,11 @@ func (d *Daemon) ContainerExecStart(ctx context.Context, name string, stdin io.R
 		return fmt.Errorf("context cancelled")
 	case err := <-attachErr:
 		if err != nil {
-			return fmt.Errorf("attach failed with error: %v", err)
+			e, ok := err.(container.AttachError)
+			if !ok || !e.IsDetached() {
+				return fmt.Errorf("attach failed with error: %v", err)
+			}
+			d.LogContainerEvent(c, "detach")
 		}
 	}
 	return nil
diff --git a/components/engine/integration-cli/docker_cli_run_unix_test.go b/components/engine/integration-cli/docker_cli_run_unix_test.go
index b1446c9137..4af37cc230 100644
--- a/components/engine/integration-cli/docker_cli_run_unix_test.go
+++ b/components/engine/integration-cli/docker_cli_run_unix_test.go
@@ -135,6 +135,11 @@ func (s *DockerSuite) TestRunAttachDetach(c *check.C) {
 
 	running := inspectField(c, name, "State.Running")
 	c.Assert(running, checker.Equals, "true", check.Commentf("expected container to still be running"))
+
+	out, _ = dockerCmd(c, "events", "--since=0", "--until", daemonUnixTime(c), "-f", "container="+name)
+	// attach and detach event should be monitored
+	c.Assert(out, checker.Contains, "attach")
+	c.Assert(out, checker.Contains, "detach")
 }
 
 // TestRunDetach checks attaching and detaching with the escape sequence specified via flags.