Reuse minikube-net network if it already exists for kvm driver

[priyawadhwa]

We've been seeing a lot of errors on KVM (#8952) which are related to this error:

Failed to start host: creating host: create: Error creating machine: Error in driver during machine creation: ensuring active networks: starting network minikube-net: virError(Code=1, Domain=0, Message='internal error: Network is already in use by interface br-f912846614e5'

I was able to fix this manually on the machine by running these commands:

#2513 (comment)

It would be nice if, instead of having to manually run these commands, minikube could:

1. Check if the minikube-net network already exists
2. Use it if it already exists OR delete it and create a new one

[prezha]

/assign

[priyawadhwa]

Hey @prezha this comment from a duplicate issue may be helpful to you: #9049 (comment)

@medyagh @tstromberg The kvm driver does share the same network for all vms. To change it we need some kind of IPAM, because right now the subnet used on the network is fixed: https://github.com/kubernetes/minikube/blob/master/pkg/drivers/kvm/network.go#L41

We need a different subnet per VM.

[prezha]

hey @priyawadhwa , thank you for that, and on a first glance, i'm not sure if the fact that kvm driver shares the same network for all vms is an issue necessarily, but i'll have a look

i'm sharing here some observations and proposals i have so far:

...ensuring active networks: starting network minikube-net...

implies that the error is raised in createNetwork()

it could be due to a race condition, ie, trying to recreate network that is still in the process of creation - depicted in call diagram:

kvm.Create()
           |_kvm.createNetwork()
           |                   |_libvirt.Create()
           |
           |_kvm.Start()
                       |_kvm.createNetwork()[*]
                       |                   |_libvirt.Create()
                       |
                       |_kvm.ensureNetwork()
                                           |_kvm.setupNetwork()
                                                              |_libvirt.Create()

[*] i'd propose to eliminate this call
it also has a comment preceding it:

// if somebody/something deleted the network in the meantime, we might need to recreate it. It's (nearly) a noop if the network exists.

now, nearly could be the key here, as LookupNetworkByName() might not immediately return the network that is still in creating process at that moment - ref: virNetworkCreate (https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkCreate):

Create and start a defined network. If the call succeeds the network moves from the defined to the running networks pools.

also, right after that createNetwork() call we call ensureNetwork() that should remediate any issues caused by someone/something messing the network

next, i'd also propose to cleanup inactive networks in setupNetwork() before trying to create a new one, adding 1sec delay before and after it

refs - all funcs mentioned above are in the kvm package:

• Create() and Start() are in pkg/drivers/kvm/kvm.go
• createNetwork(), setupNetwork() and ensureNetwork() are in pkg/drivers/kvm/network.go

i'll create a pr with those proposals and welcome any comments

[priyawadhwa]

@prezha soudns good thanks!

I think cleaning up inactive networks is a good idea, as long as we make sure we are only cleaning up networks created by minikube that are no longer in use.

[prezha]

@priyawadhwa great!

yes, the idea would be to clean up only those nets that are no longer used

btw, we are using libvirt v3.4.0 (from june 2017) and the current one is v6.8.0 (from oct 2020), but is potentially a separate topic

[priyawadhwa]

Good catch, yah we could probably open a separate issue to upgrade libvirt

[prezha]

@priyawadhwa i've submitted pr (#9641) with proposed changes above - please have a look and feedback
please note: unit tests are failing due to another issue #9631 that should be fixed with pr #9632

also, i've created a separate issue for libvirt upgrade (#9642)