Skip to content

Commit

Permalink
runc run: refuse existing cgroup
Browse files Browse the repository at this point in the history
Currently runc allows multiple containers to share the same cgroup (for
example, by having the same cgroupPath in config.json). While such
shared configuration might be OK, there are some issues:

 - When each container has its own resource limits, the order of
   containers start determines whose limits will be effectively applied.

 - When one of containers is paused, all others are paused, too.

 - When a container is paused, any attempt to do runc create/run/exec
   end up with runc init stuck inside a frozen cgroup.

- When a systemd cgroup manager is used, this becomes even worse -- such
  as, stop (or even failed start) of any container results in
  "stopTransientUnit" command being sent to systemd, and so (depending on
  unit properties) other containers can receive SIGTERM, be killed after a
  timeout etc.

All this may lead to various hard-to-debug situations in production
(runc init stuck, cgroup removal error, wrong resource limits etc).

One obvious solution is to require a non-existent cgroup for the
new container, or fail if a cgroup already exists.

This is exactly what this commit implements.

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Aug 18, 2021
1 parent 556a4d4 commit 65faece
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 4 deletions.
1 change: 1 addition & 0 deletions libcontainer/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ var (
ErrRunning = errors.New("container still running")
ErrNotRunning = errors.New("container not running")
ErrNotPaused = errors.New("container not paused")
ErrCgExist = errors.New("container's cgroup already exists")
)

type ConfigError struct {
Expand Down
13 changes: 9 additions & 4 deletions libcontainer/factory_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,19 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
} else if !os.IsNotExist(err) {
return nil, err
}
if err := os.MkdirAll(containerRoot, 0o711); err != nil {

cm, err := manager.New(config.Cgroups)
if err != nil {
return nil, err
}
if err := os.Chown(containerRoot, unix.Geteuid(), unix.Getegid()); err != nil {
if cm.Exists() {
return nil, ErrCgExist
}

if err := os.MkdirAll(containerRoot, 0o711); err != nil {
return nil, err
}
cm, err := manager.New(config.Cgroups)
if err != nil {
if err := os.Chown(containerRoot, unix.Geteuid(), unix.Getegid()); err != nil {
return nil, err
}
c := &linuxContainer{
Expand Down
21 changes: 21 additions & 0 deletions tests/integration/cgroups.bats
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,24 @@ function setup() {
[ "$status" -eq 255 ]
[[ "$output" == *"cannot exec in a paused container"* ]]
}

@test "runc run/create should refuse an existing cgroup" {
if [[ "$ROOTLESS" -ne 0 ]]; then
requires rootless_cgroup
fi

set_cgroups_path

runc run -d --console-socket "$CONSOLE_SOCKET" ct1
[ "$status" -eq 0 ]

# Run a second container sharing the cgroup with the first one.
runc --debug run -d --console-socket "$CONSOLE_SOCKET" ct2
[ "$status" -ne 0 ]
[[ "$output" == *"container's cgroup already exists"* ]]

# Same but using runc create.
runc create --console-socket "$CONSOLE_SOCKET" ct3
[ "$status" -ne 0 ]
[[ "$output" == *"container's cgroup already exists"* ]]
}

0 comments on commit 65faece

Please sign in to comment.