Synchronize /etc/hosts updates at file level

Introduced a path level lock to synchronize updates
to /etc/hosts writes. A path level cache is maintained
to only synchronize only at the file level.

Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>

libnetwork/etchosts/etchosts.go

@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"os"
 	"regexp"
+	"sync"
 )
 
 // Record Structure for a single host record
@@ -21,14 +22,47 @@ func (r Record) WriteTo(w io.Writer) (int64, error) {
 	return int64(n), err
 }
 
-// Default hosts config records slice
-var defaultContent = []Record{
-	{Hosts: "localhost", IP: "127.0.0.1"},
-	{Hosts: "localhost ip6-localhost ip6-loopback", IP: "::1"},
-	{Hosts: "ip6-localnet", IP: "fe00::0"},
-	{Hosts: "ip6-mcastprefix", IP: "ff00::0"},
-	{Hosts: "ip6-allnodes", IP: "ff02::1"},
-	{Hosts: "ip6-allrouters", IP: "ff02::2"},
+var (
+	// Default hosts config records slice
+	defaultContent = []Record{
+		{Hosts: "localhost", IP: "127.0.0.1"},
+		{Hosts: "localhost ip6-localhost ip6-loopback", IP: "::1"},
+		{Hosts: "ip6-localnet", IP: "fe00::0"},
+		{Hosts: "ip6-mcastprefix", IP: "ff00::0"},
+		{Hosts: "ip6-allnodes", IP: "ff02::1"},
+		{Hosts: "ip6-allrouters", IP: "ff02::2"},
+	}
+
+	// A cache of path level locks for synchronizing /etc/hosts
+	// updates on a file level
+	pathMap = make(map[string]*sync.Mutex)
+
+	// A package level mutex to synchronize the cache itself
+	pathMutex sync.Mutex
+)
+
+func pathLock(path string) func() {
+	pathMutex.Lock()
+	defer pathMutex.Unlock()
+
+	pl, ok := pathMap[path]
+	if !ok {
+		pl = &sync.Mutex{}
+		pathMap[path] = pl
+	}
+
+	pl.Lock()
+	return func() {
+		pl.Unlock()
+	}
+}
+
+// Drop drops the path string from the path cache
+func Drop(path string) {
+	pathMutex.Lock()
+	defer pathMutex.Unlock()
+
+	delete(pathMap, path)
 }
 
 // Build function
@@ -36,6 +70,8 @@ var defaultContent = []Record{
 // IP, hostname, and domainname set main record leave empty for no master record
 // extraContent is an array of extra host records.
 func Build(path, IP, hostname, domainname string, extraContent []Record) error {
+	defer pathLock(path)()
+
 	content := bytes.NewBuffer(nil)
 	if IP != "" {
 		//set main record
@@ -68,6 +104,8 @@ func Build(path, IP, hostname, domainname string, extraContent []Record) error {
 
 // Add adds an arbitrary number of Records to an already existing /etc/hosts file
 func Add(path string, recs []Record) error {
+	defer pathLock(path)()
+
 	if len(recs) == 0 {
 		return nil
 	}
@@ -95,6 +133,8 @@ func Add(path string, recs []Record) error {
 
 // Delete deletes an arbitrary number of Records already existing in /etc/hosts file
 func Delete(path string, recs []Record) error {
+	defer pathLock(path)()
+
 	if len(recs) == 0 {
 		return nil
 	}
@@ -118,6 +158,8 @@ func Delete(path string, recs []Record) error {
 // IP is new IP address
 // hostname is hostname to search for to replace IP
 func Update(path, IP, hostname string) error {
+	defer pathLock(path)()
+
 	old, err := ioutil.ReadFile(path)
 	if err != nil {
 		return err

libnetwork/etchosts/etchosts_test.go

@@ -2,8 +2,10 @@ package etchosts
 
 import (
 	"bytes"
+	"fmt"
 	"io/ioutil"
 	"os"
+	"sync"
 	"testing"
 
 	_ "github.com/docker/libnetwork/testutils"
@@ -247,3 +249,61 @@ func TestDelete(t *testing.T) {
 		t.Fatalf("Did not expect to find '%s' got '%s'", expected, content)
 	}
 }
+
+func TestConcurrentWrites(t *testing.T) {
+	file, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(file.Name())
+
+	err = Build(file.Name(), "", "", "", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err := Add(file.Name(), []Record{
+		Record{
+			Hosts: "inithostname",
+			IP:    "172.17.0.1",
+		},
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	var wg sync.WaitGroup
+	for i := 0; i < 10; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			rec := []Record{
+				Record{
+					IP:    fmt.Sprintf("%d.%d.%d.%d", i, i, i, i),
+					Hosts: fmt.Sprintf("testhostname%d", i),
+				},
+			}
+
+			for j := 0; j < 25; j++ {
+				if err := Add(file.Name(), rec); err != nil {
+					t.Fatal(err)
+				}
+
+				if err := Delete(file.Name(), rec); err != nil {
+					t.Fatal(err)
+				}
+			}
+		}()
+	}
+
+	wg.Wait()
+
+	content, err := ioutil.ReadFile(file.Name())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if expected := "172.17.0.1\tinithostname\n"; !bytes.Contains(content, []byte(expected)) {
+		t.Fatalf("Expected to find '%s' got '%s'", expected, content)
+	}
+}

libnetwork/sandbox.go

@@ -182,6 +182,10 @@ func (sb *sandbox) Delete() error {
 		}
 	}
 
+	// Container is going away. Path cache in etchosts is most
+	// likely not required any more. Drop it.
+	etchosts.Drop(sb.config.hostsPath)
+
 	if sb.osSbox != nil {
 		sb.osSbox.Destroy()
 	}