Merge lp:~pwlars/uci-engine/unmodified-imagebuild into lp:uci-engine

Proposed by Paul Larson on 2014-07-21
Status: Rejected
Rejected by: Paul Larson on 2014-07-23
Proposed branch: lp:~pwlars/uci-engine/unmodified-imagebuild
Merge into: lp:uci-engine
Diff against target: 549 lines (+17/-412)
5 files modified
image-builder/imagebuilder/cloud_image.py (+7/-283)
image-builder/imagebuilder/run_worker.py (+4/-9)
image-builder/imagebuilder/tests/test_modify_cloud_image.py (+0/-108)
image-builder/imagebuilder/tests/test_worker.py (+5/-11)
image-builder/setup.py (+1/-1)
To merge this branch: bzr merge lp:~pwlars/uci-engine/unmodified-imagebuild
Reviewer Review Type Date Requested Status
Vincent Ladeuil (community) 2014-07-21 Approve on 2014-07-21
PS Jenkins bot (community) continuous-integration Approve on 2014-07-21
Review via email: mp+227557@code.launchpad.net

Commit message

Strip the image builder down to just ensure we have the proper image in glance, rather than trying to modify it.

Description of the change

Putting this out as an MP so it's easier to look at, but this requires some other changes to the test runner also:
https://code.launchpad.net/~vila/uci-engine/new-tr-api-lander/+merge/227510
https://code.launchpad.net/~vila/uci-engine/tr-new-api/+merge/227542

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :

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

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

review: Approve (continuous-integration)
Vincent Ladeuil (vila) wrote :

Nice cleanup !

See comments inline.

There is still one thing surprising me here, shouldn't you change the lander too to remove the now unused parameters ?

I'll test that branch with my MPs but I expect it to work, so +1 for now ;)

Once we're both ok with them, we should look at the API transition stuff Evan mentioned.

review: Approve
Paul Larson (pwlars) wrote :

I think I'm going to supersede this version in favor of https://code.launchpad.net/~pwlars/uci-engine/versioned-imagebuild/+merge/227666 after our discussion about API versioning today. Also, the other version should have the lander changes needed.

Unmerged revisions

694. By Paul Larson on 2014-07-18

