diff --git a/apiserver/provisioner/provisioner.go b/apiserver/provisioner/provisioner.go index b393745bca7..f7772ce581a 100644 --- a/apiserver/provisioner/provisioner.go +++ b/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 { diff --git a/network/containerizer/bridgepolicy_test.go b/network/containerizer/bridgepolicy_test.go index 9b853dbd75b..438a43b3cec 100644 --- a/network/containerizer/bridgepolicy_test.go +++ b/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 diff --git a/state/linklayerdevices_test.go b/state/linklayerdevices_test.go index a6b55451b4d..a5d3f120459 100644 --- a/state/linklayerdevices_test.go +++ b/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 diff --git a/state/machine_linklayerdevices.go b/state/machine_linklayerdevices.go index 66af48163af..49ceaffeefe 100644 --- a/state/machine_linklayerdevices.go +++ b/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,32 +924,34 @@ 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) @@ -956,26 +959,37 @@ func (m *Machine) LinkLayerDevicesForSpaces(spaces []string) (map[string][]*Link } 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 diff --git a/worker/machiner/machiner.go b/worker/machiner/machiner.go index 074b8ab32f4..25a01018494 100644 --- a/worker/machiner/machiner.go +++ b/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