Auto-switch to configured snap channel for lxd during container init

When we try to initialize support for lxd on a juju machine, the code previously checked if lxd has been installed as a snap and then skipped the dependency installation step (note that settings like snap proxies etc. were still set up as expected). With the introduction of the per-model lxd-snap-channel option, the code has been updated to additionally check whether the installed snap uses the channel configured by the operator. In case of a mismatch, juju will run a 'snap channel' command to switch to the appropriate channel before following the remainder of the initialization steps. It is important to note that the images which juju uses (cosmic+) include a pre-installed snap whose channel includes a branch portion for the specific release (e.g. latest/stable/ubuntu-20.04 on focal). In order to avoid unnecessary channel changes (which trigger snap downloads), the code compares the operator-defined settings to the tracked channel via a prefix match instead of an equality check. This ensures that we can use a sane model default ("latest/stable") that works across all releases.
jangocheng · Apr 23, 2020 · 8d3053f · 8d3053f
1 parent 0909e6b
commit 8d3053f
Show file tree

Hide file tree

Showing 8 changed files with 204 additions and 30 deletions.
diff --git a/container/lxd/export_test.go b/container/lxd/export_test.go
@@ -49,6 +49,10 @@ func PatchHostSeries(patcher patcher, series string) {
 	patcher.PatchValue(&hostSeries, func() (string, error) { return series, nil })
 }
 
+func PatchGetSnapManager(patcher patcher, mgr SnapManager) {
+	patcher.PatchValue(&getSnapManager, func() SnapManager { return mgr })
+}
+
 func GetImageSources(mgr container.Manager) ([]ServerSpec, error) {
 	return mgr.(*containerManager).getImageSources()
 }

