Skip to content

Commit da9b846

Browse files
committed
libct/cg: add Resources=nil unit test
Cgroup controllers should never panic, and yet sometimes they do. Add a unit test to check that controllers never panic when called with nil arguments and/or resources, and fix a few found cases. Signed-off-by: Kir Kolyshkin <[email protected]>
1 parent cc4c822 commit da9b846

File tree

4 files changed

+57
-0
lines changed

4 files changed

+57
-0
lines changed

libcontainer/cgroups/fs2/fs2.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
129129
}
130130

131131
func (m *manager) Freeze(state configs.FreezerState) error {
132+
if m.config.Resources == nil {
133+
return errors.New("cannot toggle freezer: cgroups not configured for container")
134+
}
132135
if err := setFreezer(m.dirPath, state); err != nil {
133136
return err
134137
}
@@ -145,6 +148,9 @@ func (m *manager) Path(_ string) string {
145148
}
146149

147150
func (m *manager) Set(r *configs.Resources) error {
151+
if r == nil {
152+
return nil
153+
}
148154
if err := m.getControllers(); err != nil {
149155
return err
150156
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package manager
2+
3+
import (
4+
"testing"
5+
6+
"github.com/opencontainers/runc/libcontainer/configs"
7+
)
8+
9+
// TestNilResources checks that a cgroup manager do not panic when
10+
// config.Resources is nil. While it does not make sense to use a
11+
// manager with no resources, it should not result in a panic.
12+
//
13+
// This tests either v1 or v2 managers (both fs and systemd)
14+
// depending on what cgroup version is available on the host.
15+
16+
func TestNilResources(t *testing.T) {
17+
for _, sd := range []bool{false, true} {
18+
cg := &configs.Cgroup{} // .Resources is nil
19+
cg.Systemd = sd
20+
mgr, err := New(cg)
21+
if err != nil {
22+
// Some managers require non-nil Resources during
23+
// instantiation -- provide and retry. In such case
24+
// we're mostly testing Set(nil) below.
25+
cg.Resources = &configs.Resources{}
26+
mgr, err = New(cg)
27+
if err != nil {
28+
t.Error(err)
29+
continue
30+
}
31+
}
32+
_ = mgr.Apply(-1)
33+
_ = mgr.Set(nil)
34+
_ = mgr.Freeze(configs.Thawed)
35+
_ = mgr.Exists()
36+
_, _ = mgr.GetAllPids()
37+
_, _ = mgr.GetCgroups()
38+
_, _ = mgr.GetFreezerState()
39+
_ = mgr.Path("")
40+
_ = mgr.GetPaths()
41+
_, _ = mgr.GetStats()
42+
_, _ = mgr.OOMKillCount()
43+
_ = mgr.Destroy()
44+
}
45+
}

libcontainer/cgroups/systemd/v1.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,9 @@ func (m *legacyManager) freezeBeforeSet(unitName string, r *configs.Resources) (
389389
}
390390

391391
func (m *legacyManager) Set(r *configs.Resources) error {
392+
if r == nil {
393+
return nil
394+
}
392395
if r.Unified != nil {
393396
return cgroups.ErrV1NoUnified
394397
}

libcontainer/cgroups/systemd/v2.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,9 @@ func (m *unifiedManager) GetStats() (*cgroups.Stats, error) {
384384
}
385385

386386
func (m *unifiedManager) Set(r *configs.Resources) error {
387+
if r == nil {
388+
return nil
389+
}
387390
properties, err := genV2ResourcesProperties(r, m.dbus)
388391
if err != nil {
389392
return err

0 commit comments

Comments
 (0)