Allow containers to use both host network and user namespace by HirazawaUi · Pull Request #9634 · cri-o/cri-o

HirazawaUi · 2025-12-03T14:51:21Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This is the implementation of KEP-5607 in cri-o.

In KEP-5607, we allow containers to use both user namespace and host network simultaneously. This requires us to change the sys mount to a bind mount when the user employs both user namespace and host network, because mounting sys in the standard way is a privileged operation, which would prevent the container from using the standard /sys mount.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Allow containers to use both host network and user namespace.

Summary by CodeRabbit

Bug Fixes & Improvements
- System filesystem mounts inside containers now respect user-namespace settings, improving isolation and compatibility when host networking is used.
- Sandbox ID mappings are consulted during container filesystem setup to ensure consistent ownership and access behavior across environments.
- Container startup now fails early on mapping retrieval errors, adding clearer error handling for more reliable cross-platform initialization.

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

openshift-ci · 2025-12-03T14:51:34Z

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

coderabbitai · 2025-12-03T14:51:41Z

Walkthrough

Fetches sandbox ID mappings before preparing sysfs mounts and passes both the sandbox and mappings into addSysfsMounts; addSysfsMounts signature updated across platforms and Linux sysfs mounting now branches based on presence of user namespace mappings.

Changes

Cohort / File(s)	Summary
Container creation flow `server/container_create.go`	Retrieve sandbox ID mappings via `s.getSandboxIDMappings(ctx, sb)` with error handling, then call `addSysfsMounts(ctr, containerConfig, hostNet, sb, containerMappings)` instead of the previous call.
Function signature updates `server/container_create_freebsd.go`, `server/container_create_linux.go`	Change `addSysfsMounts` signature to add parameters `sb sandbox.Sandbox` and `containerIDMappings idtools.IDMappings`; reorder imports in FreeBSD file and add the extra parameters to callers.
User namespace-aware sysfs mounting (Linux) `server/container_create_linux.go`	Introduce `usernsEnabled := containerIDMappings != nil` and branch host-network `/sys` mounting: when userns enabled, use a read-only bind mount from `/sys` (ro,rbind); otherwise mount `/sys` as sysfs and add cgroup `/sys` mount as before.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Verify all callers of addSysfsMounts updated to the new signature.
Inspect s.getSandboxIDMappings error handling and propagation.
Review Linux mount branching for correctness (bind vs sysfs) and mount options.
Confirm FreeBSD stub/signature change is consistent and builds cleanly.

Poem

🐇 I hopped through mounts at break of day,
I fetched the mappings, then found my way,
If userns sleeps, sysfs sings free,
If userspace wakes, I bind gently—whee!
New paths, light paws, a small hooray.

Pre-merge checks and finishing touches

❌ 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%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main objective of the PR: enabling containers to use both host network and user namespace simultaneously, which is the core feature implemented across the changed files.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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

saschagrunert · 2025-12-03T14:53:35Z

Thank youf for the PR @HirazawaUi, please sign-off your commit.

/ok-to-test

giuseppe

LGTM

openshift-ci · 2025-12-03T15:16:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, HirazawaUi

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:

~~OWNERS~~ [giuseppe]

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

saschagrunert · 2025-12-03T15:34:41Z

/lgtm cancel

Please fix the linter: https://github.com/cri-o/cri-o/actions/runs/19898717159/job/57037105155?pr=9634

codecov · 2025-12-03T18:08:42Z

Codecov Report

❌ Patch coverage is 64.51613% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.90%. Comparing base (78732e4) to head (6bb8a38).
⚠️ Report is 34 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9634      +/-   ##
==========================================
- Coverage   66.97%   66.90%   -0.07%     
==========================================
  Files         205      205              
  Lines       28826    28842      +16     
==========================================
- Hits        19306    19298       -8     
- Misses       7867     7890      +23     
- Partials     1653     1654       +1

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: HirazawaUi <695097494plus@gmail.com>

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

