Skip to content

Commit c23ac0d

Browse files
committed
libct/cg/sd: ignore UnitExists only for Apply(-1)
Commit 94efc45 ("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 <[email protected]>
1 parent d8f2b25 commit c23ac0d

File tree

3 files changed

+10
-4
lines changed

3 files changed

+10
-4
lines changed

libcontainer/cgroups/systemd/common.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func isUnitExists(err error) bool {
124124
return isDbusError(err, "org.freedesktop.systemd1.UnitExists")
125125
}
126126

127-
func startUnit(cm *dbusConnManager, unitName string, properties []systemdDbus.Property) error {
127+
func startUnit(cm *dbusConnManager, unitName string, properties []systemdDbus.Property, ignoreExist bool) error {
128128
statusChan := make(chan string, 1)
129129
err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) error {
130130
_, err := c.StartTransientUnitContext(context.TODO(), unitName, "replace", properties, statusChan)
@@ -134,7 +134,13 @@ func startUnit(cm *dbusConnManager, unitName string, properties []systemdDbus.Pr
134134
if !isUnitExists(err) {
135135
return err
136136
}
137-
return nil
137+
if ignoreExist {
138+
// TODO: remove this hack.
139+
// This is kubelet making sure a slice exists (see
140+
// https://github.com/opencontainers/runc/pull/1124).
141+
return nil
142+
}
143+
return err
138144
}
139145

140146
timeout := time.NewTimer(30 * time.Second)

libcontainer/cgroups/systemd/v1.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func (m *LegacyManager) Apply(pid int) error {
204204

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

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

libcontainer/cgroups/systemd/v2.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ func (m *UnifiedManager) Apply(pid int) error {
291291

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

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

0 commit comments

Comments
 (0)