Merge pull request #36875 from vdemeester/integration-cli-some-fixme

[test/integration-cli] small cleanups of FIXME(s)

integration-cli/check_test.go

@@ -350,14 +350,7 @@ func (s *DockerSwarmSuite) TearDownTest(c *check.C) {
 	for _, d := range s.daemons {
 		if d != nil {
 			d.Stop(c)
-			// FIXME(vdemeester) should be handled by SwarmDaemon ?
-			// raft state file is quite big (64MB) so remove it after every test
-			walDir := filepath.Join(d.Root, "swarm/raft/wal")
-			if err := os.RemoveAll(walDir); err != nil {
-				c.Logf("error removing %v: %v", walDir, err)
-			}
-
-			d.CleanupExecRoot(c)
+			d.Cleanup(c)
 		}
 	}
 	s.daemons = nil

integration-cli/daemon/daemon.go

@@ -2,7 +2,6 @@ package daemon // import "github.com/docker/docker/integration-cli/daemon"
 
 import (
 	"fmt"
-	"os/exec"
 	"strings"
 	"time"
 
@@ -88,19 +87,6 @@ func (d *Daemon) inspectFieldWithError(name, field string) (string, error) {
 	return d.inspectFilter(name, fmt.Sprintf(".%s", field))
 }
 
-// BuildImageWithOut builds an image with the specified dockerfile and options and returns the output
-func (d *Daemon) BuildImageWithOut(name, dockerfile string, useCache bool, buildFlags ...string) (string, int, error) {
-	buildCmd := BuildImageCmdWithHost(d.dockerBinary, name, dockerfile, d.Sock(), useCache, buildFlags...)
-	result := icmd.RunCmd(icmd.Cmd{
-		Command: buildCmd.Args,
-		Env:     buildCmd.Env,
-		Dir:     buildCmd.Dir,
-		Stdin:   buildCmd.Stdin,
-		Stdout:  buildCmd.Stdout,
-	})
-	return result.Combined(), result.ExitCode, result.Error
-}
-
 // CheckActiveContainerCount returns the number of active containers
 // FIXME(vdemeester) should re-use ActivateContainers in some way
 func (d *Daemon) CheckActiveContainerCount(c *check.C) (interface{}, check.CommentInterface) {
@@ -155,22 +141,3 @@ func WaitInspectWithArgs(dockerBinary, name, expr, expected string, timeout time
 	}
 	return nil
 }
-
-// BuildImageCmdWithHost create a build command with the specified arguments.
-// Deprecated
-// FIXME(vdemeester) move this away
-func BuildImageCmdWithHost(dockerBinary, name, dockerfile, host string, useCache bool, buildFlags ...string) *exec.Cmd {
-	args := []string{}
-	if host != "" {
-		args = append(args, "--host", host)
-	}
-	args = append(args, "build", "-t", name)
-	if !useCache {
-		args = append(args, "--no-cache")
-	}
-	args = append(args, buildFlags...)
-	args = append(args, "-")
-	buildCmd := exec.Command(dockerBinary, args...)
-	buildCmd.Stdin = strings.NewReader(dockerfile)
-	return buildCmd
-}

integration-cli/docker_api_swarm_service_test.go

@@ -11,9 +11,12 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/integration-cli/checker"
+	"github.com/docker/docker/integration-cli/cli"
+	"github.com/docker/docker/integration-cli/cli/build"
 	"github.com/docker/docker/integration-cli/daemon"
 	testdaemon "github.com/docker/docker/internal/test/daemon"
 	"github.com/go-check/check"
+	"github.com/gotestyourself/gotestyourself/icmd"
 	"golang.org/x/net/context"
 	"golang.org/x/sys/unix"
 )
@@ -205,12 +208,12 @@ func (s *DockerSwarmSuite) TestAPISwarmServicesUpdateStartFirst(c *check.C) {
 	image2 := "testhealth:latest"
 
 	// service started from this image won't pass health check
-	_, _, err := d.BuildImageWithOut(image2,
-		`FROM busybox
+	result := cli.BuildCmd(c, image2, cli.Daemon(d),
+		build.WithDockerfile(`FROM busybox
 		HEALTHCHECK --interval=1s --timeout=30s --retries=1024 \
-		  CMD cat /status`,
-		true)
-	c.Check(err, check.IsNil)
+		  CMD cat /status`),
+	)
+	result.Assert(c, icmd.Success)
 
 	// create service
 	instances := 5

integration-cli/docker_api_swarm_test.go

@@ -7,7 +7,6 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
-	"os"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -810,10 +809,6 @@ func (s *DockerSwarmSuite) TestAPISwarmRestartCluster(c *check.C) {
 				if err := daemon.StopWithError(); err != nil {
 					errs <- err
 				}
-				// FIXME(vdemeester) This is duplicated…
-				if root := os.Getenv("DOCKER_REMAP_ROOT"); root != "" {
-					daemon.Root = filepath.Dir(daemon.Root)
-				}
 			}(d)
 		}
 		wg.Wait()

