Merge lp:~cjohnston/ubuntu-ci-services-itself/push-img-swift into lp:ubuntu-ci-services-itself

[Chris Johnston]

Push build images to swift storage for later retrieval

This is the first part of a multi-part MP to enable a user to download the built image files via the CLI.

[Andy Doan (doanac)]

is there a reason to not use the create_datatore implemention:

 http://bazaar.launchpad.net/~canonical-ci-engineering/ubuntu-ci-services-itself/trunk/view/head:/ci-utils/ci_utils/amqp_worker.py#L110

instead of creating one in cloud_image.py?

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[lp:~cjohnston/ubuntu-ci-services-itself/push-img-swift]

387. By Chris Johnston on 2014-03-14

Update per review

[Chris Johnston (cjohnston)]

review: Needs Resubmitting

[Francis Ginther (fginther)]

76 + ds = _create_data_store(ticket_id)
77 + ds.change_visibility()
53 + with open(converted_image_path, 'r') as fp:
54 + location = ds.put_file(upload_filename, fp)

I believe public/private is a setting for the container. So doesn't this just set the container to private until the next component comes along and changes visibility back to public? Perhaps I missing something.

review: Needs Fixing

[Chris Johnston (cjohnston)]

On Fri, Mar 14, 2014 at 6:13 PM, Francis Ginther <
<email address hidden>> wrote:

> Review: Needs Fixing
>
> 76 + ds = _create_data_store(ticket_id)
> 77 + ds.change_visibility()
> 53 + with open(converted_image_path, 'r') as fp:
> 54 + location = ds.put_file(upload_filename, fp)
>
> I believe public/private is a setting for the container. So doesn't this
> just set the container to private until the next component comes along and
> changes visibility back to public? Perhaps I missing something.
>

 67+ ds_name = params['ticket_id'] + '-image' 68+ ds =
self._create_data_store(ds_name)
Here you will see that we are creating a separate data store called
ticket-<id>-image... So, nothing else should be setting this container to
public.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Evan (ev)]

17 + def _create_artifact(self, name, url, retval, type='LOGS'):

This could use a docstring.

54 + location = ds.put_file(upload_filename, fp)

Shouldn't we catch and handle DataStoreException here?

[Andy Doan (doanac)]

On 03/14/2014 06:55 PM, Evan Dandrea wrote:
> 54 + location = ds.put_file(upload_filename, fp)
>
> Shouldn't we catch and handle DataStoreException here?

Good idea. If this fails, the ticket should be able to continue. eg, all
is not lost.

[Francis Ginther (fginther)]

> On Fri, Mar 14, 2014 at 6:13 PM, Francis Ginther <
> <email address hidden>> wrote:
>
> > Review: Needs Fixing
> >
> > 76 + ds = _create_data_store(ticket_id)
> > 77 + ds.change_visibility()
> > 53 + with open(converted_image_path, 'r') as fp:
> > 54 + location = ds.put_file(upload_filename, fp)
> >
> > I believe public/private is a setting for the container. So doesn't this
> > just set the container to private until the next component comes along and
> > changes visibility back to public? Perhaps I missing something.
> >
>
> 67+ ds_name = params['ticket_id'] + '-image' 68+ ds =
> self._create_data_store(ds_name)
> Here you will see that we are creating a separate data store called
> ticket-<id>-image... So, nothing else should be setting this container to
> public.

I swear I was looking for something like this, just in the wrong place. Thanks for clearing up.

[lp:~cjohnston/ubuntu-ci-services-itself/push-img-swift]

388. By Chris Johnston on 2014-03-15

Catch exception per review

[Chris Johnston (cjohnston)]

review: Needs Resubmitting

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Andy Doan (doanac)]

review: Approve

[lp:~cjohnston/ubuntu-ci-services-itself/push-img-swift]

389. By Chris Johnston on 2014-03-15

Merge trunk

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

