Merge lp://qastaging/~jibel/livecd-rootfs/add_multi_layered_squashfses_support into lp://qastaging/livecd-rootfs

Proposed by Jean-Baptiste Lallement
Status: Merged
Merge reported by: Steve Langasek
Merged at revision: not available
Proposed branch: lp://qastaging/~jibel/livecd-rootfs/add_multi_layered_squashfses_support
Merge into: lp://qastaging/livecd-rootfs
Diff against target: 1242 lines (+808/-255)
7 files modified
debian/tests/default-bootstraps (+1/-0)
live-build/auto/build (+137/-185)
live-build/auto/clean (+2/-0)
live-build/auto/config (+92/-64)
live-build/functions (+198/-6)
live-build/lb_binary_layered (+132/-0)
live-build/lb_chroot_layered (+246/-0)
To merge this branch: bzr merge lp://qastaging/~jibel/livecd-rootfs/add_multi_layered_squashfses_support
Reviewer Review Type Date Requested Status
Steve Langasek Needs Fixing
Review via email: mp+358490@code.qastaging.launchpad.net

Description of the change

Adds support for multi layer filesystem by generating one squashfs per layer and adding a new 'live-layered' image format.
squashfs are numbered so the order is preserved and mounted in order by casper.

Seeding of snap packages and seeds corresponding to the layers are not
available yet and will be proposed in the subsequent merge proposal.

To post a comment you must log in.
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

In passes, how does one specify multiple branches of stacks? Or is only a linear stack allowed by this code?

Cause the current subiquity images do not have `live` as the top of the stack.... They do this:

+--> Base +-----> Live
          |
          +-----> Rack +---> Region

Because I am expecting to have the ability to somehow specify what each pass depends on. But it looks like the base for a subsequent pass, is always the previous one?!

It would be nice to extend PASS syntax to optionally accept an arbitrary `base`, e.g.

PASSES="base rack region base:live" to encode the above graph, and such that `base:live` pass uses base as the lowerdir, instead of region (i.e. the previous pass result).

Or something...

not sure if : is acceptable pair delimiter here, or not.

Also not sure if we want to always enforce specifying `base` such that we can construct multiple root nodes in one go. E.g. `base base:rack rack:region base:live` or `:base rack region base:live` for the above graph.

Or like maybe always list all layers?! but that violates donot-repeat-yourself principle... E.g. `base base:rack base:rack:region base:live`

===

When calling includes/hooks are they aware which pass they are for? what is the source dir for each pass, for e.g. binary.includes? I'm guessing that PASS variable is set, but not sure.

===

filesystem.squashfs is somewhat is a special name, so it would be nice to keep that as the base one. And also possibly adjust logic in casper as to what it mounts by default.... cause e.g. i think i hide maas squashfes in a subdir, to prevent casper from mounting those, which is kind of a hack. I wonder if we do need to write out the valid stacks (passes?!), which casper can then use to boot to whichever stack is valid. With subiquity image this could then result in "Live Server, Live MAAS Rack, Live MAAS Region, Live Server with Installer" boot options. As example, for better or worse.

====

No idea if SUBPROJECT and IMAGE_FORMAT are the right things to extend for this..... and if they are easily extendable like this in launchpad livefs builders & ubuntu-cdimage codes.

Cause I can see the potential for using layers in SUBPROJECT=minimized, if for example, cpc builds are converted to layers they would have full and minimized layered builds.... and SUBPROJECT=minimized-layered sounds ugly =)

====

manifest diffs for layers is nice; cause in cpc we have struggled to consitently represent manifests / changelogs of "it's just like that image, but has this stuff on it"

====

packaging layers as actual static filesystems might be interesting, but i guess hooks will be able to do that anyway.

====

Overall, this looks ok, and shouldn't break any existing stuff - as long as we can clear the top level new extensions of

   SUBPROJECT=layered
   IMAGEFORMAT=live-layered

and that needs like an architect review.

Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :
Download full text (5.9 KiB)

Thanks for the review

On 14/11/2018 00:35, Dimitri John Ledkov wrote:
>
> In passes, how does one specify multiple branches of stacks? Or is only a linear stack allowed by this code?

This code only allows linear stacks to avoid not make current code more
complex than it already is and diverging too much from existing logic.
Our approach is to define the stacking defining the live image in PASSES
and put the extra logic like “branched mounts” in hooks.

For instance with langpacks which is the use case for Desktop image, we
also have multiple branches of stacks. It is difficult to represent the
structure in a generic way and it adds extra complexity to the
maintenance of those definitions.

Following your proposal to represent the layers structure as a list, it
would be something like:
PASSES=“desktop-minimal:desktop:live desktop-minimal:lang-neg-min-fr
desktop-minimal:lang-neg-min-de desktop-minimal:desktop:lang-neg-fr
desktop-minimal:desktop:lang-neg-de …”

We thus have multiple “base” here, “desktop-minimal” or
“desktop-minimal:desktop”. Shortening the syntax doesn’t seem
appropriate here.

In a tree, it’s not better:
- desktop-min
   - desktop
     - Lang-neg-de
     - Lang-neg-fr
     - Lang-neg-es
     - …
     - Live
   - Lang-neg-min-de
   - Lang-neg-min-fr
   - Lang-neg-min-es
   - …

Addition or removal of any default language would be then error-prone.
This is to compare with a hook, where we just loop over “desktop-min”
and creates langpacks negative stacks, and then looping over “desktop”
as well to achieve the same. Besides, shell seems inappropriate to
implement this type of logic.

Note that the current implementation is similar than existing
ubuntu-server:live logic (just a little bit more generic) and we made
sure we didn’t break your use case. Note that though, you will be able
to remove your first hook, creating the “live” stack using the generic
code right now. However, the maas-* stacks will still be in hooks.

>
> Cause the current subiquity images do not have `live` as the top of the stack.... They do this:
>
> +--> Base +-----> Live
> |
> +-----> Rack +---> Region
>
> Because I am expecting to have the ability to somehow specify what each pass depends on. But it looks like the base for a subsequent pass, is always the previous one?!
>
>
> It would be nice to extend PASS syntax to optionally accept an arbitrary `base`, e.g.
>
> PASSES="base rack region base:live" to encode the above graph, and such that `base:live` pass uses base as the lowerdir, instead of region (i.e. the previous pass result).
>
> Or something...
>
> not sure if : is acceptable pair delimiter here, or not.
>
> Also not sure if we want to always enforce specifying `base` such that we can construct multiple root nodes in one go. E.g. `base base:rack rack:region base:live` or `:base rack region base:live` for the above graph.
>
> Or like maybe always list all layers?! but that violates donot-repeat-yourself principle... E.g. `base base:rack base:rack:region base:live`
>
> ===
>
> When calling includes/hooks are they aware which pass they are for? what is the source dir for each pass, for e.g. bina...

Read more...

Revision history for this message
Steve Langasek (vorlon) wrote :

Thanks for working on this, it will be nice to have cleaner handling of the desktop-minimal stuff included here in livecd-rootfs.

