- 
                Notifications
    You must be signed in to change notification settings 
- Fork 134
fallback to iproute bind mount path #107
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.
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
base: master
Are you sure you want to change the base?
Conversation
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]>
| WalkthroughCentralized 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
 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
 Pre-merge checks and finishing touches✅ Passed checks (5 passed)
 ✨ Finishing touches
 🧪 Generate unit tests (beta)
 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
 🧰 Additional context used🧬 Code graph analysis (1)netns_linux.go (1)
 🔇 Additional comments (6)
 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. Comment  | 
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