create network: use locks and reservations to solve race condition

[prezha]

fixes #10833

as described in the example given in the issue's description, and as @medyagh suggested, we can use locks to resolve race condition while competing over a free network segment

this pr implements free network subnet reservations that would expire after the reservation period, which is set to one minute by default, and also have a retry mechanism if create network fails, utilising sync.Map that is safe for concurrent use by multiple goroutines without additional locking or coordination

also, to further avoid overlaps between free network scans (ie, kvm starts from 192.168.39.0 and container starts from 192.168.49.0, both with increment steps of 10), increment steps for the kvm is 11 and for the containers is 9

[prezha]

thought: shouldn't we want to fail-fast[er] instead of trying to use a network that we weren't able to create?

i haven't changed that in this pr, but i think we should (ie, not try using nonexisting or a network that was already taken for something else) - existing code:

minikube/pkg/drivers/kic/oci/network_create.go

Lines 82 to 84 in 63a8f28

	if err != nil {
	return info.gateway, fmt.Errorf("failed to create network after 20 attempts")
	}

minikube/pkg/drivers/kic/kic.go

Lines 95 to 96 in 63a8f28

	if gateway, err := oci.CreateNetwork(d.OCIBinary, networkName); err != nil {
	out.WarningT("Unable to create dedicated network, this might result in cluster IP change after restart: {{.error}}", out.V{"error": err})

[medyagh]

/ok-to-test

[minikube-pr-bot]

kvm2 Driver
error collecting results for kvm2 driver: timing run 0 with Minikube (PR 10858): timing cmd: [/home/performance-monitor/.minikube/minikube-binaries/10858/minikube start --driver=kvm2]: starting cmd: fork/exec /home/performance-monitor/.minikube/minikube-binaries/10858/minikube: exec format error
docker Driver
error collecting results for docker driver: timing run 0 with Minikube (PR 10858): timing cmd: [/home/performance-monitor/.minikube/minikube-binaries/10858/minikube start --driver=docker]: starting cmd: fork/exec /home/performance-monitor/.minikube/minikube-binaries/10858/minikube: exec format error

[medyagh]

how about using our Existing lock package ?

func PathMutexSpec(path string) mutex.Spec {

https://github.com/medyagh/minikube/blob/f95d43a2c78070ab10a4e7d134ff2d0952a5ca86/pkg/util/lock/lock.g o#L48

for better maintainability of the lock code, what is the Special about Locking a network Name that is different than other Locks in minikube?

[prezha]

here we need to lock a specific currently free network segment (not a network name) for a caller for a short period of time:

when a call to network.FreeSubnet() is made, it checks if a subnet is taken at the system level and skip those;
furthermore, in case of concurrent calls to network.FreeSubnet(), it also needs to skip all those network segments that were previously returned as 'free' (to other callers), but those other callers, within a configurable grace period, have not yet allocated the subnet at the system level

the existing locking mechanism with lock.PathMutexSpec() takes care of concurrent requests that would want to write to the same file by making them wait for their turn

on the other hand, with the free subnet allocation, we need a mechanism that would instantly know & skip if a subnet is either already taken or 'reserved' but not yet expired, and so to move on to the next free subnet

therefore i used sync.Map that has a built-in mechanism for safe concurrent access for 'reservations' with key:subnet and value:createdAt to be able to check if the subnet reservation expired (so free to reuse)

does that make sense?

[medyagh]

is there a reason to implement a new lock logic just for network ? how about using our existing Lock Package what functionatliy is missing ? could we instead improve the lock package and have all the lock related code there?

[medyagh]

/retest-this-please

[k8s-ci-robot]

[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:

• OWNERS [medyagh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment