many: allow systems installed via installer API to pick optional snap…

…s and components (#14426)

* daemon, o/devicestate: rename devicestate.OptionalInstall to better represent uses devicestate.OptionalContainers

* o/devicestate: add available optional snap and components to the System struct

* daemon: report available optional containers for a system available for install

* o/devicestate, s/seedtest: handle optional-install field on install-finish task

* tests: test seeding optional snaps and components from snapd installer api

* tests: use flags for arguments passed to the muinstaller

* tests: use encrypted installation for cases with optional snaps and components

* tests: make test work for core24 too

daemon/api_systems.go

@@ -171,7 +171,11 @@ func getSystemDetails(c *Command, r *http.Request, user *auth.UserState) Respons
 			Validation:  sys.Brand.Validation(),
 		},
 		// no body: we expect models to have empty bodies
-		Model:             sys.Model.Headers(),
+		Model: sys.Model.Headers(),
+		AvailableOptional: client.AvailableForInstall{
+			Snaps:      sys.OptionalContainers.Snaps,
+			Components: sys.OptionalContainers.Components,
+		},
 		Volumes:           gadgetInfo.Volumes,
 		StorageEncryption: storageEncryption(encryptionInfo),
 	}
@@ -325,7 +329,7 @@ func postSystemActionInstall(c *Command, systemLabel string, req *systemActionRe
 		ensureStateSoon(st)
 		return AsyncResponse(nil, chg.ID())
 	case client.InstallStepFinish:
-		var optional *devicestate.OptionalInstall
+		var optional *devicestate.OptionalContainers
 		if req.OptionalInstall != nil {
 			// note that we provide a nil optional install here in the case that
 			// the request set the All field to true. the nil optional install
@@ -336,7 +340,7 @@ func postSystemActionInstall(c *Command, systemLabel string, req *systemActionRe
 					return BadRequest("cannot specify both all and individual optional snaps and components to install")
 				}
 			} else {
-				optional = &devicestate.OptionalInstall{
+				optional = &devicestate.OptionalContainers{
 					Snaps:      req.OptionalInstall.Snaps,
 					Components: req.OptionalInstall.Components,
 				}

daemon/api_systems_test.go

@@ -864,6 +864,10 @@ func (s *systemsSuite) TestSystemsGetSystemDetailsForLabel(c *check.C) {
 				Model: s.seedModelForLabel20191119,
 				Label: "20191119",
 				Brand: s.Brands.Account("my-brand"),
+				OptionalContainers: devicestate.OptionalContainers{
+					Snaps:      []string{"snap1", "snap2"},
+					Components: map[string][]string{"snap1": {"comp1"}, "snap2": {"comp2"}},
+				},
 			}
 			return sys, mockGadgetInfo, mockEncryptionSupportInfo, nil
 		})
@@ -890,6 +894,13 @@ func (s *systemsSuite) TestSystemsGetSystemDetailsForLabel(c *check.C) {
 				UnavailableReason: tc.expectedUnavailableReason,
 			},
 			Volumes: mockGadgetInfo.Volumes,
+			AvailableOptional: client.AvailableForInstall{
+				Snaps: []string{"snap1", "snap2"},
+				Components: map[string][]string{
+					"snap1": {"comp1"},
+					"snap2": {"comp2"},
+				},
+			},
 		}, check.Commentf("%v", tc))
 	}
 }
