Merge pull request #20553 from jonboulle/yifan-gu-rkt_dns

rkt: Add DNS support

pkg/kubelet/container/helpers.go

@@ -36,10 +36,11 @@ type HandlerRunner interface {
 	Run(containerID ContainerID, pod *api.Pod, container *api.Container, handler *api.Handler) error
 }
 
-// RunContainerOptionsGenerator generates the options that necessary for
+// RuntimeHelper wraps kubelet to make container runtime
-// container runtime to run a container.
+// able to get necessary informations like the RunContainerOptions, DNS settings.
-type RunContainerOptionsGenerator interface {
+type RuntimeHelper interface {
 	GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*RunContainerOptions, error)
+	GetClusterDNS(pod *api.Pod) (dnsServers []string, dnsSearches []string, err error)
 }
 
 // ShouldContainerBeRestarted checks whether a container needs to be restarted.

pkg/kubelet/dockertools/fake_manager.go

@@ -40,13 +40,13 @@ func NewFakeDockerManager(
 	containerLogsDir string,
 	osInterface kubecontainer.OSInterface,
 	networkPlugin network.NetworkPlugin,
-	generator kubecontainer.RunContainerOptionsGenerator,
+	runtimeHelper kubecontainer.RuntimeHelper,
 	httpClient kubetypes.HttpGetter, imageBackOff *util.Backoff) *DockerManager {
 
 	fakeOOMAdjuster := oom.NewFakeOOMAdjuster()
 	fakeProcFs := procfs.NewFakeProcFS()
 	dm := NewDockerManager(client, recorder, livenessManager, containerRefManager, machineInfo, podInfraContainerImage, qps,
-		burst, containerLogsDir, osInterface, networkPlugin, generator, httpClient, &NativeExecHandler{},
+		burst, containerLogsDir, osInterface, networkPlugin, runtimeHelper, httpClient, &NativeExecHandler{},
 		fakeOOMAdjuster, fakeProcFs, false, imageBackOff, true)
 	dm.dockerPuller = &FakeDockerPuller{}
 	return dm

pkg/kubelet/dockertools/manager.go

@@ -129,8 +129,8 @@ type DockerManager struct {
 	// Health check results.
 	livenessManager proberesults.Manager
 
-	// Generator of runtime container options.
+	// RuntimeHelper that wraps kubelet to generate runtime container options.
-	generator kubecontainer.RunContainerOptionsGenerator
+	runtimeHelper kubecontainer.RuntimeHelper
 
 	// Runner of lifecycle events.
 	runner kubecontainer.HandlerRunner
@@ -163,7 +163,7 @@ func NewDockerManager(
 	containerLogsDir string,
 	osInterface kubecontainer.OSInterface,
 	networkPlugin network.NetworkPlugin,
-	generator kubecontainer.RunContainerOptionsGenerator,
+	runtimeHelper kubecontainer.RuntimeHelper,
 	httpClient kubetypes.HttpGetter,
 	execHandler ExecHandler,
 	oomAdjuster *oom.OOMAdjuster,
@@ -217,7 +217,7 @@ func NewDockerManager(
 		containerLogsDir:       containerLogsDir,
 		networkPlugin:          networkPlugin,
 		livenessManager:        livenessManager,
-		generator:              generator,
+		runtimeHelper:          runtimeHelper,
 		execHandler:            execHandler,
 		oomAdjuster:            oomAdjuster,
 		procFs:                 procFs,
@@ -1533,7 +1533,7 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe
 		glog.Errorf("Can't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
 	}
 
-	opts, err := dm.generator.GenerateRunContainerOptions(pod, container)
+	opts, err := dm.runtimeHelper.GenerateRunContainerOptions(pod, container)
 	if err != nil {
 		return kubecontainer.ContainerID{}, fmt.Errorf("GenerateRunContainerOptions: %v", err)
 	}

pkg/kubelet/dockertools/manager_test.go

@@ -58,13 +58,15 @@ func (f *fakeHTTP) Get(url string) (*http.Response, error) {
 	return nil, f.err
 }
 
-type fakeOptionGenerator struct{}
+// fakeRuntimeHelper implementes kubecontainer.RuntimeHelper inter
+// faces for testing purposes.
+type fakeRuntimeHelper struct{}
 
-var _ kubecontainer.RunContainerOptionsGenerator = &fakeOptionGenerator{}
+var _ kubecontainer.RuntimeHelper = &fakeRuntimeHelper{}
 
 var testPodContainerDir string
 
-func (*fakeOptionGenerator) GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*kubecontainer.RunContainerOptions, error) {
+func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*kubecontainer.RunContainerOptions, error) {
 	var opts kubecontainer.RunContainerOptions
 	var err error
 	if len(container.TerminationMessagePath) != 0 {
@@ -77,12 +79,15 @@ func (*fakeOptionGenerator) GenerateRunContainerOptions(pod *api.Pod, container
 	return &opts, nil
 }
 
+func (f *fakeRuntimeHelper) GetClusterDNS(pod *api.Pod) ([]string, []string, error) {
+	return nil, nil, fmt.Errorf("not implemented")
+}
+
 func newTestDockerManagerWithHTTPClient(fakeHTTPClient *fakeHTTP) (*DockerManager, *FakeDockerClient) {
 	fakeDocker := NewFakeDockerClient()
 	fakeRecorder := &record.FakeRecorder{}
 	containerRefManager := kubecontainer.NewRefManager()
 	networkPlugin, _ := network.InitNetworkPlugin([]network.NetworkPlugin{}, "", network.NewFakeHost(nil))
-	optionGenerator := &fakeOptionGenerator{}
 	dockerManager := NewFakeDockerManager(
 		fakeDocker,
 		fakeRecorder,
@@ -93,7 +98,7 @@ func newTestDockerManagerWithHTTPClient(fakeHTTPClient *fakeHTTP) (*DockerManage
 		0, 0, "",
 		kubecontainer.FakeOS{},
 		networkPlugin,
-		optionGenerator,
+		&fakeRuntimeHelper{},
 		fakeHTTPClient,
 		util.NewBackOff(time.Second, 300*time.Second))
 

pkg/kubelet/kubelet.go

@@ -1297,7 +1297,7 @@ func (kl *Kubelet) GenerateRunContainerOptions(pod *api.Pod, container *api.Cont
 		}
 	}
 
-	opts.DNS, opts.DNSSearch, err = kl.getClusterDNS(pod)
+	opts.DNS, opts.DNSSearch, err = kl.GetClusterDNS(pod)
 	if err != nil {
 		return nil, err
 	}
@@ -1466,9 +1466,9 @@ func (kl *Kubelet) podFieldSelectorRuntimeValue(fs *api.ObjectFieldSelector, pod
 	return fieldpath.ExtractFieldPathAsString(pod, internalFieldPath)
 }
 
-// getClusterDNS returns a list of the DNS servers and a list of the DNS search
+// GetClusterDNS returns a list of the DNS servers and a list of the DNS search
 // domains of the cluster.
-func (kl *Kubelet) getClusterDNS(pod *api.Pod) ([]string, []string, error) {
+func (kl *Kubelet) GetClusterDNS(pod *api.Pod) ([]string, []string, error) {
 	var hostDNS, hostSearch []string
 	// Get host DNS settings
 	if kl.resolverConfig != "" {

pkg/kubelet/rkt/fake_rkt_interface.go

@@ -21,10 +21,13 @@ import (
 	"strconv"
 	"sync"
 
+	"k8s.io/kubernetes/pkg/api"
+
 	"github.com/coreos/go-systemd/dbus"
 	rktapi "github.com/coreos/rkt/api/v1alpha"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
+	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )
 
 // fakeRktInterface mocks the rktapi.PublicAPIClient interface for testing purpose.
@@ -142,3 +145,18 @@ func (f *fakeSystemd) RestartUnit(name string, mode string, ch chan<- string) (i
 func (f *fakeSystemd) Reload() error {
 	return fmt.Errorf("Not implemented")
 }
+
+// fakeRuntimeHelper implementes kubecontainer.RuntimeHelper interfaces for testing purpose.
+type fakeRuntimeHelper struct {
+	dnsServers  []string
+	dnsSearches []string
+	err         error
+}
+
+func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*kubecontainer.RunContainerOptions, error) {
+	return nil, fmt.Errorf("Not implemented")
+}
+
+func (f *fakeRuntimeHelper) GetClusterDNS(pod *api.Pod) ([]string, []string, error) {
+	return f.dnsServers, f.dnsSearches, f.err
+}

pkg/kubelet/rkt/rkt.go

@@ -90,6 +90,12 @@ const (
 	defaultImageTag          = "latest"
 	defaultRktAPIServiceAddr = "localhost:15441"
 	defaultNetworkName       = "rkt.kubernetes.io"
+
+	// ndots specifies the minimum number of dots that a domain name must contain for the resolver to consider it as FQDN (fully-qualified)
+	// we want to able to consider SRV lookup names like _dns._udp.kube-dns.default.svc to be considered relative.
+	// hence, setting ndots to be 5.
+	// TODO(yifan): Move this and dockertools.ndotsDNSOption to a common package.
+	defaultDNSOption = "ndots:5"
 )
 
 // Runtime implements the Containerruntime for rkt. The implementation
@@ -107,7 +113,7 @@ type Runtime struct {
 	dockerKeyring credentialprovider.DockerKeyring
 
 	containerRefManager *kubecontainer.RefManager
-	generator           kubecontainer.RunContainerOptionsGenerator
+	runtimeHelper       kubecontainer.RuntimeHelper
 	recorder            record.EventRecorder
 	livenessManager     proberesults.Manager
 	volumeGetter        volumeGetter
@@ -131,7 +137,7 @@ type volumeGetter interface {
 // It will test if the rkt binary is in the $PATH, and whether we can get the
 // version of it. If so, creates the rkt container runtime, otherwise returns an error.
 func New(config *Config,
-	generator kubecontainer.RunContainerOptionsGenerator,
+	runtimeHelper kubecontainer.RuntimeHelper,
 	recorder record.EventRecorder,
 	containerRefManager *kubecontainer.RefManager,
 	livenessManager proberesults.Manager,
@@ -169,7 +175,7 @@ func New(config *Config,
 		config:              config,
 		dockerKeyring:       credentialprovider.NewDockerKeyring(),
 		containerRefManager: containerRefManager,
-		generator:           generator,
+		runtimeHelper:       runtimeHelper,
 		recorder:            recorder,
 		livenessManager:     livenessManager,
 		volumeGetter:        volumeGetter,
@@ -582,7 +588,7 @@ func (r *Runtime) newAppcRuntimeApp(pod *api.Pod, c api.Container, pullSecrets [
 		return err
 	}
 
-	opts, err := r.generator.GenerateRunContainerOptions(pod, &c)
+	opts, err := r.runtimeHelper.GenerateRunContainerOptions(pod, &c)
 	if err != nil {
 		return err
 	}
@@ -702,6 +708,35 @@ func serviceFilePath(serviceName string) string {
 	return path.Join(systemdServiceDir, serviceName)
 }
 
+// generateRunCommand crafts a 'rkt run-prepared' command with necessary parameters.
+func (r *Runtime) generateRunCommand(pod *api.Pod, uuid string) (string, error) {
+	runPrepared := r.buildCommand("run-prepared").Args
+
+	// Setup network configuration.
+	if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.HostNetwork {
+		runPrepared = append(runPrepared, "--net=host")
+	} else {
+		runPrepared = append(runPrepared, fmt.Sprintf("--net=%s", defaultNetworkName))
+	}
+
+	// Setup DNS.
+	dnsServers, dnsSearches, err := r.runtimeHelper.GetClusterDNS(pod)
+	if err != nil {
+		return "", err
+	}
+	for _, server := range dnsServers {
+		runPrepared = append(runPrepared, fmt.Sprintf("--dns=%s", server))
+	}
+	for _, search := range dnsSearches {
+		runPrepared = append(runPrepared, fmt.Sprintf("--dns-search=%s", search))
+	}
+	if len(dnsServers) > 0 || len(dnsSearches) > 0 {
+		runPrepared = append(runPrepared, fmt.Sprintf("--dns-opt=%s", defaultDNSOption))
+	}
+	runPrepared = append(runPrepared, uuid)
+	return strings.Join(runPrepared, " "), nil
+}
+
 // preparePod will:
 //
 // 1. Invoke 'rkt prepare' to prepare the pod, and get the rkt pod uuid.
@@ -754,11 +789,9 @@ func (r *Runtime) preparePod(pod *api.Pod, pullSecrets []api.Secret) (string, *k
 	glog.V(4).Infof("'rkt prepare' returns %q", uuid)
 
 	// Create systemd service file for the rkt pod.
-	var runPrepared string
+	runPrepared, err := r.generateRunCommand(pod, uuid)
-	if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.HostNetwork {
+	if err != nil {
-		runPrepared = fmt.Sprintf("%s run-prepared --mds-register=false --net=host %s", r.rktBinAbsPath, uuid)
+		return "", nil, fmt.Errorf("failed to generate 'rkt run-prepared' command: %v", err)
-	} else {
-		runPrepared = fmt.Sprintf("%s run-prepared --mds-register=false --net=%s %s", r.rktBinAbsPath, defaultNetworkName, uuid)
 	}
 
 	// TODO handle pod.Spec.HostPID

pkg/kubelet/rkt/rkt_test.go

@@ -955,3 +955,102 @@ func TestSetApp(t *testing.T) {
 		}
 	}
 }
+
+func TestGenerateRunCommand(t *testing.T) {
+	tests := []struct {
+		pod  *api.Pod
+		uuid string
+
+		dnsServers  []string
+		dnsSearches []string
+		err         error
+
+		expect string
+	}{
+		// Case #0, returns error.
+		{
+			&api.Pod{
+				Spec: api.PodSpec{},
+			},
+			"rkt-uuid-foo",
+			[]string{},
+			[]string{},
+			fmt.Errorf("failed to get cluster dns"),
+			"",
+		},
+		// Case #1, returns no dns, with private-net.
+		{
+			&api.Pod{},
+			"rkt-uuid-foo",
+			[]string{},
+			[]string{},
+			nil,
+			"/bin/rkt/rkt --debug=false --insecure-options=image,ondisk --local-config=/var/rkt/local/data --dir=/var/data run-prepared --net=rkt.kubernetes.io rkt-uuid-foo",
+		},
+		// Case #2, returns no dns, with host-net.
+		{
+			&api.Pod{
+				Spec: api.PodSpec{
+					SecurityContext: &api.PodSecurityContext{
+						HostNetwork: true,
+					},
+				},
+			},
+			"rkt-uuid-foo",
+			[]string{},
+			[]string{},
+			nil,
+			"/bin/rkt/rkt --debug=false --insecure-options=image,ondisk --local-config=/var/rkt/local/data --dir=/var/data run-prepared --net=host rkt-uuid-foo",
+		},
+		// Case #3, returns dns, dns searches, with private-net.
+		{
+			&api.Pod{
+				Spec: api.PodSpec{
+					SecurityContext: &api.PodSecurityContext{
+						HostNetwork: false,
+					},
+				},
+			},
+			"rkt-uuid-foo",
+			[]string{"127.0.0.1"},
+			[]string{"."},
+			nil,
+			"/bin/rkt/rkt --debug=false --insecure-options=image,ondisk --local-config=/var/rkt/local/data --dir=/var/data run-prepared --net=rkt.kubernetes.io --dns=127.0.0.1 --dns-search=. --dns-opt=ndots:5 rkt-uuid-foo",
+		},
+		// Case #4, returns dns, dns searches, with host-network.
+		{
+			&api.Pod{
+				Spec: api.PodSpec{
+					SecurityContext: &api.PodSecurityContext{
+						HostNetwork: true,
+					},
+				},
+			},
+			"rkt-uuid-foo",
+			[]string{"127.0.0.1"},
+			[]string{"."},
+			nil,
+			"/bin/rkt/rkt --debug=false --insecure-options=image,ondisk --local-config=/var/rkt/local/data --dir=/var/data run-prepared --net=host --dns=127.0.0.1 --dns-search=. --dns-opt=ndots:5 rkt-uuid-foo",
+		},
+	}
+
+	rkt := &Runtime{
+		rktBinAbsPath: "/bin/rkt/rkt",
+		config: &Config{
+			Path:            "/bin/rkt/rkt",
+			Stage1Image:     "/bin/rkt/stage1-coreos.aci",
+			Dir:             "/var/data",
+			InsecureOptions: "image,ondisk",
+			LocalConfigDir:  "/var/rkt/local/data",
+		},
+	}
+
+	for i, tt := range tests {
+		testCaseHint := fmt.Sprintf("test case #%d", i)
+		rkt.runtimeHelper = &fakeRuntimeHelper{tt.dnsServers, tt.dnsSearches, tt.err}
+
+		result, err := rkt.generateRunCommand(tt.pod, tt.uuid)
+		assert.Equal(t, tt.err, err, testCaseHint)
+		assert.Equal(t, tt.expect, result, testCaseHint)
+	}
+}