Merge lp:~doanac/ubuntu-ci-services-itself/lander-ppa-retry into lp:ubuntu-ci-services-itself

[Andy Doan]

lander: retry getting ppa

We should retry getting a PPA in the lander when all PPAs are
currently reserved.

There were 2 parts to dealing with the "no free ppas" issue

Part 1 was simply reporting this type of error status in the webui. This piece is now complete

The MP is for part 2: have the lander retry getting a PPA when there are none available.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Paul Larson (pwlars)]

+ while True:
This part bothers me, but maybe it's just because there's no good way to cancel operations at the moment. In this case, since it's just trying to get the ppa though, I think that should be safe. Something to think about for later perhaps though.

review: Approve

[Andy Doan (doanac)]

On 03/06/2014 03:47 PM, Paul Larson wrote:
> + while True:
> This part bothers me, but maybe it's just because there's no good way to cancel operations at the moment. In this case, since it's just trying to get the ppa though, I think that should be safe. Something to think about for later perhaps though.

I was thinking we could always configure a jenkins job timeout for this.
The code should be safe to interrupt in that way.

[Francis Ginther (fginther)]

Regarding Paul's comment. We really need a central way to terminate processing of a ticket. Something that allows the lander and components to check in with the ticket system and abort the current task gracefully. Given the limitations and the problem being addressed here, approved.

review: Approve

=== modified file 'lander/bin/lander_service_wrapper.py'
--- lander/bin/lander_service_wrapper.py	2014-03-04 19:52:45 +0000
+++ lander/bin/lander_service_wrapper.py	2014-03-06 17:35:28 +0000
@@ -19,6 +19,7 @@
 import logging
 import os
 import sys
+import time
 import urllib2
 import yaml
 
@@ -117,10 +118,24 @@
     return TicketApi(master['ts_url'], master['request_id'])
 
 
+def _get_ppa(ppa_assigner_url, ticket_id):
+    timeout = 120
+    url = '%s/api/v1/ppa/' % ppa_assigner_url
+    while True:
+        try:
+            loc = _post(url, {'ticket_id': ticket_id})
+            return loc
+        except urllib2.HTTPError as e:
+            if e.getcode() != 404:
+                raise e
+            logging.warn('No free PPAs can be found. Retrying in %ds', timeout)
+            time.sleep(timeout)
+
+
 def _handle_ppa_assigner(args):
     config = _get_config(args)
-    url = '%s/api/v1/ppa/' % config['master']['ppa_assigner_url']
-    loc = _post(url, {'ticket_id': config['master']['request_id']})
+    loc = _get_ppa(
+        config['master']['ppa_assigner_url'], config['master']['request_id'])
     data = _get(loc)
     result = {
         'result': 'PASSED',
 
=== modified file 'lander/lander/tests/test_service_wrapper.py'
--- lander/lander/tests/test_service_wrapper.py	2014-03-03 23:22:58 +0000
+++ lander/lander/tests/test_service_wrapper.py	2014-03-06 17:35:28 +0000
@@ -153,22 +153,25 @@
         self.assertItemsEqual(expected, actual)
 
     @mock.patch('lander_service_wrapper._post')
-    def testPPAAssigner404(self, post):
-        '''Ensure a "no ppas" type error generates a response'''
-        err_msg = 'foo bar'
-        post.side_effect = urllib2.HTTPError(
-            'url', 404, '404', None, StringIO.StringIO(err_msg))
+    @mock.patch('lander_service_wrapper._get')
+    @mock.patch('time.sleep')
+    def testPPAAssigner404(self, sleep, get, post):
+        '''Ensure we do proper retrying when no ppa's are available'''
+        ppa = 'ppa:foo'
+        get.return_value = {'name': ppa}
+        err = urllib2.HTTPError(
+            'url', 404, '404', None, StringIO.StringIO('foo bar'))
+        post.side_effect = [err, err, err, 'http://foo/api/v1/ppa/1']
 
         args = self.base_args + ['--service', 'ppa_assigner']
         args = lander_service_wrapper._get_parser().parse_args(args)
         rc = lander_service_wrapper.main(args)
-        self.assertEqual(1, rc)
+        self.assertEqual(0, rc)
 
         with open(self.results) as f:
             data = json.loads(f.read())
-            self.assertEqual('ERROR', data['result'])
-            self.assertEqual(err_msg, data['resp_body'])
-            self.assertEqual('404', data['resp_code'])
+            self.assertEqual('PASSED', data['result'])
+            self.assertEqual(ppa, data['ppa'])
 
     @mock.patch('lander_service_wrapper._post')
     def testPPAAssignerUnexpected(self, post):