Do not reload the deamon or restart snapd services when preseeding on

core. 

3 files changed, 219 insertions, 79 deletions

diff --git a/overlord/snapstate/backend/link.go b/overlord/snapstate/backend/link.go
index fe2ff0b37a..1d3acfb74f 100644
--- a/overlord/snapstate/backend/link.go
+++ b/overlord/snapstate/backend/link.go
@@ -275,7 +275,7 @@ func removeGeneratedWrappers(s *snap.Info, firstInstallUndo bool, meter progress
 
 func GenerateSnapdWrappers(s *snap.Info) error {
	// snapd services are handled separately via an explicit helper
-	return wrappers.AddSnapdSnapServices(s, progress.Null)
+	return wrappers.AddSnapdSnapServices(s, nil, progress.Null)
 }
 
 func removeGeneratedSnapdWrappers(s *snap.Info, firstInstall bool, meter progress.Meter) error {
diff --git a/wrappers/core18.go b/wrappers/core18.go
index 92b22c91f5..cab5791208 100644
--- a/wrappers/core18.go
+++ b/wrappers/core18.go
@@ -64,7 +64,7 @@ func snapdUnitSkipStart(unitPath string) (skip bool, err error) {
	return snapdSkipStart(content), nil
 }
 
-func writeSnapdToolingMountUnit(sysd systemd.Systemd, prefix string) error {
+func writeSnapdToolingMountUnit(sysd systemd.Systemd, prefix string, opts *AddSnapdSnapServicesOptions) error {
 
	// TODO: the following comment is wrong, we don't need RequiredBy=snapd here?
 
@@ -98,19 +98,24 @@ WantedBy=snapd.service
	if err != nil {
	return err
	}
-	if err := sysd.DaemonReload(); err != nil {
-	return err
+
+	if !opts.Preseeding {
+	if err := sysd.DaemonReload(); err != nil {
+	return err
+	}
	}
	units := []string{SnapdToolingMountUnit}
	if err := sysd.Enable(units); err != nil {
	return err
	}
 
-	// meh this is killing snap services that use Requires=<this-unit> because
-	// it doesn't use verbatim systemctl restart, it instead does it with
-	// a systemctl stop and then a systemctl start, which triggers LP #1924805
-	if err := sysd.Restart(units, 5*time.Second); err != nil {
-	return err
+	if !opts.Preseeding {
+	// meh this is killing snap services that use Requires=<this-unit> because
+	// it doesn't use verbatim systemctl restart, it instead does it with
+	// a systemctl stop and then a systemctl start, which triggers LP #1924805
+	if err := sysd.Restart(units, 5*time.Second); err != nil {
+	return err
+	}
	}
 
	return nil
@@ -136,9 +141,16 @@ func undoSnapdToolingMountUnit(sysd systemd.Systemd) error {
	return os.Remove(mountUnitPath)
 }
 
+type AddSnapdSnapServicesOptions struct {
+	// Preseeding is whether the system is currently being preseeded, in which
+	// case there is not a running systemd for EnsureSnapServicesOptions to
+	// issue commands like systemctl daemon-reload to.
+	Preseeding bool
+}
+
 // AddSnapdSnapServices sets up the services based on a given snapd snap in the
 // system.
-func AddSnapdSnapServices(s *snap.Info, inter interacter) error {
+func AddSnapdSnapServices(s *snap.Info, opts *AddSnapdSnapServicesOptions, inter interacter) error {
	if snapType := s.Type(); snapType != snap.TypeSnapd {
	return fmt.Errorf("internal error: adding explicit snapd services for snap %q type %q is unexpected", s.InstanceName(), snapType)
	}
@@ -148,9 +160,18 @@ func AddSnapdSnapServices(s *snap.Info, inter interacter) error {
	return nil
	}
 
-	sysd := systemd.New(systemd.SystemMode, inter)
+	if opts == nil {
+	opts = &AddSnapdSnapServicesOptions{}
+	}
 
-	if err := writeSnapdToolingMountUnit(sysd, s.MountDir()); err != nil {
+	var sysd systemd.Systemd
+	if !opts.Preseeding {
+	sysd = systemd.New(systemd.SystemMode, inter)
+	} else {
+	sysd = systemd.NewEmulationMode("")
+	}
+
+	if err := writeSnapdToolingMountUnit(sysd, s.MountDir(), opts); err != nil {
	return err
	}
 
@@ -202,19 +223,22 @@ func AddSnapdSnapServices(s *snap.Info, inter interacter) error {
	// nothing to do
	return nil
	}
-	// stop all removed units first
-	for _, unit := range removed {
-	serviceUnits := []string{unit}
-	if err := sysd.Stop(serviceUnits, 5*time.Second); err != nil {
-	logger.Noticef("failed to stop %q: %v", unit, err)
-	}
-	if err := sysd.Disable(serviceUnits); err != nil {
-	logger.Noticef("failed to disable %q: %v", unit, err)
+
+	if !opts.Preseeding {
+	// stop all removed units first
+	for _, unit := range removed {
+	serviceUnits := []string{unit}
+	if err := sysd.Stop(serviceUnits, 5*time.Second); err != nil {
+	logger.Noticef("failed to stop %q: %v", unit, err)
+	}
+	if err := sysd.Disable(serviceUnits); err != nil {
+	logger.Noticef("failed to disable %q: %v", unit, err)
+	}
	}
	}
 
	// daemon-reload so that we get the new services
-	if len(changed) > 0 {
+	if len(changed) > 0 && !opts.Preseeding {
	if err := sysd.DaemonReload(); err != nil {
	return err
	}
@@ -231,71 +255,75 @@ func AddSnapdSnapServices(s *snap.Info, inter interacter) error {
	// systemd version, where older versions (eg 229 in 16.04) would
	// error out unless --force is passed, while new ones remove the
	// symlink and create a new one.
-	enabled, err := sysd.IsEnabled(unit)
-	if err != nil {
-	return err
-	}
-	if enabled {
-	continue
+	if !opts.Preseeding {
+	enabled, err := sysd.IsEnabled(unit)
+	if err != nil {
+	return err
+	}
+	if enabled {
+	continue
+	}
	}
	if err := sysd.Enable([]string{unit}); err != nil {
	return err
	}
	}
 
-	for _, unit := range changed {
-	// Some units (like the snapd.system-shutdown.service) cannot
-	// be started. Others like "snapd.seeded.service" are started
-	// as dependencies of snapd.service.
-	if snapdSkipStart(snapdUnits[unit].(*osutil.MemoryFileState).Content) {
-	continue
-	}
-	// Ensure to only restart if the unit was previously
-	// active. This ensures we DTRT on firstboot and do
-	// not stop e.g. snapd.socket because doing that
-	// would mean that the snapd.seeded.service is also
-	// stopped (independently of snapd.socket being
-	// active) which confuses the boot order (the unit
-	// exists before we are fully seeded).
-	isActive, err := sysd.IsActive(unit)
-	if err != nil {
-	return err
-	}
-	serviceUnits := []string{unit}
-	if isActive {
-	// we can never restart the snapd.socket because
-	// this will also bring down snapd itself
-	if unit != "snapd.socket" {
-	if err := sysd.Restart(serviceUnits, 5*time.Second); err != nil {
-	return err
-	}
+	if !opts.Preseeding {
+	for _, unit := range changed {
+	// Some units (like the snapd.system-shutdown.service) cannot
+	// be started. Others like "snapd.seeded.service" are started
+	// as dependencies of snapd.service.
+	if snapdSkipStart(snapdUnits[unit].(*osutil.MemoryFileState).Content) {
+	continue
	}
-	} else {
-	if err := sysd.Start(serviceUnits); err != nil {
+	// Ensure to only restart if the unit was previously
+	// active. This ensures we DTRT on firstboot and do
+	// not stop e.g. snapd.socket because doing that
+	// would mean that the snapd.seeded.service is also
+	// stopped (independently of snapd.socket being
+	// active) which confuses the boot order (the unit
+	// exists before we are fully seeded).
+	isActive, err := sysd.IsActive(unit)
+	if err != nil {
	return err
	}
+	serviceUnits := []string{unit}
+	if isActive {
+	// we can never restart the snapd.socket because
+	// this will also bring down snapd itself
+	if unit != "snapd.socket" {
+	if err := sysd.Restart(serviceUnits, 5*time.Second); err != nil {
+	return err
+	}
+	}
+	} else {
+	if err := sysd.Start(serviceUnits); err != nil {
+	return err
+	}
+	}
	}
-	}
 
-	// and finally start snapd.service (it will stop by itself and gets
-	// started by systemd then)
-	// Because of the file lock held on the snapstate by the Overlord, the new
-	// snapd will block there until we release it. For this reason, we cannot
-	// start the unit in blocking mode.
-	// TODO: move/share this responsibility with daemon so that we can make the
-	// start blocking again
-	if err := sysd.StartNoBlock([]string{"snapd.service"}); err != nil {
-	return err
-	}
-	if err := sysd.StartNoBlock([]string{"snapd.seeded.service"}); err != nil {
-	return err
-	}
-	// we cannot start snapd.autoimport in blocking mode because
-	// it has a "After=snapd.seeded.service" which means that on
-	// seeding a "systemctl start" that blocks would hang forever
-	// and we deadlock.
-	if err := sysd.StartNoBlock([]string{"snapd.autoimport.service"}); err != nil {
-	return err
+	// and finally start snapd.service (it will stop by itself and gets
+	// started by systemd then)
+	// Because of the file lock held on the snapstate by the Overlord, the new
+	// snapd will block there until we release it. For this reason, we cannot
+	// start the unit in blocking mode.
+	// TODO: move/share this responsibility with daemon so that we can make the
+	// start blocking again
+	if err := sysd.StartNoBlock([]string{"snapd.service"}); err != nil {
+	return err
+	}
+	if err := sysd.StartNoBlock([]string{"snapd.seeded.service"}); err != nil {
+	return err
+	}
+	// we cannot start snapd.autoimport in blocking mode because
+	// it has a "After=snapd.seeded.service" which means that on
+	// seeding a "systemctl start" that blocks would hang forever
+	// and we deadlock.
+	if err := sysd.StartNoBlock([]string{"snapd.autoimport.service"}); err != nil {
+	return err
+	}
	}
 
	// Handle the user services
@@ -407,6 +435,7 @@ func writeSnapdUserServicesOnCore(s *snap.Info, inter interacter) error {
	return err
	}
 
+	// TODO: use EmulationMode when preseeding (teach EmulationMode about user services)?
	sysd := systemd.New(systemd.GlobalUserMode, inter)
 
	serviceUnits, err := filepath.Glob(filepath.Join(s.MountDir(), "usr/lib/systemd/user/*.service"))
diff --git a/wrappers/core18_test.go b/wrappers/core18_test.go
index 650e8c0f45..f9dec2e01e 100644
--- a/wrappers/core18_test.go
+++ b/wrappers/core18_test.go
@@ -119,7 +119,7 @@ func (s *servicesTestSuite) TestAddSnapServicesForSnapdOnCore(c *C) {
 
	info := makeMockSnapdSnap(c)
	// add the snapd service
-	err := wrappers.AddSnapdSnapServices(info, progress.Null)
+	err := wrappers.AddSnapdSnapServices(info, nil, progress.Null)
	c.Assert(err, IsNil)
 
	mountUnit := fmt.Sprintf(`[Unit]
@@ -219,13 +219,124 @@ WantedBy=snapd.service
	})
 }
 
+func (s *servicesTestSuite) TestAddSnapServicesForSnapdOnCorePreseeding(c *C) {
+	restore := release.MockOnClassic(false)
+	defer restore()
+
+	restore = release.MockReleaseInfo(&release.OS{ID: "ubuntu"})
+	defer restore()
+
+	// reset root dir
+	dirs.SetRootDir(s.tempdir)
+
+	systemctlRestorer := systemd.MockSystemctl(func(cmd ...string) ([]byte, error) {
+	s.sysdLog = append(s.sysdLog, cmd)
+	if cmd[0] == "show" && cmd[1] == "--property=Id,ActiveState,UnitFileState,Type" {
+	s := fmt.Sprintf("Type=oneshot\nId=%s\nActiveState=inactive\nUnitFileState=enabled\n", cmd[2])
+	return []byte(s), nil
+	}
+	if len(cmd) == 2 && cmd[0] == "is-enabled" {
+	// pretend snapd.socket is disabled
+	if cmd[1] == "snapd.socket" {
+	return []byte("disabled"), &mockSystemctlError{msg: "disabled", exitCode: 1}
+	}
+	return []byte("enabled"), nil
+	}
+	return []byte("ActiveState=inactive\n"), nil
+	})
+	defer systemctlRestorer()
+
+	info := makeMockSnapdSnap(c)
+	// add the snapd service
+	err := wrappers.AddSnapdSnapServices(info, &wrappers.AddSnapdSnapServicesOptions{Preseeding: true}, progress.Null)
+	c.Assert(err, IsNil)
+
+	mountUnit := fmt.Sprintf(`[Unit]
+Description=Make the snapd snap tooling available for the system
+Before=snapd.service
+
+[Mount]
+What=%s/snap/snapd/1/usr/lib/snapd
+Where=/usr/lib/snapd
+Type=none
+Options=bind
+
+[Install]
+WantedBy=snapd.service
+`, dirs.GlobalRootDir)
+	for _, entry := range [][]string{{
+	// check that snapd.service is created
+	filepath.Join(dirs.SnapServicesDir, "snapd.service"),
+	// and paths get re-written
+	fmt.Sprintf("[Unit]\n[Service]\nExecStart=%[1]s/snapd/1/usr/lib/snapd/snapd\n# X-Snapd-Snap: do-not-start\n[Unit]\nRequiresMountsFor=%[1]s/snapd/1\n", dirs.SnapMountDir),
+	}, {
+	// check that snapd.autoimport.service is created
+	filepath.Join(dirs.SnapServicesDir, "snapd.autoimport.service"),
+	// and paths get re-written
+	fmt.Sprintf("[Unit]\n[Service]\nExecStart=%[1]s/snapd/1/usr/bin/snap auto-import\n[Unit]\nRequiresMountsFor=%[1]s/snapd/1\n", dirs.SnapMountDir),
+	}, {
+	// check that snapd.system-shutdown.service is created
+	filepath.Join(dirs.SnapServicesDir, "snapd.system-shutdown.service"),
+	// and paths *do not* get re-written
+	"[Unit]\n[Service]\nExecStart=/bin/umount --everything\n# X-Snapd-Snap: do-not-start",
+	}, {
+	// check that usr-lib-snapd.mount is created
+	filepath.Join(dirs.SnapServicesDir, "usr-lib-snapd.mount"),
+	mountUnit,
+	}, {
+	// check that snapd.session-agent.service is created
+	filepath.Join(dirs.SnapUserServicesDir, "snapd.session-agent.service"),
+	// and paths get re-written
+	fmt.Sprintf("[Unit]\n[Service]\nExecStart=%[1]s/snapd/1/usr/bin/snap session-agent\n[Unit]\nRequiresMountsFor=%[1]s/snapd/1\n", dirs.SnapMountDir),
+	}, {
+	// check that snapd.session-agent.socket is created
+	filepath.Join(dirs.SnapUserServicesDir, "snapd.session-agent.socket"),
+	"[Unit]\n[Socket]\nListenStream=%t/snap-session.socket",
+	}, {
+	filepath.Join(dirs.SnapDBusSystemPolicyDir, "snapd.system-services.conf"),
+	"<busconfig/>",
+	}, {
+	filepath.Join(dirs.SnapDBusSessionPolicyDir, "snapd.session-services.conf"),
+	"<busconfig/>",
+	}, {
+	filepath.Join(dirs.SnapDBusSessionServicesDir, "io.snapcraft.Launcher.service"),
+	"[D-BUS Service]\nName=io.snapcraft.Launcher",
+	}, {
+	filepath.Join(dirs.SnapDBusSessionServicesDir, "io.snapcraft.Settings.service"),
+	"[D-BUS Service]\nName=io.snapcraft.Settings",
+	}, {
+	filepath.Join(dirs.SnapDBusSessionServicesDir, "io.snapcraft.SessionAgent.service"),
+	"[D-BUS Service]\nName=io.snapcraft.SessionAgent",
+	}} {
+	c.Check(entry[0], testutil.FileEquals, entry[1])
+	}
+
+	// Non-snapd D-Bus config is not copied
+	c.Check(filepath.Join(dirs.SnapDBusSystemPolicyDir, "io.netplan.Netplan.conf"), testutil.FileAbsent)
+
+	// check the systemctl calls
+	c.Check(s.sysdLog, DeepEquals, [][]string{
+	{"--root", s.tempdir, "enable", "usr-lib-snapd.mount"},
+	{"--root", s.tempdir, "enable", "snapd.autoimport.service"},
+	{"--root", s.tempdir, "enable", "snapd.service"},
+	{"--root", s.tempdir, "enable", "snapd.snap-repair.timer"},
+	// test pretends snapd.socket is disabled and needs enabling
+	{"--root", s.tempdir, "enable", "snapd.socket"},
+	{"--root", s.tempdir, "enable", "snapd.system-shutdown.service"},
+	{"--user", "--global", "disable", "snapd.session-agent.service"},
+	{"--user", "--global", "enable", "snapd.session-agent.service"},
+	{"--user", "--global", "disable", "snapd.session-agent.socket"},
+	{"--user", "--global", "enable", "snapd.session-agent.socket"},
+	})
+}
+
 func (s *servicesTestSuite) TestAddSnapServicesForSnapdOnClassic(c *C) {
	restore := release.MockOnClassic(true)
	defer restore()
 
	info := makeMockSnapdSnap(c)
	// add the snapd service
-	err := wrappers.AddSnapdSnapServices(info, progress.Null)
+	err := wrappers.AddSnapdSnapServices(info, nil, progress.Null)
	c.Assert(err, IsNil)
 
	// check that snapd services were *not* created
@@ -260,7 +371,7 @@ func (s *servicesTestSuite) TestAddSessionServicesWithReadOnlyFilesystem(c *C) {
	defer restore()
 
	// add the snapd service
-	err := wrappers.AddSnapdSnapServices(info, progress.Null)
+	err := wrappers.AddSnapdSnapServices(info, nil, progress.Null)
 
	// didn't fail despite of read-only SnapDBusSessionPolicyDir
	c.Assert(err, IsNil)
@@ -286,7 +397,7 @@ func (s *servicesTestSuite) TestAddSnapdServicesWithNonSnapd(c *C) {
	restore = release.MockReleaseInfo(&release.OS{ID: "ubuntu"})
	defer restore()
 
-	err := wrappers.AddSnapdSnapServices(info, progress.Null)
+	err := wrappers.AddSnapdSnapServices(info, nil, progress.Null)
	c.Assert(err, ErrorMatches, `internal error: adding explicit snapd services for snap "foo" type "app" is unexpected`)
 }