-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
docker/podman: avoid creating overlapping networks with other tools (KVM,...) #10439
docker/podman: avoid creating overlapping networks with other tools (KVM,...) #10439
Conversation
Hi @prezha. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Can one of the admins verify this patch? |
/ok-to-test |
kvm2 Driver |
kvm2 Driver Times for Minikube (PR 10439): 71.0s 67.3s 69.3s Averages Time Per Log
docker Driver Times for Minikube (PR 10439): 25.8s 26.1s 27.3s Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 10439): 67.9s 68.7s 67.5s Averages Time Per Log
docker Driver Times for Minikube (PR 10439): 31.9s 31.8s 26.6s Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 10439): 70.3s 70.2s 69.5s Averages Time Per Log
docker Driver Times for Minikube (PR 10439): 26.1s 25.9s 26.0s Averages Time Per Log
|
} | ||
|
||
// FreeSubnet will try to find free private network beginning with startSubnet, incrementing it in steps up to number of tries. | ||
func FreeSubnet(startSubnet string, step, tries int) (*Parameters, error) { |
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.
can u verify that this logic still works for Docker networks ?
manually create a docker network that is assigned to a dummy container
also we need to verify this does NOT add any slow down in minikube container create
to verify we can use Slow Jam https://github.com/google/slowjam
minikube supports setting the Stack Track that u can use with slowjam to generate the report
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.
@medyagh i've verified that docker still works with conflicting networks (i've put some examples in the pr's description, but i've also added now another set according to your proposal - with dummy docker networks)
slow jam is a great tool, thank you for pointing out!
happy to report that this pr improves on speed 25% on 10 conflicting network attempts (added in the pr's description at the end) - this is expected as we are now calling oci.tryCreateDockerNetwork() for each address candidate and detect that the network is taken by failing there, we now first check if subnet is free and only then call oci.tryCreateDockerNetwork() just once to create docker network
d3d5d9d
to
7727b85
Compare
kvm2 Driver Times for Minikube (PR 10439): 70.1s 66.5s 70.1s Averages Time Per Log
docker Driver Times for Minikube (PR 10439): 28.0s 26.1s 27.6s Averages Time Per Log
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, prezha 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 |
fixes #10005
make sure that the candidate for new container network does not overlap with any existing local networks
notes:
--delete
flag) is resolved in pr improve kvm network delete/cleanup #10479GetNetworkFrom()
(in pkg/network/network.go) for getting any interface'smtu
(i know there were some issues with it)networkTmpl
opens possibility to have different private network for each cluster (needs more work to have that if we want to); ref: KVM: starting network minikube-net: Network is already in use by interface #9049 (comment)example
before
set docker net to kvm's default (static) one to simulate conflict:
after
simulate conflict:
same goes for the other side:
additional examples (as per @medyagh's suggestion)
current master
slowjam
[oci.CreateNetwork] duration: 1 secpull request
slowjam
[oci.CreateNetwork] duration: 0.751 sec