Backport sd race network

agent.go

@@ -525,13 +525,13 @@ func (ep *endpoint) addServiceInfoToCluster(sb *sandbox) error {
 			TaskAliases:  ep.myAliases,
 			EndpointIP:   ep.Iface().Address().IP.String(),
 		})
-
 		if err != nil {
 			return err
 		}
 
 		if agent != nil {
 			if err := agent.networkDB.CreateEntry("endpoint_table", n.ID(), ep.ID(), buf); err != nil {
+				logrus.Warnf("addServiceInfoToCluster NetworkDB CreateEntry failed for %s %s err:%s", ep.id, n.id, err)
 				return err
 			}
 		}
@@ -556,13 +556,20 @@ func (ep *endpoint) deleteServiceInfoFromCluster(sb *sandbox, method string) err
 	agent := c.getAgent()
 
 	if !ep.isAnonymous() && ep.Iface().Address() != nil {
+		// Delete the entry from network DB, this will trigger a notification to other nodes
+		if agent != nil {
+			if err := agent.networkDB.DeleteEntry("endpoint_table", n.ID(), ep.ID()); err != nil {
+				logrus.Warnf("deleteServiceInfoFromCluster NetworkDB DeleteEntry failed for %s %s err:%s", ep.id, n.id, err)
+			}
+		}
+		// Update locally
 		if ep.svcID != "" {
 			// This is a task part of a service
 			var ingressPorts []*PortConfig
 			if n.ingress {
 				ingressPorts = ep.ingressPorts
 			}
-			if err := c.rmServiceBinding(ep.svcName, ep.svcID, n.ID(), ep.ID(), ep.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(), ep.Name(), ep.virtualIP, ingressPorts, ep.svcAliases, ep.myAliases, ep.Iface().Address().IP, "deleteServiceInfoFromCluster", true); err != nil {
 				return err
 			}
 		} else {
@@ -571,11 +578,6 @@ func (ep *endpoint) deleteServiceInfoFromCluster(sb *sandbox, method string) err
 				return err
 			}
 		}
-		if agent != nil {
-			if err := agent.networkDB.DeleteEntry("endpoint_table", n.ID(), ep.ID()); err != nil {
-				return err
-			}
-		}
 	}
 
 	logrus.Debugf("deleteServiceInfoFromCluster from %s END for %s %s", method, ep.svcName, ep.ID())
@@ -751,7 +753,7 @@ func (c *controller) handleEpTableEvent(ev events.Event) {
 	}
 
 	if isAdd {
-		logrus.Debugf("handleEpTableEvent ADD %s R:%v", isAdd, eid, epRec)
+		logrus.Debugf("handleEpTableEvent ADD %s R:%v", eid, epRec)
 		if svcID != "" {
 			// This is a remote task part of a service
 			if err := c.addServiceBinding(svcName, svcID, nid, eid, containerName, vip, ingressPorts, serviceAliases, taskAliases, ip, "handleEpTableEvent"); err != nil {
@@ -765,10 +767,10 @@ func (c *controller) handleEpTableEvent(ev events.Event) {
 			}
 		}
 	} else {
-		logrus.Debugf("handleEpTableEvent DEL %s R:%v", isAdd, eid, epRec)
+		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
 			}

common/setmatrix.go

@@ -10,24 +10,26 @@ import (
 type SetMatrix interface {
 	// Get returns the members of the set for a specific key as a slice.
 	Get(key string) ([]interface{}, bool)
-	// Contains is used to verify is an element is in a set for a specific key
+	// Contains is used to verify if an element is in a set for a specific key
 	// returns true if the element is in the set
 	// returns true if there is a set for the key
 	Contains(key string, value interface{}) (bool, bool)
-	// Insert inserts the mapping between the IP and the endpoint identifier
-	// returns true if the mapping was not present, false otherwise
-	// returns also the number of endpoints associated to the IP
+	// Insert inserts the value in the set of a key
+	// returns true if the value is inserted (was not already in the set), false otherwise
+	// returns also the length of the set for the key
 	Insert(key string, value interface{}) (bool, int)
-	// Remove removes the mapping between the IP and the endpoint identifier
-	// returns true if the mapping was deleted, false otherwise
-	// returns also the number of endpoints associated to the IP
+	// Remove removes the value in the set for a specific key
+	// returns true if the value is deleted, false otherwise
+	// returns also the length of the set for the key
 	Remove(key string, value interface{}) (bool, int)
-	// Cardinality returns the number of elements in the set of a specfic key
-	// returns false if the key is not in the map
+	// Cardinality returns the number of elements in the set for a key
+	// returns false if the set is not present
 	Cardinality(key string) (int, bool)
 	// String returns the string version of the set, empty otherwise
-	// returns false if the key is not in the map
+	// returns false if the set is not present
 	String(key string) (string, bool)
+	// Returns all the keys in the map
+	Keys() []string
 }
 
 type setMatrix struct {
@@ -121,3 +123,13 @@ func (s *setMatrix) String(key string) (string, bool) {
 	}
 	return set.String(), ok
 }
+
+func (s *setMatrix) Keys() []string {
+	s.Lock()
+	defer s.Unlock()
+	keys := make([]string, 0, len(s.matrix))
+	for k := range s.matrix {
+		keys = append(keys, k)
+	}
+	return keys
+}

libnetwork_internal_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/docker/libnetwork/driverapi"
 	"github.com/docker/libnetwork/ipamapi"
 	"github.com/docker/libnetwork/netlabel"
+	"github.com/docker/libnetwork/netutils"
 	"github.com/docker/libnetwork/testutils"
 	"github.com/docker/libnetwork/types"
 )
@@ -379,8 +380,8 @@ func TestSRVServiceQuery(t *testing.T) {
 	}
 
 	sr := svcInfo{
-		svcMap:     make(map[string][]net.IP),
-		svcIPv6Map: make(map[string][]net.IP),
+		svcMap:     common.NewSetMatrix(),
+		svcIPv6Map: common.NewSetMatrix(),
 		ipMap:      common.NewSetMatrix(),
 		service:    make(map[string][]servicePorts),
 	}
@@ -437,6 +438,119 @@ func TestSRVServiceQuery(t *testing.T) {
 	}
 }
 
