Skip to content

Commit

Permalink
libct/cg/sd: ignore UnitExists only for Apply(-1)
Browse files Browse the repository at this point in the history
Commit d223e2a ("Ignore error when starting transient unit
that already exists" modified the code handling errors from startUnit
to ignore UnitExists error.

Apparently it was done so that kubelet can create the same pod slice
over and over without hitting an error (see [1]).

While it works for a pod slice to ensure it exists, it is a gross bug
to ignore UnitExists when creating a container. In this case, the
container init PID won't be added to the systemd unit (and to the
required cgroup), and as a result the container will successfully
run in a current user cgroup, without any cgroup limits applied.

So, fix the code to only ignore UnitExists if we're not adding a process
to the systemd unit. This way, kubelet will keep working as is, but
runc will refuse to create containers which are not placed into a
requested cgroup.

[1] opencontainers#1124

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed Mar 23, 2023
1 parent f248d97 commit 37083ac
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 4 deletions.
10 changes: 8 additions & 2 deletions libcontainer/cgroups/systemd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func isUnitExists(err error) bool {
return isDbusError(err, "org.freedesktop.systemd1.UnitExists")
}

func startUnit(cm *dbusConnManager, unitName string, properties []systemdDbus.Property) error {
func startUnit(cm *dbusConnManager, unitName string, properties []systemdDbus.Property, ignoreExist bool) error {
statusChan := make(chan string, 1)
err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) error {
_, err := c.StartTransientUnitContext(context.TODO(), unitName, "replace", properties, statusChan)
Expand All @@ -134,7 +134,13 @@ func startUnit(cm *dbusConnManager, unitName string, properties []systemdDbus.Pr
if !isUnitExists(err) {
return err
}
return nil
if ignoreExist {
// TODO: remove this hack.
// This is kubelet making sure a slice exists (see
// https://github.com/opencontainers/runc/pull/1124).
return nil
}
return err
}

timeout := time.NewTimer(30 * time.Second)
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/systemd/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (m *LegacyManager) Apply(pid int) error {

properties = append(properties, c.SystemdProps...)

if err := startUnit(m.dbus, unitName, properties); err != nil {
if err := startUnit(m.dbus, unitName, properties, pid == -1); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/systemd/v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ func (m *UnifiedManager) Apply(pid int) error {

properties = append(properties, c.SystemdProps...)

if err := startUnit(m.dbus, unitName, properties); err != nil {
if err := startUnit(m.dbus, unitName, properties, pid == -1); err != nil {
return fmt.Errorf("unable to start unit %q (properties %+v): %w", unitName, properties, err)
}

Expand Down

0 comments on commit 37083ac

Please sign in to comment.