boot, overlord/devicestate, daemon: implement requesting boot into a given recovery system #8369
Conversation
Implement wrapper for setting up a boot into a given system and mode. The primary use is recovery process, when on a UC20 device we will want to boot into one of the seed systems in a specific mode eg. install, recovery. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…h mode Implement support for booting into a given seed system and mode. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…de keys THe choice is made by the first bootloader running from ubuntu-seed partition. Make sure to update the right environment. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
bboozzoo
force-pushed
the
bboozzoo/uc20-request-seed-mode-reboot
branch
from
5a28317
to
fd1b0b5
Compare
boot/boot.go
Outdated
| @@ -327,3 +327,44 @@ func MarkBootSuccessful(dev Device) error { | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| var ErrSystemBootModeUnsupported = errors.New("system boot mode is unsupported") | |||
There was a problem hiding this comment.
Nitpick on naming, ErrUnsupportedSystemBootMode, this reads better to me.
mvo5
changed the title
pedronis
added
the
Needs Samuele review
boot/boot.go
Outdated
|
|
||
| // bootSystemState is used for choosing the recovery system and mode for the | ||
| // next boot. | ||
| type bootSystemState interface { |
There was a problem hiding this comment.
a bit unclear to me why we need an interface given there is only one impl (the difference should be taken care at the bootloader level)
There was a problem hiding this comment.
Yeah, I managed to over complicate things a bit.
boot/boot.go
Outdated
| // not supproted by the device, an ErrSystemBootModeUnsupported error is | ||
| // returned. | ||
| func RecoverySystemInMode(dev Device, systemLabel, mode string) error { | ||
| if dev == nil { |
There was a problem hiding this comment.
do we do this check in other places?
There was a problem hiding this comment.
Hmm not really. Let me drop it in the next push.
boot/bootsystemstate20.go
Outdated
| // setup the recovery bootloader | ||
| Recovery: true, | ||
| } | ||
| bl, err := bootloader.Find(InitramfsUbuntuSeedDir, opts) |
There was a problem hiding this comment.
Just checked, ubuntu-core-initramfs mounts /run/mnt/ubuntu-seed in RW mode. The core core20 handle-writable-paths sets up a fstab entry that binds mount it to /var/lib/snapd/seed, but still RW mode.
There was a problem hiding this comment.
@bboozzoo can you create the bug xnox asked about at the writable-path bits?
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't think we should carry around that directory in run-mode IMHO. OTOH, I don't know where the code should look for to find the ubuntu-seed bootloader, so perhaps it does need to be in this location. However, if we want ubuntu-seed to be in this location, then I think we should rename the InitramfsUbuntuSeedDir to something that isn't specific to the initramfs because this function will be used from places not during the initramfs.
…ary interface Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…uest-seed-mode-reboot
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…uest-seed-mode-reboot
There was a problem hiding this comment.
This looks good to me, but I would like to see us resolve the issue about where ubuntu-seed bootloader should actually live to be writable at runtime. Is /run/mnt/ubuntu-seed okay for this? If so we should rename the InitramfsUbuntuSeedDir to reflect that this directory exists not just during the initramfs.
Also a small comment on the checks in boot
boot/boot_test.go
Outdated
| c.Assert(err, ErrorMatches, "internal error: system or mode is unset") | ||
| } | ||
|
|
||
| func (s *bootenvSystem20Suite) TestSetRecoveryBootSystemAndModeRealHappy(c *C) { |
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
overlord/devicestate/devicemgr.go
Outdated
| } | ||
|
|
||
| logger.Noticef("restarting into system %q for action %q", systemLabel, sysAction.Title) | ||
| m.state.RequestRestart(state.RestartSystem) |
There was a problem hiding this comment.
this should use RestartSystemNow right, when available ?
There was a problem hiding this comment.
Yes, I hoped to land #8386 quickly, but it seems that PR must reach a decent number of CI restarts before this can happen.
There was a problem hiding this comment.
The PR has landed, I've merged master and pushed the patches that request the new immediate reboot.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…uest-seed-mode-reboot
many: support immediate reboot Our reboot process adds a minimum of 1 minute delay from the request to when the reboot actually happens. This is undesirable in a number of scenarios, specifically when a UC20 install is finished, and when we reboot into a recovery system (once #8369 lands). The PR introduces a new restart type and hooks up the install mode to use the type. Testing this locally in a VM makes the install process feel much faster.
…uest-seed-mode-reboot
…recovery system 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>
The PR implements the bits responsible for requesting a boot into a given recovery system & mode.
Part of UC20 recovery chooser work.