From 461976d2affe3ed4a354608d1dcb266e06f1d2b9 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Thu, 3 Mar 2016 14:18:10 +0000 Subject: [PATCH] Optimize slow bottleneck test of DockerHubPullSuite.TestPullNonExistingImage. This PR fix the DockerHubPullSuite.TestPullNonExistingImage test in #19425. The majority of the execution time in this test is from multiple executions of 'docker pull', each of which takes more than one second even though it tries to pull a non-existing image. Without changing the behavior of the 'docker pull' itself, this fix tries to execute the 'docker pull' command in parallel in order to speed up the execution of the overall test. Since each 'docker pull' is independent, executions in parallel should not alter the purpose of the test. Signed-off-by: Yong Tang --- integration-cli/docker_cli_pull_test.go | 67 +++++++++++++++++++------ 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/integration-cli/docker_cli_pull_test.go b/integration-cli/docker_cli_pull_test.go index 28aa89a712..43369c8ed6 100644 --- a/integration-cli/docker_cli_pull_test.go +++ b/integration-cli/docker_cli_pull_test.go @@ -4,6 +4,7 @@ import ( "fmt" "regexp" "strings" + "sync" "time" "github.com/docker/distribution/digest" @@ -39,30 +40,68 @@ func (s *DockerHubPullSuite) TestPullFromCentralRegistry(c *check.C) { // combinations of implicit tag and library prefix. func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) { testRequires(c, DaemonIsLinux) - for _, e := range []struct { + + type entry struct { Repo string Alias string - }{ + } + + entries := []entry{ {"library/asdfasdf", "asdfasdf:foobar"}, {"library/asdfasdf", "library/asdfasdf:foobar"}, {"library/asdfasdf", "asdfasdf"}, {"library/asdfasdf", "asdfasdf:latest"}, {"library/asdfasdf", "library/asdfasdf"}, {"library/asdfasdf", "library/asdfasdf:latest"}, - } { - out, err := s.CmdWithError("pull", e.Alias) - c.Assert(err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", out)) - // Hub returns 401 rather than 404 for nonexistent repos over - // the v2 protocol - but we should end up falling back to v1, - // which does return a 404. - c.Assert(out, checker.Contains, fmt.Sprintf("Error: image %s not found", e.Repo), check.Commentf("expected image not found error messages")) + } - // pull -a on a nonexistent registry should fall back as well + // The option field indicates "-a" or not. + type record struct { + e entry + option string + out string + err error + } + + // Execute 'docker pull' in parallel, pass results (out, err) and + // necessary information ("-a" or not, and the image name) to channel. + var group sync.WaitGroup + recordChan := make(chan record, len(entries)*2) + for _, e := range entries { + group.Add(1) + go func(e entry) { + defer group.Done() + out, err := s.CmdWithError("pull", e.Alias) + recordChan <- record{e, "", out, err} + }(e) if !strings.ContainsRune(e.Alias, ':') { - out, err := s.CmdWithError("pull", "-a", e.Alias) - c.Assert(err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", out)) - c.Assert(out, checker.Contains, fmt.Sprintf("Error: image %s not found", e.Repo), check.Commentf("expected image not found error messages")) - c.Assert(out, checker.Not(checker.Contains), "unauthorized", check.Commentf(`message should not contain "unauthorized"`)) + // pull -a on a nonexistent registry should fall back as well + group.Add(1) + go func(e entry) { + defer group.Done() + out, err := s.CmdWithError("pull", "-a", e.Alias) + recordChan <- record{e, "-a", out, err} + }(e) + } + } + + // Wait for completion + group.Wait() + close(recordChan) + + // Process the results (out, err). + for record := range recordChan { + if len(record.option) == 0 { + c.Assert(record.err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", record.out)) + // Hub returns 401 rather than 404 for nonexistent repos over + // the v2 protocol - but we should end up falling back to v1, + // which does return a 404. + c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s not found", record.e.Repo), check.Commentf("expected image not found error messages")) + } else { + // pull -a on a nonexistent registry should fall back as well + c.Assert(record.err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", record.out)) + c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s not found", record.e.Repo), check.Commentf("expected image not found error messages")) + c.Assert(record.out, checker.Not(checker.Contains), "unauthorized", check.Commentf(`message should not contain "unauthorized"`)) } }