provider/lxd: fix off-by-1 LXD subnet calculation #5099

Merged
merged 1 commit into from Apr 12, 2016

Conversation

Projects
None yet
4 participants
Contributor

frobware commented Apr 12, 2016

The call to detectSubnet() was always adding 1 to the network address
that was discovered when parsing the output from `ip addr'. This has the
unfortunate effect of changing the subnet for the LXD bridge after it
has been recorded by Juju. This meant that further container <=>
apiserver communication would break as the original value was recorded
in the agent.conf file.

Fixes LP:#1569361

(Review request: http://reviews.vapour.ws/r/4538/)

provider/lxd: fix off-by-1 LXD subnet calculation
The call to detectSubnet() was always adding 1 to the network address
that was discovered when parsing the output from `ip addr'. This has the
unfortunate effect of changing the subnet for the LXD bridge after it
has been recorded by Juju. This meant that further container <=>
apiserver communication would break as the original value was recorded
in the agent.conf file.

Fixes [LP:#1569361](https://bugs.launchpad.net/juju-core/+bug/1569361)
Contributor

frobware commented Apr 12, 2016

$$merge$$

Contributor

jujubot commented Apr 12, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit e305232 into juju:next Apr 12, 2016

@frobware frobware deleted the frobware:next-lp1569361 branch Apr 12, 2016

Contributor

tych0 commented Apr 12, 2016

I'm confused by this PR. Do we not then pick an existing subnet to configure lxdbr0 with? Why doesn't it break initially now?

Owner

jameinel commented Apr 13, 2016

Right, the whole point of detectSubnet is to choose a subnet that isn't in use.
If we need to change it so that we detect the lxdbr0 and just return the one that already exists, that's fine, but the fix is to notice that we already have an address. detectSubnet should probably be renamed "findUnusedSubnet" or maybe split up?
Otherwise this is going to start creating conflicts when it finds an existing subnet.

frobware pushed a commit to frobware/juju that referenced this pull request Apr 13, 2016

Revert "Merge pull request #5099 from frobware/next-lp1569361"
This reverts commit e305232, reversing
changes made to 879933c.

The intent of the detectSubnet() was to look for an unsed subnet so this
change is not incorrect.

jujubot added a commit that referenced this pull request Apr 13, 2016

Merge pull request #5127 from frobware/next
Revert "Merge pull request #5099 from frobware/next-lp1569361"

This reverts commit e305232, reversing
changes made to 879933c.

The intent of the detectSubnet() was to look for an unsed subnet so this
change is not incorrect.

(Review request: http://reviews.vapour.ws/r/4566/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment