libct/cg/sd/v1: Fix unnecessary freeze/thaw

This fixes the behavior intended to avoid freezing containers/control
groups without it being necessary. This is important for end users of
libcontainer who rely on the behavior of no freeze.

The previous implementation would always get error trying to get
DevicePolicy from the Unit via dbus, since the Unit interface doesn't
contain DevicePolicy.

Signed-off-by: Odin Ugedal <[email protected]>

Author: odinuge

libcontainer/cgroups/systemd/common.go

@@ -311,6 +311,14 @@ func getUnitName(c *configs.Cgroup) string {
 	return c.Name
 }
 
+// This code should be in sync with getUnitName.
+func getUnitType(unitName string) string {
+	if strings.HasSuffix(unitName, ".slice") {
+		return "Slice"
+	}
+	return "Scope"
+}
+
 // isDbusError returns true if the error is a specific dbus error.
 func isDbusError(err error, name string) bool {
 	if err != nil {
@@ -389,10 +397,10 @@ func resetFailedUnit(cm *dbusConnManager, name string) {
 	}
 }
 
-func getUnitProperty(cm *dbusConnManager, unitName string, propertyName string) (*systemdDbus.Property, error) {
+func getUnitTypeProperty(cm *dbusConnManager, unitName string, unitType string, propertyName string) (*systemdDbus.Property, error) {
 	var prop *systemdDbus.Property
 	err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) (Err error) {
-		prop, Err = c.GetUnitPropertyContext(context.TODO(), unitName, propertyName)
+		prop, Err = c.GetUnitTypePropertyContext(context.TODO(), unitName, unitType, propertyName)
 		return Err
 	})
 	return prop, err

libcontainer/cgroups/systemd/systemd_test.go

@@ -40,6 +40,23 @@ func TestSystemdVersion(t *testing.T) {
 	}
 }
 
+func TestValidUnitTypes(t *testing.T) {
+	testCases := []struct {
+		unitName         string
+		expectedUnitType string
+	}{
+		{"system.slice", "Slice"},
+		{"kubepods.slice", "Slice"},
+		{"testing-container:ab.scope", "Scope"},
+	}
+	for _, sdTest := range testCases {
+		unitType := getUnitType(sdTest.unitName)
+		if unitType != sdTest.expectedUnitType {
+			t.Errorf("getUnitType(%s); want %q; got %q", sdTest.unitName, sdTest.expectedUnitType, unitType)
+		}
+	}
+}
+
 func newManager(config *configs.Cgroup) cgroups.Manager {
 	if cgroups.IsCgroup2UnifiedMode() {
 		return NewUnifiedManager(config, "", false)

libcontainer/cgroups/systemd/v1.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"sync"
 
@@ -353,15 +354,20 @@ func (m *legacyManager) freezeBeforeSet(unitName string, r *configs.Resources) (
 		// a non-existent unit returns default properties,
 		// and settings in (2) are the defaults.
 		//
-		// Do not return errors from getUnitProperty, as they alone
+		// Do not return errors from getUnitTypeProperty, as they alone
 		// should not prevent Set from working.
-		devPolicy, e := getUnitProperty(m.dbus, unitName, "DevicePolicy")
+
+		unitType := getUnitType(unitName)
+
+		devPolicy, e := getUnitTypeProperty(m.dbus, unitName, unitType, "DevicePolicy")
 		if e == nil && devPolicy.Value == dbus.MakeVariant("auto") {
-			devAllow, e := getUnitProperty(m.dbus, unitName, "DeviceAllow")
-			if e == nil && devAllow.Value == dbus.MakeVariant([]deviceAllowEntry{}) {
-				needsFreeze = false
-				needsThaw = false
-				return
+			devAllow, e := getUnitTypeProperty(m.dbus, unitName, unitType, "DeviceAllow")
+			if e == nil {
+				if rv := reflect.ValueOf(devAllow.Value.Value()); rv.Kind() == reflect.Slice && rv.Len() == 0 {
+					needsFreeze = false
+					needsThaw = false
+					return
+				}
 			}
 		}
 	}