diff --git a/container/lxd/initialisation.go b/container/lxd/initialisation.go
@@ -20,7 +20,7 @@ type containerInitialiser struct {
 var _ container.Initialiser = (*containerInitialiser)(nil)
 
 // NewContainerInitialiser  - on anything but Linux this is a NOP
-func NewContainerInitialiser() container.Initialiser {
+func NewContainerInitialiser(string) container.Initialiser {
 	return &containerInitialiser{}
 }
 

diff --git a/container/lxd/initialisation_linux.go b/container/lxd/initialisation_linux.go
@@ -17,6 +17,7 @@ import (
 
 	"github.com/juju/collections/set"
 	"github.com/juju/errors"
+	"github.com/juju/packaging/manager"
 	"github.com/juju/proxy"
 	"github.com/juju/utils/series"
 	"github.com/lxc/lxd/shared"
@@ -34,16 +35,34 @@ var requiredPackages = []string{"lxd"}
 
 type containerInitialiser struct {
 	getExecCommand func(string, ...string) *exec.Cmd
+	lxdSnapChannel string
+}
+
+//go:generate mockgen -package mocks -destination mocks/snap_manager_mock.go github.com/juju/juju/container/lxd SnapManager
+
+// SnapManager defines an interface implemented by types that can query and/or
+// change the channel for installed snaps.
+type SnapManager interface {
+	InstalledChannel(string) string
+	ChangeChannel(string, string) error
+}
+
+// getSnapManager returns a snap manager implementation that is used to query
+// and/or change the channel for the installed lxd snap. Defined as a function
+// so it can be overridden by tests.
+var getSnapManager = func() SnapManager {
+	return manager.NewSnapPackageManager()
 }
 
 // containerInitialiser implements container.Initialiser.
 var _ container.Initialiser = (*containerInitialiser)(nil)
 
 // NewContainerInitialiser returns an instance used to perform the steps
 // required to allow a host machine to run a LXC container.
-func NewContainerInitialiser() container.Initialiser {
+func NewContainerInitialiser(lxdSnapChannel string) container.Initialiser {
 	return &containerInitialiser{
-		exec.Command,
+		getExecCommand: exec.Command,
+		lxdSnapChannel: lxdSnapChannel,
 	}
 }
 
@@ -54,7 +73,7 @@ func (ci *containerInitialiser) Initialise() error {
 		return errors.Trace(err)
 	}
 
-	if err := ensureDependencies(localSeries); err != nil {
+	if err := ensureDependencies(ci.lxdSnapChannel, localSeries); err != nil {
 		return errors.Trace(err)
 	}
 	err = configureLXDBridge()
@@ -246,13 +265,31 @@ func editLXDBridgeFile(input string, subnet string) string {
 }
 
 // ensureDependencies install the required dependencies for running LXD.
-func ensureDependencies(series string) error {
+func ensureDependencies(lxdSnapChannel, series string) error {
+	// If the snap is already installed, check whether the operator asked
+	// us to use a different channel. If so, switch to it.
 	if lxdViaSnap() {
-		logger.Infof("LXD snap is installed; skipping package installation")
+		snapManager := getSnapManager()
+		trackedChannel := snapManager.InstalledChannel("lxd")
+		// Note that images with pre-installed snaps are normally
+		// tracking "latest/stable/ubuntu-$release_number". As our
+		// default model config setting is "latest/stable", we perform
+		// a starts-with check instead of an equality check to avoid
+		// switching channels when we don't actually need to.
+		if strings.HasPrefix(trackedChannel, lxdSnapChannel) {
+			logger.Infof("LXD snap is already installed (channel: %s); skipping package installation", trackedChannel)
+			return nil
+		}
+
+		// We need to switch to a different channel
+		logger.Infof("switching LXD snap channel from %s to %s", trackedChannel, lxdSnapChannel)
+		if err := snapManager.ChangeChannel("lxd", lxdSnapChannel); err != nil {
+			return errors.Trace(err)
+		}
 		return nil
 	}
 
-	if err := packaging.InstallDependency(dependency.LXD(), series); err != nil {
+	if err := packaging.InstallDependency(dependency.LXD(lxdSnapChannel), series); err != nil {
 		return errors.Trace(err)
 	}
 

diff --git a/container/lxd/initialisation_test.go b/container/lxd/initialisation_test.go
@@ -22,6 +22,7 @@ import (
 	gc "gopkg.in/check.v1"
 
 	"github.com/golang/mock/gomock"
+	"github.com/juju/juju/container/lxd/mocks"
 	lxdtesting "github.com/juju/juju/container/lxd/testing"
 	coretesting "github.com/juju/juju/testing"
 	lxd "github.com/lxc/lxd/client"
@@ -91,6 +92,8 @@ LXD_IPV6_NAT="true"
 LXD_IPV6_PROXY="true"
 `
 
+const lxdSnapChannel = "latest/stable"
+
 func (s *InitialiserSuite) SetUpTest(c *gc.C) {
 	s.initialiserTestSuite.SetUpTest(c)
 	s.calledCmds = []string{}
@@ -127,7 +130,7 @@ func (s *InitialiserSuite) TestLTSSeriesPackages(c *gc.C) {
 	PatchLXDViaSnap(s, false)
 	PatchHostSeries(s, "trusty")
 
-	err = NewContainerInitialiser().Initialise()
+	err = NewContainerInitialiser(lxdSnapChannel).Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
 	c.Assert(s.calledCmds, gc.DeepEquals, []string{
@@ -139,12 +142,58 @@ func (s *InitialiserSuite) TestSnapInstalledNoAptInstall(c *gc.C) {
 	PatchLXDViaSnap(s, true)
 	PatchHostSeries(s, "cosmic")
 
-	err := NewContainerInitialiser().Initialise()
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+
+	mgr := mocks.NewMockSnapManager(ctrl)
+	mgr.EXPECT().InstalledChannel("lxd").Return("latest/stable")
+	PatchGetSnapManager(s, mgr)
+
+	err := NewContainerInitialiser(lxdSnapChannel).Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
 	c.Assert(s.calledCmds, gc.DeepEquals, []string{})
 }
 
+func (s *InitialiserSuite) TestSnapChannelMismatch(c *gc.C) {
+	PatchLXDViaSnap(s, true)
+	PatchHostSeries(s, "focal")
+
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+
+	mgr := mocks.NewMockSnapManager(ctrl)
+	gomock.InOrder(
+		mgr.EXPECT().InstalledChannel("lxd").Return("3.2/stable"),
+		mgr.EXPECT().ChangeChannel("lxd", lxdSnapChannel),
+	)
+	PatchGetSnapManager(s, mgr)
+
+	err := NewContainerInitialiser(lxdSnapChannel).Initialise()
+	c.Assert(err, jc.ErrorIsNil)
+}
+
+func (s *InitialiserSuite) TestSnapChannelPrefixMatch(c *gc.C) {
+	PatchLXDViaSnap(s, true)
+	PatchHostSeries(s, "focal")
+
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+
+	mgr := mocks.NewMockSnapManager(ctrl)
+	gomock.InOrder(
+		// The channel for the installed lxd snap also includes the
+		// branch for the focal release. The "track/risk" prefix is
+		// the same however so the container manager should not attempt
+		// to change the channel.
+		mgr.EXPECT().InstalledChannel("lxd").Return("latest/stable/ubuntu-20.04"),
+	)
+	PatchGetSnapManager(s, mgr)
+
+	err := NewContainerInitialiser(lxdSnapChannel).Initialise()
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 func (s *InitialiserSuite) TestNoSeriesPackages(c *gc.C) {
 	PatchLXDViaSnap(s, false)
 
@@ -156,7 +205,7 @@ func (s *InitialiserSuite) TestNoSeriesPackages(c *gc.C) {
 	paccmder, err := commands.NewPackageCommander("xenial")
 	c.Assert(err, jc.ErrorIsNil)
 
-	err = NewContainerInitialiser().Initialise()
+	err = NewContainerInitialiser(lxdSnapChannel).Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
 	c.Assert(s.calledCmds, gc.DeepEquals, []string{
@@ -171,19 +220,19 @@ func (s *InitialiserSuite) TestInstallViaSnap(c *gc.C) {
 
 	paccmder := commands.NewSnapPackageCommander()
 
-	err := NewContainerInitialiser().Initialise()
+	err := NewContainerInitialiser(lxdSnapChannel).Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
 	c.Assert(s.calledCmds, gc.DeepEquals, []string{
-		paccmder.InstallCmd("--classic lxd"),
+		paccmder.InstallCmd("--classic --channel latest/stable lxd"),
 	})
 }
 
 func (s *InitialiserSuite) TestLXDInitBionic(c *gc.C) {
 	s.patchDF100GB()
 	PatchHostSeries(s, "bionic")
 
-	container := NewContainerInitialiser()
+	container := NewContainerInitialiser(lxdSnapChannel)
 	err := container.Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
@@ -194,7 +243,7 @@ func (s *InitialiserSuite) TestLXDInitTrusty(c *gc.C) {
 	s.patchDF100GB()
 	PatchHostSeries(s, "trusty")
 
-	container := NewContainerInitialiser()
+	container := NewContainerInitialiser(lxdSnapChannel)
 	err := container.Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
@@ -209,7 +258,7 @@ func (s *InitialiserSuite) TestLXDAlreadyInitialized(c *gc.C) {
 	s.patchDF100GB()
 	PatchHostSeries(s, "trusty")
 
-	container := NewContainerInitialiser()
+	container := NewContainerInitialiser(lxdSnapChannel)
 	cont, ok := container.(*containerInitialiser)
 	if !ok {
 		c.Fatalf("Unexpected type of container initialized: %T", container)
@@ -278,7 +327,7 @@ func (s *InitialiserSuite) TestInitializeSetsProxies(c *gc.C) {
 		cSvr.EXPECT().UpdateServer(updateReq, lxdtesting.ETag).Return(nil),
 	)
 
-	container := NewContainerInitialiser()
+	container := NewContainerInitialiser(lxdSnapChannel)
 	err := container.Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 }
@@ -575,6 +624,10 @@ func (s *ConfigureInitialiserSuite) TestConfigureLXDBridge(c *gc.C) {
 	cSvr := lxdtesting.NewMockContainerServer(ctrl)
 	s.PatchForProxyUpdate(c, cSvr, true)
 
+	mgr := mocks.NewMockSnapManager(ctrl)
+	mgr.EXPECT().InstalledChannel("lxd").Return("latest/stable")
+	PatchGetSnapManager(s, mgr)
+
 	// The following nic is found, so we don't create a default nic and so
 	// don't update the profile with the nic.
 	profile := &api.Profile{
@@ -605,7 +658,7 @@ func (s *ConfigureInitialiserSuite) TestConfigureLXDBridge(c *gc.C) {
 		cSvr.EXPECT().UpdateServer(gomock.Any(), lxdtesting.ETag).Return(nil),
 	)
 
-	container := NewContainerInitialiser()
+	container := NewContainerInitialiser(lxdSnapChannel)
 	err := container.Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 
@@ -622,6 +675,10 @@ func (s *ConfigureInitialiserSuite) TestConfigureLXDBridgeWithoutNicsCreatesANew
 	cSvr := lxdtesting.NewMockContainerServer(ctrl)
 	s.PatchForProxyUpdate(c, cSvr, true)
 
+	mgr := mocks.NewMockSnapManager(ctrl)
+	mgr.EXPECT().InstalledChannel("lxd").Return("latest/stable")
+	PatchGetSnapManager(s, mgr)
+
 	// If no nics are found in the profile, then the configureLXDBridge will
 	// create a default nic for you.
 	profile := &api.Profile{
@@ -659,7 +716,7 @@ func (s *ConfigureInitialiserSuite) TestConfigureLXDBridgeWithoutNicsCreatesANew
 		cSvr.EXPECT().UpdateServer(gomock.Any(), lxdtesting.ETag).Return(nil),
 	)
 
-	container := NewContainerInitialiser()
+	container := NewContainerInitialiser(lxdSnapChannel)
 	err := container.Initialise()
 	c.Assert(err, jc.ErrorIsNil)
 

diff --git a/container/lxd/mocks/snap_manager_mock.go b/container/lxd/mocks/snap_manager_mock.go