Merge pull request #38430 from freehan/cri-docker-network

Automatic merge from submit-queue (batch tested with PRs 38689, 38743, 38734, 38430)

apply sandbox network mode based on network plugin

This allows CRI to use docker's network bridge. Can be combined with noop network plugin. This allows to use docker0 with no further configuration. Good for tools like minikube/hyperkube.

pkg/kubelet/dockershim/docker_sandbox.go

@@ -294,7 +294,7 @@ func (ds *dockerService) applySandboxLinuxOptions(hc *dockercontainer.HostConfig
 	// TODO: Check if this works with per-pod cgroups.
 	hc.CgroupParent = lc.GetCgroupParent()
 	// Apply security context.
-	applySandboxSecurityContext(lc, createConfig.Config, hc)
+	applySandboxSecurityContext(lc, createConfig.Config, hc, ds.networkPlugin)
 
 	return nil
 }

pkg/kubelet/dockershim/security_context.go

@@ -24,11 +24,12 @@ import (
 
 	"k8s.io/kubernetes/pkg/api/v1"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
+	"k8s.io/kubernetes/pkg/kubelet/network"
 	"k8s.io/kubernetes/pkg/securitycontext"
 )
 
 // applySandboxSecurityContext updates docker sandbox options according to security context.
-func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *dockercontainer.Config, hc *dockercontainer.HostConfig) {
+func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *dockercontainer.Config, hc *dockercontainer.HostConfig, networkPlugin network.NetworkPlugin) {
 	if lc == nil {
 		return
 	}
@@ -45,7 +46,9 @@ func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *d
 	}
 
 	modifyContainerConfig(sc, config)
-	modifyHostConfig(sc, "", hc)
+	modifyHostConfig(sc, hc)
+	modifySandboxNamespaceOptions(sc.GetNamespaceOptions(), hc, networkPlugin)
+
 }
 
 // applyContainerSecurityContext updates docker container options according to security context.
@@ -55,7 +58,8 @@ func applyContainerSecurityContext(lc *runtimeapi.LinuxContainerConfig, sandboxI
 	}
 
 	modifyContainerConfig(lc.SecurityContext, config)
-	modifyHostConfig(lc.SecurityContext, sandboxID, hc)
+	modifyHostConfig(lc.SecurityContext, hc)
+	modifyContainerNamespaceOptions(lc.SecurityContext.GetNamespaceOptions(), sandboxID, hc)
 	return
 }
 
@@ -73,10 +77,7 @@ func modifyContainerConfig(sc *runtimeapi.LinuxContainerSecurityContext, config
 }
 
 // modifyHostConfig applies security context config to dockercontainer.HostConfig.
-func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, sandboxID string, hostConfig *dockercontainer.HostConfig) {
+func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, hostConfig *dockercontainer.HostConfig) {
-	// Apply namespace options.
-	modifyNamespaceOptions(sc.GetNamespaceOptions(), sandboxID, hostConfig)
-
 	if sc == nil {
 		return
 	}
@@ -110,13 +111,21 @@ func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, sandboxID st
 	}
 }
 
-// modifyNamespaceOptions applies namespaceoptions to dockercontainer.HostConfig.
+// modifySandboxNamespaceOptions apply namespace options for sandbox
-func modifyNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, sandboxID string, hostConfig *dockercontainer.HostConfig) {
+func modifySandboxNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, hostConfig *dockercontainer.HostConfig, networkPlugin network.NetworkPlugin) {
-	hostNetwork := false
+	modifyCommonNamespaceOptions(nsOpts, hostConfig)
+	modifyHostNetworkOptionForSandbox(nsOpts.GetHostNetwork(), networkPlugin, hostConfig)
+}
+
+// modifyContainerNamespaceOptions apply namespace options for container
+func modifyContainerNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, sandboxID string, hostConfig *dockercontainer.HostConfig) {
+	modifyCommonNamespaceOptions(nsOpts, hostConfig)
+	modifyHostNetworkOptionForContainer(nsOpts.GetHostNetwork(), sandboxID, hostConfig)
+}
+
+// modifyCommonNamespaceOptions apply common namespace options for sandbox and container
+func modifyCommonNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, hostConfig *dockercontainer.HostConfig) {
 	if nsOpts != nil {
-		if nsOpts.HostNetwork != nil {
-			hostNetwork = nsOpts.GetHostNetwork()
-		}
 		if nsOpts.GetHostPid() {
 			hostConfig.PidMode = namespaceModeHost
 		}
@@ -124,24 +133,27 @@ func modifyNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, sandboxID string
 			hostConfig.IpcMode = namespaceModeHost
 		}
 	}
