o/devicestate: raise conflict when requesting system action while seeding #8689
Conversation
…quested When we get a request for a system action, verify that the device is seeded or at least that the seeding change has finished. We cannot execute the action sooner, because the list of allowed actions for the current system depends on the mode and the system state. Eg. a fully seeded system supports 'run'/'recover'/'install' actions for itself, or install for other system seeds. However, when the seeding fails, one can only execute an 'install' action for the system. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…onflicts Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
| } | ||
|
|
||
| // not yet fully seeded, hold off requests for the system that is being | ||
| // seeded, but allow requests for other systems |
There was a problem hiding this comment.
this feels a bit complicated, can't we base this on the current system (when defined) and as the system being installed for install
There was a problem hiding this comment.
Maybe I misunderstood something, but the current system is only recorded when seeding is done. Once it's past that point, there's no conflict anymore, regardless of the current system. When it's not started yet, we don't know whether there is a conflict because we don't know which system is being installed.
There was a problem hiding this comment.
Simplified as per discussion.
Instead of inspecting the seeding change and extracting the seed system information, look at modeenv and the recovery system stored there. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
overlord/devicestate/systems.go
Outdated
| // not yet fully seeded, hold off requests for the system that is being | ||
| // seeded, but allow requests for other systems | ||
| if modeEnv.RecoverySystem == systemLabel { | ||
| return &snapstate.ChangeConflictError{Message: "cannot request system action, system is seeding"} |
There was a problem hiding this comment.
we probably want to set ChangeKind: "seed" as other similar cases
| return err | ||
| } | ||
| if modeEnv == nil { | ||
| // non UC20 systems do not support actions, no conflict can |
There was a problem hiding this comment.
we don't get here in tests, do we have tests about what RequestSystemAction should do on 16/18 ?
There was a problem hiding this comment.
I added additional checks for non-UC20 and a couple of unit tests.
…here is no mode 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>
overlord/devicestate/devicemgr.go
Outdated
| if systemLabel == "" { | ||
| return fmt.Errorf("internal error: system label is unset") | ||
| } | ||
| if m.systemMode == "" { |
There was a problem hiding this comment.
mmh, we don't do this anywhere else, usually we use the grade unset of the model to know if we are on uc16/18. we need to think a bit.
…ssor Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
| if modeEnv.RecoverySystem == systemLabel { | ||
| return &snapstate.ChangeConflictError{ | ||
| ChangeKind: "seed", | ||
| Message: "cannot request system action, system is seeding", |
There was a problem hiding this comment.
Perhaps 'cannot ..., system is being seeded'?
There was a problem hiding this comment.
This is a close one, but I think it's probably fine as is; seeding is more active (which is good) and I think it's a widely enough accepted term.
| return nil | ||
| } | ||
|
|
||
| // inspect the current system which is stored in modeenv, were are |
There was a problem hiding this comment.
note, typo this should be
// inspect the current system which is stored in modeenv, note we are
There was a problem hiding this comment.
Thanks for spotting this, I've opened #8712
Return a conflict error when trying to request system action on a system that is still seeding (or seeding hasn't started yet).
Stacked on top of #8627.