From 777ce4552762282d6818b3bea7e7416b4beb2493 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 14 Jan 2019 02:20:09 +0100 Subject: [PATCH] Integration tests: remove some duplicated code, and preserve context This introduces `NoTasksForService` and `NoTasks` poller checks, that can be used to check if no tasks are left in general, or for a specific service. Some redundant checks were also removed from some tests. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 56a68c15f8a093b1761e77a74d8b7acdfbcb30a2) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 8f6421457d3be6280236aa466e533fb32cb4509b Component: engine --- .../integration/internal/swarm/states.go | 47 ++++++++++++ .../integration/network/inspect_test.go | 16 +---- .../integration/network/service_test.go | 17 ++--- .../engine/integration/service/create_test.go | 71 ++++--------------- 4 files changed, 71 insertions(+), 80 deletions(-) create mode 100644 components/engine/integration/internal/swarm/states.go diff --git a/components/engine/integration/internal/swarm/states.go b/components/engine/integration/internal/swarm/states.go new file mode 100644 index 0000000000..51d6200594 --- /dev/null +++ b/components/engine/integration/internal/swarm/states.go @@ -0,0 +1,47 @@ +package swarm + +import ( + "context" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/client" + "gotest.tools/poll" +) + +// NoTasksForService verifies that there are no more tasks for the given service +func NoTasksForService(ctx context.Context, client client.ServiceAPIClient, serviceID string) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + tasks, err := client.TaskList(ctx, types.TaskListOptions{ + Filters: filters.NewArgs( + filters.Arg("service", serviceID), + ), + }) + if err == nil { + if len(tasks) == 0 { + return poll.Success() + } + if len(tasks) > 0 { + return poll.Continue("task count for service %s at %d waiting for 0", serviceID, len(tasks)) + } + return poll.Continue("waiting for tasks for service %s to be deleted", serviceID) + } + // TODO we should not use an error as indication that the tasks are gone. There may be other reasons for an error to occur. + return poll.Success() + } +} + +// NoTasks verifies that all tasks are gone +func NoTasks(ctx context.Context, client client.ServiceAPIClient) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + tasks, err := client.TaskList(ctx, types.TaskListOptions{}) + switch { + case err != nil: + return poll.Error(err) + case len(tasks) == 0: + return poll.Success() + default: + return poll.Continue("waiting for all tasks to be removed: task count at %d", len(tasks)) + } + } +} diff --git a/components/engine/integration/network/inspect_test.go b/components/engine/integration/network/inspect_test.go index 9003387bac..d12ad6781d 100644 --- a/components/engine/integration/network/inspect_test.go +++ b/components/engine/integration/network/inspect_test.go @@ -98,7 +98,7 @@ func TestInspectNetwork(t *testing.T) { // TODO find out why removing networks is needed; other tests fail if the network is not removed, even though they run on a new daemon. err := c.ServiceRemove(ctx, serviceID) assert.NilError(t, err) - poll.WaitOn(t, serviceIsRemoved(c, serviceID), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasksForService(ctx, c, serviceID), swarm.ServicePoll) err = c.NetworkRemove(ctx, overlayID) assert.NilError(t, err) poll.WaitOn(t, network.IsRemoved(ctx, c, overlayID), swarm.NetworkPoll) @@ -130,17 +130,3 @@ func serviceRunningTasksCount(client client.ServiceAPIClient, serviceID string, } } } - -func serviceIsRemoved(client client.ServiceAPIClient, serviceID string) func(log poll.LogT) poll.Result { - return func(log poll.LogT) poll.Result { - filter := filters.NewArgs() - filter.Add("service", serviceID) - _, err := client.TaskList(context.Background(), types.TaskListOptions{ - Filters: filter, - }) - if err == nil { - return poll.Continue("waiting for service %s to be deleted", serviceID) - } - return poll.Success() - } -} diff --git a/components/engine/integration/network/service_test.go b/components/engine/integration/network/service_test.go index c418b2eee5..a65d27564c 100644 --- a/components/engine/integration/network/service_test.go +++ b/components/engine/integration/network/service_test.go @@ -254,18 +254,19 @@ func TestServiceRemoveKeepsIngressNetwork(t *testing.T) { poll.WaitOn(t, serviceRunningCount(c, serviceID, instances), swarm.ServicePoll) - _, _, err := c.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) + ctx := context.Background() + _, _, err := c.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) assert.NilError(t, err) - err = c.ServiceRemove(context.Background(), serviceID) + err = c.ServiceRemove(ctx, serviceID) assert.NilError(t, err) - poll.WaitOn(t, serviceIsRemoved(c, serviceID), swarm.ServicePoll) - poll.WaitOn(t, noServices(c), swarm.ServicePoll) + poll.WaitOn(t, noServices(ctx, c), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasks(ctx, c), swarm.ServicePoll) // Ensure that "ingress" is not removed or corrupted time.Sleep(10 * time.Second) - netInfo, err := c.NetworkInspect(context.Background(), ingressNet, types.NetworkInspectOptions{ + netInfo, err := c.NetworkInspect(ctx, ingressNet, types.NetworkInspectOptions{ Verbose: true, Scope: "swarm", }) @@ -312,16 +313,16 @@ func swarmIngressReady(client client.NetworkAPIClient) func(log poll.LogT) poll. } } -func noServices(client client.ServiceAPIClient) func(log poll.LogT) poll.Result { +func noServices(ctx context.Context, client client.ServiceAPIClient) func(log poll.LogT) poll.Result { return func(log poll.LogT) poll.Result { - services, err := client.ServiceList(context.Background(), types.ServiceListOptions{}) + services, err := client.ServiceList(ctx, types.ServiceListOptions{}) switch { case err != nil: return poll.Error(err) case len(services) == 0: return poll.Success() default: - return poll.Continue("Service count at %d waiting for 0", len(services)) + return poll.Continue("waiting for all services to be removed: service count at %d", len(services)) } } } diff --git a/components/engine/integration/service/create_test.go b/components/engine/integration/service/create_test.go index d64d352ed3..6e79bec9a8 100644 --- a/components/engine/integration/service/create_test.go +++ b/components/engine/integration/service/create_test.go @@ -79,9 +79,10 @@ func TestCreateServiceMultipleTimes(t *testing.T) { defer d.Stop(t) client := d.NewClientT(t) defer client.Close() + ctx := context.Background() overlayName := "overlay1_" + t.Name() - overlayID := network.CreateNoError(t, context.Background(), client, overlayName, + overlayID := network.CreateNoError(t, ctx, client, overlayName, network.WithCheckDuplicate(), network.WithDriver("overlay"), ) @@ -104,8 +105,7 @@ func TestCreateServiceMultipleTimes(t *testing.T) { err = client.ServiceRemove(context.Background(), serviceID) assert.NilError(t, err) - poll.WaitOn(t, serviceIsRemoved(client, serviceID), swarm.ServicePoll) - poll.WaitOn(t, noTasks(client), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasksForService(ctx, client, serviceID), swarm.ServicePoll) serviceID2 := swarm.CreateService(t, d, serviceSpec...) poll.WaitOn(t, serviceRunningTasksCount(client, serviceID2, instances), swarm.ServicePoll) @@ -113,8 +113,7 @@ func TestCreateServiceMultipleTimes(t *testing.T) { err = client.ServiceRemove(context.Background(), serviceID2) assert.NilError(t, err) - poll.WaitOn(t, serviceIsRemoved(client, serviceID2), swarm.ServicePoll) - poll.WaitOn(t, noTasks(client), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasksForService(ctx, client, serviceID2), swarm.ServicePoll) err = client.NetworkRemove(context.Background(), overlayID) assert.NilError(t, err) @@ -129,19 +128,14 @@ func TestCreateWithDuplicateNetworkNames(t *testing.T) { defer d.Stop(t) client := d.NewClientT(t) defer client.Close() + ctx := context.Background() name := "foo_" + t.Name() - n1 := network.CreateNoError(t, context.Background(), client, name, - network.WithDriver("bridge"), - ) - n2 := network.CreateNoError(t, context.Background(), client, name, - network.WithDriver("bridge"), - ) + n1 := network.CreateNoError(t, ctx, client, name, network.WithDriver("bridge")) + n2 := network.CreateNoError(t, ctx, client, name, network.WithDriver("bridge")) // Duplicates with name but with different driver - n3 := network.CreateNoError(t, context.Background(), client, name, - network.WithDriver("overlay"), - ) + n3 := network.CreateNoError(t, ctx, client, name, network.WithDriver("overlay")) // Create Service with the same name var instances uint64 = 1 @@ -155,16 +149,14 @@ func TestCreateWithDuplicateNetworkNames(t *testing.T) { poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances), swarm.ServicePoll) - resp, _, err := client.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) + resp, _, err := client.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) assert.NilError(t, err) assert.Check(t, is.Equal(n3, resp.Spec.TaskTemplate.Networks[0].Target)) - // Remove Service - err = client.ServiceRemove(context.Background(), serviceID) + // Remove Service, and wait for its tasks to be removed + err = client.ServiceRemove(ctx, serviceID) assert.NilError(t, err) - - // Make sure task has been destroyed. - poll.WaitOn(t, serviceIsRemoved(client, serviceID), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasksForService(ctx, client, serviceID), swarm.ServicePoll) // Remove networks err = client.NetworkRemove(context.Background(), n3) @@ -240,9 +232,7 @@ func TestCreateServiceSecretFileMode(t *testing.T) { err = client.ServiceRemove(ctx, serviceID) assert.NilError(t, err) - - poll.WaitOn(t, serviceIsRemoved(client, serviceID), swarm.ServicePoll) - poll.WaitOn(t, noTasks(client), swarm.ServicePoll) + poll.WaitOn(t, swarm.NoTasksForService(ctx, client, serviceID), swarm.ServicePoll) err = client.SecretRemove(ctx, secretName) assert.NilError(t, err) @@ -306,9 +296,7 @@ func TestCreateServiceConfigFileMode(t *testing.T) { err = client.ServiceRemove(ctx, serviceID) assert.NilError(t, err) - - poll.WaitOn(t, serviceIsRemoved(client, serviceID)) - poll.WaitOn(t, noTasks(client)) + poll.WaitOn(t, swarm.NoTasksForService(ctx, client, serviceID)) err = client.ConfigRemove(ctx, configName) assert.NilError(t, err) @@ -336,34 +324,3 @@ func serviceRunningTasksCount(client client.ServiceAPIClient, serviceID string, } } } - -func noTasks(client client.ServiceAPIClient) func(log poll.LogT) poll.Result { - return func(log poll.LogT) poll.Result { - filter := filters.NewArgs() - tasks, err := client.TaskList(context.Background(), types.TaskListOptions{ - Filters: filter, - }) - switch { - case err != nil: - return poll.Error(err) - case len(tasks) == 0: - return poll.Success() - default: - return poll.Continue("task count at %d waiting for 0", len(tasks)) - } - } -} - -func serviceIsRemoved(client client.ServiceAPIClient, serviceID string) func(log poll.LogT) poll.Result { - return func(log poll.LogT) poll.Result { - filter := filters.NewArgs() - filter.Add("service", serviceID) - _, err := client.TaskList(context.Background(), types.TaskListOptions{ - Filters: filter, - }) - if err == nil { - return poll.Continue("waiting for service %s to be deleted", serviceID) - } - return poll.Success() - } -}