From f3a3ea0d3c7f4d4da035db871d3c8a8bbb51371f Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Mon, 5 Aug 2019 13:57:48 -0700 Subject: [PATCH 1/2] Fix flaky TestServiceWithDefaultAddressPoolInit 1.This commit replaces serviceRunningCount with swarm.RunningTasksCount to accurately check if the service is running with the accurate number of instances or not. serviceRunningCount was only checking the ServiceList and was not checking if the tasks were running or not This adds a safe barrier to execute docker network inspect commands for overlay networks which get created asynchronously via Swarm 2. Make sure client connections are closed 3. Make sure every service and network name is unique 4. Make sure services and networks are cleaned up Signed-off-by: Arko Dasgupta --- integration/network/service_test.go | 100 +++++++++++++--------------- 1 file changed, 46 insertions(+), 54 deletions(-) diff --git a/integration/network/service_test.go b/integration/network/service_test.go index d7e36ab446..a965a17b22 100644 --- a/integration/network/service_test.go +++ b/integration/network/service_test.go @@ -215,7 +215,7 @@ func TestServiceWithPredefinedNetwork(t *testing.T) { swarm.ServiceWithNetwork(hostName), ) - poll.WaitOn(t, serviceRunningCount(c, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(c, serviceID, instances), swarm.ServicePoll) _, _, err := c.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) assert.NilError(t, err) @@ -254,7 +254,7 @@ func TestServiceRemoveKeepsIngressNetwork(t *testing.T) { }), ) - poll.WaitOn(t, serviceRunningCount(c, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(c, serviceID, instances), swarm.ServicePoll) ctx := context.Background() _, _, err := c.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) @@ -279,20 +279,6 @@ func TestServiceRemoveKeepsIngressNetwork(t *testing.T) { assert.Assert(t, ok, "ingress-sbox not present in ingress network") } -func serviceRunningCount(client client.ServiceAPIClient, serviceID string, instances uint64) func(log poll.LogT) poll.Result { - return func(log poll.LogT) poll.Result { - services, err := client.ServiceList(context.Background(), types.ServiceListOptions{}) - if err != nil { - return poll.Error(err) - } - - if len(services) != int(instances) { - return poll.Continue("Service count at %d waiting for %d", len(services), instances) - } - return poll.Success() - } -} - func swarmIngressReady(client client.NetworkAPIClient) func(log poll.LogT) poll.Result { return func(log poll.LogT) poll.Result { netInfo, err := client.NetworkInspect(context.Background(), ingressNet, types.NetworkInspectOptions{ @@ -333,76 +319,84 @@ func TestServiceWithDataPathPortInit(t *testing.T) { skip.If(t, testEnv.OSType == "windows") skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "DataPathPort was added in API v1.40") defer setupTest(t)() - var ops = []func(*daemon.Daemon){} var datapathPort uint32 = 7777 - ops = append(ops, daemon.WithSwarmDataPathPort(datapathPort)) - d := swarm.NewSwarm(t, testEnv, ops...) - + d := swarm.NewSwarm(t, testEnv, daemon.WithSwarmDataPathPort(datapathPort)) c := d.NewClientT(t) - defer c.Close() - + ctx := context.Background() // Create a overlay network name := "saanvisthira" + t.Name() - network.CreateNoError(context.Background(), t, c, name, + overlayID := network.CreateNoError(context.Background(), t, c, name, network.WithDriver("overlay")) var instances uint64 = 1 serviceID := swarm.CreateService(t, d, swarm.ServiceWithReplicas(instances), + swarm.ServiceWithName(name), swarm.ServiceWithNetwork(name), ) - poll.WaitOn(t, serviceRunningCount(c, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(c, serviceID, instances), swarm.ServicePoll) info := d.Info(t) assert.Equal(t, info.Swarm.Cluster.DataPathPort, datapathPort) - err := c.ServiceRemove(context.Background(), serviceID) + err := c.ServiceRemove(ctx, serviceID) + assert.NilError(t, err) + poll.WaitOn(t, noServices(ctx, c), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasks(ctx, c), swarm.ServicePoll) + err = c.NetworkRemove(ctx, overlayID) + assert.NilError(t, err) + c.Close() + err = d.SwarmLeave(t, true) assert.NilError(t, err) - d.SwarmLeave(t, true) d.Stop(t) // Clean up , set it back to original one to make sure other tests don't fail // call without datapath port option. - ops = []func(*daemon.Daemon){} - d = swarm.NewSwarm(t, testEnv, ops...) - c = d.NewClientT(t) - + d = swarm.NewSwarm(t, testEnv) + nc := d.NewClientT(t) + defer nc.Close() // Create a overlay network - name = "saanvisthira" + t.Name() - network.CreateNoError(context.Background(), t, c, name, + name = "not-saanvisthira" + t.Name() + overlayID = network.CreateNoError(ctx, t, nc, name, network.WithDriver("overlay")) serviceID = swarm.CreateService(t, d, swarm.ServiceWithReplicas(instances), + swarm.ServiceWithName(name), swarm.ServiceWithNetwork(name), ) - poll.WaitOn(t, serviceRunningCount(c, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(nc, serviceID, instances), swarm.ServicePoll) info = d.Info(t) var defaultDataPathPort uint32 = 4789 assert.Equal(t, info.Swarm.Cluster.DataPathPort, defaultDataPathPort) - err = c.ServiceRemove(context.Background(), serviceID) + err = nc.ServiceRemove(ctx, serviceID) + assert.NilError(t, err) + assert.NilError(t, err) + poll.WaitOn(t, noServices(ctx, nc), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasks(ctx, nc), swarm.ServicePoll) + err = nc.NetworkRemove(ctx, overlayID) + assert.NilError(t, err) + err = d.SwarmLeave(t, true) assert.NilError(t, err) - d.SwarmLeave(t, true) defer d.Stop(t) } func TestServiceWithDefaultAddressPoolInit(t *testing.T) { skip.If(t, testEnv.OSType == "windows") defer setupTest(t)() - var ops = []func(*daemon.Daemon){} - ipAddr := []string{"20.20.0.0/16"} - ops = append(ops, daemon.WithSwarmDefaultAddrPool(ipAddr)) - ops = append(ops, daemon.WithSwarmDefaultAddrPoolSubnetSize(24)) - d := swarm.NewSwarm(t, testEnv, ops...) + d := swarm.NewSwarm(t, testEnv, + daemon.WithSwarmDefaultAddrPool([]string{"20.20.0.0/16"}), + daemon.WithSwarmDefaultAddrPoolSubnetSize(24)) cli := d.NewClientT(t) defer cli.Close() + ctx := context.Background() // Create a overlay network name := "sthira" + t.Name() - overlayID := network.CreateNoError(context.Background(), t, cli, name, + overlayID := network.CreateNoError(ctx, t, cli, name, network.WithDriver("overlay"), network.WithCheckDuplicate(), ) @@ -415,27 +409,25 @@ func TestServiceWithDefaultAddressPoolInit(t *testing.T) { swarm.ServiceWithNetwork(name), ) - poll.WaitOn(t, serviceRunningCount(cli, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(cli, serviceID, instances), swarm.ServicePoll) - _, _, err := cli.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) + _, _, err := cli.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) assert.NilError(t, err) - out, err := cli.NetworkInspect(context.Background(), overlayID, types.NetworkInspectOptions{Verbose: true}) + out, err := cli.NetworkInspect(ctx, overlayID, types.NetworkInspectOptions{Verbose: true}) assert.NilError(t, err) t.Logf("%s: NetworkInspect: %+v", t.Name(), out) assert.Assert(t, len(out.IPAM.Config) > 0) assert.Equal(t, out.IPAM.Config[0].Subnet, "20.20.0.0/24") - err = cli.ServiceRemove(context.Background(), serviceID) + err = cli.ServiceRemove(ctx, serviceID) + poll.WaitOn(t, noServices(ctx, cli), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasks(ctx, cli), swarm.ServicePoll) + assert.NilError(t, err) + err = cli.NetworkRemove(ctx, overlayID) + assert.NilError(t, err) + err = d.SwarmLeave(t, true) assert.NilError(t, err) - d.SwarmLeave(t, true) - d.Stop(t) - - // Clean up , set it back to original one to make sure other tests don't fail - ipAddr = []string{"10.0.0.0/8"} - ops = append(ops, daemon.WithSwarmDefaultAddrPool(ipAddr)) - ops = append(ops, daemon.WithSwarmDefaultAddrPoolSubnetSize(24)) - d = swarm.NewSwarm(t, testEnv, ops...) - d.SwarmLeave(t, true) defer d.Stop(t) + } From a65dee30fc36ed974940b699ed22ab9242eac6cf Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 15 Aug 2019 14:09:47 -0700 Subject: [PATCH 2/2] Move defer method to the top right after New is called Signed-off-by: Arko Dasgupta --- integration/network/service_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/integration/network/service_test.go b/integration/network/service_test.go index a965a17b22..df2404c372 100644 --- a/integration/network/service_test.go +++ b/integration/network/service_test.go @@ -353,6 +353,7 @@ func TestServiceWithDataPathPortInit(t *testing.T) { // Clean up , set it back to original one to make sure other tests don't fail // call without datapath port option. d = swarm.NewSwarm(t, testEnv) + defer d.Stop(t) nc := d.NewClientT(t) defer nc.Close() // Create a overlay network @@ -373,14 +374,12 @@ func TestServiceWithDataPathPortInit(t *testing.T) { assert.Equal(t, info.Swarm.Cluster.DataPathPort, defaultDataPathPort) err = nc.ServiceRemove(ctx, serviceID) assert.NilError(t, err) - assert.NilError(t, err) poll.WaitOn(t, noServices(ctx, nc), swarm.ServicePoll) poll.WaitOn(t, swarm.NoTasks(ctx, nc), swarm.ServicePoll) err = nc.NetworkRemove(ctx, overlayID) assert.NilError(t, err) err = d.SwarmLeave(t, true) assert.NilError(t, err) - defer d.Stop(t) } func TestServiceWithDefaultAddressPoolInit(t *testing.T) { @@ -389,7 +388,7 @@ func TestServiceWithDefaultAddressPoolInit(t *testing.T) { d := swarm.NewSwarm(t, testEnv, daemon.WithSwarmDefaultAddrPool([]string{"20.20.0.0/16"}), daemon.WithSwarmDefaultAddrPoolSubnetSize(24)) - + defer d.Stop(t) cli := d.NewClientT(t) defer cli.Close() ctx := context.Background() @@ -428,6 +427,5 @@ func TestServiceWithDefaultAddressPoolInit(t *testing.T) { assert.NilError(t, err) err = d.SwarmLeave(t, true) assert.NilError(t, err) - defer d.Stop(t) }