From bc7efa6b81f44c29ef4e4c422d91b9ccc1041e20 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 14 Nov 2014 17:22:10 -0800 Subject: [PATCH] Have cgroup.Apply return paths to setup cgroups There is no reason to have a special type returned from the cgroups Apply function for getting the paths and cleanup. With access to the paths we can just delete what we need. Signed-off-by: Michael Crosby --- cgroups/cgroups.go | 5 --- cgroups/cgutil/cgutil.go | 27 -------------- cgroups/fs/apply_raw.go | 58 ++++++++++-------------------- cgroups/systemd/apply_nosystemd.go | 2 +- cgroups/systemd/apply_systemd.go | 26 +++----------- namespaces/exec.go | 27 +++++++------- 6 files changed, 37 insertions(+), 108 deletions(-) diff --git a/cgroups/cgroups.go b/cgroups/cgroups.go index 567e9a6c..fe360059 100644 --- a/cgroups/cgroups.go +++ b/cgroups/cgroups.go @@ -53,8 +53,3 @@ type Cgroup struct { Freezer FreezerState `json:"freezer,omitempty"` // set the freeze value for the process Slice string `json:"slice,omitempty"` // Parent slice to use for systemd } - -type ActiveCgroup interface { - Cleanup() error - Paths() (map[string]string, error) -} diff --git a/cgroups/cgutil/cgutil.go b/cgroups/cgutil/cgutil.go index 3b9a9a66..db02d156 100644 --- a/cgroups/cgutil/cgutil.go +++ b/cgroups/cgutil/cgutil.go @@ -24,16 +24,6 @@ var createCommand = cli.Command{ Action: createAction, } -var destroyCommand = cli.Command{ - Name: "destroy", - Usage: "Destroy an existing cgroup container.", - Flags: []cli.Flag{ - cli.StringFlag{Name: "name, n", Value: "", Usage: "container name"}, - cli.StringFlag{Name: "parent, p", Value: "", Usage: "container parent"}, - }, - Action: destroyAction, -} - var pauseCommand = cli.Command{ Name: "pause", Usage: "Pause cgroup", @@ -170,22 +160,6 @@ func createAction(context *cli.Context) { } } -func destroyAction(context *cli.Context) { - config, err := getConfig(context) - if err != nil { - log.Fatal(err) - } - - killAll(config) - // Systemd will clean up cgroup state for empty container. - if !systemd.UseSystemd() { - err := fs.Cleanup(config) - if err != nil { - log.Fatal(err) - } - } -} - func pauseAction(context *cli.Context) { setFreezerState(context, cgroups.Frozen) } @@ -224,7 +198,6 @@ func main() { app.Commands = []cli.Command{ createCommand, - destroyCommand, pauseCommand, resumeCommand, psCommand, diff --git a/cgroups/fs/apply_raw.go b/cgroups/fs/apply_raw.go index c6860ca6..3597374b 100644 --- a/cgroups/fs/apply_raw.go +++ b/cgroups/fs/apply_raw.go @@ -57,20 +57,33 @@ type data struct { pid int } -func Apply(c *cgroups.Cgroup, pid int) (cgroups.ActiveCgroup, error) { +func Apply(c *cgroups.Cgroup, pid int) (map[string]string, error) { d, err := getCgroupData(c, pid) if err != nil { return nil, err } - for _, sys := range subsystems { + paths := make(map[string]string) + for name, sys := range subsystems { if err := sys.Set(d); err != nil { - d.Cleanup() + for _, p := range paths { + os.RemoveAll(p) + } return nil, err } + p, err := d.path(name) + if err != nil { + if cgroups.IsNotFound(err) { + continue + } + for _, p := range paths { + os.RemoveAll(p) + } + return nil, err + } + paths[name] = p } - - return d, nil + return paths, nil } // Symmetrical public function to update device based cgroups. Also available @@ -86,14 +99,6 @@ func ApplyDevices(c *cgroups.Cgroup, pid int) error { return devices.Set(d) } -func Cleanup(c *cgroups.Cgroup) error { - d, err := getCgroupData(c, 0) - if err != nil { - return fmt.Errorf("Could not get Cgroup data %s", err) - } - return d.Cleanup() -} - func GetStats(systemPaths map[string]string) (*cgroups.Stats, error) { stats := cgroups.NewStats() for name, path := range systemPaths { @@ -164,26 +169,6 @@ func (raw *data) parent(subsystem string) (string, error) { return filepath.Join(raw.root, subsystem, initPath), nil } -func (raw *data) Paths() (map[string]string, error) { - paths := make(map[string]string) - - for sysname := range subsystems { - path, err := raw.path(sysname) - if err != nil { - // Don't fail if a cgroup hierarchy was not found, just skip this subsystem - if cgroups.IsNotFound(err) { - continue - } - - return nil, err - } - - paths[sysname] = path - } - - return paths, nil -} - func (raw *data) path(subsystem string) (string, error) { // If the cgroup name/path is absolute do not look relative to the cgroup of the init process. if filepath.IsAbs(raw.cgroup) { @@ -222,13 +207,6 @@ func (raw *data) join(subsystem string) (string, error) { return path, nil } -func (raw *data) Cleanup() error { - for _, sys := range subsystems { - sys.Remove(raw) - } - return nil -} - func writeFile(dir, file, data string) error { return ioutil.WriteFile(filepath.Join(dir, file), []byte(data), 0700) } diff --git a/cgroups/systemd/apply_nosystemd.go b/cgroups/systemd/apply_nosystemd.go index 81334191..4b9a2f5b 100644 --- a/cgroups/systemd/apply_nosystemd.go +++ b/cgroups/systemd/apply_nosystemd.go @@ -12,7 +12,7 @@ func UseSystemd() bool { return false } -func Apply(c *cgroups.Cgroup, pid int) (cgroups.ActiveCgroup, error) { +func Apply(c *cgroups.Cgroup, pid int) (map[string]string, error) { return nil, fmt.Errorf("Systemd not supported") } diff --git a/cgroups/systemd/apply_systemd.go b/cgroups/systemd/apply_systemd.go index a70aa408..94f3465f 100644 --- a/cgroups/systemd/apply_systemd.go +++ b/cgroups/systemd/apply_systemd.go @@ -81,7 +81,7 @@ func getIfaceForUnit(unitName string) string { return "Unit" } -func Apply(c *cgroups.Cgroup, pid int) (cgroups.ActiveCgroup, error) { +func Apply(c *cgroups.Cgroup, pid int) (map[string]string, error) { var ( unitName = getUnitName(c) slice = "system.slice" @@ -149,14 +149,6 @@ func Apply(c *cgroups.Cgroup, pid int) (cgroups.ActiveCgroup, error) { } } - return res, nil -} - -func writeFile(dir, file, data string) error { - return ioutil.WriteFile(filepath.Join(dir, file), []byte(data), 0700) -} - -func (c *systemdCgroup) Paths() (map[string]string, error) { paths := make(map[string]string) for _, sysname := range []string{ "devices", @@ -168,7 +160,7 @@ func (c *systemdCgroup) Paths() (map[string]string, error) { "perf_event", "freezer", } { - subsystemPath, err := getSubsystemPath(c.cgroup, sysname) + subsystemPath, err := getSubsystemPath(res.cgroup, sysname) if err != nil { // Don't fail if a cgroup hierarchy was not found, just skip this subsystem if cgroups.IsNotFound(err) { @@ -181,18 +173,8 @@ func (c *systemdCgroup) Paths() (map[string]string, error) { return paths, nil } -func (c *systemdCgroup) Cleanup() error { - // systemd cleans up, we don't need to do much - paths, err := c.Paths() - if err != nil { - return err - } - - for _, path := range paths { - os.RemoveAll(path) - } - - return nil +func writeFile(dir, file, data string) error { + return ioutil.WriteFile(filepath.Join(dir, file), []byte(data), 0700) } func joinFreezer(c *cgroups.Cgroup, pid int) error { diff --git a/namespaces/exec.go b/namespaces/exec.go index bd3a4a3f..eac03d5d 100644 --- a/namespaces/exec.go +++ b/namespaces/exec.go @@ -10,7 +10,6 @@ import ( "syscall" "github.com/docker/libcontainer" - "github.com/docker/libcontainer/cgroups" "github.com/docker/libcontainer/cgroups/fs" "github.com/docker/libcontainer/cgroups/systemd" "github.com/docker/libcontainer/network" @@ -60,16 +59,11 @@ func Exec(container *libcontainer.Config, stdin io.Reader, stdout, stderr io.Wri // Do this before syncing with child so that no children // can escape the cgroup - cgroupRef, err := SetupCgroups(container, command.Process.Pid) - if err != nil { - return terminate(err) - } - defer cgroupRef.Cleanup() - - cgroupPaths, err := cgroupRef.Paths() + cgroupPaths, err := SetupCgroups(container, command.Process.Pid) if err != nil { return terminate(err) } + defer removeCgroupPaths(cgroupPaths) var networkState network.NetworkState if err := InitializeNetworking(container, command.Process.Pid, &networkState); err != nil { @@ -153,18 +147,15 @@ func DefaultCreateCommand(container *libcontainer.Config, console, dataPath, ini // SetupCgroups applies the cgroup restrictions to the process running in the container based // on the container's configuration -func SetupCgroups(container *libcontainer.Config, nspid int) (cgroups.ActiveCgroup, error) { +func SetupCgroups(container *libcontainer.Config, nspid int) (map[string]string, error) { if container.Cgroups != nil { c := container.Cgroups - if systemd.UseSystemd() { return systemd.Apply(c, nspid) } - return fs.Apply(c, nspid) } - - return nil, nil + return map[string]string{}, nil } // InitializeNetworking creates the container's network stack outside of the namespace and moves @@ -181,3 +172,13 @@ func InitializeNetworking(container *libcontainer.Config, nspid int, networkStat } return nil } + +// removeCgroupPaths takes the subsystem paths for each cgroup that was +// setup for the container and removes them from the cgroup filesystem. +// Errors are ignored during the removal because we don't want to end up +// with partially removed cgroups. +func removeCgroupPaths(paths map[string]string) { + for _, path := range paths { + os.RemoveAll(path) + } +}