Merge lp:~vila/uci-engine/tr-new-api into lp:uci-engine

[Vincent Ladeuil]

Handle the ppa list parameter in the test runner request API.

Implement the new test runner API: handle the ppa list.

This keeps migrating the test runner API without breaking backwards compatibility:

- https://code.launchpad.net/~vila/uci-engine/new-tr-api-lander/+merge/227510 was allowing the lander to be upgraded without upgrading the test runner,
- this one allows the test runnner to be upgraded without the image builder being upgraded.

I had some doubts for the later about using add-apt-repository twice but I had to do that anyway in this proposal to address a nasty bug: by default, add-apt-repository comments out the deb-src line. The fallout was that the binary packages were properly installed from the ppa but not the source package.

It's hard to notice but I was lucky enough to check just the source version being installed across precise/saucy/trusty from 'run-tests test_runner.tstrun.tests.test_worker.TestWorkerHandleRequest.test_handle_request_'

This also allowed me to remove one call to 'apt-get upgrade' which will make pitti happier (there is an ongoing discussion about minimizing the apt-get update/upgrade calls for the testbeds as this is a costly operation).

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Vincent Ladeuil (vila)]

Note to self: apt-get upgrade was handled more robustly by the image builder and the test runner should copy that from https://code.launchpad.net/~pwlars/uci-engine/unmodified-imagebuild/+merge/227557

[Paul Larson (pwlars)]

This looks ok to me, but of course I'd still like us to do some more testing with it tomorrow. Note: I have a new branch for the image builder that uses a versioned API. If we break compatibility here in the test runner also, would you like to do something similar so that we already establish a way of upgrading to new API versions in the test runner code also?

review: Needs Information

[Martin Pitt (pitti)]

I have some inline comments. Setting "needs fixing" as we really need a fallback for apt-get update, otherwise you'll get unhappy really soon :/

review: Needs Fixing

[Vincent Ladeuil (vila)]

Agreed, see inline replies.

[Vincent Ladeuil (vila)]

> This looks ok to me, but of course I'd still like us to do some more testing
> with it tomorrow. Note: I have a new branch for the image builder that uses a
> versioned API. If we break compatibility here in the test runner also, would
> you like to do something similar so that we already establish a way of
> upgrading to new API versions in the test runner code also?

I don't have a clear picture on the upgrade story, I'd be more inclined, as a first step, to describe what we're doing here for the test runner and the image builder, and how we upgrade a deployment *before* going down the versioned API route.

But yeah, I should probably split out the lander part, the test runner itself only accept an additional parameter that I can make optional without having to version the API I think.

Let's talk ! ;)

[Vincent Ladeuil (vila)]

@Martin: apt-get update and PPAs apt lines double handling are now fixed.

[lp:~vila/uci-engine/tr-new-api]

703. By Vincent Ladeuil on 2014-07-22

Make apt-get safer by retrying on failure.

704. By Vincent Ladeuil on 2014-07-22

Define a config variable for the apt-get update timeouts and avoid spawing a nova instance for the related tests (safe_apt_get_update() called on the instance is covered by other tests).

705. By Vincent Ladeuil on 2014-07-22

Fix pep8 issues.

706. By Vincent Ladeuil on 2014-07-22

Clarify how we enable the PPAs and do it once instead of catching up with cloud-init.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Paul Larson (pwlars)]

I tried this along with my imagebuild merged together, and was able to get a ticket through. It resulted in a failed test, which was actually intentional on my part because I wanted to be really sure it was getting the new package.

[lp:~vila/uci-engine/tr-new-api]

707. By Vincent Ladeuil on 2014-07-23

--enable-source was added to add-apt-repository after precise.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Martin Pitt (pitti)]

Heh, that's a rather thorough way to apt-get update, but why not :-) LGTM now, thanks!

review: Approve

[Paul Larson (pwlars)]

review: Approve

=== modified file 'test_runner/tstrun/run_worker.py'
--- test_runner/tstrun/run_worker.py	2014-07-15 09:47:45 +0000
+++ test_runner/tstrun/run_worker.py	2014-07-23 10:47:55 +0000
@@ -126,6 +126,7 @@
         progress_queue = params['progress_trigger']
         series = params['series']
         image_id = params['image_id']
+        ppa_list = params['ppa_list']
         package_list = params['package_list']
 
         results = {}
@@ -142,6 +143,7 @@
             conf = testbed.vms_config_from_auth_config(tb_name, auth_conf)
             conf.set('vm.image', image_id)
             conf.set('vm.release', series)
+            conf.set('vm.ppas', ','.join(ppa_list))
             base_dir = conf.get('vm.vms_dir')
             if not os.path.exists(base_dir):
                 os.makedirs(base_dir)

=== modified file 'test_runner/tstrun/testbed.py'
--- test_runner/tstrun/testbed.py	2014-07-11 14:44:33 +0000
+++ test_runner/tstrun/testbed.py	2014-07-23 10:47:55 +0000
@@ -84,6 +84,21 @@
                         help_string='''The base image to run the tests.'''))
 register(config.PathOption('vm.ssh_key_path', default='~/.ssh/id_rsa',
                            help_string='''The ssh key for the testbed.'''))
+register(options.ListOption('vm.apt_get.update.timeouts',
+                            default='15.0, 90.0, 240.0',
+                            help_string='''apt-get update timeouts in seconds.
+
+When apt-get update fails on hash sum mismatches, retry after the specified
+timeouts. More values mean more retries.
+'''))
+register(options.ListOption('vm.ppas',
+                            default=None,
+                            help_string='''PPAs to be added to the testbed.
+
+This works around cloud-init not activating the deb-src line and not providing
+a way to do so. This is intended to be fixed in uci-vms so vm.apt_sources can
+be used again.
+'''))
 
 
 logging.basicConfig(level=logging.INFO)
