From cd41feb46b939b6179fbc3c388e33788f72ca38c Mon Sep 17 00:00:00 2001
From: Filipe Brandenburger <filbranden@google.com>
Date: Mon, 11 Feb 2019 16:05:37 -0800
Subject: [PATCH] Remove detection for scope properties, which have always been
 broken

The detection for scope properties (whether scope units support
DefaultDependencies= or Delegate=) has always been broken, since systemd
refuses to create scopes unless at least one PID is attached to it (and
this has been so since scope units were introduced in systemd v205.)

This can be seen in journal logs whenever a container is started with
libpod:

  Feb 11 15:08:07 myhost systemd[1]: libcontainer-12345-systemd-test-default-dependencies.scope: Scope has no PIDs. Refusing.
  Feb 11 15:08:07 myhost systemd[1]: libcontainer-12345-systemd-test-default-dependencies.scope: Scope has no PIDs. Refusing.

Since this logic never worked, just assume both attributes are supported
(which is what the code does when detection fails for this reason, since
it's looking for an "unknown attribute" or "read-only attribute" to mark
them as false) and skip the detection altogether.

Signed-off-by: Filipe Brandenburger <filbranden@google.com>
---
 libcontainer/cgroups/systemd/apply_systemd.go | 72 +++----------------
 1 file changed, 10 insertions(+), 62 deletions(-)

diff --git a/libcontainer/cgroups/systemd/apply_systemd.go b/libcontainer/cgroups/systemd/apply_systemd.go
index e8defe3a..a10e3f6a 100644
--- a/libcontainer/cgroups/systemd/apply_systemd.go
+++ b/libcontainer/cgroups/systemd/apply_systemd.go
@@ -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 {
@@ -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
@@ -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
@@ -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,
@@ -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,