Try to defer filtering out lxdbr0 et al so we can choose to use them …

…when we want to.

network/containerizer/bridgepolicy.go

@@ -5,6 +5,8 @@ package containerizer
 
 import (
 	"fmt"
+	"sort"
+	"strings"
 
 	"github.com/juju/errors"
 	"github.com/juju/loggo"
@@ -27,6 +29,9 @@ type BridgePolicy struct {
 	// one of the devices being bridged is a BondDevice. This exists because of
 	// https://bugs.launchpad.net/juju/+bug/1657579
 	NetBondReconfigureDelay int
+	// UseLocalBridges decides if we should use local-only bridges ("lxdbr0", "virbr0"),
+	// to handle unnamed space requests.
+	UseLocalBridges bool
 }
 
 // Machine describes either a host machine, or a container machine. Either way
@@ -120,7 +125,7 @@ func (p *BridgePolicy) findSpacesAndDevicesForContainer(m Machine, containerMach
 
 func possibleBridgeTarget(dev *state.LinkLayerDevice) (bool, error) {
 	// LoopbackDevices can never be bridged
-	if dev.Type() == state.LoopbackDevice {
+	if dev.Type() == state.LoopbackDevice || dev.Type() == state.BridgeDevice {
 		return false, nil
 	}
 	// Devices that have no parent entry are direct host devices that can be
@@ -149,6 +154,38 @@ func possibleBridgeTarget(dev *state.LinkLayerDevice) (bool, error) {
 	return false, nil
 }
 
+func formatDeviceMap(spacesToDevices map[string][]*state.LinkLayerDevice) string {
+	spaceNames := make([]string, len(spacesToDevices))
+	i := 0
+	for spaceName := range spacesToDevices {
+		spaceNames[i] = spaceName
+		i++
+	}
+	sort.Strings(spaceNames)
+	out := []string{}
+	for _, name := range spaceNames {
+		start := fmt.Sprintf("%q:[", name)
+		devices := spacesToDevices[name]
+		deviceNames := make([]string, len(devices))
+		for i, dev := range devices {
+			deviceNames[i] = dev.Name()
+		}
+		deviceNames = network.NaturallySortDeviceNames(deviceNames...)
+		quotedNames := make([]string, len(deviceNames))
+		for i, name := range deviceNames {
+			quotedNames[i] = fmt.Sprintf("%q", name)
+		}
+		out = append(out, start + strings.Join(quotedNames, ",") + "]")
+	}
+	return "map{" + strings.Join(out, ", ") + "}"
+}
+
+var skippedDeviceNames = set.NewStrings(
+		network.DefaultLXCBridge,
+		network.DefaultLXDBridge,
+		network.DefaultKVMBridge,
+	)
+
 // FindMissingBridgesForContainer looks at the spaces that the container
 // wants to be in, and sees if there are any host devices that should be
 // bridged.
@@ -161,11 +198,15 @@ func (b *BridgePolicy) FindMissingBridgesForContainer(m Machine, containerMachin
 		return nil, 0, errors.Trace(err)
 	}
 	logger.Debugf("FindMissingBridgesForContainer(%q) spaces %s devices %v",
-		containerMachine.Id(), network.QuoteSpaceSet(containerSpaces), devicesPerSpace)
+		containerMachine.Id(), network.QuoteSpaceSet(containerSpaces),
+		formatDeviceMap(devicesPerSpace))
 	spacesFound := set.NewStrings()
 	for spaceName, devices := range devicesPerSpace {
 		for _, device := range devices {
 			if device.Type() == state.BridgeDevice {
+				if skippedDeviceNames.Contains(device.Name()) {
+					continue
+				}
 				spacesFound.Add(spaceName)
 			}
 		}
@@ -258,7 +299,7 @@ func (p *BridgePolicy) PopulateContainerLinkLayerDevices(m Machine, containerMac
 		return errors.Trace(err)
 	}
 	logger.Debugf("for container %q, found host devices spaces: %s",
-		containerMachine.Id(), devicesPerSpace)
+		containerMachine.Id(), formatDeviceMap(devicesPerSpace))
 
 	spacesFound := set.NewStrings()
 	devicesByName := make(map[string]*state.LinkLayerDevice)
@@ -267,7 +308,7 @@ func (p *BridgePolicy) PopulateContainerLinkLayerDevices(m Machine, containerMac
 	for spaceName, hostDevices := range devicesPerSpace {
 		for _, hostDevice := range hostDevices {
 			deviceType, name := hostDevice.Type(), hostDevice.Name()
-			if deviceType == state.BridgeDevice {
+			if deviceType == state.BridgeDevice && !skippedDeviceNames.Contains(name) {
 				devicesByName[name] = hostDevice
 				bridgeDeviceNames = append(bridgeDeviceNames, name)
 				spacesFound.Add(spaceName)
@@ -284,8 +325,8 @@ func (p *BridgePolicy) PopulateContainerLinkLayerDevices(m Machine, containerMac
 	}
 
 	sortedBridgeDeviceNames := network.NaturallySortDeviceNames(bridgeDeviceNames...)
-	logger.Debugf("for container %q using host machine %q bridge devices: %v",
-		containerMachine.Id(), m.Id(), sortedBridgeDeviceNames)
+	logger.Debugf("for container %q using host machine %q bridge devices: %s",
+		containerMachine.Id(), m.Id(), network.QuoteSpaces(sortedBridgeDeviceNames))
 	containerDevicesArgs := make([]state.LinkLayerDeviceArgs, len(bridgeDeviceNames))
 
 	for i, hostBridgeName := range sortedBridgeDeviceNames {

network/network.go

@@ -55,6 +55,9 @@ const AnySubnet Id = ""
 // UnknownId can be used whenever an Id is needed but not known.
 const UnknownId = ""
 
+// DefaultLXCBridge is the bridge that gets used for LXC containers
+const DefaultLXCBridge = "lxcbr0"
+
 // DefaultLXDBridge is the bridge that gets used for LXD containers
 const DefaultLXDBridge = "lxdbr0"
 

state/linklayerdevices_test.go

@@ -980,10 +980,7 @@ func (s *linkLayerDevicesStateSuite) TestLinkLayerDevicesForSpacesWithUnknown(c
 	c.Check(devices[0].Type(), gc.Equals, state.BridgeDevice)
 }
 
-func (s *linkLayerDevicesStateSuite) TestLinkLayerDevicesForSpacesUnknownIgnoresLoopAndKnownBridges(c *gc.C) {
-	// When we ask for unknown devices, we still skip lxdbr0 and virbr0,
-	// because we don't want to put containers there as they won't be
-	// routable.
+func (s *linkLayerDevicesStateSuite) TestLinkLayerDevicesForSpacesUnknownIgnoresLoopAndIncludesKnownBridges(c *gc.C) {
 	// TODO(jam): 2016-12-28 arguably we should also be aware of Docker
 	// devices, possibly the better plan is to look at whether there are
 	// routes from the given bridge out into the rest of the world.
@@ -1001,7 +998,7 @@ func (s *linkLayerDevicesStateSuite) TestLinkLayerDevicesForSpacesUnknownIgnores
 	for i, dev := range devices {
 		names[i] = dev.Name()
 	}
-	c.Check(names, gc.DeepEquals, []string{"br-ens4", "ens3"})
+	c.Check(names, gc.DeepEquals, []string{"br-ens4", "ens3", "lxcbr0", "lxdbr0", "virbr0"})
 }
 
 func (s *linkLayerDevicesStateSuite) TestSetLinkLayerDevicesWithLightStateChurn(c *gc.C) {

state/machine_linklayerdevices.go

@@ -14,7 +14,6 @@ import (
 	"gopkg.in/mgo.v2/bson"
 	"gopkg.in/mgo.v2/txn"
 
-	"github.com/juju/juju/container"
 	"github.com/juju/juju/network"
 )
 
@@ -916,17 +915,15 @@ func deviceMapToSortedList(deviceMap map[string]*LinkLayerDevice) []*LinkLayerDe
 // on this machine that are in that space. (We currently only return devices
 // that have an address, which are thus targets for being bridged, or are
 // themselves a bridge that can be used.)
+// Note that devices like 'lxdbr0' that are bridges that might might not be
+// externally accessible may be returned if "" is listed as one of the desired
+// spaces.
 func (m *Machine) LinkLayerDevicesForSpaces(spaces []string) (map[string][]*LinkLayerDevice, error) {
 	addresses, err := m.AllAddresses()
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 	requestedSpaces := set.NewStrings(spaces...)
-	devicesToSkip := set.NewStrings(
-		container.DefaultLxcBridge,
-		container.DefaultLxdBridge,
-		container.DefaultKvmBridge,
-	)
 	spaceToDevices := make(map[string]map[string]*LinkLayerDevice, 0)
 	// TODO(jam): 2016-12-08 We look up each subnet one-by-one, and then look
 	// up each Link-Layer-Device one-by-one, it feels like we should
@@ -966,7 +963,7 @@ func (m *Machine) LinkLayerDevicesForSpaces(spaces []string) (map[string][]*Link
 		if err != nil {
 			return nil, errors.Trace(err)
 		}
-		if device.Type() == LoopbackDevice || devicesToSkip.Contains(device.Name()) {
+		if device.Type() == LoopbackDevice {
 			// We skip loopback devices here
 			continue
 		}
@@ -975,7 +972,6 @@ func (m *Machine) LinkLayerDevicesForSpaces(spaces []string) (map[string][]*Link
 			spaceInfo = make(map[string]*LinkLayerDevice)
 			spaceToDevices[spaceName] = spaceInfo
 		}
-		// TODO(jam): handle seeing a device with the same name twice?
 		spaceInfo[device.Name()] = device
 	}
 	result := make(map[string][]*LinkLayerDevice, len(spaceToDevices))
@@ -1050,7 +1046,7 @@ func (m *Machine) SetDevicesAddressesIdempotently(devicesAddresses []LinkLayerDe
 
 func DefineEthernetDeviceOnBridge(name string, hostBridge *LinkLayerDevice) (LinkLayerDeviceArgs, error) {
 	if hostBridge.Type() != BridgeDevice {
-		return LinkLayerDeviceArgs{}, errors.Errorf("can only create an Ethernet device on a bridge not %q", hostBridge.Type())
+		return LinkLayerDeviceArgs{}, errors.Errorf("hostBridge must be a Bridge Device not %q", hostBridge.Type())
 	}
 	return LinkLayerDeviceArgs{
 		Name:        name,