From b813c398bb38ff5f73099e9cde61eb217551e3bf Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 24 Jul 2019 11:32:05 -0700 Subject: [PATCH 1/3] Add `FromClient` to test env execution While working on other tests I noticed that environment.Execution cannot be used for anything but the pre-configured daemon, however this can come in handy for being able share daemons across multiple tests that currently spin up a new daemon. The execution env also seems to be misused in some of these cases. Signed-off-by: Brian Goff (cherry picked from commit 1381956499d357dcae47dd1239d4f35b176fea7d) Signed-off-by: Sebastiaan van Stijn --- internal/test/environment/environment.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/test/environment/environment.go b/internal/test/environment/environment.go index 9bec84483f..3310590e7c 100644 --- a/internal/test/environment/environment.go +++ b/internal/test/environment/environment.go @@ -34,13 +34,18 @@ type PlatformDefaults struct { } // New creates a new Execution struct +// This is configured useing the env client (see client.FromEnv) func New() (*Execution, error) { - client, err := client.NewClientWithOpts(client.FromEnv) + c, err := client.NewClientWithOpts(client.FromEnv) if err != nil { return nil, errors.Wrapf(err, "failed to create client") } + return FromClient(c) +} - info, err := client.Info(context.Background()) +// FromClient creates a new Execution environment from the passed in client +func FromClient(c *client.Client) (*Execution, error) { + info, err := c.Info(context.Background()) if err != nil { return nil, errors.Wrapf(err, "failed to get info from daemon") } @@ -48,7 +53,7 @@ func New() (*Execution, error) { osType := getOSType(info) return &Execution{ - client: client, + client: c, DaemonInfo: info, OSType: osType, PlatformDefaults: getPlatformDefaults(info, osType), From 68e1150357c4dc56a5f9df3227af9c1b3f457252 Mon Sep 17 00:00:00 2001 From: HuanHuan Ye Date: Wed, 11 Sep 2019 19:40:11 +0800 Subject: [PATCH 2/3] DaemonCli: Move check into startMetricsServer Fix TODO: move into startMetricsServer() Fix errors.Wrap return nil when passed err is nil Co-Authored-By: Sebastiaan van Stijn Signed-off-by: HuanHuan Ye (cherry picked from commit 88c554f950d4667595cc2174bfab67d2cc072e62) Signed-off-by: Sebastiaan van Stijn --- cmd/dockerd/daemon.go | 14 ++++---------- cmd/dockerd/metrics.go | 11 ++++++++++- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 607604392f..b26e07186c 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -208,14 +208,10 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) { return errors.Wrap(err, "failed to validate authorization plugin") } - // TODO: move into startMetricsServer() - if cli.Config.MetricsAddress != "" { - if !d.HasExperimental() { - return errors.Wrap(err, "metrics-addr is only supported when experimental is enabled") - } - if err := startMetricsServer(cli.Config.MetricsAddress); err != nil { - return err - } + cli.d = d + + if err := cli.startMetricsServer(cli.Config.MetricsAddress); err != nil { + return err } c, err := createAndStartCluster(cli, d) @@ -230,8 +226,6 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) { logrus.Info("Daemon has completed initialization") - cli.d = d - routerOptions, err := newRouterOptions(cli.Config, d) if err != nil { return err diff --git a/cmd/dockerd/metrics.go b/cmd/dockerd/metrics.go index 20ceaf8466..1bfa37ea82 100644 --- a/cmd/dockerd/metrics.go +++ b/cmd/dockerd/metrics.go @@ -5,10 +5,19 @@ import ( "net/http" "github.com/docker/go-metrics" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) -func startMetricsServer(addr string) error { +func (cli *DaemonCli) startMetricsServer(addr string) error { + if addr == "" { + return nil + } + + if !cli.d.HasExperimental() { + return errors.New("metrics-addr is only supported when experimental is enabled") + } + if err := allocateDaemonPort(addr); err != nil { return err } From 4076c57b508e1753e123b078b43df714e7a14f2f Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 9 Sep 2019 16:41:57 -0700 Subject: [PATCH 3/3] Fix more signal handling issues in tests. Found these by doing a `grep -R 'using the force'` on a full test run. There's still a few more which are running against the main test daemon, so it is difficult to find which test they belong to. Signed-off-by: Brian Goff (cherry picked from commit fcd65ebf49a858c4f6223d1b1db728f7400a3b6d) Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_cli_daemon_test.go | 8 ++++---- integration-cli/docker_cli_network_unix_test.go | 4 ++-- .../docker_cli_service_create_test.go | 2 +- integration-cli/docker_cli_service_logs_test.go | 16 +++++++--------- integration-cli/docker_cli_userns_test.go | 4 ++-- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index e400c23088..da1668b162 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -427,7 +427,7 @@ func (s *DockerDaemonSuite) TestDaemonIPv6FixedCIDR(c *testing.T) { s.d.StartWithBusybox(c, "--ipv6", "--fixed-cidr-v6=2001:db8:2::/64", "--default-gateway-v6=2001:db8:2::100") - out, err := s.d.Cmd("run", "-itd", "--name=ipv6test", "busybox:latest") + out, err := s.d.Cmd("run", "-d", "--name=ipv6test", "busybox:latest", "top") assert.NilError(c, err, "Could not run container: %s, %v", out, err) out, err = s.d.Cmd("inspect", "--format", "{{.NetworkSettings.Networks.bridge.GlobalIPv6Address}}", "ipv6test") @@ -454,7 +454,7 @@ func (s *DockerDaemonSuite) TestDaemonIPv6FixedCIDRAndMac(c *testing.T) { s.d.StartWithBusybox(c, "--ipv6", "--fixed-cidr-v6=2001:db8:1::/64") - out, err := s.d.Cmd("run", "-itd", "--name=ipv6test", "--mac-address", "AA:BB:CC:DD:EE:FF", "busybox") + out, err := s.d.Cmd("run", "-d", "--name=ipv6test", "--mac-address", "AA:BB:CC:DD:EE:FF", "busybox", "top") assert.NilError(c, err, out) out, err = s.d.Cmd("inspect", "--format", "{{.NetworkSettings.Networks.bridge.GlobalIPv6Address}}", "ipv6test") @@ -469,7 +469,7 @@ func (s *DockerDaemonSuite) TestDaemonIPv6HostMode(c *testing.T) { deleteInterface(c, "docker0") s.d.StartWithBusybox(c, "--ipv6", "--fixed-cidr-v6=2001:db8:2::/64") - out, err := s.d.Cmd("run", "-itd", "--name=hostcnt", "--network=host", "busybox:latest") + out, err := s.d.Cmd("run", "-d", "--name=hostcnt", "--network=host", "busybox:latest", "top") assert.NilError(c, err, "Could not run container: %s, %v", out, err) out, err = s.d.Cmd("exec", "hostcnt", "ip", "-6", "addr", "show", "docker0") @@ -2703,7 +2703,7 @@ func (s *DockerDaemonSuite) TestExecWithUserAfterLiveRestore(c *testing.T) { testRequires(c, DaemonIsLinux) s.d.StartWithBusybox(c, "--live-restore") - out, err := s.d.Cmd("run", "-d", "--name=top", "busybox", "sh", "-c", "addgroup -S test && adduser -S -G test test -D -s /bin/sh && touch /adduser_end && top") + out, err := s.d.Cmd("run", "--init", "-d", "--name=top", "busybox", "sh", "-c", "addgroup -S test && adduser -S -G test test -D -s /bin/sh && touch /adduser_end && exec top") assert.NilError(c, err, "Output: %s", out) s.d.WaitRun("top") diff --git a/integration-cli/docker_cli_network_unix_test.go b/integration-cli/docker_cli_network_unix_test.go index babdedb2de..53b2f14482 100644 --- a/integration-cli/docker_cli_network_unix_test.go +++ b/integration-cli/docker_cli_network_unix_test.go @@ -956,7 +956,7 @@ func (s *DockerNetworkSuite) TestDockerNetworkDriverUngracefulRestart(c *testing _, err := s.d.Cmd("network", "create", "-d", dnd, "--subnet", "1.1.1.0/24", "net1") assert.NilError(c, err) - _, err = s.d.Cmd("run", "-itd", "--net", "net1", "--name", "foo", "--ip", "1.1.1.10", "busybox", "sh") + _, err = s.d.Cmd("run", "-d", "--net", "net1", "--name", "foo", "--ip", "1.1.1.10", "busybox", "top") assert.NilError(c, err) // Kill daemon and restart @@ -980,7 +980,7 @@ func (s *DockerNetworkSuite) TestDockerNetworkDriverUngracefulRestart(c *testing setupRemoteNetworkDrivers(c, mux, server.URL, dnd, did) // trying to reuse the same ip must succeed - _, err = s.d.Cmd("run", "-itd", "--net", "net1", "--name", "bar", "--ip", "1.1.1.10", "busybox", "sh") + _, err = s.d.Cmd("run", "-d", "--net", "net1", "--name", "bar", "--ip", "1.1.1.10", "busybox", "top") assert.NilError(c, err) } diff --git a/integration-cli/docker_cli_service_create_test.go b/integration-cli/docker_cli_service_create_test.go index d9ca41b2cb..a5c92c9411 100644 --- a/integration-cli/docker_cli_service_create_test.go +++ b/integration-cli/docker_cli_service_create_test.go @@ -362,7 +362,7 @@ func (s *DockerSwarmSuite) TestServiceCreateWithConfigReferencedTwice(c *testing func (s *DockerSwarmSuite) TestServiceCreateMountTmpfs(c *testing.T) { d := s.AddDaemon(c, true, true) - out, err := d.Cmd("service", "create", "--no-resolve-image", "--detach=true", "--mount", "type=tmpfs,target=/foo,tmpfs-size=1MB", "busybox", "sh", "-c", "mount | grep foo; tail -f /dev/null") + out, err := d.Cmd("service", "create", "--no-resolve-image", "--detach=true", "--mount", "type=tmpfs,target=/foo,tmpfs-size=1MB", "busybox", "sh", "-c", "mount | grep foo; exec tail -f /dev/null") assert.NilError(c, err, out) id := strings.TrimSpace(out) diff --git a/integration-cli/docker_cli_service_logs_test.go b/integration-cli/docker_cli_service_logs_test.go index 8668353978..72f3dbf6eb 100644 --- a/integration-cli/docker_cli_service_logs_test.go +++ b/integration-cli/docker_cli_service_logs_test.go @@ -34,7 +34,7 @@ func (s *DockerSwarmSuite) TestServiceLogs(c *testing.T) { for name, message := range services { out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", name, "busybox", - "sh", "-c", fmt.Sprintf("echo %s; tail -f /dev/null", message)) + "sh", "-c", fmt.Sprintf("echo %s; exec tail -f /dev/null", message)) assert.NilError(c, err) assert.Assert(c, strings.TrimSpace(out) != "") } @@ -75,7 +75,7 @@ func (s *DockerSwarmSuite) TestServiceLogsCompleteness(c *testing.T) { name := "TestServiceLogsCompleteness" // make a service that prints 6 lines - out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", name, "busybox", "sh", "-c", "for line in $(seq 0 5); do echo log test $line; done; sleep 100000") + out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", name, "busybox", "sh", "-c", "for line in $(seq 0 5); do echo log test $line; done; exec tail /dev/null") assert.NilError(c, err) assert.Assert(c, strings.TrimSpace(out) != "") @@ -126,7 +126,7 @@ func (s *DockerSwarmSuite) TestServiceLogsSince(c *testing.T) { name := "TestServiceLogsSince" - out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", name, "busybox", "sh", "-c", "for i in $(seq 1 3); do sleep .1; echo log$i; done; sleep 10000000") + out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", name, "busybox", "sh", "-c", "for i in $(seq 1 3); do usleep 100000; echo log$i; done; exec tail /dev/null") assert.NilError(c, err) assert.Assert(c, strings.TrimSpace(out) != "") poll.WaitOn(c, pollCheck(c, d.CheckActiveContainerCount, checker.Equals(1)), poll.WithTimeout(defaultReconciliationTimeout)) @@ -160,7 +160,7 @@ func (s *DockerSwarmSuite) TestServiceLogsFollow(c *testing.T) { name := "TestServiceLogsFollow" - out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", name, "busybox", "sh", "-c", "while true; do echo log test; sleep 0.1; done") + out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", name, "busybox", "sh", "-c", "trap 'exit 0' TERM; while true; do echo log test; usleep 100000; done") assert.NilError(c, err) assert.Assert(c, strings.TrimSpace(out) != "") @@ -215,7 +215,7 @@ func (s *DockerSwarmSuite) TestServiceLogsTaskLogs(c *testing.T) { // which has this the task id as an environment variable templated in "--env", "TASK={{.Task.ID}}", // and runs this command to print exactly 6 logs lines - "busybox", "sh", "-c", "for line in $(seq 0 5); do echo $TASK log test $line; done; sleep 100000", + "busybox", "sh", "-c", "trap 'exit 0' TERM; for line in $(seq 0 5); do echo $TASK log test $line; done; sleep 100000", )) result.Assert(c, icmd.Expected{}) // ^^ verify that we get no error @@ -323,9 +323,7 @@ func (s *DockerSwarmSuite) TestServiceLogsNoHangDeletedContainer(c *testing.T) { result = icmd.RunCmd(d.Command("ps", "-q")) containerID := strings.TrimSpace(result.Stdout()) assert.Assert(c, containerID != "") - result = icmd.RunCmd(d.Command("stop", containerID)) - result.Assert(c, icmd.Expected{Out: containerID}) - result = icmd.RunCmd(d.Command("rm", containerID)) + result = icmd.RunCmd(d.Command("rm", "-f", containerID)) result.Assert(c, icmd.Expected{Out: containerID}) // run logs. use tail 2 to make sure we don't try to get a bunch of logs @@ -360,7 +358,7 @@ func (s *DockerSwarmSuite) TestServiceLogsDetails(c *testing.T) { // busybox image, shell string "busybox", "sh", "-c", // make a log line - "echo LogLine; while true; do sleep 1; done;", + "trap 'exit 0' TERM; echo LogLine; while true; do sleep 1; done;", )) result.Assert(c, icmd.Expected{}) diff --git a/integration-cli/docker_cli_userns_test.go b/integration-cli/docker_cli_userns_test.go index b3b68c17c6..f61ed27448 100644 --- a/integration-cli/docker_cli_userns_test.go +++ b/integration-cli/docker_cli_userns_test.go @@ -46,7 +46,7 @@ func (s *DockerDaemonSuite) TestDaemonUserNamespaceRootSetting(c *testing.T) { // writable by the remapped root UID/GID pair assert.NilError(c, os.Chown(tmpDir, uid, gid)) - out, err := s.d.Cmd("run", "-d", "--name", "userns", "-v", tmpDir+":/goofy", "-v", tmpDirNotExists+":/donald", "busybox", "sh", "-c", "touch /goofy/testfile; top") + out, err := s.d.Cmd("run", "-d", "--name", "userns", "-v", tmpDir+":/goofy", "-v", tmpDirNotExists+":/donald", "busybox", "sh", "-c", "touch /goofy/testfile; exec top") assert.NilError(c, err, "Output: %s", out) user := s.findUser(c, "userns") @@ -79,7 +79,7 @@ func (s *DockerDaemonSuite) TestDaemonUserNamespaceRootSetting(c *testing.T) { assert.Equal(c, stat.GID(), uint32(gid), "Touched file not owned by remapped root GID") // use host usernamespace - out, err = s.d.Cmd("run", "-d", "--name", "userns_skip", "--userns", "host", "busybox", "sh", "-c", "touch /goofy/testfile; top") + out, err = s.d.Cmd("run", "-d", "--name", "userns_skip", "--userns", "host", "busybox", "sh", "-c", "touch /goofy/testfile; exec top") assert.Assert(c, err == nil, "Output: %s", out) user = s.findUser(c, "userns_skip") // userns are skipped, user is root