server/container_create_linux.go (1)
870-871: Tidy up addSysfsMounts parameters and userns detection (optional)

Two minor cleanups you might consider:

sb *sandbox.Sandbox is currently unused in this helper; if you don’t plan to use it soon, you could drop it to keep the function signature minimal and avoid potential linter complaints about unused parameters.

If there’s any chance of passing an “empty” but non‑nil IDMappings, usernsEnabled could track that more explicitly:
-func addSysfsMounts(ctr ctrfactory.Container, containerConfig *types.ContainerConfig, hostNet bool, sb *sandbox.Sandbox, containerIDMappings *idtools.IDMappings) {
-    usernsEnabled := containerIDMappings != nil
+func addSysfsMounts(ctr ctrfactory.Container, containerConfig *types.ContainerConfig, hostNet bool, sb *sandbox.Sandbox, containerIDMappings *idtools.IDMappings) {
+    usernsEnabled := containerIDMappings != nil && !containerIDMappings.Empty()
Not mandatory, but would make the intent around “userns actually in use” a bit clearer.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfe7542 and 6bb8a38.

📒 Files selected for processing (3)

server/container_create.go (1 hunks)
server/container_create_freebsd.go (2 hunks)
server/container_create_linux.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

server/container_create.go
server/container_create_freebsd.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/container_create_linux.go

🧬 Code graph analysis (1)

server/container_create_linux.go (2)

internal/oci/container.go (1)

Container (44-92)

internal/factory/container/container.go (1)

Container (40-142)

⏰ 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). (18)

GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
GitHub Check: rpm-build:centos-stream-9-x86_64: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-aarch64:fedora-rawhide
GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide
GitHub Check: rpm-build:centos-stream-9-aarch64:fedora-rawhide
GitHub Check: rpm-build:centos-stream-9-x86_64:fedora-rawhide
GitHub Check: rpm-build:fedora-43-x86_64:fedora-rawhide
GitHub Check: rpm-build:fedora-rawhide-aarch64:fedora-rawhide
GitHub Check: rpm-build:fedora-rawhide-x86_64:fedora-rawhide
GitHub Check: rpm-build:fedora-43-aarch64:fedora-rawhide

🔇 Additional comments (1)

server/container_create_linux.go (1)

870-898: HostNetwork + userns handling for /sys and cgroups looks correct

The new usernsEnabled branch that bind‑mounts /sys for hostNetwork pods with user namespaces cleanly avoids re‑mounting sysfs in that configuration, while the non‑userns path still mounts a fresh sysfs plus the cgroup filesystem as before. This keeps existing behavior for non‑userns pods and aligns with the KEP intent for hostNetwork+userns pods, including interaction with hasCgroupMount / setupSystemd.

HirazawaUi · 2025-12-04T15:18:39Z

Please fix the linter: https://github.com/cri-o/cri-o/actions/runs/19898717159/job/57037105155?pr=9634

Thanks for the heads up. The lint error has been fixed.

bitoku · 2025-12-11T15:38:22Z

/lgtm

bitoku · 2025-12-11T16:26:13Z

/retest

bitoku · 2025-12-11T18:43:31Z

/retest

haircommander · 2025-12-11T19:36:32Z

 }

