many: support 'ignore-running' refresh-mode for apps

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

overlord/snapstate/check_snap.go

@@ -51,6 +51,8 @@ var featureSet = map[string]bool{
 	// Support for "kernel-assets" in gadget.yaml. I.e. having volume
 	// content of the style $kernel:ref`
 	"kernel-assets": true,
+	// Support for "refresh-mode: ignore-running" in snap.yaml
+	"app-refresh-mode": true,
 }
 
 func checkAssumes(si *snap.Info) error {

overlord/snapstate/refresh.go

@@ -93,8 +93,8 @@ var genericRefreshCheck = func(info *snap.Info, canAppRunDuringRefresh func(app
 //
 // The check is designed to run early in the refresh pipeline. Before
 // downloading or stopping services for the update, we can check that only
-// services are running, that is, that no non-service apps or hooks are
-// currently running.
+// services or snaps that explicitly allow refreshes are running, that is,
+// that no non-service apps or hooks are currently running.
 //
 // Since services are stopped during the update this provides a good early
 // precondition check. The check is also deliberately racy as existing snap
@@ -106,7 +106,7 @@ var genericRefreshCheck = func(info *snap.Info, canAppRunDuringRefresh func(app
 // block refresh to continue executing.
 func SoftNothingRunningRefreshCheck(info *snap.Info) error {
 	return genericRefreshCheck(info, func(app *snap.AppInfo) bool {
-		return app.IsService() || app.RefreshAllowed
+		return app.IsService() || app.RefreshMode == "ignore-running"
 	})
 }
 
@@ -118,14 +118,14 @@ func SoftNothingRunningRefreshCheck(info *snap.Info) error {
 // externally (e.g. by using a new inhibition mechanism for snap run).
 //
 // The check fails if any process belonging to the snap, apart from services
-// that are enduring refresh, is still alive. If a snap is busy it cannot be
-// refreshed and the refresh process is aborted.
+// and snaps that explicitly allow refreshes, is still alive. If a snap is busy
+// it cannot be refreshed and the refresh process is aborted.
 //
 // Apart from ignoring services, the check allows apps that want not to
 // block refresh to continue executing.
 func HardNothingRunningRefreshCheck(info *snap.Info) error {
 	return genericRefreshCheck(info, func(app *snap.AppInfo) bool {
-		return app.IsService() || app.RefreshAllowed
+		return app.IsService() || app.RefreshMode == "ignore-running"
 	})
 }
 

overlord/snapstate/refresh_test.go

@@ -90,6 +90,12 @@ func (s *refreshSuite) TestSoftNothingRunningRefreshCheck(c *C) {
 	c.Check(err.Error(), Equals, `snap "pkg" has running apps (app), pids: 101`)
 	c.Check(err.(*snapstate.BusySnapError).Pids(), DeepEquals, []int{101})
 
+	// Unless refresh-mode is set to "ignore-running"
+	s.info.Apps["app"].RefreshMode = "ignore-running"
+	err = snapstate.SoftNothingRunningRefreshCheck(s.info)
+	c.Assert(err, IsNil)
+	s.info.Apps["app"].RefreshMode = ""
+
 	// Hooks behave just like apps. IDEA: perhaps hooks should not be
 	// killed this way? They have their own life-cycle management.
 	s.pids = map[string][]int{
@@ -136,6 +142,12 @@ func (s *refreshSuite) TestHardNothingRunningRefreshCheck(c *C) {
 	c.Assert(err, FitsTypeOf, &snapstate.BusySnapError{})
 	c.Check(err.(*snapstate.BusySnapError).Pids(), DeepEquals, []int{101})
 
+	// Unless refresh-mode is set to "ignore-running"
+	s.info.Apps["app"].RefreshMode = "ignore-running"
+	err = snapstate.HardNothingRunningRefreshCheck(s.info)
+	c.Assert(err, IsNil)
+	s.info.Apps["app"].RefreshMode = ""
+
 	// Hooks are equally blocking hard refresh check.
 	s.pids = map[string][]int{
 		"snap.pkg.hook.configure": {105},

sandbox/cgroup/scanning.go

@@ -70,7 +70,7 @@ func securityTagFromCgroupPath(path string) naming.SecurityTag {
 // PidsOfSnap returns the association of security tags to PIDs.
 //
 // NOTE: This function returns a reliable result only if the refresh-app-awareness
-// feature was enabled since all processes related to the given snap were started.
+// feature was enabled before all processes related to the given snap were started.
 // If the feature wasn't always enabled then only service process are correctly
 // accounted for.
 //

snap/info.go

@@ -943,7 +943,6 @@ type AppInfo struct {
 	RestartDelay    timeout.Timeout
 	Completer       string
 	RefreshMode     string
-	RefreshAllowed  bool
 	StopMode        StopModeType
 	InstallMode     string
 

snap/info_snap_yaml.go

@@ -84,7 +84,6 @@ type appYaml struct {
 	WatchdogTimeout timeout.Timeout `yaml:"watchdog-timeout,omitempty"`
 	Completer       string          `yaml:"completer,omitempty"`
 	RefreshMode     string          `yaml:"refresh-mode,omitempty"`
-	RefreshAllowed  bool            `yaml:"refresh-allowed,omitempty"`
 	StopMode        StopModeType    `yaml:"stop-mode,omitempty"`
 	InstallMode     string          `yaml:"install-mode,omitempty"`
 
@@ -372,7 +371,6 @@ func setAppsFromSnapYaml(y snapYaml, snap *Info, strk *scopedTracker) error {
 			Completer:       yApp.Completer,
 			StopMode:        yApp.StopMode,
 			RefreshMode:     yApp.RefreshMode,
-			RefreshAllowed:  yApp.RefreshAllowed,
 			InstallMode:     yApp.InstallMode,
 			Before:          yApp.Before,
 			After:           yApp.After,

snap/validate.go

@@ -788,7 +788,7 @@ func ValidateApp(app *AppInfo) error {
 	}
 	// validate refresh-mode
 	switch app.RefreshMode {
-	case "", "endure", "restart":
+	case "", "endure", "restart", "ignore-running":
 		// valid
 	default:
 		return fmt.Errorf(`"refresh-mode" field contains invalid value %q`, app.RefreshMode)
@@ -803,8 +803,12 @@ func ValidateApp(app *AppInfo) error {
 	if app.StopMode != "" && app.Daemon == "" {
 		return fmt.Errorf(`"stop-mode" cannot be used for %q, only for services`, app.Name)
 	}
-	if app.RefreshMode != "" && app.Daemon == "" {
-		return fmt.Errorf(`"refresh-mode" cannot be used for %q, only for services`, app.Name)
+	if app.RefreshMode != "" {
+		if app.Daemon != "" && app.RefreshMode == "ignore-running" {
+			return errors.New(`"refresh-mode" cannot be set to "ignore-running" for services`)
+		} else if app.Daemon == "" && app.RefreshMode != "ignore-running" {
+			return fmt.Errorf(`"refresh-mode" for app %q can only have value "ignore-running"`, app.Name)
+		}
 	}
 	if app.InstallMode != "" && app.Daemon == "" {
 		return fmt.Errorf(`"install-mode" cannot be used for %q, only for services`, app.Name)

snap/validate_test.go

@@ -539,26 +539,32 @@ func (s *ValidateSuite) TestAppRefreshMode(c *C) {
 	// check services
 	for _, t := range []struct {
 		refreshMode string
-		ok          bool
+		daemon      string
+		errMsg      string
 	}{
 		// good
-		{"", true},
-		{"endure", true},
-		{"restart", true},
+		{"", "simple", ""},
+		{"endure", "simple", ""},
+		{"restart", "simple", ""},
+		{"ignore-running", "", ""},
 		// bad
-		{"invalid-thing", false},
+		{"invalid-thing", "simple", `"refresh-mode" field contains invalid value "invalid-thing"`},
+		{"endure", "", `"refresh-mode" for app "foo" can only have value "ignore-running"`},
+		{"restart", "", `"refresh-mode" for app "foo" can only have value "ignore-running"`},
+		{"ignore-running", "simple", `"refresh-mode" cannot be set to "ignore-running" for services`},
 	} {
-		err := ValidateApp(&AppInfo{Name: "foo", Daemon: "simple", DaemonScope: SystemDaemon, RefreshMode: t.refreshMode})
-		if t.ok {
+		var daemonScope DaemonScope
+		if t.daemon != "" {
+			daemonScope = SystemDaemon
+		}
+
+		err := ValidateApp(&AppInfo{Name: "foo", Daemon: t.daemon, DaemonScope: daemonScope, RefreshMode: t.refreshMode})
+		if t.errMsg == "" {
 			c.Check(err, IsNil)
 		} else {
-			c.Check(err, ErrorMatches, fmt.Sprintf(`"refresh-mode" field contains invalid value %q`, t.refreshMode))
+			c.Check(err, ErrorMatches, t.errMsg)
 		}
 	}
-
-	// non-services cannot have a refresh-mode
-	err := ValidateApp(&AppInfo{Name: "foo", Daemon: "", RefreshMode: "endure"})
-	c.Check(err, ErrorMatches, `"refresh-mode" cannot be used for "foo", only for services`)
 }
 
 func (s *ValidateSuite) TestAppWhitelistError(c *C) {

tests/main/refresh-mode-ignore-running/task.yaml

@@ -0,0 +1,30 @@
+summary: apps can request not to be tracked
+
+systems: [-ubuntu-14.04-*]
+
+prepare: |
+  snap set system experimental.refresh-app-awareness=true
+  snap pack test-snapd-refresh
+  snap install --dangerous ./test-snapd-refresh_1_all.snap
+
+restore: |
+  snap remove --purge test-snapd-refresh
+  rm -f test-snapd-refresh_1.all.snap
+  snap unset system experimental.refresh-app-awareness
+
+execute: |
+  tests.cleanup defer rm -f stamp
+
+  trap 'touch stamp' EXIT
+
+  test-snapd-refresh.sh -c "while [ ! -e stamp ]; do sleep 1; done" &
+  not snap install --dangerous ./test-snapd-refresh_1_all.snap
+  touch stamp
+  wait
+
+  rm -f stamp
+
+  test-snapd-refresh.refresh-allowed-sh -c "while [ ! -e stamp ]; do sleep 1; done" &
+  snap install --dangerous ./test-snapd-refresh_1_all.snap
+  touch stamp
+  wait

tests/regression/lp-1978005/test-snapd-refresh/meta/snap.yaml → tests/main/refresh-mode-ignore-running/test-snapd-refresh/meta/snap.yaml

@@ -1,9 +1,10 @@
 name: test-snapd-refresh
 version: 1
 architectures: [all]
+assumes: [app-refresh-mode]
 apps:
   sh:
     command: bin/sh
   refresh-allowed-sh:
     command: bin/sh
-    refresh-allowed: true
+    refresh-mode: ignore-running