Skip to content

Commit 4c2bfa3

Browse files
authored
Merge pull request juju#11480 from achilleasa/dev-configurable-channel-for-lxd-snap
juju#11480 ## Description of change This PR enables juju operators to specify the snap channel used to install lxd on a **per model** basis via a new model configuration option called `lxd-snap-channel` (defaults to `latest/stable`). The new option comes into play as follows: - When installing lxd as a snap dependency in cosmic+ (IIRC, this happened with eoan/s390 images) - When **setting up container support** on a machine that comes with lxd pre-installed (most of the images that juju uses). The first case is simply handled by passing the configured lxd channel to the dependency resolution system which then injects it into `snap install lxd` commands. The latter case is handled by first introspecting the currently tracked channel and comparing it to the one specified by the operator (or the default). In case of a mismatch, the code will first switch to the selected channel (`snap refresh --channel BLA lxd`) before performing the rest of the snap-related setup steps (e.g. configure proxies etc.) Note that the above channel change will only occur when you attempt to deploy an **lxd workload**. If you simply spin up a new machine via `juju add-machine` it will still be using whatever lxd snap comes pre-installed with it _until_ you actually deploy something to it (see QA steps for an example). Finally, our images seem to use the `track/risk/branch` channel format where branch is tied to the release (e.g. `latest/stable/ubuntu-20.04` on focal). To avoid redundant `snap refresh` calls while still being able to use a sane default for the model option, the code will compare the model setting and the reported tracking channel via a **prefix match** check. Caveats: changes to this model setting only apply to machines with no pre-existing lxd-based workloads. The setting has no effect to existing machines with already deployed lxd workloads; these machines will use whatever lxd snap was initially made available to them. Implementing this would probably require a worker but operators can still use `juju ssh` to run the `snap refresh` commands manually if they need to. ## QA steps ```console $ juju bootstrap lxd test --no-gui $ juju model-config lxd-snap-channel latest/stable $ juju model-config lxd-snap-channel=3.23/edge # Start some machines and check their current lxd version # This should match whatever is *pre-installed* in the image # (note: bionic still uses apt) $ juju add-machine --series=bionic created machine 0 $ juju ssh 0 'sudo which lxd' /usr/bin/lxd $ juju add-machine --series=eoan created machine 1 $ juju ssh 1 'sudo which lxd' /snap/bin/lxd $ juju ssh 1 'snap info lxd | grep tracking:' tracking: latest/stable/ubuntu-19.10 $ juju add-machine --series=focal created machine 2 $ juju ssh 2 'which lxd' /usr/bin/lxd $ juju ssh 2 'snap info lxd | grep tracking:' tracking: latest/stable/ubuntu-20.04 # Deploy an lxd workload on machines 0 (bionic) and 1 (eoan) # and wait for everything to come up. $ juju deploy cs:~jameinel/ubuntu-lite-7 --to lxd:0 $ juju add-unit ubuntu-lite --to lxd:1 # Now check the lxd versions on machines 1(eoan), 2 (focal) # NOTE: it may take a few moments for the snap to update! Be patient ;-) # Notice that the focal machine still uses its built in snap as it currently has no lxd workload $ juju ssh 1 'snap info lxd | grep tracking:' tracking: 3.23/edge $ juju ssh 2 'snap info lxd | grep tracking:' tracking: latest/stable/ubuntu-20.04 # Now change the model config again and deploy an lxd workload to machine 2 (focal) $ juju model-config lxd-snap-channel=4.0/edge $ juju add-unit ubuntu-lite --to lxd:2 # Also deploy an lxd workload to a new machine $ juju model-config default-series=eoan $ juju add-unit ubuntu-lite --to lxd # Finally check all lxd versions again after everything settles ``` ## Documentation changes We need to document the new model config option and the series that it affects (essentially, everything after bionic)
2 parents 7a0e42a + 8d3053f commit 4c2bfa3

File tree

14 files changed

+236
-35
lines changed

14 files changed

