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

[Paul Larson]

New version of the imagebuild API that simply copies an image into glance, rather than trying to modify it.

New version of the imagebuild changes I was working on. This one implements a versioning scheme for the API and also handles making sure the lander doesn't send extra parameters. I'd like to do a little more testing on this first, but wanted to get it out there to look at in an easier form.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:699
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1136/
Executed test runs:

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

review: Needs Fixing (continuous-integration)

[lp:~pwlars/uci-engine/versioned-imagebuild]

700. By Paul Larson on 2014-07-22

flake8 fixups

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Vincent Ladeuil (vila)]

This is clearer to expose what each version of the API is doing. Thanks !

For the sake of the thought experiment, if I get that right, your plan would be to have the 'lander/bin/lander_service_wrapper.py' part that could be an independent MP landed/deployed last.

So all workers start implementing the new API and can be deployed while they still support the old API.

Then the lander one is deployed and tickets start using that new API.

Is that correct ?

I have several questions I don't have a good answer for yet:

- what's the upgrade story for the webops ?

- what happens if an old worker receives a request for the new API (that one is the most annoying I think ;) ?

- for how long do we keep old lander/workers running ?

- how can we make the "right" workers talk to each other ?

- do we have different kind of API migrations ?

- can we try to sort out the simplest first ? ;-)

- is this MP the right place to ask ?

None of the above have to be answered right now nor should they block that experiment ;)

But a first round of answers can help define the next steps.

review: Needs Information

[Paul Larson (pwlars)]

> I have several questions I don't have a good answer for yet:
>
> - what's the upgrade story for the webops ?
For a small enough deployment, I think the way we've always done upgrades would work. The risk here would be that in that window while upgrading, a ticket comes in and makes it all the way through to the imagebuilder before the upgrade is complete. This would require the fastest ppa build ever I guess, but we should still be careful of course. The situation we don't want to end up with is where the lander sends a request for an API version that NONE of the imagebuild workers (all one of them) support. For a more paranoid upgrade, see below.

> - what happens if an old worker receives a request for the new API (that one
> is the most annoying I think ;) ?
I think the idea is that to do the upgrade as carefully as possible, you upgrade things from the bottom up. Upgrade workers first. If they get a ticket for the previous version of the API, that's ok. Once they are all upgraded, you upgrade the lander. Also, this makes me realize that I think we might have much deeper problems in the upgrade don't we? What happens if I the upgrade happens while a component is in the middle of something? Does it get upgraded part-way through a ticket?

>
> - for how long do we keep old lander/workers running ?
I think as short a time as possible for now. Eventually we might want to explore bigger testing scenarios where we have some deployed with the new version and some with the old and ease into the upgrade as we see tickets working. I think we need a lot of things in place before we can really do that though.
>
> - how can we make the "right" workers talk to each other ?
That, I don't know. I've not seen a good solution with rabbit to do this. Best I can suggest is perhaps we should reject the request somehow by sending a message back to the lander, and the lander retries the request, maybe after a delay? It's imprecise, but would hopefully result in either trying a different worker or retrying the same worker after it upgrades properly. Also, if we're really sure the workers get upgraded before the lander, then they are all the "right" one to send the request to.
>
> - do we have different kind of API migrations ?
Not sure what you mean here.
>
> - can we try to sort out the simplest first ? ;-)
This MP seemed like as good a place to start as any.
>
> - is this MP the right place to ask ?
Probably not, I think we should have a broader discussion about this.
>
> None of the above have to be answered right now nor should they block that
> experiment ;)
>
> But a first round of answers can help define the next steps.

review: Needs Resubmitting

[lp:~pwlars/uci-engine/versioned-imagebuild]

701. By Paul Larson on 2014-07-22

turns out the amqp worker still needs the ticket id, even if the image build worker doesn't

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Vincent Ladeuil (vila)]

Let's land !

review: Approve

=== modified file 'ci-utils/ci_utils/__init__.py'
--- ci-utils/ci_utils/__init__.py	2014-07-17 20:05:30 +0000
+++ ci-utils/ci_utils/__init__.py	2014-07-22 21:12:41 +0000
@@ -17,6 +17,12 @@
 import tempfile
 
 
