Fix containerd resolv.conf + DHCP behavior

pkg/operations/start.go

@@ -102,7 +102,7 @@ func StartVM(vm *api.VM, debug bool) error {
 	}
 
 	// Run the VM container in Docker
-	containerID, err := providers.Runtime.RunContainer(igniteImage, config, util.NewPrefixer().Prefix(vm.GetUID()))
+	containerID, err := providers.Runtime.RunContainer(igniteImage, config, util.NewPrefixer().Prefix(vm.GetUID()), vm.GetUID().String())
 	if err != nil {
 		return fmt.Errorf("failed to start container for VM %q: %v", vm.GetUID(), err)
 	}

pkg/resolvconf/resolvconf.go

@@ -0,0 +1,109 @@
+package resolvconf
+
+import (
+	"fmt"
+	"net"
+	"os"
+	"strings"
+
+	"github.com/weaveworks/ignite/pkg/util"
+
+	"github.com/miekg/dns"
+	log "github.com/sirupsen/logrus"
+)
+
+const (
+	resolvDefault = "/etc/resolv.conf"
+	resolvSystemd = "/run/systemd/resolve/resolv.conf"
+)
+
+var (
+	// fallbackNameServers uses Google DNS
+	fallbackNameServers = []string{
+		"8.8.8.8",
+		"8.8.4.4",
+	}
+)
+
+// EnsureResolvConf ensures a valid, usable resolvConf is written to the filepath
+func EnsureResolvConf(filepath string, perm os.FileMode) error {
+	cfg, err := readDNSConfig()
+	if err != nil {
+		log.Warn(err)
+	}
+
+	filterLoopback(cfg)
+	// Fallback to default DNS servers
+	if len(cfg.Servers) == 0 {
+		for _, s := range fallbackNameServers {
+			cfg.Servers = append(cfg.Servers, s)
+		}
+	}
+
+	data := buildResolvConf(cfg)
+	err = util.WriteFileIfChanged(filepath, data, perm)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// readDNSConfig reads settings from /etc/resolv.conf -- if those settings indicate
+// systemd-resolved is in use, it reads them again from /run/systemd/resolve/resolv.conf.
+// If an error occurs, cfg will be defaulted to an empty dns.ClientConfig{}.
+func readDNSConfig() (*dns.ClientConfig, error) {
+	cfg, err := dns.ClientConfigFromFile(resolvDefault)
+	if err != nil {
+		return &dns.ClientConfig{}, fmt.Errorf("Using default DNS config: %v", err)
+	}
+
+	if isSystemdResolved(cfg) {
+		systemdCfg, err := dns.ClientConfigFromFile(resolvSystemd)
+		if err == nil {
+			return systemdCfg, nil
+		}
+	}
+
+	return cfg, nil
+}
+
+// isSystemdResolved returns whether the cfg likely indicate usage of systemd-resolved.
+// This function should be used with the ClientConfig from /etc/resolv.conf.
+func isSystemdResolved(cfg *dns.ClientConfig) bool {
+	if len(cfg.Servers) == 1 {
+		ip := net.ParseIP(cfg.Servers[0])
+		if ip != nil && ip.IsLoopback() && ip[len(ip)-1] == 53 {
+			return true // cfg.Servers is equivalent to ["127.0.0.53"] or ["::FFff:127.0.0.53"]
+		}
+	}
+	return false
+}
+
+// filterLoopback removes loopback addresses from cfg.Servers since they're
+// not usable as an upstream resolver for a vm
+func filterLoopback(cfg *dns.ClientConfig) {
+	servers := make([]string, 0)
+	for _, s := range cfg.Servers {
+		ip := net.ParseIP(s)
+		if ip != nil && !ip.IsLoopback() {
+			servers = append(servers, s)
+		}
+	}
+	cfg.Servers = servers
+}
+
+// buildResolvConf returns the bytes for a resolv.conf representing clientConfig
+func buildResolvConf(cfg *dns.ClientConfig) []byte {
+	s := "# The following config was built by ignite:\n"
+	if len(cfg.Servers) > 0 {
+		s += "nameserver " +
+			strings.Join(cfg.Servers, "\nnameserver ") +
+			"\n"
+	}
+	if len(cfg.Search) > 0 {
+		s += "search " + strings.Join(cfg.Search, " ") +
+			"\n"
+	}
+	return []byte(s)
+}

pkg/resolvconf/resolvconf_test.go

@@ -0,0 +1,201 @@
+package resolvconf
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/miekg/dns"
+)
+
+func TestBuildResolvConf(t *testing.T) {
+	data := buildResolvConf(&dns.ClientConfig{
+		Servers: []string{"1.1.1.1", "8.8.8.8", "9.9.9.9"},
+		Search:  []string{"fire.local", "test.fire.local"},
+	})
+	expected := `# The following config was built by ignite:
+nameserver 1.1.1.1
+nameserver 8.8.8.8
+nameserver 9.9.9.9
+search fire.local test.fire.local
+`
+	if string(data) != string(expected) {
+		t.Errorf(
+			"\nexpected:\n%q\nactual:\n%q",
+			string(expected),
+			string(data),
+		)
+	}
+}
+
+func TestBuildResolvConfEmpty(t *testing.T) {
+	data := buildResolvConf(&dns.ClientConfig{})
+	expected := `# The following config was built by ignite:
+`
+	if string(data) != string(expected) {
+		t.Errorf(
+			"\nexpected:\n%q\nactual:\n%q",
+			string(expected),
+			string(data),
+		)
+	}
+}
+
+func TestIsSystemdResolved(t *testing.T) {
+	type tc struct {
+		cfg      dns.ClientConfig
+		expected bool
+	}
+	testCases := []tc{
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.53"},
+			},
+			expected: true,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"::ffff:127.0.0.53"},
+			},
+			expected: true,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"::fFFf:127.0.0.53"},
+			},
+			expected: true,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.1", "::ffFF:127.0.0.53", "::ffff:127.0.0.53"},
+			},
+			expected: false,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.1", "::ffFF:127.0.0.53", "::ffff:127.0.0.53"},
+			},
+			expected: false,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"1.1.1.1"},
+			},
+			expected: false,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.1", "1.1.1.1"},
+			},
+			expected: false,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.53", "1.1.1.1", "8.8.8.8", "9.9.9.9"},
+			},
+			expected: false,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.1"},
+			},
+			expected: false,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"::1"},
+			},
+			expected: false,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"::53"},
+			},
+			expected: false,
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{},
+			},
+			expected: false,
+		},
+	}
+	for i, tc := range testCases {
+		res := isSystemdResolved(&tc.cfg)
+		if res != tc.expected {
+			t.Errorf("Case #%d expected %t, got %t: %q", i, tc.expected, res, tc.cfg.Servers)
+		}
+	}
+}
+
+func TestFilterLoopBack(t *testing.T) {
+	type tc struct {
+		cfg      dns.ClientConfig
+		expected []string
+	}
+	testCases := []tc{
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{},
+			},
+			expected: []string{},
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.1", "1.1.1.1", "::ffFF:127.0.0.53", "::ffff:127.0.0.1"},
+			},
+			expected: []string{"1.1.1.1"},
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.1", "::ffFF:127.0.0.53", "::ffff:127.0.0.53"},
+			},
+			expected: []string{},
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"1.1.1.1"},
+			},
+			expected: []string{"1.1.1.1"},
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.1", "1.1.1.1"},
+			},
+			expected: []string{"1.1.1.1"},
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.53", "1.1.1.1", "8.8.8.8", "9.9.9.9"},
+			},
+			expected: []string{"1.1.1.1", "8.8.8.8", "9.9.9.9"},
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"127.0.0.1"},
+			},
+			expected: []string{},
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"::1"},
+			},
+			expected: []string{""},
+		},
+		{
+			cfg: dns.ClientConfig{
+				Servers: []string{"::53"},
+			},
+			expected: []string{"::53"},
+		},
+	}
+	for _, tc := range testCases {
+		filterLoopback(&tc.cfg)
+		if strings.Join(tc.cfg.Servers, ", ") != strings.Join(tc.expected, ", ") {
+			t.Errorf(
+				"\nexpected:\n%v\nactual:\n%v",
+				tc.expected,
+				tc.cfg.Servers,
+			)
+		}
+	}
+}

