Follow up on PR #9634 to clean up redundant code #9660
Conversation
|
Hi @HirazawaUi. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
WalkthroughRefactors addSysfsMounts usage and signatures: callers derive a boolean usernsEnabled from container ID mappings instead of passing mapping objects and sandbox; platform-specific implementations updated to accept (hostNet, usernsEnabled) and Linux logic now branches on that flag for /sys and cgroup mounts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)server/container_create_linux.go (1)
⏰ 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). (44)
🔇 Additional comments (3)
Comment |
Signed-off-by: HirazawaUi <[email protected]>
e862125 to
01b2c74
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9660 +/- ##
==========================================
- Coverage 67.28% 67.13% -0.15%
==========================================
Files 208 208
Lines 28983 28978 -5
==========================================
- Hits 19500 19454 -46
- Misses 7819 7863 +44
+ Partials 1664 1661 -3 🚀 New features to boost your workflow:
|
| } | ||
| }() | ||
|
|
||
| containerMappings, err := s.getSandboxIDMappings(ctx, sb) |
There was a problem hiding this comment.
I noticed that the variable containerIDMappings has already been created in this function, so I cleaned it up :)
|
/assign @haircommander |
|
/ok-to-test |
|
@HirazawaUi: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
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. |
|
/ok-to-test |
|
/retest |
|
/lgtm |
|
/retest |
|
@cri-o/cri-o-maintainers PTAL |
|
/approve thank you! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, HirazawaUi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Follow up on PR #9634 to clean up redundant code
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.