libnetwork: add rootlessnetns package #1761
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This link was added in kernel 3.17 so it should be safe to use now as we do not support running on such old kernels anyway. This makes the code a bit simpler and safes two syscalls. Signed-off-by: Paul Holzinger <[email protected]>
We never use the origNS other than closing it again so we can just remove this code as it does nothing useful. Signed-off-by: Paul Holzinger <[email protected]>
Logging the error and returning it makes no sense, instead add the context to the error before returning it. Signed-off-by: Paul Holzinger <[email protected]>
Add new function to create a netns bind mounted to a specific path. This is useful for the new rootless netns logic were I intend to mount the netns at a different directory to avoid naming conflicts. Signed-off-by: Paul Holzinger <[email protected]>
Add a new rootlessnetns package based on the rootless netns code from podman. It however makes some significant changes: - First it uses a directory in the runroot and not tmpdir. - The netns mount is stored in the directoy and not the global netns runtime dir to prevent name collisions. The old code used the sha256 to do that. - The teardown and setup logic has been made more robust and now used a reference counter to keep track on when to cleanup. The podman cleanup logic was racy and tied to running podman containers. Given the plan to allow buildah to use this as well we need this. - There is no lock for this code, the goal is to have this called through the network interface which is already locked so there is no need for another lock here. Future work: - add pasta support - add port forwarding logic here Signed-off-by: Paul Holzinger <[email protected]>
Integrate the new rootlessnetns package into netavark. This means when this is vendored into podman we need to drop the rootless netns code there. Signed-off-by: Paul Holzinger <[email protected]>
With the new NewNSAtPath function it is possible to create netns files outside the normal netns directory. We need to make sure they can get unmounted as well. We can check the the path is not under /proc. Signed-off-by: Paul Holzinger <[email protected]>
Call directly into the rootlessnetns code. Signed-off-by: Paul Holzinger <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 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 |
In podman we have code to move a process into a new systemd cgroup. This code lived in the podman utils package. Because the new rootlessnetns must call into that move this code to c/common. Instead of dumping this again into a "util" package create a systemd package which should have a better name. Also move the cgroup code directly into pkg/cgroup. I am sure we can do some cleanup there in a followup to prevent duplication. Signed-off-by: Paul Holzinger <[email protected]>
The old rootlessnetns logic overwrote PATH for the current process to make sure /usr/sbin (where iptables is normally installed) is in $PATH. Now instead of adding it for the current process we can just always set it for the cni/iptables exec only. Signed-off-by: Paul Holzinger <[email protected]>
Make sure we correctly cleanup the netns if there was an error and the netns was just created. Also make sure the parent dir for the netns is always created because a previous cleanup() may have it deleted. Signed-off-by: Paul Holzinger <[email protected]>
Just pass down the full containers.conf as this is needed by rootlessnetns code, also remove the now duplicated fields and read the options directly from the config struct. Signed-off-by: Paul Holzinger <[email protected]>
The cni code is used by freebsd so this package must build for it as well. Given the logic is linux specific and not called by freebsd just return an error. Signed-off-by: Paul Holzinger <[email protected]>
Just add stubs so podman can still compile. Signed-off-by: Paul Holzinger <[email protected]>
|
Podman PR for testing: containers/podman#20772 |
|
This is ready for review, podman PR passes tests: containers/podman#20772 |
|
thanks for your work @Luap99 LGTM |
|
/lgtm |
Luap99
added a commit
to Luap99/libpod
that referenced
this pull request
Dec 7, 2023
Use the new rootlessnetns logic from c/common, drop the podman code here and make use of the new much simpler API. ref: containers/common#1761 [NO NEW TESTS NEEDED] Signed-off-by: Paul Holzinger <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new rootlessnetns package based on the rootless netns code from
podman. It however makes some significant changes:
runtime dir to prevent name collisions. The old code used the sha256
to do that.
reference counter to keep track on when to cleanup. The podman
cleanup logic was racy and tied to running podman containers. Given
the plan to allow buildah to use this as well we need this.
through the network interface which is already locked so there is no
need for another lock here.
Future work:
see commits for more details