From f3ddace46876373f481fbbd4be24d3227b5567b9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 12 Jun 2017 14:47:27 +0200 Subject: [PATCH 1/2] Use tempdir instead of working directory as build-context The `makefile()` utility was used to create a temporary Dockerfile, and after tests completed, this file was deleted. However, the _build_ used the current path (`/usr/local/bin/docker`) as build-context. As a result, roughtly 20 MB was sent as build-context for each build, but none of the builds actually required a build-context. This patch; - creates a temp-dir for the test, which can be used as build-context - changes the `makefile()` utility and removes the `cleanup` functionality - instead, the `temp-dir` is removed after the test finishes (which also removes the temporary `Dockerfile`) Signed-off-by: Sebastiaan van Stijn Upstream-commit: ebe66b1d0f52dc58a98a428d4efa4d2f2743b96e Component: engine --- .../docker_cli_registry_user_agent_test.go | 15 ++++--- .../docker_cli_v2_only_test.go | 44 ++++++++----------- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_registry_user_agent_test.go b/components/engine/integration-cli/docker_cli_registry_user_agent_test.go index 9f50aa676a..62ec130e51 100644 --- a/components/engine/integration-cli/docker_cli_registry_user_agent_test.go +++ b/components/engine/integration-cli/docker_cli_registry_user_agent_test.go @@ -2,7 +2,9 @@ package main import ( "fmt" + "io/ioutil" "net/http" + "os" "regexp" "github.com/docker/docker/integration-cli/registry" @@ -100,10 +102,14 @@ func (s *DockerRegistrySuite) TestUserAgentPassThrough(c *check.C) { "--insecure-registry", pushReg.URL(), "--insecure-registry", loginReg.URL()) - dockerfileName, cleanup1, err := makefile(fmt.Sprintf("FROM %s", buildRepoName)) + tmp, err := ioutil.TempDir("", "integration-cli-") + c.Assert(err, check.IsNil) + defer os.RemoveAll(tmp) + + dockerfileName, err := makefile(tmp, fmt.Sprintf("FROM %s", buildRepoName)) c.Assert(err, check.IsNil, check.Commentf("Unable to create test dockerfile")) - defer cleanup1() - s.d.Cmd("build", "--file", dockerfileName, ".") + + s.d.Cmd("build", "--file", dockerfileName, tmp) regexpCheckUA(c, buildUA) s.d.Cmd("login", "-u", "richard", "-p", "testtest", loginReg.URL()) @@ -112,10 +118,9 @@ func (s *DockerRegistrySuite) TestUserAgentPassThrough(c *check.C) { s.d.Cmd("pull", pullRepoName) regexpCheckUA(c, pullUA) - dockerfileName, cleanup2, err := makefile(`FROM scratch + dockerfileName, err = makefile(tmp, `FROM scratch ENV foo bar`) c.Assert(err, check.IsNil, check.Commentf("Unable to create test dockerfile")) - defer cleanup2() s.d.Cmd("build", "-t", pushRepoName, "--file", dockerfileName, ".") s.d.Cmd("push", pushRepoName) diff --git a/components/engine/integration-cli/docker_cli_v2_only_test.go b/components/engine/integration-cli/docker_cli_v2_only_test.go index 3500e787ca..b82cdbde1f 100644 --- a/components/engine/integration-cli/docker_cli_v2_only_test.go +++ b/components/engine/integration-cli/docker_cli_v2_only_test.go @@ -10,28 +10,16 @@ import ( "github.com/go-check/check" ) -func makefile(contents string) (string, func(), error) { - cleanup := func() { - - } - - f, err := ioutil.TempFile(".", "tmp") +func makefile(path string, contents string) (string, error) { + f, err := ioutil.TempFile(path, "tmp") if err != nil { - return "", cleanup, err + return "", err } err = ioutil.WriteFile(f.Name(), []byte(contents), os.ModePerm) if err != nil { - return "", cleanup, err + return "", err } - - cleanup = func() { - err := os.Remove(f.Name()) - if err != nil { - fmt.Println("Error removing tmpfile") - } - } - return f.Name(), cleanup, nil - + return f.Name(), nil } // TestV2Only ensures that a daemon by default does not @@ -53,11 +41,14 @@ func (s *DockerRegistrySuite) TestV2Only(c *check.C) { s.d.Start(c, "--insecure-registry", reg.URL()) - dockerfileName, cleanup, err := makefile(fmt.Sprintf("FROM %s/busybox", reg.URL())) - c.Assert(err, check.IsNil, check.Commentf("Unable to create test dockerfile")) - defer cleanup() + tmp, err := ioutil.TempDir("", "integration-cli-") + c.Assert(err, check.IsNil) + defer os.RemoveAll(tmp) - s.d.Cmd("build", "--file", dockerfileName, ".") + dockerfileName, err := makefile(tmp, fmt.Sprintf("FROM %s/busybox", reg.URL())) + c.Assert(err, check.IsNil, check.Commentf("Unable to create test dockerfile")) + + s.d.Cmd("build", "--file", dockerfileName, tmp) s.d.Cmd("run", repoName) s.d.Cmd("login", "-u", "richard", "-p", "testtest", reg.URL()) @@ -102,11 +93,14 @@ func (s *DockerRegistrySuite) TestV1(c *check.C) { s.d.Start(c, "--insecure-registry", reg.URL(), "--disable-legacy-registry=false") - dockerfileName, cleanup, err := makefile(fmt.Sprintf("FROM %s/busybox", reg.URL())) - c.Assert(err, check.IsNil, check.Commentf("Unable to create test dockerfile")) - defer cleanup() + tmp, err := ioutil.TempDir("", "integration-cli-") + c.Assert(err, check.IsNil) + defer os.RemoveAll(tmp) - s.d.Cmd("build", "--file", dockerfileName, ".") + dockerfileName, err := makefile(tmp, fmt.Sprintf("FROM %s/busybox", reg.URL())) + c.Assert(err, check.IsNil, check.Commentf("Unable to create test dockerfile")) + + s.d.Cmd("build", "--file", dockerfileName, tmp) c.Assert(v1Repo, check.Equals, 1, check.Commentf("Expected v1 repository access after build")) repoName := fmt.Sprintf("%s/busybox", reg.URL()) From 01267bd230e481a2e5fdb241c301a96205fd3281 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 12 Jun 2017 15:02:51 +0200 Subject: [PATCH 2/2] Simplify DockerRegistrySuite.TestUserAgentPassThrough() This patch simplifies the test by; - re-using the registry-mock / handler - skipping the last `docker build`, which was only used to make sure a local image was present. Instead, the daemon is started with a `busybox` image loaded. Also added a comment, explaining why the mock always returns a 404 (hence, error/output-string should not be checked in the test), and made the mock return a valid/correctly formatted error response. Signed-off-by: Sebastiaan van Stijn Upstream-commit: 5d04fe73bf9fa7cff1b99206f39536aed807efb3 Component: engine --- .../docker_cli_registry_user_agent_test.go | 67 ++++++------------- 1 file changed, 21 insertions(+), 46 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_registry_user_agent_test.go b/components/engine/integration-cli/docker_cli_registry_user_agent_test.go index 62ec130e51..6cbe6e7e66 100644 --- a/components/engine/integration-cli/docker_cli_registry_user_agent_test.go +++ b/components/engine/integration-cli/docker_cli_registry_user_agent_test.go @@ -49,9 +49,14 @@ func regexpCheckUA(c *check.C, ua string) { c.Assert(bMatchUpstreamUA, check.Equals, true, check.Commentf("(Upstream) Docker Client User-Agent malformed")) } +// registerUserAgentHandler registers a handler for the `/v2/*` endpoint. +// Note that a 404 is returned to prevent the client to proceed. +// We are only checking if the client sent a valid User Agent string along +// with the request. func registerUserAgentHandler(reg *registry.Mock, result *string) { reg.RegisterHandler("/v2/", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(404) + w.Write([]byte(`{"errors":[{"code": "UNSUPPORTED","message": "this is a mock registry"}]}`)) var ua string for k, v := range r.Header { if k == "User-Agent" { @@ -66,63 +71,33 @@ func registerUserAgentHandler(reg *registry.Mock, result *string) { // a registry, the registry should see a User-Agent string of the form // [docker engine UA] UpstreamClientSTREAM-CLIENT([client UA]) func (s *DockerRegistrySuite) TestUserAgentPassThrough(c *check.C) { - var ( - buildUA string - pullUA string - pushUA string - loginUA string - ) + var ua string - buildReg, err := registry.NewMock(c) - defer buildReg.Close() + reg, err := registry.NewMock(c) + defer reg.Close() c.Assert(err, check.IsNil) - registerUserAgentHandler(buildReg, &buildUA) - buildRepoName := fmt.Sprintf("%s/busybox", buildReg.URL()) + registerUserAgentHandler(reg, &ua) + repoName := fmt.Sprintf("%s/busybox", reg.URL()) - pullReg, err := registry.NewMock(c) - defer pullReg.Close() - c.Assert(err, check.IsNil) - registerUserAgentHandler(pullReg, &pullUA) - pullRepoName := fmt.Sprintf("%s/busybox", pullReg.URL()) - - pushReg, err := registry.NewMock(c) - defer pushReg.Close() - c.Assert(err, check.IsNil) - registerUserAgentHandler(pushReg, &pushUA) - pushRepoName := fmt.Sprintf("%s/busybox", pushReg.URL()) - - loginReg, err := registry.NewMock(c) - defer loginReg.Close() - c.Assert(err, check.IsNil) - registerUserAgentHandler(loginReg, &loginUA) - - s.d.Start(c, - "--insecure-registry", buildReg.URL(), - "--insecure-registry", pullReg.URL(), - "--insecure-registry", pushReg.URL(), - "--insecure-registry", loginReg.URL()) + s.d.StartWithBusybox(c, "--insecure-registry", reg.URL()) tmp, err := ioutil.TempDir("", "integration-cli-") c.Assert(err, check.IsNil) defer os.RemoveAll(tmp) - dockerfileName, err := makefile(tmp, fmt.Sprintf("FROM %s", buildRepoName)) + dockerfile, err := makefile(tmp, fmt.Sprintf("FROM %s", repoName)) c.Assert(err, check.IsNil, check.Commentf("Unable to create test dockerfile")) - s.d.Cmd("build", "--file", dockerfileName, tmp) - regexpCheckUA(c, buildUA) + s.d.Cmd("build", "--file", dockerfile, tmp) + regexpCheckUA(c, ua) - s.d.Cmd("login", "-u", "richard", "-p", "testtest", loginReg.URL()) - regexpCheckUA(c, loginUA) + s.d.Cmd("login", "-u", "richard", "-p", "testtest", reg.URL()) + regexpCheckUA(c, ua) - s.d.Cmd("pull", pullRepoName) - regexpCheckUA(c, pullUA) + s.d.Cmd("pull", repoName) + regexpCheckUA(c, ua) - dockerfileName, err = makefile(tmp, `FROM scratch - ENV foo bar`) - c.Assert(err, check.IsNil, check.Commentf("Unable to create test dockerfile")) - s.d.Cmd("build", "-t", pushRepoName, "--file", dockerfileName, ".") - - s.d.Cmd("push", pushRepoName) - regexpCheckUA(c, pushUA) + s.d.Cmd("tag", "busybox", repoName) + s.d.Cmd("push", repoName) + regexpCheckUA(c, ua) }