=== modified file 'ci-utils/ci_utils/amqp_worker.py'
--- ci-utils/ci_utils/amqp_worker.py	2014-03-12 21:45:22 +0000
+++ ci-utils/ci_utils/amqp_worker.py	2014-03-15 11:29:58 +0000
@@ -128,14 +128,17 @@
         try:
             name = '{}.output.log'.format(logger.name)
             url = store.put_file(name, content, 'text/plain')
-            retval.setdefault('artifacts', []).append({
-                'name': name,
-                'reference': url,
-                'type': 'LOGS',
-            })
+            self._create_artifact(name, url, retval)
         except:
             log.exception('unable to store worker log')
 
+    def _create_artifact(self, name, url, retval, type='LOGS'):
+        retval.setdefault('artifacts', []).append({
+            'name': name,
+            'reference': url,
+            'type': type,
+        })
+
     def _handle_cancel(self, params):
         '''provide an advisory mechansim to inform the worker to stop
 

=== modified file 'image-builder/imagebuilder/cloud_image.py'
--- image-builder/imagebuilder/cloud_image.py	2014-03-15 03:27:45 +0000
+++ image-builder/imagebuilder/cloud_image.py	2014-03-15 11:29:58 +0000
@@ -31,7 +31,7 @@
 
 from imagebuilder import ImageException
 from ci_utils.image_store import ImageStore
-from ci_utils import dump_stack, launchpad
+from ci_utils import dump_stack, data_store, launchpad
 
 
 log = logging.getLogger('Imagebuilder')
@@ -228,7 +228,7 @@
         runcmd('qemu-nbd -d /dev/%s' % self.nbd_dev)
 
 
-def modify_image(image, repos, series, packages, status_cb=None):
+def modify_image(image, repos, series, packages, ds, status_cb=None):
     '''Add packges and PPAs to a cloud image mounted in a chroot'''
     if not status_cb:
         status_cb = lambda x: 0   # just provide a no-op
@@ -250,11 +250,17 @@
         status_cb('Shrinking the converted image...')
         runcmd('qemu-img convert -O qcow2 -c %s %s' % (
             image_path, converted_image_path))
-        glance_filename = str(uuid.uuid1()) + '.img'
+        upload_filename = str(uuid.uuid1()) + '.img'
         status_cb(
-            'Uploading converted image to glance as %s' % glance_filename)
-        glance.image_create(glance_filename, converted_image_path)
-        return glance_filename
+            'Uploading converted image to glance as %s' % upload_filename)
+        glance.image_create(upload_filename, converted_image_path)
+        try:
+            ds.change_visibility()
+            with open(converted_image_path, 'r') as fp:
+                location = ds.put_file(upload_filename, fp)
+        except data_store.DataStoreException:
+            status_cb('ERROR: unable to upload image to swift')
+        return upload_filename, location
 
 
 def main():

=== modified file 'image-builder/imagebuilder/run_worker.py'
--- image-builder/imagebuilder/run_worker.py	2014-03-14 21:54:37 +0000
+++ image-builder/imagebuilder/run_worker.py	2014-03-15 11:29:58 +0000
@@ -29,13 +29,18 @@
         series = params['series']
         packages = params['package_list']
         trigger = params['progress_trigger']
+        ds_name = params['ticket_id'] + '-image'
+        ds = self._create_data_store(ds_name)
 
         def status_cb(msg):
             log.info(msg)
             amqp_utils.progress_update(trigger, {'message': msg})
 
-        image_id = modify_image(image, repos, series, packages, status_cb)
-        return amqp_utils.progress_completed, {'image_id': image_id}
+        image_id, location = modify_image(image, repos, series, packages,
+                                          ds, status_cb)
+        retval = {'image_id': image_id}
+        self._create_artifact(image_id, location, retval, type='IMAGE')
+        return amqp_utils.progress_completed, retval
 
 
 if __name__ == '__main__':