Merge lp:~semiosis/livecd-rootfs/fix-for-1565985 into lp:livecd-rootfs

[Louis Zuckerman]

Several problems with the official ubuntu vagrant boxes for Xenial:

- Standard packages were missing (virtualbox-guest stuff, config management stuff)
- Box name was statically assigned, limiting to only one instance per host
- Default synced folder was disabled

This change fixes all of the above issues, except for config management.
As per discussion in #ubuntu-devel we have opened a new bug for the missing config management packages: https://bugs.launchpad.net/cloud-images/+bug/1599531

Also cleaned up some inconsistent indentation.

Last of all, had to change the shebang to not use -u. This is because clean_loops (from functions) gets called twice and the second time (by trap) has unset variables so the build would die.

As a side note, a solution to allow clean_loops to run twice under bash -u would be to use ${var_name-} instead of ${var_name} in the two variable test lines, but that was out of scope for my issues. Just FYI.

Thank you!

[Louis Zuckerman (semiosis)]

How to use/test:

I did all my development & testing on the official xenial vagrant box (dogfooding!)

The build will exceed the space available on / on the vagrant box so I had to disable some of the other hook scripts that were unrelated.

If you have about 10 GB free on / you should be able to build all of the images in ubuntu-cpc.

Here's how...

# First, set up the build tools and workspace.
# The scripts require that you work in /build

sudo -i
apt-get install -y livecd-rootfs
mkdir -p /build/chroot
cd /build
cp -a /usr/share/livecd-rootfs/live-build/auto .

# All the hard work is done with live-build (lb command)
# and we have to configure it with environment variables

export SUITE=xenial
export ARCH=amd64
export PROJECT=ubuntu-cpc
export MIRROR=http://archive.ubuntu.com/ubuntu/

# Now we can have live-build set up the workspace

lb config

# If you're short on time or disk space you can disable
# some of the builders. Go into /build/config/hooks and
# remove these scripts which are unrelated to vagrant:
# 032-root-squashfs.binary 033-disk-image-uefi.binary
# 034-disk-image-ppc64el.binary 040-qcow2-image.binary
# 040-vmdk-image.binary 041-vmdk-ova-image.binary

# And finally, start the build

lb build

[Dan Watkins (daniel-thewatkins)]

Hi Louis,

I've tested this using our actual build stack, and hit some issues with package installation; I'm going to try a couple of things to get it working myself, and will let you know if I make progress.

Dan

[Dan Watkins (daniel-thewatkins)]

Hi Louis,

Added some comments inline; this includes the change I had to make, but also a couple of other thoughts. :)

Thanks again for looking at this!

Dan

review: Needs Fixing

[lp:~semiosis/livecd-rootfs/fix-for-1565985]

1413. By Louis Zuckerman on 2016-07-06

[ Louis Zuckerman ]
* Merged the following changes from upstream
[ Łukasz 'sil2100' Zemczak ]
* Remove the ubuntu-pd project from the scripts
* Add instead an ubuntu-touch-custom project for custom re-builds of
  ubuntu-touch
[ Daniel Watkins ]
* Consolidate cloud images
  - Remove .tar.gz and .tar.xz (removed in favour of the squashfs).
  - Remove the MBR-only disk image in favour of the shared GPT/MBR UEFI
    image.
  - Remove '-disk1' from bootable image names.
[ Phil Roche ]
* Removed HWCLOCKACCESS=no from /etc/default/rcS (LP: #1581044)

1414. By Louis Zuckerman on 2016-07-06

[ Louis Zuckerman ]
* Fixes for vagrant box builder in ubuntu-cpc LP: #1565985
* Remove extra packages added on 24 Jun 2016 except virtualbox-guest stuff
* Add cloud-init config to manage /etc/hosts for localhost

1415. By Louis Zuckerman on 2016-07-06

fixed changelog

1416. By Louis Zuckerman on 2016-07-06

fix changelog

1417. By Louis Zuckerman on 2016-07-06

clean up changelog

[Dan Watkins (daniel-thewatkins)]

I've started the process of building a test box now (which, unfortunately, I probably won't complete before I have to head out this (BST) evening).

One thought: could we restore u to the hashbang and then just wrap the offending line in `set +u; ...; set -u`?

[Louis Zuckerman (semiosis)]

> One thought: could we restore u to the hashbang and then just wrap the
> offending line in `set +u; ...; set -u`?

The offending line in clean_loops is in the functions file. If we have to change that, wouldn't it be better to use ${var-} instead of ${var} like I mentioned earlier? That seems closer to the intention of the code, which wants to know if a variable contains a path or not.

http://bazaar.launchpad.net/~ubuntu-core-dev/livecd-rootfs/trunk/view/head:/live-build/ubuntu-cpc/functions#L15

I may not be catching your suggestion, since I'm not a bash expert. If the above isn't the offending line you have in mind then please help me understand better.

[lp:~semiosis/livecd-rootfs/fix-for-1565985]

1418. By Louis Zuckerman on 2016-07-07

update changelog: Add cloud-init config to manage /etc/hosts LP: #1561250

1419. By Louis Zuckerman on 2016-07-07

remove comment from vagrant script

[Louis Zuckerman (semiosis)]

Just realized I forgot to point out where the new changed file goes! See below...

> # Now we can have live-build set up the workspace
>
> lb config
>

After running the `lb config` command above you'll have a /build/config directory. Replace /build/config/hooks/042-vagrant.binary with the version in this branch. Then continue on to run `lb build`.

[Dan Watkins (daniel-thewatkins)]

review: Approve

[Steve Langasek (vorlon)]

Questions inline.

review: Needs Fixing

[lp:~semiosis/livecd-rootfs/fix-for-1565985]

1420. By Louis Zuckerman on 2016-07-11

updated per Steve Langasek's feedback:
- put umount in trap
- wrap comment lines at 78 chars
- remove unnecessary DEBIAN_FRONTENT env var from apt-get lines

[Adam Conrad (adconrad)]

Surely, virtualbox-guest-dkms isn't necessary if you're installing an Ubuntu kernel, which has the modules built in.

[Adam Conrad (adconrad)]

$ apt-cache show linux-image-$(uname -r) | grep ^Provides
Provides: fuse-module, ivtv-modules, kvm-api-4, linux-image, redhat-cluster-modules, spl-dkms, virtualbox-guest-modules, zfs-dkms

[lp:~semiosis/livecd-rootfs/fix-for-1565985]

1421. By Louis Zuckerman on 2016-07-21

remove unnecessary virtualbox-guest-dkms package from vagrant image builder

[Louis Zuckerman (semiosis)]

Removed virtualbox-guest-dkms from the vagrant builder. Now the *only* extra package added to the vagrant box is virtualbox-guest-utils.

[Steve Langasek (vorlon)]

On Thu, Jul 21, 2016 at 10:52:49AM -0000, Dan Watkins wrote:

> > === modified file 'live-build/ubuntu-cpc/hooks/042-vagrant.binary'
> > --- live-build/ubuntu-cpc/hooks/042-vagrant.binary 2016-06-09 09:47:25 +0000
> > +++ live-build/ubuntu-cpc/hooks/042-vagrant.binary 2016-07-07 13:22:31 +0000
> > @@ -70,6 +78,7 @@
> > password: ${ubuntu_user_pass}
> > chpasswd: { expire: False }
> > ssh_pwauth: True
> > +manage_etc_hosts: localhost

> OK, I've finally had time to dig in to this. We don't actually create
> user-data in 999-cpc-fixes.chroot in the general case; it's created in
> fake_cloud_init which is only called when we're creating a Raspberry Pi
> image.

Oh, of course! Sorry :)

> We _could_ investigate applying this more generally, but I think it's
> appropriate for this to be a Vagrant-only fix until such a time as we can
> investigate more fully.

Ok, fine by me.

The only remaining issue I see with this merge is that one of my earlier
review comments seems to have been misunderstood; there's still an
invocation of apt-get that's using --no-install-recommends, which I think
needs to be accounted for. Louis, you dropped the use of
DEBIAN_FRONTEND=noninteractive, which was not my concern; that should be a
no-op at this level because you shouldn't need to set it in an individual
script, but it doesn't hurt anything. The real issue is using
--no-install-recommends. I've reviewed the Recommends of
virtualbox-guest-utils, and see that it has a (reasonable by default)
Recommends: on virtualbox-guest-x11, which we definitely do not want pulled
in to our cloud image.

So this use of --no-install-recommends is appropriate, and I will include a
comment in the script when merging to this effect.

[Louis Zuckerman (semiosis)]

Steve, I just saw your last comment. I addressed the recommends issue in an earlier comment but I guess it got lost among all the changes I pushed. You're right, we need --no-install-recommends so that we don't pull in X11.

Thanks!

[Louis Zuckerman (semiosis)]

Oh ok, I now see that my comments weren't saved. I'll try to save them now. Better late than never, right?

=== modified file 'debian/changelog'
--- debian/changelog	2016-07-21 08:11:04 +0000
+++ debian/changelog	2016-07-21 13:25:32 +0000
@@ -1,4 +1,19 @@
+<<<<<<< TREE
 livecd-rootfs (2.420) yakkety; urgency=medium
+=======
+livecd-rootfs (2.420ubuntu1) UNRELEASED; urgency=medium
+
+  [ Louis Zuckerman ]
+  * Fixes for vagrant box builder in ubuntu-cpc LP: #1565985
+    - Install virtualbox-guest-utils & virtualbox-guest-dkms
+    - Don't disable default synced folder
+    - Don't set vm name
+    - Add cloud-init config to manage /etc/hosts LP: #1561250
+
+ -- Louis Zuckerman <[e-mail]>  Wed, 06 Jul 2016 09:51:20 -0400
+
+livecd-rootfs (2.420) UNRELEASED; urgency=medium
+>>>>>>> MERGE-SOURCE
 
   [ Łukasz 'sil2100' Zemczak ]
   * Remove the ubuntu-pd project from the scripts

=== modified file 'live-build/ubuntu-cpc/hooks/042-vagrant.binary'
--- live-build/ubuntu-cpc/hooks/042-vagrant.binary	2016-06-09 09:47:25 +0000
+++ live-build/ubuntu-cpc/hooks/042-vagrant.binary	2016-07-21 13:25:32 +0000
@@ -1,22 +1,23 @@
-#!/bin/bash -eux
+#!/bin/bash -ex
 # vi: ts=4 noexpandtab
 #
 # Generate a generic Vagrant Box.
 #
-# Vagrant images are essentially nothing more than OVA's with extra-metadata.
-#
-# We can't use the OVA's for Vagrant since Vagrant uses SSH to modify the instance.
-# This build step creates a cloud-config ISO so that Cloud-Init will configure
-# the initial user, creates meta-data that tells Vagrant how to interact with
-# the cloud-init created users, and finally create the OVA.
-#
-# For this step, we re-use the VMDK's made in 040-vmdk-image.binary
+# Vagrant images are essentially nothing more than OVA's with extra-metadata
+# and some preinstalled packages.
+#
+# We can't use the OVA's for Vagrant since Vagrant uses SSH to modify the
+# instance. This build step creates a cloud-config ISO so that Cloud-Init
+# will configure the initial user, creates meta-data that tells Vagrant how
+# to interact with the cloud-init created users, and finally create the OVA.
+#
+# For this step, we make a deriviative of binary/boot/disk.ext4 and install
+# some packages in it, convert it to a vmdk, and then assemble the vagrant
+# box.
 
 cur_d=${PWD}
 my_d=$(dirname $(readlink -f ${0}))
 
-base_vmdk="livecd.ubuntu-cpc.vmdk"
-
 case $ARCH in
 	amd64|i386) ;;
 	*)
@@ -24,11 +25,6 @@
 		exit 0
 esac
 
-if [ ! -e ${base_vmdk} ]; then
-	echo "Did not find VMDK to produce Vagrant images."
-    exit 0
-fi
-
 . /build/config/functions
 
 # Virtualbox is needed for making a small VMDK
@@ -37,7 +33,20 @@
 # Lets be safe about this
 box_d=$(mktemp -d)
 seed_d=$(mktemp -d)