@@ -127,7 +142,6 @@
         self.floating_ip = None
         self.nova = self.build_nova_client()
         self.test_bed_key_path = None
-        self.conf.set('vm.update', 'True')
         self.conf.set('vm.ssh_authorized_keys',
                       self.conf.get('vm.ssh_key_path') + '.pub')
         # No need to reboot a nova instance
@@ -192,6 +206,33 @@
             self.instance.add_floating_ip(self.floating_ip)
         self.wait_for_ip()
         self.wait_for_cloud_init()
+        ppas = self.conf.get('vm.ppas')
+        if ppas:
+            cmd = ['sudo', 'add-apt-repository']
+            if self.conf.get('vm.release') > 'precise':
+                cmd.append('--enable-source')
+            for ppa in ppas:
+                self.ssh(*(cmd + [ppa]))
+        # Now we can apt-get update (doing it earlier would lead to the wrong
+        # source package to be installed).
+        self.safe_apt_get_update()
+
+    def apt_get_update(self):
+        return self.ssh('sudo', 'apt-get', 'update')
+
+    def safe_apt_get_update(self):
+        for timeout in self.conf.get('vm.apt_get.update.timeouts'):
+            proc, out, err = self.apt_get_update()
+            if proc.returncode == 0:
+                # We're done
+                return
+            else:
+                msg = ('apt-get update failed, wait {}s'
+                       'stdout:\n{}\n'
+                       'stderr:\n{}\n')
+                log.info(msg.format(timeout, out, err))
+                time.sleep(float(timeout))
+        raise TestBedException('apt-get update never succeeded')
 
     def update_instance(self):
         # MISSINGTEST: What if the instance disappear ? (could be approximated

=== modified file 'test_runner/tstrun/tests/test_testbed.py'
--- test_runner/tstrun/tests/test_testbed.py	2014-07-12 13:30:40 +0000
+++ test_runner/tstrun/tests/test_testbed.py	2014-07-23 10:47:55 +0000
@@ -123,3 +123,45 @@
                                 out=subprocess.PIPE, err=subprocess.PIPE)
         self.assertEqual(0, proc.returncode)
         self.assertEqual('ubuntu\n', out)
+
+    def test_apt_get_update_retries(self):
+        tb = testbed.TestBed(self.conf)
+        self.conf.set('vm.image', self.get_image_id())
+        self.conf.set('vm.apt_get.update.timeouts', '0.1, 0.1')
+        self.nb_calls = 0
+
+        class Proc(object):
+            returncode = 0
+
+        def failing_update():
+            self.nb_calls += 1
+            if self.nb_calls > 1:
+                return Proc(), 'stdout success', 'stderr success'
+            else:
+                # Fake a failed apt-get update
+                proc = Proc()
+                proc.returncode = 1
+                return proc, 'stdout error', 'stderr error'
+
+        tb.apt_get_update = failing_update
+        tb.safe_apt_get_update()
+        self.assertEqual(2, self.nb_calls)
+
+    def test_apt_get_update_fails(self):
+        tb = testbed.TestBed(self.conf)
+        self.conf.set('vm.image', self.get_image_id())
+        self.conf.set('vm.apt_get.update.timeouts', '0.1, 0.1, 0.1')
+
+        def failing_update():
+            class Proc(object):
+                pass
+
+            proc = Proc()
+            proc.returncode = 1
+            return proc, 'stdout', 'stderr'
+
+        tb.apt_get_update = failing_update
+        with self.assertRaises(testbed.TestBedException) as cm:
+            tb.safe_apt_get_update()
+        self.assertEqual('apt-get update never succeeded',
+                         unicode(cm.exception))

=== modified file 'test_runner/tstrun/tests/test_worker.py'
--- test_runner/tstrun/tests/test_worker.py	2014-07-15 09:47:45 +0000
+++ test_runner/tstrun/tests/test_worker.py	2014-07-23 10:47:55 +0000
@@ -22,7 +22,10 @@
     assertions,
 )
 from ucivms.tests import fixtures as vms_fixtures
-from ci_utils import amqp_utils
+from ci_utils import (
+    amqp_utils,
+    unit_config,
+)
 from ci_utils.testing import (
     features,
     fixtures,
@@ -57,6 +60,12 @@
 
         self.ds_factory = create_data_store
 
+    def get_ppa_list(self):
+        # We're testing only the test runner so there is no ppa assigned to the
+        # ticket, only the master ppa is relevant.
+        conf = unit_config.get_auth_config()
+        return [conf.get('master_ppa')]
+
     def assertTestRun(self, params):
         worker = run_worker.TestRunnerWorker(self.ds_factory)
         self.queue.publish(params)
@@ -76,6 +85,7 @@
                       series='precise',
                       image_id=('Ubuntu 12.04.4 LTS (amd64 20140613)'
                                 ' - CI Engineering'),
+                      ppa_list=self.get_ppa_list(),
                       package_list=['libpng'])
         returns = self.assertTestRun(params)
         (retcode, results) = returns[0]
@@ -94,6 +104,7 @@
                       series='saucy',
                       image_id=('Ubuntu Server 13.10 (amd64 20140409.1)'
                                 ' - Partner Image'),
+                      ppa_list=self.get_ppa_list(),
                       package_list=['libpng'])
         returns = self.assertTestRun(params)
         (retcode, results) = returns[0]
@@ -111,6 +122,7 @@
                       series='trusty',
                       image_id=('Ubuntu Server 14.04 LTS (amd64 20140607.1)'
                                 ' - Partner Image'),
+                      ppa_list=self.get_ppa_list(),
                       package_list=['libpng'])
         returns = self.assertTestRun(params)
         (retcode, results) = returns[0]