From cdb82dc22dbeaba81b94da294c987c7bb561b728 Mon Sep 17 00:00:00 2001 From: Jana Radhakrishnan Date: Tue, 20 Oct 2015 22:50:23 -0700 Subject: [PATCH] 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 --- libnetwork/etchosts/etchosts.go | 58 +++++++++++++++++++++++---- libnetwork/etchosts/etchosts_test.go | 60 ++++++++++++++++++++++++++++ libnetwork/sandbox.go | 4 ++ 3 files changed, 114 insertions(+), 8 deletions(-) diff --git a/libnetwork/etchosts/etchosts.go b/libnetwork/etchosts/etchosts.go index 466143baee..92597b71b4 100644 --- a/libnetwork/etchosts/etchosts.go +++ b/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 diff --git a/libnetwork/etchosts/etchosts_test.go b/libnetwork/etchosts/etchosts_test.go index f4a2eedbe9..edea8f746d 100644 --- a/libnetwork/etchosts/etchosts_test.go +++ b/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) + } +} diff --git a/libnetwork/sandbox.go b/libnetwork/sandbox.go index 7b30ade614..ff0c66fc9c 100644 --- a/libnetwork/sandbox.go +++ b/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() }