-func addSysfsMounts(ctr ctrfactory.Container, containerConfig *types.ContainerConfig, hostNet bool) {
+func addSysfsMounts(ctr ctrfactory.Container, containerConfig *types.ContainerConfig, hostNet bool, sb *sandbox.Sandbox, containerIDMappings *idtools.IDMappings) {


I think this would be better as hostUser and do the usernsEnabled check in the caller. WDYT?

Of course. This code was modified in PR #9660, and some additional cleanup was performed.

Signed-off-by: HirazawaUi <695097494plus@gmail.com>

Follow up on PR #9634 to clean up redundant code

HirazawaUi requested a review from mrunalp as a code owner December 3, 2025 14:51

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: no Indicates the PR's author has not DCO signed all their commits. labels Dec 3, 2025

openshift-ci Bot requested review from QiWang19 and hasan4791 December 3, 2025 14:51

openshift-ci Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 3, 2025

openshift-ci Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 3, 2025

giuseppe reviewed Dec 3, 2025

View reviewed changes

Comment thread server/container_create_linux.go

HirazawaUi changed the title ~~Allow user namespace with hostNetwork in container~~ Allow containers to use both host network and user namespace Dec 3, 2025

HirazawaUi force-pushed the support-hostnetwork-userns branch from 4c89848 to 3a70524 Compare December 3, 2025 14:58

openshift-ci Bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Dec 3, 2025

HirazawaUi force-pushed the support-hostnetwork-userns branch from 3a70524 to dfe7542 Compare December 3, 2025 15:11

giuseppe approved these changes Dec 3, 2025

View reviewed changes

openshift-ci Bot assigned giuseppe Dec 3, 2025

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2025

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 3, 2025

openshift-ci Bot assigned saschagrunert Dec 3, 2025

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2025

kannon92 mentioned this pull request Dec 3, 2025

REQUEST: Enable coderabbit github integration for jobset and kueue kubernetes/org#5930

Closed

Allow containers to use both host network and user namespace

6bb8a38

Signed-off-by: HirazawaUi <695097494plus@gmail.com>

HirazawaUi force-pushed the support-hostnetwork-userns branch from dfe7542 to 6bb8a38 Compare December 4, 2025 13:59

coderabbitai Bot reviewed Dec 4, 2025

View reviewed changes

openshift-ci Bot assigned bitoku Dec 11, 2025

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2025

bitoku added this to the 1.35 milestone Dec 11, 2025

haircommander reviewed Dec 11, 2025

View reviewed changes

openshift-merge-bot Bot merged commit 22417ca into cri-o:main Dec 11, 2025
69 of 71 checks passed

HirazawaUi added a commit to HirazawaUi/cri-o that referenced this pull request Dec 13, 2025

Follow up on PR cri-o#9634 to clean up redundant code.

d3ae3ff

HirazawaUi added a commit to HirazawaUi/cri-o that referenced this pull request Dec 13, 2025

Follow up on PR cri-o#9634 to clean up redundant code.

e862125

Signed-off-by: HirazawaUi <695097494plus@gmail.com>

HirazawaUi mentioned this pull request Dec 13, 2025

Follow up on PR #9634 to clean up redundant code #9660

Merged

openshift-merge-bot Bot added a commit that referenced this pull request Dec 23, 2025

Merge pull request #9660 from HirazawaUi/follow-up-9634

3f37af3

Follow up on PR #9634 to clean up redundant code

coderabbitai Bot mentioned this pull request Jan 14, 2026

server: Always include UID/GID mappings for user namespace containers #9712

Merged

coderabbitai Bot mentioned this pull request May 6, 2026

Set the default of runtimeFeatures.UserNamespacesHostNetwork to true #9930

Merged

coderabbitai Bot mentioned this pull request Jun 8, 2026

User namespace - Handle UID/GID mappings from the infra container's config.json #10014

Open

Conversation

HirazawaUi commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Summary by CodeRabbit

Uh oh!

openshift-ci Bot commented Dec 3, 2025

Uh oh!

coderabbitai Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

saschagrunert commented Dec 3, 2025

Uh oh!

Uh oh!

giuseppe left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Dec 3, 2025

Uh oh!

saschagrunert commented Dec 3, 2025

Uh oh!

codecov Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

HirazawaUi commented Dec 4, 2025

Uh oh!

bitoku commented Dec 11, 2025

Uh oh!

bitoku commented Dec 11, 2025

Uh oh!

bitoku commented Dec 11, 2025

Uh oh!

haircommander Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

HirazawaUi Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

HirazawaUi commented Dec 3, 2025 •

edited

Loading

coderabbitai Bot commented Dec 3, 2025 •

edited

Loading

codecov Bot commented Dec 3, 2025 •

edited

Loading