From 2a7513a9721a222e384850c524aa499f80a40a7a Mon Sep 17 00:00:00 2001
From: Arko Dasgupta <arko.dasgupta@docker.com>
Date: Mon, 6 May 2019 08:46:12 -0700
Subject: [PATCH] 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 <arko.dasgupta@docker.com>
(cherry picked from commit 680d0ba4abfb00c8ac959638c72003dba0826b46)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 .../cluster/executor/container/controller.go  | 18 +++----
 integration/service/update_test.go            | 54 +++++++++++++++++++
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/daemon/cluster/executor/container/controller.go b/daemon/cluster/executor/container/controller.go
index 5552b13328951..ec758c5bd4e9f 100644
--- a/daemon/cluster/executor/container/controller.go
+++ b/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/integration/service/update_test.go b/integration/service/update_test.go
index 0fdc238fcf958..92a4368545081 100644
--- a/integration/service/update_test.go
+++ b/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{})