From 496a4bd15e57890ae0c2320a7d4c25e3b9d2838f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 18 Feb 2022 23:23:32 +0100 Subject: [PATCH 1/2] integration-cli: TestSlowStdinClosing: add logs, and potential naming conflict This test has become quite flaky on Windows / Windows with Containerd. Looking at the test, I noticed that it's running a test three times (according to the comment "as it failed ~ 50% of the time"). However; - it uses the `--rm` option to clean up the container after it terminated - it uses a fixed name for the containers that are started I had a quick look at the issue that it was created for, and neither of those options were mentioned in the reported bug (so are just part of the test setup). I think the test was written when the `--rm` option was still client-side, in which case the cli would not terminate until it removed the container (making the remove synchronous). Current versions of docker have moved the `--rm` to the daemon side, and (if I'm not mistaken) performed asynchronous, and therefore could potentially cause a conflicting name. This patch: - removes the fixed name (the test doesn't require the container to have a specific name, so we can just use a random name) - adds logs to capture the stderr and stdout output of the run (so that we're able to capture failure messages). Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_cli_run_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 7977f73bb2..1a1a479a71 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -4157,17 +4157,19 @@ func (s *DockerSuite) TestRunEmptyEnv(c *testing.T) { // #28658 func (s *DockerSuite) TestSlowStdinClosing(c *testing.T) { - name := "testslowstdinclosing" - repeat := 3 // regression happened 50% of the time + const repeat = 3 // regression happened 50% of the time for i := 0; i < repeat; i++ { cmd := icmd.Cmd{ - Command: []string{dockerBinary, "run", "--rm", "--name", name, "-i", "busybox", "cat"}, + Command: []string{dockerBinary, "run", "--rm", "-i", "busybox", "cat"}, Stdin: &delayedReader{}, } done := make(chan error, 1) go func() { - err := icmd.RunCmd(cmd).Error - done <- err + result := icmd.RunCmd(cmd) + if out := result.Combined(); out != "" { + c.Log(out) + } + done <- result.Error }() select { From 3f0abde50d9256437e48262be965a56bb2884f63 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 20 Feb 2022 13:30:16 +0100 Subject: [PATCH 2/2] integration-cli: TestSlowStdinClosing: use sub-tests Use sub-tests so that the iterations can run in parallel (instead of sequential), and to make failures show up for the iteration that they're part of. Note that changing to subtests means that we'll always run 3 iterations of the test, and no longer fail early (but the test still fails if any of those iterations fails. Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_cli_run_test.go | 36 ++++++++++++++------------ 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 1a1a479a71..6a93ac0654 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -4159,25 +4159,27 @@ func (s *DockerSuite) TestRunEmptyEnv(c *testing.T) { func (s *DockerSuite) TestSlowStdinClosing(c *testing.T) { const repeat = 3 // regression happened 50% of the time for i := 0; i < repeat; i++ { - cmd := icmd.Cmd{ - Command: []string{dockerBinary, "run", "--rm", "-i", "busybox", "cat"}, - Stdin: &delayedReader{}, - } - done := make(chan error, 1) - go func() { - result := icmd.RunCmd(cmd) - if out := result.Combined(); out != "" { - c.Log(out) + c.Run(strconv.Itoa(i), func(c *testing.T) { + cmd := icmd.Cmd{ + Command: []string{dockerBinary, "run", "--rm", "-i", "busybox", "cat"}, + Stdin: &delayedReader{}, } - done <- result.Error - }() + done := make(chan error, 1) + go func() { + result := icmd.RunCmd(cmd) + if out := result.Combined(); out != "" { + c.Log(out) + } + done <- result.Error + }() - select { - case <-time.After(30 * time.Second): - c.Fatal("running container timed out") // cleanup in teardown - case err := <-done: - assert.NilError(c, err) - } + select { + case <-time.After(30 * time.Second): + c.Fatal("running container timed out") // cleanup in teardown + case err := <-done: + assert.NilError(c, err) + } + }) } }