integration-cli/docker_cli_daemon_test.go

@@ -31,6 +31,7 @@ import (
 	moby_daemon "github.com/docker/docker/daemon"
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/cli"
+	"github.com/docker/docker/integration-cli/cli/build"
 	"github.com/docker/docker/integration-cli/daemon"
 	testdaemon "github.com/docker/docker/internal/test/daemon"
 	"github.com/docker/docker/opts"
@@ -1155,14 +1156,16 @@ func (s *DockerDaemonSuite) TestDaemonLoggingDriverNoneLogsError(c *check.C) {
 func (s *DockerDaemonSuite) TestDaemonLoggingDriverShouldBeIgnoredForBuild(c *check.C) {
 	s.d.StartWithBusybox(c, "--log-driver=splunk")
 
-	out, err := s.d.Cmd("build")
-	out, code, err := s.d.BuildImageWithOut("busyboxs", `
+	result := cli.BuildCmd(c, "busyboxs", cli.Daemon(s.d),
+		build.WithDockerfile(`
         FROM busybox
-        RUN echo foo`, false)
-	comment := check.Commentf("Failed to build image. output %s, exitCode %d, err %v", out, code, err)
-	c.Assert(err, check.IsNil, comment)
-	c.Assert(code, check.Equals, 0, comment)
-	c.Assert(out, checker.Contains, "foo", comment)
+        RUN echo foo`),
+		build.WithoutCache,
+	)
+	comment := check.Commentf("Failed to build image. output %s, exitCode %d, err %v", result.Combined(), result.ExitCode, result.Error)
+	c.Assert(result.Error, check.IsNil, comment)
+	c.Assert(result.ExitCode, check.Equals, 0, comment)
+	c.Assert(result.Combined(), checker.Contains, "foo", comment)
 }
 
 func (s *DockerDaemonSuite) TestDaemonUnixSockCleanedUp(c *check.C) {
@@ -2403,12 +2406,16 @@ func (s *DockerDaemonSuite) TestDaemonMaxConcurrencyWithConfigFileReload(c *chec
 
 func (s *DockerDaemonSuite) TestBuildOnDisabledBridgeNetworkDaemon(c *check.C) {
 	s.d.StartWithBusybox(c, "-b=none", "--iptables=false")
-	out, code, err := s.d.BuildImageWithOut("busyboxs",
-		`FROM busybox
-                RUN cat /etc/hosts`, false)
-	comment := check.Commentf("Failed to build image. output %s, exitCode %d, err %v", out, code, err)
-	c.Assert(err, check.IsNil, comment)
-	c.Assert(code, check.Equals, 0, comment)
+
+	result := cli.BuildCmd(c, "busyboxs", cli.Daemon(s.d),
+		build.WithDockerfile(`
+        FROM busybox
+        RUN cat /etc/hosts`),
+		build.WithoutCache,
+	)
+	comment := check.Commentf("Failed to build image. output %s, exitCode %d, err %v", result.Combined(), result.ExitCode, result.Error)
+	c.Assert(result.Error, check.IsNil, comment)
+	c.Assert(result.ExitCode, check.Equals, 0, comment)
 }
 
 // Test case for #21976

integration-cli/docker_cli_prune_unix_test.go

@@ -12,8 +12,10 @@ import (
 
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/cli"
+	"github.com/docker/docker/integration-cli/cli/build"
 	"github.com/docker/docker/integration-cli/daemon"
 	"github.com/go-check/check"
+	"github.com/gotestyourself/gotestyourself/icmd"
 )
 
 func pruneNetworkAndVerify(c *check.C, d *daemon.Daemon, kept, pruned []string) {
@@ -79,13 +81,15 @@ func (s *DockerSwarmSuite) TestPruneNetwork(c *check.C) {
 func (s *DockerDaemonSuite) TestPruneImageDangling(c *check.C) {
 	s.d.StartWithBusybox(c)
 
-	out, _, err := s.d.BuildImageWithOut("test",
-		`FROM busybox
-                 LABEL foo=bar`, true, "-q")
-	c.Assert(err, checker.IsNil)
-	id := strings.TrimSpace(out)
+	result := cli.BuildCmd(c, "test", cli.Daemon(s.d),
+		build.WithDockerfile(`FROM busybox
+                 LABEL foo=bar`),
+		cli.WithFlags("-q"),
+	)
+	result.Assert(c, icmd.Success)
+	id := strings.TrimSpace(result.Combined())
 
-	out, err = s.d.Cmd("images", "-q", "--no-trunc")
+	out, err := s.d.Cmd("images", "-q", "--no-trunc")
 	c.Assert(err, checker.IsNil)
 	c.Assert(strings.TrimSpace(out), checker.Contains, id)
 
@@ -266,20 +270,24 @@ func (s *DockerSuite) TestPruneNetworkLabel(c *check.C) {
 func (s *DockerDaemonSuite) TestPruneImageLabel(c *check.C) {
 	s.d.StartWithBusybox(c)
 
-	out, _, err := s.d.BuildImageWithOut("test1",
-		`FROM busybox
-                 LABEL foo=bar`, true, "-q")
-	c.Assert(err, checker.IsNil)
-	id1 := strings.TrimSpace(out)
-	out, err = s.d.Cmd("images", "-q", "--no-trunc")
+	result := cli.BuildCmd(c, "test1", cli.Daemon(s.d),
+		build.WithDockerfile(`FROM busybox
+                 LABEL foo=bar`),
+		cli.WithFlags("-q"),
+	)
+	result.Assert(c, icmd.Success)
+	id1 := strings.TrimSpace(result.Combined())
+	out, err := s.d.Cmd("images", "-q", "--no-trunc")
 	c.Assert(err, checker.IsNil)
 	c.Assert(strings.TrimSpace(out), checker.Contains, id1)
 
-	out, _, err = s.d.BuildImageWithOut("test2",
-		`FROM busybox
-                 LABEL bar=foo`, true, "-q")
-	c.Assert(err, checker.IsNil)
-	id2 := strings.TrimSpace(out)
+	result = cli.BuildCmd(c, "test2", cli.Daemon(s.d),
+		build.WithDockerfile(`FROM busybox
+                 LABEL bar=foo`),
+		cli.WithFlags("-q"),
+	)
+	result.Assert(c, icmd.Success)
+	id2 := strings.TrimSpace(result.Combined())
 	out, err = s.d.Cmd("images", "-q", "--no-trunc")
 	c.Assert(err, checker.IsNil)
 	c.Assert(strings.TrimSpace(out), checker.Contains, id2)

integration-cli/docker_cli_service_health_test.go

@@ -9,7 +9,10 @@ import (
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/daemon/cluster/executor/container"
 	"github.com/docker/docker/integration-cli/checker"
+	"github.com/docker/docker/integration-cli/cli"
+	"github.com/docker/docker/integration-cli/cli/build"
 	"github.com/go-check/check"
+	"github.com/gotestyourself/gotestyourself/icmd"
 )
 
 // start a service, and then make its task unhealthy during running
@@ -20,15 +23,14 @@ func (s *DockerSwarmSuite) TestServiceHealthRun(c *check.C) {
 	d := s.AddDaemon(c, true, true)
 
 	// build image with health-check
-	// note: use `daemon.buildImageWithOut` to build, do not use `buildImage` to build
 	imageName := "testhealth"
-	_, _, err := d.BuildImageWithOut(imageName,
-		`FROM busybox
+	result := cli.BuildCmd(c, imageName, cli.Daemon(d),
+		build.WithDockerfile(`FROM busybox
 		RUN touch /status
 		HEALTHCHECK --interval=1s --timeout=1s --retries=1\
-		  CMD cat /status`,
-		true)
-	c.Check(err, check.IsNil)
+		  CMD cat /status`),
+	)
+	result.Assert(c, icmd.Success)
 
 	serviceName := "healthServiceRun"
 	out, err := d.Cmd("service", "create", "--no-resolve-image", "--detach=true", "--name", serviceName, imageName, "top")
@@ -84,12 +86,12 @@ func (s *DockerSwarmSuite) TestServiceHealthStart(c *check.C) {
 
 	// service started from this image won't pass health check
 	imageName := "testhealth"
-	_, _, err := d.BuildImageWithOut(imageName,
-		`FROM busybox
+	result := cli.BuildCmd(c, imageName, cli.Daemon(d),
+		build.WithDockerfile(`FROM busybox
 		HEALTHCHECK --interval=1s --timeout=1s --retries=1024\
-		  CMD cat /status`,
-		true)
-	c.Check(err, check.IsNil)
+		  CMD cat /status`),
+	)
+	result.Assert(c, icmd.Success)
 
 	serviceName := "healthServiceStart"
 	out, err := d.Cmd("service", "create", "--no-resolve-image", "--detach=true", "--name", serviceName, imageName, "top")

internal/test/daemon/daemon.go

@@ -176,9 +176,11 @@ func (d *Daemon) NewClientT(t assert.TestingT) *client.Client {
 	return c
 }
 
-// CleanupExecRoot cleans the daemon exec root (network namespaces, ...)
-func (d *Daemon) CleanupExecRoot(t testingT) {
-	cleanupExecRoot(t, d.execRoot)
+// Cleanup cleans the daemon files : exec root (network namespaces, ...), swarmkit files
+func (d *Daemon) Cleanup(t testingT) {
+	// Cleanup swarmkit wal files if present
+	cleanupRaftDir(t, d.Root)
+	cleanupNetworkNamespace(t, d.execRoot)
 }
 
 // Start starts the daemon and return once it is ready to receive requests.
@@ -201,6 +203,7 @@ func (d *Daemon) StartWithError(args ...string) error {
 
 // StartWithLogFile will start the daemon and attach its streams to a given file.
 func (d *Daemon) StartWithLogFile(out *os.File, providedArgs ...string) error {
+	d.handleUserns()
 	dockerdBinary, err := exec.LookPath(d.dockerdBinary)
 	if err != nil {
 		return errors.Wrapf(err, "[%s] could not find docker binary in $PATH", d.id)
@@ -446,7 +449,6 @@ out2:
 // If an error occurs while starting the daemon, the test will fail.
 func (d *Daemon) Restart(t testingT, args ...string) {
 	d.Stop(t)
-	d.handleUserns()
 	d.Start(t, args...)
 }
 
@@ -455,7 +457,6 @@ func (d *Daemon) RestartWithError(arg ...string) error {
 	if err := d.StopWithError(); err != nil {
 		return err
 	}
-	d.handleUserns()
 	return d.StartWithError(arg...)
 }
 
@@ -636,3 +637,10 @@ func (d *Daemon) Info(t assert.TestingT) types.Info {
 	assert.NilError(t, err)
 	return info
 }
+
+func cleanupRaftDir(t testingT, rootPath string) {
+	walDir := filepath.Join(rootPath, "swarm/raft/wal")
+	if err := os.RemoveAll(walDir); err != nil {
+		t.Logf("error removing %v: %v", walDir, err)
+	}
+}

internal/test/daemon/daemon_unix.go

@@ -9,7 +9,7 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-func cleanupExecRoot(t testingT, execRoot string) {
+func cleanupNetworkNamespace(t testingT, execRoot string) {
 	// Cleanup network namespaces in the exec root of this
 	// daemon because this exec root is specific to this
 	// daemon instance and has no chance of getting

internal/test/daemon/daemon_windows.go

@@ -21,5 +21,5 @@ func signalDaemonReload(pid int) error {
 	return fmt.Errorf("daemon reload not supported")
 }
 
-func cleanupExecRoot(t testingT, execRoot string) {
+func cleanupNetworkNamespace(t testingT, execRoot string) {
 }