Skip to content

server,nri: pass any POSIX rlimits to plugins.#9707

Merged
openshift-merge-bot[bot] merged 1 commit into
cri-o:mainfrom
klihub:devel/main/nri-input/rlimits
Jan 13, 2026
Merged

server,nri: pass any POSIX rlimits to plugins.#9707
openshift-merge-bot[bot] merged 1 commit into
cri-o:mainfrom
klihub:devel/main/nri-input/rlimits

Conversation

@klihub

@klihub klihub commented Jan 12, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind other

What this PR does / why we need it:

Add missing support for passing any container POSIX rlimits to NRI plugins as input.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

- nri: pass any container POSIX rlimits to NRI plugins as input.

Summary by CodeRabbit

  • New Features
    • Containers now surface POSIX resource limits through the NRI interface, with NRI container records populated with configured rlimit values for improved visibility into runtime constraints.

✏️ Tip: You can customize this high-level summary in your review settings.

@klihub klihub requested a review from mrunalp as a code owner January 12, 2026 14:32
@openshift-ci openshift-ci Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/other Categorizes issue or PR as not clearly related to any existing kind/* category labels Jan 12, 2026
@openshift-ci openshift-ci Bot requested review from bitoku and hasan4791 January 12, 2026 14:32
@openshift-ci openshift-ci Bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Jan 12, 2026
@coderabbitai

coderabbitai Bot commented Jan 12, 2026 •
edited
Loading

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Added a GetRlimits() accessor to the Container interface and implemented it on criContainer; container-to-NRI mapping now populates the Rlimits field from this accessor, exposing POSIX rlimit entries collected from the container spec.

Changes

Cohort / File(s) Summary
Container interface & mapping
internal/nri/container.go		 Added GetRlimits() []*nri.POSIXRlimit to the Container interface; containerToNRI() now sets Rlimits using ctr.GetRlimits().
criContainer implementation
server/nri-api.go		 Added func (c *criContainer) GetRlimits() []*api.POSIXRlimit which iterates spec.Process.Rlimits (returns nil if spec.Process is nil) and returns slice of POSIX rlimit entries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰
Hippity-hop, limits in view,
I fetched rlimits just for you.
From spec to NRI they flow,
A tiny bridge to help things grow.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding POSIX rlimits support to NRI plugins, which is exactly what the changeset implements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c282b5 and 8df271a.

📒 Files selected for processing (2)
  • internal/nri/container.go
  • server/nri-api.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/nri/container.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use interface-based design and dependency injection patterns in Go code
Propagate context.Context through function calls in Go code
Use fmt.Errorf with %w for error wrapping in Go code
Use logrus with structured fields for logging in Go code
Add comments explaining 'why' not 'what' in Go code
Use platform-specific file naming: *_{linux,freebsd}.go for platform-dependent code

Files:

  • server/nri-api.go
🧠 Learnings (2)
📚 Learning: 2025-12-17T13:38:34.646Z 
Learnt from: bitoku
Repo: cri-o/cri-o PR: 9667
File: server/container_create.go:1233-1236
Timestamp: 2025-12-17T13:38:34.646Z
Learning: In the cri-o/cri-o repository, protobuf-generated Get* methods for k8s.io/cri-api types are nil-safe: if the receiver is nil, GetX() returns the zero value instead of panicking. Do not add explicit nil checks before chaining calls on such getters. Apply this guidance to all Go code that uses these generated getters across the codebase.

Applied to files:

  • server/nri-api.go
📚 Learning: 2025-12-18T13:28:24.244Z 
Learnt from: bitoku
Repo: cri-o/cri-o PR: 9676
File: internal/lib/stats/cgroup_stats_unsupported.go:1-7
Timestamp: 2025-12-18T13:28:24.244Z
Learning: In the cri-o/cri-o repository, for platform-specific types guarded by Go build tags (for example //go:build !linux), implement empty structs for unsupported platforms to permit compilation and clearly indicate the feature is not available rather than mirroring the Linux struct with unpopulated fields. Apply this pattern to all relevant platform-specific files across the codebase (i.e., any file under build-taged sections that should compile on all targets but lacks full implementation for some platforms).

Applied to files:

  • server/nri-api.go
🧬 Code graph analysis (1)
server/nri-api.go (3)
vendor/github.com/containerd/nri/pkg/adaptation/api.go (1)
  • POSIXRlimit (104-104)
vendor/github.com/containerd/nri/pkg/api/api.pb.go (3)
  • POSIXRlimit (3410-3418)
  • POSIXRlimit (3433-3433)
  • POSIXRlimit (3448-3450)
vendor/github.com/opencontainers/runtime-spec/specs-go/config.go (1)
  • Process (67-101)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
  • GitHub Check: integration / userns / crun / amd64
  • GitHub Check: integration / conmon / crun / amd64
  • GitHub Check: critest / conmon-rs / crun / amd64
  • GitHub Check: critest / conmon / crun / amd64
  • GitHub Check: integration / conmon / crun / arm64
  • GitHub Check: critest / conmon / crun / arm64
  • GitHub Check: critest / conmon-rs / crun / arm64
  • GitHub Check: integration / conmon-rs / crun / amd64
  • GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
  • GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
  • GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
  • GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
  • GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
  • GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
  • GitHub Check: security-checks
  • GitHub Check: build static / amd64
  • GitHub Check: build static / s390x
  • GitHub Check: lint
  • GitHub Check: build static / arm64
  • GitHub Check: build static / ppc64le
  • GitHub Check: unit / arm64 / root
  • GitHub Check: build-freebsd
  • GitHub Check: codeql-build
  • GitHub Check: unit / amd64 / rootless
  • GitHub Check: build
  • GitHub Check: unit / amd64 / root
  • GitHub Check: docs
🔇 Additional comments (1)
server/nri-api.go (1)

829-846: LGTM!

The implementation is correct and consistent with similar accessor methods in this file (e.g., GetUser, GetIOPriority). The nil check on spec.Process, pre-allocation with capacity, and field mapping are all appropriate. The type conversion from OCI's POSIXRlimit to NRI's api.POSIXRlimit correctly maps Type, Hard, and Soft fields.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@klihub klihub force-pushed the devel/main/nri-input/rlimits branch from 7c282b5 to b7f8ba3 Compare January 12, 2026 14:50
@codecov

codecov Bot commented Jan 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.47%. Comparing base (eba847c) to head (8df271a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #9707      +/-   ##
==========================================
+ Coverage   64.39%   64.47%   +0.08%     
==========================================
  Files         209      209              
  Lines       29059    29075      +16     
==========================================
+ Hits        18713    18747      +34     
+ Misses       8664     8651      -13     
+ Partials     1682     1677       -5
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@klihub

klihub commented Jan 13, 2026

Copy link
Copy Markdown
Contributor Author

/retest-required

@openshift-ci openshift-ci Bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 13, 2026
@saschagrunert

saschagrunert commented Jan 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

/lgtm cancel

This now needs a rebase, code LGTM. 👍

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 13, 2026
@klihub
server,nri: pass any POSIX rlimits to plugins.
8df271a 
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@klihub klihub force-pushed the devel/main/nri-input/rlimits branch from b7f8ba3 to 8df271a Compare January 13, 2026 07:35
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jan 13, 2026
@openshift-ci

openshift-ci Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: klihub, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@klihub

klihub commented Jan 13, 2026

Copy link
Copy Markdown
Contributor Author

/retest-required

@openshift-merge-bot openshift-merge-bot Bot merged commit 8e35b9d into cri-o:main Jan 13, 2026
70 of 71 checks passed
@openshift-ci

openshift-ci Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

@klihub: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ci-cgroupv2-e2e 8df271a link unknown /test ci-cgroupv2-e2e
ci/prow/ci-cgroupv2-e2e-features 8df271a link unknown /test ci-cgroupv2-e2e-features
ci/prow/e2e-gcp-ovn 8df271a link unknown /test e2e-gcp-ovn

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@klihub klihub deleted the devel/main/nri-input/rlimits branch January 14, 2026 08:03
@coderabbitai coderabbitai Bot mentioned this pull request Jan 14, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@saschagrunert saschagrunert saschagrunert approved these changes

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner

@bitoku bitoku Awaiting requested review from bitoku

@hasan4791 hasan4791 Awaiting requested review from hasan4791

Assignees

@saschagrunert saschagrunert

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/other Categorizes issue or PR as not clearly related to any existing kind/* category lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@klihub @saschagrunert