Skip to content

expose Pod assigned IPs to NRI plugins #8731

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
Merged
merged 2 commits into from
Nov 4, 2024
Merged

expose Pod assigned IPs to NRI plugins #8731

merged 2 commits into from
Nov 4, 2024

Conversation

@aojea
Copy link
Contributor

@aojea aojea commented Nov 1, 2024 •
edited
Loading

/kind feature

Follow up on containerd/nri#119

Expose the assigned IPs to the Pod sandbox to the NRI plugins

Does this PR introduce a user-facing change?

No

nri plugins can obtain access to the assigned Pod IPs on the PodSandbox hooks

@aojea aojea requested a review from mrunalp as a code owner November 1, 2024 10:13
@openshift-ci openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/flake Categorizes issue or PR as related to a flaky test. labels Nov 1, 2024
@openshift-ci openshift-ci bot requested review from klihub and kwilczynski November 1, 2024 10:13
@openshift-ci openshift-ci bot added the kind/other Categorizes issue or PR as not clearly related to any existing kind/* category label Nov 1, 2024
@aojea
Copy link
Contributor Author

aojea commented Nov 1, 2024

/assign @saschagrunert

Sending to run on CI, not familiar with the policies about dependencies in crio, so you may want to wait for the tagged release of NRI, so feel free to hold until that

aojea
aojea commented Nov 1, 2024
View reviewed changes
@codecov
Copy link

codecov bot commented Nov 1, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 46.42%. Comparing base (671833b) to head (9c008e8).
Report is 18 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8731      +/-   ##
==========================================
+ Coverage   46.41%   46.42%   +0.01%     
==========================================
  Files         151      150       -1     
  Lines       21999    21964      -35     
==========================================
- Hits        10210    10197      -13     
+ Misses      10720    10698      -22     
  Partials     1069     1069

saschagrunert
saschagrunert reviewed Nov 1, 2024
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

Thanks! Just a nit about the release.

@aojea
Copy link
Contributor Author

aojea commented Nov 1, 2024

@saschagrunert updated the PR to use the latest tag

@haircommander
Copy link
Member

/approve

LGTM, thanks!

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 1, 2024
@aojea
expose Pod assigned IPs to NRI plugins
9c008e8 
Add a new getter to the sandbox object GetIPs that wraps the existing
one to match the NRI interface and avoid drifting of the methods.

Signed-off-by: Antonio Ojea <[email protected]>
saschagrunert
saschagrunert approved these changes Nov 4, 2024
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

Thanks!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 4, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, haircommander, saschagrunert

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [haircommander,saschagrunert]

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

@saschagrunert
Copy link
Member

/retest

@openshift-merge-bot openshift-merge-bot bot merged commit 91bf170 into cri-o:main Nov 4, 2024
68 of 70 checks passed
@github-actions github-actions bot mentioned this pull request Dec 22, 2024
Open
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

@klihub klihub Awaiting requested review from klihub

@kwilczynski kwilczynski Awaiting requested review from kwilczynski

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/feature Categorizes issue or PR as related to a new feature. kind/flake Categorizes issue or PR as related to a flaky test. 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.

4 participants

@aojea @haircommander @saschagrunert @kwilczynski