Merge pull request #4832 from marquiz/devel/rdt-enablemonitoring

libcontainer/intelrdt: add support for EnableMonitoring field

features.go

@@ -56,8 +56,9 @@ var featuresCommand = cli.Command{
 					Enabled: &t,
 				},
 				IntelRdt: &features.IntelRdt{
-					Enabled:  &t,
-					Schemata: &t,
+					Enabled:    &t,
+					Schemata:   &t,
+					Monitoring: &t,
 				},
 				MemoryPolicy: &features.MemoryPolicy{
 					Modes: specconv.KnownMemoryPolicyModes(),

libcontainer/configs/intelrdt.go

@@ -17,4 +17,7 @@ type IntelRdt struct {
 	// The unit of memory bandwidth is specified in "percentages" by
 	// default, and in "MBps" if MBA Software Controller is enabled.
 	MemBwSchema string `json:"memBwSchema,omitempty"`
+
+	// Create a monitoring group for the container.
+	EnableMonitoring bool `json:"enableMonitoring,omitempty"`
 }

libcontainer/configs/validate/intelrdt_test.go

@@ -19,22 +19,17 @@ func TestValidateIntelRdt(t *testing.T) {
 		isErr      bool
 	}{
 		{
-			name:       "rdt not supported, no config",
-			rdtEnabled: false,
-			config:     nil,
-			isErr:      false,
+			name: "rdt not supported, no config",
 		},
 		{
-			name:       "rdt not supported, with config",
-			rdtEnabled: false,
-			config:     &configs.IntelRdt{},
-			isErr:      true,
+			name:   "rdt not supported, with config",
+			config: &configs.IntelRdt{},
+			isErr:  true,
 		},
 		{
 			name:       "empty config",
 			rdtEnabled: true,
 			config:     &configs.IntelRdt{},
-			isErr:      false,
 		},
 		{
 			name:       "root clos",
@@ -42,7 +37,6 @@ func TestValidateIntelRdt(t *testing.T) {
 			config: &configs.IntelRdt{
 				ClosID: "/",
 			},
-			isErr: false,
 		},
 		{
 			name:       "invalid ClosID (.)",
@@ -71,7 +65,6 @@ func TestValidateIntelRdt(t *testing.T) {
 		{
 			name:       "cat not supported",
 			rdtEnabled: true,
-			catEnabled: false,
 			config: &configs.IntelRdt{
 				L3CacheSchema: "0=ff",
 			},
@@ -80,7 +73,6 @@ func TestValidateIntelRdt(t *testing.T) {
 		{
 			name:       "mba not supported",
 			rdtEnabled: true,
-			mbaEnabled: false,
 			config: &configs.IntelRdt{
 				MemBwSchema: "0=100",
 			},
@@ -96,7 +88,6 @@ func TestValidateIntelRdt(t *testing.T) {
 				L3CacheSchema: "0=ff",
 				MemBwSchema:   "0=100",
 			},
-			isErr: false,
 		},
 	}
 	for _, tc := range testCases {

libcontainer/container_linux.go

@@ -72,6 +72,11 @@ type State struct {
 
 	// Intel RDT "resource control" filesystem path.
 	IntelRdtPath string `json:"intel_rdt_path,omitempty"`
+
+	// Path of the container specific monitoring group in resctrl filesystem.
+	// Empty if the container does not have aindividual dedicated monitoring
+	// group.
+	IntelRdtMonPath string `json:"intel_rdt_mon_path,omitempty"`
 }
 
 // ID returns the container's unique ID
@@ -911,8 +916,10 @@ func (c *Container) currentState() *State {
 	}
 
 	intelRdtPath := ""
+	intelRdtMonPath := ""
 	if c.intelRdtManager != nil {
 		intelRdtPath = c.intelRdtManager.GetPath()
+		intelRdtMonPath = c.intelRdtManager.GetMonPath()
 	}
 	state := &State{
 		BaseState: BaseState{
@@ -925,6 +932,7 @@ func (c *Container) currentState() *State {
 		Rootless:            c.config.RootlessEUID && c.config.RootlessCgroups,
 		CgroupPaths:         c.cgroupManager.GetPaths(),
 		IntelRdtPath:        intelRdtPath,
+		IntelRdtMonPath:     intelRdtMonPath,
 		NamespacePaths:      make(map[configs.NamespaceType]string),
 		ExternalDescriptors: externalDescriptors,
 	}

libcontainer/intelrdt/intelrdt.go

@@ -468,22 +468,41 @@ func (m *Manager) Apply(pid int) (err error) {
 		return newLastCmdError(err)
 	}
 
+	// Create MON group
+	if monPath := m.GetMonPath(); monPath != "" {
+		if err := os.Mkdir(monPath, 0o755); err != nil && !os.IsExist(err) {
+			return newLastCmdError(err)
+		}
+		if err := WriteIntelRdtTasks(monPath, pid); err != nil {
+			return newLastCmdError(err)
+		}
+	}
+
 	m.path = path
 	return nil
 }
 
 // Destroy destroys the Intel RDT container-specific container_id group.
 func (m *Manager) Destroy() error {
+	if m.config.IntelRdt == nil {
+		return nil
+	}
 	// Don't remove resctrl group if closid has been explicitly specified. The
 	// group is likely externally managed, i.e. by some other entity than us.
 	// There are probably other containers/tasks sharing the same group.
-	if m.config.IntelRdt != nil && m.config.IntelRdt.ClosID == "" {
+	if m.config.IntelRdt.ClosID == "" {
 		m.mu.Lock()
 		defer m.mu.Unlock()
 		if err := os.Remove(m.GetPath()); err != nil && !os.IsNotExist(err) {
 			return err
 		}
 		m.path = ""
+	} else if monPath := m.GetMonPath(); monPath != "" {
+		// If ClosID is not specified the possible monintoring group was
+		// removed with the CLOS above.
+		if err := os.Remove(monPath); err != nil && !os.IsNotExist(err) {
+			return err
+		}
 	}
 	return nil
 }
@@ -494,6 +513,21 @@ func (m *Manager) GetPath() string {
 	return m.path
 }
 
+// GetMonPath returns path of the monitoring group of the container. Returns an
+// empty string if the container does not have a individual dedicated
+// monitoring group.
+func (m *Manager) GetMonPath() string {
+	if !m.config.IntelRdt.EnableMonitoring {
+		return ""
+	}
+	closPath := m.GetPath()
+	if closPath == "" {
+		return ""
+	}
+
+	return filepath.Join(closPath, "mon_groups", m.id)
+}
+
 // GetStats returns statistics for Intel RDT.
 func (m *Manager) GetStats() (*Stats, error) {
 	// If intelRdt is not specified in config
@@ -573,7 +607,16 @@ func (m *Manager) GetStats() (*Stats, error) {
 	}
 
 	if IsMBMEnabled() || IsCMTEnabled() {
-		err = getMonitoringStats(containerPath, stats)
+		monPath := m.GetMonPath()
+		if monPath == "" {
+			// NOTE: If per-container monitoring is not enabled, the monitoring
+			// data we get here might have little to do with this container as
+			// there might be anything from this single container to the half
+			// of the system assigned in the group. Should consider not
+			// exposing stats in this case(?)
+			monPath = containerPath
+		}
+		err = getMonitoringStats(monPath, stats)
 		if err != nil {
 			return nil, err
 		}

libcontainer/intelrdt/intelrdt_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"path/filepath"
 	"slices"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -126,31 +127,176 @@ func TestIntelRdtSet(t *testing.T) {
 }
 
 func TestApply(t *testing.T) {
-	helper := NewIntelRdtTestUtil(t)
-
-	const closID = "test-clos"
-	closPath := filepath.Join(helper.IntelRdtPath, closID)
-
-	helper.config.IntelRdt.ClosID = closID
-	intelrdt := newManager(helper.config, "container-1", closPath)
-	if err := intelrdt.Apply(1234); err == nil {
-		t.Fatal("unexpected success when applying pid")
-	}
-	if _, err := os.Stat(closPath); err == nil {
-		t.Fatal("closid dir should not exist")
+	const pid = 1234
+	tests := []struct {
+		name            string
+		config          configs.IntelRdt
+		precreateClos   bool
+		isError         bool
+		postApplyAssert func(*Manager)
+	}{
+		{
+			name: "failure because non-pre-existing CLOS",
+			config: configs.IntelRdt{
+				ClosID: "non-existing-clos",
+			},
+			isError: true,
+			postApplyAssert: func(m *Manager) {
+				if _, err := os.Stat(m.path); err == nil {
+					t.Fatal("closid dir should not exist")
+				}
+			},
+		},
+		{
+			name: "CLOS dir should be created if some schema has been specified",
+			config: configs.IntelRdt{
+				ClosID:        "clos-to-be-created",
+				L3CacheSchema: "L3:0=f",
+			},
+			postApplyAssert: func(m *Manager) {
+				pids, err := getIntelRdtParamString(m.path, "tasks")
+				if err != nil {
+					t.Fatalf("failed to read tasks file: %v", err)
+				}
+				if pids != strconv.Itoa(pid) {
+					t.Fatalf("unexpected tasks file, expected '%d', got %q", pid, pids)
+				}
+			},
+		},
+		{
+			name: "clos and monitoring group should be created if EnableMonitoring is true",
+			config: configs.IntelRdt{
+				EnableMonitoring: true,
+			},
+			precreateClos: true,
+			postApplyAssert: func(m *Manager) {
+				pids, err := getIntelRdtParamString(m.path, "tasks")
+				if err != nil {
+					t.Fatalf("failed to read tasks file: %v", err)
+				}
+				if pids != strconv.Itoa(pid) {
+					t.Fatalf("unexpected tasks file, expected '%d', got %q", pid, pids)
+				}
+			},
+		},
 	}
 
-	// Dir should be created if some schema has been specified
-	intelrdt.config.IntelRdt.L3CacheSchema = "L3:0=f"
-	if err := intelrdt.Apply(1235); err != nil {
-		t.Fatalf("Apply() failed: %v", err)
-	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			NewIntelRdtTestUtil(t)
+			id := "abcd-1234"
+			closPath := filepath.Join(intelRdtRoot, id)
+			if tt.config.ClosID != "" {
+				closPath = filepath.Join(intelRdtRoot, tt.config.ClosID)
+			}
 
-	pids, err := getIntelRdtParamString(intelrdt.GetPath(), "tasks")
-	if err != nil {
-		t.Fatalf("failed to read tasks file: %v", err)
-	}
-	if pids != "1235" {
-		t.Fatalf("unexpected tasks file, expected '1235', got %q", pids)
+			if tt.precreateClos {
+				if err := os.MkdirAll(filepath.Join(closPath, "mon_groups"), 0o755); err != nil {
+					t.Fatal(err)
+				}
+			}
+			m := newManager(&configs.Config{IntelRdt: &tt.config}, id, closPath)
+			err := m.Apply(pid)
+			if tt.isError && err == nil {
+				t.Fatal("expected error, got nil")
+			} else if !tt.isError && err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			tt.postApplyAssert(m)
+		})
+	}
+}
+
+func TestDestroy(t *testing.T) {
+	tests := []struct {
+		name     string
+		config   configs.IntelRdt
+		testFunc func(*Manager)
+	}{
+		{
+			name: "per-container CLOS dir should be removed",
+			testFunc: func(m *Manager) {
+				closPath := m.path
+				if _, err := os.Stat(closPath); err != nil {
+					t.Fatal("CLOS dir should exist")
+				}
+				// Need to delete the tasks file so that the dir is empty
+				if err := os.Remove(filepath.Join(closPath, "tasks")); err != nil {
+					t.Fatalf("failed to remove tasks file: %v", err)
+				}
+				if err := m.Destroy(); err != nil {
+					t.Fatalf("Destroy() failed: %v", err)
+				}
+				if _, err := os.Stat(closPath); err == nil {
+					t.Fatal("CLOS dir should not exist")
+				}
+			},
+		},
+		{
+			name: "pre-existing CLOS should not be removed",
+			config: configs.IntelRdt{
+				ClosID: "pre-existing-clos",
+			},
+			testFunc: func(m *Manager) {
+				closPath := m.path
+
+				if _, err := os.Stat(closPath); err != nil {
+					t.Fatal("CLOS dir should exist")
+				}
+				if err := m.Destroy(); err != nil {
+					t.Fatalf("Destroy() failed: %v", err)
+				}
+				if _, err := os.Stat(closPath); err != nil {
+					t.Fatal("CLOS dir should exist")
+				}
+			},
+		},
+		{
+			name: "per-container MON dir in pre-existing CLOS should be removed",
+			config: configs.IntelRdt{
+				ClosID:           "pre-existing-clos",
+				EnableMonitoring: true,
+			},
+			testFunc: func(m *Manager) {
+				closPath := m.path
+
+				monPath := filepath.Join(closPath, "mon_groups", m.id)
+				if _, err := os.Stat(monPath); err != nil {
+					t.Fatal("MON dir should exist")
+				}
+				// Need to delete the tasks file so that the dir is empty
+				os.Remove(filepath.Join(monPath, "tasks"))
+				if err := m.Destroy(); err != nil {
+					t.Fatalf("Destroy() failed: %v", err)
+				}
+				if _, err := os.Stat(closPath); err != nil {
+					t.Fatalf("CLOS dir should exist: %f", err)
+				}
+				if _, err := os.Stat(monPath); err == nil {
+					t.Fatal("MON dir should not exist")
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			NewIntelRdtTestUtil(t)
+
+			id := "abcd-1234"
+			closPath := filepath.Join(intelRdtRoot, id)
+			if tt.config.ClosID != "" {
+				closPath = filepath.Join(intelRdtRoot, tt.config.ClosID)
+				// Pre-create the CLOS directory
+				if err := os.MkdirAll(filepath.Join(closPath, "mon_groups"), 0o755); err != nil {
+					t.Fatal(err)
+				}
+			}
+			m := newManager(&configs.Config{IntelRdt: &tt.config}, id, closPath)
+			if err := m.Apply(1234); err != nil {
+				t.Fatalf("Apply() failed: %v", err)
+			}
+			tt.testFunc(m)
+		})
 	}
 }

libcontainer/specconv/spec_linux.go

@@ -495,10 +495,11 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 		}
 		if spec.Linux.IntelRdt != nil {
 			config.IntelRdt = &configs.IntelRdt{
-				ClosID:        spec.Linux.IntelRdt.ClosID,
-				Schemata:      spec.Linux.IntelRdt.Schemata,
-				L3CacheSchema: spec.Linux.IntelRdt.L3CacheSchema,
-				MemBwSchema:   spec.Linux.IntelRdt.MemBwSchema,
+				ClosID:           spec.Linux.IntelRdt.ClosID,
+				Schemata:         spec.Linux.IntelRdt.Schemata,
+				L3CacheSchema:    spec.Linux.IntelRdt.L3CacheSchema,
+				MemBwSchema:      spec.Linux.IntelRdt.MemBwSchema,
+				EnableMonitoring: spec.Linux.IntelRdt.EnableMonitoring,
 			}
 		}
 		if spec.Linux.MemoryPolicy != nil {