-
-	// Set for sandbox if sandboxID is not provided.
-	if sandboxID == "" {
-		modifyHostNetworkOptionForSandbox(hostNetwork, hostConfig)
-	} else {
-		// Set for container if sandboxID is provided.
-		modifyHostNetworkOptionForContainer(hostNetwork, sandboxID, hostConfig)
-	}
 }
 
 // modifyHostNetworkOptionForSandbox applies NetworkMode/UTSMode to sandbox's dockercontainer.HostConfig.
-func modifyHostNetworkOptionForSandbox(hostNetwork bool, hc *dockercontainer.HostConfig) {
+func modifyHostNetworkOptionForSandbox(hostNetwork bool, networkPlugin network.NetworkPlugin, hc *dockercontainer.HostConfig) {
 	if hostNetwork {
 		hc.NetworkMode = namespaceModeHost
-	} else {
+		return
-		// Assume kubelet uses either the cni or the kubenet plugin.
+	}
-		// TODO: support docker networking.
+
+	if networkPlugin == nil {
+		hc.NetworkMode = "default"
+		return
+	}
+
+	switch networkPlugin.Name() {
+	case "cni":
+		fallthrough
+	case "kubenet":
 		hc.NetworkMode = "none"
+	default:
+		hc.NetworkMode = "default"
 	}
 }
 

pkg/kubelet/dockershim/security_context_test.go

@@ -71,22 +71,17 @@ func TestModifyContainerConfig(t *testing.T) {
 
 func TestModifyHostConfig(t *testing.T) {
 	priv := true
-	setNetworkHC := &dockercontainer.HostConfig{
+	setNetworkHC := &dockercontainer.HostConfig{}
-		NetworkMode: "none",
-	}
 	setPrivSC := &runtimeapi.LinuxContainerSecurityContext{}
 	setPrivSC.Privileged = &priv
 	setPrivHC := &dockercontainer.HostConfig{
-		Privileged:  true,
+		Privileged: true,
-		NetworkMode: "none",
 	}
 	setCapsHC := &dockercontainer.HostConfig{
-		NetworkMode: "none",
+		CapAdd:  []string{"addCapA", "addCapB"},
-		CapAdd:      []string{"addCapA", "addCapB"},
+		CapDrop: []string{"dropCapA", "dropCapB"},
-		CapDrop:     []string{"dropCapA", "dropCapB"},
 	}
 	setSELinuxHC := &dockercontainer.HostConfig{
-		NetworkMode: "none",
 		SecurityOpt: []string{
 			fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser, "user"),
 			fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole, "role"),
@@ -133,7 +128,7 @@ func TestModifyHostConfig(t *testing.T) {
 
 	for _, tc := range cases {
 		dockerCfg := &dockercontainer.HostConfig{}
-		modifyHostConfig(tc.sc, "", dockerCfg)
+		modifyHostConfig(tc.sc, dockerCfg)
 		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
 	}
 }
@@ -141,7 +136,7 @@ func TestModifyHostConfig(t *testing.T) {
 func TestModifyHostConfigWithGroups(t *testing.T) {
 	supplementalGroupsSC := &runtimeapi.LinuxContainerSecurityContext{}
 	supplementalGroupsSC.SupplementalGroups = []int64{2222}
-	supplementalGroupHC := &dockercontainer.HostConfig{NetworkMode: "none"}
+	supplementalGroupHC := &dockercontainer.HostConfig{}
 	supplementalGroupHC.GroupAdd = []string{"2222"}
 
 	testCases := []struct {
@@ -152,7 +147,7 @@ func TestModifyHostConfigWithGroups(t *testing.T) {
 		{
 			name:            "nil",
 			securityContext: nil,
-			expected:        &dockercontainer.HostConfig{NetworkMode: "none"},
+			expected:        &dockercontainer.HostConfig{},
 		},
 		{
 			name:            "SupplementalGroup",
@@ -163,12 +158,12 @@ func TestModifyHostConfigWithGroups(t *testing.T) {
 
 	for _, tc := range testCases {
 		dockerCfg := &dockercontainer.HostConfig{}
-		modifyHostConfig(tc.securityContext, "", dockerCfg)
+		modifyHostConfig(tc.securityContext, dockerCfg)
 		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
 	}
 }
 
-func TestModifyHostConfigWithSandboxID(t *testing.T) {
+func TestModifyHostConfigAndNamespaceOptionsForContainer(t *testing.T) {
 	priv := true
 	sandboxID := "sandbox"
 	sandboxNSMode := fmt.Sprintf("container:%v", sandboxID)
@@ -224,7 +219,100 @@ func TestModifyHostConfigWithSandboxID(t *testing.T) {
 
 	for _, tc := range cases {
 		dockerCfg := &dockercontainer.HostConfig{}
-		modifyHostConfig(tc.sc, sandboxID, dockerCfg)
+		modifyHostConfig(tc.sc, dockerCfg)
+		modifyContainerNamespaceOptions(tc.sc.GetNamespaceOptions(), sandboxID, dockerCfg)
+		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
+	}
+}
+
+func TestModifySandboxNamespaceOptions(t *testing.T) {
+	set := true
+	cases := []struct {
+		name     string
+		nsOpt    *runtimeapi.NamespaceOption
+		expected *dockercontainer.HostConfig
+	}{
+		{
+			name: "NamespaceOption.HostNetwork",
+			nsOpt: &runtimeapi.NamespaceOption{
+				HostNetwork: &set,
+			},
+			expected: &dockercontainer.HostConfig{
+				NetworkMode: namespaceModeHost,
+			},
+		},
+		{
+			name: "NamespaceOption.HostIpc",
+			nsOpt: &runtimeapi.NamespaceOption{
+				HostIpc: &set,
+			},
+			expected: &dockercontainer.HostConfig{
+				IpcMode:     namespaceModeHost,
+				NetworkMode: "default",
+			},
+		},
+		{
+			name: "NamespaceOption.HostPid",
+			nsOpt: &runtimeapi.NamespaceOption{
+				HostPid: &set,
+			},
+			expected: &dockercontainer.HostConfig{
+				PidMode:     namespaceModeHost,
+				NetworkMode: "default",
+			},
+		},
+	}
+	for _, tc := range cases {
+		dockerCfg := &dockercontainer.HostConfig{}
+		modifySandboxNamespaceOptions(tc.nsOpt, dockerCfg, nil)
+		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
+	}
+}
+
+func TestModifyContainerNamespaceOptions(t *testing.T) {
+	set := true
+	sandboxID := "sandbox"
+	sandboxNSMode := fmt.Sprintf("container:%v", sandboxID)
+	cases := []struct {
+		name     string
+		nsOpt    *runtimeapi.NamespaceOption
+		expected *dockercontainer.HostConfig
+	}{
+		{
+			name: "NamespaceOption.HostNetwork",
+			nsOpt: &runtimeapi.NamespaceOption{
+				HostNetwork: &set,
+			},
+			expected: &dockercontainer.HostConfig{
+				NetworkMode: dockercontainer.NetworkMode(sandboxNSMode),
+				IpcMode:     dockercontainer.IpcMode(sandboxNSMode),
+				UTSMode:     namespaceModeHost,
+			},
+		},
+		{
+			name: "NamespaceOption.HostIpc",
+			nsOpt: &runtimeapi.NamespaceOption{
+				HostIpc: &set,
+			},
+			expected: &dockercontainer.HostConfig{
+				NetworkMode: dockercontainer.NetworkMode(sandboxNSMode),
+				IpcMode:     dockercontainer.IpcMode(sandboxNSMode),
+			},
+		},
+		{
+			name: "NamespaceOption.HostPid",
+			nsOpt: &runtimeapi.NamespaceOption{
+				HostPid: &set,
+			},
+			expected: &dockercontainer.HostConfig{
+				NetworkMode: dockercontainer.NetworkMode(sandboxNSMode),
+				IpcMode:     dockercontainer.IpcMode(sandboxNSMode),
+			},
+		},
+	}
+	for _, tc := range cases {
+		dockerCfg := &dockercontainer.HostConfig{}
+		modifyContainerNamespaceOptions(tc.nsOpt, sandboxID, dockerCfg)
 		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
 	}
 }
@@ -261,10 +349,9 @@ func inputSELinuxOptions() *runtimeapi.SELinuxOption {
 
 func fullValidHostConfig() *dockercontainer.HostConfig {
 	return &dockercontainer.HostConfig{
-		Privileged:  true,
+		Privileged: true,
-		NetworkMode: "none",
+		CapAdd:     []string{"addCapA", "addCapB"},
-		CapAdd:      []string{"addCapA", "addCapB"},
+		CapDrop:    []string{"dropCapA", "dropCapB"},
-		CapDrop:     []string{"dropCapA", "dropCapB"},
 		SecurityOpt: []string{
 			fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser, "user"),
 			fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole, "role"),