+# Current supported API Versions
+API_VERSIONS = {
+    'image_builder': '20140721',
+}
+
+
 SUPPORTED_SERIES = [
     'precise',
     'trusty',

=== modified file 'image-builder/imagebuilder/cloud_image.py'
--- image-builder/imagebuilder/cloud_image.py	2014-07-17 16:00:41 +0000
+++ image-builder/imagebuilder/cloud_image.py	2014-07-22 21:12:41 +0000
@@ -300,6 +300,22 @@
         runcmd('qemu-nbd -d /dev/%s' % self.nbd_dev)
 
 
+def get_image(image, status_cb=None):
+    '''Copy the requested cloud image to glance'''
+    # API Version 20140721
+    if not status_cb:
+        status_cb = lambda x: 0  # just provide a no-op
+
+    with ci_utils.TmpDir() as tmpdir:
+        status_cb('downloading %s...' % image)
+        image_path = _download(image, tmpdir)
+        glance = ImageStore(ci_utils.unit_config.get_auth_config())
+        upload_filename = str(uuid.uuid1()) + '.img'
+        status_cb('Uploading image to glance as %s' % upload_filename)
+        glance.image_create(upload_filename, image_path)
+        return upload_filename
+
+
 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:

=== modified file 'image-builder/imagebuilder/run_worker.py'
--- image-builder/imagebuilder/run_worker.py	2014-07-15 20:20:00 +0000
+++ image-builder/imagebuilder/run_worker.py	2014-07-22 21:12:41 +0000
@@ -19,6 +19,7 @@
 from ci_utils import amqp_utils, amqp_worker
 
 from imagebuilder.cloud_image import modify_image
+from imagebuilder.cloud_image import get_image
 
 
 class ImageBuildWorker(amqp_worker.AMQPWorker):
@@ -47,22 +48,31 @@
             js_status.add_fail('integration test', 'has not run')
 
     def handle_request(self, params, log):
+
         image = params['image']
-        repos = params['ppa_list']
-        series = params['series']
-        packages = params['package_list']
         trigger = params['progress_trigger']
-        ticket_id = params['ticket_id']
-        ds = self._create_data_store(ticket_id)
 
         def status_cb(msg):
             log.info(msg)
             amqp_utils.progress_update(trigger, {'message': msg})
 
-        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')
+        #Check API version
+        api = params.get('api_version')
+        if api == '20140721':
+            image_id = get_image(image, status_cb)
+            retval = {'image_id': image_id}
+        else:
+            repos = params['ppa_list']
+            series = params['series']
+            packages = params['package_list']
+            ticket_id = params['ticket_id']
+            ds = self._create_data_store(ticket_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
 
 

=== modified file 'image-builder/imagebuilder/tests/test_worker.py'
--- image-builder/imagebuilder/tests/test_worker.py	2014-06-27 18:05:26 +0000
+++ image-builder/imagebuilder/tests/test_worker.py	2014-07-22 21:12:41 +0000
@@ -47,3 +47,22 @@
         self.worker.handle_request(params, mocked_log)
 
         mocked_create_data_store.assert_called_once_with(params['ticket_id'])
+
+    @mock.patch('imagebuilder.run_worker.get_image')
+    @mock.patch('ci_utils.amqp_utils.progress_completed')
+    def test_worker_get_image(self, mocked_progress_completed,
+                              mocked_get_image):
+        params = {
+            'api_version': '20140721',
+            'image': 'http://fake/location',
+            'ticket_id': 1,
+            'series': 'saucy',
+            'progress_trigger': 'ticket_1_trigger'
+        }
+
+        mocked_log = mock.MagicMock()
+        mocked_get_image.return_value = ('fake_image_id')
+
+        self.worker.handle_request(params, mocked_log)
+
+        mocked_get_image.assert_called_once()

=== modified file 'lander/bin/lander_service_wrapper.py'
--- lander/bin/lander_service_wrapper.py	2014-07-21 06:53:05 +0000
+++ lander/bin/lander_service_wrapper.py	2014-07-22 21:12:41 +0000
@@ -28,6 +28,7 @@
 from ci_utils import (
     amqp_utils,
     dump_stack,
+    API_VERSIONS,
 )
 
 
@@ -218,16 +219,11 @@
 
     # Extract the image parameters from the master request parameters
     request_parameters = ticket.get_full_ticket()
-    ppa_list = [
-        workflow.get_step('ppa_assigner')['ppa'],
-        request_parameters['master_ppa'],
-    ]
+
     params = {
         'ticket_id': workflow.ticket_id,
+        'api_version': API_VERSIONS.get('image_builder'),
         'image': request_parameters['base_image'],
-        'series': request_parameters['series'],
-        'package_list': _get_binary_packages(ticket, request_parameters),
-        'ppa_list': ppa_list,
         'progress_trigger': queue,
     }
     ticket.set_image_waiting()