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 33d484a ("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/runc#1124

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit c253342)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed Apr 3, 2023
1 parent 6c831ec commit e0dd6fb
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 @@ -353,7 +353,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 @@ -363,7 +363,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 @@ -206,7 +206,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 @@ -284,7 +284,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 e0dd6fb

Please sign in to comment.