Merge pull request #6942 from jameinel/2.1-into-develop

2.1 into develop

Merge 2.1 into develop including:

- Add SNAP path to bash completion
- Don't fallback to lxdbr0 when there is an error during container initialization.

apiserver/provisioner/provisioner.go

@@ -723,7 +723,7 @@ func (ctx *prepareOrGetContext) ProcessOneContainer(env environs.Environ, idx in
 		}
 
 		if len(parentAddrs) > 0 {
-			logger.Infof("host machine device %q has addresses %v", parentDevice.Name(), parentAddrs)
+			logger.Debugf("host machine device %q has addresses %v", parentDevice.Name(), parentAddrs)
 			firstAddress := parentAddrs[0]
 			if supportContainerAddresses {
 				parentDeviceSubnet, err := firstAddress.Subnet()
@@ -744,7 +744,10 @@ func (ctx *prepareOrGetContext) ProcessOneContainer(env environs.Environ, idx in
 				info.VLANTag = 0
 			}
 		} else {
-			logger.Infof("host machine device %q has no addresses %v", parentDevice.Name(), parentAddrs)
+			logger.Debugf("host machine device %q has no addresses %v", parentDevice.Name(), parentAddrs)
+			info.ConfigType = network.ConfigDHCP
+			info.ProviderSubnetId = ""
+			info.VLANTag = 0
 		}
 
 		logger.Tracef("prepared info for container interface %q: %+v", info.InterfaceName, info)
@@ -765,10 +768,12 @@ func (ctx *prepareOrGetContext) ProcessOneContainer(env environs.Environ, idx in
 			return err
 		}
 		logger.Debugf("got allocated info from provider: %+v", allocatedInfo)
+	} else {
+		logger.Debugf("using dhcp allocated addresses")
 	}
 
 	allocatedConfig := networkingcommon.NetworkConfigFromInterfaceInfo(allocatedInfo)
-	logger.Tracef("allocated network config: %+v", allocatedConfig)
+	logger.Debugf("allocated network config: %+v", allocatedConfig)
 	ctx.result.Results[idx].Config = allocatedConfig
 	return nil
 }

container/network.go

@@ -49,12 +49,3 @@ func BridgeNetworkConfig(device string, mtu int, interfaces []network.InterfaceI
 	}
 	return &NetworkConfig{BridgeNetwork, device, mtu, interfaces}
 }
-
-// PhysicalNetworkConfig returns a valid NetworkConfig to use the
-// specified device as the network device for the container. It also
-// allows passing in specific configuration for the container's
-// network interfaces and default MTU to use. If interfaces is nil the
-// default configuration is used for the respective container type.
-func PhysicalNetworkConfig(device string, mtu int, interfaces []network.InterfaceInfo) *NetworkConfig {
-	return &NetworkConfig{PhysicalNetwork, device, mtu, interfaces}
-}

etc/bash_completion.d/juju-2

@@ -344,6 +344,9 @@ if [ -x "$GOPATH/bin/juju" ]; then
 # Detect installed juju-2 binary (next highest priority)
 elif [ -x "/usr/bin/juju-2" ]; then
     export _juju_cmd_JUJU_2="/usr/bin/juju-2"
+# Snap version of juju
+elif [ -x "/snap/bin/juju" ]; then
+    export _juju_cmd_JUJU_2="/snap/bin/juju"
 # Use /usr/bin/juju as a last resort.
 else
     export _juju_cmd_JUJU_2="/usr/bin/juju"

worker/provisioner/broker.go

