libct/cg/fs: move paths init to NewManager

1. Separate path initialization logic from Apply to initPaths,
   and call initPaths from NewManager, so:
   - we can error out early (in NewManager rather than Apply);
   - always have m.paths available (e.g. in Destroy or Exists).
   - do not unnecessarily call subsysPath from Apply in case
     the paths were already provided.

2. Add a check for non-nil cgroups.Resources to NewManager,
   since initPaths, as well as some controller's Apply methods,
   need it.

3. Move cgroups.Resources.Unified check from Apply to NewManager,
   so we can error out early (same check exists in Set).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin
2021-08-10 17:02:42 -07:00
parent 097c6d7425
commit 6c5441e5cb
3 changed files with 58 additions and 25 deletions

View File

@@ -55,6 +55,23 @@ type manager struct {
}
func NewManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, error) {
// Some v1 controllers (cpu, cpuset, and devices) expect
// cgroups.Resources to not be nil in Apply.
if cg.Resources == nil {
return nil, errors.New("cgroup v1 manager needs configs.Resources to be set during manager creation")
}
if cg.Resources.Unified != nil {
return nil, cgroups.ErrV1NoUnified
}
if paths == nil {
var err error
paths, err = initPaths(cg)
if err != nil {
return nil, err
}
}
return &manager{
cgroups: cg,
paths: paths,
@@ -90,37 +107,21 @@ func (m *manager) Apply(pid int) (err error) {
defer m.mu.Unlock()
c := m.cgroups
if c.Resources.Unified != nil {
return cgroups.ErrV1NoUnified
}
root, err := rootPath()
if err != nil {
return err
}
inner, err := innerPath(c)
m.paths = make(map[string]string)
for _, sys := range subsystems {
p, err := subsysPath(root, inner, sys.Name())
if err != nil {
// The non-presence of the devices subsystem is
// considered fatal for security reasons.
if cgroups.IsNotFound(err) && (c.SkipDevices || sys.Name() != "devices") {
name := sys.Name()
p, ok := m.paths[name]
if !ok {
continue
}
return err
}
m.paths[sys.Name()] = p
if err := sys.Apply(p, c.Resources, pid); err != nil {
// In the case of rootless (including euid=0 in userns), where an
// explicit cgroup path hasn't been set, we don't bail on error in
// case of permission problems. Cases where limits have been set
// (and we couldn't create our own cgroup) are handled by Set.
if isIgnorableError(c.Rootless, err) && m.cgroups.Path == "" {
delete(m.paths, sys.Name())
if isIgnorableError(c.Rootless, err) && c.Path == "" {
delete(m.paths, name)
continue
}
return err

View File

@@ -21,6 +21,36 @@ var (
const defaultCgroupRoot = "/sys/fs/cgroup"
func initPaths(cg *configs.Cgroup) (map[string]string, error) {
root, err := rootPath()
if err != nil {
return nil, err
}
inner, err := innerPath(cg)
if err != nil {
return nil, err
}
paths := make(map[string]string)
for _, sys := range subsystems {
name := sys.Name()
path, err := subsysPath(root, inner, name)
if err != nil {
// The non-presence of the devices subsystem
// is considered fatal for security reasons.
if cgroups.IsNotFound(err) && (cg.SkipDevices || name != "devices") {
continue
}
return nil, err
}
paths[name] = path
}
return paths, nil
}
func tryDefaultCgroupRoot() string {
var st, pst unix.Stat_t

View File

@@ -140,7 +140,9 @@ func TestFactoryLoadContainer(t *testing.T) {
expectedConfig = &configs.Config{
Rootfs: "/mycontainer/root",
Hooks: expectedHooks,
Cgroups: &configs.Cgroup{},
Cgroups: &configs.Cgroup{
Resources: &configs.Resources{},
},
}
expectedState = &State{
BaseState: BaseState{