diff --git a/daemon/api.go b/daemon/api.go index 39a0884f833..4b8a1d35375 100644 --- a/daemon/api.go +++ b/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) } diff --git a/daemon/api_test.go b/daemon/api_test.go index a111eebeec7..bfe5179ba5f 100644 --- a/daemon/api_test.go +++ b/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() diff --git a/dirs/dirs.go b/dirs/dirs.go index 2d031c017d1..49641f6b545 100644 --- a/dirs/dirs.go +++ b/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 /* + // * in snapstate to auto-cleans them up using the same prefix + LocalInstallBlobTempPrefix = ".local-install-" ) var ( diff --git a/osutil/unlink.go b/osutil/unlink.go index 056627608ab..bb117d258e6 100644 --- a/osutil/unlink.go +++ b/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) +} diff --git a/osutil/unlink_test.go b/osutil/unlink_test.go index 05016dcecd0..84e0aa82b37 100644 --- a/osutil/unlink_test.go +++ b/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 diff --git a/overlord/snapstate/export_test.go b/overlord/snapstate/export_test.go index 96d4922ed99..91c759e8c5e 100644 --- a/overlord/snapstate/export_test.go +++ b/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}) } diff --git a/overlord/snapstate/snapmgr.go b/overlord/snapstate/snapmgr.go index 44bfa72ddb0..2a90f3b908d 100644 --- a/overlord/snapstate/snapmgr.go +++ b/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 diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index b6ba1ecb2e4..5030f76a511 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/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