I would like to see this converged with the ubuntu-server-live handling as part of landing of this branch (see also Dimitri's review comments). You've gone to some length to implement this in a way that's generalizable across projects, so we ought to make sure the implementation is actually reusable by the only other image that currently uses layered squashfs.

review: Needs Fixing
Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

We intended to propose several MP for snap and “sub-layers” to reduce the complexity of the review, but we’ll finally merge them into this one so you’ll have a good understanding of the full implementation.

Supporting the server-live use case adds makes the implementation more complex due to its specific requirements (hooks and includes). But we agree that a more generalizable implementation is better, so let’s spend some time to ensure those 2 additional requirements are supported, and once foundation/server team want to switch to it, that they will not encounter any major limitations.

Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

Following previous comments, this MP has been updated with these changes:
- Merged preseeding of snap packages and adds snap packages to the image manifest.
- Added lb_binary_layered to handle multi branches of squashfses with sublayers.
- Factorized several helpers called by lb_chroot_layered
- Removed obsolete chroot helpers from lb_chroot_layered and minor cleanup.
- Renamed subproject layered -> ubiquity-ng
- Use PASSES instead of IMAGEFORMAT to detect a multilayer project. Defining PASSES switches the image build into layered build.
- Adds includes by pass to customize chroot for specific passes.
- Moved back specific helpers and functions from functions to config.
- For Ubuntu Desktop:ubiquity-ng, build negative language packs and corresponding squashfs.

These changes have been tested against ubuntu-server live which still uses its current implementation (with maas-region and maas-rack squashfses built in hooks).
All the requirements of ubuntu-server:live can be ported from the current hooks to the new implementation of the layered images.
To do so:
- The project have to define PASSES.
- Snaps and packages for sublayers added to separated seeds and referenced in auto/config.
- Layers must be customized with chroot hooks (eg to modify existing files) and chroot includes (eg to add new files) depending on PASS.

Ubuntu Desktop (standard disco) has been modified to use layered images as requested. As a consequence the following MPs must be reviewed together with this one and released before uploading livecd-rootfs:
- debian-cd: https://code.launchpad.net/~jibel/debian-cd/support_for_multilayer_images/+merge/359228
- ubuntu-cdimage: https://code.launchpad.net/~jibel/ubuntu-cdimage/support_for_multilayer/+merge/359512

The subproject ubiquity-ng must be created for building the ubuntu-desktop:ubiquity-ng rootfs and match https://launchpad.net/~ubuntu-cdimage/+livefs/ubuntu/disco/ubuntu-desktop-ubiquity-ng/
This whole set of changes (livecd-rootfs, ubuntu-cdimage and debian-cd) have been tested with the following projects:
- Ubuntu Desktop layered (iso) (disco) -> test new set of layers
- Ubuntu Desktop unlayered (iso) (disco) -> test previous image format (one squashfs)
- Ubuntu Desktop ubiquity-ng (iso) -> test new set of layers + sublayers
- Ubuntu Server live (iso) (bionic and disco) -> test backward compatibility with current server image and hooks impacts.
- Lubuntu (rootfs only, no local archive with universe for building iso, similar build to ubuntu-desktop/unlayered) (disco) -> test traditional one squashfs image
- Ubuntu Mate (rootfs only, no local archive with universe for building iso, similar build to ubuntu-desktop/unlayered) (bionic) -> test traditional one squashfs image + snaps

Revision history for this message
Adam Conrad (adconrad) wrote :

I don't have time for a full review this morning, but unless we intend to keep these forked from the "real" ISOs for testing for a while, and then converge, I really don't like the SUBPROJECT use here. I'd expect the real 'ubuntu' project to be building these, not some ubuntu-random-subproject project.

As noted, you already can decide if you're building this type of image based on PASSES being defined, so it seems a bit odd to then also key on a subproject name.

1758. By Jean-Baptiste Lallement

configure network manager _after_ installing network-manager

Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

> I don't have time for a full review this morning, but unless we intend to keep
> these forked from the "real" ISOs for testing for a while, and then converge,
> I really don't like the SUBPROJECT use here. I'd expect the real 'ubuntu'
> project to be building these, not some ubuntu-random-subproject project.
>
> As noted, you already can decide if you're building this type of image based
> on PASSES being defined, so it seems a bit odd to then also key on a
> subproject name.
We need a subproject because the real 'ubuntu' and the new installer work will diverge quickly. Note that Ubuntu Desktop uses the layer system on Disco with this merge proposal without defining a subproject.

Revision history for this message
Balint Reczey (rbalint) wrote :

The target Bazaar branch is not active anymore.
Please resubmit the merge proposal against https://code.launchpad.net/~ubuntu-core-dev/livecd-rootfs/+git/livecd-rootfs/+ref/ubuntu/master .

Revision history for this message
Steve Langasek (vorlon) wrote :

This is done now in git.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches