Skip to content
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.

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

[1.0] libct/cg/sd/v1: fix freezeBeforeSet (alt 2) #3167

Merged
merged 3 commits into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
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 <odin@uged.al>
(cherry picked from commit 4104367)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
odinuge authored and kolyshkin committed Aug 18, 2021
commit 4ce440f22ae89cd28abd20f4dcb6e40ddee2ab7d
12 changes: 10 additions & 2 deletions libcontainer/cgroups/systemd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,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 {
Expand Down Expand Up @@ -388,10 +396,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
Expand Down
17 changes: 17 additions & 0 deletions libcontainer/cgroups/systemd/systemd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 13 additions & 7 deletions libcontainer/cgroups/systemd/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"os"
"path/filepath"
"reflect"
"strings"
"sync"

Expand Down Expand Up @@ -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
}
}
}
}
Expand Down