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 <aboch@docker.com>

libnetwork/resolvconf/resolvconf.go

@@ -39,46 +39,69 @@ var lastModified struct {
 	contents []byte
 }
 
-// Get returns the contents of /etc/resolv.conf
+// File contains the resolv.conf content and its hash
-func Get() ([]byte, error) {
+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) (*File, error) {
-func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool) ([]byte, bool) {
-	changed := false
 	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) {
+	hash, err := ioutils.HashData(bytes.NewReader(cleanedResolvConf))
-		changed = true
+	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)
 }

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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed No Localhost: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed Localhost: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed Localhost+IPv6 off: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed Localhost+IPv6 on: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed no Localhost+IPv6 enabled: expected \n<%s> got \n<%s>", ns0, string(result))
+			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) {
+		if ns0 != string(result.Content) {
-			t.Fatalf("Failed no Localhost+IPv6 enabled: expected \n<%s> got \n<%s>", ns0, string(result))
+			t.Fatalf("Failed no Localhost+IPv6 enabled: expected \n<%s> got \n<%s>", ns0, string(result.Content))
 		}
 	}
 }

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)
+	// Write hash
-	if err != nil {
+	if err := ioutil.WriteFile(sb.config.resolvConfHashFile, []byte(newRC.Hash), filePerm); err != nil {
-		return err
+		return types.InternalErrorf("failed to write resolv.conf hash file when setting up dns for sandbox %s: %v", sb.ID(), 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)
 	}
 
 	return nil
 }
 
 func (sb *sandbox) updateDNS(ipv6Enabled bool) error {
-	var oldHash []byte
+	var (
-	hashFile := sb.config.resolvConfHashFile
+		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
 			}
-
+		} else {
-			oldHash = []byte{}
+			currHash = string(h)
 		}
 	}
 
-	curHash, err := ioutils.HashData(bytes.NewReader(resolvConf))
+	if currHash != "" && currHash != currRC.Hash {
-	if err != nil {
-		return err
-	}
-
-	if string(oldHash) != "" && curHash != string(oldHash) {
 		// 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)
+	newRC, err := resolvconf.FilterResolvDNS(currRC.Content, ipv6Enabled)
-
-	newHash, err := ioutils.HashData(bytes.NewReader(resolvConf))
 	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
 	}