Move chroot from multus main process to its child processes #1161
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
|
Thank you so much for great PR, @pliurh ! Overall desgin makes sense to me. One minor comments but this PR implicitly requires to manifest to mount CNI bin directory to same path in container host. (e.g. if /opt/cni/bin is the CNI bin directory, multus-daemon pod has to mount it with same path /opt/cni/bin, not /opt/bin. |
pliurh
force-pushed
the
chroot
branch
2 times, most recently
from
aa19590
to
e0f3681
Compare
pliurh
changed the title
s1061123
requested changes
Sep 22, 2023
We used to run chroot in multus main process when calling other CNI plugin binary. We also use a mutex to lock the access to pod files. But this causes performance issues when facing heavy CNI_ADD/CNI_DEL requests. With this patch, we do chroot in the child processes instead. So file operations in the main process will not be affected by chroot. This change requires the multus thick plugin pod to mount CNI bin directory to the same path in the container host. Signed-off-by: Peng Liu <[email protected]>
pliurh
added a commit
to pliurh/multus-cni
that referenced
this pull request
Sep 22, 2023
…rkplumbingwg#1161) We used to run chroot in multus main process when calling other CNI plugin binary. We also use a mutex to lock the access to pod files. But this causes performance issues when facing heavy CNI_ADD/CNI_DEL requests. With this patch, we do chroot in the child processes instead. So file operations in the main process will not be affected by chroot. This change requires the multus thick plugin pod to mount CNI bin directory to the same path in the container host. Signed-off-by: Peng Liu <[email protected]>
|
line 173 of deployments/multus-daemonset-thick.yml should be:
|
kichankwon
pushed a commit
to kichankwon/multus-cni
that referenced
this pull request
Nov 26, 2023
…rkplumbingwg#1161) We used to run chroot in multus main process when calling other CNI plugin binary. We also use a mutex to lock the access to pod files. But this causes performance issues when facing heavy CNI_ADD/CNI_DEL requests. With this patch, we do chroot in the child processes instead. So file operations in the main process will not be affected by chroot. This change requires the multus thick plugin pod to mount CNI bin directory to the same path in the container host. Signed-off-by: Peng Liu <[email protected]>
s1061123
pushed a commit
to s1061123/multus-cni
that referenced
this pull request
Feb 21, 2024
…rkplumbingwg#1161) We used to run chroot in multus main process when calling other CNI plugin binary. We also use a mutex to lock the access to pod files. But this causes performance issues when facing heavy CNI_ADD/CNI_DEL requests. With this patch, we do chroot in the child processes instead. So file operations in the main process will not be affected by chroot. This change requires the multus thick plugin pod to mount CNI bin directory to the same path in the container host. Signed-off-by: Peng Liu <[email protected]>
pliurh
pushed a commit
to pliurh/multus-cni
that referenced
this pull request
May 21, 2024
OCPBUGS-18995: Move chroot from multus main process to its child processes (k8snetworkplumbingwg#1161)
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.
We used to run chroot in multus main process when calling other CNI plugin binary. We also use a mutex to lock the access to pod files. But this causes performance issues when facing heavy CNI_ADD/CNI_DEL requests.
With this patch, we do chroot in the child processes instead. So file operations in the main process will not be affected by chroot.
This change requires the multus thick plugin pod to mount CNI bin directory to the same path in the container host.
fixes https://issues.redhat.com/browse/OCPBUGS-18995