On leader switchover preserve the vxlan id for existing networks #1773
Conversation
Current coverage is 55.18% (diff: 66.66%)@@ master #1773 diff @@
==========================================
Files 102 102
Lines 16871 16875 +4
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 9294 9313 +19
+ Misses 6419 6404 -15
Partials 1158 1158
|
|
LGTM |
|
moby/moby#28493 reports it on mixed Swarm versions. If this is the root cause, the problem should also occur with same version of docker Swarm. Can we reproduce it? |
|
This code looks like it could modify |
|
@dongluochen In 1.13 we changed the default range of vxlan ids to start from 4096 (it was 256 in 1.12) This exposes the problem right away. This problem exists in 1.12.X though. Just that its hard to hit because it depends on the order in which networks are read from the store. |
|
@aaronlehmann Yes, with the current change options in |
Signed-off-by: Santhosh Manohar <[email protected]>
|
@aaronlehmann Updated the change to use a local copy of the options to avoid modifying |
| } | ||
| if n.DriverState != nil { | ||
| for k, v := range n.DriverState.Options { | ||
| options[k] = v |
There was a problem hiding this comment.
Is there any possibility of overlapping keys in n.Spec.DriverConfig.Options and n.DriverState.Options? If yes what does that mean?
There was a problem hiding this comment.
Yes, its possible. If user explicitly specifies a driver option during network create it will be in n.Spec.DriverConfig.Options. After the resource allocation by the driver all the options including the one from the spec will be available in n.DriverState.Options which is the operational state.
There was a problem hiding this comment.
After the resource allocation by the driver all the options including the one from the spec will be available in n.DriverState.Options which is the operational state.
In that case, wouldn't n.DriverState.Options be enough? It should be a superset of n.Spec.DriverConfig.Options.
There was a problem hiding this comment.
In that case, wouldn't n.DriverState.Options be enough? It should be a superset of n.Spec.DriverConfig.Options.
Its not enough. A leader manager could lose the leader status right after a network create before the allocation and the subsequent store update.
|
LGTM |
|
@sanimej @dongluochen @aaronlehmann i could reproduce this issue in 1.12.3 as well. it seems fairly basic issue and we need to cherry-pick this in the 1.12.x branch as well. |
|
@mavenugo: Makes sense. When it gets merged, I'll make sure it gets cherry-picked to both branches. |
|
LGTM |
|
Cherry-picked to |
|
Thanks @aaronlehmann |
On a leader change, the new leader restores the state for the existing networks from the store. But the overlay network's vxlan id was getting allocated afresh instead of restoring the current vxlan id allocated for the network.
related to docker #28493
Signed-off-by: Santhosh Manohar [email protected]