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

Proposed by Louis Zuckerman
Status: Merged
Merged at revision: 1419
Proposed branch: lp:~semiosis/livecd-rootfs/fix-for-1565985
Merge into: lp:livecd-rootfs
Diff against target: 165 lines (+51/-30) (has conflicts)
2 files modified
debian/changelog (+15/-0)
live-build/ubuntu-cpc/hooks/042-vagrant.binary (+36/-30)
Text conflict in debian/changelog
To merge this branch: bzr merge lp:~semiosis/livecd-rootfs/fix-for-1565985
Reviewer Review Type Date Requested Status
Steve Langasek Needs Fixing
Dan Watkins (community) Approve
Review via email: mp+298305@code.launchpad.net

Description of the change

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!

To post a comment you must log in.
Louis Zuckerman (semiosis) wrote :

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 (oddbloke) wrote :

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 (oddbloke) wrote :

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
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 (oddbloke) wrote :

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) wrote :

> 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.

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) wrote :

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 (oddbloke) :
review: Approve
Steve Langasek (vorlon) wrote :

Questions inline.

review: Needs Fixing
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

Patricia Gaughen (gaughen) :
Dan Watkins (oddbloke) :
Adam Conrad (adconrad) wrote :

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

Adam Conrad (adconrad) wrote :

$ 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

1421. By Louis Zuckerman on 2016-07-21

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

