From 9295462ecc2adb1e1f0dc13e1312a7d9e46dda0b Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 6 Feb 2019 15:33:07 -0800 Subject: [PATCH] Completely remove `d.NewClient` from testing tools Favor `d.NewClientT` instead. Signed-off-by: Brian Goff (cherry picked from commit e063099f918a01891db4389aaa6ba843e5d37fa9) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 1efe4720feb93c7f47d717001091279636f2039e Component: engine --- .../integration-cli/docker_api_swarm_test.go | 16 +++---- .../integration-cli/docker_cli_swarm_test.go | 4 +- .../integration/network/service_test.go | 4 +- .../engine/integration/service/plugin_test.go | 48 +++++++++---------- .../engine/internal/test/daemon/daemon.go | 9 ---- .../engine/internal/test/daemon/plugin.go | 28 +++++------ .../engine/internal/test/daemon/swarm.go | 31 ++++++------ 7 files changed, 64 insertions(+), 76 deletions(-) diff --git a/components/engine/integration-cli/docker_api_swarm_test.go b/components/engine/integration-cli/docker_api_swarm_test.go index 2260f2cff1..c48e669c0d 100644 --- a/components/engine/integration-cli/docker_api_swarm_test.go +++ b/components/engine/integration-cli/docker_api_swarm_test.go @@ -47,7 +47,7 @@ func (s *DockerSwarmSuite) TestAPISwarmInit(c *check.C) { c.Assert(info.LocalNodeState, checker.Equals, swarm.LocalNodeStateActive) // Leaving cluster - c.Assert(d2.SwarmLeave(false), checker.IsNil) + c.Assert(d2.SwarmLeave(c, false), checker.IsNil) info = d2.SwarmInfo(c) c.Assert(info.ControlAvailable, checker.False) @@ -115,7 +115,7 @@ func (s *DockerSwarmSuite) TestAPISwarmJoinToken(c *check.C) { }) info = d2.SwarmInfo(c) c.Assert(info.LocalNodeState, checker.Equals, swarm.LocalNodeStateActive) - c.Assert(d2.SwarmLeave(false), checker.IsNil) + c.Assert(d2.SwarmLeave(c, false), checker.IsNil) info = d2.SwarmInfo(c) c.Assert(info.LocalNodeState, checker.Equals, swarm.LocalNodeStateInactive) @@ -137,7 +137,7 @@ func (s *DockerSwarmSuite) TestAPISwarmJoinToken(c *check.C) { d2.SwarmJoin(c, swarm.JoinRequest{JoinToken: workerToken, RemoteAddrs: []string{d1.SwarmListenAddr()}}) info = d2.SwarmInfo(c) c.Assert(info.LocalNodeState, checker.Equals, swarm.LocalNodeStateActive) - c.Assert(d2.SwarmLeave(false), checker.IsNil) + c.Assert(d2.SwarmLeave(c, false), checker.IsNil) info = d2.SwarmInfo(c) c.Assert(info.LocalNodeState, checker.Equals, swarm.LocalNodeStateInactive) @@ -156,7 +156,7 @@ func (s *DockerSwarmSuite) TestAPISwarmJoinToken(c *check.C) { d2.SwarmJoin(c, swarm.JoinRequest{JoinToken: workerToken, RemoteAddrs: []string{d1.SwarmListenAddr()}}) info = d2.SwarmInfo(c) c.Assert(info.LocalNodeState, checker.Equals, swarm.LocalNodeStateActive) - c.Assert(d2.SwarmLeave(false), checker.IsNil) + c.Assert(d2.SwarmLeave(c, false), checker.IsNil) info = d2.SwarmInfo(c) c.Assert(info.LocalNodeState, checker.Equals, swarm.LocalNodeStateInactive) } @@ -423,8 +423,8 @@ func (s *DockerSwarmSuite) TestAPISwarmLeaveRemovesContainer(c *check.C) { waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, instances+1) - c.Assert(d.SwarmLeave(false), checker.NotNil) - c.Assert(d.SwarmLeave(true), checker.IsNil) + c.Assert(d.SwarmLeave(c, false), checker.NotNil) + c.Assert(d.SwarmLeave(c, true), checker.IsNil) waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1) @@ -454,7 +454,7 @@ func (s *DockerSwarmSuite) TestAPISwarmLeaveOnPendingJoin(c *check.C) { info := d2.SwarmInfo(c) c.Assert(info.LocalNodeState, checker.Equals, swarm.LocalNodeStatePending) - c.Assert(d2.SwarmLeave(true), checker.IsNil) + c.Assert(d2.SwarmLeave(c, true), checker.IsNil) waitAndAssert(c, defaultReconciliationTimeout, d2.CheckActiveContainerCount, checker.Equals, 1) @@ -878,7 +878,7 @@ func (s *DockerSwarmSuite) TestAPISwarmServicesUpdateWithName(c *check.C) { // Unlocking an unlocked swarm results in an error func (s *DockerSwarmSuite) TestAPISwarmUnlockNotLocked(c *check.C) { d := s.AddDaemon(c, true, true) - err := d.SwarmUnlock(swarm.UnlockRequest{UnlockKey: "wrong-key"}) + err := d.SwarmUnlock(c, swarm.UnlockRequest{UnlockKey: "wrong-key"}) c.Assert(err, checker.NotNil) c.Assert(err.Error(), checker.Contains, "swarm is not locked") } diff --git a/components/engine/integration-cli/docker_cli_swarm_test.go b/components/engine/integration-cli/docker_cli_swarm_test.go index dafe8c96ad..4ee946f795 100644 --- a/components/engine/integration-cli/docker_cli_swarm_test.go +++ b/components/engine/integration-cli/docker_cli_swarm_test.go @@ -116,7 +116,7 @@ func (s *DockerSwarmSuite) TestSwarmInit(c *check.C) { c.Assert(spec.CAConfig.ExternalCAs[0].CACert, checker.Equals, "") c.Assert(spec.CAConfig.ExternalCAs[1].CACert, checker.Equals, string(expected)) - c.Assert(d.SwarmLeave(true), checker.IsNil) + c.Assert(d.SwarmLeave(c, true), checker.IsNil) cli.Docker(cli.Args("swarm", "init"), cli.Daemon(d)).Assert(c, icmd.Success) spec = getSpec() @@ -426,7 +426,7 @@ func (s *DockerSwarmSuite) TestOverlayAttachableOnSwarmLeave(c *check.C) { c.Assert(err, checker.IsNil, check.Commentf("%s", out)) // Leave the swarm - c.Assert(d.SwarmLeave(true), checker.IsNil) + c.Assert(d.SwarmLeave(c, true), checker.IsNil) // Check the container is disconnected out, err = d.Cmd("inspect", "c1", "--format", "{{.NetworkSettings.Networks."+nwName+"}}") diff --git a/components/engine/integration/network/service_test.go b/components/engine/integration/network/service_test.go index a65d27564c..13f471446c 100644 --- a/components/engine/integration/network/service_test.go +++ b/components/engine/integration/network/service_test.go @@ -365,7 +365,7 @@ func TestServiceWithDefaultAddressPoolInit(t *testing.T) { err = cli.ServiceRemove(context.Background(), serviceID) assert.NilError(t, err) - d.SwarmLeave(true) + d.SwarmLeave(t, true) d.Stop(t) // Clean up , set it back to original one to make sure other tests don't fail @@ -373,6 +373,6 @@ func TestServiceWithDefaultAddressPoolInit(t *testing.T) { ops = append(ops, daemon.WithSwarmDefaultAddrPool(ipAddr)) ops = append(ops, daemon.WithSwarmDefaultAddrPoolSubnetSize(24)) d = swarm.NewSwarm(t, testEnv, ops...) - d.SwarmLeave(true) + d.SwarmLeave(t, true) defer d.Stop(t) } diff --git a/components/engine/integration/service/plugin_test.go b/components/engine/integration/service/plugin_test.go index e476c2f1f4..16f0be567e 100644 --- a/components/engine/integration/service/plugin_test.go +++ b/components/engine/integration/service/plugin_test.go @@ -64,45 +64,45 @@ func TestServicePlugin(t *testing.T) { defer d3.Stop(t) id := d1.CreateService(t, makePlugin(repo, name, nil)) - poll.WaitOn(t, d1.PluginIsRunning(name), swarm.ServicePoll) - poll.WaitOn(t, d2.PluginIsRunning(name), swarm.ServicePoll) - poll.WaitOn(t, d3.PluginIsRunning(name), swarm.ServicePoll) + poll.WaitOn(t, d1.PluginIsRunning(t, name), swarm.ServicePoll) + poll.WaitOn(t, d2.PluginIsRunning(t, name), swarm.ServicePoll) + poll.WaitOn(t, d3.PluginIsRunning(t, name), swarm.ServicePoll) service := d1.GetService(t, id) d1.UpdateService(t, service, makePlugin(repo2, name, nil)) - poll.WaitOn(t, d1.PluginReferenceIs(name, repo2), swarm.ServicePoll) - poll.WaitOn(t, d2.PluginReferenceIs(name, repo2), swarm.ServicePoll) - poll.WaitOn(t, d3.PluginReferenceIs(name, repo2), swarm.ServicePoll) - poll.WaitOn(t, d1.PluginIsRunning(name), swarm.ServicePoll) - poll.WaitOn(t, d2.PluginIsRunning(name), swarm.ServicePoll) - poll.WaitOn(t, d3.PluginIsRunning(name), swarm.ServicePoll) + poll.WaitOn(t, d1.PluginReferenceIs(t, name, repo2), swarm.ServicePoll) + poll.WaitOn(t, d2.PluginReferenceIs(t, name, repo2), swarm.ServicePoll) + poll.WaitOn(t, d3.PluginReferenceIs(t, name, repo2), swarm.ServicePoll) + poll.WaitOn(t, d1.PluginIsRunning(t, name), swarm.ServicePoll) + poll.WaitOn(t, d2.PluginIsRunning(t, name), swarm.ServicePoll) + poll.WaitOn(t, d3.PluginIsRunning(t, name), swarm.ServicePoll) d1.RemoveService(t, id) - poll.WaitOn(t, d1.PluginIsNotPresent(name), swarm.ServicePoll) - poll.WaitOn(t, d2.PluginIsNotPresent(name), swarm.ServicePoll) - poll.WaitOn(t, d3.PluginIsNotPresent(name), swarm.ServicePoll) + poll.WaitOn(t, d1.PluginIsNotPresent(t, name), swarm.ServicePoll) + poll.WaitOn(t, d2.PluginIsNotPresent(t, name), swarm.ServicePoll) + poll.WaitOn(t, d3.PluginIsNotPresent(t, name), swarm.ServicePoll) // constrain to managers only id = d1.CreateService(t, makePlugin(repo, name, []string{"node.role==manager"})) - poll.WaitOn(t, d1.PluginIsRunning(name), swarm.ServicePoll) - poll.WaitOn(t, d2.PluginIsRunning(name), swarm.ServicePoll) - poll.WaitOn(t, d3.PluginIsNotPresent(name), swarm.ServicePoll) + poll.WaitOn(t, d1.PluginIsRunning(t, name), swarm.ServicePoll) + poll.WaitOn(t, d2.PluginIsRunning(t, name), swarm.ServicePoll) + poll.WaitOn(t, d3.PluginIsNotPresent(t, name), swarm.ServicePoll) d1.RemoveService(t, id) - poll.WaitOn(t, d1.PluginIsNotPresent(name), swarm.ServicePoll) - poll.WaitOn(t, d2.PluginIsNotPresent(name), swarm.ServicePoll) - poll.WaitOn(t, d3.PluginIsNotPresent(name), swarm.ServicePoll) + poll.WaitOn(t, d1.PluginIsNotPresent(t, name), swarm.ServicePoll) + poll.WaitOn(t, d2.PluginIsNotPresent(t, name), swarm.ServicePoll) + poll.WaitOn(t, d3.PluginIsNotPresent(t, name), swarm.ServicePoll) // with no name id = d1.CreateService(t, makePlugin(repo, "", nil)) - poll.WaitOn(t, d1.PluginIsRunning(repo), swarm.ServicePoll) - poll.WaitOn(t, d2.PluginIsRunning(repo), swarm.ServicePoll) - poll.WaitOn(t, d3.PluginIsRunning(repo), swarm.ServicePoll) + poll.WaitOn(t, d1.PluginIsRunning(t, repo), swarm.ServicePoll) + poll.WaitOn(t, d2.PluginIsRunning(t, repo), swarm.ServicePoll) + poll.WaitOn(t, d3.PluginIsRunning(t, repo), swarm.ServicePoll) d1.RemoveService(t, id) - poll.WaitOn(t, d1.PluginIsNotPresent(repo), swarm.ServicePoll) - poll.WaitOn(t, d2.PluginIsNotPresent(repo), swarm.ServicePoll) - poll.WaitOn(t, d3.PluginIsNotPresent(repo), swarm.ServicePoll) + poll.WaitOn(t, d1.PluginIsNotPresent(t, repo), swarm.ServicePoll) + poll.WaitOn(t, d2.PluginIsNotPresent(t, repo), swarm.ServicePoll) + poll.WaitOn(t, d3.PluginIsNotPresent(t, repo), swarm.ServicePoll) } func makePlugin(repo, name string, constraints []string) func(*swarmtypes.Service) { diff --git a/components/engine/internal/test/daemon/daemon.go b/components/engine/internal/test/daemon/daemon.go index cfa06fa934..ecc2e093a3 100644 --- a/components/engine/internal/test/daemon/daemon.go +++ b/components/engine/internal/test/daemon/daemon.go @@ -165,16 +165,7 @@ func (d *Daemon) ReadLogFile() ([]byte, error) { return ioutil.ReadFile(d.logFile.Name()) } -// NewClient creates new client based on daemon's socket path -// FIXME(vdemeester): replace NewClient with NewClientT -func (d *Daemon) NewClient() (*client.Client, error) { - return client.NewClientWithOpts( - client.FromEnv, - client.WithHost(d.Sock())) -} - // NewClientT creates new client based on daemon's socket path -// FIXME(vdemeester): replace NewClient with NewClientT func (d *Daemon) NewClientT(t assert.TestingT) *client.Client { if ht, ok := t.(test.HelperT); ok { ht.Helper() diff --git a/components/engine/internal/test/daemon/plugin.go b/components/engine/internal/test/daemon/plugin.go index 63bbeed219..ad66ebbf07 100644 --- a/components/engine/internal/test/daemon/plugin.go +++ b/components/engine/internal/test/daemon/plugin.go @@ -5,12 +5,13 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/client" + "gotest.tools/assert" "gotest.tools/poll" ) // PluginIsRunning provides a poller to check if the specified plugin is running -func (d *Daemon) PluginIsRunning(name string) func(poll.LogT) poll.Result { - return withClient(d, withPluginInspect(name, func(plugin *types.Plugin, t poll.LogT) poll.Result { +func (d *Daemon) PluginIsRunning(t assert.TestingT, name string) func(poll.LogT) poll.Result { + return withClient(t, d, withPluginInspect(name, func(plugin *types.Plugin, t poll.LogT) poll.Result { if plugin.Enabled { return poll.Success() } @@ -19,8 +20,8 @@ func (d *Daemon) PluginIsRunning(name string) func(poll.LogT) poll.Result { } // PluginIsNotRunning provides a poller to check if the specified plugin is not running -func (d *Daemon) PluginIsNotRunning(name string) func(poll.LogT) poll.Result { - return withClient(d, withPluginInspect(name, func(plugin *types.Plugin, t poll.LogT) poll.Result { +func (d *Daemon) PluginIsNotRunning(t assert.TestingT, name string) func(poll.LogT) poll.Result { + return withClient(t, d, withPluginInspect(name, func(plugin *types.Plugin, t poll.LogT) poll.Result { if !plugin.Enabled { return poll.Success() } @@ -29,8 +30,8 @@ func (d *Daemon) PluginIsNotRunning(name string) func(poll.LogT) poll.Result { } // PluginIsNotPresent provides a poller to check if the specified plugin is not present -func (d *Daemon) PluginIsNotPresent(name string) func(poll.LogT) poll.Result { - return withClient(d, func(c client.APIClient, t poll.LogT) poll.Result { +func (d *Daemon) PluginIsNotPresent(t assert.TestingT, name string) func(poll.LogT) poll.Result { + return withClient(t, d, func(c client.APIClient, t poll.LogT) poll.Result { _, _, err := c.PluginInspectWithRaw(context.Background(), name) if client.IsErrNotFound(err) { return poll.Success() @@ -43,8 +44,8 @@ func (d *Daemon) PluginIsNotPresent(name string) func(poll.LogT) poll.Result { } // PluginReferenceIs provides a poller to check if the specified plugin has the specified reference -func (d *Daemon) PluginReferenceIs(name, expectedRef string) func(poll.LogT) poll.Result { - return withClient(d, withPluginInspect(name, func(plugin *types.Plugin, t poll.LogT) poll.Result { +func (d *Daemon) PluginReferenceIs(t assert.TestingT, name, expectedRef string) func(poll.LogT) poll.Result { + return withClient(t, d, withPluginInspect(name, func(plugin *types.Plugin, t poll.LogT) poll.Result { if plugin.PluginReference == expectedRef { return poll.Success() } @@ -66,12 +67,9 @@ func withPluginInspect(name string, f func(*types.Plugin, poll.LogT) poll.Result } -func withClient(d *Daemon, f func(client.APIClient, poll.LogT) poll.Result) func(poll.LogT) poll.Result { - return func(t poll.LogT) poll.Result { - c, err := d.NewClient() - if err != nil { - poll.Error(err) - } - return f(c, t) +func withClient(t assert.TestingT, d *Daemon, f func(client.APIClient, poll.LogT) poll.Result) func(poll.LogT) poll.Result { + return func(pt poll.LogT) poll.Result { + c := d.NewClientT(t) + return f(c, pt) } } diff --git a/components/engine/internal/test/daemon/swarm.go b/components/engine/internal/test/daemon/swarm.go index ae6a62c9eb..4631222fcd 100644 --- a/components/engine/internal/test/daemon/swarm.go +++ b/components/engine/internal/test/daemon/swarm.go @@ -96,17 +96,14 @@ func (d *Daemon) SwarmJoin(t assert.TestingT, req swarm.JoinRequest) { } // SwarmLeave forces daemon to leave current cluster. -func (d *Daemon) SwarmLeave(force bool) error { - cli, err := d.NewClient() - if err != nil { - return fmt.Errorf("leaving swarm: failed to create client %v", err) - } +// +// The passed in TestingT is only used to validate that the client was successfully created +// Some tests rely on error checking the result of the actual unlock, so allow +// the error to be returned. +func (d *Daemon) SwarmLeave(t assert.TestingT, force bool) error { + cli := d.NewClientT(t) defer cli.Close() - err = cli.SwarmLeave(context.Background(), force) - if err != nil { - err = fmt.Errorf("leaving swarm: %v", err) - } - return err + return cli.SwarmLeave(context.Background(), force) } // SwarmInfo returns the swarm information of the daemon @@ -121,13 +118,15 @@ func (d *Daemon) SwarmInfo(t assert.TestingT) swarm.Info { } // SwarmUnlock tries to unlock a locked swarm -func (d *Daemon) SwarmUnlock(req swarm.UnlockRequest) error { - cli, err := d.NewClient() - if err != nil { - return fmt.Errorf("unlocking swarm: failed to create client %v", err) - } +// +// The passed in TestingT is only used to validate that the client was successfully created +// Some tests rely on error checking the result of the actual unlock, so allow +// the error to be returned. +func (d *Daemon) SwarmUnlock(t assert.TestingT, req swarm.UnlockRequest) error { + cli := d.NewClientT(t) defer cli.Close() - err = cli.SwarmUnlock(context.Background(), req) + + err := cli.SwarmUnlock(context.Background(), req) if err != nil { err = errors.Wrap(err, "unlocking swarm") }