Merge changes from 2.4 branch.

SimonRichardson · Nov 22, 2018 · 53d1bf2 · 53d1bf2
2 parents 62ab711 + bc96a4d
commit 53d1bf2
Show file tree

Hide file tree

Showing 29 changed files with 484 additions and 86 deletions.
diff --git a/README.md b/README.md
@@ -96,14 +96,21 @@ Installing prerequisites
 
 ### *Making use of Makefile*
 
-The `juju` repository contains a `Makefile`, which is the preferred way to install dependencies and other features.
-It is advisable, when installing `juju` from source, to look at the [Makefile](./Makefile), located in `$GOPATH/src/github.com/juju/juju/Makefile`.
+The `juju` repository contains a `Makefile`, which is the preferred way to
+install dependencies and other features.  It is advisable, when installing
+`juju` from source, to look at the [Makefile](./Makefile), located in
+`$GOPATH/src/github.com/juju/juju/Makefile`.
 
 ### *Dependencies*
 
 Juju needs some dependencies in order to be installed and the preferred way to
 collect the necessary packages is to use the provided `Makefile`.
+<<<<<<< HEAD
 The target `dep` will download the go packages listed in `Gopkg.lock`. The following bash code will install the dependencies.
+=======
+The target `godeps` will download the go packages listed in `dependencies.tsv`.
+The following bash code will install the dependencies.
+>>>>>>> upstream/2.4
 
     cd $GOPATH/src/github.com/juju/juju
     export JUJU_MAKE_GODEPS=true

diff --git a/apiserver/common/crossmodel/state.go b/apiserver/common/crossmodel/state.go
@@ -31,7 +31,8 @@ func (p *statePoolShim) Get(modelUUID string) (Backend, func(), error) {
 	}
 	model, err := st.Model()
 	if err != nil {
-		return stateShim{}, closer, err
+		closer()
+		return nil, nil, err
 	}
 	return stateShim{st.State, model}, closer, err
 }

diff --git a/apiserver/common/modelmanagerinterface.go b/apiserver/common/modelmanagerinterface.go
@@ -151,6 +151,7 @@ func (st modelManagerStateShim) GetBackend(modelUUID string) (ModelManagerBacken
 	}
 	otherModel, err := otherState.Model()
 	if err != nil {
+		otherState.Release()
 		return nil, nil, err
 	}
 	return modelManagerStateShim{otherState.State, otherModel, st.pool}, otherState.Release, nil

diff --git a/apiserver/common/storagecommon/blockdevices.go b/apiserver/common/storagecommon/blockdevices.go
@@ -4,10 +4,14 @@
 package storagecommon
 
 import (
+	"github.com/juju/loggo"
+
 	"github.com/juju/juju/state"
 	"github.com/juju/juju/storage"
 )
 
