Find lxdbr0 even when it doesn't have an address.

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.

apiserver/provisioner/provisioner.go

@@ -825,6 +825,7 @@ type hostChangesContext struct {
 func (ctx *hostChangesContext) ProcessOneContainer(env environs.Environ, idx int, host, container *state.Machine) error {
 	bridgePolicy := containerizer.BridgePolicy{
 		NetBondReconfigureDelay: env.Config().NetBondReconfigureDelay(),
+		UseLocalBridges:         !environs.SupportsContainerAddresses(env),
 	}
 	bridges, reconfigureDelay, err := bridgePolicy.FindMissingBridgesForContainer(host, container)
 	if err != nil {

network/containerizer/bridgepolicy_test.go

@@ -737,6 +737,32 @@ func (s *bridgePolicyStateSuite) TestFindMissingBridgesForContainerUseLocalBridg
 	c.Check(reconfigureDelay, gc.Equals, 0)
 }
 
+func (s *bridgePolicyStateSuite) TestFindMissingBridgesForContainerUseLocalBridgesNoAddress(c *gc.C) {
+	// We should only use 'lxdbr0' instead of bridging the host device.
+	s.setupTwoSpaces(c)
+	s.createNICWithIP(c, s.machine, "ens3", "172.12.0.10/24")
+	err := s.machine.SetLinkLayerDevices(
+		state.LinkLayerDeviceArgs{
+			Name:       "lxdbr0",
+			Type:       state.BridgeDevice,
+			ParentName: "",
+			IsUp:       true,
+		},
+	)
+	c.Assert(err, jc.ErrorIsNil)
+	s.addContainerMachine(c)
+	bridgePolicy := &containerizer.BridgePolicy{
+		NetBondReconfigureDelay: 13,
+		UseLocalBridges:         true,
+	}
+	// No defined spaces for the container, no *known* spaces for the host
+	// machine. Triggers the fallback code to have us bridge all devices.
+	missing, reconfigureDelay, err := bridgePolicy.FindMissingBridgesForContainer(s.machine, s.containerMachine)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Check(missing, jc.DeepEquals, []network.DeviceToBridge{})
+	c.Check(reconfigureDelay, gc.Equals, 0)
+}
+
 func (s *bridgePolicyStateSuite) TestFindMissingBridgesForContainerUnknownWithConstraint(c *gc.C) {
 	// If we have a host machine where we don't understand its spaces, but
 	// the container requests a specific space, we won't use the unknown

state/linklayerdevices_test.go

@@ -980,6 +980,33 @@ func (s *linkLayerDevicesStateSuite) TestLinkLayerDevicesForSpacesWithUnknown(c
 	c.Check(devices[0].Type(), gc.Equals, state.BridgeDevice)
 }
 
+func (s *linkLayerDevicesStateSuite) TestLinkLayerDevicesForSpacesWithNoAddress(c *gc.C) {
+	// We create a record for the 'lxdbr0' bridge, but it doesn't have an
+	// address yet (which is the case when we first show up on a machine.)
+	err := s.machine.SetLinkLayerDevices(
+		state.LinkLayerDeviceArgs{
+			Name:       "lxdbr0",
+			Type:       state.BridgeDevice,
+			ParentName: "",
+			IsUp:       true,
+		},
+	)
+	c.Assert(err, jc.ErrorIsNil)
+	// unknown space
+	s.createNICWithIP(c, s.machine, "ens5", "192.168.10.12/24")
+	res, err := s.machine.LinkLayerDevicesForSpaces([]string{""})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(res, gc.HasLen, 1)
+	devices, ok := res[""]
+	c.Assert(ok, jc.IsTrue)
+	c.Assert(devices, gc.HasLen, 2)
+	names := make([]string, len(devices))
+	for i, dev := range devices {
+		names[i] = dev.Name()
+	}
+	c.Check(names, gc.DeepEquals, []string{"ens5", "lxdbr0"})
+}
+
 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

state/machine_linklayerdevices.go

@@ -911,10 +911,11 @@ func deviceMapToSortedList(deviceMap map[string]*LinkLayerDevice) []*LinkLayerDe
 	return result
 }
 
-// LinkLayerDevicesForSpaces takes a list of spaces, and returns the devices
-// 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.)
+// LinkLayerDevicesForSpaces takes a list of spaces, and returns the devices on
+// this machine that are in that space that we feel would be useful for
+// containers to know about.  (eg, if there is a host device that has been
+// bridged, we return the bridge, rather than the underlying device, but if we
+// have only the host device, we return that.)
 // 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.
@@ -923,59 +924,72 @@ func (m *Machine) LinkLayerDevicesForSpaces(spaces []string) (map[string][]*Link
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
+	devices, err := m.AllLinkLayerDevices()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	deviceByName := make(map[string]*LinkLayerDevice, len(devices))
+	for _, dev := range devices {
+		deviceByName[dev.Name()] = dev
+	}
 	requestedSpaces := set.NewStrings(spaces...)
 	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
-	// 'aggregate all subnet CIDR' and then grab them in one pass, and then
-	// filter them to find the link layer devices we care about, and ask for
-	// them in a single pass again.
-	// Further, we should be tracking this more by Layer 2 connections, rather
-	// than requiring CIDR. Eventually we want to get rid of host network
-	// devices from having an IP address at all, so that only the containers
-	// have Layer 3 addresses.
+	processedDeviceNames := set.NewStrings()
+	includeDevice := func(spaceName string, device *LinkLayerDevice) {
+		spaceInfo, ok := spaceToDevices[spaceName]
+		if !ok {
+			spaceInfo = make(map[string]*LinkLayerDevice)
+			spaceToDevices[spaceName] = spaceInfo
+		}
+		spaceInfo[device.Name()] = device
+	}
+	// First pass, iterate the addresses, lookup the associated spaces, and
+	// gather the devices.
 	for _, addr := range addresses {
 		subnet, err := addr.Subnet()
 		spaceName := ""
 		if err != nil {
 			if errors.IsNotFound(err) {
-				// We record all addresses that we find on the
-				// machine. However, some devices may have IP
-				// addresses that are not in known subnets or spaces.
-				// (loopback devices aren't in a space, arbitrary
-				// application based addresses, etc.)
-				// The caller can request those devices by asking us for the
-				// "" space.
-				if !requestedSpaces.Contains("") {
-					continue
-				}
+				// unknown subnets are considered part of the "unknown" space
+				spaceName = ""
 			} else {
 				// We don't understand the error, so error out for now
 				return nil, errors.Trace(err)
 			}
 		} else {
 			spaceName = subnet.SpaceName()
 		}
-		if !requestedSpaces.Contains(spaceName) {
-			continue
-		}
-		device, err := addr.Device()
-		if err != nil {
-			return nil, errors.Trace(err)
+		device, ok := deviceByName[addr.DeviceName()]
+		if !ok {
+			return nil, errors.Errorf("address %v for machine %q refers to a missing device %q",
+				addr, m.Id(), addr.DeviceName())
 		}
+		processedDeviceNames.Add(device.Name())
 		if device.Type() == LoopbackDevice {
 			// We skip loopback devices here
 			continue
 		}
-		spaceInfo, ok := spaceToDevices[spaceName]
-		if !ok {
-			spaceInfo = make(map[string]*LinkLayerDevice)
-			spaceToDevices[spaceName] = spaceInfo
+		includeDevice(spaceName, device)
+	}
+	// Now grab any devices we may have missed. For now, any device without an
+	// address must be in the "unknown" space.
+	for devName, device := range deviceByName {
+		if processedDeviceNames.Contains(devName) {
+			continue
 		}
-		spaceInfo[device.Name()] = device
+		// Loopback devices aren't considered part of the empty space
+		// Also, devices that are attached to another device also aren't
+		// considered to be in the unknown space.
+		if device.Type() == LoopbackDevice || device.ParentName() != "" {
+			continue
+		}
+		includeDevice("", device)
 	}
 	result := make(map[string][]*LinkLayerDevice, len(spaceToDevices))
 	for spaceName, deviceMap := range spaceToDevices {
+		if !requestedSpaces.Contains(spaceName) {
+			continue
+		}
 		result[spaceName] = deviceMapToSortedList(deviceMap)
 	}
 	return result, nil

worker/machiner/machiner.go

@@ -144,7 +144,6 @@ func setMachineAddresses(tag names.MachineTag, m Machine) error {
 }
 
 func (mr *Machiner) Handle(_ <-chan struct{}) error {
-	logger.Infof("got a Machiner.Handle() event")
 	if err := mr.machine.Refresh(); params.IsCodeNotFoundOrCodeUnauthorized(err) {
 		// NOTE(axw) we can distinguish between NotFound and CodeUnauthorized,
 		// so we could call NotifyMachineDead here in case the agent failed to