libcontainer: refactor resource manager interface

Currently, cgroups manager is the single resource manager in libcontainer. Linux kernel 4.10 will introduce Intel RDT/CAT feature, the kernel interface is exposed via "resource control" filesystem, which is a cgroup-like interface. In order to support Intel RDT/CAT in libcontainer, we need a new resource manager outside cgroups. This patch adds a new "ResourceManager" structure as the base interface for all resource managers, such as cgroups manager and incoming IntelRdt manager. All registered resource managers are consolidated in linuxContainer structure. We can apply to unified operations (e.g., Apply(), Set(), Destroy()) using all of the registered resource managers. Signed-off-by: Xiaochen Shen <[email protected]>
opencontainers · May 15, 2017 · d5ac70d · d5ac70d
1 parent c302b70
commit d5ac70d
Show file tree

Hide file tree

Showing 11 changed files with 123 additions and 89 deletions.
diff --git a/libcontainer/cgroups/cgroups.go b/libcontainer/cgroups/cgroups.go
@@ -5,40 +5,11 @@ package cgroups
 import (
 	"fmt"
 
-	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/runc/libcontainer/resourcemanager"
 )
 
 type Manager interface {
-	// Applies cgroup configuration to the process with the specified pid
-	Apply(pid int) error
-
-	// Returns the PIDs inside the cgroup set
-	GetPids() ([]int, error)
-
-	// Returns the PIDs inside the cgroup set & all sub-cgroups
-	GetAllPids() ([]int, error)
-
-	// Returns statistics for the cgroup set
-	GetStats() (*Stats, error)
-
-	// Toggles the freezer cgroup according with specified state
-	Freeze(state configs.FreezerState) error
-
-	// Destroys the cgroup set
-	Destroy() error
-
-	// The option func SystemdCgroups() and Cgroupfs() require following attributes:
-	// 	Paths   map[string]string
-	// 	Cgroups *configs.Cgroup
-	// Paths maps cgroup subsystem to path at which it is mounted.
-	// Cgroups specifies specific cgroup settings for the various subsystems
-
-	// Returns cgroup paths to save in a state file and to be able to
-	// restore the object later.
-	GetPaths() map[string]string
-
-	// Sets the cgroup as configured.
-	Set(container *configs.Config) error
+	resourcemanager.ResourceManager
 }
 
 type NotFoundError struct {

diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go
@@ -171,7 +171,7 @@ func (m *Manager) GetPaths() map[string]string {
 	return paths
 }
 
-func (m *Manager) GetStats() (*cgroups.Stats, error) {
+func (m *Manager) GetStats() (interface{}, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	stats := cgroups.NewStats()

diff --git a/libcontainer/cgroups/rootless/rootless.go b/libcontainer/cgroups/rootless/rootless.go
@@ -107,7 +107,7 @@ func (m *Manager) GetAllPids() ([]int, error) {
 	return cgroups.GetAllPids(dir)
 }
 
-func (m *Manager) GetStats() (*cgroups.Stats, error) {
+func (m *Manager) GetStats() (interface{}, error) {
 	// TODO(cyphar): We can make this work if we figure out a way to allow usage
 	//               of cgroups with a rootless container. While this doesn't
 	//               actually require write access to a cgroup directory, the

diff --git a/libcontainer/cgroups/systemd/apply_nosystemd.go b/libcontainer/cgroups/systemd/apply_nosystemd.go
@@ -38,7 +38,7 @@ func (m *Manager) GetPaths() map[string]string {
 	return nil
 }
 
-func (m *Manager) GetStats() (*cgroups.Stats, error) {
+func (m *Manager) GetStats() (interface{}, error) {
 	return nil, fmt.Errorf("Systemd not supported")
 }
 

diff --git a/libcontainer/cgroups/systemd/apply_systemd.go b/libcontainer/cgroups/systemd/apply_systemd.go
@@ -481,7 +481,7 @@ func (m *Manager) GetAllPids() ([]int, error) {
 	return cgroups.GetAllPids(path)
 }
 
-func (m *Manager) GetStats() (*cgroups.Stats, error) {
+func (m *Manager) GetStats() (interface{}, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	stats := cgroups.NewStats()

diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
@@ -22,6 +22,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/criurpc"
+	"github.com/opencontainers/runc/libcontainer/resourcemanager"
 	"github.com/opencontainers/runc/libcontainer/system"
 	"github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/syndtr/gocapability/capability"
@@ -34,7 +35,7 @@ type linuxContainer struct {
 	id                   string
 	root                 string
 	config               *configs.Config
-	cgroupManager        cgroups.Manager
+	resourceManagers     map[string]resourcemanager.ResourceManager
 	initArgs             []string
 	initProcess          parentProcess
 	initProcessStartTime string
@@ -145,7 +146,7 @@ func (c *linuxContainer) State() (*State, error) {
 }
 
 func (c *linuxContainer) Processes() ([]int, error) {
-	pids, err := c.cgroupManager.GetAllPids()
+	pids, err := c.resourceManagers["cgroups"].GetAllPids()
 	if err != nil {
 		return nil, newSystemErrorWithCause(err, "getting all container pids from cgroups")
 	}
@@ -157,7 +158,9 @@ func (c *linuxContainer) Stats() (*Stats, error) {
 		err   error
 		stats = &Stats{}
 	)
-	if stats.CgroupStats, err = c.cgroupManager.GetStats(); err != nil {
+	cgroupStats, err := c.resourceManagers["cgroups"].GetStats()
+	stats.CgroupStats = cgroupStats.(*cgroups.Stats)
+	if err != nil {
 		return stats, newSystemErrorWithCause(err, "getting container stats from cgroups")
 	}
 	for _, iface := range c.config.Networks {
@@ -184,7 +187,12 @@ func (c *linuxContainer) Set(config configs.Config) error {
 		return newGenericError(fmt.Errorf("container not running"), ContainerNotRunning)
 	}
 	c.config = &config
-	return c.cgroupManager.Set(c.config)
+	for _, resourceManager := range c.resourceManagers {
+		if err := resourceManager.Set(c.config); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 func (c *linuxContainer) Start(process *Process) error {
@@ -299,7 +307,7 @@ func (c *linuxContainer) start(process *Process, isInit bool) error {
 
 func (c *linuxContainer) Signal(s os.Signal, all bool) error {
 	if all {
-		return signalAllProcesses(c.cgroupManager, s)
+		return signalAllProcesses(c.resourceManagers["cgroups"], s)
 	}
 	if err := c.initProcess.signal(s); err != nil {
 		return newSystemErrorWithCause(err, "signaling init process")
@@ -411,7 +419,7 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c
 		cmd:           cmd,
 		childPipe:     childPipe,
 		parentPipe:    parentPipe,
-		manager:       c.cgroupManager,
+		managers:      c.resourceManagers,
 		config:        c.newInitConfig(p),
 		container:     c,
 		process:       p,
@@ -435,7 +443,7 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe,
 	}
 	return &setnsProcess{
 		cmd:           cmd,
-		cgroupPaths:   c.cgroupManager.GetPaths(),
+		cgroupPaths:   c.resourceManagers["cgroups"].GetPaths(),
 		childPipe:     childPipe,
 		parentPipe:    parentPipe,
 		config:        c.newInitConfig(p),
@@ -492,7 +500,7 @@ func (c *linuxContainer) Pause() error {
 	}
 	switch status {
 	case Running, Created:
-		if err := c.cgroupManager.Freeze(configs.Frozen); err != nil {
+		if err := c.resourceManagers["cgroups"].Freeze(configs.Frozen); err != nil {
 			return err
 		}
 		return c.state.transition(&pausedState{
@@ -512,7 +520,7 @@ func (c *linuxContainer) Resume() error {
 	if status != Paused {
 		return newGenericError(fmt.Errorf("container not paused"), ContainerNotPaused)
 	}
-	if err := c.cgroupManager.Freeze(configs.Thawed); err != nil {
+	if err := c.resourceManagers["cgroups"].Freeze(configs.Thawed); err != nil {
 		return err
 	}
 	return c.state.transition(&runningState{
@@ -525,15 +533,15 @@ func (c *linuxContainer) NotifyOOM() (<-chan struct{}, error) {
 	if c.config.Rootless {
 		return nil, fmt.Errorf("cannot get OOM notifications from rootless container")
 	}
-	return notifyOnOOM(c.cgroupManager.GetPaths())
+	return notifyOnOOM(c.resourceManagers["cgroups"].GetPaths())
 }
 
 func (c *linuxContainer) NotifyMemoryPressure(level PressureLevel) (<-chan struct{}, error) {
 	// XXX(cyphar): This requires cgroups.
 	if c.config.Rootless {
 		return nil, fmt.Errorf("cannot get memory pressure notifications from rootless container")
 	}
-	return notifyMemoryPressure(c.cgroupManager.GetPaths(), level)
+	return notifyMemoryPressure(c.resourceManagers["cgroups"].GetPaths(), level)
 }
 
 var criuFeatures *criurpc.CriuFeatures
@@ -1004,11 +1012,11 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error {
 
 func (c *linuxContainer) criuApplyCgroups(pid int, req *criurpc.CriuReq) error {
 	// XXX: Do we need to deal with this case? AFAIK criu still requires root.
-	if err := c.cgroupManager.Apply(pid); err != nil {
+	if err := c.resourceManagers["cgroups"].Apply(pid); err != nil {
 		return err
 	}
 
-	if err := c.cgroupManager.Set(c.config); err != nil {
+	if err := c.resourceManagers["cgroups"].Set(c.config); err != nil {
 		return newSystemError(err)
 	}
 
@@ -1383,7 +1391,7 @@ func (c *linuxContainer) runType() (Status, error) {
 }
 
 func (c *linuxContainer) isPaused() (bool, error) {
-	fcg := c.cgroupManager.GetPaths()["freezer"]
+	fcg := c.resourceManagers["cgroups"].GetPaths()["freezer"]
 	if fcg == "" {
 		// A container doesn't have a freezer cgroup
 		return false, nil
@@ -1419,7 +1427,7 @@ func (c *linuxContainer) currentState() (*State, error) {
 			Created:              c.created,
 		},
 		Rootless:            c.config.Rootless,
-		CgroupPaths:         c.cgroupManager.GetPaths(),
+		CgroupPaths:         c.resourceManagers["cgroups"].GetPaths(),
 		NamespacePaths:      make(map[configs.NamespaceType]string),
 		ExternalDescriptors: externalDescriptors,
 	}

diff --git a/libcontainer/container_linux_test.go b/libcontainer/container_linux_test.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/runc/libcontainer/resourcemanager"
 )
 
 type mockCgroupManager struct {
@@ -26,7 +27,7 @@ func (m *mockCgroupManager) GetAllPids() ([]int, error) {
 	return m.allPids, nil
 }
 
-func (m *mockCgroupManager) GetStats() (*cgroups.Stats, error) {
+func (m *mockCgroupManager) GetStats() (interface{}, error) {
 	return m.stats, nil
 }
 
@@ -88,10 +89,11 @@ func (m *mockProcess) setExternalDescriptors(newFds []string) {
 
 func TestGetContainerPids(t *testing.T) {
 	container := &linuxContainer{
-		id:            "myid",
-		config:        &configs.Config{},
-		cgroupManager: &mockCgroupManager{allPids: []int{1, 2, 3}},
+		id:     "myid",
+		config: &configs.Config{},
 	}
+	container.resourceManagers = make(map[string]resourcemanager.ResourceManager)
+	container.resourceManagers["cgroups"] = &mockCgroupManager{allPids: []int{1, 2, 3}}
 	pids, err := container.Processes()
 	if err != nil {
 		t.Fatal(err)
@@ -107,13 +109,14 @@ func TestGetContainerStats(t *testing.T) {
 	container := &linuxContainer{
 		id:     "myid",
 		config: &configs.Config{},
-		cgroupManager: &mockCgroupManager{
-			pids: []int{1, 2, 3},
-			stats: &cgroups.Stats{
-				MemoryStats: cgroups.MemoryStats{
-					Usage: cgroups.MemoryData{
-						Usage: 1024,
-					},
+	}
+	container.resourceManagers = make(map[string]resourcemanager.ResourceManager)
+	container.resourceManagers["cgroups"] = &mockCgroupManager{
+		pids: []int{1, 2, 3},
+		stats: &cgroups.Stats{
+			MemoryStats: cgroups.MemoryStats{
+				Usage: cgroups.MemoryData{
+					Usage: 1024,
 				},
 			},
 		},
@@ -152,18 +155,19 @@ func TestGetContainerState(t *testing.T) {
 			_pid:    pid,
 			started: "010",
 		},
-		cgroupManager: &mockCgroupManager{
-			pids: []int{1, 2, 3},
-			stats: &cgroups.Stats{
-				MemoryStats: cgroups.MemoryStats{
-					Usage: cgroups.MemoryData{
-						Usage: 1024,
-					},
+	}
+	container.resourceManagers = make(map[string]resourcemanager.ResourceManager)
+	container.resourceManagers["cgroups"] = &mockCgroupManager{
+		pids: []int{1, 2, 3},
+		stats: &cgroups.Stats{
+			MemoryStats: cgroups.MemoryStats{
+				Usage: cgroups.MemoryData{
+					Usage: 1024,
 				},
 			},
-			paths: map[string]string{
-				"memory": expectedMemoryPath,
-			},
+		},
+		paths: map[string]string{
+			"memory": expectedMemoryPath,
 		},
 	}
 	container.state = &createdState{c: container}

diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go
@@ -19,6 +19,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/configs/validate"
+	"github.com/opencontainers/runc/libcontainer/resourcemanager"
 	"github.com/opencontainers/runc/libcontainer/utils"
 )
 
@@ -185,13 +186,15 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
 		RootlessCgroups(l)
 	}
 	c := &linuxContainer{
-		id:            id,
-		root:          containerRoot,
-		config:        config,
-		initArgs:      l.InitArgs,
-		criuPath:      l.CriuPath,
-		cgroupManager: l.NewCgroupsManager(config.Cgroups, nil),
-	}
+		id:       id,
+		root:     containerRoot,
+		config:   config,
+		initArgs: l.InitArgs,
+		criuPath: l.CriuPath,
+	}
+	resourceManagers := make(map[string]resourcemanager.ResourceManager)
+	resourceManagers["cgroups"] = l.NewCgroupsManager(config.Cgroups, nil)
+	c.resourceManagers = resourceManagers
 	c.state = &stoppedState{c: c}
 	return c, nil
 }
@@ -221,10 +224,12 @@ func (l *LinuxFactory) Load(id string) (Container, error) {
 		config:               &state.Config,
 		initArgs:             l.InitArgs,
 		criuPath:             l.CriuPath,
-		cgroupManager:        l.NewCgroupsManager(state.Config.Cgroups, state.CgroupPaths),
 		root:                 containerRoot,
 		created:              state.Created,
 	}
+	resourceManagers := make(map[string]resourcemanager.ResourceManager)
+	resourceManagers["cgroups"] = l.NewCgroupsManager(state.Config.Cgroups, state.CgroupPaths)
+	c.resourceManagers = resourceManagers
 	c.state = &loadedState{c: c}
 	if err := c.refreshState(); err != nil {
 		return nil, err