+var logger = loggo.GetLogger("juju.apiserver.storagecommon")
+
 // BlockDeviceFromState translates a state.BlockDeviceInfo to a
 // storage.BlockDevice.
 func BlockDeviceFromState(in state.BlockDeviceInfo) storage.BlockDevice {
@@ -34,6 +38,7 @@ func MatchingBlockDevice(
 	attachmentInfo state.VolumeAttachmentInfo,
 	planBlockInfo state.BlockDeviceInfo,
 ) (*state.BlockDeviceInfo, bool) {
+	logger.Tracef("looking for block device for volume %#v", volumeInfo)
 	for _, dev := range blockDevices {
 		if planBlockInfo.HardwareId != "" {
 			if planBlockInfo.HardwareId == dev.HardwareId {
@@ -56,18 +61,21 @@ func MatchingBlockDevice(
 			if volumeInfo.WWN == dev.WWN {
 				return &dev, true
 			}
+			logger.Tracef("no match for block device WWN: %v", dev.WWN)
 			continue
 		}
 		if volumeInfo.HardwareId != "" {
 			if volumeInfo.HardwareId == dev.HardwareId {
 				return &dev, true
 			}
+			logger.Tracef("no match for block device hardware id: %v", dev.HardwareId)
 			continue
 		}
 		if attachmentInfo.BusAddress != "" {
 			if attachmentInfo.BusAddress == dev.BusAddress {
 				return &dev, true
 			}
+			logger.Tracef("no match for block device bus address: %v", dev.BusAddress)
 			continue
 		}
 		if attachmentInfo.DeviceLink != "" {
@@ -76,11 +84,13 @@ func MatchingBlockDevice(
 					return &dev, true
 				}
 			}
+			logger.Tracef("no match for block device dev links: %v", dev.DeviceLinks)
 			continue
 		}
 		if attachmentInfo.DeviceName == dev.DeviceName {
 			return &dev, true
 		}
+		logger.Tracef("no match for block device name: %v", dev.DeviceName)
 	}
 	return nil, false
 }
diff --git a/cloudconfig/cloudinit/network_ubuntu.go b/cloudconfig/cloudinit/network_ubuntu.go
@@ -432,6 +432,5 @@ if __name__ == "__main__":
     main()
 `
 
-const CloudInitNetworkConfigDisabled = `network:
-  config: "disabled"
+const CloudInitNetworkConfigDisabled = `config: "disabled"
 `
diff --git a/cmd/juju/cloud/list_test.go b/cmd/juju/cloud/list_test.go
@@ -29,6 +29,7 @@ func (s *listSuite) TestListPublic(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	out := cmdtesting.Stdout(ctx)
 	out = strings.Replace(out, "\n", "", -1)
+
 	// Check that we are producing the expected fields
 	c.Assert(out, gc.Matches, `Cloud        Regions  Default        Type        Description.*`)
 	// // Just check couple of snippets of the output to make sure it looks ok.

diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go
@@ -459,8 +459,9 @@ func (a *MachineAgent) Run(*cmd.Context) (err error) {
 	}
 
 	agentConfig := a.CurrentConfig()
+	agentName := a.Tag().String()
 	machineLock, err := machinelock.New(machinelock.Config{
-		AgentName:   a.Tag().String(),
+		AgentName:   agentName,
 		Clock:       clock.WallClock,
 		Logger:      loggo.GetLogger("juju.machinelock"),
 		LogFilename: agent.MachineLockLogFilename(agentConfig),
@@ -473,7 +474,7 @@ func (a *MachineAgent) Run(*cmd.Context) (err error) {
 	a.machineLock = machineLock
 	a.upgradeComplete = upgradesteps.NewLock(agentConfig)
 
-	createEngine := a.makeEngineCreator(agentConfig.UpgradedToVersion())
+	createEngine := a.makeEngineCreator(agentName, agentConfig.UpgradedToVersion())
 	charmrepo.CacheDir = filepath.Join(agentConfig.DataDir(), "charmcache")
 	if err := a.createJujudSymlinks(agentConfig.DataDir()); err != nil {
 		return err
@@ -496,7 +497,7 @@ func (a *MachineAgent) Run(*cmd.Context) (err error) {
 	return cmdutil.AgentDone(logger, err)
 }
 
-func (a *MachineAgent) makeEngineCreator(previousAgentVersion version.Number) func() (worker.Worker, error) {
+func (a *MachineAgent) makeEngineCreator(agentName string, previousAgentVersion version.Number) func() (worker.Worker, error) {
 	return func() (worker.Worker, error) {
 		engine, err := dependency.NewEngine(dependencyEngineConfig())
 		if err != nil {
@@ -549,6 +550,7 @@ func (a *MachineAgent) makeEngineCreator(previousAgentVersion version.Number) fu
 
 		manifolds := machineManifolds(machine.ManifoldsConfig{
 			PreviousAgentVersion:    previousAgentVersion,
+			AgentName:               agentName,
 			Agent:                   agent.APIHostPortsSetter{Agent: a},
 			RootDir:                 a.rootDir,
 			AgentConfigChanged:      a.configChangedVal,

diff --git a/cmd/jujud/agent/machine/manifolds.go b/cmd/jujud/agent/machine/manifolds.go
@@ -108,6 +108,12 @@ const (
 // ManifoldsConfig allows specialisation of the result of Manifolds.
 type ManifoldsConfig struct {
 
+	// AgentName is the name of the machine agent, like "machine-12".
+	// This will never change during the execution of an agent, and
+	// is used to provide this as config into a worker rather than
+	// making the worker get it from the agent worker itself.
+	AgentName string
+
 	// Agent contains the agent that will be wrapped and made available to
 	// its dependencies via a dependency.Engine.
 	Agent coreagent.Agent
@@ -735,6 +741,7 @@ func Manifolds(config ManifoldsConfig) dependency.Manifolds {
 			APIServerName:        apiServerName,
 			RaftTransportName:    raftTransportName,
 			PrometheusRegisterer: config.PrometheusRegisterer,
+			AgentName:            config.AgentName,
 			Clock:                config.Clock,
 			GetControllerConfig:  httpserver.GetControllerConfig,
 			NewTLSConfig:         httpserver.NewTLSConfig,

diff --git a/container/lxd/export_test.go b/container/lxd/export_test.go
@@ -40,6 +40,10 @@ func PatchGenerateVirtualMACAddress(patcher patcher) {
 	})
 }
 
+func PatchLXDViaSnap(patcher patcher, isSnap bool) {
+	patcher.PatchValue(&lxdViaSnap, func() bool { return isSnap })
+}
+
 func GetImageSources(mgr container.Manager) ([]ServerSpec, error) {
 	return mgr.(*containerManager).getImageSources()
 }

diff --git a/container/lxd/initialisation_linux.go b/container/lxd/initialisation_linux.go
@@ -21,6 +21,7 @@ import (
 	"github.com/juju/packaging/manager"
 	"github.com/juju/proxy"
 	"github.com/juju/utils/series"
+	"github.com/lxc/lxd/shared"
 
 	"github.com/juju/juju/container"
 	"github.com/juju/juju/service"
@@ -255,7 +256,12 @@ func editLXDBridgeFile(input string, subnet string) string {
 // apt.GetInstall.
 func ensureDependencies(series string) error {
 	if series == "precise" {
-		return fmt.Errorf("LXD is not supported in precise.")
+		return errors.NotSupportedf(`LXD containers on series "precise"`)
+	}
+
+	if lxdViaSnap() {
+		logger.Infof("LXD snap is installed; skipping package installation")
+		return nil
 	}
 
 	pacman, err := getPackageManager(series)
@@ -286,6 +292,12 @@ func ensureDependencies(series string) error {
 	return errors.Trace(err)
 }
 
+// lxdViaSnap interrogates the location of the Snap LXD socket in order
+// to determine if LXD is being provided via that method.
+var lxdViaSnap = func() bool {
+	return shared.IsUnixSocket("/var/snap/lxd/common/lxd/unix.socket")
+}
+
 // randomizedOctetRange is a variable for testing purposes.
 var randomizedOctetRange = func() []int {
 	rand.Seed(time.Now().UnixNano())
@@ -376,26 +388,22 @@ func parseLXDBridgeConfigValues(input string) map[string]string {
 
 	for _, line := range strings.Split(input, "\n") {
 		line = strings.TrimSpace(line)
-
 		if line == "" || strings.HasPrefix(line, "#") || !strings.Contains(line, "=") {
 			continue
 		}
 
 		tokens := strings.Split(line, "=")
-
 		if tokens[0] == "" {
 			continue // no key
 		}
 
 		value := ""
-
 		if len(tokens) > 1 {
 			value = tokens[1]
 			if strings.HasPrefix(value, `"`) && strings.HasSuffix(value, `"`) {
 				value = strings.Trim(value, `"`)
 			}
 		}
-
 		values[tokens[0]] = value
 	}
 	return values
@@ -424,41 +432,44 @@ func bridgeConfiguration(input string) (string, error) {
 var IsRunningLocally = isRunningLocally
 
 func isRunningLocally() (bool, error) {
-	installed, err := IsInstalledLocally()
-	if err != nil {
-		return installed, errors.Trace(err)
-	}
-	if !installed {
-		return false, nil
+	svcName, err := InstalledServiceName()
+	if svcName == "" || err != nil {
+		return false, errors.Trace(err)
 	}
 
 	hostSeries, err := series.HostSeries()
 	if err != nil {
 		return false, errors.Trace(err)
 	}
-	svc, err := service.NewService("lxd", common.Conf{}, hostSeries)
+
+	svc, err := service.NewService(svcName, common.Conf{}, hostSeries)
 	if err != nil {
 		return false, errors.Trace(err)
 	}
-
 	running, err := svc.Running()
 	if err != nil {
 		return running, errors.Trace(err)
 	}
-
 	return running, nil
 }
 
-// IsInstalledLocally returns true if LXD is installed locally.
-func IsInstalledLocally() (bool, error) {
+// InstalledServiceName returns the name of the running service for the LXD
+// daemon. If LXD is not installed, the return is an empty string.
+func InstalledServiceName() (string, error) {
 	names, err := service.ListServices()
 	if err != nil {
-		return false, errors.Trace(err)
+		return "", errors.Trace(err)
 	}
+
+	// Prefer the Snap service.
+	svcName := ""
 	for _, name := range names {
+		if name == "snap.lxd.daemon" {
+			return name, nil
+		}
 		if name == "lxd" {
-			return true, nil
+			svcName = name
 		}
 	}
-	return false, nil
+	return svcName, nil
 }
diff --git a/container/lxd/initialisation_test.go b/container/lxd/initialisation_test.go
@@ -110,32 +110,40 @@ func getMockRunCommandWithRetry(calledCmds *[]string) func(string, func(string)
 }
 
 func (s *InitialiserSuite) TestLTSSeriesPackages(c *gc.C) {
-	// Momentarily, the only series with a dedicated cloud archive is precise,
-	// which we will use for the following test:
 	paccmder, err := commands.NewPackageCommander("trusty")
 	c.Assert(err, jc.ErrorIsNil)
 
+	PatchLXDViaSnap(s, false)
 	s.PatchValue(&series.MustHostSeries, func() string { return "trusty" })
-	container := NewContainerInitialiser("trusty")
 
-	err = container.Initialise()
+	err = NewContainerInitialiser("trusty").Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
 	c.Assert(s.calledCmds, gc.DeepEquals, []string{
 		paccmder.InstallCmd("--target-release", "trusty-backports", "lxd"),
 	})
 }
 
+func (s *InitialiserSuite) TestSnapInstalledNoAptInstall(c *gc.C) {
+	PatchLXDViaSnap(s, true)
+	s.PatchValue(&series.MustHostSeries, func() string { return "cosmic" })
+
+	err := NewContainerInitialiser("cosmic").Initialise()
+	c.Assert(err, jc.ErrorIsNil)
+
+	c.Assert(s.calledCmds, gc.DeepEquals, []string{})
+}
+
 func (s *InitialiserSuite) TestNoSeriesPackages(c *gc.C) {
+	PatchLXDViaSnap(s, false)
+
 	// Here we want to test for any other series whilst avoiding the
 	// possibility of hitting a cloud archive-requiring release.
 	// As such, we simply pass an empty series.
 	paccmder, err := commands.NewPackageCommander("xenial")
 	c.Assert(err, jc.ErrorIsNil)
 
-	container := NewContainerInitialiser("")
-
-	err = container.Initialise()
+	err = NewContainerInitialiser("").Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
 	c.Assert(s.calledCmds, gc.DeepEquals, []string{

diff --git a/network/containerizer/bridgepolicy.go b/network/containerizer/bridgepolicy.go
@@ -180,7 +180,9 @@ var skippedDeviceNames = set.NewStrings(
 // 2. Add b- to device name, if it doesn't fit in 15 characters then:
 // 3a. For devices starting in 'en' remove 'en' and add 'b-'
 // 3b. For all other devices 'b-' + 6-char hash of name + '-' + last 6 chars of name
+// 4. If using the device name directly always replace '.' with '-', to make sure that bridges from VLANs won't break
 func BridgeNameForDevice(device string) string {
+	device = strings.Replace(device, ".", "-", -1)
 	switch {
 	case len(device) < 13:
 		return fmt.Sprintf("br-%s", device)