From 269960a4c63dac35e16844eee807d4ecbf68f498 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 26 Nov 2024 13:04:44 +0100 Subject: [PATCH] integration-cli: TestConcurrentPush: refactor to improve failure logs This tests, when failing, only produced a non-informative "exit status 1", which limits investigating why it failed. This patch: - Rewrites the parallel pushes to use an error-group, and asserts each push to get the failure output of the command. - Simplifies the Dockerfile that's used for building the test-image, removing steps that were not needed for the test. - Adds a cleanup step to make sure the images are deleted after the test, or if the test fails (although the test-suite should already handle this). Before this, a failure looked like: make BIND_DIR=. TEST_FILTER='TestConcurrentPush' test-integration-cli === FAIL: arm64.integration-cli TestDockerRegistrySuite/TestConcurrentPush (5.49s) docker_cli_push_test.go:159: assertion failed: error is not nil: exit status 1: concurrent push failed with error: exit status 1 check_test.go:476: [dfa779e71fdf8] daemon is not started --- FAIL: TestDockerRegistrySuite/TestConcurrentPush (5.49s) With this patch applied: make BIND_DIR=. TEST_FILTER='TestConcurrentPush' test-integration-cli === FAIL: arm64.integration-cli TestDockerRegistrySuite/TestConcurrentPush (2.47s) docker_cli_push_test.go:156: assertion failed: Command: /usr/local/cli-integration/docker push 127.0.0.1:5000/dockercli/busybox:push2nosuch ExitCode: 1 Error: exit status 1 Stdout: The push refers to repository 127.0.0.1:5000/dockercli/busybox Stderr: tag does not exist: 127.0.0.1:5000/dockercli/busybox:push2nosuch Failures: ExitCode was 1 expected 0 Expected no error docker_cli_push_test.go:160: assertion failed: error is not nil: exit status 1 check_test.go:476: [db77ef03a8fd8] daemon is not started --- FAIL: TestDockerRegistrySuite/TestConcurrentPush (2.47s) Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_cli_push_test.go | 38 ++++++++++++------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/integration-cli/docker_cli_push_test.go b/integration-cli/docker_cli_push_test.go index f90913753d..ddd72b385b 100644 --- a/integration-cli/docker_cli_push_test.go +++ b/integration-cli/docker_cli_push_test.go @@ -15,6 +15,7 @@ import ( "github.com/moby/moby/api/types/versions" "github.com/moby/moby/v2/integration-cli/cli" "github.com/moby/moby/v2/integration-cli/cli/build" + "golang.org/x/sync/errgroup" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/icmd" @@ -134,41 +135,38 @@ func (s *DockerRegistrySuite) TestConcurrentPush(c *testing.T) { var repos []string for _, tag := range []string{"push1", "push2", "push3"} { repo := fmt.Sprintf("%v:%v", imgRepo, tag) - buildImageSuccessfully(c, repo, build.WithDockerfile(fmt.Sprintf(` - FROM busybox - ENTRYPOINT ["/bin/echo"] - ENV FOO foo - ENV BAR bar - CMD echo %s -`, repo))) + buildImageSuccessfully(c, repo, build.WithDockerfile(fmt.Sprintf("FROM busybox\nCMD echo hello from %s\n", repo))) repos = append(repos, repo) } + cleanup := func() { + args := append([]string{"rmi", "--force"}, repos...) + cli.DockerCmd(c, args...) + } + defer cleanup() + // Push tags, in parallel - results := make(chan error, len(repos)) - + var eg errgroup.Group for _, repo := range repos { - go func(repo string) { + eg.Go(func() error { result := icmd.RunCommand(dockerBinary, "push", repo) - results <- result.Error - }(repo) - } - - for range repos { - err := <-results - assert.NilError(c, err, "concurrent push failed with error: %v", err) + // check the result, but don't fail immediately (result.Assert) + // to prevent the errgroup from not completing. + assert.Check(c, result.Equal(icmd.Success)) + return result.Error + }) } + assert.NilError(c, eg.Wait()) // Clear local images store. - args := append([]string{"rmi"}, repos...) - cli.DockerCmd(c, args...) + cleanup() // Re-pull and run individual tags, to make sure pushes succeeded for _, repo := range repos { cli.DockerCmd(c, "pull", repo) cli.DockerCmd(c, "inspect", repo) out := cli.DockerCmd(c, "run", "--rm", repo).Combined() - assert.Equal(c, strings.TrimSpace(out), "/bin/sh -c echo "+repo) + assert.Equal(c, strings.TrimSpace(out), "hello from "+repo) } }