From 2012c45c5a8d6b12d3b11cbdd40e7a470b8c8678 Mon Sep 17 00:00:00 2001 From: Abhinandan Prativadi Date: Mon, 8 Jan 2018 14:25:50 -0800 Subject: [PATCH 1/2] Disable service on release network This PR contains a fix for moby/moby#30321. There was a moby/moby#31142 PR intending to fix the issue by adding a delay between disabling the service in the cluster and the shutdown of the tasks. However disabling the service was not deleting the service info in the cluster. Added a fix to delete service info from cluster and verified using siege to ensure there is zero downtime on rolling update of a service.In order to support it and ensure consitency of enabling and disable service knob from the daemon, we need to ensure we disable service when we release the network from the container. This helps in making the enable and disable service less racy. The corresponding part of libnetwork fix is part of docker/libnetwork#1824 Signed-off-by: abhi Upstream-commit: a042e5a20a7801efc936daf7a639487bb37ca966 Component: engine --- components/engine/daemon/container_operations.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/engine/daemon/container_operations.go b/components/engine/daemon/container_operations.go index 1a1c7c4b3f..3900e95e8d 100644 --- a/components/engine/daemon/container_operations.go +++ b/components/engine/daemon/container_operations.go @@ -966,6 +966,9 @@ func (daemon *Daemon) releaseNetwork(container *container.Container) { logrus.Warnf("error locating sandbox id %s: %v", sid, err) return } + if err := sb.DisableService(); err != nil { + logrus.WithFields(logrus.Fields{"container": container.ID, "sandbox": sid}).WithError(err).Error("Error removing service from sandbox") + } if err := sb.Delete(); err != nil { logrus.Errorf("Error deleting sandbox id %s for container %s: %v", sid, container.ID, err) From 2b31b5d6e4259d64cf1da1c7a78784c20896af66 Mon Sep 17 00:00:00 2001 From: Abhinandan Prativadi Date: Mon, 8 Jan 2018 15:10:34 -0800 Subject: [PATCH 2/2] libnetwork vendor Signed-off-by: abhi Upstream-commit: dad093cc34497bb7912c769e469a3e79a0c2c63c Component: engine --- components/engine/vendor.conf | 2 +- .../github.com/docker/libnetwork/endpoint.go | 29 +++++++++++-------- .../github.com/docker/libnetwork/sandbox.go | 27 +++++++++++++---- 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index 31630c0dba..855858f539 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -30,7 +30,7 @@ github.com/moby/buildkit aaff9d591ef128560018433fe61beb802e149de8 github.com/tonistiigi/fsutil dea3a0da73aee887fc02142d995be764106ac5e2 #get libnetwork packages -github.com/docker/libnetwork a1dfea384b39779552a3b4837ea9303194950976 +github.com/docker/libnetwork 315a076a4e9ded2abc950318c71d5f1637547977 github.com/docker/go-events 9461782956ad83b30282bf90e31fa6a70c255ba9 github.com/armon/go-radix e39d623f12e8e41c7b5529e9a9dd67a1e2261f80 github.com/armon/go-metrics eb0af217e5e9747e41dd5303755356b62d28e3ec diff --git a/components/engine/vendor/github.com/docker/libnetwork/endpoint.go b/components/engine/vendor/github.com/docker/libnetwork/endpoint.go index a2d1dbc4c6..1e1b6a1675 100644 --- a/components/engine/vendor/github.com/docker/libnetwork/endpoint.go +++ b/components/engine/vendor/github.com/docker/libnetwork/endpoint.go @@ -311,16 +311,25 @@ func (ep *endpoint) isAnonymous() bool { return ep.anonymous } -// enableService sets ep's serviceEnabled to the passed value if it's not in the -// current state and returns true; false otherwise. -func (ep *endpoint) enableService(state bool) bool { +// isServiceEnabled check if service is enabled on the endpoint +func (ep *endpoint) isServiceEnabled() bool { ep.Lock() defer ep.Unlock() - if ep.serviceEnabled != state { - ep.serviceEnabled = state - return true - } - return false + return ep.serviceEnabled +} + +// enableService sets service enabled on the endpoint +func (ep *endpoint) enableService() { + ep.Lock() + defer ep.Unlock() + ep.serviceEnabled = true +} + +// disableService disables service on the endpoint +func (ep *endpoint) disableService() { + ep.Lock() + defer ep.Unlock() + ep.serviceEnabled = false } func (ep *endpoint) needResolver() bool { @@ -759,10 +768,6 @@ func (ep *endpoint) sbLeave(sb *sandbox, force bool, options ...EndpointOption) return err } - if e := ep.deleteServiceInfoFromCluster(sb, "sbLeave"); e != nil { - logrus.Errorf("Could not delete service state for endpoint %s from cluster: %v", ep.Name(), e) - } - if e := ep.deleteDriverInfoFromCluster(); e != nil { logrus.Errorf("Could not delete endpoint state for endpoint %s from cluster: %v", ep.Name(), e) } diff --git a/components/engine/vendor/github.com/docker/libnetwork/sandbox.go b/components/engine/vendor/github.com/docker/libnetwork/sandbox.go index 315195ebb8..423066c128 100644 --- a/components/engine/vendor/github.com/docker/libnetwork/sandbox.go +++ b/components/engine/vendor/github.com/docker/libnetwork/sandbox.go @@ -674,24 +674,41 @@ func (sb *sandbox) SetKey(basePath string) error { return nil } -func (sb *sandbox) EnableService() error { +func (sb *sandbox) EnableService() (err error) { logrus.Debugf("EnableService %s START", sb.containerID) + defer func() { + if err != nil { + sb.DisableService() + } + }() for _, ep := range sb.getConnectedEndpoints() { - if ep.enableService(true) { + if !ep.isServiceEnabled() { if err := ep.addServiceInfoToCluster(sb); err != nil { - ep.enableService(false) return fmt.Errorf("could not update state for endpoint %s into cluster: %v", ep.Name(), err) } + ep.enableService() } } logrus.Debugf("EnableService %s DONE", sb.containerID) return nil } -func (sb *sandbox) DisableService() error { +func (sb *sandbox) DisableService() (err error) { logrus.Debugf("DisableService %s START", sb.containerID) + failedEps := []string{} + defer func() { + if len(failedEps) > 0 { + err = fmt.Errorf("failed to disable service on sandbox:%s, for endpoints %s", sb.ID(), strings.Join(failedEps, ",")) + } + }() for _, ep := range sb.getConnectedEndpoints() { - ep.enableService(false) + if ep.isServiceEnabled() { + if err := ep.deleteServiceInfoFromCluster(sb, "DisableService"); err != nil { + failedEps = append(failedEps, ep.Name()) + logrus.Warnf("failed update state for endpoint %s into cluster: %v", ep.Name(), err) + } + ep.disableService() + } } logrus.Debugf("DisableService %s DONE", sb.containerID) return nil