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

libct/cg/sd/v1: add a knob to skip freeze on set #3161

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
9 changes: 0 additions & 9 deletions libcontainer/cgroups/systemd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,15 +389,6 @@ func resetFailedUnit(cm *dbusConnManager, name string) {
}
}

func getUnitProperty(cm *dbusConnManager, unitName 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)
return Err
})
return prop, err
}

func setUnitProperties(cm *dbusConnManager, name string, properties ...systemdDbus.Property) error {
return cm.retryOnDisconnect(func(c *systemdDbus.Conn) error {
return c.SetUnitPropertiesContext(context.TODO(), name, true, properties...)
Expand Down
28 changes: 5 additions & 23 deletions libcontainer/cgroups/systemd/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"sync"

systemdDbus "github.com/coreos/go-systemd/v22/dbus"
"github.com/godbus/dbus/v5"
"github.com/sirupsen/logrus"

"github.com/opencontainers/runc/libcontainer/cgroups"
Expand Down Expand Up @@ -342,28 +341,11 @@ func (m *legacyManager) GetStats() (*cgroups.Stats, error) {
// containers). So we have to freeze the container to avoid the container get
// an occasional "permission denied" error.
func (m *legacyManager) freezeBeforeSet(unitName string, r *configs.Resources) (needsFreeze, needsThaw bool, err error) {
// Special case for SkipDevices, as used by Kubernetes to create pod
// cgroups with allow-all device policy).
if r.SkipDevices {
// No need to freeze if SkipDevices is set, and either
// (1) systemd unit does not (yet) exist, or
// (2) it has DevicePolicy=auto and empty DeviceAllow list.
//
// Interestingly, (1) and (2) are the same here because
// a non-existent unit returns default properties,
// and settings in (2) are the defaults.
//
// Do not return errors from getUnitProperty, as they alone
// should not prevent Set from working.
devPolicy, e := getUnitProperty(m.dbus, unitName, "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
}
}
// Special case for Kubernetes (which sets pod slice resources
// for a slice that has no device rules set).
if r.SkipDevices && r.SkipFreezeOnSet {
// Both needsFreeze and needsThaw are false.
return
}

needsFreeze = true
Expand Down
7 changes: 7 additions & 0 deletions libcontainer/configs/cgroup_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,11 @@ type Resources struct {
//
// NOTE it is impossible to start a container which has this flag set.
SkipDevices bool `json:"-"`

// SkipFreezeOnSet is a flag for cgroup manager to skip the cgroup
// freeze when setting resources. Only applicable to systemd legacy
// (i.e. cgroup v1) manager (which uses freeze by default to avoid
// spurious permission errors caused by systemd inability to update
// device rules in a non-disruptive manner).
SkipFreezeOnSet bool `json:"-"`
}