Merge pull request #10877 from mvo5/system-restart-immediate-2.52

many: support an API flag system-restart-immediate to make snap ops proceed immediately with system restarts (2.52)

daemon/api_sideload_n_try.go

@@ -71,6 +71,7 @@ func sideloadOrTrySnap(c *Command, body io.ReadCloser, boundary string, user *au
 
 	flags.Unaliased = isTrue(form, "unaliased")
 	flags.IgnoreRunning = isTrue(form, "ignore-running")
+	systemRestartImmediate := isTrue(form, "system-restart-immediate")
 
 	// find the file for the "snap" form field
 	var snapBody multipart.File
@@ -192,6 +193,9 @@ out:
 	}
 
 	chg := newChange(st, "install-snap", msg, []*state.TaskSet{tset}, []string{instanceName})
+	if systemRestartImmediate {
+		chg.Set("system-restart-immediate", true)
+	}
 	chg.Set("api-data", map[string]string{"snap-name": instanceName})
 
 	ensureStateSoon(st)

daemon/api_sideload_n_try_test.go

@@ -79,8 +79,9 @@ func (s *sideloadSuite) TestSideloadSnapOnNonDevModeDistro(c *check.C) {
 	// try a multipart/form-data upload
 	body := sideLoadBodyWithoutDevMode
 	head := map[string]string{"Content-Type": "multipart/thing; boundary=--hello--"}
-	chgSummary := s.sideloadCheck(c, body, head, "local", snapstate.Flags{RemoveSnapPath: true})
+	chgSummary, systemRestartImmediate := s.sideloadCheck(c, body, head, "local", snapstate.Flags{RemoveSnapPath: true})
 	c.Check(chgSummary, check.Equals, `Install "local" snap from file "a/b/local.snap"`)
+	c.Check(systemRestartImmediate, check.Equals, false)
 }
 
 func (s *sideloadSuite) TestSideloadSnapOnDevModeDistro(c *check.C) {
@@ -90,7 +91,7 @@ func (s *sideloadSuite) TestSideloadSnapOnDevModeDistro(c *check.C) {
 	restore := sandbox.MockForceDevMode(true)
 	defer restore()
 	flags := snapstate.Flags{RemoveSnapPath: true}
-	chgSummary := s.sideloadCheck(c, body, head, "local", flags)
+	chgSummary, _ := s.sideloadCheck(c, body, head, "local", flags)
 	c.Check(chgSummary, check.Equals, `Install "local" snap from file "a/b/local.snap"`)
 }
 
@@ -109,7 +110,7 @@ func (s *sideloadSuite) TestSideloadSnapDevMode(c *check.C) {
 	// try a multipart/form-data upload
 	flags := snapstate.Flags{RemoveSnapPath: true}
 	flags.DevMode = true
-	chgSummary := s.sideloadCheck(c, body, head, "local", flags)
+	chgSummary, _ := s.sideloadCheck(c, body, head, "local", flags)
 	c.Check(chgSummary, check.Equals, `Install "local" snap from file "x"`)
 }
 
@@ -131,11 +132,11 @@ func (s *sideloadSuite) TestSideloadSnapJailMode(c *check.C) {
 	head := map[string]string{"Content-Type": "multipart/thing; boundary=--hello--"}
 	// try a multipart/form-data upload
 	flags := snapstate.Flags{JailMode: true, RemoveSnapPath: true}
-	chgSummary := s.sideloadCheck(c, body, head, "local", flags)
+	chgSummary, _ := s.sideloadCheck(c, body, head, "local", flags)
 	c.Check(chgSummary, check.Equals, `Install "local" snap from file "x"`)
 }
 
-func (s *sideloadSuite) sideloadCheck(c *check.C, content string, head map[string]string, expectedInstanceName string, expectedFlags snapstate.Flags) string {
+func (s *sideloadSuite) sideloadCheck(c *check.C, content string, head map[string]string, expectedInstanceName string, expectedFlags snapstate.Flags) (summary string, systemRestartImmediate bool) {
 	d := s.daemonWithFakeSnapManager(c)
 
 	soon := 0
@@ -214,7 +215,12 @@ func (s *sideloadSuite) sideloadCheck(c *check.C, content string, head map[strin
 		"snap-name": expectedInstanceName,
 	})
 
-	return chg.Summary()
+	summary = chg.Summary()
+	err = chg.Get("system-restart-immediate", &systemRestartImmediate)
+	if err != nil && err != state.ErrNoState {
+		c.Error(err)
+	}
+	return summary, systemRestartImmediate
 }
 
 func (s *sideloadSuite) TestSideloadSnapJailModeAndDevmode(c *check.C) {
@@ -420,7 +426,7 @@ func (s *sideloadSuite) TestSideloadSnapInstanceName(c *check.C) {
 		"local_instance\r\n" +
 		"----hello--\r\n"
 	head := map[string]string{"Content-Type": "multipart/thing; boundary=--hello--"}
-	chgSummary := s.sideloadCheck(c, body, head, "local_instance", snapstate.Flags{RemoveSnapPath: true})
+	chgSummary, _ := s.sideloadCheck(c, body, head, "local_instance", snapstate.Flags{RemoveSnapPath: true})
 	c.Check(chgSummary, check.Equals, `Install "local_instance" snap from file "a/b/local.snap"`)
 }
 
@@ -432,7 +438,7 @@ func (s *sideloadSuite) TestSideloadSnapInstanceNameNoKey(c *check.C) {
 		"local\r\n" +
 		"----hello--\r\n"
 	head := map[string]string{"Content-Type": "multipart/thing; boundary=--hello--"}
-	chgSummary := s.sideloadCheck(c, body, head, "local", snapstate.Flags{RemoveSnapPath: true})
+	chgSummary, _ := s.sideloadCheck(c, body, head, "local", snapstate.Flags{RemoveSnapPath: true})
 	c.Check(chgSummary, check.Equals, `Install "local" snap from file "a/b/local.snap"`)
 }
 
@@ -475,8 +481,31 @@ func (s *sideloadSuite) TestInstallPathUnaliased(c *check.C) {
 	head := map[string]string{"Content-Type": "multipart/thing; boundary=--hello--"}
 	// try a multipart/form-data upload
 	flags := snapstate.Flags{Unaliased: true, RemoveSnapPath: true, DevMode: true}
-	chgSummary := s.sideloadCheck(c, body, head, "local", flags)
+	chgSummary, _ := s.sideloadCheck(c, body, head, "local", flags)
+	c.Check(chgSummary, check.Equals, `Install "local" snap from file "x"`)
+}
+
+func (s *sideloadSuite) TestInstallPathSystemRestartImmediate(c *check.C) {
+	body := "" +
+		"----hello--\r\n" +
+		"Content-Disposition: form-data; name=\"snap\"; filename=\"x\"\r\n" +
+		"\r\n" +
+		"xyzzy\r\n" +
+		"----hello--\r\n" +
+		"Content-Disposition: form-data; name=\"devmode\"\r\n" +
+		"\r\n" +
+		"true\r\n" +
+		"----hello--\r\n" +
+		"Content-Disposition: form-data; name=\"system-restart-immediate\"\r\n" +
+		"\r\n" +
+		"true\r\n" +
+		"----hello--\r\n"
+	head := map[string]string{"Content-Type": "multipart/thing; boundary=--hello--"}
+	// try a multipart/form-data upload
+	flags := snapstate.Flags{RemoveSnapPath: true, DevMode: true}
+	chgSummary, systemRestartImmediate := s.sideloadCheck(c, body, head, "local", flags)
 	c.Check(chgSummary, check.Equals, `Install "local" snap from file "x"`)
+	c.Check(systemRestartImmediate, check.Equals, true)
 }
 
 type trySuite struct {

daemon/api_snaps.go

@@ -147,6 +147,9 @@ func postSnap(c *Command, r *http.Request, user *auth.UserState) Response {
 	}
 
 	chg := newChange(state, inst.Action+"-snap", msg, tsets, inst.Snaps)
+	if inst.SystemRestartImmediate {
+		chg.Set("system-restart-immediate", true)
+	}
 
 	ensureStateSoon(state)
 
@@ -186,15 +189,16 @@ type snapInstruction struct {
 	Action string `json:"action"`
 	Amend  bool   `json:"amend"`
 	snapRevisionOptions
-	DevMode          bool     `json:"devmode"`
-	JailMode         bool     `json:"jailmode"`
-	Classic          bool     `json:"classic"`
-	IgnoreValidation bool     `json:"ignore-validation"`
-	IgnoreRunning    bool     `json:"ignore-running"`
-	Unaliased        bool     `json:"unaliased"`
-	Purge            bool     `json:"purge,omitempty"`
-	Snaps            []string `json:"snaps"`
-	Users            []string `json:"users"`
+	DevMode                bool     `json:"devmode"`
+	JailMode               bool     `json:"jailmode"`
+	Classic                bool     `json:"classic"`
+	IgnoreValidation       bool     `json:"ignore-validation"`
+	IgnoreRunning          bool     `json:"ignore-running"`
+	Unaliased              bool     `json:"unaliased"`
+	Purge                  bool     `json:"purge,omitempty"`
+	SystemRestartImmediate bool     `json:"system-restart-immediate"`
+	Snaps                  []string `json:"snaps"`
+	Users                  []string `json:"users"`
 
 	// The fields below should not be unmarshalled into. Do not export them.
 	userID int
@@ -528,6 +532,10 @@ func snapOpMany(c *Command, r *http.Request, user *auth.UserState) Response {
 		ensureStateSoon(st)
 	}
 
+	if inst.SystemRestartImmediate {
+		chg.Set("system-restart-immediate", true)
+	}
+
 	chg.Set("api-data", map[string]interface{}{"snap-names": res.Affected})
 
 	return AsyncResponse(res.Result, chg.ID())

daemon/api_snaps_test.go

@@ -470,14 +470,21 @@ func (s *snapsSuite) TestPostSnapsNoWeirdses(c *check.C) {
 }
 
 func (s *snapsSuite) TestPostSnapsOp(c *check.C) {
-	s.testPostSnapsOp(c, "application/json")
+	systemRestartImmediate := s.testPostSnapsOp(c, "", "application/json")
+	c.Check(systemRestartImmediate, check.Equals, false)
 }
 
 func (s *snapsSuite) TestPostSnapsOpMoreComplexContentType(c *check.C) {
-	s.testPostSnapsOp(c, "application/json; charset=utf-8")
+	systemRestartImmediate := s.testPostSnapsOp(c, "", "application/json; charset=utf-8")
+	c.Check(systemRestartImmediate, check.Equals, false)
 }
 
-func (s *snapsSuite) testPostSnapsOp(c *check.C, contentType string) {
+func (s *snapsSuite) TestPostSnapsOpSystemRestartImmediate(c *check.C) {
+	systemRestartImmediate := s.testPostSnapsOp(c, `"system-restart-immediate": true`, "application/json")
+	c.Check(systemRestartImmediate, check.Equals, true)
+}
+
+func (s *snapsSuite) testPostSnapsOp(c *check.C, extraJSON, contentType string) (systemRestartImmediate bool) {
 	defer daemon.MockAssertstateRefreshSnapDeclarations(func(*state.State, int) error { return nil })()
 	defer daemon.MockSnapstateUpdateMany(func(_ context.Context, s *state.State, names []string, userID int, flags *snapstate.Flags) ([]string, []*state.TaskSet, error) {
 		c.Check(names, check.HasLen, 0)
@@ -487,7 +494,10 @@ func (s *snapsSuite) testPostSnapsOp(c *check.C, contentType string) {
 
 	d := s.daemonWithOverlordMockAndStore(c)
 
-	buf := bytes.NewBufferString(`{"action": "refresh"}`)
+	if extraJSON != "" {
+		extraJSON = "," + extraJSON
+	}
+	buf := bytes.NewBufferString(fmt.Sprintf(`{"action": "refresh"%s}`, extraJSON))
 	req, err := http.NewRequest("POST", "/v2/snaps", buf)
 	c.Assert(err, check.IsNil)
 	req.Header.Set("Content-Type", contentType)
@@ -502,6 +512,11 @@ func (s *snapsSuite) testPostSnapsOp(c *check.C, contentType string) {
 	var apiData map[string]interface{}
 	c.Check(chg.Get("api-data", &apiData), check.IsNil)
 	c.Check(apiData["snap-names"], check.DeepEquals, []interface{}{"fake1", "fake2"})
+	err = chg.Get("system-restart-immediate", &systemRestartImmediate)
+	if err != nil && err != state.ErrNoState {
+		c.Error(err)
+	}
+	return systemRestartImmediate
 }
 
 func (s *snapsSuite) TestPostSnapsOpInvalidCharset(c *check.C) {
@@ -1106,14 +1121,32 @@ func (s *snapsSuite) TestPostSnapBadChannel(c *check.C) {
 }
 
 func (s *snapsSuite) TestPostSnap(c *check.C) {
-	s.testPostSnap(c, false)
+	checkOpts := func(opts *snapstate.RevisionOptions) {
+		// no channel in -> no channel out
+		c.Check(opts.Channel, check.Equals, "")
+	}
+	summary, systemRestartImmediate := s.testPostSnap(c, "", checkOpts)
+	c.Check(summary, check.Equals, `Install "foo" snap`)
+	c.Check(systemRestartImmediate, check.Equals, false)
 }
 
 func (s *snapsSuite) TestPostSnapWithChannel(c *check.C) {
-	s.testPostSnap(c, true)
+	checkOpts := func(opts *snapstate.RevisionOptions) {
+		// channel in -> channel out
+		c.Check(opts.Channel, check.Equals, "xyzzy")
+	}
+	summary, systemRestartImmediate := s.testPostSnap(c, `"channel": "xyzzy"`, checkOpts)
+	c.Check(summary, check.Equals, `Install "foo" snap from "xyzzy" channel`)
+	c.Check(systemRestartImmediate, check.Equals, false)
+}
+
+func (s *snapsSuite) TestPostSnapSystemRestartImmediate(c *check.C) {
+	checkOpts := func(opts *snapstate.RevisionOptions) {}
+	_, systemRestartImmediate := s.testPostSnap(c, `"system-restart-immediate": true`, checkOpts)
+	c.Check(systemRestartImmediate, check.Equals, true)
 }
 
-func (s *snapsSuite) testPostSnap(c *check.C, withChannel bool) {
+func (s *snapsSuite) testPostSnap(c *check.C, extraJSON string, checkOpts func(opts *snapstate.RevisionOptions)) (summary string, systemRestartImmediate bool) {
 	d := s.daemonWithOverlordMock(c)
 
 	soon := 0
@@ -1126,25 +1159,17 @@ func (s *snapsSuite) testPostSnap(c *check.C, withChannel bool) {
 
 	checked := false
 	defer daemon.MockSnapstateInstall(func(ctx context.Context, s *state.State, name string, opts *snapstate.RevisionOptions, userID int, flags snapstate.Flags) (*state.TaskSet, error) {
-		if withChannel {
-			// channel in -> channel out
-			c.Check(opts.Channel, check.Equals, "xyzzy")
-		} else {
-			// no channel in -> no channel out
-			c.Check(opts.Channel, check.Equals, "")
-		}
+		checkOpts(opts)
 		checked = true
-
 		t := s.NewTask("fake-install-snap", "Doing a fake install")
 		return state.NewTaskSet(t), nil
 	})()
 
 	var buf *bytes.Buffer
-	if withChannel {
-		buf = bytes.NewBufferString(`{"action": "install", "channel": "xyzzy"}`)
-	} else {
-		buf = bytes.NewBufferString(`{"action": "install"}`)
+	if extraJSON != "" {
+		extraJSON = "," + extraJSON
 	}
+	buf = bytes.NewBufferString(fmt.Sprintf(`{"action": "install"%s}`, extraJSON))
 	req, err := http.NewRequest("POST", "/v2/snaps/foo", buf)
 	c.Assert(err, check.IsNil)
 
@@ -1155,11 +1180,7 @@ func (s *snapsSuite) testPostSnap(c *check.C, withChannel bool) {
 	defer st.Unlock()
 	chg := st.Change(rsp.Change)
 	c.Assert(chg, check.NotNil)
-	if withChannel {
-		c.Check(chg.Summary(), check.Equals, `Install "foo" snap from "xyzzy" channel`)
-	} else {
-		c.Check(chg.Summary(), check.Equals, `Install "foo" snap`)
-	}
+
 	var names []string
 	err = chg.Get("snap-names", &names)
 	c.Assert(err, check.IsNil)
@@ -1168,6 +1189,13 @@ func (s *snapsSuite) testPostSnap(c *check.C, withChannel bool) {
 	c.Check(checked, check.Equals, true)
 	c.Check(soon, check.Equals, 1)
 	c.Check(chg.Tasks()[0].Summary(), check.Equals, "Doing a fake install")
+
+	summary = chg.Summary()
+	err = chg.Get("system-restart-immediate", &systemRestartImmediate)
+	if err != nil && err != state.ErrNoState {
+		c.Error(err)
+	}
+	return summary, systemRestartImmediate
 }
 
 func (s *snapsSuite) TestPostSnapVerifySnapInstruction(c *check.C) {

overlord/devicestate/devicestate_bootconfig_test.go

@@ -105,6 +105,7 @@ func (s *deviceMgrBootconfigSuite) testBootConfigUpdateRun(c *C, updateAttempted
 	tsk := s.state.NewTask("update-managed-boot-config", "update boot config")
 	chg := s.state.NewChange("dummy", "...")
 	chg.AddTask(tsk)
+	chg.Set("system-restart-immediate", true)
 	s.state.Unlock()
 
 	s.settle(c)
@@ -128,7 +129,7 @@ func (s *deviceMgrBootconfigSuite) testBootConfigUpdateRun(c *C, updateAttempted
 			c.Assert(log, HasLen, 1)
 			c.Check(log[0], Matches, ".* updated boot config assets")
 			// update was applied, thus a restart was requested
-			c.Check(s.restartRequests, DeepEquals, []state.RestartType{state.RestartSystem})
+			c.Check(s.restartRequests, DeepEquals, []state.RestartType{state.RestartSystemNow})
 		} else {
 			// update was not applied or failed
 			c.Check(s.restartRequests, HasLen, 0)