New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
o/devicestate: cleanup system actions supported by recover mode #8686
o/devicestate: cleanup system actions supported by recover mode #8686
Conversation
Tweak the system actions to allow recovery mode system to enter reinstall or run modes. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one comment not to address here, one about the test situation
@@ -891,7 +908,7 @@ func (m *DeviceManager) Systems() ([]*System, error) { | |||
} | |||
if currentSys != nil && isCurrentSystem(currentSys, system) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still feel this should actually be integrated back in systemFromSeed by passing all the needed info in, but can be done when moving it to systems.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, we'll do this after #8689 lands.
@@ -335,40 +335,38 @@ func (s *deviceMgrSystemsSuite) TestRequestSameModeSameSystem(c *C) { | |||
s.state.Unlock() | |||
|
|||
label := s.mockedSystemSeeds[0].label | |||
|
|||
happyModes := []string{"run", "recover"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we have just moved recover to sadModes, are we not testing that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, silly me. I'll restore the test.
@bboozzoo is this related?
|
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…fferent seed Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…rent modes Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…ded-tweaks-cleaned-up-actions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you
The failure on 18.04 is unrelated:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
s.restartRequests = nil | ||
s.bootloader.BootVars = map[string]string{} | ||
} | ||
s.testRequestModeWithRestart(c, []string{"install", "run"}, s.mockedSystemSeeds[0].label) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
It does not make sense for recover mode to go boot into recover mode again.
This is stacked on top of #8672