From 0955d890959a44831729b27e5358c659da2f1642 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Thu, 14 May 2020 16:20:28 +0200 Subject: [PATCH 1/7] o/devicestate: tweak the list of actions supported by recovery mode Tweak the system actions to allow recovery mode system to enter reinstall or run modes. Signed-off-by: Maciej Borzecki --- overlord/devicestate/devicemgr.go | 22 ++++++- .../devicestate/devicestate_systems_test.go | 63 +++++++------------ 2 files changed, 43 insertions(+), 42 deletions(-) diff --git a/overlord/devicestate/devicemgr.go b/overlord/devicestate/devicemgr.go index 40117fa580f..dc95c2df9e4 100644 --- a/overlord/devicestate/devicemgr.go +++ b/overlord/devicestate/devicemgr.go @@ -772,6 +772,10 @@ var currentSystemActions = []SystemAction{ {Title: "Recover", Mode: "recover"}, {Title: "Run normally", Mode: "run"}, } +var recoverSystemActions = []SystemAction{ + {Title: "Reinstall", Mode: "install"}, + {Title: "Run normally", Mode: "run"}, +} func systemFromSeed(label string) (*System, error) { s, err := seed.Open(dirs.SnapSeedDir, label) @@ -864,6 +868,19 @@ func isCurrentSystem(current *seededSystem, other *System) bool { current.BrandID == other.Brand.AccountID() } +func currentSystemActionsForMode(mode string) []SystemAction { + switch mode { + case "install": + // there should be no current system in install mode + return nil + case "run": + return currentSystemActions + case "recover": + return recoverSystemActions + } + return nil +} + // Systems list the available recovery/seeding systems. Returns the list of // systems, ErrNoSystems when no systems seeds were found or other error. func (m *DeviceManager) Systems() ([]*System, error) { @@ -891,7 +908,7 @@ func (m *DeviceManager) Systems() ([]*System, error) { } if currentSys != nil && isCurrentSystem(currentSys, system) { system.Current = true - system.Actions = currentSystemActions + system.Actions = currentSystemActionsForMode(m.systemMode) } systems = append(systems, system) } @@ -915,8 +932,7 @@ func (m *DeviceManager) RequestSystemAction(systemLabel string, action SystemAct return fmt.Errorf("cannot load seed system: %v", err) } if currentSys != nil && isCurrentSystem(currentSys, system) { - system.Current = true - system.Actions = currentSystemActions + system.Actions = currentSystemActionsForMode(m.systemMode) } var sysAction *SystemAction diff --git a/overlord/devicestate/devicestate_systems_test.go b/overlord/devicestate/devicestate_systems_test.go index 50a8b53f5dc..71356e80366 100644 --- a/overlord/devicestate/devicestate_systems_test.go +++ b/overlord/devicestate/devicestate_systems_test.go @@ -38,7 +38,6 @@ import ( "github.com/snapcore/snapd/seed/seedtest" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" - "github.com/snapcore/snapd/strutil" ) type mockedSystemSeed struct { @@ -324,7 +323,7 @@ func (s *deviceMgrSystemsSuite) TestRequestModeInstallHappyForAny(c *C) { c.Check(s.restartRequests, DeepEquals, []state.RestartType{state.RestartSystemNow}) } -func (s *deviceMgrSystemsSuite) TestRequestSameModeSameSystem(c *C) { +func (s *deviceMgrSystemsSuite) TestRequestRunToRun(c *C) { s.state.Lock() s.state.Set("seeded-systems", []devicestate.SeededSystem{ { @@ -336,44 +335,30 @@ func (s *deviceMgrSystemsSuite) TestRequestSameModeSameSystem(c *C) { s.state.Unlock() label := s.mockedSystemSeeds[0].label - - happyModes := []string{"run", "recover"} - sadModes := []string{"install"} - - for _, mode := range append(happyModes, sadModes...) { - c.Logf("checking mode: %q", mode) - // non run modes use modeenv - modeenv := boot.Modeenv{ - Mode: mode, - } - if mode != "run" { - modeenv.RecoverySystem = s.mockedSystemSeeds[0].label - } - err := modeenv.WriteTo("") - c.Assert(err, IsNil) - - devicestate.SetSystemMode(s.mgr, mode) - err = s.bootloader.SetBootVars(map[string]string{ - "snapd_recovery_mode": mode, - "snapd_recovery_system": label, - }) - c.Assert(err, IsNil) - err = s.mgr.RequestSystemAction(label, devicestate.SystemAction{Mode: mode}) - if strutil.ListContains(sadModes, mode) { - c.Assert(err, Equals, devicestate.ErrUnsupportedAction) - } else { - c.Assert(err, IsNil) - } - // bootloader vars shouldn't change - m, err := s.bootloader.GetBootVars("snapd_recovery_mode", "snapd_recovery_system") - c.Assert(err, IsNil) - c.Check(m, DeepEquals, map[string]string{ - "snapd_recovery_mode": mode, - "snapd_recovery_system": label, - }) - // should never restart - c.Check(s.restartRequests, HasLen, 0) + // non run modes use modeenv + modeenv := boot.Modeenv{ + Mode: "run", } + err := modeenv.WriteTo("") + c.Assert(err, IsNil) + + devicestate.SetSystemMode(s.mgr, "run") + err = s.bootloader.SetBootVars(map[string]string{ + "snapd_recovery_mode": "", + "snapd_recovery_system": "", + }) + c.Assert(err, IsNil) + err = s.mgr.RequestSystemAction(label, devicestate.SystemAction{Mode: "run"}) + c.Assert(err, IsNil) + // bootloader vars shouldn't change + m, err := s.bootloader.GetBootVars("snapd_recovery_mode", "snapd_recovery_system") + c.Assert(err, IsNil) + c.Check(m, DeepEquals, map[string]string{ + "snapd_recovery_mode": "", + "snapd_recovery_system": "", + }) + // should never restart + c.Check(s.restartRequests, HasLen, 0) } func (s *deviceMgrSystemsSuite) TestRequestNotYetSeeded(c *C) { From 11ae8ebc5b2c4cbbd9995d0dc55b5b69e894f113 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Thu, 14 May 2020 18:15:49 +0200 Subject: [PATCH 2/7] daemon: recover to recover is no longer supported Signed-off-by: Maciej Borzecki --- daemon/api_systems_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/daemon/api_systems_test.go b/daemon/api_systems_test.go index a433056afed..4b3fb9c5b20 100644 --- a/daemon/api_systems_test.go +++ b/daemon/api_systems_test.go @@ -349,9 +349,10 @@ func (s *apiSuite) TestSystemActionRequestWithSeeded(c *check.C) { }, { // from recover mode -> recover mode is no-op - currentMode: "recover", - actionMode: "recover", - comment: "recover mode to recover mode", + currentMode: "recover", + actionMode: "recover", + expUnsupported: true, + comment: "recover mode to recover mode", }, { // from install mode -> install mode is no-no From ac040e25637e7a7eb9b0d6294cfb5274098d99e6 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Mon, 18 May 2020 13:58:39 +0200 Subject: [PATCH 3/7] tests/core/uc20-recovery: update system actions in recover mode check Signed-off-by: Maciej Borzecki --- tests/core/uc20-recovery/task.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/core/uc20-recovery/task.yaml b/tests/core/uc20-recovery/task.yaml index f2ce6992237..21a8ef89b8f 100644 --- a/tests/core/uc20-recovery/task.yaml +++ b/tests/core/uc20-recovery/task.yaml @@ -86,7 +86,8 @@ execute: | MATCH 'snapd_recovery_mode=recover' < /proc/cmdline # verify we are in recovery mode via the API test-snapd-curl.curl -s --unix-socket /run/snapd.socket http://localhost/v2/system-info > system-info - jq -r '.result["system-mode"]' < system-info | MATCH 'recover' + jq -r '.result["system-mode"]' < system-info | not MATCH 'recover' + jq -r '.result["system-mode"]' < system-info | MATCH 'run' # host data should be accessible test -e /host/ubuntu-data/systems.json.run From 17bda66e5326dfa4186edc7f183f6d30ec447271 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Tue, 19 May 2020 14:27:53 +0200 Subject: [PATCH 4/7] o/devicestate: restore and update same-mode system action unit test Signed-off-by: Maciej Borzecki --- .../devicestate/devicestate_systems_test.go | 63 ++++++++++++------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/overlord/devicestate/devicestate_systems_test.go b/overlord/devicestate/devicestate_systems_test.go index 71356e80366..28bcb2357ff 100644 --- a/overlord/devicestate/devicestate_systems_test.go +++ b/overlord/devicestate/devicestate_systems_test.go @@ -38,6 +38,7 @@ import ( "github.com/snapcore/snapd/seed/seedtest" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" + "github.com/snapcore/snapd/strutil" ) type mockedSystemSeed struct { @@ -323,7 +324,7 @@ func (s *deviceMgrSystemsSuite) TestRequestModeInstallHappyForAny(c *C) { c.Check(s.restartRequests, DeepEquals, []state.RestartType{state.RestartSystemNow}) } -func (s *deviceMgrSystemsSuite) TestRequestRunToRun(c *C) { +func (s *deviceMgrSystemsSuite) TestRequestSameModeSameSystem(c *C) { s.state.Lock() s.state.Set("seeded-systems", []devicestate.SeededSystem{ { @@ -335,30 +336,44 @@ func (s *deviceMgrSystemsSuite) TestRequestRunToRun(c *C) { s.state.Unlock() label := s.mockedSystemSeeds[0].label - // non run modes use modeenv - modeenv := boot.Modeenv{ - Mode: "run", - } - err := modeenv.WriteTo("") - c.Assert(err, IsNil) - devicestate.SetSystemMode(s.mgr, "run") - err = s.bootloader.SetBootVars(map[string]string{ - "snapd_recovery_mode": "", - "snapd_recovery_system": "", - }) - c.Assert(err, IsNil) - err = s.mgr.RequestSystemAction(label, devicestate.SystemAction{Mode: "run"}) - c.Assert(err, IsNil) - // bootloader vars shouldn't change - m, err := s.bootloader.GetBootVars("snapd_recovery_mode", "snapd_recovery_system") - c.Assert(err, IsNil) - c.Check(m, DeepEquals, map[string]string{ - "snapd_recovery_mode": "", - "snapd_recovery_system": "", - }) - // should never restart - c.Check(s.restartRequests, HasLen, 0) + happyModes := []string{"run"} + sadModes := []string{"install", "recover"} + + for _, mode := range append(happyModes, sadModes...) { + c.Logf("checking mode: %q", mode) + // non run modes use modeenv + modeenv := boot.Modeenv{ + Mode: mode, + } + if mode != "run" { + modeenv.RecoverySystem = s.mockedSystemSeeds[0].label + } + err := modeenv.WriteTo("") + c.Assert(err, IsNil) + + devicestate.SetSystemMode(s.mgr, mode) + err = s.bootloader.SetBootVars(map[string]string{ + "snapd_recovery_mode": mode, + "snapd_recovery_system": label, + }) + c.Assert(err, IsNil) + err = s.mgr.RequestSystemAction(label, devicestate.SystemAction{Mode: mode}) + if strutil.ListContains(sadModes, mode) { + c.Assert(err, Equals, devicestate.ErrUnsupportedAction) + } else { + c.Assert(err, IsNil) + } + // bootloader vars shouldn't change + m, err := s.bootloader.GetBootVars("snapd_recovery_mode", "snapd_recovery_system") + c.Assert(err, IsNil) + c.Check(m, DeepEquals, map[string]string{ + "snapd_recovery_mode": mode, + "snapd_recovery_system": label, + }) + // should never restart + c.Check(s.restartRequests, HasLen, 0) + } } func (s *deviceMgrSystemsSuite) TestRequestNotYetSeeded(c *C) { From c0a9eefcd7d6b1193d11b632a61db85fb4f60c21 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Tue, 19 May 2020 14:33:28 +0200 Subject: [PATCH 5/7] o/devicestate: cleanup common test scenarios, check reinstall from different seed Signed-off-by: Maciej Borzecki --- .../devicestate/devicestate_systems_test.go | 72 +++++++++++-------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/overlord/devicestate/devicestate_systems_test.go b/overlord/devicestate/devicestate_systems_test.go index 28bcb2357ff..21dc971799d 100644 --- a/overlord/devicestate/devicestate_systems_test.go +++ b/overlord/devicestate/devicestate_systems_test.go @@ -393,6 +393,24 @@ func (s *deviceMgrSystemsSuite) TestRequestNotYetSeeded(c *C) { } } +func (s *deviceMgrSystemsSuite) testRequestModeWithRestart(c *C, toModes []string, label string) { + for _, mode := range toModes { + c.Logf("checking mode: %q", mode) + err := s.mgr.RequestSystemAction(label, + devicestate.SystemAction{Mode: mode}) + c.Assert(err, IsNil) + m, err := s.bootloader.GetBootVars("snapd_recovery_mode", "snapd_recovery_system") + c.Assert(err, IsNil) + c.Check(m, DeepEquals, map[string]string{ + "snapd_recovery_system": label, + "snapd_recovery_mode": mode, + }) + c.Check(s.restartRequests, DeepEquals, []state.RestartType{state.RestartSystemNow}) + s.restartRequests = nil + s.bootloader.BootVars = map[string]string{} + } +} + func (s *deviceMgrSystemsSuite) TestRequestModeRunInstallForRecover(c *C) { // we are in recover mode here devicestate.SetSystemMode(s.mgr, "recover") @@ -414,21 +432,7 @@ func (s *deviceMgrSystemsSuite) TestRequestModeRunInstallForRecover(c *C) { }) s.state.Unlock() - for _, mode := range []string{"install", "run"} { - c.Logf("checking mode: %q", mode) - err := s.mgr.RequestSystemAction(s.mockedSystemSeeds[0].label, - devicestate.SystemAction{Mode: mode}) - c.Assert(err, IsNil) - m, err := s.bootloader.GetBootVars("snapd_recovery_mode", "snapd_recovery_system") - c.Assert(err, IsNil) - c.Check(m, DeepEquals, map[string]string{ - "snapd_recovery_system": s.mockedSystemSeeds[0].label, - "snapd_recovery_mode": mode, - }) - c.Check(s.restartRequests, DeepEquals, []state.RestartType{state.RestartSystemNow}) - s.restartRequests = nil - s.bootloader.BootVars = map[string]string{} - } + s.testRequestModeWithRestart(c, []string{"install", "run"}, s.mockedSystemSeeds[0].label) } func (s *deviceMgrSystemsSuite) TestRequestModeInstallRecoverForCurrent(c *C) { @@ -450,21 +454,7 @@ func (s *deviceMgrSystemsSuite) TestRequestModeInstallRecoverForCurrent(c *C) { }) s.state.Unlock() - for _, mode := range []string{"install", "recover"} { - c.Logf("checking mode: %q", mode) - err := s.mgr.RequestSystemAction(s.mockedSystemSeeds[0].label, - devicestate.SystemAction{Mode: mode}) - c.Assert(err, IsNil) - m, err := s.bootloader.GetBootVars("snapd_recovery_mode", "snapd_recovery_system") - c.Assert(err, IsNil) - c.Check(m, DeepEquals, map[string]string{ - "snapd_recovery_system": s.mockedSystemSeeds[0].label, - "snapd_recovery_mode": mode, - }) - c.Check(s.restartRequests, DeepEquals, []state.RestartType{state.RestartSystemNow}) - s.restartRequests = nil - s.bootloader.BootVars = map[string]string{} - } + s.testRequestModeWithRestart(c, []string{"install", "recover"}, s.mockedSystemSeeds[0].label) } func (s *deviceMgrSystemsSuite) TestRequestModeErrInBoot(c *C) { @@ -523,3 +513,25 @@ func (s *deviceMgrSystemsSuite) TestRequestModeForNonCurrent(c *C) { c.Assert(err, Equals, devicestate.ErrUnsupportedAction) c.Check(s.restartRequests, HasLen, 0) } + +func (s *deviceMgrSystemsSuite) TestRequestInstallForOther(c *C) { + devicestate.SetSystemMode(s.mgr, "run") + // non run modes use modeenv + modeenv := boot.Modeenv{ + Mode: "run", + } + err := modeenv.WriteTo("") + c.Assert(err, IsNil) + + s.state.Lock() + s.state.Set("seeded-systems", []devicestate.SeededSystem{ + { + System: s.mockedSystemSeeds[0].label, + Model: s.mockedSystemSeeds[0].model.Model(), + BrandID: s.mockedSystemSeeds[0].brand.AccountID(), + }, + }) + s.state.Unlock() + // reinstall from different system seed is ok + s.testRequestModeWithRestart(c, []string{"install"}, s.mockedSystemSeeds[1].label) +} From e2421c980570d573281a87ab7be2387734916513 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Tue, 19 May 2020 14:48:50 +0200 Subject: [PATCH 6/7] tests/core/uc20-recovery: update test Signed-off-by: Maciej Borzecki --- tests/core/uc20-recovery/task.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/core/uc20-recovery/task.yaml b/tests/core/uc20-recovery/task.yaml index 21a8ef89b8f..b4f98290d95 100644 --- a/tests/core/uc20-recovery/task.yaml +++ b/tests/core/uc20-recovery/task.yaml @@ -14,6 +14,9 @@ debug: | if [ -e systems.json ]; then cat systems.json fi + if [ -e system-info ]; then + cat system-info + fi if [ -e /tmp/mock-shutdown.calls ]; then cat /tmp/mock-shutdown.calls fi @@ -86,8 +89,7 @@ execute: | MATCH 'snapd_recovery_mode=recover' < /proc/cmdline # verify we are in recovery mode via the API test-snapd-curl.curl -s --unix-socket /run/snapd.socket http://localhost/v2/system-info > system-info - jq -r '.result["system-mode"]' < system-info | not MATCH 'recover' - jq -r '.result["system-mode"]' < system-info | MATCH 'run' + jq -r '.result["system-mode"]' < system-info | MATCH 'recover' # host data should be accessible test -e /host/ubuntu-data/systems.json.run From bf59e748ed993d2c749e853960852c8305204a15 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Tue, 19 May 2020 15:23:34 +0200 Subject: [PATCH 7/7] tests/core/uc20-recovery: verify system actions in run/recovery different modes Signed-off-by: Maciej Borzecki --- tests/core/uc20-recovery/task.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/core/uc20-recovery/task.yaml b/tests/core/uc20-recovery/task.yaml index b4f98290d95..08abb5af808 100644 --- a/tests/core/uc20-recovery/task.yaml +++ b/tests/core/uc20-recovery/task.yaml @@ -42,7 +42,7 @@ execute: | test -n "$label" # make sure that the seed exists test -d "/var/lib/snapd/seed/systems/$label" - jq -r .result.systems[0].actions[].mode < systems.json | MATCH 'recover' + jq -r .result.systems[0].actions[].mode < systems.json | sort | tr '\n' ' ' | MATCH 'install recover run' # keep a copy of the systems dump for later reference cp systems.json /writable/systems.json.run @@ -95,8 +95,8 @@ execute: | test -e /host/ubuntu-data/systems.json.run test-snapd-curl.curl -s --unix-socket /run/snapd.socket http://localhost/v2/systems > systems.json - # systems list should be identical - diff -up /host/ubuntu-data/systems.json.run systems.json + jq -r .result.systems[0].actions[].mode < systems.json | sort | tr '\n' ' ' | MATCH 'install run' + label="$(cat /host/ubuntu-data/systems.label)" test -n "$label" # seed in mounted in recover mode too