From 9ced509e6d89d1ab4e0c4b49485be7931b505354 Mon Sep 17 00:00:00 2001 From: Dan Walsh Date: Tue, 7 Oct 2014 16:04:06 -0400 Subject: [PATCH 1/3] Check /etc/resolv.conf every time for 127.* content Currently if you start the docker -d on a system with 127.0.0.1 in /etc/resolv.conf It will set the default dns to 8.8.8.8 8.8.4.4 permanently. This causes a problem at boot on Fedora machines where NetworkManager has not populated /etc/resolv.conf before docker gets started. This fix checks /etc/resolv.conf on every docker run. And only populates daemon.config.Dns if the user specified it on the command line. Docker-DCO-1.1-Signed-off-by: Dan Walsh (github: rhatdan) --- daemon/container.go | 42 +++++++++++++++++++++++++++--------------- daemon/daemon.go | 19 ------------------- 2 files changed, 27 insertions(+), 34 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index 221edaad2c..d0fb77faeb 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -1,6 +1,7 @@ package daemon import ( + "bytes" "encoding/json" "errors" "fmt" @@ -918,22 +919,33 @@ func (container *Container) setupContainerDns() error { return err } - if config.NetworkMode != "host" && (len(config.Dns) > 0 || len(daemon.config.Dns) > 0 || len(config.DnsSearch) > 0 || len(daemon.config.DnsSearch) > 0) { - var ( - dns = resolvconf.GetNameservers(resolvConf) - dnsSearch = resolvconf.GetSearchDomains(resolvConf) - ) - if len(config.Dns) > 0 { - dns = config.Dns - } else if len(daemon.config.Dns) > 0 { - dns = daemon.config.Dns + if config.NetworkMode != "host" { + if len(config.Dns) > 0 || len(daemon.config.Dns) > 0 || len(config.DnsSearch) > 0 || len(daemon.config.DnsSearch) > 0 { + var ( + dns = resolvconf.GetNameservers(resolvConf) + dnsSearch = resolvconf.GetSearchDomains(resolvConf) + ) + if len(config.Dns) > 0 { + dns = config.Dns + } else if len(daemon.config.Dns) > 0 { + dns = daemon.config.Dns + } + if len(config.DnsSearch) > 0 { + dnsSearch = config.DnsSearch + } else if len(daemon.config.DnsSearch) > 0 { + dnsSearch = daemon.config.DnsSearch + } + return resolvconf.Build(container.ResolvConfPath, dns, dnsSearch) + } else { + resolvConf = utils.RemoveLocalDns(resolvConf) + if !bytes.Contains(resolvConf, []byte("nameserver")) { + for _, dns := range DefaultDns { + log.Infof("No non localhost DNS resolver found in resolv.conf and containers can't use it. Using default external servers : %v", DefaultDns) + resolvConf = append(append(resolvConf, []byte("\nnameserver ")...), dns...) + } + resolvConf = append(resolvConf, []byte("\n")...) + } } - if len(config.DnsSearch) > 0 { - dnsSearch = config.DnsSearch - } else if len(daemon.config.DnsSearch) > 0 { - dnsSearch = daemon.config.DnsSearch - } - return resolvconf.Build(container.ResolvConfPath, dns, dnsSearch) } return ioutil.WriteFile(container.ResolvConfPath, resolvConf, 0644) } diff --git a/daemon/daemon.go b/daemon/daemon.go index 1757b4fea1..8d8fd3d309 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1,7 +1,6 @@ package daemon import ( - "bytes" "fmt" "io" "io/ioutil" @@ -32,7 +31,6 @@ import ( "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/log" "github.com/docker/docker/pkg/namesgenerator" - "github.com/docker/docker/pkg/networkfs/resolvconf" "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/parsers/kernel" "github.com/docker/docker/pkg/sysinfo" @@ -925,9 +923,6 @@ func NewDaemonFromDirectory(config *Config, eng *engine.Engine) (*Daemon, error) eng: eng, trustStore: t, } - if err := daemon.checkLocaldns(); err != nil { - return nil, err - } if err := daemon.restore(); err != nil { return nil, err } @@ -1087,20 +1082,6 @@ func (daemon *Daemon) ContainerGraph() *graphdb.Database { return daemon.containerGraph } -func (daemon *Daemon) checkLocaldns() error { - resolvConf, err := resolvconf.Get() - if err != nil { - return err - } - resolvConf = utils.RemoveLocalDns(resolvConf) - - if len(daemon.config.Dns) == 0 && !bytes.Contains(resolvConf, []byte("nameserver")) { - log.Infof("No non localhost DNS resolver found in resolv.conf and containers can't use it. Using default external servers : %v", DefaultDns) - daemon.config.Dns = DefaultDns - } - return nil -} - func (daemon *Daemon) ImageGetCached(imgID string, config *runconfig.Config) (*image.Image, error) { // Retrieve all images images, err := daemon.Graph().Map() From acd511786e5d9d2aa440f4265ed8d4dda149d1cc Mon Sep 17 00:00:00 2001 From: Jessica Frazelle Date: Tue, 7 Oct 2014 15:35:32 -0700 Subject: [PATCH 2/3] Test for check /etc/resolv.conf on every docker run for 127.* content. Docker-DCO-1.1-Signed-off-by: Jessica Frazelle (github: jfrazelle) --- integration-cli/docker_cli_run_test.go | 84 +++++++++++++++++++------- 1 file changed, 62 insertions(+), 22 deletions(-) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 2fd545af8d..c9475d46f1 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -751,7 +751,7 @@ func TestRunEnvironment(t *testing.T) { } sort.Strings(goodEnv) if len(goodEnv) != len(actualEnv) { - t.Fatalf("Wrong environment: should be %d variables, not: '%s'\n", len(goodEnv), strings.Join(actualEnv, ", ")) + t.Fatalf("Wrong environment: should be %d variables, not: %q\n", len(goodEnv), strings.Join(actualEnv, ", ")) } for i := range goodEnv { if actualEnv[i] != goodEnv[i] { @@ -1168,7 +1168,7 @@ func TestRunModeHostname(t *testing.T) { } if actual := strings.Trim(out, "\r\n"); actual != "testhostname" { - t.Fatalf("expected 'testhostname', but says: '%s'", actual) + t.Fatalf("expected 'testhostname', but says: %q", actual) } cmd = exec.Command(dockerBinary, "run", "--net=host", "busybox", "cat", "/etc/hostname") @@ -1182,7 +1182,7 @@ func TestRunModeHostname(t *testing.T) { t.Fatal(err) } if actual := strings.Trim(out, "\r\n"); actual != hostname { - t.Fatalf("expected '%s', but says: '%s'", hostname, actual) + t.Fatalf("expected %q, but says: '%s'", hostname, actual) } deleteAllContainers() @@ -1196,7 +1196,7 @@ func TestRunRootWorkdir(t *testing.T) { t.Fatal(s, err) } if s != "/\n" { - t.Fatalf("pwd returned '%s' (expected /\\n)", s) + t.Fatalf("pwd returned %q (expected /\\n)", s) } deleteAllContainers() @@ -1297,7 +1297,7 @@ func TestRunDnsOptions(t *testing.T) { actual := strings.Replace(strings.Trim(out, "\r\n"), "\n", " ", -1) if actual != "nameserver 127.0.0.1 search mydomain" { - t.Fatalf("expected 'nameserver 127.0.0.1 search mydomain', but says: '%s'", actual) + t.Fatalf("expected 'nameserver 127.0.0.1 search mydomain', but says: %q", actual) } cmd = exec.Command(dockerBinary, "run", "--dns=127.0.0.1", "--dns-search=.", "busybox", "cat", "/etc/resolv.conf") @@ -1309,61 +1309,101 @@ func TestRunDnsOptions(t *testing.T) { actual = strings.Replace(strings.Trim(strings.Trim(out, "\r\n"), " "), "\n", " ", -1) if actual != "nameserver 127.0.0.1" { - t.Fatalf("expected 'nameserver 127.0.0.1', but says: '%s'", actual) + t.Fatalf("expected 'nameserver 127.0.0.1', but says: %q", actual) } logDone("run - dns options") } func TestRunDnsOptionsBasedOnHostResolvConf(t *testing.T) { - resolvConf, err := ioutil.ReadFile("/etc/resolv.conf") + var out string + + origResolvConf, err := ioutil.ReadFile("/etc/resolv.conf") if os.IsNotExist(err) { t.Fatalf("/etc/resolv.conf does not exist") } - hostNamservers := resolvconf.GetNameservers(resolvConf) - hostSearch := resolvconf.GetSearchDomains(resolvConf) + hostNamservers := resolvconf.GetNameservers(origResolvConf) + hostSearch := resolvconf.GetSearchDomains(origResolvConf) cmd := exec.Command(dockerBinary, "run", "--dns=127.0.0.1", "busybox", "cat", "/etc/resolv.conf") - out, _, err := runCommandWithOutput(cmd) - if err != nil { + if out, _, err = runCommandWithOutput(cmd); err != nil { t.Fatal(err, out) } if actualNameservers := resolvconf.GetNameservers([]byte(out)); string(actualNameservers[0]) != "127.0.0.1" { - t.Fatalf("expected '127.0.0.1', but says: '%s'", string(actualNameservers[0])) + t.Fatalf("expected '127.0.0.1', but says: %q", string(actualNameservers[0])) } actualSearch := resolvconf.GetSearchDomains([]byte(out)) if len(actualSearch) != len(hostSearch) { - t.Fatalf("expected '%s' search domain(s), but it has: '%s'", len(hostSearch), len(actualSearch)) + t.Fatalf("expected %q search domain(s), but it has: '%s'", len(hostSearch), len(actualSearch)) } for i := range actualSearch { if actualSearch[i] != hostSearch[i] { - t.Fatalf("expected '%s' domain, but says: '%s'", actualSearch[i], hostSearch[i]) + t.Fatalf("expected %q domain, but says: '%s'", actualSearch[i], hostSearch[i]) } } cmd = exec.Command(dockerBinary, "run", "--dns-search=mydomain", "busybox", "cat", "/etc/resolv.conf") - out, _, err = runCommandWithOutput(cmd) - if err != nil { + if out, _, err = runCommandWithOutput(cmd); err != nil { t.Fatal(err, out) } actualNameservers := resolvconf.GetNameservers([]byte(out)) if len(actualNameservers) != len(hostNamservers) { - t.Fatalf("expected '%s' nameserver(s), but it has: '%s'", len(hostNamservers), len(actualNameservers)) + t.Fatalf("expected %q nameserver(s), but it has: '%s'", len(hostNamservers), len(actualNameservers)) } for i := range actualNameservers { if actualNameservers[i] != hostNamservers[i] { - t.Fatalf("expected '%s' nameserver, but says: '%s'", actualNameservers[i], hostNamservers[i]) + t.Fatalf("expected %q nameserver, but says: '%s'", actualNameservers[i], hostNamservers[i]) } } if actualSearch = resolvconf.GetSearchDomains([]byte(out)); string(actualSearch[0]) != "mydomain" { - t.Fatalf("expected 'mydomain', but says: '%s'", string(actualSearch[0])) + t.Fatalf("expected 'mydomain', but says: %q", string(actualSearch[0])) + } + + // test with file + tmpResolvConf := []byte("search example.com\nnameserver 12.34.56.78\nnameserver 127.0.0.1") + if err := ioutil.WriteFile("/etc/resolv.conf", tmpResolvConf, 0644); err != nil { + t.Fatal(err) + } + // put the old resolvconf back + defer func() { + if err := ioutil.WriteFile("/etc/resolv.conf", origResolvConf, 0644); err != nil { + t.Fatal(err) + } + }() + + resolvConf, err := ioutil.ReadFile("/etc/resolv.conf") + if os.IsNotExist(err) { + t.Fatalf("/etc/resolv.conf does not exist") + } + + hostNamservers = resolvconf.GetNameservers(resolvConf) + hostSearch = resolvconf.GetSearchDomains(resolvConf) + + cmd = exec.Command(dockerBinary, "run", "busybox", "cat", "/etc/resolv.conf") + + if out, _, err = runCommandWithOutput(cmd); err != nil { + t.Fatal(err, out) + } + + if actualNameservers = resolvconf.GetNameservers([]byte(out)); string(actualNameservers[0]) != "12.34.56.78" || len(actualNameservers) != 1 { + t.Fatalf("expected '12.34.56.78', but has: %v", actualNameservers) + } + + actualSearch = resolvconf.GetSearchDomains([]byte(out)) + if len(actualSearch) != len(hostSearch) { + t.Fatalf("expected %q search domain(s), but it has: %q", len(hostSearch), len(actualSearch)) + } + for i := range actualSearch { + if actualSearch[i] != hostSearch[i] { + t.Fatalf("expected %q domain, but says: '%s'", actualSearch[i], hostSearch[i]) + } } deleteAllContainers() @@ -1382,7 +1422,7 @@ func TestRunAddHost(t *testing.T) { actual := strings.Trim(out, "\r\n") if actual != "86.75.30.9\textra" { - t.Fatalf("expected '86.75.30.9\textra', but says: '%s'", actual) + t.Fatalf("expected '86.75.30.9\textra', but says: %q", actual) } logDone("run - add-host option") @@ -1989,7 +2029,7 @@ func TestRunCidFileCleanupIfEmpty(t *testing.T) { } if _, err := os.Stat(tmpCidFile); err == nil { - t.Fatalf("empty CIDFile '%s' should've been deleted", tmpCidFile) + t.Fatalf("empty CIDFile %q should've been deleted", tmpCidFile) } deleteAllContainers() logDone("run - cleanup empty cidfile on fail") @@ -2017,7 +2057,7 @@ func TestRunCidFileCheckIDLength(t *testing.T) { } cid := string(buffer) if len(cid) != 64 { - t.Fatalf("--cidfile should be a long id, not '%s'", id) + t.Fatalf("--cidfile should be a long id, not %q", id) } if cid != id { t.Fatalf("cid must be equal to %s, got %s", id, cid) From dbe6c6651e744bd6f6cab04fcaed2871779d36a0 Mon Sep 17 00:00:00 2001 From: Jessica Frazelle Date: Tue, 7 Oct 2014 17:58:39 -0700 Subject: [PATCH 3/3] cleanup resolve.conf code Docker-DCO-1.1-Signed-off-by: Jessica Frazelle (github: jfrazelle) --- daemon/container.go | 19 +++++++------ integration-cli/docker_cli_run_test.go | 39 +++++++++++++++++++------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index d0fb77faeb..de255ca886 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -920,6 +920,7 @@ func (container *Container) setupContainerDns() error { } if config.NetworkMode != "host" { + // check configurations for any container/daemon dns settings if len(config.Dns) > 0 || len(daemon.config.Dns) > 0 || len(config.DnsSearch) > 0 || len(daemon.config.DnsSearch) > 0 { var ( dns = resolvconf.GetNameservers(resolvConf) @@ -936,15 +937,15 @@ func (container *Container) setupContainerDns() error { dnsSearch = daemon.config.DnsSearch } return resolvconf.Build(container.ResolvConfPath, dns, dnsSearch) - } else { - resolvConf = utils.RemoveLocalDns(resolvConf) - if !bytes.Contains(resolvConf, []byte("nameserver")) { - for _, dns := range DefaultDns { - log.Infof("No non localhost DNS resolver found in resolv.conf and containers can't use it. Using default external servers : %v", DefaultDns) - resolvConf = append(append(resolvConf, []byte("\nnameserver ")...), dns...) - } - resolvConf = append(resolvConf, []byte("\n")...) - } + } + + // replace any localhost/127.* nameservers + resolvConf = utils.RemoveLocalDns(resolvConf) + // if the resulting resolvConf is empty, use DefaultDns + if !bytes.Contains(resolvConf, []byte("nameserver")) { + log.Infof("No non localhost DNS resolver found in resolv.conf and containers can't use it. Using default external servers : %v", DefaultDns) + // prefix the default dns options with nameserver + resolvConf = append(resolvConf, []byte("\nnameserver "+strings.Join(DefaultDns, "\nnameserver "))...) } } return ioutil.WriteFile(container.ResolvConfPath, resolvConf, 0644) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index c9475d46f1..dd2ec5cec4 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -1266,20 +1266,39 @@ func TestRunWithVolumesIsRecursive(t *testing.T) { } func TestRunDnsDefaultOptions(t *testing.T) { - cmd := exec.Command(dockerBinary, "run", "busybox", "cat", "/etc/resolv.conf") - - actual, _, err := runCommandWithOutput(cmd) - if err != nil { - t.Fatal(err, actual) - } - - resolvConf, err := ioutil.ReadFile("/etc/resolv.conf") + // ci server has default resolv.conf + // so rewrite it for the test + origResolvConf, err := ioutil.ReadFile("/etc/resolv.conf") if os.IsNotExist(err) { t.Fatalf("/etc/resolv.conf does not exist") } - if actual != string(resolvConf) { - t.Fatalf("expected resolv.conf is not the same of actual") + // test with file + tmpResolvConf := []byte("nameserver 127.0.0.1") + if err := ioutil.WriteFile("/etc/resolv.conf", tmpResolvConf, 0644); err != nil { + t.Fatal(err) + } + // put the old resolvconf back + defer func() { + if err := ioutil.WriteFile("/etc/resolv.conf", origResolvConf, 0644); err != nil { + t.Fatal(err) + } + }() + + cmd := exec.Command(dockerBinary, "run", "busybox", "cat", "/etc/resolv.conf") + + actual, _, err := runCommandWithOutput(cmd) + if err != nil { + t.Error(err, actual) + return + } + + // check that the actual defaults are there + // if we ever change the defaults from google dns, this will break + expected := "\nnameserver 8.8.8.8\nnameserver 8.8.4.4" + if actual != expected { + t.Errorf("expected resolv.conf be: %q, but was: %q", expected, actual) + return } deleteAllContainers()