From 80c3ec027d9fd4f7ea2080adc08fc741f8909b2e Mon Sep 17 00:00:00 2001
From: Drew Erny <drew.erny@docker.com>
Date: Wed, 3 May 2017 16:04:51 -0700
Subject: [PATCH] Fix an issue with service logs hanging

Fixed an issue where service logs would hang if the container backing a
task was deleted by not waiting for containers to be ready if we're not
following logs.

Signed-off-by: Drew Erny <drew.erny@docker.com>
---
 .../cluster/executor/container/controller.go  | 15 +++++-
 .../docker_cli_service_logs_test.go           | 49 +++++++++++++++++++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/daemon/cluster/executor/container/controller.go b/daemon/cluster/executor/container/controller.go
index 5c6f803509..d20040afbd 100644
--- a/daemon/cluster/executor/container/controller.go
+++ b/daemon/cluster/executor/container/controller.go
@@ -437,9 +437,20 @@ func (r *controller) Logs(ctx context.Context, publisher exec.LogPublisher, opti
 		return err
 	}
 
-	if err := r.waitReady(ctx); err != nil {
-		return errors.Wrap(err, "container not ready for logs")
+	// if we're following, wait for this container to be ready. there is a
+	// problem here: if the container will never be ready (for example, it has
+	// been totally deleted) then this will wait forever. however, this doesn't
+	// actually cause any UI issues, and shouldn't be a problem. the stuck wait
+	// will go away when the follow (context) is canceled.
+	if options.Follow {
+		if err := r.waitReady(ctx); err != nil {
+			return errors.Wrap(err, "container not ready for logs")
+		}
 	}
+	// if we're not following, we're not gonna wait for the container to be
+	// ready. just call logs. if the container isn't ready, the call will fail
+	// and return an error. no big deal, we don't care, we only want the logs
+	// we can get RIGHT NOW with no follow
 
 	logsContext, cancel := context.WithCancel(ctx)
 	msgs, err := r.adapter.logs(logsContext, options)
diff --git a/integration-cli/docker_cli_service_logs_test.go b/integration-cli/docker_cli_service_logs_test.go
index 340cbc035c..f320b3582c 100644
--- a/integration-cli/docker_cli_service_logs_test.go
+++ b/integration-cli/docker_cli_service_logs_test.go
@@ -283,3 +283,52 @@ func (s *DockerSwarmSuite) TestServiceLogsTTY(c *check.C) {
 	// just expected.
 	c.Assert(result, icmd.Matches, icmd.Expected{Out: "out\r\nerr\r\n"})
 }
+
+func (s *DockerSwarmSuite) TestServiceLogsNoHangDeletedContainer(c *check.C) {
+	d := s.AddDaemon(c, true, true)
+
+	name := "TestServiceLogsNoHangDeletedContainer"
+
+	result := icmd.RunCmd(d.Command(
+		// create a service
+		"service", "create",
+		// name it $name
+		"--name", name,
+		// busybox image, shell string
+		"busybox", "sh", "-c",
+		// echo to stdout and stderr
+		"while true; do echo line; sleep 2; done",
+	))
+
+	// confirm that the command succeeded
+	c.Assert(result, icmd.Matches, icmd.Expected{})
+	// get the service id
+	id := strings.TrimSpace(result.Stdout())
+	c.Assert(id, checker.Not(checker.Equals), "")
+
+	// make sure task has been deployed.
+	waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1)
+	// and make sure we have all the log lines
+	waitAndAssert(c, defaultReconciliationTimeout, countLogLines(d, name), checker.Equals, 2)
+
+	// now find and nuke the container
+	result = icmd.RunCmd(d.Command("ps", "-q"))
+	containerID := strings.TrimSpace(result.Stdout())
+	c.Assert(containerID, checker.Not(checker.Equals), "")
+	result = icmd.RunCmd(d.Command("stop", containerID))
+	c.Assert(result, icmd.Matches, icmd.Expected{Out: containerID})
+	result = icmd.RunCmd(d.Command("rm", containerID))
+	c.Assert(result, icmd.Matches, icmd.Expected{Out: containerID})
+
+	// run logs. use tail 2 to make sure we don't try to get a bunch of logs
+	// somehow and slow down execution time
+	cmd := d.Command("service", "logs", "--tail", "2", id)
+	// start the command and then wait for it to finish with a 3 second timeout
+	result = icmd.StartCmd(cmd)
+	result = icmd.WaitOnCmd(3*time.Second, result)
+
+	// then, assert that the result matches expected. if the command timed out,
+	// if the command is timed out, result.Timeout will be true, but the
+	// Expected defaults to false
+	c.Assert(result, icmd.Matches, icmd.Expected{})
+}