Add leader election #113
Add leader election #113
Conversation
Pull Request Test Coverage Report for Build 1052431734
💛 - Coveralls |
pkg/storage/kubernetes.go
Outdated
| /* | ||
| */ |
pkg/storage/kubernetes.go
Outdated
| LeaseDuration: 1500 * time.Millisecond, | ||
| RenewDeadline: 1000 * time.Millisecond, | ||
| RetryPeriod: 500 * time.Millisecond, |
There was a problem hiding this comment.
Can we make it configurable from CNI json config? It might be key for performance tuning.
There was a problem hiding this comment.
Added changes for configurable parameters for lease/renew/retry durations
|
|
||
| var rl = &resourcelock.LeaseLock{ | ||
| LeaseMeta: metav1.ObjectMeta{ | ||
| Name: "whereabouts", |
There was a problem hiding this comment.
you could potentially use the name of the pool here (perhaps with whereabouts as a prefix), so that allocations in other pools are not also blocked
There was a problem hiding this comment.
I suppose we should use leaselock to lock whole whereabouts, not per ip pool basis because Kubernetes/etcd does not support fine grain lock (because etcd is not DB, just KVS).
| switch ipamConf.Datastore { | ||
| case types.DatastoreETCD: | ||
| ipam, err = NewETCDIPAM(ipamConf) | ||
| case types.DatastoreKubernetes: | ||
| ipam, err = NewKubernetesIPAM(containerID, ipamConf) | ||
| } |
There was a problem hiding this comment.
It seems strange to me to find this switch in the etcd implementation. Shouldn't the correct config be passed as a parameter to the implementations ?
dougbtv
force-pushed
the
add-leader-election
branch
2 times, most recently
from
15e7cb4
to
b3ef26c
Compare
|
New commit addresses configurability of the duration parameters, adds defaults based on original code |
pkg/storage/kubernetes.go
Outdated
| func IPManagementKubernetesUpdate(mode int, ipam *KubernetesIPAM, ipamConf whereaboutstypes.IPAMConfig, containerID string, podRef string) (net.IPNet, error) { | ||
| logging.Debugf("IPManagement -- mode: %v / containerID: %v / podRef: %v", mode, containerID, podRef) | ||
|
|
||
| now := time.Now() |
There was a problem hiding this comment.
Please remove this because it is only for debug purpose.
pkg/storage/kubernetes.go
Outdated
| //XXX | ||
| logging.Debugf("Took: %vms\n", time.Since(now).Milliseconds()) |
There was a problem hiding this comment.
Please remove this because it is only for debug purpose.
There was a problem hiding this comment.
Right! And this line causes the openshift img build failure, since it uses golang-1.12 - time.Milliseconds is available on golang >= 1.13 as per https://golang.org/pkg/time/#Duration.Milliseconds
…e lease durations
dougbtv
force-pushed
the
add-leader-election
branch
from
b3ef26c
to
5b52948
Compare
Uses
k8s.io/client-go/tools/leaderelectionto add leader election to choose a leader during CRD read/write in order to properly handle concurrency before ippools are calculated and modified.