-
Notifications
You must be signed in to change notification settings - Fork 241
Check for non-active/supported cgroups #77
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,24 +30,49 @@ import ( | |
) | ||
|
||
// New returns a new control via the cgroup cgroups interface | ||
func New(hierarchy Hierarchy, path Path, resources *specs.LinuxResources) (Cgroup, error) { | ||
func New(hierarchy Hierarchy, path Path, resources *specs.LinuxResources, opts ...InitOpts) (Cgroup, error) { | ||
config := newInitConfig() | ||
for _, o := range opts { | ||
if err := o(config); err != nil { | ||
return nil, err | ||
} | ||
} | ||
subsystems, err := hierarchy() | ||
if err != nil { | ||
return nil, err | ||
} | ||
var active []Subsystem | ||
for _, s := range subsystems { | ||
// check if subsystem exists | ||
if err := initializeSubsystem(s, path, resources); err != nil { | ||
if err == ErrControllerNotActive { | ||
if config.InitCheck != nil { | ||
if skerr := config.InitCheck(s, path, err); skerr != nil { | ||
if skerr != ErrIgnoreSubsystem { | ||
return nil, skerr | ||
} | ||
} | ||
} | ||
continue | ||
} | ||
return nil, err | ||
} | ||
active = append(active, s) | ||
} | ||
return &cgroup{ | ||
path: path, | ||
subsystems: subsystems, | ||
subsystems: active, | ||
}, nil | ||
} | ||
|
||
// Load will load an existing cgroup and allow it to be controlled | ||
func Load(hierarchy Hierarchy, path Path) (Cgroup, error) { | ||
func Load(hierarchy Hierarchy, path Path, opts ...InitOpts) (Cgroup, error) { | ||
config := newInitConfig() | ||
for _, o := range opts { | ||
if err := o(config); err != nil { | ||
return nil, err | ||
} | ||
} | ||
var activeSubsystems []Subsystem | ||
subsystems, err := hierarchy() | ||
if err != nil { | ||
|
@@ -60,6 +85,16 @@ func Load(hierarchy Hierarchy, path Path) (Cgroup, error) { | |
if os.IsNotExist(errors.Cause(err)) { | ||
return nil, ErrCgroupDeleted | ||
} | ||
if err == ErrControllerNotActive { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to change the location for
first check controller is support, then check the directory is exist ? if the controller is not support, we can ignore the controller directory is not exist There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine to keep the existing logics for Load, the isnotexist is for when the cgroup directory structure has been removed, not an active controller. I think this is fine for the Load case to get the ErrCgroupDeleted error which is handled |
||
if config.InitCheck != nil { | ||
if skerr := config.InitCheck(s, path, err); skerr != nil { | ||
if skerr != ErrIgnoreSubsystem { | ||
return nil, skerr | ||
} | ||
} | ||
} | ||
continue | ||
} | ||
return nil, err | ||
} | ||
if _, err := os.Lstat(s.Path(p)); err != nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/* | ||
Copyright The containerd Authors. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package cgroups | ||
|
||
import ( | ||
"github.com/pkg/errors" | ||
) | ||
|
||
var ( | ||
// ErrIgnoreSubsystem allows the specific subsystem to be skipped | ||
ErrIgnoreSubsystem = errors.New("skip subsystem") | ||
// ErrDevicesRequired is returned when the devices subsystem is required but | ||
// does not exist or is not active | ||
ErrDevicesRequired = errors.New("devices subsystem is required") | ||
) | ||
|
||
// InitOpts allows configuration for the creation or loading of a cgroup | ||
type InitOpts func(*InitConfig) error | ||
|
||
// InitConfig provides configuration options for the creation | ||
// or loading of a cgroup and its subsystems | ||
type InitConfig struct { | ||
// InitCheck can be used to check initialization errors from the subsystem | ||
InitCheck InitCheck | ||
} | ||
|
||
func newInitConfig() *InitConfig { | ||
return &InitConfig{ | ||
InitCheck: RequireDevices, | ||
} | ||
} | ||
|
||
// InitCheck allows subsystems errors to be checked when initialized or loaded | ||
type InitCheck func(Subsystem, Path, error) error | ||
|
||
// AllowAny allows any subsystem errors to be skipped | ||
func AllowAny(s Subsystem, p Path, err error) error { | ||
return ErrIgnoreSubsystem | ||
} | ||
|
||
// RequireDevices requires the device subsystem but no others | ||
func RequireDevices(s Subsystem, p Path, err error) error { | ||
if s.Name() == Devices { | ||
return ErrDevicesRequired | ||
} | ||
return ErrIgnoreSubsystem | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,3 +112,29 @@ func TestEmptySubsystem(t *testing.T) { | |
} | ||
} | ||
} | ||
|
||
func TestSystemd240(t *testing.T) { | ||
const data = `8:net_cls:/ | ||
7:memory:/system.slice/docker.service | ||
6:freezer:/ | ||
5:blkio:/system.slice/docker.service | ||
4:devices:/system.slice/docker.service | ||
3:cpuset:/ | ||
2:cpu,cpuacct:/system.slice/docker.service | ||
1:name=systemd:/system.slice/docker.service | ||
0::/system.slice/docker.service` | ||
r := strings.NewReader(data) | ||
paths, err := parseCgroupFromReader(r) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
path := existingPath(paths, "") | ||
_, err = path("net_prio") | ||
if err == nil { | ||
t.Fatal("error for net_prio shoulld not be nil") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /s/shoulld/should/ |
||
} | ||
if err != ErrControllerNotActive { | ||
t.Fatalf("expected error %q but received %q", ErrControllerNotActive, err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about adding a list for skipped .. or inactive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure we need it