Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Find lxdbr0 even when it doesn't have an address. #6905
Conversation
axw
approved these changes
Feb 2, 2017
LGTM.
Does this change mean that a device may start out being reported in the unknown space, and then move to a named space later when it gets an address? Is that OK?
| @@ -923,59 +923,72 @@ func (m *Machine) LinkLayerDevicesForSpaces(spaces []string) (map[string][]*Link | ||
| if err != nil { |
axw
Feb 2, 2017
Member
I think the method's doc comment is incorrect now, since devices without addresses are being returned.
|
Ultimately, we'd want to be looking more at L2 connectivity to determine whether a device is in a space or not. As for "moving", its certainly possible for us to bridge at the wrong time because we looked at the wrong time. However, the current reality is that you should really either always be in "undefined" space (for places where we don't have space support) or always only in defined space (for places where we do), so there shouldn't really be a possibility of moving between them. |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Does not match ['fixes-1660877'] |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Generating tarball failed |
|
$$merge$$ (go fmt + squash) |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ (test failed with a nil-pointer exception in a mongo.sync call) |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jameinel commentedFeb 2, 2017
When looking on the host machine for how we might want to
configure a container, we actually weren't finding 'lxdbr0'
because it didn't have an address. This changes the
machine.LinkLayerDevicesForSpaces so that it first builds
up devices based on their addresses, and then does a
pass to create the devices based on what they are
connected to.
This properly connects things together for places like
AWS that only have the local bridge.
QA steps
Without this patch, you'll see lines like:
Which indicates it was trying to use the host device (it fails so the later fallback to use lxdbr0 when the one we wanted fails makes it ultimately succeed by accident.)
With this patch, you'll see
Which means that we correctly identified that 'lxdbr0' should be used (there should be no call to the bridging script.)
Documentation changes
This is fixing containers under the covers, so no doc changes.
Bug reference
This is all part of doing https://bugs.launchpad.net/juju/+bug/1657449 correctly.
It also is a step along the path for https://bugs.launchpad.net/juju/+bug/1656326 because by directly requesting 'lxdbr0' we can remove the code that 'falls back' to lxdbr0.