+236
-35
lines changed

apiserver/facades/agent/provisioner/provisioner.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -421,15 +421,17 @@ func (api *ProvisionerAPI) ContainerManagerConfig(args params.ContainerManagerCo
421421
cfg := make(map[string]string)
422422
cfg[container.ConfigModelUUID] = api.st.ModelUUID()
423423

424+
mConfig, err := api.m.ModelConfig()
425+
if err != nil {
426+
return result, err
427+
}
428+
424429
switch args.Type {
425430
case instance.LXD:
431+
cfg[config.LXDSnapChannel] = mConfig.LXDSnapChannel()
426432
// TODO(jam): DefaultMTU needs to be handled here
427433
}
428434

429-
mConfig, err := api.m.ModelConfig()
430-
if err != nil {
431-
return result, err
432-
}
433435
if url, set := mConfig.ContainerImageMetadataURL(); set {
434436
cfg[config.ContainerImageMetadataURLKey] = url
435437
}

apiserver/facades/agent/provisioner/provisioner_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1842,6 +1842,7 @@ func (s *withImageMetadataSuite) TestContainerManagerConfigImageMetadata(c *gc.C
18421842
container.ConfigModelUUID: coretesting.ModelTag.Id(),
18431843
config.ContainerImageStreamKey: "daily",
18441844
config.ContainerImageMetadataURLKey: "https://images.linuxcontainers.org/",
1845+
config.LXDSnapChannel: "latest/stable",
18451846
})
18461847
}
18471848

container/lxd/export_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ func PatchHostSeries(patcher patcher, series string) {
4949
patcher.PatchValue(&hostSeries, func() (string, error) { return series, nil })
5050
}
5151

52+
func PatchGetSnapManager(patcher patcher, mgr SnapManager) {
53+
patcher.PatchValue(&getSnapManager, func() SnapManager { return mgr })
54+
}
55+
5256
func GetImageSources(mgr container.Manager) ([]ServerSpec, error) {
5357
return mgr.(*containerManager).getImageSources()
5458
}

