Merge pull request #41766 from feiskyer/sysctls

pkg/kubelet/apis/cri/v1alpha1/api.proto

@@ -205,6 +205,8 @@ message LinuxPodSandboxConfig {
     string cgroup_parent = 1;
     // LinuxSandboxSecurityContext holds sandbox security attributes.
     LinuxSandboxSecurityContext security_context = 2;
+    // Sysctls holds linux sysctls config for the sandbox.
+    map<string, string> sysctls = 3;
 }
 
 // PodSandboxMetadata holds all necessary information for building the sandbox name.
@@ -290,16 +292,6 @@ message PodSandboxConfig {
     //        local seccomp profile root. Note that profile root is set in
     //        kubelet, and it is not passed in CRI yet, see https://issues.k8s.io/36997.
     //
-    // 2. Sysctls
-    //
-    //      key: security.alpha.kubernetes.io/sysctls
-    //      description: list of safe sysctls which are set for the sandbox.
-    //      value: comma separated list of sysctl_name=value key-value pairs.
-    //
-    //      key: security.alpha.kubernetes.io/unsafe-sysctls
-    //      description: list of unsafe sysctls which are set for the sandbox.
-    //      value: comma separated list of sysctl_name=value key-value pairs.
-    //
     map<string, string> annotations = 7;
     // Optional configurations specific to Linux hosts.
     LinuxPodSandboxConfig linux = 8;

pkg/kubelet/dockershim/BUILD

@@ -30,7 +30,6 @@ go_library(
     tags = ["automanaged"],
     deps = [
         "//pkg/api/v1:go_default_library",
-        "//pkg/api/v1/helper:go_default_library",
         "//pkg/apis/componentconfig:go_default_library",
         "//pkg/client/unversioned/remotecommand:go_default_library",
         "//pkg/kubelet/apis/cri:go_default_library",

pkg/kubelet/dockershim/docker_sandbox.go

@@ -477,6 +477,9 @@ func (ds *dockerService) applySandboxLinuxOptions(hc *dockercontainer.HostConfig
 		return err
 	}
 
+	// Set sysctls.
+	hc.Sysctls = lc.Sysctls
+
 	return nil
 }
 
@@ -508,13 +511,6 @@ func (ds *dockerService) makeSandboxDockerConfig(c *runtimeapi.PodSandboxConfig,
 		HostConfig: hc,
 	}
 
-	// Set sysctls if requested
-	sysctls, err := getSysctlsFromAnnotations(c.Annotations)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get sysctls from annotations %v for sandbox %q: %v", c.Annotations, c.Metadata.Name, err)
-	}
-	hc.Sysctls = sysctls
-
 	// Apply linux-specific options.
 	if lc := c.GetLinux(); lc != nil {
 		if err := ds.applySandboxLinuxOptions(hc, lc, createConfig, image, securityOptSep); err != nil {

pkg/kubelet/dockershim/helpers.go

@@ -34,7 +34,6 @@ import (
 	"github.com/golang/glog"
 
 	"k8s.io/kubernetes/pkg/api/v1"
-	v1helper "k8s.io/kubernetes/pkg/api/v1/helper"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1"
 	"k8s.io/kubernetes/pkg/kubelet/types"
 	"k8s.io/kubernetes/pkg/security/apparmor"
@@ -274,27 +273,6 @@ func getNetworkNamespace(c *dockertypes.ContainerJSON) string {
 	return fmt.Sprintf(dockerNetNSFmt, c.State.Pid)
 }
 
-// getSysctlsFromAnnotations gets sysctls from annotations.
-func getSysctlsFromAnnotations(annotations map[string]string) (map[string]string, error) {
-	var results map[string]string
-
-	sysctls, unsafeSysctls, err := v1helper.SysctlsFromPodAnnotations(annotations)
-	if err != nil {
-		return nil, err
-	}
-	if len(sysctls)+len(unsafeSysctls) > 0 {
-		results = make(map[string]string, len(sysctls)+len(unsafeSysctls))
-		for _, c := range sysctls {
-			results[c.Name] = c.Value
-		}
-		for _, c := range unsafeSysctls {
-			results[c.Name] = c.Value
-		}
-	}
-
-	return results, nil
-}
-
 // dockerFilter wraps around dockerfilters.Args and provides methods to modify
 // the filter easily.
 type dockerFilter struct {

pkg/kubelet/dockershim/helpers_test.go

@@ -175,46 +175,6 @@ func TestGetApparmorSecurityOpts(t *testing.T) {
 	}
 }
 
-// TestGetSystclsFromAnnotations tests the logic of getting sysctls from annotations.
-func TestGetSystclsFromAnnotations(t *testing.T) {
-	tests := []struct {
-		annotations     map[string]string
-		expectedSysctls map[string]string
-	}{{
-		annotations: map[string]string{
-			v1.SysctlsPodAnnotationKey:       "kernel.shmmni=32768,kernel.shmmax=1000000000",
-			v1.UnsafeSysctlsPodAnnotationKey: "knet.ipv4.route.min_pmtu=1000",
-		},
-		expectedSysctls: map[string]string{
-			"kernel.shmmni":            "32768",
-			"kernel.shmmax":            "1000000000",
-			"knet.ipv4.route.min_pmtu": "1000",
-		},
-	}, {
-		annotations: map[string]string{
-			v1.SysctlsPodAnnotationKey: "kernel.shmmni=32768,kernel.shmmax=1000000000",
-		},
-		expectedSysctls: map[string]string{
-			"kernel.shmmni": "32768",
-			"kernel.shmmax": "1000000000",
-		},
-	}, {
-		annotations: map[string]string{
-			v1.UnsafeSysctlsPodAnnotationKey: "knet.ipv4.route.min_pmtu=1000",
-		},
-		expectedSysctls: map[string]string{
-			"knet.ipv4.route.min_pmtu": "1000",
-		},
-	}}
-
-	for i, test := range tests {
-		actual, err := getSysctlsFromAnnotations(test.annotations)
-		assert.NoError(t, err, "TestCase[%d]", i)
-		assert.Len(t, actual, len(test.expectedSysctls), "TestCase[%d]", i)
-		assert.Equal(t, test.expectedSysctls, actual, "TestCase[%d]", i)
-	}
-}
-
 // TestGetUserFromImageUser tests the logic of getting image uid or user name of image user.
 func TestGetUserFromImageUser(t *testing.T) {
 	newI64 := func(i int64) *int64 { return &i }

pkg/kubelet/kuberuntime/BUILD

@@ -29,6 +29,7 @@ go_library(
     deps = [
         "//pkg/api:go_default_library",
         "//pkg/api/v1:go_default_library",
+        "//pkg/api/v1/helper:go_default_library",
         "//pkg/api/v1/ref:go_default_library",
         "//pkg/credentialprovider:go_default_library",
         "//pkg/kubelet/apis/cri:go_default_library",

pkg/kubelet/kuberuntime/helpers.go

@@ -24,6 +24,7 @@ import (
 	"github.com/golang/glog"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubernetes/pkg/api/v1"
+	v1helper "k8s.io/kubernetes/pkg/api/v1/helper"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )
@@ -236,3 +237,21 @@ func toKubeRuntimeStatus(status *runtimeapi.RuntimeStatus) *kubecontainer.Runtim
 	}
 	return &kubecontainer.RuntimeStatus{Conditions: conditions}
 }
+
+// getSysctlsFromAnnotations gets sysctls and unsafeSysctls from annotations.
+func getSysctlsFromAnnotations(annotations map[string]string) (map[string]string, error) {
+	apiSysctls, apiUnsafeSysctls, err := v1helper.SysctlsFromPodAnnotations(annotations)
+	if err != nil {
+		return nil, err
+	}
+
+	sysctls := make(map[string]string)
+	for _, c := range apiSysctls {
+		sysctls[c.Name] = c.Value
+	}
+	for _, c := range apiUnsafeSysctls {
+		sysctls[c.Name] = c.Value
+	}
+
+	return sysctls, nil
+}

pkg/kubelet/kuberuntime/helpers_test.go

@@ -46,3 +46,43 @@ func TestStableKey(t *testing.T) {
 	newKey := getStableKey(pod, container)
 	assert.NotEqual(t, oldKey, newKey)
 }
+
+// TestGetSystclsFromAnnotations tests the logic of getting sysctls from annotations.
+func TestGetSystclsFromAnnotations(t *testing.T) {
+	tests := []struct {
+		annotations     map[string]string
+		expectedSysctls map[string]string
+	}{{
+		annotations: map[string]string{
+			v1.SysctlsPodAnnotationKey:       "kernel.shmmni=32768,kernel.shmmax=1000000000",
+			v1.UnsafeSysctlsPodAnnotationKey: "knet.ipv4.route.min_pmtu=1000",
+		},
+		expectedSysctls: map[string]string{
+			"kernel.shmmni":            "32768",
+			"kernel.shmmax":            "1000000000",
+			"knet.ipv4.route.min_pmtu": "1000",
+		},
+	}, {
+		annotations: map[string]string{
+			v1.SysctlsPodAnnotationKey: "kernel.shmmni=32768,kernel.shmmax=1000000000",
+		},
+		expectedSysctls: map[string]string{
+			"kernel.shmmni": "32768",
+			"kernel.shmmax": "1000000000",
+		},
+	}, {
+		annotations: map[string]string{
+			v1.UnsafeSysctlsPodAnnotationKey: "knet.ipv4.route.min_pmtu=1000",
+		},
+		expectedSysctls: map[string]string{
+			"knet.ipv4.route.min_pmtu": "1000",
+		},
+	}}
+
+	for i, test := range tests {
+		actualSysctls, err := getSysctlsFromAnnotations(test.annotations)
+		assert.NoError(t, err, "TestCase[%d]", i)
+		assert.Len(t, actualSysctls, len(test.expectedSysctls), "TestCase[%d]", i)
+		assert.Equal(t, test.expectedSysctls, actualSysctls, "TestCase[%d]", i)
+	}
+}

pkg/kubelet/kuberuntime/kuberuntime_sandbox.go

@@ -116,18 +116,22 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxConfig(pod *v1.Pod, attemp
 		}
 
 	}
-
-	cgroupParent := m.runtimeHelper.GetPodCgroupParent(pod)
-	podSandboxConfig.Linux = m.generatePodSandboxLinuxConfig(pod, cgroupParent)
 	if len(portMappings) > 0 {
 		podSandboxConfig.PortMappings = portMappings
 	}
 
+	lc, err := m.generatePodSandboxLinuxConfig(pod)
+	if err != nil {
+		return nil, err
+	}
+	podSandboxConfig.Linux = lc
+
 	return podSandboxConfig, nil
 }
 
 // generatePodSandboxLinuxConfig generates LinuxPodSandboxConfig from v1.Pod.
-func (m *kubeGenericRuntimeManager) generatePodSandboxLinuxConfig(pod *v1.Pod, cgroupParent string) *runtimeapi.LinuxPodSandboxConfig {
+func (m *kubeGenericRuntimeManager) generatePodSandboxLinuxConfig(pod *v1.Pod) (*runtimeapi.LinuxPodSandboxConfig, error) {
+	cgroupParent := m.runtimeHelper.GetPodCgroupParent(pod)
 	lc := &runtimeapi.LinuxPodSandboxConfig{
 		CgroupParent: cgroupParent,
 		SecurityContext: &runtimeapi.LinuxSandboxSecurityContext{
@@ -135,6 +139,12 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxLinuxConfig(pod *v1.Pod, c
 		},
 	}
 
+	sysctls, err := getSysctlsFromAnnotations(pod.Annotations)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get sysctls from annotations %v for pod %q: %v", pod.Annotations, format.Pod(pod), err)
+	}
+	lc.Sysctls = sysctls
+
 	if pod.Spec.SecurityContext != nil {
 		sc := pod.Spec.SecurityContext
 		if sc.RunAsUser != nil {
@@ -167,7 +177,7 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxLinuxConfig(pod *v1.Pod, c
 		}
 	}
 
-	return lc
+	return lc, nil
 }
 
 // getKubeletSandboxes lists all (or just the running) sandboxes managed by kubelet.