From ef659c904966d5a1419b795fe32d940940f71333 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Mon, 31 Aug 2015 15:17:24 -0700 Subject: [PATCH] Fix resolv.conf and hosts handling in sandbox Two issues: - container resolv.conf getting regenerated even when no dns configs are passed - updateHosts should be skipped for host networking mode - incorrect check on dnsOptions Signed-off-by: Alessandro Boch --- libnetwork/resolvconf/resolvconf.go | 66 +++++++++++++------- libnetwork/resolvconf/resolvconf_test.go | 50 ++++++++------- libnetwork/sandbox.go | 78 ++++++++++++++---------- 3 files changed, 119 insertions(+), 75 deletions(-) diff --git a/libnetwork/resolvconf/resolvconf.go b/libnetwork/resolvconf/resolvconf.go index aa710ebfbd..d5169ada35 100644 --- a/libnetwork/resolvconf/resolvconf.go +++ b/libnetwork/resolvconf/resolvconf.go @@ -39,46 +39,69 @@ var lastModified struct { contents []byte } -// Get returns the contents of /etc/resolv.conf -func Get() ([]byte, error) { +// File contains the resolv.conf content and its hash +type File struct { + Content []byte + Hash string +} + +// Get returns the contents of /etc/resolv.conf and its hash +func Get() (*File, error) { resolv, err := ioutil.ReadFile("/etc/resolv.conf") if err != nil { return nil, err } - return resolv, nil + hash, err := ioutils.HashData(bytes.NewReader(resolv)) + if err != nil { + return nil, err + } + return &File{Content: resolv, Hash: hash}, nil +} + +// GetSpecific returns the contents of the user specified resolv.conf file and its hash +func GetSpecific(path string) (*File, error) { + resolv, err := ioutil.ReadFile(path) + if err != nil { + return nil, err + } + hash, err := ioutils.HashData(bytes.NewReader(resolv)) + if err != nil { + return nil, err + } + return &File{Content: resolv, Hash: hash}, nil } // GetIfChanged retrieves the host /etc/resolv.conf file, checks against the last hash // and, if modified since last check, returns the bytes and new hash. // This feature is used by the resolv.conf updater for containers -func GetIfChanged() ([]byte, string, error) { +func GetIfChanged() (*File, error) { lastModified.Lock() defer lastModified.Unlock() resolv, err := ioutil.ReadFile("/etc/resolv.conf") if err != nil { - return nil, "", err + return nil, err } newHash, err := ioutils.HashData(bytes.NewReader(resolv)) if err != nil { - return nil, "", err + return nil, err } if lastModified.sha256 != newHash { lastModified.sha256 = newHash lastModified.contents = resolv - return resolv, newHash, nil + return &File{Content: resolv, Hash: newHash}, nil } // nothing changed, so return no data - return nil, "", nil + return nil, nil } // GetLastModified retrieves the last used contents and hash of the host resolv.conf. // Used by containers updating on restart -func GetLastModified() ([]byte, string) { +func GetLastModified() *File { lastModified.Lock() defer lastModified.Unlock() - return lastModified.contents, lastModified.sha256 + return &File{Content: lastModified.contents, Hash: lastModified.sha256} } // FilterResolvDNS cleans up the config in resolvConf. It has two main jobs: @@ -88,9 +111,7 @@ func GetLastModified() ([]byte, string) { // 2. Given the caller provides the enable/disable state of IPv6, the filter // code will remove all IPv6 nameservers if it is not enabled for containers // -// It returns a boolean to notify the caller if changes were made at all -func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool) ([]byte, bool) { - changed := false +func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool) (*File, error) { cleanedResolvConf := localhostNSRegexp.ReplaceAll(resolvConf, []byte{}) // if IPv6 is not enabled, also clean out any IPv6 address nameserver if !ipv6Enabled { @@ -107,10 +128,11 @@ func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool) ([]byte, bool) { } cleanedResolvConf = append(cleanedResolvConf, []byte("\n"+strings.Join(dns, "\n"))...) } - if !bytes.Equal(resolvConf, cleanedResolvConf) { - changed = true + hash, err := ioutils.HashData(bytes.NewReader(cleanedResolvConf)) + if err != nil { + return nil, err } - return cleanedResolvConf, changed + return &File{Content: cleanedResolvConf, Hash: hash}, nil } // getLines parses input into lines and strips away comments. @@ -184,32 +206,32 @@ func GetOptions(resolvConf []byte) []string { // Build writes a configuration file to path containing a "nameserver" entry // for every element in dns, a "search" entry for every element in // dnsSearch, and an "options" entry for every element in dnsOptions. -func Build(path string, dns, dnsSearch, dnsOptions []string) (string, error) { +func Build(path string, dns, dnsSearch, dnsOptions []string) (*File, error) { content := bytes.NewBuffer(nil) if len(dnsSearch) > 0 { if searchString := strings.Join(dnsSearch, " "); strings.Trim(searchString, " ") != "." { if _, err := content.WriteString("search " + searchString + "\n"); err != nil { - return "", err + return nil, err } } } for _, dns := range dns { if _, err := content.WriteString("nameserver " + dns + "\n"); err != nil { - return "", err + return nil, err } } if len(dnsOptions) > 0 { if optsString := strings.Join(dnsOptions, " "); strings.Trim(optsString, " ") != "" { if _, err := content.WriteString("options " + optsString + "\n"); err != nil { - return "", err + return nil, err } } } hash, err := ioutils.HashData(bytes.NewReader(content.Bytes())) if err != nil { - return "", err + return nil, err } - return hash, ioutil.WriteFile(path, content.Bytes(), 0644) + return &File{Content: content.Bytes(), Hash: hash}, ioutil.WriteFile(path, content.Bytes(), 0644) } diff --git a/libnetwork/resolvconf/resolvconf_test.go b/libnetwork/resolvconf/resolvconf_test.go index 74f30e4889..9776acc5dd 100644 --- a/libnetwork/resolvconf/resolvconf_test.go +++ b/libnetwork/resolvconf/resolvconf_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/docker/docker/pkg/ioutils" _ "github.com/docker/libnetwork/netutils" ) @@ -18,9 +19,16 @@ func TestGet(t *testing.T) { if err != nil { t.Fatal(err) } - if string(resolvConfUtils) != string(resolvConfSystem) { + if string(resolvConfUtils.Content) != string(resolvConfSystem) { t.Fatalf("/etc/resolv.conf and GetResolvConf have different content.") } + hashSystem, err := ioutils.HashData(bytes.NewReader(resolvConfSystem)) + if err != nil { + t.Fatal(err) + } + if resolvConfUtils.Hash != hashSystem { + t.Fatalf("/etc/resolv.conf and GetResolvConf have different hashes.") + } } func TestGetNameservers(t *testing.T) { @@ -214,51 +222,51 @@ func TestFilterResolvDns(t *testing.T) { ns0 := "nameserver 10.16.60.14\nnameserver 10.16.60.21\n" if result, _ := FilterResolvDNS([]byte(ns0), false); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed No Localhost: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed No Localhost: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } ns1 := "nameserver 10.16.60.14\nnameserver 10.16.60.21\nnameserver 127.0.0.1\n" if result, _ := FilterResolvDNS([]byte(ns1), false); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } ns1 = "nameserver 10.16.60.14\nnameserver 127.0.0.1\nnameserver 10.16.60.21\n" if result, _ := FilterResolvDNS([]byte(ns1), false); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } ns1 = "nameserver 127.0.1.1\nnameserver 10.16.60.14\nnameserver 10.16.60.21\n" if result, _ := FilterResolvDNS([]byte(ns1), false); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } ns1 = "nameserver ::1\nnameserver 10.16.60.14\nnameserver 127.0.2.1\nnameserver 10.16.60.21\n" if result, _ := FilterResolvDNS([]byte(ns1), false); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } ns1 = "nameserver 10.16.60.14\nnameserver ::1\nnameserver 10.16.60.21\nnameserver ::1" if result, _ := FilterResolvDNS([]byte(ns1), false); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } // with IPv6 disabled (false param), the IPv6 nameserver should be removed ns1 = "nameserver 10.16.60.14\nnameserver 2002:dead:beef::1\nnameserver 10.16.60.21\nnameserver ::1" if result, _ := FilterResolvDNS([]byte(ns1), false); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed Localhost+IPv6 off: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed Localhost+IPv6 off: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } @@ -266,8 +274,8 @@ func TestFilterResolvDns(t *testing.T) { ns0 = "nameserver 10.16.60.14\nnameserver 2002:dead:beef::1\nnameserver 10.16.60.21\n" ns1 = "nameserver 10.16.60.14\nnameserver 2002:dead:beef::1\nnameserver 10.16.60.21\nnameserver ::1" if result, _ := FilterResolvDNS([]byte(ns1), true); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed Localhost+IPv6 on: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed Localhost+IPv6 on: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } @@ -275,8 +283,8 @@ func TestFilterResolvDns(t *testing.T) { ns0 = "\nnameserver 8.8.8.8\nnameserver 8.8.4.4\nnameserver 2001:4860:4860::8888\nnameserver 2001:4860:4860::8844" ns1 = "nameserver 127.0.0.1\nnameserver ::1\nnameserver 127.0.2.1" if result, _ := FilterResolvDNS([]byte(ns1), true); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed no Localhost+IPv6 enabled: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed no Localhost+IPv6 enabled: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } @@ -284,8 +292,8 @@ func TestFilterResolvDns(t *testing.T) { ns0 = "\nnameserver 8.8.8.8\nnameserver 8.8.4.4" ns1 = "nameserver 127.0.0.1\nnameserver ::1\nnameserver 127.0.2.1" if result, _ := FilterResolvDNS([]byte(ns1), false); result != nil { - if ns0 != string(result) { - t.Fatalf("Failed no Localhost+IPv6 enabled: expected \n<%s> got \n<%s>", ns0, string(result)) + if ns0 != string(result.Content) { + t.Fatalf("Failed no Localhost+IPv6 enabled: expected \n<%s> got \n<%s>", ns0, string(result.Content)) } } } diff --git a/libnetwork/sandbox.go b/libnetwork/sandbox.go index 69c31989a3..0ba08a0558 100644 --- a/libnetwork/sandbox.go +++ b/libnetwork/sandbox.go @@ -1,7 +1,6 @@ package libnetwork import ( - "bytes" "container/heap" "encoding/json" "fmt" @@ -12,7 +11,6 @@ import ( "sync" log "github.com/Sirupsen/logrus" - "github.com/docker/docker/pkg/ioutils" "github.com/docker/libnetwork/etchosts" "github.com/docker/libnetwork/osl" "github.com/docker/libnetwork/resolvconf" @@ -339,6 +337,10 @@ func (sb *sandbox) buildHostsFile() error { } func (sb *sandbox) updateHostsFile(ifaceIP string, svcRecords []etchosts.Record) error { + if sb.config.originHostsPath != "" { + return nil + } + // Rebuild the hosts file accounting for the passed interface IP and service records extraContent := make([]etchosts.Record, 0, len(sb.config.extraHosts)+len(svcRecords)) @@ -382,6 +384,8 @@ func (sb *sandbox) updateParentHosts() error { } func (sb *sandbox) setupDNS() error { + var newRC *resolvconf.File + if sb.config.resolvConfPath == "" { sb.config.resolvConfPath = defaultPrefix + "/" + sb.id + "/resolv.conf" } @@ -401,15 +405,18 @@ func (sb *sandbox) setupDNS() error { return nil } - resolvConf, err := resolvconf.Get() + currRC, err := resolvconf.Get() if err != nil { return err } - dnsList := resolvconf.GetNameservers(resolvConf) - dnsSearchList := resolvconf.GetSearchDomains(resolvConf) - dnsOptionsList := resolvconf.GetOptions(resolvConf) - if len(sb.config.dnsList) > 0 || len(sb.config.dnsSearchList) > 0 || len(dnsOptionsList) > 0 { + if len(sb.config.dnsList) > 0 || len(sb.config.dnsSearchList) > 0 || len(sb.config.dnsOptionsList) > 0 { + var ( + err error + dnsList = resolvconf.GetNameservers(currRC.Content) + dnsSearchList = resolvconf.GetSearchDomains(currRC.Content) + dnsOptionsList = resolvconf.GetOptions(currRC.Content) + ) if len(sb.config.dnsList) > 0 { dnsList = sb.config.dnsList } @@ -419,47 +426,56 @@ func (sb *sandbox) setupDNS() error { if len(sb.config.dnsOptionsList) > 0 { dnsOptionsList = sb.config.dnsOptionsList } + newRC, err = resolvconf.Build(sb.config.resolvConfPath, dnsList, dnsSearchList, dnsOptionsList) + if err != nil { + return err + } + } else { + // Replace any localhost/127.* (at this point we have no info about ipv6, pass it as true) + if newRC, err = resolvconf.FilterResolvDNS(currRC.Content, true); err != nil { + return err + } + // No contention on container resolv.conf file at sandbox creation + if err := ioutil.WriteFile(sb.config.resolvConfPath, newRC.Content, filePerm); err != nil { + return types.InternalErrorf("failed to write unhaltered resolv.conf file content when setting up dns for sandbox %s: %v", sb.ID(), err) + } } - hash, err := resolvconf.Build(sb.config.resolvConfPath, dnsList, dnsSearchList, dnsOptionsList) - if err != nil { - return err - } - - // write hash - if err := ioutil.WriteFile(sb.config.resolvConfHashFile, []byte(hash), filePerm); err != nil { - return types.InternalErrorf("failed to write resol.conf hash file when setting up dns for sandbox %s: %v", sb.ID(), err) + // Write hash + if err := ioutil.WriteFile(sb.config.resolvConfHashFile, []byte(newRC.Hash), filePerm); err != nil { + return types.InternalErrorf("failed to write resolv.conf hash file when setting up dns for sandbox %s: %v", sb.ID(), err) } return nil } func (sb *sandbox) updateDNS(ipv6Enabled bool) error { - var oldHash []byte - hashFile := sb.config.resolvConfHashFile + var ( + currHash string + hashFile = sb.config.resolvConfHashFile + ) - resolvConf, err := ioutil.ReadFile(sb.config.resolvConfPath) + if len(sb.config.dnsList) > 0 || len(sb.config.dnsSearchList) > 0 || len(sb.config.dnsOptionsList) > 0 { + return nil + } + + currRC, err := resolvconf.GetSpecific(sb.config.resolvConfPath) if err != nil { if !os.IsNotExist(err) { return err } } else { - oldHash, err = ioutil.ReadFile(hashFile) + h, err := ioutil.ReadFile(hashFile) if err != nil { if !os.IsNotExist(err) { return err } - - oldHash = []byte{} + } else { + currHash = string(h) } } - curHash, err := ioutils.HashData(bytes.NewReader(resolvConf)) - if err != nil { - return err - } - - if string(oldHash) != "" && curHash != string(oldHash) { + if currHash != "" && currHash != currRC.Hash { // Seems the user has changed the container resolv.conf since the last time // we checked so return without doing anything. log.Infof("Skipping update of resolv.conf file with ipv6Enabled: %t because file was touched by user", ipv6Enabled) @@ -467,9 +483,7 @@ func (sb *sandbox) updateDNS(ipv6Enabled bool) error { } // replace any localhost/127.* and remove IPv6 nameservers if IPv6 disabled. - resolvConf, _ = resolvconf.FilterResolvDNS(resolvConf, ipv6Enabled) - - newHash, err := ioutils.HashData(bytes.NewReader(resolvConf)) + newRC, err := resolvconf.FilterResolvDNS(currRC.Content, ipv6Enabled) if err != nil { return err } @@ -491,10 +505,10 @@ func (sb *sandbox) updateDNS(ipv6Enabled bool) error { } // write the updates to the temp files - if err = ioutil.WriteFile(tmpHashFile.Name(), []byte(newHash), filePerm); err != nil { + if err = ioutil.WriteFile(tmpHashFile.Name(), []byte(newRC.Hash), filePerm); err != nil { return err } - if err = ioutil.WriteFile(tmpResolvFile.Name(), resolvConf, filePerm); err != nil { + if err = ioutil.WriteFile(tmpResolvFile.Name(), newRC.Content, filePerm); err != nil { return err }