Skip removing vnet namespace in Linux CNI Multitenancy #1841
Conversation
…working into tamanoha/deletens_fix
|
|
||
| if routesLeft <= numDefaultRoutes { | ||
| // Deletes default arp, default routes, vlan veth; there are two default routes | ||
| // so when we have <= numDefaultRoutes routes left, no containers use this namespace |
There was a problem hiding this comment.
We are skipping deletion of vnet namespace if there are no containers but when there are new containers for that namespace, we won't need to create that namespace again ? Is that correct ?
Wanted to know if there is a scenario in which the namespaces are getting accumulated since they were not deleted.
There was a problem hiding this comment.
good question. cni creates netns per vlanid and at max vlanid can range from 1-4096 and also we dont support more than 150 containers in multitenancy. so not deleting netns should not cause any issue
vmss000000:/# cat /proc/sys/user/max_net_namespaces
27653
|
|
||
| log.Printf("[transparent vlan] There are %d routes remaining after deletion", routesLeft) | ||
|
|
||
| if routesLeft <= numDefaultRoutes { |
There was a problem hiding this comment.
Just curious, will commenting this out won't affect the unit test cases we run for it ? https://github.com/Azure/azure-container-networking/blob/master/network/transparent_vlan_endpointclient_linux_test.go#L467
If so, do we need to update the unit test cases as well ?
There was a problem hiding this comment.
But this does limit the user's ability to go beyond 23k( approximately) even if the vlan is not at max i.e 4k
I understand that this is safe as we don't have the requirement or a user that has that many namespace, but why are we doing it? Isn't it is safer to delete and make sure it is created when needed, instead of letting it be there.
OR Does this help in reducing the latency since we don't need to create a namespace again ?
(Do correct me if I am understanding it wrong)
There was a problem hiding this comment.
so this will be a problem when we parallelize the execution of cni in future.. one doing add and other doing delete in same namespace based on number of default routes
merging it since this PR already passes e2e once
Reason for Change:
Its not required to tear down vnet namespace if there no endpoints on that vnet and then create back again later
Issue Fixed:
Requirements:
Notes: