Skip to content

Commit e3761ea

Browse files
runcomruncom
committed
Revert "Backport of cgroups fixes"
This reverts commit 613d265.
1 parent a9d1096 commit e3761ea

File tree

2 files changed

+44
-197
lines changed

2 files changed

+44
-197
lines changed

‎libcontainer/cgroups/systemd/apply_systemd.go

Lines changed: 42 additions & 171 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,22 @@
1-
// +build linux,!static_build
1+
// +build linux
22

33
package systemd
44

55
import (
66
"errors"
77
"fmt"
8+
"io/ioutil"
89
"os"
910
"path/filepath"
1011
"strings"
1112
"sync"
12-
"time"
1313

1414
systemdDbus "github.com/coreos/go-systemd/dbus"
1515
systemdUtil "github.com/coreos/go-systemd/util"
1616
"github.com/godbus/dbus"
1717
"github.com/opencontainers/runc/libcontainer/cgroups"
1818
"github.com/opencontainers/runc/libcontainer/cgroups/fs"
1919
"github.com/opencontainers/runc/libcontainer/configs"
20-
"github.com/Sirupsen/logrus"
2120
)
2221

2322
type Manager struct {
@@ -70,13 +69,8 @@ const (
7069
)
7170

7271
var (
73-
connLock sync.Mutex
74-
theConn *systemdDbus.Conn
75-
hasStartTransientUnit bool
76-
hasStartTransientSliceUnit bool
77-
hasTransientDefaultDependencies bool
78-
hasDelegateScope bool
79-
hasDelegateSlice bool
72+
connLock sync.Mutex
73+
theConn *systemdDbus.Conn
8074
)
8175

8276
func newProp(name string, units interface{}) systemdDbus.Property {
@@ -87,128 +81,7 @@ func newProp(name string, units interface{}) systemdDbus.Property {
8781
}
8882

8983
func UseSystemd() bool {
90-
if !systemdUtil.IsRunningSystemd() {
91-
return false
92-
}
93-
94-
connLock.Lock()
95-
defer connLock.Unlock()
96-
97-
if theConn == nil {
98-
var err error
99-
theConn, err = systemdDbus.New()
100-
if err != nil {
101-
return false
102-
}
103-
104-
// Assume we have StartTransientUnit
105-
hasStartTransientUnit = true
106-
107-
// But if we get UnknownMethod error we don't
108-
if _, err := theConn.StartTransientUnit("test.scope", "invalid", nil, nil); err != nil {
109-
if dbusError, ok := err.(dbus.Error); ok {
110-
if dbusError.Name == "org.freedesktop.DBus.Error.UnknownMethod" {
111-
hasStartTransientUnit = false
112-
return hasStartTransientUnit
113-
}
114-
}
115-
}
116-
117-
// Ensure the scope name we use doesn't exist. Use the Pid to
118-
// avoid collisions between multiple libcontainer users on a
119-
// single host.
120-
scope := fmt.Sprintf("libcontainer-%d-systemd-test-default-dependencies.scope", os.Getpid())
121-
testScopeExists := true
122-
for i := 0; i <= testScopeWait; i++ {
123-
if _, err := theConn.StopUnit(scope, "replace", nil); err != nil {
124-
if dbusError, ok := err.(dbus.Error); ok {
125-
if strings.Contains(dbusError.Name, "org.freedesktop.systemd1.NoSuchUnit") {
126-
testScopeExists = false
127-
break
128-
}
129-
}
130-
}
131-
time.Sleep(time.Millisecond)
132-
}
133-
134-
// Bail out if we can't kill this scope without testing for DefaultDependencies
135-
if testScopeExists {
136-
return hasStartTransientUnit
137-
}
138-
139-
// Assume StartTransientUnit on a scope allows DefaultDependencies
140-
hasTransientDefaultDependencies = true
141-
ddf := newProp("DefaultDependencies", false)
142-
if _, err := theConn.StartTransientUnit(scope, "replace", []systemdDbus.Property{ddf}, nil); err != nil {
143-
if dbusError, ok := err.(dbus.Error); ok {
144-
if strings.Contains(dbusError.Name, "org.freedesktop.DBus.Error.PropertyReadOnly") {
145-
hasTransientDefaultDependencies = false
146-
}
147-
}
148-
}
149-
150-
// Not critical because of the stop unit logic above.
151-
theConn.StopUnit(scope, "replace", nil)
152-
153-
// Assume StartTransientUnit on a scope allows Delegate
154-
hasDelegateScope = true
155-
dlScope := newProp("Delegate", true)
156-
if _, err := theConn.StartTransientUnit(scope, "replace", []systemdDbus.Property{dlScope}, nil); err != nil {
157-
if dbusError, ok := err.(dbus.Error); ok {
158-
if strings.Contains(dbusError.Name, "org.freedesktop.DBus.Error.PropertyReadOnly") {
159-
hasDelegateScope = false
160-
}
161-
}
162-
}
163-
164-
// Assume we have the ability to start a transient unit as a slice
165-
// This was broken until systemd v229, but has been back-ported on RHEL environments >= 219
166-
// For details, see: https://bugzilla.redhat.com/show_bug.cgi?id=1370299
167-
hasStartTransientSliceUnit = true
168-
169-
// To ensure simple clean-up, we create a slice off the root with no hierarchy
170-
slice := fmt.Sprintf("libcontainer_%d_systemd_test_default.slice", os.Getpid())
171-
if _, err := theConn.StartTransientUnit(slice, "replace", nil, nil); err != nil {
172-
if _, ok := err.(dbus.Error); ok {
173-
hasStartTransientSliceUnit = false
174-
}
175-
}
176-
177-
for i := 0; i <= testSliceWait; i++ {
178-
if _, err := theConn.StopUnit(slice, "replace", nil); err != nil {
179-
if dbusError, ok := err.(dbus.Error); ok {
180-
if strings.Contains(dbusError.Name, "org.freedesktop.systemd1.NoSuchUnit") {
181-
hasStartTransientSliceUnit = false
182-
break
183-
}
184-
}
185-
} else {
186-
break
187-
}
188-
time.Sleep(time.Millisecond)
189-
}
190-
191-
// Not critical because of the stop unit logic above.
192-
theConn.StopUnit(slice, "replace", nil)
193-
194-
// Assume StartTransientUnit on a slice allows Delegate
195-
hasDelegateSlice = true
196-
dlSlice := newProp("Delegate", true)
197-
if _, err := theConn.StartTransientUnit(slice, "replace", []systemdDbus.Property{dlSlice}, nil); err != nil {
198-
if dbusError, ok := err.(dbus.Error); ok {
199-
// Starting with systemd v237, Delegate is not even a property of slices anymore,
200-
// so the D-Bus call fails with "InvalidArgs" error.
201-
if strings.Contains(dbusError.Name, "org.freedesktop.DBus.Error.PropertyReadOnly") || strings.Contains(dbusError.Name, "org.freedesktop.DBus.Error.InvalidArgs") {
202-
hasDelegateSlice = false
203-
}
204-
}
205-
}
206-
207-
// Not critical because of the stop unit logic above.
208-
theConn.StopUnit(scope, "replace", nil)
209-
theConn.StopUnit(slice, "replace", nil)
210-
}
211-
return hasStartTransientUnit
84+
return systemdUtil.IsRunningSystemd()
21285
}
21386

21487
func (m *Manager) Apply(pid int) error {
@@ -244,10 +117,6 @@ func (m *Manager) Apply(pid int) error {
244117

245118
// if we create a slice, the parent is defined via a Wants=
246119
if strings.HasSuffix(unitName, ".slice") {
247-
// This was broken until systemd v229, but has been back-ported on RHEL environments >= 219
248-
if !hasStartTransientSliceUnit {
249-
return fmt.Errorf("systemd version does not support ability to start a slice as transient unit")
250-
}
251120
properties = append(properties, systemdDbus.PropWants(slice))
252121
} else {
253122
// otherwise, we use Slice=
@@ -259,17 +128,8 @@ func (m *Manager) Apply(pid int) error {
259128
properties = append(properties, newProp("PIDs", []uint32{uint32(pid)}))
260129
}
261130

262-
// Check if we can delegate. This is only supported on systemd versions 218 and above.
263-
if strings.HasSuffix(unitName, ".slice") {
264-
if hasDelegateSlice {
265-
// systemd 237 and above no longer allows delegation on a slice
266-
properties = append(properties, newProp("Delegate", true))
267-
}
268-
} else {
269-
if hasDelegateScope {
270-
properties = append(properties, newProp("Delegate", true))
271-
}
272-
}
131+
// This is only supported on systemd versions 218 and above.
132+
properties = append(properties, newProp("Delegate", true))
273133

274134
// Always enable accounting, this gets us the same behaviour as the fs implementation,
275135
// plus the kernel has some problems with joining the memory cgroup at a later time.
@@ -278,10 +138,7 @@ func (m *Manager) Apply(pid int) error {
278138
newProp("CPUAccounting", true),
279139
newProp("BlockIOAccounting", true))
280140

281-
if hasTransientDefaultDependencies {
282-
properties = append(properties,
283-
newProp("DefaultDependencies", false))
284-
}
141+
properties = append(properties, newProp("DefaultDependencies", false))
285142

286143
if c.Resources.Memory != 0 {
287144
properties = append(properties,
@@ -290,21 +147,14 @@ func (m *Manager) Apply(pid int) error {
290147

291148
if c.Resources.CpuShares != 0 {
292149
properties = append(properties,
293-
newProp("CPUShares", c.Resources.CpuShares))
150+
newProp("CPUShares", uint64(c.Resources.CpuShares)))
294151
}
295152

296153
// cpu.cfs_quota_us and cpu.cfs_period_us are controlled by systemd.
297154
if c.Resources.CpuQuota != 0 && c.Resources.CpuPeriod != 0 {
298-
cpuQuotaPerSecUSec := c.Resources.CpuQuota*1000000 / c.Resources.CpuPeriod
299-
// systemd converts CPUQuotaPerSecUSec (microseconds per CPU second) to CPUQuota
300-
// (integer percentage of CPU) internally. This means that if a fractional percent of
301-
// CPU is indicated by Resources.CpuQuota, we need to round up to the nearest
302-
// 10ms (1% of a second) such that child cgroups can set the cpu.cfs_quota_us they expect.
303-
if cpuQuotaPerSecUSec%10000 != 0 {
304-
cpuQuotaPerSecUSec = ((cpuQuotaPerSecUSec / 10000) + 1) * 10000
305-
}
155+
cpuQuotaPerSecUSec := c.Resources.CpuQuota * 1000000 / c.Resources.CpuPeriod
306156
properties = append(properties,
307-
newProp("CPUQuotaPerSecUSec", cpuQuotaPerSecUSec))
157+
newProp("CPUQuotaPerSecUSec", uint64(cpuQuotaPerSecUSec)))
308158
}
309159

310160
if c.Resources.BlkioWeight != 0 {
@@ -320,16 +170,17 @@ func (m *Manager) Apply(pid int) error {
320170
}
321171
}
322172

173+
var err error
174+
theConn, err = systemdDbus.New()
175+
if err != nil {
176+
return err
177+
}
178+
323179
statusChan := make(chan string)
324-
if _, err := theConn.StartTransientUnit(unitName, "replace", properties, statusChan); err == nil {
325-
select {
326-
case <-statusChan:
327-
case <-time.After(time.Second):
328-
logrus.Warnf("Timed out while waiting for StartTransientUnit(%s) completion signal from dbus. Continuing...", unitName)
329-
}
330-
} else if !isUnitExists(err) {
180+
if _, err := theConn.StartTransientUnit(unitName, "replace", properties, statusChan); err != nil && !isUnitExists(err) {
331181
return err
332182
}
183+
<-statusChan
333184

334185
if err := joinCgroups(c, pid); err != nil {
335186
return err
@@ -357,6 +208,14 @@ func (m *Manager) Destroy() error {
357208
}
358209
m.mu.Lock()
359210
defer m.mu.Unlock()
211+
212+
if theConn == nil {
213+
var err error
214+
theConn, err = systemdDbus.New()
215+
if err != nil {
216+
return err
217+
}
218+
}
360219
theConn.StopUnit(getUnitName(m.Cgroups), "replace", nil)
361220
if err := cgroups.RemovePaths(m.Paths); err != nil {
362221
return err
@@ -372,6 +231,15 @@ func (m *Manager) GetPaths() map[string]string {
372231
return paths
373232
}
374233

234+
func writeFile(dir, file, data string) error {
235+
// Normally dir should not be empty, one case is that cgroup subsystem
236+
// is not mounted, we will get empty dir, and we want it fail here.
237+
if dir == "" {
238+
return fmt.Errorf("no such directory for %s", file)
239+
}
240+
return ioutil.WriteFile(filepath.Join(dir, file), []byte(data), 0700)
241+
}
242+
375243
func join(c *configs.Cgroup, subsystem string, pid int) (string, error) {
376244
path, err := getSubsystemPath(c, subsystem)
377245
if err != nil {
@@ -392,6 +260,7 @@ func joinCgroups(c *configs.Cgroup, pid int) error {
392260
switch name {
393261
case "name=systemd":
394262
// let systemd handle this
263+
break
395264
case "cpuset":
396265
path, err := getSubsystemPath(c, name)
397266
if err != nil && !cgroups.IsNotFound(err) {
@@ -401,6 +270,7 @@ func joinCgroups(c *configs.Cgroup, pid int) error {
401270
if err := s.ApplyDir(path, c, pid); err != nil {
402271
return err
403272
}
273+
break
404274
default:
405275
_, err := join(c, name, pid)
406276
if err != nil {
@@ -424,7 +294,7 @@ func joinCgroups(c *configs.Cgroup, pid int) error {
424294

425295
// systemd represents slice hierarchy using `-`, so we need to follow suit when
426296
// generating the path of slice. Essentially, test-a-b.slice becomes
427-
// /test.slice/test-a.slice/test-a-b.slice.
297+
// test.slice/test-a.slice/test-a-b.slice.
428298
func ExpandSlice(slice string) (string, error) {
429299
suffix := ".slice"
430300
// Name has to end with ".slice", but can't be just ".slice".
@@ -450,9 +320,10 @@ func ExpandSlice(slice string) (string, error) {
450320
}
451321

452322
// Append the component to the path and to the prefix.
453-
path += "/" + prefix + component + suffix
323+
path += prefix + component + suffix + "/"
454324
prefix += component + "-"
455325
}
326+
456327
return path, nil
457328
}
458329

@@ -462,7 +333,7 @@ func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) {
462333
return "", err
463334
}
464335

465-
initPath, err := cgroups.GetInitCgroup(subsystem)
336+
initPath, err := cgroups.GetInitCgroupDir(subsystem)
466337
if err != nil {
467338
return "", err
468339
}

‎libcontainer/cgroups/utils.go

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,8 @@ func GetThisCgroupDir(subsystem string) (string, error) {
235235
return getControllerPath(subsystem, cgroups)
236236
}
237237

238-
func GetInitCgroup(subsystem string) (string, error) {
238+
func GetInitCgroupDir(subsystem string) (string, error) {
239+
239240
cgroups, err := ParseCgroupFile("/proc/1/cgroup")
240241
if err != nil {
241242
return "", err
@@ -244,31 +245,6 @@ func GetInitCgroup(subsystem string) (string, error) {
244245
return getControllerPath(subsystem, cgroups)
245246
}
246247

247-
func GetInitCgroupPath(subsystem string) (string, error) {
248-
cgroup, err := GetInitCgroup(subsystem)
249-
if err != nil {
250-
return "", err
251-
}
252-
253-
return getCgroupPathHelper(subsystem, cgroup)
254-
}
255-
256-
func getCgroupPathHelper(subsystem, cgroup string) (string, error) {
257-
mnt, root, err := FindCgroupMountpointAndRoot(subsystem)
258-
if err != nil {
259-
return "", err
260-
}
261-
262-
// This is needed for nested containers, because in /proc/self/cgroup we
263-
// see pathes from host, which don't exist in container.
264-
relCgroup, err := filepath.Rel(root, cgroup)
265-
if err != nil {
266-
return "", err
267-
}
268-
269-
return filepath.Join(mnt, relCgroup), nil
270-
}
271-
272248
func readProcsFile(dir string) ([]int, error) {
273249
f, err := os.Open(filepath.Join(dir, CgroupProcesses))
274250
if err != nil {

0 commit comments

Comments
 (0)