Skip to content

Commit

Permalink
CleanupServiceBindings to clean the SD records
Browse files Browse the repository at this point in the history
Allow the cleanupServicebindings to take care of the service discovery
cleanup. Also avoid to trigger the cleanup for each endpoint from an SD
point of view
LB and SD will be separated in the future

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
  • Loading branch information
Flavio Crisciani committed Jun 17, 2017
1 parent ee1326a commit ccc21bc
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 30 deletions.
4 changes: 2 additions & 2 deletions agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ func (ep *endpoint) deleteServiceInfoFromCluster(sb *sandbox, method string) err
if n.ingress {
ingressPorts = ep.ingressPorts
}
if err := c.rmServiceBinding(ep.svcName, ep.svcID, n.ID(), ep.ID(), name, ep.virtualIP, ingressPorts, ep.svcAliases, ep.myAliases, ep.Iface().Address().IP, "deleteServiceInfoFromCluster"); err != nil {
if err := c.rmServiceBinding(ep.svcName, ep.svcID, n.ID(), ep.ID(), name, ep.virtualIP, ingressPorts, ep.svcAliases, ep.myAliases, ep.Iface().Address().IP, "deleteServiceInfoFromCluster", true); err != nil {
return err
}
} else {
Expand Down Expand Up @@ -900,7 +900,7 @@ func (c *controller) handleEpTableEvent(ev events.Event) {
logrus.Debugf("handleEpTableEvent DEL %s R:%v", eid, epRec)
if svcID != "" {
// This is a remote task part of a service
if err := c.rmServiceBinding(svcName, svcID, nid, eid, containerName, vip, ingressPorts, serviceAliases, taskAliases, ip, "handleEpTableEvent"); err != nil {
if err := c.rmServiceBinding(svcName, svcID, nid, eid, containerName, vip, ingressPorts, serviceAliases, taskAliases, ip, "handleEpTableEvent", true); err != nil {
logrus.Errorf("failed removing service binding for %s epRec:%v err:%s", eid, epRec, err)
return
}
Expand Down
6 changes: 0 additions & 6 deletions network.go
Original file line number Diff line number Diff line change
Expand Up @@ -1002,12 +1002,6 @@ func (n *network) delete(force bool) error {

c.cleanupServiceBindings(n.ID())

// The network had been left, the service discovery can be cleaned up
c.Lock()
logrus.Debugf("network %s delete, clean svcRecords", n.id)
delete(c.svcRecords, n.id)
c.Unlock()

removeFromStore:
// deleteFromStore performs an atomic delete operation and the
// network.epCnt will help prevent any possible
Expand Down
8 changes: 1 addition & 7 deletions service.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,8 @@ type loadBalancer struct {

// Map of backend IPs backing this loadbalancer on this
// network. It is keyed with endpoint ID.
backEnds map[string]loadBalancerBackend
backEnds map[string]net.IP

// Back pointer to service to which the loadbalancer belongs.
service *service
}

type loadBalancerBackend struct {
ip net.IP
containerName string
taskAliases []string
}
38 changes: 25 additions & 13 deletions service_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func (c *controller) getLBIndex(sid, nid string, ingressPorts []*PortConfig) int
func (c *controller) cleanupServiceBindings(cleanupNID string) {
var cleanupFuncs []func()

logrus.Debugf("cleanupServiceBindings for %s", cleanupNID)
c.Lock()
services := make([]*service, 0, len(c.serviceBindings))
for _, s := range c.serviceBindings {
Expand All @@ -171,16 +172,27 @@ func (c *controller) cleanupServiceBindings(cleanupNID string) {
continue
}

for eid, be := range lb.backEnds {
// The network is being deleted, erase all the associated service discovery records
// TODO(fcrisciani) separate the Load Balancer from the Service discovery, this operation
// can be done safely here, but the rmServiceBinding is still keeping consistency in the
// data structures that are tracking the endpoint to IP mapping.
c.Lock()
logrus.Debugf("cleanupServiceBindings erasing the svcRecords for %s", nid)
delete(c.svcRecords, nid)
c.Unlock()

for eid, ip := range lb.backEnds {
epID := eid
epIP := ip
service := s
loadBalancer := lb
networkID := nid
epID := eid
epIP := be.ip

cleanupFuncs = append(cleanupFuncs, func() {
if err := c.rmServiceBinding(service.name, service.id, networkID, epID, be.containerName, loadBalancer.vip,
service.ingressPorts, service.aliases, be.taskAliases, epIP, "cleanupServiceBindings"); err != nil {
// ContainerName and taskAliases are not available here, this is still fine because the Service discovery
// cleanup already happened before. The only thing that rmServiceBinding is still doing here a part from the Load
// Balancer bookeeping, is to keep consistent the mapping of endpoint to IP.
if err := c.rmServiceBinding(service.name, service.id, networkID, epID, "", loadBalancer.vip,
service.ingressPorts, service.aliases, []string{}, epIP, "cleanupServiceBindings", false); err != nil {
logrus.Errorf("Failed to remove service bindings for service %s network %s endpoint %s while cleanup: %v",
service.id, networkID, epID, err)
}
Expand Down Expand Up @@ -241,7 +253,7 @@ func (c *controller) addServiceBinding(svcName, svcID, nID, eID, containerName s
lb = &loadBalancer{
vip: vip,
fwMark: fwMarkCtr,
backEnds: make(map[string]loadBalancerBackend),
backEnds: make(map[string]net.IP),
service: s,
}

Expand All @@ -252,9 +264,7 @@ func (c *controller) addServiceBinding(svcName, svcID, nID, eID, containerName s
addService = true
}

lb.backEnds[eID] = loadBalancerBackend{ip: ip,
containerName: containerName,
taskAliases: taskAliases}
lb.backEnds[eID] = ip

ok, entries := s.assignIPToEndpoint(ip.String(), eID)
if !ok || entries > 1 {
Expand All @@ -276,7 +286,7 @@ func (c *controller) addServiceBinding(svcName, svcID, nID, eID, containerName s
return nil
}

func (c *controller) rmServiceBinding(svcName, svcID, nID, eID, containerName string, vip net.IP, ingressPorts []*PortConfig, serviceAliases []string, taskAliases []string, ip net.IP, method string) error {
func (c *controller) rmServiceBinding(svcName, svcID, nID, eID, containerName string, vip net.IP, ingressPorts []*PortConfig, serviceAliases []string, taskAliases []string, ip net.IP, method string, deleteSvcRecords bool) error {

var rmService bool

Expand All @@ -300,7 +310,7 @@ func (c *controller) rmServiceBinding(svcName, svcID, nID, eID, containerName st

s.Lock()
defer s.Unlock()
logrus.Debugf("rmServiceBinding from %s START for %s %s p:%p nid:%s sKey:%v", method, svcName, eID, s, nID, skey)
logrus.Debugf("rmServiceBinding from %s START for %s %s p:%p nid:%s sKey:%v deleteSvc:%t", method, svcName, eID, s, nID, skey, deleteSvcRecords)
lb, ok := s.loadBalancers[nID]
if !ok {
logrus.Warnf("rmServiceBinding %s %s %s aborted s.loadBalancers[nid] !ok", method, svcName, eID)
Expand Down Expand Up @@ -337,7 +347,9 @@ func (c *controller) rmServiceBinding(svcName, svcID, nID, eID, containerName st
}

// Delete the name resolutions
c.deleteEndpointNameResolution(svcName, svcID, nID, eID, containerName, vip, serviceAliases, taskAliases, ip, rmService, entries > 0, "rmServiceBinding")
if deleteSvcRecords {
c.deleteEndpointNameResolution(svcName, svcID, nID, eID, containerName, vip, serviceAliases, taskAliases, ip, rmService, entries > 0, "rmServiceBinding")
}

if len(s.loadBalancers) == 0 {
// All loadbalancers for the service removed. Time to
Expand Down
4 changes: 2 additions & 2 deletions service_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ func (sb *sandbox) populateLoadbalancers(ep *endpoint) {
}

lb.service.Lock()
for _, l := range lb.backEnds {
sb.addLBBackend(l.ip, lb.vip, lb.fwMark, lb.service.ingressPorts, eIP, gwIP, n.ingress)
for _, ip := range lb.backEnds {
sb.addLBBackend(ip, lb.vip, lb.fwMark, lb.service.ingressPorts, eIP, gwIP, n.ingress)
}
lb.service.Unlock()
}
Expand Down

0 comments on commit ccc21bc

Please sign in to comment.