Modification of cloud images is no longer necessary. Now all we need to do is ensure the image is in glance, and adt-run takes care of the rest

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'image-builder/imagebuilder/cloud_image.py'
2--- image-builder/imagebuilder/cloud_image.py 2014-07-17 16:00:41 +0000
3+++ image-builder/imagebuilder/cloud_image.py 2014-07-21 13:55:48 +0000
4@@ -15,23 +15,17 @@
5 # along with this program. If not, see <http://www.gnu.org/licenses/>.
6
7 import argparse
8-import fcntl
9 import logging
10 import os
11-import re
12-import shutil
13 import subprocess
14-import textwrap
15-import time
16 import urllib
17 import urlparse
18 import uuid
19
20 import ci_utils
21
22-from imagebuilder import ImageException
23 from ci_utils.image_store import ImageStore
24-from ci_utils import dump_stack, data_store, launchpad
25+from ci_utils import dump_stack
26
27
28 log = logging.getLogger('Imagebuilder')
29@@ -42,144 +36,11 @@
30 parser.add_argument(
31 "-i", "--image", required=True,
32 help="URL for the cloud image to modify"),
33- parser.add_argument(
34- "-r", "--repository", required=True, action='append',
35- help="List of PPAs to add")
36- parser.add_argument(
37- "-p", "--package", required=True, action='append',
38- help="List of extra packages to install")
39- parser.add_argument(
40- "-s", "--series", required=True,
41- help="Series of Ubuntu the image is based on")
42
43 args = parser.parse_args()
44 return args
45
46
47-def _parse_ppas(lp, ppalist, series):
48- aptlines = []
49- keys = {}
50- for ppa in ppalist:
51- ppamatch = re.match("ppa:(.+)/(.+)", ppa)
52- # This will throw an exception for malformed ppas, intentional for now
53- (team, repo) = ppamatch.groups()
54- ppa = lp.people[team].getPPAByName(name=repo)
55- keys[ppa.name] = ppa.signing_key_fingerprint
56- if ppa.private:
57- # lp.me won't work, you have to get yourself by person
58- user = lp.people[launchpad.get_lp_user_name()]
59- url = user.getArchiveSubscriptionURL(archive=ppa)
60- else:
61- url = 'http://ppa.launchpad.net/{}/{}/ubuntu'.format(team, repo)
62- aptlines.append('deb {} {} main'.format(url, series))
63- aptlines.append('deb-src {} {} main'.format(url, series))
64- return aptlines, keys
65-
66-
67-def _chrooted(chroot, cmd):
68- log.info('Running command in image: {0}'.format(cmd))
69- if type(cmd) is not list:
70- cmd = cmd.split(' ')
71- output = subprocess.check_output(
72- ['chroot', chroot] + cmd, stderr=subprocess.STDOUT)
73- if output:
74- log.info(output)
75-
76-
77-def _apt_get_update(chroot):
78- # apt-get update isn't always reliable, give it a few tries
79- retries = 5
80- for x in range(retries):
81- try:
82- _chrooted(chroot, "/usr/bin/apt-get update")
83- break
84- except subprocess.CalledProcessError as e:
85- x += 1
86- if x == retries:
87- raise e
88- else:
89- # retry delays will be 1, 16, 81, 256, 625
90- # ie about 1s, 16s, 1.5m, 4m, 10m
91- timeout = x * x * x * x
92- fmt = 'apt-get update failed, wait %ds and try %d more times'
93- log.info(fmt, timeout, retries - x)
94- time.sleep(timeout)
95-
96-
97-def _setup_apt_mirror(status_cb, fname):
98- try:
99- mirror = ci_utils.unit_config.get('apt_mirror')
100- except KeyError:
101- status_cb('no apt mirror configured')
102- return False
103- status_cb('updating apt mirror to: {}'.format(mirror))
104- with open(fname) as f:
105- lines = f.readlines()
106- with open(fname, 'w') as f:
107- for line in lines:
108- f.write(line.replace('archive.ubuntu.com', mirror))
109- return True
110-
111-
112-def _setup_ppas(chroot, repos, series, status_cb):
113- status_cb('importing ppa signing keys...')
114- lp = launchpad.lp_login()
115-
116- status_cb('grabbing ppa subscription urls and keys...')
117- deblines, keys = _parse_ppas(lp, repos, series)
118-
119- for ppa, key in keys.iteritems():
120- status_cb('importing signing key for: %s' % ppa)
121- cmd_base = 'apt-key adv --keyserver keyserver.ubuntu.com --recv-keys '
122- _chrooted(chroot, cmd_base + key)
123-
124- status_cb('adding ppas to apt list...')
125- with open('%s/etc/apt/sources.list.d/ci.list' % chroot, 'a+') as f:
126- for debline in deblines:
127- f.write('\n%s' % debline)
128-
129- status_cb('running apt-get-update...')
130- _apt_get_update(chroot)
131-
132-
133-def _run_chroot_cmds(chroot, pkglist, status_cb):
134- '''Run commands in the chroot to do update the sources and add packages
135- '''
136- status_cb('Adding requested packages to the image...')
137- os.environ['DEBIAN_FRONTEND'] = 'noninteractive'
138- _chrooted(chroot, 'apt-get install -y {}'.format(' '.join(pkglist)))
139- _chrooted(chroot, "/usr/bin/apt-get clean")
140-
141- # create a big null data file and remove it to make it more compressable
142- status_cb('updating chroot to be more compressible...')
143- zerofile = '%s/zero.tmp' % chroot
144- filldata = '\0' * 4096
145- with open(zerofile, 'wb') as f:
146- while True:
147- try:
148- f.write(filldata)
149- except IOError:
150- break
151- os.unlink(zerofile)
152-
153-
154-def _setup_grub(mountpoint, status_cb):
155- '''ensure grub uses the proper rootfs
156-
157- When installing kernels grub will reconfigure but set the wrong root
158- device to boot from. This ensures its probed correctly
159- '''
160- status_cb('ensuring grub uses proper rootfs')
161- path = os.path.join(mountpoint, 'etc/default/grub.d/99-ci-eng.cfg')
162- if not os.path.exists(path):
163- # FIXME: On precise, this all appears to have changed, but I
164- # don't know yet whether this will break kernel installs
165- return
166- with open(path, 'w') as f:
167- f.write('LINUX_ROOT_DEVICE="LABEL=cloudimg-rootfs"\n')
168- f.write('GRUB_DEVICE="LABEL=cloudimg-rootfs"\n')
169-
170-
171 def _download(url, path):
172 urlpath = urlparse.urlsplit(url).path
173 filename = os.path.basename(urlpath)
174@@ -188,160 +49,24 @@
175 return filename
176
177
178-def _nbd_loaded():
179- log.info('Checking to see if nbd is loaded')
180- with open('/proc/modules') as f:
181- if '\nnbd ' in f.read():
182- return True
183- else:
184- return False
185-
186-
187-def _load_nbd():
188- '''load the nbd kernel module unless it's loaded already'''
189- if not _nbd_loaded():
190- runcmd('modprobe nbd')
191- if not _nbd_loaded():
192- raise ImageException(
193- 'nbd module is unavailable and could not be loaded')
194-
195-
196-def runcmd(cmd):
197- log.info('Running command: {0}'.format(cmd))
198- output = subprocess.check_output(cmd.split(' '))
199- if output:
200- log.info(output)
201-
202-
203-class CloudImage(object):
204- def __init__(self, image_path, tmpdir):
205- image_size = ci_utils.unit_config.get('image_size')
206-
207- # we need to ensure only one of these runs on the system
208- log.info('waiting for cloud image lock')
209- self.lockfile = open('/tmp/cloudimage.lck', 'a+')
210- fcntl.lockf(self.lockfile, fcntl.LOCK_EX)
211-
212- log.info('resizing image to be %s', image_size)
213- subprocess.check_call(['qemu-img', 'resize', image_path, image_size])
214- self.mountpoint = self._mount_image(image_path, tmpdir)
215-
216- def __enter__(self):
217- return self.mountpoint
218-
219- def __exit__(self, exc, value, tb):
220- self._umount_image()
221- fcntl.lockf(self.lockfile, fcntl.LOCK_UN)
222- self.lockfile.close()
223-
224- def _mount_image(self, image_path, tmpdir):
225- # Find all the unused network block devices
226- mountpoint = '%s/mountpoint' % tmpdir
227- os.mkdir(mountpoint)
228- nbd_devs = [
229- dev for dev in os.listdir('/sys/block') if dev.startswith('nbd')
230- and not os.path.exists('/sys/block/%s/pid' % dev)
231- ]
232- self.nbd_dev = nbd_devs[0]
233- runcmd('qemu-nbd -c /dev/%s %s' % (self.nbd_dev, image_path))
234- for x in xrange(10):
235- if os.path.exists('/sys/block/%s/pid' % self.nbd_dev):
236- break
237- time.sleep(1)
238- else:
239- raise ImageException('Block device %s never became available' %
240- self.nbd_dev)
241- runcmd('udevadm settle')
242- runcmd('mount /dev/%sp1 %s' % (self.nbd_dev, mountpoint))
243-
244- # Mount /proc /sys and /dev in the image
245- for bmount in ['/proc', '/sys', '/dev']:
246- runcmd('mount --bind %s %s/%s' % (bmount, mountpoint, bmount))
247-
248- # Creating /run/lock
249- run_lock = os.path.join(mountpoint, 'run', 'lock')
250- os.mkdir(run_lock)
251- runcmd(
252- 'mount -t tmpfs -o nodev,noexec,nosuid,size=5242880 none '
253- '{}'.format(run_lock))
254- # Backup resolv.conf and copy in the one from the local system
255- if os.path.lexists('%s/etc/resolv.conf' % mountpoint):
256- shutil.move('%s/etc/resolv.conf' % mountpoint,
257- '%s/etc/resolv.conf.orig' % mountpoint)
258- shutil.copy('/etc/resolv.conf', '%s/etc/resolv.conf' % mountpoint)
259- policyfile = textwrap.dedent("""\
260- #!/bin/sh
261- while true; do
262- case "$1" in
263- -*) shift ;;
264- makedev) exit 0;;
265- x11-common) exit 0;;
266- *) exit 101;;
267- esac
268- done""")
269- with open('%s/usr/sbin/policy-rc.d' % mountpoint, 'w') as f:
270- f.write(policyfile)
271- os.chmod('%s/usr/sbin/policy-rc.d' % mountpoint, 0o755)
272-
273- return mountpoint
274-
275- def _umount_image(self):
276- for bmount in ['/proc', '/sys', '/dev']:
277- runcmd('umount %s/%s' % (self.mountpoint, bmount))
278- # Removing /run/lock
279- run_lock = os.path.join(self.mountpoint, 'run', 'lock')
280- runcmd('umount {}'.format(run_lock))
281- shutil.rmtree(run_lock)
282- if os.path.lexists('%s/etc/resolv.conf.orig' % self.mountpoint):
283- shutil.move('%s/etc/resolv.conf.orig' % self.mountpoint,
284- '%s/etc/resolv.conf' % self.mountpoint)
285- os.unlink('%s/usr/sbin/policy-rc.d' % self.mountpoint)
286- runcmd('umount %s' % self.mountpoint)
287- runcmd('qemu-nbd -d /dev/%s' % self.nbd_dev)
288-
289-
290-def modify_image(image, repos, series, packages, ds, status_cb=None):
291- '''Add packges and PPAs to a cloud image mounted in a chroot'''
292+def get_image(image, status_cb=None):
293+ '''Copy the requested cloud image to glance'''
294 if not status_cb:
295 status_cb = lambda x: 0 # just provide a no-op
296
297- status_cb('loading nbd...')
298- _load_nbd()
299 with ci_utils.TmpDir() as tmpdir:
300 status_cb('downloading %s...' % image)
301 image_path = _download(image, tmpdir)
302- with CloudImage(image_path, tmpdir) as mountpoint:
303- status_cb('updating image %s...' % image)
304-
305- _setup_grub(mountpoint, status_cb)
306-
307- _setup_apt_mirror(
308- status_cb, os.path.join(mountpoint, 'etc/apt/sources.list'))
309- _setup_ppas(mountpoint, repos, series, status_cb)
310-
311- _run_chroot_cmds(mountpoint, packages, status_cb)
312
313 glance = ImageStore(ci_utils.unit_config.get_auth_config())
314- converted_image_path = os.path.join(tmpdir, 'new.img')
315- status_cb('Shrinking the converted image...')
316- runcmd('qemu-img convert -O qcow2 -c %s %s' % (
317- image_path, converted_image_path))
318 upload_filename = str(uuid.uuid1()) + '.img'
319 status_cb(
320- 'Uploading converted image to glance as %s' % upload_filename)
321- glance.image_create(upload_filename, converted_image_path)
322- try:
323- ds.change_visibility()
324- with open(converted_image_path, 'r') as fp:
325- location = ds.put_file(upload_filename, fp)
326- except data_store.DataStoreException:
327- status_cb('ERROR: unable to upload image to swift')
328- return upload_filename, location
329+ 'Uploading image to glance as %s' % upload_filename)
330+ glance.image_create(upload_filename, image_path)
331+ return upload_filename
332
333
334 def main():
335- import mock
336- ds = mock.Mock()
337 dump_stack.install_stack_dump_signal()
338 args = _get_args()
339
340@@ -349,8 +74,7 @@
341 print(msg)
342
343 try:
344- return modify_image(args.image, args.repository, args.series,
345- args.package, ds, cb)
346+ return get_image(args.image, cb)
347 except subprocess.CalledProcessError as e:
348 print(e.output)
349
350
351=== modified file 'image-builder/imagebuilder/run_worker.py'
352--- image-builder/imagebuilder/run_worker.py 2014-07-15 20:20:00 +0000
353+++ image-builder/imagebuilder/run_worker.py 2014-07-21 13:55:48 +0000
354@@ -18,7 +18,7 @@
355
356 from ci_utils import amqp_utils, amqp_worker
357
358-from imagebuilder.cloud_image import modify_image
359+from imagebuilder.cloud_image import get_image
360
361
362 class ImageBuildWorker(amqp_worker.AMQPWorker):
363@@ -48,21 +48,16 @@
364
365 def handle_request(self, params, log):
366 image = params['image']
367- repos = params['ppa_list']
368- series = params['series']
369- packages = params['package_list']
370+ # FIXME: We will need the series again soon
371+ # series = params['series']
372 trigger = params['progress_trigger']
373- ticket_id = params['ticket_id']
374- ds = self._create_data_store(ticket_id)
375
376 def status_cb(msg):
377 log.info(msg)
378 amqp_utils.progress_update(trigger, {'message': msg})
379
380- image_id, location = modify_image(image, repos, series, packages,
381- ds, status_cb)
382+ image_id = get_image(image, status_cb)
383 retval = {'image_id': image_id}
384- self._create_artifact(image_id, location, retval, type='IMAGE')
385 return amqp_utils.progress_completed, retval
386
387
388
389=== removed file 'image-builder/imagebuilder/tests/test_modify_cloud_image.py'
390--- image-builder/imagebuilder/tests/test_modify_cloud_image.py 2014-04-16 18:36:03 +0000
391+++ image-builder/imagebuilder/tests/test_modify_cloud_image.py 1970-01-01 00:00:00 +0000
392@@ -1,108 +0,0 @@
393-# Ubuntu CI Engine
394-# Copyright 2014 Canonical Ltd.
395-
396-# This program is free software: you can redistribute it and/or modify it
397-# under the terms of the GNU Affero General Public License version 3, as
398-# published by the Free Software Foundation.
399-
400-# This program is distributed in the hope that it will be useful, but
401-# WITHOUT ANY WARRANTY; without even the implied warranties of
402-# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
403-# PURPOSE. See the GNU Affero General Public License for more details.
404-
405-# You should have received a copy of the GNU Affero General Public License
406-# along with this program. If not, see <http://www.gnu.org/licenses/>.
407-
408-import subprocess
409-import tempfile
410-import textwrap
411-import unittest
412-
413-import mock
414-
415-from imagebuilder import cloud_image
416-
417-
418-class TestCloudImage(unittest.TestCase):
419- def setUp(self):
420- super(TestCloudImage, self).setUp()
421-
422- @mock.patch('ci_utils.unit_config')
423- def test_apt_mirror_not_configured(self, unit_config):
424- unit_config.get.side_effect = KeyError('apt_mirror')
425- status_cb = mock.Mock()
426- self.assertFalse(cloud_image._setup_apt_mirror(status_cb, '/ignored'))
427-
428- @mock.patch('ci_utils.unit_config')
429- def test_apt_mirror_configured(self, unit_config):
430- unit_config.get.return_value = 'foo.archive.ubuntu.com'
431- status_cb = mock.Mock()
432-
433- orig = textwrap.dedent('''\
434- deb http://archive.ubuntu.com/ubuntu saucy main restricted universe
435- deb http://archive.ubuntu.com/ubuntu saucy-updates main restricted
436- deb http://security.ubuntu.com/ubuntu saucy-security main restricted
437- ''')
438- expected = textwrap.dedent('''\
439- deb http://foo.archive.ubuntu.com/ubuntu saucy main restricted universe
440- deb http://foo.archive.ubuntu.com/ubuntu saucy-updates main restricted
441- deb http://security.ubuntu.com/ubuntu saucy-security main restricted
442- ''')
443-
444- with tempfile.NamedTemporaryFile() as f:
445- f.write(orig)
446- f.flush()
447- self.assertTrue(cloud_image._setup_apt_mirror(status_cb, f.name))
448- with open(f.name) as f:
449- self.assertEqual(expected, f.read())
450-
451- @mock.patch('imagebuilder.cloud_image.launchpad')
452- def test_parse_ppas(self, launchpad):
453- launchpad.get_lp_user_name.return_value = 'foo'
454-
455- lp = mock.MagicMock()
456- ppa = mock.Mock()
457- ppa.name = 'foo'
458- ppa.signing_key_fingerprint = 'foofp'
459- ppa.private = True
460- lp.people['foo'].getPPAByName.return_value = ppa
461- lp.people['foo'].getArchiveSubscriptionURL.return_value = 'foo_url'
462-
463- series = 'precise'
464- deblines, keys = cloud_image._parse_ppas(lp, ['ppa:/foo/bar'], series)
465- self.assertEqual({'foo': 'foofp'}, keys)
466- expected = ['deb foo_url precise main', 'deb-src foo_url precise main']
467- self.assertListEqual(expected, deblines)
468-
469- @mock.patch('time.sleep')
470- @mock.patch('imagebuilder.cloud_image._chrooted')
471- def test_apt_get_update_fails(self, chrooted, sleep):
472- chrooted.side_effect = subprocess.CalledProcessError(1, 'cmd', 'out')
473- with self.assertRaises(subprocess.CalledProcessError):
474- cloud_image._apt_get_update('/blah')
475-
476- @mock.patch('time.sleep')
477- @mock.patch('imagebuilder.cloud_image._chrooted')
478- def test_apt_get_update_retries(self, chrooted, sleep):
479- '''ensure it can apt-get update works after retrying'''
480- chrooted.side_effect = [
481- subprocess.CalledProcessError(1, 'cmd', 'out'),
482- subprocess.CalledProcessError(1, 'cmd', 'out'),
483- None,
484- ]
485- cloud_image._apt_get_update('/blah')
486- self.assertEqual(3, chrooted.call_count)
487- self.assertEqual(2, sleep.call_count)
488-
489- @mock.patch('imagebuilder.cloud_image.launchpad')
490- @mock.patch('imagebuilder.cloud_image._chrooted')
491- @mock.patch('__builtin__.open')
492- def test_setup_ppas(self, mopen, chroot, lp):
493- status_cb = mock.Mock()
494- series = 'saucy'
495- ppas = ['ppa:user/foo', 'ppa:team/bar']
496- mopen.return_value = mock.MagicMock(spec=file)
497- fd = mopen.return_value.__enter__.return_value
498- cloud_image._setup_ppas('/chroot', ppas, series, status_cb)
499- self.assertTrue(fd.write.call_count)
500- # todo look at chroot calls
501
502=== modified file 'image-builder/imagebuilder/tests/test_worker.py'
503--- image-builder/imagebuilder/tests/test_worker.py 2014-06-27 18:05:26 +0000
504+++ image-builder/imagebuilder/tests/test_worker.py 2014-07-21 13:55:48 +0000
505@@ -24,26 +24,20 @@
506 super(TestImagebuildWorker, self).setUp()
507 self.worker = run_worker.ImageBuildWorker()
508
509- @mock.patch('imagebuilder.run_worker.modify_image')
510+ @mock.patch('imagebuilder.run_worker.get_image')
511 @mock.patch('ci_utils.amqp_utils.progress_completed')
512- @mock.patch('ci_utils.amqp_worker.AMQPWorker._create_data_store')
513- def test_data_store_container(self, mocked_create_data_store,
514- mocked_progress_completed,
515- mocked_modify_image):
516+ def test_worker_get_image(self, mocked_progress_completed,
517+ mocked_get_image):
518 params = {
519 'image': 'http://fake/location',
520 'ticket_id': 1,
521- 'ppa_list': [],
522- 'package_list': [],
523 'series': 'saucy',
524 'progress_trigger': 'ticket_1_trigger'
525 }
526
527 mocked_log = mock.MagicMock()
528- mocked_ds = mock.MagicMock()
529- mocked_modify_image.return_value = ('fake_image_id', 'fake_location')
530- mocked_create_data_store.return_value = mocked_ds
531+ mocked_get_image.return_value = ('fake_image_id')
532
533 self.worker.handle_request(params, mocked_log)
534
535- mocked_create_data_store.assert_called_once_with(params['ticket_id'])
536+ mocked_get_image.assert_called_once()
537
538=== modified file 'image-builder/setup.py'
539--- image-builder/setup.py 2014-07-17 16:18:14 +0000
540+++ image-builder/setup.py 2014-07-21 13:55:48 +0000
541@@ -18,7 +18,7 @@
542 import sys
543
544 requires = [
545- 'pyOpenSSL==0.14',
546+ #'pyOpenSSL==0.14',
547 'PyYAML==3.10',
548 'amqplib==1.0.0',
549 'launchpadlib==1.10.2',

Subscribers

People subscribed via source and target branches