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 <crosbymichael@gmail.com>

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)
-}

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,

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 paths, nil
-	return d, 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)
 }

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")
 }
 

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 {
+func writeFile(dir, file, data string) error {
-	// systemd cleans up, we don't need to do much
+	return ioutil.WriteFile(filepath.Join(dir, file), []byte(data), 0700)
-	paths, err := c.Paths()
-	if err != nil {
-		return err
-	}
-
-	for _, path := range paths {
-		os.RemoveAll(path)
-	}
-
-	return nil
 }
 
 func joinFreezer(c *cgroups.Cgroup, pid int) error {

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)
+	cgroupPaths, err := SetupCgroups(container, command.Process.Pid)
-	if err != nil {
-		return terminate(err)
-	}
-	defer cgroupRef.Cleanup()
-
-	cgroupPaths, err := cgroupRef.Paths()
 	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 map[string]string{}, nil
-	return nil, 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)
+	}
+}