Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
overlord, snap: add firstboot state sync #1033
Conversation
mvo5
added some commits
Apr 18, 2016
pedronis
reviewed
Apr 18, 2016
| + var snapst snapstate.SnapState | ||
| + snapst.Sequence = append(snapst.Sequence, &info.SideInfo) | ||
| + snapst.Channel = info.Channel | ||
| + snapst.Active = sn.IsActive() |
pedronis
Apr 18, 2016
Contributor
this will always be false? unless booted does the right thing with symlinks which maybe it does (don't know order myself)
pedronis
reviewed
Apr 18, 2016
| + return nil | ||
| +} | ||
| + | ||
| +func FirstBoot() error { |
pedronis
Apr 18, 2016
Contributor
could we have some kind of comment that this a temporary approach/solution
pedronis
reviewed
Apr 18, 2016
| @@ -40,7 +41,7 @@ func init() { | ||
| } | ||
| func (x *cmdInternalFirstBoot) Execute(args []string) error { | ||
| - err := snappy.FirstBoot() | ||
| + err := overlord.FirstBoot() |
pedronis
Apr 18, 2016
Contributor
just return overlord.FirstBoot() given we are not returning the special error, anyway all this is going away so i18n not so important
|
@pedronis thanks a bunch for the review, I addressed the issues. |
mvo5
added some commits
Apr 18, 2016
niemeyer
reviewed
Apr 18, 2016
| "github.com/jessevdk/go-flags" | ||
| - "github.com/ubuntu-core/snappy/i18n" | ||
| - "github.com/ubuntu-core/snappy/snappy" | ||
| + "github.com/ubuntu-core/snappy/overlord" | ||
| ) | ||
| type cmdInternalFirstBoot struct{} |
niemeyer
Apr 18, 2016
Contributor
Can we change that command to be snap internal --firstboot instead of snap firstboot? (not necessarily in this branch)
mvo5
Apr 18, 2016
Collaborator
Sure. Note that this is a hidden command already. I can also move it.
niemeyer
reviewed
Apr 18, 2016
| +// This is not the final way we will do the state sync. This is just | ||
| +// an intermediate step to have working images again. We need to | ||
| +// figure out how we want first-boot to look like. | ||
| +func FirstBoot() error { |
niemeyer
Apr 18, 2016
•
Edited 1 time
-
niemeyer
Apr 18, 2016
Contributor
-
Apr 18, 2016
Can we please move all this logic to overlord/firstboot, under a Booted function? Next (not this branch), we must kill the use of Overlord.Installed and perhaps all use of the snappy package too, and have something like firstboot.Prepare that prepares only the strictly necessary snaps for the first boot without requiring them to be actually installed. Then, Booted will use standard snapstate operations to really install the snaps, out of a pre-defined directory (snapd/firstboot/*.snap?), which will contain kernel, os, and anything else udf puts there.
How does that plan sound?
niemeyer
Apr 18, 2016
Contributor
(changed the snap collection place to be snapd/firstboot/*.snap instead)
pedronis
Apr 18, 2016
Contributor
we have already something called booted (that is called on every boot and checks if there was OS failover or not and resyncs what should be the active OS/kernel) which needs to be migrated to the state engine world, so not sure about using that term in the context of first boot logic;
2nd) as we discussed kernel and OS need to be in place to some extent already as far I understand what @mvo5 explained for things to boot at all, so the logic described will work for other snaps but for kernel and os we'll need a bit more work, something slightly different
pedronis
Apr 18, 2016
Contributor
I also wonder whether the final place for this won't be under gadgetstate as we discussed many times the idea of having a gadget manager
niemeyer
Apr 18, 2016
Contributor
If we have a "booted" instruction, why do we need a "firstboot" one? Isn't the "firstboot" one the first time the "booted" one runs (which is easy to detect)?
pedronis
via email
Apr 18, 2016
Contributor
|
I did a manual test of this on a freshly created image and it appears to be working ok and with that images work again. |
|
Okay, I'm merging this then. Let's discuss and evaluate those suggested improvements tomorrow with more time. We can easily move both the command and where the logic lives without breaking anyone. |
mvo5 commentedApr 18, 2016
This branch makes our images work again. It will take what is installed on the system and populate the state with that. This is an intermediate step until we know better how to represent state when using u-d-f. As long as we keep the manifests on disk this will work and is a cheap way for us to unbreak the images and to let people play with snappy 2.0 on devices and images.