Merge lp:~doanac/ubuntu-ci-services-itself/imgbuild-kernel-fixes into lp:ubuntu-ci-services-itself

Proposed by Andy Doan
Status: Merged
Approved by: Andy Doan
Approved revision: 404
Merged at revision: 409
Proposed branch: lp:~doanac/ubuntu-ci-services-itself/imgbuild-kernel-fixes
Merge into: lp:ubuntu-ci-services-itself
Diff against target: 144 lines (+40/-13)
3 files modified
image-builder/imagebuilder/cloud_image.py (+32/-11)
image-builder/imagebuilder/tests/test_modify_cloud_image.py (+5/-2)
juju-deployer/configs/unit_config.yaml.tmpl (+3/-0)
To merge this branch: bzr merge lp:~doanac/ubuntu-ci-services-itself/imgbuild-kernel-fixes
Reviewer Review Type Date Requested Status
Paul Larson Approve
PS Jenkins bot (community) continuous-integration Approve
Evan (community) Approve
Review via email: mp+211178@code.launchpad.net

Commit message

image-builder: fixes required to get kernel testing working

Description of the change

This fixes the issues I've discovered in the image-builder. The whole diff is small, but it might make more sense to read each commit on its own to see *why* I made these chages.

To post a comment you must log in.
Revision history for this message
Evan (ev) wrote :

57 + _chrooted(chroot, 'apt-get install -y {}'.format(' '.join(pkglist)))

We have all the apt keys imported and the cache updated, yes? I'm typing this from my iPad, otherwise I'd check myself.

139 +image_size: 4G

Can we have a comment documenting what this does and why it's set to 4G?

Otherwise +1. Good stuff.

Have you checked with Paul on why we had those dpkg's options set in the first place?

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:403
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/438/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/438/rebuild

review: Approve (continuous-integration)
Revision history for this message
Paul Larson (pwlars) wrote :

> Have you checked with Paul on why we had those dpkg's options set in the first
> place?
Yes, those were set to get around grub wanting to give an interactive prompt to decide what to do about the config file. From what Andy and I discussed earlier on IRC, it should be ok to remove them since he has 'etc/default/grub.d/99-ci-eng.cfg' getting created and used, but I'm working on testing it now using this branch.

404. By Andy Doan

document the 4G size in our unit_config

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:404
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/439/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/439/rebuild

review: Approve (continuous-integration)
Revision history for this message
Paul Larson (pwlars) wrote :

