Skip to content

Conversation

@aojea
Copy link

@aojea aojea commented Oct 7, 2025

It turns out the bindmounth path used was not the same that the one used by iproute2, existing one is /run/netns but iproute2 uses /var/run/netns.

It seems that in FHS 3.0, /var/run is replaced by /run; a system should either continue to provide a /var/run directory or provide a symbolic link from /var/run to /run for backward compatibility.

When using kubernetes pods or containers, users just mount one of the folders but the OS will not do the symlink, so is on users the responsability to provide the right path. To keep the same compatibility we just implement a fallback from /run/netns to /var/run/netns so user will have the same experience when dealing with network namespace inside Pods or containers.

Fixes: #106

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability when creating, opening, and deleting named network namespaces by adding fallback path resolution, reducing errors across different environments.
  • Refactor
    • Centralized path handling for network namespaces to ensure consistent and predictable behavior without changing public interfaces.

It turns out the bindmounth path used was not the same that the one used
by iproute2, existing one is /run/netns but iproute2 uses
/var/run/netns.

It seems that in FHS 3.0, /var/run is replaced by /run; a system should
either continue to provide a /var/run directory or provide a symbolic
link from /var/run to /run for backward compatibility.

When using kubernetes pods or containers, users just mount one of the
folders but the OS will not do the symlink, so is on users the
responsability to provide the right path. To keep the same compatibility
we just implement a fallback from /run/netns to /var/run/netns so user
will have the same experience when dealing with network namespace inside
Pods or containers.

Signed-off-by: Antonio Ojea <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Oct 7, 2025

Walkthrough

Centralized network namespace path resolution by introducing bind and fallback mount paths with helper functions to select existing directories. Updated NewNamed, DeleteNamed, and GetFromName to use these helpers for creating, locating, and opening named namespaces without changing public APIs.

Changes

Cohort / File(s) Summary
Netns path resolution helpers and call-site updates
netns_linux.go
Added bindMountPath and fallbackMountPath constants; introduced getNetnsPath and getNetnsPathForName to resolve namespace dirs. Refactored NewNamed, DeleteNamed, and GetFromName to use these helpers for directory creation and lookup with fallback between /run/netns and /var/run/netns.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Netns as netns package
  participant FS as Filesystem

  rect rgba(200,230,255,0.3)
  note over Netns: Path resolution (new)
  Caller->>Netns: NewNamed(name)
  Netns->>FS: exists(/run/netns)?
  alt /run/netns exists
    Netns-->>Netns: dir = /run/netns
  else /var/run/netns exists
    Netns-->>Netns: dir = /var/run/netns
  else none
    Netns-->>Netns: dir = /run/netns (default)
  end
  Netns->>FS: mkdir/open dir/name
  Netns-->>Caller: handle/error
  end

  rect rgba(200,255,200,0.3)
  note over Netns: Lookup with fallback (new)
  Caller->>Netns: GetFromName(name)
  Netns->>FS: check /run/netns/name
  alt found
    Netns-->>Netns: path = /run/netns/name
  else not found
    Netns->>FS: check /var/run/netns/name
    alt found
      Netns-->>Netns: path = /var/run/netns/name
    else not found
      Netns-->>Netns: path = getNetnsPath()/name
    end
  end
  Netns->>FS: open(path)
  Netns-->>Caller: handle/error
  end

  rect rgba(255,230,200,0.3)
  note over Netns: Deletion with fallback (new)
  Caller->>Netns: DeleteNamed(name)
  Netns->>FS: resolve path via getNetnsPathForName
  Netns->>FS: unlink(path)
  Netns-->>Caller: ok/error
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I hop between /run and /var with glee,
Two burrows for one netns tree.
If one path’s dark, the other’s bright—
I sniff, I check, I choose the right.
With gentle paws, I mount and roam—
Now every namespace finds its home. 🐇🗺️

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title directly reflects the primary change of adding a fallback for the iproute2 bind-mount path, concisely summarizing the introduction of support for an alternate namespace directory without extraneous detail.
Linked Issues Check ✅ Passed The implementation adds a fallbackMountPath constant for “/var/run/netns” and updates NewNamed, DeleteNamed, and GetFromName via helper functions to resolve both the primary and fallback paths, fully addressing the coding requirements of issue #106.
Out of Scope Changes Check ✅ Passed All modifications are confined to netns_linux.go’s path-resolution logic and related function updates, with no unrelated files or functionality altered outside the scope of providing a fallback for the netns bind-mount path.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 596a397 and 0e0a224.

📒 Files selected for processing (1)
  • netns_linux.go (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
netns_linux.go (1)
netns_others.go (2)
  • New (22-24)
  • GetFromPath (38-40)
🔇 Additional comments (6)
netns_linux.go (6)

24-30: LGTM!

The constants are well-documented with clear comments explaining the backward compatibility rationale and referencing issue #106. The naming convention distinguishes the primary path from the fallback path appropriately.


32-43: LGTM!

The fallback logic is sound:

  • Prioritizes bindMountPath for backward compatibility
  • Falls back to fallbackMountPath (iproute2 convention)
  • Defaults to bindMountPath if neither exists (maintains existing behavior for directory creation)

The error handling (treating all os.Stat errors as "does not exist") is acceptable here since the actual operations (mkdir, mount, etc.) will fail with clearer errors if there are permission or I/O issues.


45-60: LGTM!

The path resolution logic correctly checks both locations for existing named namespaces before falling back to directory-based resolution. This ensures operations work regardless of where the namespace was created, addressing the container environment compatibility concern from the PR objectives.


86-91: LGTM!

The changes correctly use getNetnsPath() to determine the appropriate directory for new namespace creation while maintaining backward compatibility. The resolved directory is consistently used for both MkdirAll and path construction.

Also applies to: 98-98


119-119: LGTM!

Using getNetnsPathForName() ensures DeleteNamed can locate and remove namespaces regardless of which path convention they were created under.


147-147: LGTM!

Using getNetnsPathForName() enables GetFromName to open named namespaces from either path, directly addressing the compatibility requirement for Kubernetes/container environments that mount only one of the two paths.


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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"/var/run/netns" vs "/run/netns"

2 participants