overlord,usersession: initial notifications of pending refreshes

overlord/snapstate/autorefresh.go

@@ -20,6 +20,7 @@
 package snapstate
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"time"
@@ -35,6 +36,7 @@ import (
 	"github.com/snapcore/snapd/strutil"
 	"github.com/snapcore/snapd/timeutil"
 	"github.com/snapcore/snapd/timings"
+	userclient "github.com/snapcore/snapd/usersession/client"
 )
 
 // the default refresh pattern
@@ -531,21 +533,45 @@ func getTime(st *state.State, timeKey string) (time.Time, error) {
 	return t1, nil
 }
 
+// asyncPendingRefreshNotification broadcasts desktop notification in a goroutine.
+//
+// This allows the, possibly slow, communication with each snapd session agent,
+// to be performed without holding the snap state lock.
+var asyncPendingRefreshNotification = func(context context.Context, client *userclient.Client, refreshInfo *userclient.PendingSnapRefreshInfo) {
+	go func() {
+		if err := client.PendingRefreshNotification(context, refreshInfo); err != nil {
+			logger.Noticef("Cannot send notification about pending refresh: %v", err)
+		}
+	}()
+}
+
 // inhibitRefresh returns an error if refresh is inhibited by running apps.
 //
 // Internally the snap state is updated to remember when the inhibition first
 // took place. Apps can inhibit refreshes for up to "maxInhibition", beyond
 // that period the refresh will go ahead despite application activity.
 func inhibitRefresh(st *state.State, snapst *SnapState, info *snap.Info, checker func(*snap.Info) error) error {
 	if err := checker(info); err != nil {
+		refreshInfo := &userclient.PendingSnapRefreshInfo{
+			InstanceName: info.InstanceName(),
+		}
+		if err, ok := err.(*BusySnapError); ok {
+			refreshInfo = err.PendingSnapRefreshInfo()
+		}
+
 		days := int(maxInhibition.Truncate(time.Hour).Hours() / 24)
 		now := time.Now()
+		client := userclient.New()
 		if snapst.RefreshInhibitedTime == nil {
 			// Store the instant when the snap was first inhibited.
 			// This is reset to nil on successful refresh.
 			snapst.RefreshInhibitedTime = &now
 			Set(st, info.InstanceName(), snapst)
 			if _, ok := err.(*BusySnapError); ok {
+				refreshInfo.TimeRemaining = (maxInhibition - now.Sub(*snapst.RefreshInhibitedTime)).Truncate(time.Second)
+				// Send the notification asynchronously to avoid holding the state lock.
+				asyncPendingRefreshNotification(context.TODO(), client, refreshInfo)
+				// XXX: remove the warning or send it only if no notification was delivered?
 				st.Warnf(i18n.NG(
 					"snap %q is currently in use. Its refresh will be postponed for up to %d day to wait for the snap to no longer be in use.",
 					"snap %q is currently in use. Its refresh will be postponed for up to %d days to wait for the snap to no longer be in use.", days),
@@ -557,9 +583,16 @@ func inhibitRefresh(st *state.State, snapst *SnapState, info *snap.Info, checker
 		if now.Sub(*snapst.RefreshInhibitedTime) < maxInhibition {
 			// If we are still in the allowed window then just return
 			// the error but don't change the snap state again.
+			refreshInfo.TimeRemaining = (maxInhibition - now.Sub(*snapst.RefreshInhibitedTime)).Truncate(time.Second)
+			asyncPendingRefreshNotification(context.TODO(), client, refreshInfo)
+			// TODO: as time left shrinks, send additional notifications with
+			// increasing frequency, allowing the user to understand the
+			// urgency.
 			return err
 		}
 		if _, ok := err.(*BusySnapError); ok {
+			asyncPendingRefreshNotification(context.TODO(), client, refreshInfo)
+			// XXX: remove the warning or send it only if no notification was delivered?
 			st.Warnf(i18n.NG(
 				"snap %q has been running for the maximum allowable %d day since its refresh was postponed. It will now be refreshed.",
 				"snap %q has been running for the maximum allowable %d days since its refresh was postponed. It will now be refreshed.", days),

overlord/snapstate/autorefresh_test.go

@@ -43,6 +43,7 @@ import (
 	"github.com/snapcore/snapd/store/storetest"
 	"github.com/snapcore/snapd/testutil"
 	"github.com/snapcore/snapd/timeutil"
+	userclient "github.com/snapcore/snapd/usersession/client"
 )
 
 type autoRefreshStore struct {
@@ -800,30 +801,82 @@ func (s *autoRefreshTestSuite) TestRefreshOnMeteredConnNotMetered(c *C) {
 	c.Check(s.store.ops, DeepEquals, []string{"list-refresh"})
 }
 
-func (s *autoRefreshTestSuite) TestInhibitRefreshWithinInhibitWindow(c *C) {
+func (s *autoRefreshTestSuite) TestInitialInhibitRefreshWithinInhibitWindow(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
 
+	notificationCount := 0
+	restore := snapstate.MockAsyncPendingRefreshNotification(func(ctx context.Context, client *userclient.Client, refreshInfo *userclient.PendingSnapRefreshInfo) {
+		notificationCount++
+		c.Check(refreshInfo.InstanceName, Equals, "pkg")
+		c.Check(refreshInfo.TimeRemaining, Equals, time.Hour*14*24)
+	})
+	defer restore()
+
 	si := &snap.SideInfo{RealName: "pkg", Revision: snap.R(1)}
 	info := &snap.Info{SideInfo: *si}
 	snapst := &snapstate.SnapState{
 		Sequence: []*snap.SideInfo{si},
 		Current:  si.Revision,
 	}
 	err := snapstate.InhibitRefresh(s.state, snapst, info, func(si *snap.Info) error {
-		return &snapstate.BusySnapError{SnapName: "pkg"}
+		return &snapstate.BusySnapError{SnapInfo: si}
 	})
 	c.Assert(err, ErrorMatches, `snap "pkg" has running apps or hooks`)
 
 	pending, _ := s.state.PendingWarnings()
 	c.Assert(pending, HasLen, 1)
 	c.Check(pending[0].String(), Equals, `snap "pkg" is currently in use. Its refresh will be postponed for up to 14 days to wait for the snap to no longer be in use.`)
+	c.Check(notificationCount, Equals, 1)
+}
+
+func (s *autoRefreshTestSuite) TestSubsequentInhibitRefreshWithinInhibitWindow(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	notificationCount := 0
+	restore := snapstate.MockAsyncPendingRefreshNotification(func(ctx context.Context, client *userclient.Client, refreshInfo *userclient.PendingSnapRefreshInfo) {
+		notificationCount++
+		c.Check(refreshInfo.InstanceName, Equals, "pkg")
+		// XXX: This test measures real time, with second granularity.
+		// It takes non-zero (hence the subtracted second) to execute the test.
+		c.Check(refreshInfo.TimeRemaining, Equals, time.Hour*14*24/2-time.Second)
+	})
+	defer restore()
+
+	instant := time.Now()
+	pastInstant := instant.Add(-snapstate.MaxInhibition / 2) // In the middle of the allowed window
+
+	si := &snap.SideInfo{RealName: "pkg", Revision: snap.R(1)}
+	info := &snap.Info{SideInfo: *si}
+	snapst := &snapstate.SnapState{
+		Sequence:             []*snap.SideInfo{si},
+		Current:              si.Revision,
+		RefreshInhibitedTime: &pastInstant,
+	}
+
+	err := snapstate.InhibitRefresh(s.state, snapst, info, func(si *snap.Info) error {
+		return &snapstate.BusySnapError{SnapInfo: si}
+	})
+	c.Assert(err, ErrorMatches, `snap "pkg" has running apps or hooks`)
+
+	pending, _ := s.state.PendingWarnings()
+	c.Assert(pending, HasLen, 0) // This case does not warn
+	c.Check(notificationCount, Equals, 1)
 }
 
 func (s *autoRefreshTestSuite) TestInhibitRefreshWarnsAndRefreshesWhenOverdue(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
 
+	notificationCount := 0
+	restore := snapstate.MockAsyncPendingRefreshNotification(func(ctx context.Context, client *userclient.Client, refreshInfo *userclient.PendingSnapRefreshInfo) {
+		notificationCount++
+		c.Check(refreshInfo.InstanceName, Equals, "pkg")
+		c.Check(refreshInfo.TimeRemaining, Equals, time.Duration(0))
+	})
+	defer restore()
+
 	instant := time.Now()
 	pastInstant := instant.Add(-snapstate.MaxInhibition * 2)
 
@@ -835,11 +888,12 @@ func (s *autoRefreshTestSuite) TestInhibitRefreshWarnsAndRefreshesWhenOverdue(c
 		RefreshInhibitedTime: &pastInstant,
 	}
 	err := snapstate.InhibitRefresh(s.state, snapst, info, func(si *snap.Info) error {
-		return &snapstate.BusySnapError{SnapName: "pkg"}
+		return &snapstate.BusySnapError{SnapInfo: si}
 	})
 	c.Assert(err, IsNil)
 
 	pending, _ := s.state.PendingWarnings()
 	c.Assert(pending, HasLen, 1)
 	c.Check(pending[0].String(), Equals, `snap "pkg" has been running for the maximum allowable 14 days since its refresh was postponed. It will now be refreshed.`)
+	c.Check(notificationCount, Equals, 1)
 }

overlord/snapstate/export_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/snap"
 	"github.com/snapcore/snapd/store"
+	userclient "github.com/snapcore/snapd/usersession/client"
 )
 
 type ManagerBackend managerBackend
@@ -190,6 +191,14 @@ func MockLocalInstallLastCleanup(t time.Time) (restore func()) {
 	}
 }
 
+func MockAsyncPendingRefreshNotification(fn func(context.Context, *userclient.Client, *userclient.PendingSnapRefreshInfo)) (restore func()) {
+	old := asyncPendingRefreshNotification
+	asyncPendingRefreshNotification = fn
+	return func() {
+		asyncPendingRefreshNotification = old
+	}
+}
+
 // re-refresh related
 var (
 	RefreshedSnaps  = refreshedSnaps
@@ -263,6 +272,15 @@ var (
 	MaxInhibition  = maxInhibition
 )
 
+func NewBusySnapError(info *snap.Info, pids []int, busyAppNames, busyHookNames []string) *BusySnapError {
+	return &BusySnapError{
+		SnapInfo:      info,
+		pids:          pids,
+		busyAppNames:  busyAppNames,
+		busyHookNames: busyHookNames,
+	}
+}
+
 func MockGenericRefreshCheck(fn func(info *snap.Info, canAppRunDuringRefresh func(app *snap.AppInfo) bool) error) (restore func()) {
 	old := genericRefreshCheck
 	genericRefreshCheck = fn

overlord/snapstate/refresh.go

@@ -21,6 +21,7 @@ package snapstate
 
 import (
 	"fmt"
+	"path/filepath"
 	"sort"
 	"strings"
 
@@ -30,6 +31,7 @@ import (
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/sandbox/cgroup"
 	"github.com/snapcore/snapd/snap"
+	userclient "github.com/snapcore/snapd/usersession/client"
 )
 
 // pidsOfSnap is a mockable version of PidsOfSnap
@@ -79,7 +81,7 @@ var genericRefreshCheck = func(info *snap.Info, canAppRunDuringRefresh func(app
 	sort.Strings(busyHookNames)
 	sort.Ints(busyPIDs)
 	return &BusySnapError{
-		SnapName:      info.SnapName(),
+		SnapInfo:      info,
 		busyAppNames:  busyAppNames,
 		busyHookNames: busyHookNames,
 		pids:          busyPIDs,
@@ -123,26 +125,49 @@ func HardNothingRunningRefreshCheck(info *snap.Info) error {
 
 // BusySnapError indicates that snap has apps or hooks running and cannot refresh.
 type BusySnapError struct {
-	SnapName      string
+	SnapInfo      *snap.Info
 	pids          []int
 	busyAppNames  []string
 	busyHookNames []string
 }
 
+// PendingSnapRefreshInfo computes information necessary to perform user notification
+// of postponed refresh of a snap, based on the information about snap "business".
+//
+// The returned value contains the instance name of the snap as well as, if possible,
+// information relevant for desktop notification services, such as application name
+// and the snapd-generated desktop file name.
+func (err *BusySnapError) PendingSnapRefreshInfo() *userclient.PendingSnapRefreshInfo {
+	refreshInfo := &userclient.PendingSnapRefreshInfo{
+		InstanceName: err.SnapInfo.InstanceName(),
+	}
+	for _, appName := range err.busyAppNames {
+		if app, ok := err.SnapInfo.Apps[appName]; ok {
+			path := app.DesktopFile()
+			if osutil.FileExists(path) {
+				refreshInfo.BusyAppName = appName
+				refreshInfo.BusyAppDesktopEntry = strings.SplitN(filepath.Base(path), ".", 2)[0]
+				break
+			}
+		}
+	}
+	return refreshInfo
+}
+
 // Error formats an error string describing what is running.
 func (err *BusySnapError) Error() string {
 	switch {
 	case len(err.busyAppNames) > 0 && len(err.busyHookNames) > 0:
 		return fmt.Sprintf("snap %q has running apps (%s) and hooks (%s)",
-			err.SnapName, strings.Join(err.busyAppNames, ", "), strings.Join(err.busyHookNames, ", "))
+			err.SnapInfo.InstanceName(), strings.Join(err.busyAppNames, ", "), strings.Join(err.busyHookNames, ", "))
 	case len(err.busyAppNames) > 0:
 		return fmt.Sprintf("snap %q has running apps (%s)",
-			err.SnapName, strings.Join(err.busyAppNames, ", "))
+			err.SnapInfo.InstanceName(), strings.Join(err.busyAppNames, ", "))
 	case len(err.busyHookNames) > 0:
 		return fmt.Sprintf("snap %q has running hooks (%s)",
-			err.SnapName, strings.Join(err.busyHookNames, ", "))
+			err.SnapInfo.InstanceName(), strings.Join(err.busyHookNames, ", "))
 	default:
-		return fmt.Sprintf("snap %q has running apps or hooks", err.SnapName)
+		return fmt.Sprintf("snap %q has running apps or hooks", err.SnapInfo.InstanceName())
 	}
 }
 

overlord/snapstate/refresh_test.go

@@ -20,6 +20,10 @@
 package snapstate_test
 
 import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
 	. "gopkg.in/check.v1"
 
 	"github.com/snapcore/snapd/cmd/snaplock/runinhibit"
@@ -145,6 +149,26 @@ func (s *refreshSuite) TestHardNothingRunningRefreshCheck(c *C) {
 	c.Check(err.(*snapstate.BusySnapError).Pids(), DeepEquals, []int{105})
 }
 
+func (s *refreshSuite) TestPendingSnapRefreshInfo(c *C) {
+	err := snapstate.NewBusySnapError(s.info, nil, nil, nil)
+	refreshInfo := err.PendingSnapRefreshInfo()
+	c.Check(refreshInfo.InstanceName, Equals, s.info.InstanceName())
+	// The information about a busy app is not populated because
+	// the original error did not have the corresponding information.
+	c.Check(refreshInfo.BusyAppName, Equals, "")
+	c.Check(refreshInfo.BusyAppDesktopEntry, Equals, "")
+
+	// If we create a matching desktop entry then relevant meta-data is added.
+	err = snapstate.NewBusySnapError(s.info, nil, []string{"app"}, nil)
+	desktopFile := s.info.Apps["app"].DesktopFile()
+	c.Assert(os.MkdirAll(filepath.Dir(desktopFile), 0755), IsNil)
+	c.Assert(ioutil.WriteFile(desktopFile, nil, 0644), IsNil)
+	refreshInfo = err.PendingSnapRefreshInfo()
+	c.Check(refreshInfo.InstanceName, Equals, s.info.InstanceName())
+	c.Check(refreshInfo.BusyAppName, Equals, "app")
+	c.Check(refreshInfo.BusyAppDesktopEntry, Equals, "pkg_app")
+}
+
 func (s *refreshSuite) addInstalledSnap(snapst *snapstate.SnapState) (*snapstate.SnapState, *snap.Info) {
 	snapName := snapst.Sequence[0].RealName
 	snapstate.Set(s.state, snapName, snapst)
@@ -194,7 +218,7 @@ func (s *refreshSuite) TestDoSoftRefreshCheckDisallowed(c *C) {
 
 	// Pretend that snaps cannot refresh.
 	restore := snapstate.MockGenericRefreshCheck(func(info *snap.Info, canAppRunDuringRefresh func(app *snap.AppInfo) bool) error {
-		return &snapstate.BusySnapError{SnapName: info.InstanceName()}
+		return &snapstate.BusySnapError{SnapInfo: info}
 	})
 	defer restore()
 
@@ -245,7 +269,7 @@ func (s *refreshSuite) TestDoHardRefreshFlowRefreshDisallowed(c *C) {
 
 	// Pretend that snaps cannot refresh.
 	restore := snapstate.MockGenericRefreshCheck(func(info *snap.Info, canAppRunDuringRefresh func(app *snap.AppInfo) bool) error {
-		return &snapstate.BusySnapError{SnapName: info.InstanceName()}
+		return &snapstate.BusySnapError{SnapInfo: info}
 	})
 	defer restore()
 

usersession/agent/export_test.go

@@ -25,8 +25,9 @@ import (
 )
 
 var (
-	SessionInfoCmd    = sessionInfoCmd
-	ServiceControlCmd = serviceControlCmd
+	SessionInfoCmd                = sessionInfoCmd
+	ServiceControlCmd             = serviceControlCmd
+	PendingRefreshNotificationCmd = pendingRefreshNotificationCmd
 )
 
 func MockStopTimeouts(stop, kill time.Duration) (restore func()) {