I've been testing this over the weekend, and it's working well for me +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'image-builder/imagebuilder/cloud_image.py'
--- image-builder/imagebuilder/cloud_image.py 2014-03-15 13:37:37 +0000
+++ image-builder/imagebuilder/cloud_image.py 2014-03-15 23:16:04 +0000
@@ -19,7 +19,6 @@
19import os19import os
20import re20import re
21import shutil21import shutil
22import string
23import subprocess22import subprocess
24import textwrap23import textwrap
25import time24import time
@@ -80,22 +79,30 @@
80 log.info('Running command in image: {0}'.format(cmd))79 log.info('Running command in image: {0}'.format(cmd))
81 if type(cmd) is not list:80 if type(cmd) is not list:
82 cmd = cmd.split(' ')81 cmd = cmd.split(' ')
83 output = subprocess.check_output(['chroot', chroot] + cmd)82 output = subprocess.check_output(
83 ['chroot', chroot] + cmd, stderr=subprocess.STDOUT)
84 if output:84 if output:
85 log.info(output)85 log.info(output)
8686
8787
88def _apt_get_update(chroot):88def _apt_get_update(chroot):
89 # apt-get update isn't always reliable give it 3 tries:89 # apt-get update isn't always reliable, give it a few tries
90 for x in (2, 1, 0):90 retries = 5
91 for x in range(retries):
91 try:92 try:
92 _chrooted(chroot, "/usr/bin/apt-get update")93 _chrooted(chroot, "/usr/bin/apt-get update")
93 break94 break
94 except subprocess.CalledProcessError as e:95 except subprocess.CalledProcessError as e:
95 if x == 0:96 x += 1
97 if x == retries:
96 raise e98 raise e
97 else:99 else:
98 log.info('apt-get update failed, retrying %d more times', x)100 # retry delays will be 1, 16, 81, 256, 625
101 # ie about 1s, 16s, 1.5m, 4m, 10m
102 timeout = x * x * x * x
103 fmt = 'apt-get update failed, wait %ds and try %d more times'
104 log.info(fmt, timeout, retries - x)
105 time.sleep(timeout)
99106
100107
101def _setup_ppas(chroot, repos, series, status_cb):108def _setup_ppas(chroot, repos, series, status_cb):
@@ -124,11 +131,7 @@
124 '''131 '''
125 status_cb('Adding requested packages to the image...')132 status_cb('Adding requested packages to the image...')
126 os.environ['DEBIAN_FRONTEND'] = 'noninteractive'133 os.environ['DEBIAN_FRONTEND'] = 'noninteractive'
127 _chrooted(chroot,134 _chrooted(chroot, 'apt-get install -y {}'.format(' '.join(pkglist)))
128 ['/bin/bash', '-c',
129 ('/usr/bin/apt-get -o Dpkg::Options::=\"--force-confdef\" '
130 '-o Dpkg::Options::=\"--force-confold\" install -y '
131 '{pkglist}'.format(pkglist=string.join(pkglist, " ")))])
132 _chrooted(chroot, "/usr/bin/apt-get clean")135 _chrooted(chroot, "/usr/bin/apt-get clean")
133136
134 #create a big null data file and remove it to make it more compressable137 #create a big null data file and remove it to make it more compressable
@@ -144,6 +147,19 @@
144 os.unlink(zerofile)147 os.unlink(zerofile)
145148
146149
150def _setup_grub(mountpoint, status_cb):
151 '''ensure grub uses the proper rootfs
152
153 When installing kernels grub will reconfigure but set the wrong root
154 device to boot from. This ensures its probed correctly
155 '''
156 status_cb('ensuring grub uses proper rootfs')
157 path = os.path.join(mountpoint, 'etc/default/grub.d/99-ci-eng.cfg')
158 with open(path, 'w') as f:
159 f.write('LINUX_ROOT_DEVICE="LABEL=cloudimg-rootfs"\n')
160 f.write('GRUB_DEVICE="LABEL=cloudimg-rootfs"\n')
161
162
147def _download(url, path):163def _download(url, path):
148 urlpath = urlparse.urlsplit(url).path164 urlpath = urlparse.urlsplit(url).path
149 filename = os.path.basename(urlpath)165 filename = os.path.basename(urlpath)
@@ -166,6 +182,9 @@
166182
167class CloudImage(object):183class CloudImage(object):
168 def __init__(self, image_path, tmpdir):184 def __init__(self, image_path, tmpdir):
185 image_size = ci_utils.unit_config.get('image_size')
186 log.info('resizing image to be %s', image_size)
187 subprocess.check_call(['qemu-img', 'resize', image_path, image_size])
169 self.mountpoint = self._mount_image(image_path, tmpdir)188 self.mountpoint = self._mount_image(image_path, tmpdir)
170189
171 def __enter__(self):190 def __enter__(self):
@@ -241,6 +260,8 @@
241 with CloudImage(image_path, tmpdir) as mountpoint:260 with CloudImage(image_path, tmpdir) as mountpoint:
242 status_cb('updating image %s...' % image)261 status_cb('updating image %s...' % image)
243262
263 _setup_grub(mountpoint, status_cb)
264
244 _setup_ppas(mountpoint, repos, series, status_cb)265 _setup_ppas(mountpoint, repos, series, status_cb)
245266
246 _run_chroot_cmds(mountpoint, packages, status_cb)267 _run_chroot_cmds(mountpoint, packages, status_cb)
247268
=== modified file 'image-builder/imagebuilder/tests/test_modify_cloud_image.py'
--- image-builder/imagebuilder/tests/test_modify_cloud_image.py 2014-03-15 03:22:48 +0000
+++ image-builder/imagebuilder/tests/test_modify_cloud_image.py 2014-03-15 23:16:04 +0000
@@ -43,14 +43,16 @@
43 expected = ['deb foo_url precise main', 'deb-src foo_url precise main']43 expected = ['deb foo_url precise main', 'deb-src foo_url precise main']
44 self.assertListEqual(expected, deblines)44 self.assertListEqual(expected, deblines)
4545
46 @mock.patch('time.sleep')
46 @mock.patch('imagebuilder.cloud_image._chrooted')47 @mock.patch('imagebuilder.cloud_image._chrooted')
47 def test_apt_get_update_fails(self, chrooted):48 def test_apt_get_update_fails(self, chrooted, sleep):
48 chrooted.side_effect = subprocess.CalledProcessError(1, 'cmd', 'out')49 chrooted.side_effect = subprocess.CalledProcessError(1, 'cmd', 'out')
49 with self.assertRaises(subprocess.CalledProcessError):50 with self.assertRaises(subprocess.CalledProcessError):
50 cloud_image._apt_get_update('/blah')51 cloud_image._apt_get_update('/blah')
5152
53 @mock.patch('time.sleep')
52 @mock.patch('imagebuilder.cloud_image._chrooted')54 @mock.patch('imagebuilder.cloud_image._chrooted')
53 def test_apt_get_update_retries(self, chrooted):55 def test_apt_get_update_retries(self, chrooted, sleep):
54 '''ensure it can apt-get update works after retrying'''56 '''ensure it can apt-get update works after retrying'''
55 chrooted.side_effect = [57 chrooted.side_effect = [
56 subprocess.CalledProcessError(1, 'cmd', 'out'),58 subprocess.CalledProcessError(1, 'cmd', 'out'),
@@ -59,6 +61,7 @@
59 ]61 ]
60 cloud_image._apt_get_update('/blah')62 cloud_image._apt_get_update('/blah')
61 self.assertEqual(3, chrooted.call_count)63 self.assertEqual(3, chrooted.call_count)
64 self.assertEqual(2, sleep.call_count)
6265
63 @mock.patch('imagebuilder.cloud_image.launchpad')66 @mock.patch('imagebuilder.cloud_image.launchpad')
64 @mock.patch('imagebuilder.cloud_image._chrooted')67 @mock.patch('imagebuilder.cloud_image._chrooted')
6568
=== modified file 'juju-deployer/configs/unit_config.yaml.tmpl'
--- juju-deployer/configs/unit_config.yaml.tmpl 2014-03-14 23:53:04 +0000
+++ juju-deployer/configs/unit_config.yaml.tmpl 2014-03-15 23:16:04 +0000
@@ -21,6 +21,9 @@
2121
22# for ticket system22# for ticket system
23base_image: "http://cloud-images.ubuntu.com/releases/13.10/release-20131015/ubuntu-13.10-server-cloudimg-amd64-disk1.img"23base_image: "http://cloud-images.ubuntu.com/releases/13.10/release-20131015/ubuntu-13.10-server-cloudimg-amd64-disk1.img"
24# Cloud images are 2G. We need more space for the packages that might
25# be added with tickets.
26image_size: 4G
24series: saucy27series: saucy
25master_ppa: $CI_MASTER_PPA28master_ppa: $CI_MASTER_PPA
26tr_flavors:29tr_flavors:

Subscribers

People subscribed via source and target branches