+func TestServiceVIPReuse(t *testing.T) {
+	c, err := New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer c.Stop()
+
+	n, err := c.NewNetwork("bridge", "net1", "", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := n.Delete(); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	ep, err := n.CreateEndpoint("testep")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	sb, err := c.NewSandbox("c1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := sb.Delete(); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	err = ep.Join(sb)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Add 2 services with same name but different service ID to share the same VIP
+	n.(*network).addSvcRecords("ep1", "service_test", "serviceID1", net.ParseIP("192.168.0.1"), net.IP{}, true, "test")
+	n.(*network).addSvcRecords("ep2", "service_test", "serviceID2", net.ParseIP("192.168.0.1"), net.IP{}, true, "test")
+
+	ipToResolve := netutils.ReverseIP("192.168.0.1")
+
+	ipList, _ := n.(*network).ResolveName("service_test", types.IPv4)
+	if len(ipList) == 0 {
+		t.Fatal("There must be the VIP")
+	}
+	if len(ipList) != 1 {
+		t.Fatal("It must return only 1 VIP")
+	}
+	if ipList[0].String() != "192.168.0.1" {
+		t.Fatal("The service VIP is 192.168.0.1")
+	}
+	name := n.(*network).ResolveIP(ipToResolve)
+	if name == "" {
+		t.Fatal("It must return a name")
+	}
+	if name != "service_test.net1" {
+		t.Fatalf("It must return the service_test.net1 != %s", name)
+	}
+
+	// Delete service record for one of the services, the IP should remain because one service is still associated with it
+	n.(*network).deleteSvcRecords("ep1", "service_test", "serviceID1", net.ParseIP("192.168.0.1"), net.IP{}, true, "test")
+	ipList, _ = n.(*network).ResolveName("service_test", types.IPv4)
+	if len(ipList) == 0 {
+		t.Fatal("There must be the VIP")
+	}
+	if len(ipList) != 1 {
+		t.Fatal("It must return only 1 VIP")
+	}
+	if ipList[0].String() != "192.168.0.1" {
+		t.Fatal("The service VIP is 192.168.0.1")
+	}
+	name = n.(*network).ResolveIP(ipToResolve)
+	if name == "" {
+		t.Fatal("It must return a name")
+	}
+	if name != "service_test.net1" {
+		t.Fatalf("It must return the service_test.net1 != %s", name)
+	}
+
+	// Delete again the service using the previous service ID, nothing should happen
+	n.(*network).deleteSvcRecords("ep2", "service_test", "serviceID1", net.ParseIP("192.168.0.1"), net.IP{}, true, "test")
+	ipList, _ = n.(*network).ResolveName("service_test", types.IPv4)
+	if len(ipList) == 0 {
+		t.Fatal("There must be the VIP")
+	}
+	if len(ipList) != 1 {
+		t.Fatal("It must return only 1 VIP")
+	}
+	if ipList[0].String() != "192.168.0.1" {
+		t.Fatal("The service VIP is 192.168.0.1")
+	}
+	name = n.(*network).ResolveIP(ipToResolve)
+	if name == "" {
+		t.Fatal("It must return a name")
+	}
+	if name != "service_test.net1" {
+		t.Fatalf("It must return the service_test.net1 != %s", name)
+	}
+
+	// Delete now using the second service ID, now all the entries should be gone
+	n.(*network).deleteSvcRecords("ep2", "service_test", "serviceID2", net.ParseIP("192.168.0.1"), net.IP{}, true, "test")
+	ipList, _ = n.(*network).ResolveName("service_test", types.IPv4)
+	if len(ipList) != 0 {
+		t.Fatal("All the VIPs should be gone now")
+	}
+	name = n.(*network).ResolveIP(ipToResolve)
+	if name != "" {
+		t.Fatalf("It must return empty no more services associated, instead:%s", name)
+	}
+}
+
 func TestIpamReleaseOnNetDriverFailures(t *testing.T) {
 	if !testutils.IsRunningInContainer() {
 		defer testutils.SetupTestOSContext(t)()