[Merge] ~jibel/livecd-rootfs/+git/add_multi_layered_squashfses_support:ubuntu/master into livecd-rootfs:ubuntu/master
Jean-Baptiste Lallement
jean-baptiste.lallement at ubuntu.com
Wed Jan 9 13:21:51 UTC 2019
>
> Aaah. I missed that on my first reading of the code. So I think I'm
> probably a bit happier about the general state of things... what I think I
> need to do is to try to port the live-server code over to this and see how
> that goes. I think it will make the code a lot smaller and better factored.
> Unfortunately for you I'm on leave for the next week, though.
>
We’d really to separate the work for Desktop and Server into different MPs and not add more constraints to this already complex MP. When Server is converted to this pattern, we are sure, even if most of logic is compatible with it, that some refinements can be made.
> I don't really like the fact that you record the sublayer structure in the
> name of package list files but maybe I'll get over that.
As you said yourself, this is bash, and having to parse complex structure there is a PITA. This solution seems the best in term of maintainability.
>
> Yeah that is simple but seems a bit ... I don't know ... wrong somehow.
> Have you seen what happened to hooks in
> https://git.launchpad.net/livecd-
> rootfs/commit/?id=cbd4eb57171877e019b408923709478b96040348
> and considered doing something like that? (I don't entirely understand that
> commit nor how to "do something like that" but well, it seems relevant).
>
This code is specific to CPC (which AFAIU is not layered) and what you’re asking is that we make it generic for any kind of image, layered or not. IMHO this is beyond the scope of this MP. It is targeted to Desktop images and made generic to support the MaaS image case which is the only existing layered build.
We don’t expect (based on MaaS and desktop use-case) to see specific chroot hooks. Indeed, with this proposal, most of the hooks from the MaaS image can be removed in favor of a more declarative approach for packages and snap via seeds, like any other kind of images. (No need to run apt install or snap install anymore). This is why we think the $PASS would be a good decision point to run a hook or not, at least, AFAIK.
> > >
> > > 2) While lb_chroot_layered seems a full copy of lb_chroot,
> > lb_binary_layered
> > > seems a poor cousin. Unless I'm missing things it does not support hooks
> > or
> > > includes (or well lots of other things but I don't think we will every
> > use any
> > > of those features)
> >
> > Hooks and includes are executed on the finalized filesystem and cannot run
> > for an inner layer once the filesystem is built otherwise the overlay would
> > be invalidated. This is why this is not supported.
> >
>
> Well OK but we also (mostly?) use binary hooks to make artefacts. Which is
> something else I may have forgotten to complain about in my first review:
> it seems that your branch always makes a squashfs out of a layer, but for
> the ga/hwe kernel part of live-server I don't want to do that, I want to
> pull initrds and kernels out instead. How should I do that in your model? I
> don't really want to have to add code to auto/build for that, although I
> guess that will work for a first cut.
>
>From what we understood in the livecd-rootfs model, in its original spirit, binary hooks and includes are originally designed to run on the “live” base, meaning the last layer.
Indeed, those hooks shouldn’t change the chroot at all, as they are running after the manifest creation by lb_binary. Those would be for instance to put specific boot options depending on $INIT for instance.
It is possible to solve your use case by creating a seed for GA and a seed for HWE and do 2 builds and set the kernel/initrd layers as the lowest layers. This would be as well one example for a common chroot hooks (to create the module files) between different PASS. The benefit of that approach is to ensure the packages database (apt/dpkg/snapd) is consistent with what is installed in the chroot.
> > >
> > > A couple more comments inline.
> > >
> > > Have you tested this? Can I see what a livefs built using this looks
> > like?
> > Yes, this has been thoroughly tested together with debian-cd and
> > ubuntu-cdimage. See the list of extensive testing that has been done for
> > each rebase we have on new master in
> > https://code.launchpad.net/~jibel/livecd-
> rootfs/add_multi_layered_squashfses_support/+merge/358490/comments/935368
> > .
> > The final desktop image built with this patch has been successfully booted
> > to the live session and installed by curtin
>
>
> OK thanks. What I really wanted to see was a livefs in launchpad with a
> build log and artefacts but of course the artefacts get deleted pretty
> quickly so if I want to see that I should make one myself I guess.
>
We cannot build an image on LP until this is merged and the package is uploaded to the distro.
>
> > I don’t have an ISO built with this changes handy but you can see an
> > example of such a structure above.
> > As testing is taking a long time as you can imagine, we will apply the
> > “overlay.” and “subtract” changes if there is no other blockers so that we
> > can rebase (for the last time, hopefully) on livecd-rootfs master and test.
> >
>
> I think that's reasonable. I would encourage you to think about the
> "per-pass chroot hooks" thing and the "control over artefacts" thing. I'll
> get back to this in a week, or if you're impatient you can try to get
> someone else to review it :)
These changes have been reviewed by several people since November (xnox, vorlon and infinity) and we timely addressed the comments each time. The code base keeps changing, rebasing this MP is far from trivial and really time consuming. I don’t think it is unreasonable or being impatient to have it reviewed promptly and merged if there is no major objections as it is blocking further work on the Desktop image.
The code can always be updated later for your specific requirements.
Also note the this MP goes with 2 related changes in ubuntu-cdimage and debian-cd:
https://code.launchpad.net/~jibel/ubuntu-cdimage/support_for_multilayer/+merge/359512
https://code.launchpad.net/~jibel/debian-cd/support_for_multilayer_images/+merge/359228
--
https://code.launchpad.net/~jibel/livecd-rootfs/+git/add_multi_layered_squashfses_support/+merge/360878
Your team Ubuntu Core Development Team is subscribed to branch livecd-rootfs:ubuntu/master.
More information about the Ubuntu-reviews
mailing list