[Merge] ~mwhudson/livecd-rootfs/+git/livecd-rootfs:minimal-server-layer-3 into livecd-rootfs:ubuntu/master

Tue Aug 3 04:32:27 UTC 2021

Hi, thanks for the review. I don't think I want to make any changes in response but feel free to argue back :) and your comments prodded me to notice a bug, so that's good.

Diff comments:

> diff --git a/live-build/auto/config b/live-build/auto/config
> index 6d5fce1..d6d9d40 100755
> --- a/live-build/auto/config
> +++ b/live-build/auto/config
> @@ -773,15 +773,17 @@ case $PROJECT in
>  		case ${SUBPROJECT:-} in
>  			live)
>  				PASSES_TO_LAYERS=true
> -				add_task ubuntu-server minimal standard server
> +				add_task ubuntu-server-minimal server-minimal
> +				add_package ubuntu-server-minimal lxd-installer

Yes, it's basically a shell script that runs "snap install lxd" as soon as the user tries to run lxc or lxd. It was written expressly for the minimal cloud images.

> +				add_task ubuntu-server-minimal.ubuntu-server minimal standard server

Yeah this is a bit unfortunate really but yes, the minimal seed contains things that don't seem required. Fwiw, the things explicitly seeded into the minimal seed that are not in the server-minimal seed are https://paste.ubuntu.com/p/3Y9JK4yWd6/. Most of these are pretty small and could probably be included without much impact but "locales" is pretty big.

Perhaps some of these should be dropped from the minimal seed but it's not clear that we really want to change what that means -- the seed contains phrases like "Programs and packages necessary for both the proper functionality of the system and that any user would expect to find on a modern Unix-like system." which doesn't actually match what we want for the minimal server image (which is targeted at systems that will never have a human log in). So yeah, this is a muddle and I'm not super sure how to un-muddle it.

HOWEVER, thinking about all this made me realize that the ubuntu-server-minimal layer is bootstrapped with the default debootstrap variant, so all the Priority: important packages (a.k.a. packages from the minimal seed) are being installed anyway.  So that's something for me to fix, thanks!

>  				# add_task really should do this itself but for now...
> -				snap_from_seed server config/package-lists/livecd-rootfs.snaplist.chroot_ubuntu-server.full
> -				add_package ubuntu-server cloud-init
> +				snap_from_seed server config/package-lists/livecd-rootfs.snaplist.chroot_ubuntu-server-minimal.ubuntu-server.full
> +				add_package ubuntu-server-minimal.ubuntu-server cloud-init
>  
> -				add_package ubuntu-server.installer linux-firmware lupin-casper openssh-server
> -				add_snap ubuntu-server.installer subiquity/classic
> +				add_package ubuntu-server-minimal.ubuntu-server.installer linux-firmware lupin-casper openssh-server
> +				add_snap ubuntu-server-minimal.ubuntu-server.installer subiquity/classic
>  				if [ $ARCH = s390x ]; then
> -					add_package ubuntu-server.installer s390-tools-zkey
> +					add_package ubuntu-server-minimal.ubuntu-server.installer s390-tools-zkey
>  				fi
>  
>  				# Live server ISOs for LTS point releases past .2 offer both
> diff --git a/live-build/ubuntu-server/hooks/02-installer-bits.chroot b/live-build/ubuntu-server/hooks/02-installer-bits.chroot
> index d71157c..49a6c27 100755
> --- a/live-build/ubuntu-server/hooks/02-installer-bits.chroot
> +++ b/live-build/ubuntu-server/hooks/02-installer-bits.chroot
> @@ -1,7 +1,7 @@
>  #!/bin/bash -ex
>  # vi: ts=4 noexpandtab
>  
> -if [ "${PASS}" != "ubuntu-server.installer" ]; then
> +if [ "${PASS}" != "ubuntu-server-minimal.ubuntu-server.installer" ]; then

Certainly the ugliest part of all this is how splitting a layer means I have to go around and update all these checks. One thing I thought about that would reduce the noise would be to enforce that each component of the layer name is unique (so you couldn't have foo.bar.bar or whatever) and then key off some PASS_NAME variable or something. Using is_live_layer here seems like a hack though -- surely the definition of a "live" layer is one that can be used as a live session, and this one can't. You could imagine having some machine to offer all available live layers in grub, or something like that. Also, conceptually this hook should only run for the named hook, not the kernel-containing layers on top of it although in practice it's idempotent and won't cause any problems in that regard.

At the end of the day, I think I'd prefer to leave these checks as they are. If you can think of a neat way of tying a hook to a layer more declaratively, I'd love to hear it. (I mean, we could have files called $PASS.$whatever.chroot_early and so on... that seems fairly ugly too though).

>      exit 0
>  fi
>  
> diff --git a/live-build/ubuntu-server/hooks/03-kernel-metapkg.chroot_early b/live-build/ubuntu-server/hooks/03-kernel-metapkg.chroot_early
> index 7e99c6b..864d4c4 100755
> --- a/live-build/ubuntu-server/hooks/03-kernel-metapkg.chroot_early
> +++ b/live-build/ubuntu-server/hooks/03-kernel-metapkg.chroot_early
> @@ -2,7 +2,7 @@
>  # vi: ts=4 noexpandtab
>  
>  case $PASS in
> -    ubuntu-server.installer.*)
> +    ubuntu-server-minimal.ubuntu-server.installer.*)

Here seems like a better candidate indeed, but for consistency I'd slightly prefer to leave it as is.

>          flavor=${PASS##*.}
>          if [ "$flavor" = "generic" ]; then
>              kernel_metapkg=linux-generic
> diff --git a/live-build/ubuntu-server/hooks/04-kernel-bits.binary b/live-build/ubuntu-server/hooks/04-kernel-bits.binary
> index 3b5f086..c0b2e3d 100755
> --- a/live-build/ubuntu-server/hooks/04-kernel-bits.binary
> +++ b/live-build/ubuntu-server/hooks/04-kernel-bits.binary
> @@ -2,7 +2,7 @@
>  # vi: ts=4 noexpandtab
>  
>  case $PASS in
> -    ubuntu-server.installer.*)
> +    ubuntu-server-minimal.ubuntu-server.installer.*)

ditto reply. FWIW, there are various bits of generic machinery that seem like they ought to render this hook redundant but they don't all quite fit together right. It would be nice to fix that one day. (I do want the new desktop installer to move to a model where the base image doesn't have a kernel in it and it's always installed from the pool, but that's an only slightly related conversation I need to go and have with some people...)

>          flavor=${PASS##*.}
>          ;;
>      *)

-- 
https://code.launchpad.net/~mwhudson/livecd-rootfs/+git/livecd-rootfs/+merge/406342
Your team Ubuntu Core Development Team is subscribed to branch livecd-rootfs:ubuntu/master.