Mimic pinns namespace path layout #1067
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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:
Approvers can indicate their approval by writing |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1067 +/- ##
==========================================
+ Coverage 33.39% 33.83% +0.44%
==========================================
Files 13 13
Lines 1126 1126
Branches 389 387 -2
==========================================
+ Hits 376 381 +5
+ Misses 494 486 -8
- Partials 256 259 +3 |
saschagrunert
force-pushed
the
pause-paths
branch
6 times, most recently
from
a974feb
to
48a559e
Compare
CNI expects namespaces available under `/var/run/netns`, which means we now change the overall layout of the namespaces to match `pinns`. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
saschagrunert
force-pushed
the
pause-paths
branch
from
48a559e
to
82618ac
Compare
| /// Run pause, which bind mounts selected namespaces to the local file system. | ||
| /// | ||
| /// If a namespace is not selected by one of the flags, then it will fallback to the host | ||
| /// namespace and still create the bind mount to it. All namespaces are mounted to |
There was a problem hiding this comment.
I know this behavior was added in #1064 but something I've just thought of: we will needessly mount a namespace for private namespaces (container level), as we unconditionally do the host mounting, but then it wouldn't be used. I think that's okay and not that much overhead, but it would be more mounts than needed (for instance, kube can't even specify mount namespaces, so it's always private, but we always create a host level mount namespace)
There was a problem hiding this comment.
I think I'm gonna open an issue to track
|
LGTM, thanks! |
|
@rphillips @haircommander I'd say we merge this (already a large PR) and iterate on the open issues |
|
/lgtm |
What type of PR is this?
/kind feature
What this PR does / why we need it:
CNI expects namespaces available under
/var/run/netns, which means we now change the overall layout of the namespaces to matchpinns.Which issue(s) this PR fixes:
Fixes #1065
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?