Merge pull request #6621 from zyga/feature/per-snap-refresh-skip-time

overlord/snapstate: track time of postponed refreshes

overlord/snapstate/handlers.go

@@ -877,6 +877,7 @@ func (m *SnapManager) doLinkSnap(t *state.Task, _ *tomb.Tomb) (err error) {
 	if snapsup.Required { // set only on install and left alone on refresh
 		snapst.Required = true
 	}
+	oldRefreshInhibitedTime := snapst.RefreshInhibitedTime
 	// only set userID if unset or logged out in snapst and if we
 	// actually have an associated user
 	if snapsup.UserID > 0 {
@@ -940,6 +941,11 @@ func (m *SnapManager) doLinkSnap(t *state.Task, _ *tomb.Tomb) (err error) {
 	t.Set("old-channel", oldChannel)
 	t.Set("old-current", oldCurrent)
 	t.Set("old-candidate-index", oldCandidateIndex)
+	t.Set("old-refresh-inhibited-time", oldRefreshInhibitedTime)
+
+	// Record the fact that the snap was refreshed successfully.
+	snapst.RefreshInhibitedTime = nil
+
 	// Do at the end so we only preserve the new state if it worked.
 	Set(st, snapsup.InstanceName(), snapst)
 
@@ -1090,6 +1096,10 @@ func (m *SnapManager) undoLinkSnap(t *state.Task, _ *tomb.Tomb) error {
 	if err := t.Get("old-candidate-index", &oldCandidateIndex); err != nil {
 		return err
 	}
+	var oldRefreshInhibitedTime *time.Time
+	if err := t.Get("old-refresh-inhibited-time", &oldRefreshInhibitedTime); err != nil && err != state.ErrNoState {
+		return err
+	}
 
 	if len(snapst.Sequence) == 1 {
 		// XXX: shouldn't these two just log and carry on? this is an undo handler...
@@ -1130,6 +1140,7 @@ func (m *SnapManager) undoLinkSnap(t *state.Task, _ *tomb.Tomb) error {
 	snapst.DevMode = oldDevMode
 	snapst.JailMode = oldJailMode
 	snapst.Classic = oldClassic
+	snapst.RefreshInhibitedTime = oldRefreshInhibitedTime
 
 	newInfo, err := readInfo(snapsup.InstanceName(), snapsup.SideInfo, 0)
 	if err != nil {

overlord/snapstate/handlers_link_test.go

@@ -794,3 +794,89 @@ func (s *linkSnapSuite) TestDoLinkSnapFailureCleansUpAux(c *C) {
 	// we end without the auxiliary store info
 	c.Check(snapstate.AuxStoreInfoFilename("foo-id"), testutil.FileAbsent)
 }
+
+func (s *linkSnapSuite) TestLinkSnapResetsRefreshInhibitedTime(c *C) {
+	// When a snap is linked the refresh-inhibited-time is reset to zero
+	// to indicate a successful refresh. The old value is stored in task
+	// state for task undo logic.
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	instant := time.Now()
+
+	si := &snap.SideInfo{RealName: "snap", Revision: snap.R(1)}
+	sup := &snapstate.SnapSetup{SideInfo: si}
+	snapstate.Set(s.state, "snap", &snapstate.SnapState{
+		Sequence:             []*snap.SideInfo{si},
+		Current:              si.Revision,
+		RefreshInhibitedTime: &instant,
+	})
+
+	task := s.state.NewTask("link-snap", "")
+	task.Set("snap-setup", sup)
+	chg := s.state.NewChange("test", "")
+	chg.AddTask(task)
+
+	s.state.Unlock()
+
+	for i := 0; i < 10; i++ {
+		s.se.Ensure()
+		s.se.Wait()
+	}
+
+	s.state.Lock()
+
+	c.Assert(chg.Err(), IsNil)
+	c.Assert(chg.Tasks(), HasLen, 1)
+
+	var snapst snapstate.SnapState
+	err := snapstate.Get(s.state, "snap", &snapst)
+	c.Assert(err, IsNil)
+	c.Check(snapst.RefreshInhibitedTime, IsNil)
+
+	var oldTime time.Time
+	c.Assert(task.Get("old-refresh-inhibited-time", &oldTime), IsNil)
+	c.Check(oldTime.Equal(instant), Equals, true)
+}
+
+func (s *linkSnapSuite) TestDoUndoLinkSnapRestoresRefreshInhibitedTime(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	instant := time.Now()
+
+	si := &snap.SideInfo{RealName: "snap", Revision: snap.R(1)}
+	sup := &snapstate.SnapSetup{SideInfo: si}
+	snapstate.Set(s.state, "snap", &snapstate.SnapState{
+		Sequence:             []*snap.SideInfo{si},
+		Current:              si.Revision,
+		RefreshInhibitedTime: &instant,
+	})
+
+	task := s.state.NewTask("link-snap", "")
+	task.Set("snap-setup", sup)
+	chg := s.state.NewChange("test", "")
+	chg.AddTask(task)
+
+	terr := s.state.NewTask("error-trigger", "provoking total undo")
+	terr.WaitFor(task)
+	chg.AddTask(terr)
+
+	s.state.Unlock()
+
+	for i := 0; i < 6; i++ {
+		s.se.Ensure()
+		s.se.Wait()
+	}
+
+	s.state.Lock()
+
+	c.Assert(chg.Err(), NotNil)
+	c.Assert(chg.Tasks(), HasLen, 2)
+	c.Check(task.Status(), Equals, state.UndoneStatus)
+
+	var snapst snapstate.SnapState
+	err := snapstate.Get(s.state, "snap", &snapst)
+	c.Assert(err, IsNil)
+	c.Check(snapst.RefreshInhibitedTime.Equal(instant), Equals, true)
+}

overlord/snapstate/snapmgr.go

@@ -139,6 +139,11 @@ type SnapState struct {
 	// InstanceKey is set by the user during installation and differs for
 	// each instance of given snap
 	InstanceKey string `json:"instance-key,omitempty"`
+
+	// RefreshInhibitedime records the time when the refresh was first
+	// attempted but inhibited because the snap was busy. This value is
+	// reset on each successful refresh.
+	RefreshInhibitedTime *time.Time `json:"refresh-inhibited-time,omitempty"`
 }
 
 // Type returns the type of the snap or an error.