@@ -1112,8 +1123,8 @@ func (s *systemsSuite) testSystemInstallActionFinishCallsDevicestate(c *check.C,
 	nCalls := 0
 	var gotOnVolumes map[string]*gadget.Volume
 	var gotLabel string
-	var gotOptionalInstall *devicestate.OptionalInstall
-	r := daemon.MockDevicestateInstallFinish(func(st *state.State, label string, onVolumes map[string]*gadget.Volume, optionalInstall *devicestate.OptionalInstall) (*state.Change, error) {
+	var gotOptionalInstall *devicestate.OptionalContainers
+	r := daemon.MockDevicestateInstallFinish(func(st *state.State, label string, onVolumes map[string]*gadget.Volume, optionalInstall *devicestate.OptionalContainers) (*state.Change, error) {
 		gotLabel = label
 		gotOnVolumes = onVolumes
 		gotOptionalInstall = optionalInstall
@@ -1160,7 +1171,7 @@ func (s *systemsSuite) testSystemInstallActionFinishCallsDevicestate(c *check.C,
 	if optionalInstall.All {
 		c.Check(gotOptionalInstall, check.IsNil)
 	} else {
-		c.Check(gotOptionalInstall, check.DeepEquals, &devicestate.OptionalInstall{
+		c.Check(gotOptionalInstall, check.DeepEquals, &devicestate.OptionalContainers{
 			Snaps:      optionalInstall.Snaps,
 			Components: optionalInstall.Components,
 		})

daemon/export_api_systems_test.go

@@ -45,7 +45,7 @@ func MockDeviceManagerSystemAndGadgetAndEncryptionInfo(f func(*devicestate.Devic
 	return restore
 }
 
-func MockDevicestateInstallFinish(f func(*state.State, string, map[string]*gadget.Volume, *devicestate.OptionalInstall) (*state.Change, error)) (restore func()) {
+func MockDevicestateInstallFinish(f func(*state.State, string, map[string]*gadget.Volume, *devicestate.OptionalContainers) (*state.Change, error)) (restore func()) {
 	restore = testutil.Backup(&devicestateInstallFinish)
 	devicestateInstallFinish = f
 	return restore

overlord/devicestate/devicemgr.go

@@ -2104,6 +2104,10 @@ type System struct {
 	// DefaultRecoverySystem is true when the system is the default recovery
 	// system.
 	DefaultRecoverySystem bool
+	// OptionalContainers is a set of snaps and components that are optional in
+	// the system's model, but are available to be installed when installing this
+	// system.
+	OptionalContainers OptionalContainers
 }
 
 var defaultSystemActions = []SystemAction{

overlord/devicestate/devicestate.go

@@ -1906,20 +1906,20 @@ func extractSnapSetupTaskIDs(tss []*state.TaskSet) ([]string, error) {
 	return taskIDs, nil
 }
 
-// OptionalInstall is used to define the snaps and components that are optional
-// in a system's model, but should be installed when installing the system.
-type OptionalInstall struct {
-	// Snaps is a list of optional snap names that should be installed.
+// OptionalContainers is used to define the snaps and components that are
+// optional in a system's model, but can be installed when installing a system.
+type OptionalContainers struct {
+	// Snaps is a list of optional snap names that can be installed.
 	Snaps []string `json:"snaps,omitempty"`
 	// Components is a mapping of snap names to lists of optional components
-	// names that should be installed.
+	// names that can be installed.
 	Components map[string][]string `json:"components,omitempty"`
 }
 
 // InstallFinish creates a change that will finish the install for the given
 // label and volumes. This includes writing missing volume content, seting
 // up the bootloader and installing the kernel.
-func InstallFinish(st *state.State, label string, onVolumes map[string]*gadget.Volume, optionalInstall *OptionalInstall) (*state.Change, error) {
+func InstallFinish(st *state.State, label string, onVolumes map[string]*gadget.Volume, optionalContainers *OptionalContainers) (*state.Change, error) {
 	if label == "" {
 		return nil, fmt.Errorf("cannot finish install with an empty system label")
 	}
@@ -1931,8 +1931,8 @@ func InstallFinish(st *state.State, label string, onVolumes map[string]*gadget.V
 	finishTask := st.NewTask("install-finish", fmt.Sprintf("Finish setup of run system for %q", label))
 	finishTask.Set("system-label", label)
 	finishTask.Set("on-volumes", onVolumes)
-	if optionalInstall != nil {
-		finishTask.Set("optional-install", *optionalInstall)
+	if optionalContainers != nil {
+		finishTask.Set("optional-install", *optionalContainers)
 	}
 	chg.AddTask(finishTask)
 

overlord/devicestate/devicestate_install_api_test.go

@@ -21,7 +21,6 @@
 package devicestate_test
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -48,6 +47,7 @@ import (
 	"github.com/snapcore/snapd/secboot/keys"
 	"github.com/snapcore/snapd/seed"
 	"github.com/snapcore/snapd/seed/seedtest"
+	"github.com/snapcore/snapd/seed/seedwriter"
 	"github.com/snapcore/snapd/snap"
 	"github.com/snapcore/snapd/testutil"
 	"github.com/snapcore/snapd/timings"
@@ -119,6 +119,8 @@ func (s *deviceMgrInstallAPISuite) setupSystemSeed(c *C, sysLabel, gadgetYaml st
 			{"shim.efi.signed", ""}, {"grub.conf", ""}},
 		snap.R(1), "my-brand", s.StoreSigning.Database)
 
+	s.MakeAssertedSnapWithComps(c, seedtest.SampleSnapYaml["optional22"], nil, snap.R(1), nil, "my-brand", s.StoreSigning.Database)
+
 	model := map[string]interface{}{
 		"display-name": "my model",
 		"architecture": "amd64",
@@ -147,20 +149,33 @@ func (s *deviceMgrInstallAPISuite) setupSystemSeed(c *C, sysLabel, gadgetYaml st
 				"id":   s.AssertedSnapID("core22"),
 				"type": "base",
 			},
+			map[string]interface{}{
+				"name": "optional22",
+				"id":   s.AssertedSnapID("optional22"),
+				"components": map[string]interface{}{
+					"comp1": "optional",
+				},
+			},
 		},
 	}
 	if isClassic {
 		model["classic"] = "true"
 		model["distribution"] = "ubuntu"
 	}
 
-	return s.MakeSeed(c, sysLabel, "my-brand", "my-model", model, nil)
+	return s.MakeSeed(c, sysLabel, "my-brand", "my-model", model, []*seedwriter.OptionsSnap{
+		{
+			Name:       "optional22",
+			Components: []seedwriter.OptionsComponent{{Name: "comp1"}},
+		},
+	})
 }
 
 type finishStepOpts struct {
-	encrypted      bool
-	installClassic bool
-	hasPartial     bool
+	encrypted          bool
+	installClassic     bool
+	hasPartial         bool
+	optionalContainers *seed.OptionalContainers
 }
 
 func (s *deviceMgrInstallAPISuite) mockSystemSeedWithLabel(c *C, label string, isClassic, hasPartial bool, seedCopyFn func(string, seed.CopyOptions, timings.Measurer) error) (gadgetSnapPath, kernelSnapPath string, ginfo *gadget.Info, mountCmd *testutil.MockCmd) {
@@ -195,6 +210,10 @@ func (s *deviceMgrInstallAPISuite) mockSystemSeedWithLabel(c *C, label string, i
 	restore = devicestate.MockSeedOpen(func(seedDir, label string) (seed.Seed, error) {
 		return &fakeSeedCopier{
 			copyFn: seedCopyFn,
+			optionalContainers: seed.OptionalContainers{
+				Snaps:      []string{"optional22"},
+				Components: map[string][]string{"optional22": {"comp1"}},
+			},
 			fakeSeed: fakeSeed{
 				essentialSnaps: []*seed.Snap{
 					{
@@ -397,15 +416,16 @@ var mockFilledPartialDiskVolume = gadget.OnDiskVolume{
 
 type fakeSeedCopier struct {
 	fakeSeed
-	copyFn func(seedDir string, opts seed.CopyOptions, tm timings.Measurer) error
+	optionalContainers seed.OptionalContainers
+	copyFn             func(seedDir string, opts seed.CopyOptions, tm timings.Measurer) error
 }
 
 func (s *fakeSeedCopier) Copy(seedDir string, opts seed.CopyOptions, tm timings.Measurer) error {
 	return s.copyFn(seedDir, opts, tm)
 }
 
 func (s *fakeSeedCopier) OptionalContainers() (seed.OptionalContainers, error) {
-	return seed.OptionalContainers{}, errors.New("not implemented")
+	return s.optionalContainers, nil
 }
 
 // TODO encryption case for the finish step is not tested yet, it needs more mocking
@@ -430,9 +450,10 @@ func (s *deviceMgrInstallAPISuite) testInstallFinishStep(c *C, opts finishStepOp
 	}
 	seedCopyCalled := false
 	if !opts.installClassic {
-		seedCopyFn = func(seedDir string, opts seed.CopyOptions, tm timings.Measurer) error {
+		seedCopyFn = func(seedDir string, copyOpts seed.CopyOptions, tm timings.Measurer) error {
 			c.Check(seedDir, Equals, filepath.Join(dirs.RunDir, "mnt/ubuntu-seed"))
-			c.Check(opts.Label, Equals, label)
+			c.Check(copyOpts.Label, Equals, label)
+			c.Check(copyOpts.OptionalContainers, DeepEquals, opts.optionalContainers)
 			seedCopyCalled = true
 			return nil
 		}
@@ -587,6 +608,9 @@ func (s *deviceMgrInstallAPISuite) testInstallFinishStep(c *C, opts finishStepOp
 		}
 	}
 	finishTask.Set("on-volumes", ginfo.Volumes)
+	if opts.optionalContainers != nil {
+		finishTask.Set("optional-install", *opts.optionalContainers)
+	}
 
 	chg.AddTask(finishTask)
 
@@ -663,6 +687,17 @@ func (s *deviceMgrInstallAPISuite) TestInstallCoreFinishEncryptionHappy(c *C) {
 	s.testInstallFinishStep(c, finishStepOpts{encrypted: true, installClassic: false})
 }
 
+func (s *deviceMgrInstallAPISuite) TestInstallCoreFinishWithOptionalContainers(c *C) {
+	s.testInstallFinishStep(c, finishStepOpts{
+		encrypted:      true,
+		installClassic: false,
+		optionalContainers: &seed.OptionalContainers{
+			Snaps:      []string{"optional22"},
+			Components: map[string][]string{"optional22": {"comp1"}},
+		},
+	})
+}
+
 func (s *deviceMgrInstallAPISuite) TestInstallFinishNoLabel(c *C) {
 	// Mock partitioned disk, but there will be no label in the system
 	gadgetYaml := gadgettest.SingleVolumeClassicWithModesGadgetYaml

overlord/devicestate/devicestate_install_mode_test.go

@@ -2474,7 +2474,7 @@ func (s *installStepSuite) TestDeviceManagerInstallFinishNoVolumesError(c *C) {
 }
 
 func (s *installStepSuite) TestDeviceManagerInstallFinishTasksAndChange(c *C) {
-	s.testDeviceManagerInstallFinishTasksAndChange(c, &devicestate.OptionalInstall{
+	s.testDeviceManagerInstallFinishTasksAndChange(c, &devicestate.OptionalContainers{
 		Snaps: []string{"snap1", "snap2"},
 		Components: map[string][]string{
 			"snap1": {"component1"},
@@ -2487,7 +2487,7 @@ func (s *installStepSuite) TestDeviceManagerInstallFinishTasksAndChangeNilOption
 	s.testDeviceManagerInstallFinishTasksAndChange(c, nil)
 }
 
-func (s *installStepSuite) testDeviceManagerInstallFinishTasksAndChange(c *C, optionalInstall *devicestate.OptionalInstall) {
+func (s *installStepSuite) testDeviceManagerInstallFinishTasksAndChange(c *C, optionalInstall *devicestate.OptionalContainers) {
 	s.state.Lock()
 	defer s.state.Unlock()
 
@@ -2510,7 +2510,7 @@ func (s *installStepSuite) testDeviceManagerInstallFinishTasksAndChange(c *C, op
 	c.Assert(err, IsNil)
 	c.Assert(onVols, DeepEquals, mockOnVolumes)
 
-	var gotOptionalInstall devicestate.OptionalInstall
+	var gotOptionalInstall devicestate.OptionalContainers
 	err = tskInstallFinish.Get("optional-install", &gotOptionalInstall)
 	if optionalInstall == nil {
 		c.Assert(err, testutil.ErrorIs, &state.NoStateError{Key: "optional-install"})
@@ -2526,7 +2526,7 @@ func (s *installStepSuite) TestDeviceManagerInstallFinishRunthrough(c *C) {
 	defer st.Unlock()
 
 	s.state.Set("seeded", true)
-	chg, err := devicestate.InstallFinish(s.state, "1234", mockOnVolumes, &devicestate.OptionalInstall{})
+	chg, err := devicestate.InstallFinish(s.state, "1234", mockOnVolumes, &devicestate.OptionalContainers{})
 	c.Assert(err, IsNil)
 
 	st.Unlock()

overlord/devicestate/devicestate_systems_test.go

@@ -56,6 +56,7 @@ import (
 	"github.com/snapcore/snapd/secboot"
 	"github.com/snapcore/snapd/seed"
 	"github.com/snapcore/snapd/seed/seedtest"
+	"github.com/snapcore/snapd/seed/seedwriter"
 	"github.com/snapcore/snapd/snap"
 	"github.com/snapcore/snapd/snap/naming"
 	"github.com/snapcore/snapd/snap/snaptest"
@@ -2779,6 +2780,7 @@ func (s *modelAndGadgetInfoSuite) makeMockUC20SeedWithGadgetYaml(c *C, label, ga
 	seed20.MakeAssertedSnap(c, "name: snapd\nversion: 1\ntype: snapd", nil, snap.R(1), "my-brand", s.storeSigning.Database)
 	seed20.MakeAssertedSnap(c, "name: pc-kernel\nversion: 1\ntype: kernel", nil, snap.R(1), "my-brand", s.storeSigning.Database)
 	seed20.MakeAssertedSnap(c, "name: core20\nversion: 1\ntype: base", nil, snap.R(1), "my-brand", s.storeSigning.Database)
+	seed20.MakeAssertedSnap(c, "name: optional-snap\nversion: 1\ntype: app\nbase: core20", nil, snap.R(1), "my-brand", s.storeSigning.Database)
 	gadgetFiles := [][]string{
 		{"meta/gadget.yaml", string(gadgetYaml)},
 	}
@@ -2800,13 +2802,23 @@ func (s *modelAndGadgetInfoSuite) makeMockUC20SeedWithGadgetYaml(c *C, label, ga
 				"id":              seed20.AssertedSnapID("pc"),
 				"type":            "gadget",
 				"default-channel": "20",
+			},
+			map[string]interface{}{
+				"name":            "optional-snap",
+				"presence":        "optional",
+				"id":              seed20.AssertedSnapID("optional-snap"),
+				"default-channel": "20",
 			}},
 	}
 	if isClassic {
 		headers["classic"] = "true"
 		headers["distribution"] = "ubuntu"
 	}
-	return seed20.MakeSeed(c, label, "my-brand", "my-model", headers, nil)
+	return seed20.MakeSeed(c, label, "my-brand", "my-model", headers, []*seedwriter.OptionsSnap{
+		{
+			Name: "optional-snap",
+		},
+	})
 }
 
 func (s *modelAndGadgetInfoSuite) TestSystemAndGadgetAndEncyptionInfoHappy(c *C) {
@@ -2825,6 +2837,9 @@ func (s *modelAndGadgetInfoSuite) TestSystemAndGadgetAndEncyptionInfoHappy(c *C)
 		Model:   fakeModel,
 		Brand:   s.brands.Account("my-brand"),
 		Actions: defaultSystemActions,
+		OptionalContainers: devicestate.OptionalContainers{
+			Snaps: []string{"optional-snap"},
+		},
 	})
 	c.Check(gadgetInfo.Volumes, DeepEquals, expectedGadgetInfo.Volumes)
 	c.Check(encInfo, DeepEquals, &install.EncryptionSupportInfo{

overlord/devicestate/handlers_install.go

@@ -949,8 +949,6 @@ func (m *DeviceManager) doInstallFinish(t *state.Task, _ *tomb.Tomb) error {
 		return err
 	}
 
-	// TODO:COMPS: handle the "optional-install" field on the task
-
 	var encryptSetupData *install.EncryptionSetupData
 	cached := st.Cached(encryptionSetupDataKey{systemLabel})
 	if cached != nil {
@@ -1068,9 +1066,22 @@ func (m *DeviceManager) doInstallFinish(t *state.Task, _ *tomb.Tomb) error {
 			return fmt.Errorf("internal error: seed does not support copying: %s", systemAndSnaps.Label)
 		}
 
+		var optional *seed.OptionalContainers
+		if t.Has("optional-install") {
+			var oc OptionalContainers
+			if err := t.Get("optional-install", &oc); err != nil {
+				return err
+			}
+			optional = &seed.OptionalContainers{
+				Snaps:      oc.Snaps,
+				Components: oc.Components,
+			}
+		}
+
 		logger.Debugf("copying label %q to seed partition", systemAndSnaps.Label)
 		if err := copier.Copy(seedMntDir, seed.CopyOptions{
-			Label: systemAndSnaps.Label,
+			Label:              systemAndSnaps.Label,
+			OptionalContainers: optional,
 		}, perfTimings); err != nil {
 			return fmt.Errorf("cannot copy seed: %w", err)
 		}