From c9575647537970e29103bd8e34b78c39ac2d7207 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Wed, 30 Mar 2016 12:21:00 -0700 Subject: [PATCH] Do not leave/delete gw endpoint twice - On sandbox delete, the leave and delete of each endpoint is performed, regardless of whether the endpoint is the gw network endpoint. This endpoint is already automatically removed in endpoint.sbLeave() by sb.clearDefaultGW() when the sandbox is marked for deletion. - Also restoring otiginal behavior where on disconnect from overlay network (only connected network), it also disconnects from default gw network. - Also do not let internal network dictate container does not need external connectivity. Before this fix, if a container was connected to an overlay and an internal network, it may not get attached to the default gw network. - needDefaultGw() takes now into account whether the sandbox is marked for deletion Signed-off-by: Alessandro Boch --- libnetwork/default_gateway.go | 23 ++++++++++--------- libnetwork/endpoint.go | 23 ++++++++++++++----- libnetwork/sandbox.go | 4 ++++ libnetwork/test/integration/dnet/helpers.bash | 4 ---- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/libnetwork/default_gateway.go b/libnetwork/default_gateway.go index d8eb732701..8f91dccbcd 100644 --- a/libnetwork/default_gateway.go +++ b/libnetwork/default_gateway.go @@ -65,20 +65,13 @@ func (sb *sandbox) setupDefaultGW() error { return nil } -// If present, removes the endpoint connecting the sandbox to the default gw network. -// Unless it is the endpoint designated to provide the external connectivity. -// If the sandbox is being deleted, removes the endpoint unconditionally. +// If present, detach and remove the endpoint connecting the sandbox to the default gw network. func (sb *sandbox) clearDefaultGW() error { var ep *endpoint if ep = sb.getEndpointInGWNetwork(); ep == nil { return nil } - - if ep == sb.getGatewayEndpoint() && !sb.inDelete { - return nil - } - if err := ep.sbLeave(sb, false); err != nil { return fmt.Errorf("container %s: endpoint leaving GW Network failed: %v", sb.containerID, err) } @@ -88,20 +81,27 @@ func (sb *sandbox) clearDefaultGW() error { return nil } +// Evaluate whether the sandbox needs to be attached to the default +// gateway network. func (sb *sandbox) needDefaultGW() bool { var needGW bool + if sb.inDelete { + return false + } + for _, ep := range sb.getConnectedEndpoints() { if ep.endpointInGWNetwork() { - return false + continue } if ep.getNetwork().Type() == "null" || ep.getNetwork().Type() == "host" { continue } if ep.getNetwork().Internal() { - return false + continue } - if ep.joinInfo.disableGatewayService { + // During stale sandbox cleanup, joinInfo may be nil + if ep.joinInfo != nil && ep.joinInfo.disableGatewayService { return false } // TODO v6 needs to be handled. @@ -115,6 +115,7 @@ func (sb *sandbox) needDefaultGW() bool { } needGW = true } + return needGW } diff --git a/libnetwork/endpoint.go b/libnetwork/endpoint.go index 6f142d0af3..c227a3bae3 100644 --- a/libnetwork/endpoint.go +++ b/libnetwork/endpoint.go @@ -479,7 +479,14 @@ func (ep *endpoint) sbJoin(sb *sandbox, options ...EndpointOption) error { } } - return sb.clearDefaultGW() + if !sb.needDefaultGW() { + if err := sb.clearDefaultGW(); err != nil { + log.Warnf("Failure while disconnecting sandbox %s (%s) from gateway network: %v", + sb.ID(), sb.ContainerID(), err) + } + } + + return nil } func (ep *endpoint) rename(name string) error { @@ -622,10 +629,7 @@ func (ep *endpoint) sbLeave(sb *sandbox, force bool, options ...EndpointOption) } sb.deleteHostsEntries(n.getSvcRecords(ep)) - if !sb.inDelete && sb.needDefaultGW() { - if sb.getEPwithoutGateway() == nil { - return fmt.Errorf("endpoint without GW expected, but not found") - } + if sb.needDefaultGW() { return sb.setupDefaultGW() } @@ -639,7 +643,14 @@ func (ep *endpoint) sbLeave(sb *sandbox, force bool, options ...EndpointOption) } } - return sb.clearDefaultGW() + if !sb.needDefaultGW() { + if err := sb.clearDefaultGW(); err != nil { + log.Warnf("Failure while disconnecting sandbox %s (%s) from gateway network: %v", + sb.ID(), sb.ContainerID(), err) + } + } + + return nil } func (n *network) validateForceDelete(locator string) error { diff --git a/libnetwork/sandbox.go b/libnetwork/sandbox.go index 18cca78dca..5517301bcc 100644 --- a/libnetwork/sandbox.go +++ b/libnetwork/sandbox.go @@ -197,6 +197,10 @@ func (sb *sandbox) delete(force bool) error { // Detach from all endpoints retain := false for _, ep := range sb.getConnectedEndpoints() { + // gw network endpoint detach and removal are automatic + if ep.endpointInGWNetwork() { + continue + } // Retain the sanbdox if we can't obtain the network from store. if _, err := c.getNetworkFromStore(ep.getNetwork().ID()); err != nil { retain = true diff --git a/libnetwork/test/integration/dnet/helpers.bash b/libnetwork/test/integration/dnet/helpers.bash index 9f3d59263f..11196638fa 100644 --- a/libnetwork/test/integration/dnet/helpers.bash +++ b/libnetwork/test/integration/dnet/helpers.bash @@ -345,10 +345,6 @@ function test_overlay() { # Disconnect from overlay network net_disconnect ${start} container_${start} multihost - # Make sure external connectivity works - runc $(dnet_container_name ${start} $dnet_suffix) $(get_sbox_id ${start} container_${start}) \ - "ping -c 1 www.google.com" - # Connect to overlay network again net_connect ${start} container_${start} multihost