Merge pull request #11365 from stolowski/core20-preseed/wrappers

wrappers: do not reload the deamon or restart snapd services when preseeding on core

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 {

systemd/emulation.go

@@ -51,7 +51,7 @@ func (s *emulation) Backend() Backend {
 }
 
 func (s *emulation) DaemonReload() error {
-	return &notImplementedError{"DaemonReload"}
+	return nil
 }
 
 func (s *emulation) DaemonReexec() error {
@@ -69,23 +69,23 @@ func (s *emulation) Disable(services []string) error {
 }
 
 func (s *emulation) Start(services []string) error {
-	return &notImplementedError{"Start"}
+	return nil
 }
 
 func (s *emulation) StartNoBlock(services []string) error {
-	return &notImplementedError{"StartNoBlock"}
+	return nil
 }
 
 func (s *emulation) Stop(services []string, timeout time.Duration) error {
-	return &notImplementedError{"Stop"}
+	return nil
 }
 
 func (s *emulation) Kill(service, signal, who string) error {
 	return &notImplementedError{"Kill"}
 }
 
 func (s *emulation) Restart(services []string, timeout time.Duration) error {
-	return &notImplementedError{"Restart"}
+	return nil
 }
 
 func (s *emulation) ReloadOrRestart(service string) error {

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,9 +98,11 @@ WantedBy=snapd.service
 	if err != nil {
 		return err
 	}
+
 	if err := sysd.DaemonReload(); err != nil {
 		return err
 	}
+
 	units := []string{SnapdToolingMountUnit}
 	if err := sysd.Enable(units); err != nil {
 		return err
@@ -136,9 +138,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 +157,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,6 +220,7 @@ 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}
@@ -231,49 +250,54 @@ 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
+				}
+			}
 		}
 	}
 
@@ -407,6 +431,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"))

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,106 @@ 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)
+
+	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"},
+		{"--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 +353,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 +379,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`)
 }