Skip to content

Remove detection for scope properties, which have always been broken #1978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mrunalp merged 1 commit into from
Feb 13, 2019
Merged

Remove detection for scope properties, which have always been broken #1978

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 10 additions & 62 deletions libcontainer/cgroups/systemd/apply_systemd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,11 @@ const (
)

var (
connLock sync.Mutex
theConn *systemdDbus.Conn
hasStartTransientUnit bool
hasStartTransientSliceUnit bool
hasTransientDefaultDependencies bool
hasDelegateScope bool
hasDelegateSlice bool
connLock sync.Mutex
theConn *systemdDbus.Conn
hasStartTransientUnit bool
hasStartTransientSliceUnit bool
hasDelegateSlice bool
)

func newProp(name string, units interface{}) systemdDbus.Property {
Expand Down Expand Up @@ -116,53 +114,6 @@ func UseSystemd() bool {
}
}

// Ensure the scope name we use doesn't exist. Use the Pid to
// avoid collisions between multiple libcontainer users on a
// single host.
scope := fmt.Sprintf("libcontainer-%d-systemd-test-default-dependencies.scope", os.Getpid())
testScopeExists := true
for i := 0; i <= testScopeWait; i++ {
if _, err := theConn.StopUnit(scope, "replace", nil); err != nil {
if dbusError, ok := err.(dbus.Error); ok {
if strings.Contains(dbusError.Name, "org.freedesktop.systemd1.NoSuchUnit") {
testScopeExists = false
break
}
}
}
time.Sleep(time.Millisecond)
}

// Bail out if we can't kill this scope without testing for DefaultDependencies
if testScopeExists {
return hasStartTransientUnit
}

// Assume StartTransientUnit on a scope allows DefaultDependencies
hasTransientDefaultDependencies = true
ddf := newProp("DefaultDependencies", false)
if _, err := theConn.StartTransientUnit(scope, "replace", []systemdDbus.Property{ddf}, nil); err != nil {
if dbusError, ok := err.(dbus.Error); ok {
if strings.Contains(dbusError.Name, "org.freedesktop.DBus.Error.PropertyReadOnly") {
hasTransientDefaultDependencies = false
}
}
}

// Not critical because of the stop unit logic above.
theConn.StopUnit(scope, "replace", nil)

// Assume StartTransientUnit on a scope allows Delegate
hasDelegateScope = true
dlScope := newProp("Delegate", true)
if _, err := theConn.StartTransientUnit(scope, "replace", []systemdDbus.Property{dlScope}, nil); err != nil {
if dbusError, ok := err.(dbus.Error); ok {
if strings.Contains(dbusError.Name, "org.freedesktop.DBus.Error.PropertyReadOnly") {
hasDelegateScope = false
}
}
}

// Assume we have the ability to start a transient unit as a slice
// This was broken until systemd v229, but has been back-ported on RHEL environments >= 219
// For details, see: https://bugzilla.redhat.com/show_bug.cgi?id=1370299
Expand Down Expand Up @@ -207,7 +158,6 @@ func UseSystemd() bool {
}

// Not critical because of the stop unit logic above.
theConn.StopUnit(scope, "replace", nil)
theConn.StopUnit(slice, "replace", nil)
}
return hasStartTransientUnit
Expand Down Expand Up @@ -268,9 +218,8 @@ func (m *Manager) Apply(pid int) error {
properties = append(properties, newProp("Delegate", true))
}
} else {
if hasDelegateScope {
properties = append(properties, newProp("Delegate", true))
}
// Assume scopes always support delegation.
properties = append(properties, newProp("Delegate", true))
}

// Always enable accounting, this gets us the same behaviour as the fs implementation,
Expand All @@ -280,10 +229,9 @@ func (m *Manager) Apply(pid int) error {
newProp("CPUAccounting", true),
newProp("BlockIOAccounting", true))

if hasTransientDefaultDependencies {
properties = append(properties,
newProp("DefaultDependencies", false))
}
// Assume DefaultDependencies= will always work (the check for it was previously broken.)
properties = append(properties,
newProp("DefaultDependencies", false))

if c.Resources.Memory != 0 {
properties = append(properties,
Expand Down