Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
2.1.1 fix network choosing 1665648 #7054
Conversation
perrito666
and others
added some commits
Feb 23, 2017
| @@ -163,10 +165,17 @@ var configureLXDBridge = func() error { | ||
| return errors.Trace(err) | ||
| } | ||
| + // If LXD itself supports managing networks (added in LXD 2.3) we can allow | ||
| + // it to do all of the network configuration. | ||
| if shared.StringInSlice("network", status.APIExtensions) { | ||
| return lxdclient.CreateDefaultBridgeInDefaultProfile(client) |
anastasiamac
Mar 1, 2017
Member
What versions of LXD do we support? should we check for lxd version < 2.3 here?
| @@ -304,47 +349,39 @@ func ensureDependencies(series string) error { | ||
| // | ||
| // TODO(frobware): this only caters for IPv4 setups. | ||
| func findNextAvailableIPv4Subnet() (string, error) { | ||
| - _, ip10network, err := net.ParseCIDR("10.0.0.0/16") |
| + if !collides { | ||
| + for _, kNet := range knownCIDRs { | ||
| + if kNet.Contains(ip) || ip10network.Contains(kNet.IP) { | ||
| + collides = true |
anastasiamac
Mar 1, 2017
Member
I could not find any units tests for collision. Wouldn't it be nice to have some?
jameinel
Mar 1, 2017
Owner
https://github.com/juju/juju/pull/7054/files#diff-04db2fba070a0bd3ebfc387dfc486967R315 is the test I updated to test that we handle colliding values.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Generating tarball failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit 10a1076
into
juju:2.1
Mar 1, 2017
jameinel
deleted the
jameinel:2.1.1-fix_network_choosing-1665648
branch
Apr 22, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
jameinel commentedMar 1, 2017
This is a couple of small tweaks from Horacio's Issue #7030
Description of change
One issue that we are addressing is that we were just using 10.0.X.0/24 and starting at 0 and just walking up until we found a value. Now we are walking randomly so 2 machines are less likely to collide.
The other factor is that the old algorithm assumed all networks were /24, so if it encountered a 10.0.3.0/23 then it had a chance to use 10.0.4.0/24 which is actually still in use.
QA steps
You can bootstrap to somewhere like AWS, and then ssh into that machine, create bridges/assign IP addresses for a bunch of 10.0.* address ranges. Then do
juju add-machine lxd:0. We should avoid all of the ranges that you've already specified.Documentation changes
I don't believe we documented that lxdbr0 address ranges will always be the next available 10.0.*.0/24 range, which gives us freedom to change the algorithm without changing what we were offering.
Bug reference
https://bugs.launchpad.net/juju/+bug/1665648
This also touches on:
https://bugs.launchpad.net/juju/+bug/1657850
It doesn't do everything asked there, but it does improve the situation because every machine won't try to choose exactly the same prefix.