Browse code

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>

Drew Erny authored on 2017/05/04 08:04:51
Showing 2 changed files
... ...
@@ -437,9 +437,20 @@ func (r *controller) Logs(ctx context.Context, publisher exec.LogPublisher, opti
437 437
 		return err
438 438
 	}
439 439
 
440
-	if err := r.waitReady(ctx); err != nil {
441
-		return errors.Wrap(err, "container not ready for logs")
442
-	}
440
+	// if we're following, wait for this container to be ready. there is a
441
+	// problem here: if the container will never be ready (for example, it has
442
+	// been totally deleted) then this will wait forever. however, this doesn't
443
+	// actually cause any UI issues, and shouldn't be a problem. the stuck wait
444
+	// will go away when the follow (context) is canceled.
445
+	if options.Follow {
446
+		if err := r.waitReady(ctx); err != nil {
447
+			return errors.Wrap(err, "container not ready for logs")
448
+		}
449
+	}
450
+	// if we're not following, we're not gonna wait for the container to be
451
+	// ready. just call logs. if the container isn't ready, the call will fail
452
+	// and return an error. no big deal, we don't care, we only want the logs
453
+	// we can get RIGHT NOW with no follow
443 454
 
444 455
 	logsContext, cancel := context.WithCancel(ctx)
445 456
 	msgs, err := r.adapter.logs(logsContext, options)
... ...
@@ -283,3 +283,52 @@ func (s *DockerSwarmSuite) TestServiceLogsTTY(c *check.C) {
283 283
 	// just expected.
284 284
 	c.Assert(result, icmd.Matches, icmd.Expected{Out: "out\r\nerr\r\n"})
285 285
 }
286
+
287
+func (s *DockerSwarmSuite) TestServiceLogsNoHangDeletedContainer(c *check.C) {
288
+	d := s.AddDaemon(c, true, true)
289
+
290
+	name := "TestServiceLogsNoHangDeletedContainer"
291
+
292
+	result := icmd.RunCmd(d.Command(
293
+		// create a service
294
+		"service", "create",
295
+		// name it $name
296
+		"--name", name,
297
+		// busybox image, shell string
298
+		"busybox", "sh", "-c",
299
+		// echo to stdout and stderr
300
+		"while true; do echo line; sleep 2; done",
301
+	))
302
+
303
+	// confirm that the command succeeded
304
+	c.Assert(result, icmd.Matches, icmd.Expected{})
305
+	// get the service id
306
+	id := strings.TrimSpace(result.Stdout())
307
+	c.Assert(id, checker.Not(checker.Equals), "")
308
+
309
+	// make sure task has been deployed.
310
+	waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1)
311
+	// and make sure we have all the log lines
312
+	waitAndAssert(c, defaultReconciliationTimeout, countLogLines(d, name), checker.Equals, 2)
313
+
314
+	// now find and nuke the container
315
+	result = icmd.RunCmd(d.Command("ps", "-q"))
316
+	containerID := strings.TrimSpace(result.Stdout())
317
+	c.Assert(containerID, checker.Not(checker.Equals), "")
318
+	result = icmd.RunCmd(d.Command("stop", containerID))
319
+	c.Assert(result, icmd.Matches, icmd.Expected{Out: containerID})
320
+	result = icmd.RunCmd(d.Command("rm", containerID))
321
+	c.Assert(result, icmd.Matches, icmd.Expected{Out: containerID})
322
+
323
+	// run logs. use tail 2 to make sure we don't try to get a bunch of logs
324
+	// somehow and slow down execution time
325
+	cmd := d.Command("service", "logs", "--tail", "2", id)
326
+	// start the command and then wait for it to finish with a 3 second timeout
327
+	result = icmd.StartCmd(cmd)
328
+	result = icmd.WaitOnCmd(3*time.Second, result)
329
+
330
+	// then, assert that the result matches expected. if the command timed out,
331
+	// if the command is timed out, result.Timeout will be true, but the
332
+	// Expected defaults to false
333
+	c.Assert(result, icmd.Matches, icmd.Expected{})
334
+}