diff --git a/cmd/plugins/resctrl-mon/plugin.go b/cmd/plugins/resctrl-mon/plugin.go index b90530db5..c627e9582 100644 --- a/cmd/plugins/resctrl-mon/plugin.go +++ b/cmd/plugins/resctrl-mon/plugin.go @@ -74,6 +74,9 @@ func newPlugin() *plugin { // Configure handles connecting to container runtime's NRI server. func (p *plugin) Configure(ctx context.Context, config, runtime, version string) (stub.EventMask, error) { log.Infof("Connected to %s %s...", runtime, version) + if err := checkRuntimeVersion(runtime, version); err != nil { + return 0, err + } if config != "" { log.Debugf("loading configuration from NRI server") if err := p.setConfig([]byte(config)); err != nil { @@ -143,9 +146,6 @@ func (p *plugin) Synchronize(ctx context.Context, pods []*api.PodSandbox, contai continue } pid := int(ctr.GetPid()) - if pid == 0 { - pid = readContainerPID(ctr.GetId()) - } if pid > 0 { monGroupDir := p.state.getMonGroupDir(podUID) if err := p.rdt.writeTaskPID(monGroupDir, pid); err != nil { @@ -275,15 +275,6 @@ func (p *plugin) PostStartContainer(ctx context.Context, pod *api.PodSandbox, ct return nil } - // Fallback: if the NRI event has no PID (CRI-O <= 1.35 does not - // populate Container.Pid), read it from the CRI-O pidfile. - if pid == 0 { - pid = readContainerPID(ctr.GetId()) - if pid > 0 { - log.Debugf("PostStartContainer %s: read pid %d from pidfile", ctrName, pid) - } - } - if pid > 0 { if err := p.rdt.writeTaskPID(monGroupDir, pid); err != nil { log.Warnf("PostStartContainer %s: failed to write PID %d to tasks: %v", ctrName, pid, err) @@ -291,7 +282,7 @@ func (p *plugin) PostStartContainer(ctx context.Context, pod *api.PodSandbox, ct log.Infof("PostStartContainer %s: assigned pid %d to mon_group %s", ctrName, pid, monGroupDir) } } else { - log.Warnf("PostStartContainer %s: PID not available from NRI or pidfile", ctrName) + log.Warnf("PostStartContainer %s: PID=0, cannot assign to mon_group (runtime did not provide PID via NRI)", ctrName) } return nil @@ -396,22 +387,32 @@ func pprintCtr(pod *api.PodSandbox, ctr *api.Container) string { return fmt.Sprintf("%s/%s:%s", pod.GetNamespace(), pod.GetName(), ctr.GetName()) } -// readContainerPID reads the container init PID from the CRI-O pidfile. -// CRI-O versions <= 1.35 do not populate Container.Pid in NRI events. -// As a fallback, we read the PID from the container's pidfile at -// /run/containers/storage/overlay-containers//userdata/pidfile. -// Returns 0 if the PID cannot be read (e.g., running under containerd). -func readContainerPID(containerID string) int { - pidfile := filepath.Join("/run/containers/storage/overlay-containers", containerID, "userdata/pidfile") - data, err := os.ReadFile(pidfile) +// checkRuntimeVersion verifies that the container runtime provides PIDs via NRI. +// CRI-O versions before 1.36 do not populate Container.Pid in NRI events, +// making the plugin unable to assign tasks to monitoring groups. +func checkRuntimeVersion(runtime, version string) error { + if !strings.EqualFold(runtime, "cri-o") { + return nil + } + // Normalize: strip leading "v" and any pre-release/build suffix. + version = strings.TrimPrefix(version, "v") + if idx := strings.IndexAny(version, "-+"); idx != -1 { + version = version[:idx] + } + parts := strings.SplitN(version, ".", 3) + if len(parts) < 2 { + return fmt.Errorf("CRI-O version %q: unable to parse; require >= 1.36 for NRI PID support", version) + } + major, err := strconv.Atoi(parts[0]) if err != nil { - log.Debugf("readContainerPID: cannot read %s: %v", pidfile, err) - return 0 + return fmt.Errorf("CRI-O version %q: unable to parse major version: %w", version, err) } - pid, err := strconv.Atoi(strings.TrimSpace(string(data))) + minor, err := strconv.Atoi(parts[1]) if err != nil { - log.Debugf("readContainerPID: invalid pid in %s: %v", pidfile, err) - return 0 + return fmt.Errorf("CRI-O version %q: unable to parse minor version: %w", version, err) + } + if major < 1 || (major == 1 && minor < 36) { + return fmt.Errorf("CRI-O %s does not provide container PIDs via NRI (requires >= 1.36)", version) } - return pid + return nil } diff --git a/cmd/plugins/resctrl-mon/plugin_test.go b/cmd/plugins/resctrl-mon/plugin_test.go index 272c91754..7633760cb 100644 --- a/cmd/plugins/resctrl-mon/plugin_test.go +++ b/cmd/plugins/resctrl-mon/plugin_test.go @@ -367,3 +367,40 @@ func TestStopContainer_RemovesStateOnRmdirFailure(t *testing.T) { require.NoError(t, err) assert.Equal(t, 0, p.state.podCount()) } + +func TestCheckRuntimeVersion(t *testing.T) { + tests := []struct { + name string + runtime string + version string + wantErr bool + }{ + {"containerd any version", "containerd", "2.0.0", false}, + {"cri-o 1.36.0", "cri-o", "1.36.0", false}, + {"cri-o 1.37.0", "cri-o", "1.37.0", false}, + {"cri-o 2.0.0", "cri-o", "2.0.0", false}, + {"cri-o 1.35.0 rejected", "cri-o", "1.35.0", true}, + {"cri-o 1.35.2 rejected", "cri-o", "1.35.2", true}, + {"cri-o 1.31.5 rejected", "cri-o", "1.31.5", true}, + {"cri-o 0.99.0 rejected", "cri-o", "0.99.0", true}, + {"CRI-O case insensitive", "CRI-O", "1.35.0", true}, + {"cri-o no patch", "cri-o", "1.36", false}, + {"cri-o unparsable", "cri-o", "latest", true}, + {"cri-o v prefix accepted", "cri-o", "v1.36.0", false}, + {"cri-o v prefix rejected", "cri-o", "v1.35.0", true}, + {"cri-o strips pre-release suffix", "cri-o", "1.36.0-rc1", false}, + {"cri-o strips pre-release old version", "cri-o", "1.35.0-beta.1", true}, + {"cri-o strips build metadata", "cri-o", "1.36.0+build123", false}, + {"cri-o strips v prefix and pre-release", "cri-o", "v1.35.0-alpha.0", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkRuntimeVersion(tt.runtime, tt.version) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/deployment/helm/resctrl-mon/README.md b/deployment/helm/resctrl-mon/README.md index 206e3eced..b77809acf 100644 --- a/deployment/helm/resctrl-mon/README.md +++ b/deployment/helm/resctrl-mon/README.md @@ -30,8 +30,10 @@ to support Application Energy Telemetry (AET) via Kepler passive mode. installation. - CRI-O - - At least [v1.26.0](https://github.com/cri-o/cri-o/releases/tag/v1.26.0) - release version to use the NRI feature + - At least [v1.36.0](https://github.com/cri-o/cri-o/releases/tag/v1.36.0) + release version. CRI-O >= 1.36 is required because earlier versions do + not provide container PIDs via NRI, which prevents the plugin from + assigning tasks to monitoring groups. - Enable NRI feature by following [these](https://github.com/cri-o/cri-o/blob/main/docs/crio.conf.5.md#crionri-table) detailed instructions. You can optionally enable the NRI in CRI-O using diff --git a/docs/monitoring/resctrl-mon.md b/docs/monitoring/resctrl-mon.md index 7cd6c3198..ec98a0aa3 100644 --- a/docs/monitoring/resctrl-mon.md +++ b/docs/monitoring/resctrl-mon.md @@ -86,8 +86,10 @@ RMID allocation is delegated entirely to the Linux kernel: ### Prerequisites -- Containerd v1.7+ or CRI-O v1.26+ -- Enable NRI in /etc/containerd/config.toml: +- Containerd v1.7+ or CRI-O v1.36+ +- Enable NRI in the container runtime: + + **containerd** — in `/etc/containerd/config.toml`: ```toml [plugins."io.containerd.nri.v1.nri"] @@ -100,6 +102,16 @@ RMID allocation is delegated entirely to the Linux kernel: socket_path = "/var/run/nri/nri.sock" ``` + **CRI-O** — in `/etc/crio/crio.conf.d/10-nri.conf` (or equivalent): + + ```toml + [crio.nri] + enable_nri = true + ``` + + See the [CRI-O NRI documentation](https://github.com/cri-o/cri-o/blob/main/docs/crio.conf.5.md#crionri-table) + for additional options. + - Intel CPU with RDT monitoring support - resctrl filesystem mounted at `/sys/fs/resctrl`