diff --git a/virtcontainers/api.go b/virtcontainers/api.go index a5d33fe7ac..769ae660f1 100644 --- a/virtcontainers/api.go +++ b/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 - } } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 2a2850eb55..96ba4b441a 100644 --- a/virtcontainers/sandbox.go +++ b/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 } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 04ee10f258..920af8ed0f 100644 --- a/virtcontainers/sandbox_test.go +++ b/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) } })