-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Statically link CNI plugins #3859
Statically link CNI plugins #3859
Conversation
Signed-off-by: Davanum Srinivas <[email protected]>
/assign @aojea @BenTheElder |
/lgtm |
It seems we already discussed this in the original PR #3214 (comment) , can't remember why we did one and not the other since there should not be any requirement on the plugins to use cgo |
IIRC we were matching the upstream builds at the time? Not using CGO makes DNS weird, for things "on the host" we generally build with cgo, unless they're not using the network. I think none of the CNI plugins should need to use DNS though? |
So, to be clear though: https://kind.sigs.k8s.io/docs/design/node-image/
statically linking these seems ~ok given upstream is and we're not aware of a specific need to dynamically link these binaries (also we should consider deleting the ones we're not using, but lets do that separately) ... but I generally expect CNI installs to ship their own binaries and not depend on ours, the same as installing on metal etc. Commented in k8snetworkplumbingwg/multus-cni#1213 (comment) That shouldn't block matching the upstream binaries though, we don't have a strong reason to deviate in that regard. |
NOTE: We're also not currently releasing at the moment due to being part way through sorting out containerd 2.0 kubernetes/kubernetes#129818 / containerd/containerd#11323 / #3795 / containerd/containerd#11344 / #3853 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, dims 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 |
Picking a release of cni plugins and inspecting the binaries shows that they are statically linked. So we should do the same.
Following up to a multus-cni testing issue: