daemon: spool sideloaded snap into blob dir (#6093)

* daemon: spool sideloaded snap into blob dir

Before this change a sideloaded snap would get written out to /tmp
before being copied into /var/lib/snapd/snaps. As these are often
separate devices this means sideloading a snap would be unnecessarily
slow, and use up a potentially large amount of /tmp (which might be
small and in-memory, on core).

This change makes it so that the snap is instead written to a
temporary (and hidden) file in /var/lib/snapd/snaps directly.

* osutil, overlord/snapstate: remove old sideload files from snapstate.Ensure

* overlord/snapstate: make the test a little quicker

* dirs, daemon, overlord/snapstate: move the sideload blob temp prefix def to dirs

* daemon, dirs, osutil, overlord/snapstate: address review feedback

This is mosty a rename and a small change to only run the cleanup code
once a day.

Thanks @pedronis for the excellent feedback.

daemon/api.go

@@ -1489,7 +1489,8 @@ out:
 	// we are in charge of the tempfile life cycle until we hand it off to the change
 	changeTriggered := false
 	// if you change this prefix, look for it in the tests
-	tmpf, err := ioutil.TempFile("", "snapd-sideload-pkg-")
+	// also see localInstallCleanup in snapstate/snapmgr.go
+	tmpf, err := ioutil.TempFile(dirs.SnapBlobDir, dirs.LocalInstallBlobTempPrefix)
 	if err != nil {
 		return InternalError("cannot create temporary file: %v", err)
 	}

daemon/api_test.go

@@ -35,6 +35,7 @@ import (
 	"os"
 	"os/user"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -246,6 +247,7 @@ func (s *apiBaseSuite) SetUpTest(c *check.C) {
 	err := os.MkdirAll(filepath.Dir(dirs.SnapStateFile), 0755)
 	c.Assert(err, check.IsNil)
 	c.Assert(os.MkdirAll(dirs.SnapMountDir, 0755), check.IsNil)
+	c.Assert(os.MkdirAll(dirs.SnapBlobDir, 0755), check.IsNil)
 
 	s.rsnaps = nil
 	s.suggestedCurrency = ""
@@ -2513,7 +2515,7 @@ func (s *apiSuite) sideloadCheck(c *check.C, content string, head map[string]str
 	c.Assert(rsp.Type, check.Equals, ResponseTypeAsync)
 	n := 1
 	c.Assert(installQueue, check.HasLen, n)
-	c.Check(installQueue[n-1], check.Matches, "local::.*/snapd-sideload-pkg-.*")
+	c.Check(installQueue[n-1], check.Matches, "local::.*/"+regexp.QuoteMeta(dirs.LocalInstallBlobTempPrefix)+".*")
 
 	st := d.overlord.State()
 	st.Lock()

dirs/dirs.go

@@ -122,6 +122,11 @@ const (
 
 	// Directory with snap data inside user's home
 	UserHomeSnapDir = "snap"
+
+	// LocalInstallBlobTempPrefix is used by local install code:
+	// * in daemon to spool the snap file to <SnapBlobDir>/<LocalInstallBlobTempPrefix>*
+	// * in snapstate to auto-cleans them up using the same prefix
+	LocalInstallBlobTempPrefix = ".local-install-"
 )
 
 var (

osutil/unlink.go

@@ -48,6 +48,11 @@ func UnlinkMany(dirname string, filenames []string) error {
 	}
 	defer syscall.Close(dirfd)
 
+	return unlinkMany(dirfd, filenames)
+}
+
+func unlinkMany(dirfd int, filenames []string) error {
+	var err error
 	for _, filename := range filenames {
 		if err = sysUnlinkat(dirfd, filename); err != nil && err != syscall.ENOENT {
 			return &os.PathError{
@@ -59,3 +64,9 @@ func UnlinkMany(dirname string, filenames []string) error {
 	}
 	return nil
 }
+
+// UnlinkManyAt is like UnlinkMany but takes an open directory *os.File
+// instead of a dirname.
+func UnlinkManyAt(dir *os.File, filenames []string) error {
+	return unlinkMany(int(dir.Fd()), filenames)
+}

osutil/unlink_test.go

@@ -71,6 +71,14 @@ func (s *unlinkSuite) TestUnlinkMany(c *check.C) {
 	s.checkDirnames(c, []string{"foo", "quux", "dir"})
 }
 
+func (s *unlinkSuite) TestUnlinkManyAt(c *check.C) {
+	d, err := os.Open(s.d)
+	c.Assert(err, check.IsNil)
+	c.Assert(osutil.UnlinkManyAt(d, []string{"bar", "does-not-exist", "baz"}), check.IsNil)
+
+	s.checkDirnames(c, []string{"foo", "quux", "dir"})
+}
+
 func (s *unlinkSuite) TestUnlinkManyFails(c *check.C) {
 	type T struct {
 		dirname   string

overlord/snapstate/export_test.go

@@ -143,6 +143,22 @@ func MockIsOnMeteredConnection(mock func() (bool, error)) func() {
 	}
 }
 
+func MockLocalInstallCleanupWait(d time.Duration) (restore func()) {
+	old := localInstallCleanupWait
+	localInstallCleanupWait = d
+	return func() {
+		localInstallCleanupWait = old
+	}
+}
+
+func MockLocalInstallLastCleanup(t time.Time) (restore func()) {
+	old := localInstallLastCleanup
+	localInstallLastCleanup = t
+	return func() {
+		localInstallLastCleanup = old
+	}
+}
+
 func SetModelWithBase(baseName string) {
 	setModel(map[string]string{"base": baseName})
 }

overlord/snapstate/snapmgr.go

@@ -22,7 +22,9 @@ package snapstate
 import (
 	"errors"
 	"fmt"
+	"io"
 	"os"
+	"strings"
 	"time"
 
 	"gopkg.in/tomb.v2"
@@ -31,6 +33,7 @@ import (
 	"github.com/snapcore/snapd/errtracker"
 	"github.com/snapcore/snapd/i18n"
 	"github.com/snapcore/snapd/logger"
+	"github.com/snapcore/snapd/osutil"
 	"github.com/snapcore/snapd/overlord/snapstate/backend"
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/release"
@@ -575,6 +578,62 @@ func (m *SnapManager) atSeed() error {
 	return nil
 }
 
+var (
+	localInstallCleanupWait = time.Duration(24 * time.Hour)
+	localInstallLastCleanup time.Time
+)
+
+// cleanOldTemps removes files that might've been left behind by an
+// old aborted local install
+//
+// They're usually cleaned up, but if they're created and then snapd
+// stops before writing the change to disk (killed, light cut, etc)
+// it'll be left behind.
+//
+// The code that creates the files is in daemon/api.go's postSnaps
+func (m *SnapManager) localInstallCleanup() error {
+	m.state.Lock()
+	defer m.state.Unlock()
+
+	now := time.Now()
+	cutoff := now.Add(-localInstallCleanupWait)
+	if localInstallLastCleanup.After(cutoff) {
+		return nil
+	}
+	localInstallLastCleanup = now
+
+	d, err := os.Open(dirs.SnapBlobDir)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return err
+	}
+	defer d.Close()
+
+	var filenames []string
+	var fis []os.FileInfo
+	for err == nil {
+		// TODO: if we had fstatat we could avoid a bunch of stats
+		fis, err = d.Readdir(100)
+		// fis is nil if err isn't
+		for _, fi := range fis {
+			name := fi.Name()
+			if !strings.HasPrefix(name, dirs.LocalInstallBlobTempPrefix) {
+				continue
+			}
+			if fi.ModTime().After(cutoff) {
+				continue
+			}
+			filenames = append(filenames, name)
+		}
+	}
+	if err != io.EOF {
+		return err
+	}
+	return osutil.UnlinkManyAt(d, filenames)
+}
+
 // Ensure implements StateManager.Ensure.
 func (m *SnapManager) Ensure() error {
 	// do not exit right away on error
@@ -588,6 +647,7 @@ func (m *SnapManager) Ensure() error {
 		m.autoRefresh.Ensure(),
 		m.refreshHints.Ensure(),
 		m.catalogRefresh.Ensure(),
+		m.localInstallCleanup(),
 	}
 
 	//FIXME: use firstErr helper

overlord/snapstate/snapstate_test.go

@@ -8204,6 +8204,56 @@ func (s *snapmgrTestSuite) TestEnsureRefreshesAtSeedPolicy(c *C) {
 	c.Check(err, IsNil)
 }
 
+func (s *snapmgrTestSuite) TestEsnureCleansOldSideloads(c *C) {
+	filenames := func() []string {
+		filenames, _ := filepath.Glob(filepath.Join(dirs.SnapBlobDir, "*"))
+		return filenames
+	}
+
+	defer snapstate.MockLocalInstallCleanupWait(200 * time.Millisecond)()
+	c.Assert(os.MkdirAll(dirs.SnapBlobDir, 0700), IsNil)
+	// sanity check; note * in go glob matches .foo
+	c.Assert(filenames(), HasLen, 0)
+
+	s0 := filepath.Join(dirs.SnapBlobDir, "some.snap")
+	s1 := filepath.Join(dirs.SnapBlobDir, dirs.LocalInstallBlobTempPrefix+"-12345")
+	s2 := filepath.Join(dirs.SnapBlobDir, dirs.LocalInstallBlobTempPrefix+"-67890")
+
+	c.Assert(ioutil.WriteFile(s0, nil, 0600), IsNil)
+	c.Assert(ioutil.WriteFile(s1, nil, 0600), IsNil)
+	c.Assert(ioutil.WriteFile(s2, nil, 0600), IsNil)
+
+	t1 := time.Now()
+	t0 := t1.Add(-time.Hour)
+
+	c.Assert(os.Chtimes(s0, t0, t0), IsNil)
+	c.Assert(os.Chtimes(s1, t0, t0), IsNil)
+	c.Assert(os.Chtimes(s2, t1, t1), IsNil)
+
+	// all there
+	c.Assert(filenames(), DeepEquals, []string{s1, s2, s0})
+
+	// set last cleanup in the future
+	defer snapstate.MockLocalInstallLastCleanup(t1.Add(time.Minute))()
+	s.snapmgr.Ensure()
+	// all there ( -> cleanup not done)
+	c.Assert(filenames(), DeepEquals, []string{s1, s2, s0})
+
+	// set last cleanup to epoch
+	snapstate.MockLocalInstallLastCleanup(time.Time{})
+
+	s.snapmgr.Ensure()
+	// oldest sideload gone
+	c.Assert(filenames(), DeepEquals, []string{s2, s0})
+
+	time.Sleep(200 * time.Millisecond)
+
+	s.snapmgr.Ensure()
+	// all sideloads gone
+	c.Assert(filenames(), DeepEquals, []string{s0})
+
+}
+
 func (s *snapmgrTestSuite) verifyRefreshLast(c *C) {
 	var lastRefresh time.Time