Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[1.1] fix intelrdt #3978

Merged
merged 11 commits into from
Aug 10, 2023
Merged

[1.1] fix intelrdt #3978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dc8d0cc
libct/intelrdt: wrap Root in sync.Once
kolyshkin Dec 2, 2021
a5407b9
libct/intelrdt: remove findMountpointDir test
kolyshkin Dec 2, 2021
5ba1b8e
libct/intelrdt: faster init if rdt is unsupported
kolyshkin Dec 3, 2021
dfdc7d0
libct/intelrdt: explain why mountinfo is required
kolyshkin Jan 19, 2022
69473d0
libct: rm TestGetContainerStats, mockIntelRdtManager
kolyshkin Dec 9, 2021
5ebcfa6
[1.1] libct: rm intelrtd.Manager interface, NewIntelRdtManager
kolyshkin Aug 9, 2023
7c83dbe
libct/intelrdt: delete IsMBAScEnabled()
corhere Jul 26, 2022
6a7a6a5
libct/intelrdt: skip reading /proc/cpuinfo
corhere Jul 26, 2022
4796f49
libct/intelrdt: elide parsing mountinfo
corhere Jul 26, 2022
6cf9ac1
libct/intelrdt: skip remove unless configured
corhere Jul 26, 2022
f44190e
libct/intelrdt: check if available iff configured
corhere Jul 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libcontainer/container_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type linuxContainer struct {
root string
config *configs.Config
cgroupManager cgroups.Manager
intelRdtManager intelrdt.Manager
intelRdtManager *intelrdt.Manager
initPath string
initArgs []string
initProcess parentProcess
Expand Down
112 changes: 4 additions & 108 deletions libcontainer/container_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,15 @@ import (

"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/intelrdt"
"github.com/opencontainers/runc/libcontainer/system"
)

type mockCgroupManager struct {
pids []int
allPids []int
stats *cgroups.Stats
paths map[string]string
}

type mockIntelRdtManager struct {
stats *intelrdt.Stats
path string
}

func (m *mockCgroupManager) GetPids() ([]int, error) {
return m.pids, nil
}
Expand All @@ -32,7 +25,7 @@ func (m *mockCgroupManager) GetAllPids() ([]int, error) {
}

func (m *mockCgroupManager) GetStats() (*cgroups.Stats, error) {
return m.stats, nil
return nil, nil
}

func (m *mockCgroupManager) Apply(pid int) error {
Expand Down Expand Up @@ -76,30 +69,6 @@ func (m *mockCgroupManager) GetFreezerState() (configs.FreezerState, error) {
return configs.Thawed, nil
}

func (m *mockIntelRdtManager) Apply(pid int) error {
return nil
}

func (m *mockIntelRdtManager) GetStats() (*intelrdt.Stats, error) {
return m.stats, nil
}

func (m *mockIntelRdtManager) Destroy() error {
return nil
}

func (m *mockIntelRdtManager) GetPath() string {
return m.path
}

func (m *mockIntelRdtManager) Set(container *configs.Config) error {
return nil
}

func (m *mockIntelRdtManager) GetCgroups() (*configs.Cgroup, error) {
return nil, nil
}

type mockProcess struct {
_pid int
started uint64
Expand Down Expand Up @@ -173,61 +142,11 @@ func TestGetContainerPids(t *testing.T) {
}
}

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,
},
},
},
},
intelRdtManager: &mockIntelRdtManager{
stats: &intelrdt.Stats{
L3CacheSchema: "L3:0=f;1=f0",
MemBwSchema: "MB:0=20;1=70",
},
},
}
stats, err := container.Stats()
if err != nil {
t.Fatal(err)
}
if stats.CgroupStats == nil {
t.Fatal("cgroup stats are nil")
}
if stats.CgroupStats.MemoryStats.Usage.Usage != 1024 {
t.Fatalf("expected memory usage 1024 but received %d", stats.CgroupStats.MemoryStats.Usage.Usage)
}
if intelrdt.IsCATEnabled() {
if stats.IntelRdtStats == nil {
t.Fatal("intel rdt stats are nil")
}
if stats.IntelRdtStats.L3CacheSchema != "L3:0=f;1=f0" {
t.Fatalf("expected L3CacheSchema L3:0=f;1=f0 but received %s", stats.IntelRdtStats.L3CacheSchema)
}
}
if intelrdt.IsMBAEnabled() {
if stats.IntelRdtStats == nil {
t.Fatal("intel rdt stats are nil")
}
if stats.IntelRdtStats.MemBwSchema != "MB:0=20;1=70" {
t.Fatalf("expected MemBwSchema MB:0=20;1=70 but received %s", stats.IntelRdtStats.MemBwSchema)
}
}
}

