Merge pull request #52842 from yanxuean/reduntdant-cgroups

Automatic merge from submit-queue (batch tested with PRs 50457, 55558, 53483, 55731, 52842). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. improve the logic setting cgroupparent in RunPodSandbox Signed-off-by: yanxuean <yan.xuean@zte.com.cn> **What this PR does / why we need it**: The setting of cgroupparent is too confused! The old logic is: 1. set CgroupParent correctly 2. reset CgroupParent incorrectly 3. set CgroupParent again (refer to #42055 ) The login is too confused, and It is sure that there are many people who drop in trap. We only need to set it in one place. kubernetes/pkg/kubelet/dockershim/docker_sandbox.go ``` func (ds *dockerService) makeSandboxDockerConfig(c *runtimeapi.PodSandboxConfig, image string) (*dockertypes.ContainerCreateConfig, error) { .... // Apply linux-specific options. if lc := c.GetLinux(); lc != nil { if err := ds.applySandboxLinuxOptions(hc, lc, createConfig, image, securityOptSep); err != nil { return nil, err } } // Apply resource options. setSandboxResources(hc) **<-- reset the CgroupParent incorrectly** // Apply cgroupsParent derived from the sandbox config. if lc := c.GetLinux(); lc != nil { // Apply Cgroup options. cgroupParent, err := ds.GenerateExpectedCgroupParent(lc.CgroupParent) if err != nil { return nil, fmt.Errorf("failed to generate cgroup parent in expected syntax for container %q: %v", c.Metadata.Name, err) } hc.CgroupParent = cgroupParent } ``` **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # **Special notes for your reviewer**: **Release note**: ```release-note NONE ```

Merge pull request #52842 from yanxuean/reduntdant-cgroups
02f803cc · Kubernetes Submit Queue · GitHub · cf5d4011 · 3f3dae56 · 02f803cc
Unverified Commit 02f803cc authored Nov 18, 2017 by Kubernetes Submit Queue Committed by GitHub Nov 18, 2017
Hide whitespace changes
Inline Side-by-side

Showing with 27 additions and 30 deletions

docker_sandbox.go pkg/kubelet/dockershim/docker_sandbox.go +27 -30

No files found.
--- a/pkg/kubelet/dockershim/docker_sandbox.go
+++ b/pkg/kubelet/dockershim/docker_sandbox.go
@@ -487,20 +487,34 @@ func (ds *dockerService) ListPodSandbox(filter *runtimeapi.PodSandboxFilter) ([]
 // applySandboxLinuxOptions applies LinuxPodSandboxConfig to dockercontainer.HostConfig and dockercontainer.ContainerCreateConfig.
 func (ds *dockerService) applySandboxLinuxOptions(hc *dockercontainer.HostConfig, lc *runtimeapi.LinuxPodSandboxConfig, createConfig *dockertypes.ContainerCreateConfig, image string, separator rune) error {
-	// Apply Cgroup options.
+	if lc == nil {
-	cgroupParent, err := ds.GenerateExpectedCgroupParent(lc.CgroupParent)
+		return nil
-	if err != nil {
-		return err
 	}
-	hc.CgroupParent = cgroupParent
 	// Apply security context.
-	if err = applySandboxSecurityContext(lc, createConfig.Config, hc, ds.network, separator); err != nil {
+	if err := applySandboxSecurityContext(lc, createConfig.Config, hc, ds.network, separator); err != nil {
 		return err
 	}
 	// Set sysctls.
 	hc.Sysctls = lc.Sysctls
+	return nil
+}
+func (ds *dockerService) applySandboxResources(hc *dockercontainer.HostConfig, lc *runtimeapi.LinuxPodSandboxConfig) error {
+	hc.Resources = dockercontainer.Resources{
+		MemorySwap: DefaultMemorySwap(),
+		CPUShares:  defaultSandboxCPUshares,
+		// Use docker's default cpu quota/period.
+	}
+	if lc != nil {
+		// Apply Cgroup options.
+		cgroupParent, err := ds.GenerateExpectedCgroupParent(lc.CgroupParent)
+		if err != nil {
+			return err
+		}
+		hc.CgroupParent = cgroupParent
+	}
 	return nil
 }
@@ -533,10 +547,8 @@ func (ds *dockerService) makeSandboxDockerConfig(c *runtimeapi.PodSandboxConfig,
 	}
 	// Apply linux-specific options.
-	if lc := c.GetLinux(); lc != nil {
+	if err := ds.applySandboxLinuxOptions(hc, c.GetLinux(), createConfig, image, securityOptSep); err != nil {
-		if err := ds.applySandboxLinuxOptions(hc, lc, createConfig, image, securityOptSep); err != nil {
+		return nil, err
-			return nil, err
-		}
 	}
 	// Set port mappings.
@@ -544,17 +556,12 @@ func (ds *dockerService) makeSandboxDockerConfig(c *runtimeapi.PodSandboxConfig,
 	createConfig.Config.ExposedPorts = exposedPorts
 	hc.PortBindings = portBindings
-	// Apply resource options.
+	// TODO: Get rid of the dependency on kubelet internal package.
-	setSandboxResources(hc)
+	hc.OomScoreAdj = qos.PodInfraOOMAdj
-	// Apply cgroupsParent derived from the sandbox config.
+	// Apply resource options.
-	if lc := c.GetLinux(); lc != nil {
+	if err := ds.applySandboxResources(hc, c.GetLinux()); err != nil {
-		// Apply Cgroup options.
+		return nil, err
-		cgroupParent, err := ds.GenerateExpectedCgroupParent(lc.CgroupParent)
-		if err != nil {
-			return nil, fmt.Errorf("failed to generate cgroup parent in expected syntax for container %q: %v", c.Metadata.Name, err)
-		}
-		hc.CgroupParent = cgroupParent
 	}
 	// Set security options.
@@ -593,16 +600,6 @@ func sharesHostIpc(container *dockertypes.ContainerJSON) bool {
 	return false
 }
-func setSandboxResources(hc *dockercontainer.HostConfig) {
-	hc.Resources = dockercontainer.Resources{
-		MemorySwap: DefaultMemorySwap(),
-		CPUShares:  defaultSandboxCPUshares,
-		// Use docker's default cpu quota/period.
-	}
-	// TODO: Get rid of the dependency on kubelet internal package.
-	hc.OomScoreAdj = qos.PodInfraOOMAdj
-}
 func constructPodSandboxCheckpoint(config *runtimeapi.PodSandboxConfig) *PodSandboxCheckpoint {
 	checkpoint := NewPodSandboxCheckpoint(config.Metadata.Namespace, config.Metadata.Name)
 	for _, pm := range config.GetPortMappings() {