From 29d8b5118fad032ccdb2a58ed2b7cdeae84bb1b2 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 18 Jul 2018 17:34:19 -0400 Subject: [PATCH 1/6] migrated service integration tests from integration-cli/docker_cli_service_update_test.go to integration/service Signed-off-by: Arash Deshmeh Signed-off-by: Sebastiaan van Stijn (cherry picked from commit be151a73f0c0f362ff9775d2ff4d32329c01b8ba) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 4fe4e891723569fde7703747903289883db1dec0 Component: engine --- .../docker_cli_service_update_test.go | 137 ----------- .../engine/integration/service/update_test.go | 216 ++++++++++++++++++ 2 files changed, 216 insertions(+), 137 deletions(-) delete mode 100644 components/engine/integration-cli/docker_cli_service_update_test.go create mode 100644 components/engine/integration/service/update_test.go diff --git a/components/engine/integration-cli/docker_cli_service_update_test.go b/components/engine/integration-cli/docker_cli_service_update_test.go deleted file mode 100644 index c729860ec9..0000000000 --- a/components/engine/integration-cli/docker_cli_service_update_test.go +++ /dev/null @@ -1,137 +0,0 @@ -// +build !windows - -package main - -import ( - "encoding/json" - "fmt" - - "github.com/docker/docker/api/types/swarm" - "github.com/docker/docker/integration-cli/checker" - "github.com/go-check/check" -) - -func (s *DockerSwarmSuite) TestServiceUpdateLabel(c *check.C) { - d := s.AddDaemon(c, true, true) - out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name=test", "busybox", "top") - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - service := d.GetService(c, "test") - c.Assert(service.Spec.Labels, checker.HasLen, 0) - - // add label to empty set - out, err = d.Cmd("service", "update", "--detach", "test", "--label-add", "foo=bar") - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - service = d.GetService(c, "test") - c.Assert(service.Spec.Labels, checker.HasLen, 1) - c.Assert(service.Spec.Labels["foo"], checker.Equals, "bar") - - // add label to non-empty set - out, err = d.Cmd("service", "update", "--detach", "test", "--label-add", "foo2=bar") - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - service = d.GetService(c, "test") - c.Assert(service.Spec.Labels, checker.HasLen, 2) - c.Assert(service.Spec.Labels["foo2"], checker.Equals, "bar") - - out, err = d.Cmd("service", "update", "--detach", "test", "--label-rm", "foo2") - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - service = d.GetService(c, "test") - c.Assert(service.Spec.Labels, checker.HasLen, 1) - c.Assert(service.Spec.Labels["foo2"], checker.Equals, "") - - out, err = d.Cmd("service", "update", "--detach", "test", "--label-rm", "foo") - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - service = d.GetService(c, "test") - c.Assert(service.Spec.Labels, checker.HasLen, 0) - c.Assert(service.Spec.Labels["foo"], checker.Equals, "") - - // now make sure we can add again - out, err = d.Cmd("service", "update", "--detach", "test", "--label-add", "foo=bar") - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - service = d.GetService(c, "test") - c.Assert(service.Spec.Labels, checker.HasLen, 1) - c.Assert(service.Spec.Labels["foo"], checker.Equals, "bar") -} - -func (s *DockerSwarmSuite) TestServiceUpdateSecrets(c *check.C) { - d := s.AddDaemon(c, true, true) - testName := "test_secret" - id := d.CreateSecret(c, swarm.SecretSpec{ - Annotations: swarm.Annotations{ - Name: testName, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("secrets: %s", id)) - testTarget := "testing" - serviceName := "test" - - out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", serviceName, "busybox", "top") - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - - // add secret - out, err = d.Cmd("service", "update", "--detach", "test", "--secret-add", fmt.Sprintf("source=%s,target=%s", testName, testTarget)) - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - - out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.TaskTemplate.ContainerSpec.Secrets }}", serviceName) - c.Assert(err, checker.IsNil) - - var refs []swarm.SecretReference - c.Assert(json.Unmarshal([]byte(out), &refs), checker.IsNil) - c.Assert(refs, checker.HasLen, 1) - - c.Assert(refs[0].SecretName, checker.Equals, testName) - c.Assert(refs[0].File, checker.Not(checker.IsNil)) - c.Assert(refs[0].File.Name, checker.Equals, testTarget) - - // remove - out, err = d.Cmd("service", "update", "--detach", "test", "--secret-rm", testName) - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - - out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.TaskTemplate.ContainerSpec.Secrets }}", serviceName) - c.Assert(err, checker.IsNil) - - c.Assert(json.Unmarshal([]byte(out), &refs), checker.IsNil) - c.Assert(refs, checker.HasLen, 0) -} - -func (s *DockerSwarmSuite) TestServiceUpdateConfigs(c *check.C) { - d := s.AddDaemon(c, true, true) - testName := "test_config" - id := d.CreateConfig(c, swarm.ConfigSpec{ - Annotations: swarm.Annotations{ - Name: testName, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("configs: %s", id)) - testTarget := "/testing" - serviceName := "test" - - out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name", serviceName, "busybox", "top") - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - - // add config - out, err = d.Cmd("service", "update", "--detach", "test", "--config-add", fmt.Sprintf("source=%s,target=%s", testName, testTarget)) - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - - out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.TaskTemplate.ContainerSpec.Configs }}", serviceName) - c.Assert(err, checker.IsNil) - - var refs []swarm.ConfigReference - c.Assert(json.Unmarshal([]byte(out), &refs), checker.IsNil) - c.Assert(refs, checker.HasLen, 1) - - c.Assert(refs[0].ConfigName, checker.Equals, testName) - c.Assert(refs[0].File, checker.Not(checker.IsNil)) - c.Assert(refs[0].File.Name, checker.Equals, testTarget) - - // remove - out, err = d.Cmd("service", "update", "--detach", "test", "--config-rm", testName) - c.Assert(err, checker.IsNil, check.Commentf("%s", out)) - - out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.TaskTemplate.ContainerSpec.Configs }}", serviceName) - c.Assert(err, checker.IsNil) - - c.Assert(json.Unmarshal([]byte(out), &refs), checker.IsNil) - c.Assert(refs, checker.HasLen, 0) -} diff --git a/components/engine/integration/service/update_test.go b/components/engine/integration/service/update_test.go new file mode 100644 index 0000000000..7e9e247e52 --- /dev/null +++ b/components/engine/integration/service/update_test.go @@ -0,0 +1,216 @@ +package service // import "github.com/docker/docker/integration/service" + +import ( + "context" + "testing" + + "github.com/docker/docker/api/types" + swarmtypes "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/client" + "github.com/docker/docker/integration/internal/swarm" + "gotest.tools/assert" + is "gotest.tools/assert/cmp" + "gotest.tools/poll" + "gotest.tools/skip" +) + +func TestServiceUpdateLabel(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + defer setupTest(t)() + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + cli := d.NewClientT(t) + defer cli.Close() + + ctx := context.Background() + serviceName := "TestService_" + t.Name() + serviceID := swarm.CreateService(t, d, swarm.ServiceWithName(serviceName)) + service := getService(t, cli, serviceID) + assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{})) + + // add label to empty set + service.Spec.Labels["foo"] = "bar" + _, err := cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + service = getService(t, cli, serviceID) + assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) + + // add label to non-empty set + service.Spec.Labels["foo2"] = "bar" + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + service = getService(t, cli, serviceID) + assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar", "foo2": "bar"})) + + delete(service.Spec.Labels, "foo2") + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + service = getService(t, cli, serviceID) + assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) + + delete(service.Spec.Labels, "foo") + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + service = getService(t, cli, serviceID) + assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{})) + + // now make sure we can add again + service.Spec.Labels["foo"] = "bar" + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + service = getService(t, cli, serviceID) + assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) + + err = cli.ServiceRemove(context.Background(), serviceID) + assert.NilError(t, err) +} + +func TestServiceUpdateSecrets(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + defer setupTest(t)() + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + cli := d.NewClientT(t) + defer cli.Close() + + ctx := context.Background() + secretName := "TestSecret_" + t.Name() + secretTarget := "targetName" + resp, err := cli.SecretCreate(ctx, swarmtypes.SecretSpec{ + Annotations: swarmtypes.Annotations{ + Name: secretName, + }, + Data: []byte("TESTINGDATA"), + }) + assert.NilError(t, err) + assert.Check(t, resp.ID != "") + + serviceName := "TestService_" + t.Name() + serviceID := swarm.CreateService(t, d, swarm.ServiceWithName(serviceName)) + service := getService(t, cli, serviceID) + + // add secret + service.Spec.TaskTemplate.ContainerSpec.Secrets = append(service.Spec.TaskTemplate.ContainerSpec.Secrets, + &swarmtypes.SecretReference{ + File: &swarmtypes.SecretReferenceFileTarget{ + Name: secretTarget, + UID: "0", + GID: "0", + Mode: 0600, + }, + SecretID: resp.ID, + SecretName: secretName, + }, + ) + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + + service = getService(t, cli, serviceID) + secrets := service.Spec.TaskTemplate.ContainerSpec.Secrets + assert.Assert(t, is.Equal(1, len(secrets))) + + secret := *secrets[0] + assert.Check(t, is.Equal(secretName, secret.SecretName)) + assert.Check(t, nil != secret.File) + assert.Check(t, is.Equal(secretTarget, secret.File.Name)) + + // remove + service.Spec.TaskTemplate.ContainerSpec.Secrets = []*swarmtypes.SecretReference{} + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + service = getService(t, cli, serviceID) + assert.Check(t, is.Equal(0, len(service.Spec.TaskTemplate.ContainerSpec.Secrets))) + + err = cli.ServiceRemove(context.Background(), serviceID) + assert.NilError(t, err) +} + +func TestServiceUpdateConfigs(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + defer setupTest(t)() + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + cli := d.NewClientT(t) + defer cli.Close() + + ctx := context.Background() + configName := "TestConfig_" + t.Name() + configTarget := "targetName" + resp, err := cli.ConfigCreate(ctx, swarmtypes.ConfigSpec{ + Annotations: swarmtypes.Annotations{ + Name: configName, + }, + Data: []byte("TESTINGDATA"), + }) + assert.NilError(t, err) + assert.Check(t, resp.ID != "") + + serviceName := "TestService_" + t.Name() + serviceID := swarm.CreateService(t, d, swarm.ServiceWithName(serviceName)) + service := getService(t, cli, serviceID) + + // add config + service.Spec.TaskTemplate.ContainerSpec.Configs = append(service.Spec.TaskTemplate.ContainerSpec.Configs, + &swarmtypes.ConfigReference{ + File: &swarmtypes.ConfigReferenceFileTarget{ + Name: configTarget, + UID: "0", + GID: "0", + Mode: 0600, + }, + ConfigID: resp.ID, + ConfigName: configName, + }, + ) + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + + service = getService(t, cli, serviceID) + configs := service.Spec.TaskTemplate.ContainerSpec.Configs + assert.Assert(t, is.Equal(1, len(configs))) + + config := *configs[0] + assert.Check(t, is.Equal(configName, config.ConfigName)) + assert.Check(t, nil != config.File) + assert.Check(t, is.Equal(configTarget, config.File.Name)) + + // remove + service.Spec.TaskTemplate.ContainerSpec.Configs = []*swarmtypes.ConfigReference{} + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + service = getService(t, cli, serviceID) + assert.Check(t, is.Equal(0, len(service.Spec.TaskTemplate.ContainerSpec.Configs))) + + err = cli.ServiceRemove(context.Background(), serviceID) + assert.NilError(t, err) +} + +func getService(t *testing.T, cli client.ServiceAPIClient, serviceID string) swarmtypes.Service { + t.Helper() + service, _, err := cli.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) + assert.NilError(t, err) + return service +} + +func serviceIsUpdated(client client.ServiceAPIClient, serviceID string) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + service, _, err := client.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) + switch { + case err != nil: + return poll.Error(err) + case service.UpdateStatus == nil || service.UpdateStatus.State == swarmtypes.UpdateStateCompleted: + return poll.Success() + default: + return poll.Continue("waiting for service %s to be updated, state: %s, message: %s", serviceID, service.UpdateStatus.State, service.UpdateStatus.Message) + } + } +} From 9d41ea2d82cd0fed78d28cb3a047d0c889a6532c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 2 Jan 2019 14:36:22 +0100 Subject: [PATCH 2/6] integration: wait for service update to be completed Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 8edcd4c3cd294bf276ffbe29bc58afbb006593b8) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 2ae0365c99627343d001a24e171fba2aa0fea791 Component: engine --- components/engine/integration/service/update_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/components/engine/integration/service/update_test.go b/components/engine/integration/service/update_test.go index 7e9e247e52..0fdc238fcf 100644 --- a/components/engine/integration/service/update_test.go +++ b/components/engine/integration/service/update_test.go @@ -207,10 +207,13 @@ func serviceIsUpdated(client client.ServiceAPIClient, serviceID string) func(log switch { case err != nil: return poll.Error(err) - case service.UpdateStatus == nil || service.UpdateStatus.State == swarmtypes.UpdateStateCompleted: + case service.UpdateStatus != nil && service.UpdateStatus.State == swarmtypes.UpdateStateCompleted: return poll.Success() default: - return poll.Continue("waiting for service %s to be updated, state: %s, message: %s", serviceID, service.UpdateStatus.State, service.UpdateStatus.Message) + if service.UpdateStatus != nil { + return poll.Continue("waiting for service %s to be updated, state: %s, message: %s", serviceID, service.UpdateStatus.State, service.UpdateStatus.Message) + } + return poll.Continue("waiting for service %s to be updated", serviceID) } } } From e91395081e6580ce00228a7e64aff31a0ce5a688 Mon Sep 17 00:00:00 2001 From: Olli Janatuinen Date: Sat, 5 Jan 2019 15:13:06 +0200 Subject: [PATCH 3/6] integration: Corrected service update tests logic Tests which will re-deploy containers uses function serviceIsUpdated() to make sure that service update really reached state UpdateStateCompleted. Tests which will not re-deploy container uses function serviceSpecIsUpdated to make sure that service version is increased. Signed-off-by: Olli Janatuinen (cherry picked from commit b868ada474b5c214ed9bddb792dd36f794dc1fa6) Signed-off-by: Sebastiaan van Stijn Upstream-commit: ccc1abea092a54d99dec51345ef9303b4629572d Component: engine --- .../engine/integration/service/update_test.go | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/components/engine/integration/service/update_test.go b/components/engine/integration/service/update_test.go index 0fdc238fcf..4c1b94f8c0 100644 --- a/components/engine/integration/service/update_test.go +++ b/components/engine/integration/service/update_test.go @@ -32,7 +32,7 @@ func TestServiceUpdateLabel(t *testing.T) { service.Spec.Labels["foo"] = "bar" _, err := cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) @@ -40,21 +40,21 @@ func TestServiceUpdateLabel(t *testing.T) { service.Spec.Labels["foo2"] = "bar" _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar", "foo2": "bar"})) delete(service.Spec.Labels, "foo2") _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) delete(service.Spec.Labels, "foo") _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{})) @@ -62,7 +62,7 @@ func TestServiceUpdateLabel(t *testing.T) { service.Spec.Labels["foo"] = "bar" _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) @@ -217,3 +217,17 @@ func serviceIsUpdated(client client.ServiceAPIClient, serviceID string) func(log } } } + +func serviceSpecIsUpdated(client client.ServiceAPIClient, serviceID string, serviceOldVersion uint64) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + service, _, err := client.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) + switch { + case err != nil: + return poll.Error(err) + case service.Version.Index > serviceOldVersion: + return poll.Success() + default: + return poll.Continue("waiting for service %s to be updated", serviceID) + } + } +} From e00a39ba2ba772551ba0cb40e9cb565cb72ef33e Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sat, 19 Jan 2019 18:54:32 +0000 Subject: [PATCH 4/6] Move serviceRunningTasksCount to integration/internal/swarm This fix moves multiple places of serviceRunningTasksCount to one location in integration/internal/swarm, so that code duplication could be removed. Signed-off-by: Yong Tang (cherry picked from commit e485a60e2bcc59860f387c94f6afaa0130ea7040) Signed-off-by: Sebastiaan van Stijn Upstream-commit: c087f681d40332bd7d158baf615c03f6a72274f1 Component: engine --- .../integration/internal/swarm/states.go | 25 ++++++++++++ .../integration/network/inspect_test.go | 32 +-------------- .../engine/integration/service/create_test.go | 39 ++++--------------- 3 files changed, 34 insertions(+), 62 deletions(-) diff --git a/components/engine/integration/internal/swarm/states.go b/components/engine/integration/internal/swarm/states.go index 51d6200594..c51e1eed40 100644 --- a/components/engine/integration/internal/swarm/states.go +++ b/components/engine/integration/internal/swarm/states.go @@ -5,6 +5,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" + swarmtypes "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/client" "gotest.tools/poll" ) @@ -45,3 +46,27 @@ func NoTasks(ctx context.Context, client client.ServiceAPIClient) func(log poll. } } } + +// RunningTasksCount verifies there are `instances` tasks running for `serviceID` +func RunningTasksCount(client client.ServiceAPIClient, serviceID string, instances uint64) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + filter := filters.NewArgs() + filter.Add("service", serviceID) + tasks, err := client.TaskList(context.Background(), types.TaskListOptions{ + Filters: filter, + }) + switch { + case err != nil: + return poll.Error(err) + case len(tasks) == int(instances): + for _, task := range tasks { + if task.Status.State != swarmtypes.TaskStateRunning { + return poll.Continue("waiting for tasks to enter run state") + } + } + return poll.Success() + default: + return poll.Continue("task count at %d waiting for %d", len(tasks), instances) + } + } +} diff --git a/components/engine/integration/network/inspect_test.go b/components/engine/integration/network/inspect_test.go index d12ad6781d..02d2b754f5 100644 --- a/components/engine/integration/network/inspect_test.go +++ b/components/engine/integration/network/inspect_test.go @@ -5,9 +5,6 @@ import ( "testing" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/filters" - swarmtypes "github.com/docker/docker/api/types/swarm" - "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/network" "github.com/docker/docker/integration/internal/swarm" "gotest.tools/assert" @@ -38,7 +35,7 @@ func TestInspectNetwork(t *testing.T) { swarm.ServiceWithNetwork(networkName), ) - poll.WaitOn(t, serviceRunningTasksCount(c, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(c, serviceID, instances), swarm.ServicePoll) tests := []struct { name string @@ -103,30 +100,3 @@ func TestInspectNetwork(t *testing.T) { assert.NilError(t, err) poll.WaitOn(t, network.IsRemoved(ctx, c, overlayID), swarm.NetworkPoll) } - -func serviceRunningTasksCount(client client.ServiceAPIClient, serviceID string, instances uint64) func(log poll.LogT) poll.Result { - return func(log poll.LogT) poll.Result { - tasks, err := client.TaskList(context.Background(), types.TaskListOptions{ - Filters: filters.NewArgs( - filters.Arg("service", serviceID), - filters.Arg("desired-state", string(swarmtypes.TaskStateRunning)), - ), - }) - switch { - case err != nil: - return poll.Error(err) - case len(tasks) == int(instances): - for _, task := range tasks { - if task.Status.Err != "" { - log.Log("task error:", task.Status.Err) - } - if task.Status.State != swarmtypes.TaskStateRunning { - return poll.Continue("waiting for tasks to enter run state (current status: %s)", task.Status.State) - } - } - return poll.Success() - default: - return poll.Continue("task count for service %s at %d waiting for %d", serviceID, len(tasks), instances) - } - } -} diff --git a/components/engine/integration/service/create_test.go b/components/engine/integration/service/create_test.go index 6e79bec9a8..91e3274313 100644 --- a/components/engine/integration/service/create_test.go +++ b/components/engine/integration/service/create_test.go @@ -42,18 +42,18 @@ func testServiceCreateInit(daemonEnabled bool) func(t *testing.T) { booleanFalse := false serviceID := swarm.CreateService(t, d) - poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, 1), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(client, serviceID, 1), swarm.ServicePoll) i := inspectServiceContainer(t, client, serviceID) // HostConfig.Init == nil means that it delegates to daemon configuration assert.Check(t, i.HostConfig.Init == nil) serviceID = swarm.CreateService(t, d, swarm.ServiceWithInit(&booleanTrue)) - poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, 1), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(client, serviceID, 1), swarm.ServicePoll) i = inspectServiceContainer(t, client, serviceID) assert.Check(t, is.Equal(true, *i.HostConfig.Init)) serviceID = swarm.CreateService(t, d, swarm.ServiceWithInit(&booleanFalse)) - poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, 1), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(client, serviceID, 1), swarm.ServicePoll) i = inspectServiceContainer(t, client, serviceID) assert.Check(t, is.Equal(false, *i.HostConfig.Init)) } @@ -97,7 +97,7 @@ func TestCreateServiceMultipleTimes(t *testing.T) { } serviceID := swarm.CreateService(t, d, serviceSpec...) - poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(client, serviceID, instances), swarm.ServicePoll) _, _, err := client.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) assert.NilError(t, err) @@ -108,7 +108,7 @@ func TestCreateServiceMultipleTimes(t *testing.T) { 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) + poll.WaitOn(t, swarm.RunningTasksCount(client, serviceID2, instances), swarm.ServicePoll) err = client.ServiceRemove(context.Background(), serviceID2) assert.NilError(t, err) @@ -147,7 +147,7 @@ func TestCreateWithDuplicateNetworkNames(t *testing.T) { swarm.ServiceWithNetwork(name), ) - poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(client, serviceID, instances), swarm.ServicePoll) resp, _, err := client.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) assert.NilError(t, err) @@ -210,7 +210,7 @@ func TestCreateServiceSecretFileMode(t *testing.T) { }), ) - poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances), swarm.ServicePoll) + poll.WaitOn(t, swarm.RunningTasksCount(client, serviceID, instances), swarm.ServicePoll) filter := filters.NewArgs() filter.Add("service", serviceID) @@ -274,7 +274,7 @@ func TestCreateServiceConfigFileMode(t *testing.T) { }), ) - poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances)) + poll.WaitOn(t, swarm.RunningTasksCount(client, serviceID, instances)) filter := filters.NewArgs() filter.Add("service", serviceID) @@ -301,26 +301,3 @@ func TestCreateServiceConfigFileMode(t *testing.T) { err = client.ConfigRemove(ctx, configName) assert.NilError(t, err) } - -func serviceRunningTasksCount(client client.ServiceAPIClient, serviceID string, instances uint64) func(log poll.LogT) poll.Result { - return func(log poll.LogT) poll.Result { - filter := filters.NewArgs() - filter.Add("service", serviceID) - tasks, err := client.TaskList(context.Background(), types.TaskListOptions{ - Filters: filter, - }) - switch { - case err != nil: - return poll.Error(err) - case len(tasks) == int(instances): - for _, task := range tasks { - if task.Status.State != swarmtypes.TaskStateRunning { - return poll.Continue("waiting for tasks to enter run state") - } - } - return poll.Success() - default: - return poll.Continue("task count at %d waiting for %d", len(tasks), instances) - } - } -} From ac643ab1345947a7d9cc9de752907cf2a49b2826 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Mon, 6 May 2019 08:46:12 -0700 Subject: [PATCH 5/6] Remove a network during task SHUTDOWN instead of REMOVE to make sure the LB sandbox is removed when a service is updated with a --network-rm option Signed-off-by: Arko Dasgupta (cherry picked from commit 680d0ba4abfb00c8ac959638c72003dba0826b46) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 280b8dff7df137dd21fcf27cacf6d12ed531770a Component: engine --- .../cluster/executor/container/controller.go | 18 +++---- .../engine/integration/service/update_test.go | 54 +++++++++++++++++++ 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/components/engine/daemon/cluster/executor/container/controller.go b/components/engine/daemon/cluster/executor/container/controller.go index 8d070799f3..6c74d4222a 100644 --- a/components/engine/daemon/cluster/executor/container/controller.go +++ b/components/engine/daemon/cluster/executor/container/controller.go @@ -376,6 +376,15 @@ func (r *controller) Shutdown(ctx context.Context) error { return err } + // Try removing networks referenced in this task in case this + // task is the last one referencing it + if err := r.adapter.removeNetworks(ctx); err != nil { + if isUnknownContainer(err) { + return nil + } + return err + } + return nil } @@ -419,15 +428,6 @@ func (r *controller) Remove(ctx context.Context) error { log.G(ctx).WithError(err).Debug("shutdown failed on removal") } - // Try removing networks referenced in this task in case this - // task is the last one referencing it - if err := r.adapter.removeNetworks(ctx); err != nil { - if isUnknownContainer(err) { - return nil - } - return err - } - if err := r.adapter.remove(ctx); err != nil { if isUnknownContainer(err) { return nil diff --git a/components/engine/integration/service/update_test.go b/components/engine/integration/service/update_test.go index 4c1b94f8c0..8575e56857 100644 --- a/components/engine/integration/service/update_test.go +++ b/components/engine/integration/service/update_test.go @@ -7,6 +7,7 @@ import ( "github.com/docker/docker/api/types" swarmtypes "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/client" + "github.com/docker/docker/integration/internal/network" "github.com/docker/docker/integration/internal/swarm" "gotest.tools/assert" is "gotest.tools/assert/cmp" @@ -194,6 +195,59 @@ func TestServiceUpdateConfigs(t *testing.T) { assert.NilError(t, err) } +func TestServiceUpdateNetwork(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + defer setupTest(t)() + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + cli := d.NewClientT(t) + defer cli.Close() + + ctx := context.Background() + + // Create a overlay network + testNet := "testNet" + t.Name() + overlayID := network.CreateNoError(t, ctx, cli, testNet, + network.WithDriver("overlay")) + + var instances uint64 = 1 + // Create service with the overlay network + serviceName := "TestServiceUpdateNetworkRM_" + t.Name() + serviceID := swarm.CreateService(t, d, + swarm.ServiceWithReplicas(instances), + swarm.ServiceWithName(serviceName), + swarm.ServiceWithNetwork(testNet)) + + poll.WaitOn(t, swarm.RunningTasksCount(cli, serviceID, instances), swarm.ServicePoll) + service := getService(t, cli, serviceID) + netInfo, err := cli.NetworkInspect(ctx, testNet, types.NetworkInspectOptions{ + Verbose: true, + Scope: "swarm", + }) + assert.NilError(t, err) + assert.Assert(t, len(netInfo.Containers) == 2, "Expected 2 endpoints, one for container and one for LB Sandbox") + + //Remove network from service + service.Spec.TaskTemplate.Networks = []swarmtypes.NetworkAttachmentConfig{} + _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) + assert.NilError(t, err) + poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + + netInfo, err = cli.NetworkInspect(ctx, testNet, types.NetworkInspectOptions{ + Verbose: true, + Scope: "swarm", + }) + + assert.NilError(t, err) + assert.Assert(t, len(netInfo.Containers) == 0, "Load balancing endpoint still exists in network") + + err = cli.NetworkRemove(ctx, overlayID) + assert.NilError(t, err) + + err = cli.ServiceRemove(ctx, serviceID) + assert.NilError(t, err) +} + func getService(t *testing.T, cli client.ServiceAPIClient, serviceID string) swarmtypes.Service { t.Helper() service, _, err := cli.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) From 8642bba6c4a28e4db2b21042cebfdc7c488d1fac Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Mon, 20 May 2019 14:23:35 -0700 Subject: [PATCH 6/6] Network not deleted after stack is removed Make sure adapter.removeNetworks executes during task Remove adapter.removeNetworks was being skipped for cases when isUnknownContainer(err) was true after adapter.remove was executed This fix eliminates the nil return case forcing the function to continue executing unless there is a true error Fixes https://github.com/moby/moby/issues/39225 Signed-off-by: Arko Dasgupta (cherry picked from commit 70fa7b6a3fd9aaada582ae02c50710f218b54d1a) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 75887d37e1ddbef579e239ff0b1b7a2508e486fd Component: engine --- .../daemon/cluster/executor/container/controller.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/components/engine/daemon/cluster/executor/container/controller.go b/components/engine/daemon/cluster/executor/container/controller.go index 6c74d4222a..466bca2f59 100644 --- a/components/engine/daemon/cluster/executor/container/controller.go +++ b/components/engine/daemon/cluster/executor/container/controller.go @@ -369,20 +369,17 @@ func (r *controller) Shutdown(ctx context.Context) error { } if err := r.adapter.shutdown(ctx); err != nil { - if isUnknownContainer(err) || isStoppedContainer(err) { - return nil + if !(isUnknownContainer(err) || isStoppedContainer(err)) { + return err } - - return err } // Try removing networks referenced in this task in case this // task is the last one referencing it if err := r.adapter.removeNetworks(ctx); err != nil { - if isUnknownContainer(err) { - return nil + if !isUnknownContainer(err) { + return err } - return err } return nil