Louis Zuckerman (semiosis) wrote :

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) wrote :

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) wrote :

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) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2016-07-21 08:11:04 +0000
3+++ debian/changelog 2016-07-21 13:25:32 +0000
4@@ -1,4 +1,19 @@
5+<<<<<<< TREE
6 livecd-rootfs (2.420) yakkety; urgency=medium
7+=======
8+livecd-rootfs (2.420ubuntu1) UNRELEASED; urgency=medium
9+
10+ [ Louis Zuckerman ]
11+ * Fixes for vagrant box builder in ubuntu-cpc LP: #1565985
12+ - Install virtualbox-guest-utils & virtualbox-guest-dkms
13+ - Don't disable default synced folder
14+ - Don't set vm name
15+ - Add cloud-init config to manage /etc/hosts LP: #1561250
16+
17+ -- Louis Zuckerman <me@louiszuckerman.com> Wed, 06 Jul 2016 09:51:20 -0400
18+
19+livecd-rootfs (2.420) UNRELEASED; urgency=medium
20+>>>>>>> MERGE-SOURCE
21
22 [ Łukasz 'sil2100' Zemczak ]
23 * Remove the ubuntu-pd project from the scripts
24
25=== modified file 'live-build/ubuntu-cpc/hooks/042-vagrant.binary'
26--- live-build/ubuntu-cpc/hooks/042-vagrant.binary 2016-06-09 09:47:25 +0000
27+++ live-build/ubuntu-cpc/hooks/042-vagrant.binary 2016-07-21 13:25:32 +0000
28@@ -1,22 +1,23 @@
29-#!/bin/bash -eux
30+#!/bin/bash -ex
31 # vi: ts=4 noexpandtab
32 #
33 # Generate a generic Vagrant Box.
34 #
35-# Vagrant images are essentially nothing more than OVA's with extra-metadata.
36-#
37-# We can't use the OVA's for Vagrant since Vagrant uses SSH to modify the instance.
38-# This build step creates a cloud-config ISO so that Cloud-Init will configure
39-# the initial user, creates meta-data that tells Vagrant how to interact with
40-# the cloud-init created users, and finally create the OVA.
41-#
42-# For this step, we re-use the VMDK's made in 040-vmdk-image.binary
43+# Vagrant images are essentially nothing more than OVA's with extra-metadata
44+# and some preinstalled packages.
45+#
46+# We can't use the OVA's for Vagrant since Vagrant uses SSH to modify the
47+# instance. This build step creates a cloud-config ISO so that Cloud-Init
48+# will configure the initial user, creates meta-data that tells Vagrant how
49+# to interact with the cloud-init created users, and finally create the OVA.
50+#
51+# For this step, we make a deriviative of binary/boot/disk.ext4 and install
52+# some packages in it, convert it to a vmdk, and then assemble the vagrant
53+# box.
54
55 cur_d=${PWD}
56 my_d=$(dirname $(readlink -f ${0}))
57
58-base_vmdk="livecd.ubuntu-cpc.vmdk"
59-
60 case $ARCH in
61 amd64|i386) ;;
62 *)
63@@ -24,11 +25,6 @@
64 exit 0
65 esac
66
67-if [ ! -e ${base_vmdk} ]; then
68- echo "Did not find VMDK to produce Vagrant images."
69- exit 0
70-fi
71-
72 . /build/config/functions
73
74 # Virtualbox is needed for making a small VMDK
75@@ -37,7 +33,20 @@
76 # Lets be safe about this
77 box_d=$(mktemp -d)
78 seed_d=$(mktemp -d)
79-trap "rm -rf ${box_d} ${seed_d}" EXIT
80+mount_d=$(mktemp -d)
81+
82+create_derivative "disk" "vagrant" #sets ${derivative_img}
83+mount_disk_image ${derivative_img} ${mount_d}
84+
85+cleanup_vagrant() {
86+ umount_disk_image ${mount_d}
87+ rm -rf ${box_d} ${seed_d} ${mount_d} ${derivative_img}
88+}
89+trap cleanup_vagrant EXIT
90+
91+chroot ${mount_d} apt-get update
92+chroot ${mount_d} apt-get install --no-install-recommends -y virtualbox-guest-utils
93+chroot ${mount_d} apt-get clean
94
95 # Used to identify bits
96 suite=$(chroot chroot lsb_release -c -s)
97@@ -47,7 +56,7 @@
98 # Get the VMDK in place
99 prefix="${distro}-${suite}-${version}-cloudimg"
100 vmdk_f="${box_d}/${prefix}.vmdk"
101-cp ${base_vmdk} ${vmdk_f}
102+create_vmdk ${derivative_img} ${vmdk_f}
103
104 # Vagrant needs a base user. We either inject the well-known SSH key
105 # or use password authentication. Both are ugly. So we'll use a password
106@@ -70,6 +79,7 @@
107 password: ${ubuntu_user_pass}
108 chpasswd: { expire: False }
109 ssh_pwauth: True
110+manage_etc_hosts: localhost
111 END
112
113 # Create the fake meta-data
114@@ -111,15 +121,11 @@
115 config.vm.base_mac = "${macaddr}"
116 config.ssh.username = "ubuntu"
117 config.ssh.password = "${ubuntu_user_pass}"
118- config.vm.synced_folder '.', '/vagrant', disabled: true
119
120 config.vm.provider "virtualbox" do |vb|
121- vb.name = "${prefix}"
122 vb.customize [ "modifyvm", :id, "--uart1", "0x3F8", "4" ]
123- vb.customize [ "modifyvm", :id, "--uartmode1", "file", File.join(Dir.pwd, "%s-console.log" % vb.name) ]
124+ vb.customize [ "modifyvm", :id, "--uartmode1", "file", File.join(Dir.pwd, "${prefix}-console.log") ]
125 end
126-
127-
128 end
129 EOF
130
131@@ -151,10 +157,10 @@
132 serial_stamp=$(date +%Y%m%d)
133 sed -i "${ovf}" \
134 -e "s/@@NAME@@/${prefix}-${serial_stamp}/g" \
135- -e "s/@@FILENAME1@@/${vmdk_f##*/}/g" \
136+ -e "s/@@FILENAME1@@/${vmdk_f##*/}/g" \
137 -e "s/@@VMDK_FILE_SIZE@@/${vmdk_size}/g" \
138 -e "s/@@VMDK_CAPACITY@@/${vmdk_capacity}/g" \
139- -e "s/@@FILENAME2@@/${cdrom_vmdk_f##*/}/g" \
140+ -e "s/@@FILENAME2@@/${cdrom_vmdk_f##*/}/g" \
141 -e "s/@@VMDK_FILE_SIZE2@@/${cdrom_size}/g" \
142 -e "s/@@VMDK_CAPACITY2@@/${cdrom_capacity}/g" \
143 -e "s/@@NUM_CPUS@@/2/g" \
144@@ -181,16 +187,16 @@
145 VMDK Name: ${vmdk_f##*/}
146 VMDK Capacity: ${vmdk_capacity}
147 VMDK SHA256: ${vmdk_sha256}
148- CDROM Name: ${cdrom_vmdk_f##*/}
149+ CDROM Name: ${cdrom_vmdk_f##*/}
150 CDROM Capacity: ${cdrom_capacity}
151- CDROM SHA256: ${cdrom_sha256}
152+ CDROM SHA256: ${cdrom_sha256}
153 EOM
154
155 tar -C ${box_d} \
156 -cf ${cur_d}/livecd.ubuntu-cpc.vagrant.box \
157 box.ovf \
158- Vagrantfile \
159- metadata.json \
160+ Vagrantfile \
161+ metadata.json \
162 ${prefix}.mf \
163 ${vmdk_f##*/} \
164- ${cdrom_vmdk_f##*/}
165+ ${cdrom_vmdk_f##*/}

Subscribers

People subscribed via source and target branches