From e063099f918a01891db4389aaa6ba843e5d37fa9 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 --- integration-cli/docker_api_swarm_test.go | 16 ++++---- integration-cli/docker_cli_swarm_test.go | 4 +- integration/network/service_test.go | 8 ++-- integration/service/plugin_test.go | 48 ++++++++++++------------ internal/test/daemon/daemon.go | 9 ----- internal/test/daemon/plugin.go | 28 +++++++------- internal/test/daemon/swarm.go | 31 ++++++++------- 7 files changed, 66 insertions(+), 78 deletions(-) diff --git a/integration-cli/docker_api_swarm_test.go b/integration-cli/docker_api_swarm_test.go index 2995174aca..fecbf1e860 100644 --- a/integration-cli/docker_api_swarm_test.go +++ b/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) @@ -874,7 +874,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/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 5f3c14c341..673bef870a 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/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/integration/network/service_test.go b/integration/network/service_test.go index a12b4ee1b0..8c5bc80a0e 100644 --- a/integration/network/service_test.go +++ b/integration/network/service_test.go @@ -356,7 +356,7 @@ func TestServiceWithDataPathPortInit(t *testing.T) { assert.Equal(t, info.Swarm.Cluster.DataPathPort, datapathPort) err := c.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 @@ -382,7 +382,7 @@ func TestServiceWithDataPathPortInit(t *testing.T) { assert.Equal(t, info.Swarm.Cluster.DataPathPort, defaultDataPathPort) err = c.ServiceRemove(context.Background(), serviceID) assert.NilError(t, err) - d.SwarmLeave(true) + d.SwarmLeave(t, true) defer d.Stop(t) } @@ -424,7 +424,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 @@ -432,6 +432,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/integration/service/plugin_test.go b/integration/service/plugin_test.go index e476c2f1f4..16f0be567e 100644 --- a/integration/service/plugin_test.go +++ b/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/internal/test/daemon/daemon.go b/internal/test/daemon/daemon.go index 90ec525bed..9aabd3c1de 100644 --- a/internal/test/daemon/daemon.go +++ b/internal/test/daemon/daemon.go @@ -167,16 +167,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/internal/test/daemon/plugin.go b/internal/test/daemon/plugin.go index 63bbeed219..ad66ebbf07 100644 --- a/internal/test/daemon/plugin.go +++ b/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/internal/test/daemon/swarm.go b/internal/test/daemon/swarm.go index e500fe0fdc..b37593a3fb 100644 --- a/internal/test/daemon/swarm.go +++ b/internal/test/daemon/swarm.go @@ -111,17 +111,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 @@ -136,13 +133,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") }