container/lxd/initialisation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type containerInitialiser struct {
2020
var _ container.Initialiser = (*containerInitialiser)(nil)
2121

2222
// NewContainerInitialiser - on anything but Linux this is a NOP
23-
func NewContainerInitialiser() container.Initialiser {
23+
func NewContainerInitialiser(string) container.Initialiser {
2424
return &containerInitialiser{}
2525
}
2626

container/lxd/initialisation_linux.go

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"github.com/juju/collections/set"
1919
"github.com/juju/errors"
20+
"github.com/juju/packaging/manager"
2021
"github.com/juju/proxy"
2122
"github.com/juju/utils/series"
2223
"github.com/lxc/lxd/shared"
@@ -34,16 +35,34 @@ var requiredPackages = []string{"lxd"}
3435

3536
type containerInitialiser struct {
3637
getExecCommand func(string, ...string) *exec.Cmd
38+
lxdSnapChannel string
39+
}
40+
41+
//go:generate mockgen -package mocks -destination mocks/snap_manager_mock.go github.com/juju/juju/container/lxd SnapManager
42+
43+
// SnapManager defines an interface implemented by types that can query and/or
44+
// change the channel for installed snaps.
45+
type SnapManager interface {
46+
InstalledChannel(string) string
47+
ChangeChannel(string, string) error
48+
}
49+
50+
// getSnapManager returns a snap manager implementation that is used to query
51+
// and/or change the channel for the installed lxd snap. Defined as a function
52+
// so it can be overridden by tests.
53+
var getSnapManager = func() SnapManager {
54+
return manager.NewSnapPackageManager()
3755
}
3856

3957
// containerInitialiser implements container.Initialiser.
4058
var _ container.Initialiser = (*containerInitialiser)(nil)
4159

4260
// NewContainerInitialiser returns an instance used to perform the steps
4361
// required to allow a host machine to run a LXC container.
44-
func NewContainerInitialiser() container.Initialiser {
62+
func NewContainerInitialiser(lxdSnapChannel string) container.Initialiser {
4563
return &containerInitialiser{
46-
exec.Command,
64+
getExecCommand: exec.Command,
65+
lxdSnapChannel: lxdSnapChannel,
4766
}
4867
}
4968

@@ -54,7 +73,7 @@ func (ci *containerInitialiser) Initialise() error {
5473
return errors.Trace(err)
5574
}
5675

57-
if err := ensureDependencies(localSeries); err != nil {
76+
if err := ensureDependencies(ci.lxdSnapChannel, localSeries); err != nil {
5877
return errors.Trace(err)
5978
}
6079
err = configureLXDBridge()
@@ -246,13 +265,31 @@ func editLXDBridgeFile(input string, subnet string) string {
246265
}
247266

248267
// ensureDependencies install the required dependencies for running LXD.
249-
func ensureDependencies(series string) error {
268+
func ensureDependencies(lxdSnapChannel, series string) error {
269+
// If the snap is already installed, check whether the operator asked
270+
// us to use a different channel. If so, switch to it.
250271
if lxdViaSnap() {
251-
logger.Infof("LXD snap is installed; skipping package installation")
272+
snapManager := getSnapManager()
273+
trackedChannel := snapManager.InstalledChannel("lxd")
274+
// Note that images with pre-installed snaps are normally
275+
// tracking "latest/stable/ubuntu-$release_number". As our
276+
// default model config setting is "latest/stable", we perform
277+
// a starts-with check instead of an equality check to avoid
278+
// switching channels when we don't actually need to.
279+
if strings.HasPrefix(trackedChannel, lxdSnapChannel) {
280+
logger.Infof("LXD snap is already installed (channel: %s); skipping package installation", trackedChannel)
281+
return nil
282+
}
283+
284+
// We need to switch to a different channel
285+
logger.Infof("switching LXD snap channel from %s to %s", trackedChannel, lxdSnapChannel)
286+
if err := snapManager.ChangeChannel("lxd", lxdSnapChannel); err != nil {
287+
return errors.Trace(err)
288+
}
252289
return nil
253290
}
254291

255-
if err := packaging.InstallDependency(dependency.LXD(), series); err != nil {
292+
if err := packaging.InstallDependency(dependency.LXD(lxdSnapChannel), series); err != nil {
256293
return errors.Trace(err)
257294
}
258295

container/lxd/initialisation_test.go

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
gc "gopkg.in/check.v1"
2323

2424
"github.com/golang/mock/gomock"
25+
"github.com/juju/juju/container/lxd/mocks"
2526
lxdtesting "github.com/juju/juju/container/lxd/testing"
2627
coretesting "github.com/juju/juju/testing"
2728
lxd "github.com/lxc/lxd/client"
@@ -91,6 +92,8 @@ LXD_IPV6_NAT="true"
9192
LXD_IPV6_PROXY="true"
9293
`
9394

95+
const lxdSnapChannel = "latest/stable"
96+
9497
func (s *InitialiserSuite) SetUpTest(c *gc.C) {
9598
s.initialiserTestSuite.SetUpTest(c)
9699
s.calledCmds = []string{}
@@ -127,7 +130,7 @@ func (s *InitialiserSuite) TestLTSSeriesPackages(c *gc.C) {
127130
PatchLXDViaSnap(s, false)
128131
PatchHostSeries(s, "trusty")
129132

130-
err = NewContainerInitialiser().Initialise()
133+
err = NewContainerInitialiser(lxdSnapChannel).Initialise()
131134
c.Assert(err, jc.ErrorIsNil)
132135

133136
c.Assert(s.calledCmds, gc.DeepEquals, []string{
@@ -139,12 +142,58 @@ func (s *InitialiserSuite) TestSnapInstalledNoAptInstall(c *gc.C) {
139142
PatchLXDViaSnap(s, true)
140143
PatchHostSeries(s, "cosmic")
141144

142-
err := NewContainerInitialiser().Initialise()
145+
ctrl := gomock.NewController(c)
146+
defer ctrl.Finish()
147+
148+
mgr := mocks.NewMockSnapManager(ctrl)
149+
mgr.EXPECT().InstalledChannel("lxd").Return("latest/stable")
150+
PatchGetSnapManager(s, mgr)
151+
152+
err := NewContainerInitialiser(lxdSnapChannel).Initialise()
143153
c.Assert(err, jc.ErrorIsNil)
144154

145155
c.Assert(s.calledCmds, gc.DeepEquals, []string{})
146156
}
147157

158+
func (s *InitialiserSuite) TestSnapChannelMismatch(c *gc.C) {
159+
PatchLXDViaSnap(s, true)
160+
PatchHostSeries(s, "focal")
161+
162+
ctrl := gomock.NewController(c)
163+
defer ctrl.Finish()
164+
165+
mgr := mocks.NewMockSnapManager(ctrl)
166+
gomock.InOrder(
167+
mgr.EXPECT().InstalledChannel("lxd").Return("3.2/stable"),
168+
mgr.EXPECT().ChangeChannel("lxd", lxdSnapChannel),
169+
)
170+
PatchGetSnapManager(s, mgr)
171+
172+
err := NewContainerInitialiser(lxdSnapChannel).Initialise()
173+
c.Assert(err, jc.ErrorIsNil)
174+
}
175+
176+
func (s *InitialiserSuite) TestSnapChannelPrefixMatch(c *gc.C) {
177+
PatchLXDViaSnap(s, true)
178+
PatchHostSeries(s, "focal")
179+
180+
ctrl := gomock.NewController(c)
181+
defer ctrl.Finish()
182+
183+
mgr := mocks.NewMockSnapManager(ctrl)
184+
gomock.InOrder(
185+
// The channel for the installed lxd snap also includes the
186+
// branch for the focal release. The "track/risk" prefix is
187+
// the same however so the container manager should not attempt
188+
// to change the channel.
189+
mgr.EXPECT().InstalledChannel("lxd").Return("latest/stable/ubuntu-20.04"),
190+
)
191+
PatchGetSnapManager(s, mgr)
192+
193+
err := NewContainerInitialiser(lxdSnapChannel).Initialise()
194+
c.Assert(err, jc.ErrorIsNil)
195+
}
196+
148197
func (s *InitialiserSuite) TestNoSeriesPackages(c *gc.C) {
149198
PatchLXDViaSnap(s, false)
150199

@@ -156,7 +205,7 @@ func (s *InitialiserSuite) TestNoSeriesPackages(c *gc.C) {
156205
paccmder, err := commands.NewPackageCommander("xenial")
157206
c.Assert(err, jc.ErrorIsNil)
158207

159-
err = NewContainerInitialiser().Initialise()
208+
err = NewContainerInitialiser(lxdSnapChannel).Initialise()
160209
c.Assert(err, jc.ErrorIsNil)
161210

162211
c.Assert(s.calledCmds, gc.DeepEquals, []string{
@@ -171,19 +220,19 @@ func (s *InitialiserSuite) TestInstallViaSnap(c *gc.C) {
171220

172221
paccmder := commands.NewSnapPackageCommander()
173222

174-
err := NewContainerInitialiser().Initialise()
223+
err := NewContainerInitialiser(lxdSnapChannel).Initialise()
175224
c.Assert(err, jc.ErrorIsNil)
176225

177226
c.Assert(s.calledCmds, gc.DeepEquals, []string{
178-
paccmder.InstallCmd("--classic lxd"),
227+
paccmder.InstallCmd("--classic --channel latest/stable lxd"),
179228
})
180229
}
181230

182231
func (s *InitialiserSuite) TestLXDInitBionic(c *gc.C) {
183232
s.patchDF100GB()
184233
PatchHostSeries(s, "bionic")
185234

186-
container := NewContainerInitialiser()
235+
container := NewContainerInitialiser(lxdSnapChannel)
187236
err := container.Initialise()
188237
c.Assert(err, jc.ErrorIsNil)
189238

@@ -194,7 +243,7 @@ func (s *InitialiserSuite) TestLXDInitTrusty(c *gc.C) {
194243
s.patchDF100GB()
195244
PatchHostSeries(s, "trusty")
196245

197-
container := NewContainerInitialiser()
246+
container := NewContainerInitialiser(lxdSnapChannel)
198247
err := container.Initialise()
199248
c.Assert(err, jc.ErrorIsNil)
200249

@@ -209,7 +258,7 @@ func (s *InitialiserSuite) TestLXDAlreadyInitialized(c *gc.C) {
209258
s.patchDF100GB()
210259
PatchHostSeries(s, "trusty")
211260

212-
container := NewContainerInitialiser()
261+
container := NewContainerInitialiser(lxdSnapChannel)
213262
cont, ok := container.(*containerInitialiser)
214263
if !ok {
215264
c.Fatalf("Unexpected type of container initialized: %T", container)
@@ -278,7 +327,7 @@ func (s *InitialiserSuite) TestInitializeSetsProxies(c *gc.C) {
278327
cSvr.EXPECT().UpdateServer(updateReq, lxdtesting.ETag).Return(nil),
279328
)
280329

281-
container := NewContainerInitialiser()
330+
container := NewContainerInitialiser(lxdSnapChannel)
282331
err := container.Initialise()
283332
c.Assert(err, jc.ErrorIsNil)
284333
}
@@ -575,6 +624,10 @@ func (s *ConfigureInitialiserSuite) TestConfigureLXDBridge(c *gc.C) {
575624
cSvr := lxdtesting.NewMockContainerServer(ctrl)
576625
s.PatchForProxyUpdate(c, cSvr, true)
577626

627+
mgr := mocks.NewMockSnapManager(ctrl)
628+
mgr.EXPECT().InstalledChannel("lxd").Return("latest/stable")
629+
PatchGetSnapManager(s, mgr)
630+
578631
// The following nic is found, so we don't create a default nic and so
579632
// don't update the profile with the nic.
580633
profile := &api.Profile{
@@ -605,7 +658,7 @@ func (s *ConfigureInitialiserSuite) TestConfigureLXDBridge(c *gc.C) {
605658
cSvr.EXPECT().UpdateServer(gomock.Any(), lxdtesting.ETag).Return(nil),
606659
)
607660

608-
container := NewContainerInitialiser()
661+
container := NewContainerInitialiser(lxdSnapChannel)
609662
err := container.Initialise()
610663
c.Assert(err, jc.ErrorIsNil)
611664

@@ -622,6 +675,10 @@ func (s *ConfigureInitialiserSuite) TestConfigureLXDBridgeWithoutNicsCreatesANew
622675
cSvr := lxdtesting.NewMockContainerServer(ctrl)
623676
s.PatchForProxyUpdate(c, cSvr, true)
624677

678+
mgr := mocks.NewMockSnapManager(ctrl)
679+
mgr.EXPECT().InstalledChannel("lxd").Return("latest/stable")
680+
PatchGetSnapManager(s, mgr)
681+
625682
// If no nics are found in the profile, then the configureLXDBridge will
626683
// create a default nic for you.
627684
profile := &api.Profile{
@@ -659,7 +716,7 @@ func (s *ConfigureInitialiserSuite) TestConfigureLXDBridgeWithoutNicsCreatesANew
659716
cSvr.EXPECT().UpdateServer(gomock.Any(), lxdtesting.ETag).Return(nil),
660717
)
661718

662-
container := NewContainerInitialiser()
719+
container := NewContainerInitialiser(lxdSnapChannel)
663720
err := container.Initialise()
664721
c.Assert(err, jc.ErrorIsNil)
665722

container/lxd/mocks/snap_manager_mock.go

Lines changed: 61 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)