sandbox: combine sandbox cgroup functions

Simplify the tests and the code by combining the create and join
functions into a single function.

Signed-off-by: Eric Ernst <[email protected]>

virtcontainers/api.go

@@ -77,13 +77,8 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f
 
 	// Move runtime to sandbox cgroup so all process are created there.
 	if s.config.SandboxCgroupOnly {
-		if err := s.setupSandboxCgroupOnly(); err != nil {
+		if err := s.setupSandboxCgroup(); err != nil {
 			return nil, err
-
-		}
-		if err := s.joinSandboxCgroup(); err != nil {
-			return nil, err
-
 		}
 	}
 

virtcontainers/sandbox.go

@@ -2080,28 +2080,31 @@ func (s *Sandbox) cpuResources() *specs.LinuxCPU {
 	return validCPUResources(cpu)
 }
 
-// setupSandboxCgroup creates sandbox cgroups for the sandbox config
-func (s *Sandbox) setupSandboxCgroupOnly() error {
-	var PodSandboxConfig *ContainerConfig
+// setupSandboxCgroup creates and joins sandbox cgroups for the sandbox config
+func (s *Sandbox) setupSandboxCgroup() error {
+	var podSandboxConfig *ContainerConfig
 
 	if s.config == nil {
 		return fmt.Errorf("Sandbox config is nil")
 	}
 
+	// get the container associated with the PodSandbox annotation. In Kubernetes, this
+	// represents the pause container. In Docker, this is the container. We derive the
+	// cgroup path from this container.
 	for _, cConfig := range s.config.Containers {
 		if cConfig.Annotations[annotations.ContainerTypeKey] == string(PodSandbox) {
-			PodSandboxConfig = &cConfig
+			podSandboxConfig = &cConfig
 			break
 		}
 	}
 
-	if PodSandboxConfig == nil {
-		return fmt.Errorf("Failed to find cgroup path for Sandbox: Container of type '%s' not found", PodSandbox)
+	if podSandboxConfig == nil {
+		return fmt.Errorf("Failed to find cgroup path for sandbox: Container of type '%s' not found", PodSandbox)
 	}
 
-	configJSON, ok := PodSandboxConfig.Annotations[annotations.ConfigJSONKey]
+	configJSON, ok := podSandboxConfig.Annotations[annotations.ConfigJSONKey]
 	if !ok {
-		return fmt.Errorf("Could not find json config in annotations for container '%s'", PodSandboxConfig.ID)
+		return fmt.Errorf("Could not find json config in annotations for container '%s'", podSandboxConfig.ID)
 	}
 
 	var spec specs.Spec
@@ -2110,41 +2113,25 @@ func (s *Sandbox) setupSandboxCgroupOnly() error {
 	}
 
 	if spec.Linux == nil {
-		// Cgroup path is optional, just skip the setup
+		// Cgroup path is optional, though expected. If not defined, skip the setup
+		s.Logger().WithField("sandboxid", podSandboxConfig.ID).Warning("no cgroup path provided for pod sandbox, not creating sandbox cgroup")
 		return nil
 	}
 	validContainerCgroup := utils.ValidCgroupPath(spec.Linux.CgroupsPath)
 
-	// Use the parent cgroup of the container sandbox as the sandbox cgroup
-
-	s.state.CgroupPath = filepath.Join(filepath.Dir(validContainerCgroup), cgroupKataPrefix+"_"+PodSandboxConfig.ID)
-	_, err := cgroupsNewFunc(cgroups.V1, cgroups.StaticPath(s.state.CgroupPath), &specs.LinuxResources{})
+	// Create a Kata sandbox cgroup with the cgroup of the sandbox container as the parent
+	s.state.CgroupPath = filepath.Join(filepath.Dir(validContainerCgroup), cgroupKataPrefix+"_"+podSandboxConfig.ID)
+	cgroup, err := cgroupsNewFunc(cgroups.V1, cgroups.StaticPath(s.state.CgroupPath), &specs.LinuxResources{})
 	if err != nil {
 		return fmt.Errorf("Could not create sandbox cgroup in %v: %v", s.state.CgroupPath, err)
 
 	}
 
-	return nil
-}
-
-// joinSandboxCgroup adds the runtime PID to the sandbox defined in sandboxes' CgroupPath
-func (s *Sandbox) joinSandboxCgroup() error {
-
-	if s.state.CgroupPath == "" {
-		// This is an optional value
-		return nil
-	}
-
-	cgroup, err := cgroupsLoadFunc(cgroups.V1, cgroups.StaticPath(s.state.CgroupPath))
-	if err != nil {
-		return fmt.Errorf("Could not load sandbox cgroup in %v: %v", s.state.CgroupPath, err)
-	}
-
-	s.Logger().WithField("cgroup:", s.state.CgroupPath).Debug("joining to sandbox cgroup")
+	// Add the runtime to the Kata sandbox cgroup
 	runtimePid := os.Getpid()
-
 	if err := cgroup.Add(cgroups.Process{Pid: runtimePid}); err != nil {
 		return fmt.Errorf("Could not add runtime PID %d to sandbox cgroup:  %v", runtimePid, err)
 	}
+
 	return nil
 }

virtcontainers/sandbox_test.go

@@ -1474,6 +1474,7 @@ func TestSandboxExperimentalFeature(t *testing.T) {
 	assert.True(t, sconfig.valid())
 }
 
+/*
 func TestSandbox_joinSandboxCgroup(t *testing.T) {
 
 	mockValidCgroup := &Sandbox{}
@@ -1495,8 +1496,9 @@ func TestSandbox_joinSandboxCgroup(t *testing.T) {
 		})
 	}
 }
+*/
 
-func TestSandbox_SetupSandboxCgroupOnly(t *testing.T) {
+func TestSandbox_SetupSandboxCgroup(t *testing.T) {
 	sandboxContainer := ContainerConfig{}
 	sandboxContainer.Annotations = make(map[string]string)
 	sandboxContainer.Annotations[annotations.ContainerTypeKey] = string(PodSandbox)
@@ -1561,7 +1563,7 @@ func TestSandbox_SetupSandboxCgroupOnly(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := tt.s.setupSandboxCgroupOnly(); (err != nil) != tt.wantErr {
+			if err := tt.s.setupSandboxCgroup(); (err != nil) != tt.wantErr {
 				t.Errorf("Sandbox.SetupSandboxCgroupOnly() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})