pkg/runtime/containerd/client.go

@@ -7,10 +7,18 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strconv"
 	"syscall"
 	"time"
 
+	meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
+	"github.com/weaveworks/ignite/pkg/constants"
+	"github.com/weaveworks/ignite/pkg/preflight"
+	"github.com/weaveworks/ignite/pkg/resolvconf"
+	"github.com/weaveworks/ignite/pkg/runtime"
+	"github.com/weaveworks/ignite/pkg/util"
+
 	"github.com/containerd/console"
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cio"
@@ -28,17 +36,14 @@ import (
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	log "github.com/sirupsen/logrus"
-	meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
-	"github.com/weaveworks/ignite/pkg/preflight"
-	"github.com/weaveworks/ignite/pkg/runtime"
-	"github.com/weaveworks/ignite/pkg/util"
 	"golang.org/x/sys/unix"
 )
 
 const (
 	ctdNamespace     = "firecracker"
 	stopTimeoutLabel = "IgniteStopTimeout"
 	logPathTemplate  = "/tmp/%s.log"
+	resolvConfName   = "runtime.containerd.resolv.conf"
 )
 
 var (
@@ -361,7 +366,7 @@ func (cc *ctdClient) AttachContainer(container string) (err error) {
 	return
 }
 
-func (cc *ctdClient) RunContainer(image meta.OCIImageRef, config *runtime.ContainerConfig, name string) (s string, err error) {
+func (cc *ctdClient) RunContainer(image meta.OCIImageRef, config *runtime.ContainerConfig, name, id string) (s string, err error) {
 	img, err := cc.client.GetImage(cc.ctx, image.Normalized())
 	if err != nil {
 		return
@@ -376,7 +381,26 @@ func (cc *ctdClient) RunContainer(image meta.OCIImageRef, config *runtime.Contai
 	snapshotter := cc.client.SnapshotService(containerd.DefaultSnapshotter)
 
 	// Add the /etc/resolv.conf mount, this isn't done automatically by containerd
-	config.Binds = append(config.Binds, runtime.BindBoth("/etc/resolv.conf"))
+	// Ensure a resolv.conf exists in the vmDir. Calculate path using the vm id
+	// TODO(stealthybox):
+	//  - create snapshot ahead of time?
+	//    - is there a performance penalty for creating snapshot ahead of time?
+	//    - maybe we can use containerd.NewContainerOpts{} to do it just-in-time
+	//  - write this file to snapshot mount instead of vmDir
+	//  - commit snapshot?
+	//  - deprecate vm id from this function's signature
+	resolvConfPath := filepath.Join(constants.VM_DIR, id, resolvConfName)
+	err = resolvconf.EnsureResolvConf(resolvConfPath, constants.DATA_DIR_FILE_PERM)
+	if err != nil {
+		return
+	}
+	config.Binds = append(
+		config.Binds,
+		&runtime.Bind{
+			HostPath:      resolvConfPath,
+			ContainerPath: "/etc/resolv.conf",
+		},
+	)
 
 	// Add the stop timeout as a label, as containerd doesn't natively support it
 	config.Labels[stopTimeoutLabel] = strconv.FormatUint(uint64(config.StopTimeout), 10)