Completely remove `d.NewClient` from testing tools

Favor `d.NewClientT` instead.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

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")
 }

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+"}}")

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)
 }

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) {

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()

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)
 	}
 }

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")
 	}