Merge pull request #33006 from dperny/service-logs-fix-hanging-logs-removed-containers-no-wait

Fix an issue with service logs hanging

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)

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