-trap "rm -rf ${box_d} ${seed_d}" EXIT
+mount_d=$(mktemp -d)
+
+create_derivative "disk" "vagrant" #sets ${derivative_img}
+mount_disk_image ${derivative_img} ${mount_d}
+
+cleanup_vagrant() {
+    umount_disk_image ${mount_d}
+    rm -rf ${box_d} ${seed_d} ${mount_d} ${derivative_img}
+}
+trap cleanup_vagrant EXIT
+
+chroot ${mount_d} apt-get update
+chroot ${mount_d} apt-get install --no-install-recommends -y virtualbox-guest-utils
+chroot ${mount_d} apt-get clean
 
 # Used to identify bits
 suite=$(chroot chroot lsb_release -c -s)
@@ -47,7 +56,7 @@
 # Get the VMDK in place
 prefix="${distro}-${suite}-${version}-cloudimg"
 vmdk_f="${box_d}/${prefix}.vmdk"
-cp ${base_vmdk} ${vmdk_f}
+create_vmdk ${derivative_img} ${vmdk_f}
 
 # Vagrant needs a base user. We either inject the well-known SSH key
 # or use password authentication. Both are ugly. So we'll use a password
@@ -70,6 +79,7 @@
 password: ${ubuntu_user_pass}
 chpasswd: { expire: False }
 ssh_pwauth: True
+manage_etc_hosts: localhost
 END
 
 # Create the fake meta-data
@@ -111,15 +121,11 @@
   config.vm.base_mac = "${macaddr}"
   config.ssh.username = "ubuntu"
   config.ssh.password = "${ubuntu_user_pass}"
-  config.vm.synced_folder '.', '/vagrant', disabled: true
 
   config.vm.provider "virtualbox" do |vb|
-     vb.name = "${prefix}"
      vb.customize [ "modifyvm", :id, "--uart1", "0x3F8", "4" ]
-     vb.customize [ "modifyvm", :id, "--uartmode1", "file", File.join(Dir.pwd, "%s-console.log" % vb.name) ]
+     vb.customize [ "modifyvm", :id, "--uartmode1", "file", File.join(Dir.pwd, "${prefix}-console.log") ]
   end
-
-
 end
 EOF
 
@@ -151,10 +157,10 @@
 serial_stamp=$(date +%Y%m%d)
 sed -i "${ovf}" \
     -e "s/@@NAME@@/${prefix}-${serial_stamp}/g" \
-	-e "s/@@FILENAME1@@/${vmdk_f##*/}/g" \
+    -e "s/@@FILENAME1@@/${vmdk_f##*/}/g" \
     -e "s/@@VMDK_FILE_SIZE@@/${vmdk_size}/g" \
     -e "s/@@VMDK_CAPACITY@@/${vmdk_capacity}/g" \
-	-e "s/@@FILENAME2@@/${cdrom_vmdk_f##*/}/g" \
+    -e "s/@@FILENAME2@@/${cdrom_vmdk_f##*/}/g" \
     -e "s/@@VMDK_FILE_SIZE2@@/${cdrom_size}/g" \
     -e "s/@@VMDK_CAPACITY2@@/${cdrom_capacity}/g" \
     -e "s/@@NUM_CPUS@@/2/g" \
@@ -181,16 +187,16 @@
     VMDK Name:      ${vmdk_f##*/}
     VMDK Capacity:  ${vmdk_capacity}
     VMDK SHA256:    ${vmdk_sha256}
-	CDROM Name:     ${cdrom_vmdk_f##*/}
+    CDROM Name:     ${cdrom_vmdk_f##*/}
     CDROM Capacity: ${cdrom_capacity}
-	CDROM SHA256:   ${cdrom_sha256}
+    CDROM SHA256:   ${cdrom_sha256}
 EOM
 
 tar -C ${box_d} \
     -cf ${cur_d}/livecd.ubuntu-cpc.vagrant.box \
     box.ovf \
-	Vagrantfile \
-	metadata.json \
+    Vagrantfile \
+    metadata.json \
     ${prefix}.mf \
     ${vmdk_f##*/} \
-	${cdrom_vmdk_f##*/}
+    ${cdrom_vmdk_f##*/}