@@ -86,7 +86,6 @@ func prepareHost(bridger network.Bridger, hostMachineID string, containerTag nam
 func prepareOrGetContainerInterfaceInfo(
 	api APICalls,
 	machineID string,
-	bridgeDevice string,
 	allocateOrMaintain bool,
 	log loggo.Logger,
 ) ([]network.InterfaceInfo, error) {

worker/provisioner/kvm-broker.go

@@ -53,14 +53,6 @@ func (broker *kvmBroker) StartInstance(args environs.StartInstanceParams) (*envi
 	containerMachineID := args.InstanceConfig.MachineId
 	kvmLogger.Infof("starting kvm container for containerMachineID: %s", containerMachineID)
 
-	// TODO: Default to using the host network until we can configure.  Yes,
-	// this is using the LxcBridge value, we should put it in the api call for
-	// container config.
-	bridgeDevice := broker.agentConfig.Value(agent.LxcBridge)
-	if bridgeDevice == "" {
-		bridgeDevice = container.DefaultKvmBridge
-	}
-
 	config, err := broker.api.ContainerConfig()
 	if err != nil {
 		kvmLogger.Errorf("failed to get container config: %v", err)
@@ -75,24 +67,28 @@ func (broker *kvmBroker) StartInstance(args environs.StartInstanceParams) (*envi
 	preparedInfo, err := prepareOrGetContainerInterfaceInfo(
 		broker.api,
 		containerMachineID,
-		bridgeDevice,
 		true, // allocate if possible, do not maintain existing.
 		kvmLogger,
 	)
 	if err != nil {
-		// It's not fatal (yet) if we couldn't pre-allocate addresses for the
-		// container.
-		logger.Infof("failed to prepare container %q network config: %v", containerMachineID, err)
-	} else {
-		args.NetworkInfo = preparedInfo
+		return nil, errors.Trace(err)
 	}
 
-	network := container.BridgeNetworkConfig(bridgeDevice, 0, args.NetworkInfo)
-	interfaces, err := finishNetworkConfig(bridgeDevice, args.NetworkInfo)
+	// Something to fallback to if there are no devices given in args.NetworkInfo
+	// TODO(jam): 2017-02-07, this feels like something that should never need
+	// to be invoked, because either StartInstance or
+	// prepareOrGetContainerInterfaceInfo should always return a value. The
+	// test suite currently doesn't think so, and I'm hesitant to munge it too
+	// much.
+	bridgeDevice := broker.agentConfig.Value(agent.LxcBridge)
+	if bridgeDevice == "" {
+		bridgeDevice = container.DefaultKvmBridge
+	}
+	interfaces, err := finishNetworkConfig(bridgeDevice, preparedInfo)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	network.Interfaces = interfaces
+	network := container.BridgeNetworkConfig(bridgeDevice, 0, interfaces)
 
 	// The provisioner worker will provide all tools it knows about
 	// (after applying explicitly specified constraints), which may
@@ -152,17 +148,10 @@ func (broker *kvmBroker) StartInstance(args environs.StartInstanceParams) (*envi
 func (broker *kvmBroker) MaintainInstance(args environs.StartInstanceParams) error {
 	machineID := args.InstanceConfig.MachineId
 
-	// Default to using the host network until we can configure.
-	bridgeDevice := broker.agentConfig.Value(agent.LxcBridge)
-	if bridgeDevice == "" {
-		bridgeDevice = container.DefaultKvmBridge
-	}
-
 	// There's no InterfaceInfo we expect to get below.
 	_, err := prepareOrGetContainerInterfaceInfo(
 		broker.api,
 		machineID,
-		bridgeDevice,
 		false, // maintain, do not allocate.
 		kvmLogger,
 	)

worker/provisioner/kvm-broker_test.go

@@ -302,18 +302,8 @@ func (s *kvmBrokerSuite) TestStartInstancePopulatesFallbackNetworkInfo(c *gc.C)
 		nil, // HostChangesForContainer succeeds
 		errors.NotSupportedf("container address allocation"),
 	)
-	result, err := s.startInstance(c, broker, "1/kvm/2")
-	c.Assert(err, jc.ErrorIsNil)
-
-	c.Assert(result.NetworkInfo, jc.DeepEquals, []network.InterfaceInfo{{
-		DeviceIndex:         0,
-		InterfaceName:       "eth0",
-		InterfaceType:       network.EthernetInterface,
-		ConfigType:          network.ConfigDHCP,
-		ParentInterfaceName: "virbr0",
-		DNSServers:          network.NewAddresses("ns1.dummy", "ns2.dummy"),
-		DNSSearchDomains:    []string{"dummy", "invalid"},
-	}})
+	_, err := s.startInstance(c, broker, "1/kvm/2")
+	c.Assert(err, gc.ErrorMatches, "container address allocation not supported")
 }
 
 type kvmProvisionerSuite struct {

worker/provisioner/lxd-broker.go

@@ -44,10 +44,6 @@ type lxdBroker struct {
 
 func (broker *lxdBroker) StartInstance(args environs.StartInstanceParams) (*environs.StartInstanceResult, error) {
 	containerMachineID := args.InstanceConfig.MachineId
-	bridgeDevice := broker.agentConfig.Value(agent.LxdBridge)
-	if bridgeDevice == "" {
-		bridgeDevice = network.DefaultLXDBridge
-	}
 
 	config, err := broker.api.ContainerConfig()
 	if err != nil {
@@ -63,24 +59,27 @@ func (broker *lxdBroker) StartInstance(args environs.StartInstanceParams) (*envi
 	preparedInfo, err := prepareOrGetContainerInterfaceInfo(
 		broker.api,
 		containerMachineID,
-		bridgeDevice,
 		true, // allocate if possible, do not maintain existing.
 		lxdLogger,
 	)
 	if err != nil {
-		// It's not fatal (yet) if we couldn't pre-allocate addresses for the
-		// container.
-		logger.Warningf("failed to prepare container %q network config: %v", containerMachineID, err)
-	} else {
-		args.NetworkInfo = preparedInfo
+		return nil, errors.Trace(err)
 	}
-
-	network := container.BridgeNetworkConfig(bridgeDevice, 0, args.NetworkInfo)
-	interfaces, err := finishNetworkConfig(bridgeDevice, args.NetworkInfo)
+	// Something to fallback to if there are no devices given in args.NetworkInfo
+	// TODO(jam): 2017-02-07, this feels like something that should never need
+	// to be invoked, because either StartInstance or
+	// prepareOrGetContainerInterfaceInfo should always return a value. The
+	// test suite currently doesn't think so, and I'm hesitant to munge it too
+	// much.
+	bridgeDevice := broker.agentConfig.Value(agent.LxcBridge)
+	if bridgeDevice == "" {
+		bridgeDevice = container.DefaultLxdBridge
+	}
+	interfaces, err := finishNetworkConfig(bridgeDevice, preparedInfo)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	network.Interfaces = interfaces
+	network := container.BridgeNetworkConfig(bridgeDevice, 0, interfaces)
 
 	// The provisioner worker will provide all tools it knows about
 	// (after applying explicitly specified constraints), which may
@@ -152,17 +151,10 @@ func (broker *lxdBroker) AllInstances() (result []instance.Instance, err error)
 func (broker *lxdBroker) MaintainInstance(args environs.StartInstanceParams) error {
 	machineID := args.InstanceConfig.MachineId
 
-	// Default to using the host network until we can configure.
-	bridgeDevice := broker.agentConfig.Value(agent.LxdBridge)
-	if bridgeDevice == "" {
-		bridgeDevice = network.DefaultLXDBridge
-	}
-
 	// There's no InterfaceInfo we expect to get below.
 	_, err := prepareOrGetContainerInterfaceInfo(
 		broker.api,
 		machineID,
-		bridgeDevice,
 		false, // maintain, do not allocate.
 		lxdLogger,
 	)

worker/provisioner/lxd-broker_test.go

@@ -204,18 +204,8 @@ func (s *lxdBrokerSuite) TestStartInstancePopulatesFallbackNetworkInfo(c *gc.C)
 		nil, // HostChangesForContainer succeeds
 		errors.NotSupportedf("container address allocation"),
 	)
-	result, err := s.startInstance(c, broker, "1/lxd/0")
-	c.Assert(err, jc.ErrorIsNil)
-
-	c.Assert(result.NetworkInfo, jc.DeepEquals, []network.InterfaceInfo{{
-		DeviceIndex:         0,
-		InterfaceName:       "eth0",
-		InterfaceType:       network.EthernetInterface,
-		ConfigType:          network.ConfigDHCP,
-		ParentInterfaceName: "lxdbr0",
-		DNSServers:          network.NewAddresses("ns1.dummy", "ns2.dummy"),
-		DNSSearchDomains:    []string{"dummy", "invalid"},
-	}})
+	_, err := s.startInstance(c, broker, "1/lxd/0")
+	c.Assert(err, gc.ErrorMatches, "container address allocation not supported")
 }
 
 func (s *lxdBrokerSuite) TestStartInstanceNoHostArchTools(c *gc.C) {