func TestGetContainerState(t *testing.T) {
var (
pid = os.Getpid()
expectedMemoryPath = "/sys/fs/cgroup/memory/myid"
expectedNetworkPath = fmt.Sprintf("/proc/%d/ns/net", pid)
expectedIntelRdtPath = "/sys/fs/resctrl/myid"
pid = os.Getpid()
expectedMemoryPath = "/sys/fs/cgroup/memory/myid"
expectedNetworkPath = fmt.Sprintf("/proc/%d/ns/net", pid)
)
container := &linuxContainer{
id: "myid",
Expand All @@ -248,24 +167,10 @@ func TestGetContainerState(t *testing.T) {
},
cgroupManager: &mockCgroupManager{
pids: []int{1, 2, 3},
stats: &cgroups.Stats{
MemoryStats: cgroups.MemoryStats{
Usage: cgroups.MemoryData{
Usage: 1024,
},
},
},
paths: map[string]string{
"memory": expectedMemoryPath,
},
},
intelRdtManager: &mockIntelRdtManager{
stats: &intelrdt.Stats{
L3CacheSchema: "L3:0=f0;1=f",
MemBwSchema: "MB:0=70;1=20",
},
path: expectedIntelRdtPath,
},
}
container.state = &createdState{c: container}
state, err := container.State()
Expand All @@ -285,15 +190,6 @@ func TestGetContainerState(t *testing.T) {
if memPath := paths["memory"]; memPath != expectedMemoryPath {
t.Fatalf("expected memory path %q but received %q", expectedMemoryPath, memPath)
}
if intelrdt.IsCATEnabled() || intelrdt.IsMBAEnabled() {
intelRdtPath := state.IntelRdtPath
if intelRdtPath == "" {
t.Fatal("intel rdt path should not be empty")
}
if intelRdtPath != expectedIntelRdtPath {
t.Fatalf("expected intel rdt path %q but received %q", expectedIntelRdtPath, intelRdtPath)
}
}
for _, ns := range container.config.Namespaces {
path := state.NamespacePaths[ns.Type]
if path == "" {
Expand Down
43 changes: 11 additions & 32 deletions libcontainer/factory_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,6 @@ func InitArgs(args ...string) func(*LinuxFactory) error {
}
}

// IntelRdtfs is an options func to configure a LinuxFactory to return
// containers that use the Intel RDT "resource control" filesystem to
// create and manage Intel RDT resources (e.g., L3 cache, memory bandwidth).
func IntelRdtFs(l *LinuxFactory) error {
if !intelrdt.IsCATEnabled() && !intelrdt.IsMBAEnabled() {
l.NewIntelRdtManager = nil
Comment on lines -55 to -56
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Removal of this check is THE fix!

} else {
l.NewIntelRdtManager = func(config *configs.Config, id string, path string) intelrdt.Manager {
return intelrdt.NewManager(config, id, path)
}
}
return nil
}

// TmpfsRoot is an option func to mount LinuxFactory.Root to tmpfs.
func TmpfsRoot(l *LinuxFactory) error {
mounted, err := mountinfo.Mounted(l.Root)
Expand Down Expand Up @@ -136,9 +122,6 @@ type LinuxFactory struct {

// Validator provides validation to container configurations.
Validator validate.Validator

// NewIntelRdtManager returns an initialized Intel RDT manager for a single container.
NewIntelRdtManager func(config *configs.Config, id string, path string) intelrdt.Manager
}

func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, error) {
Expand Down Expand Up @@ -208,18 +191,16 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
return nil, err
}
c := &linuxContainer{
id: id,
root: containerRoot,
config: config,
initPath: l.InitPath,
initArgs: l.InitArgs,
criuPath: l.CriuPath,
newuidmapPath: l.NewuidmapPath,
newgidmapPath: l.NewgidmapPath,
cgroupManager: cm,
}
if l.NewIntelRdtManager != nil {
c.intelRdtManager = l.NewIntelRdtManager(config, id, "")
id: id,
root: containerRoot,
config: config,
initPath: l.InitPath,
initArgs: l.InitArgs,
criuPath: l.CriuPath,
newuidmapPath: l.NewuidmapPath,
newgidmapPath: l.NewgidmapPath,
cgroupManager: cm,
intelRdtManager: intelrdt.NewManager(config, id, ""),
}
c.state = &stoppedState{c: c}
return c, nil
Expand Down Expand Up @@ -261,12 +242,10 @@ func (l *LinuxFactory) Load(id string) (Container, error) {
newuidmapPath: l.NewuidmapPath,
newgidmapPath: l.NewgidmapPath,
cgroupManager: cm,
intelRdtManager: intelrdt.NewManager(&state.Config, id, state.IntelRdtPath),
root: containerRoot,
created: state.Created,
}
if l.NewIntelRdtManager != nil {
c.intelRdtManager = l.NewIntelRdtManager(&state.Config, id, state.IntelRdtPath)
}
c.state = &loadedState{c: c}
if err := c.refreshState(); err != nil {
return nil, err
Expand Down
24 changes: 1 addition & 23 deletions libcontainer/factory_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,6 @@ func TestFactoryNew(t *testing.T) {
}
}

func TestFactoryNewIntelRdt(t *testing.T) {
root := t.TempDir()
factory, err := New(root, IntelRdtFs)
if err != nil {
t.Fatal(err)
}
if factory == nil {
t.Fatal("factory should not be nil")
}
lfactory, ok := factory.(*LinuxFactory)
if !ok {
t.Fatal("expected linux factory returned on linux based systems")
}
if lfactory.Root != root {
t.Fatalf("expected factory root to be %q but received %q", root, lfactory.Root)
}

if factory.Type() != "libcontainer" {
t.Fatalf("unexpected factory type: %q, expected %q", factory.Type(), "libcontainer")
}
}

func TestFactoryNewTmpfs(t *testing.T) {
root := t.TempDir()
factory, err := New(root, TmpfsRoot)
Expand Down Expand Up @@ -157,7 +135,7 @@ func TestFactoryLoadContainer(t *testing.T) {
if err := marshal(filepath.Join(root, id, stateFilename), expectedState); err != nil {
t.Fatal(err)
}
factory, err := New(root, IntelRdtFs)
factory, err := New(root)
if err != nil {
t.Fatal(err)
}
Expand Down
Loading
Loading