Merge lp:~fgiff/linaro-android-build-tools/linaro-android-connect-android-build-to-lava into lp:linaro-android-build-tools

Proposed by Frans Gifford on 2011-07-26
Status: Merged
Approved by: Paul Sokolovsky on 2011-07-26
Approved revision: 302
Merged at revision: 289
Proposed branch: lp:~fgiff/linaro-android-build-tools/linaro-android-connect-android-build-to-lava
Merge into: lp:linaro-android-build-tools
Diff against target: 131 lines (+109/-0)
4 files modified
build-scripts/build-android (+5/-0)
build-scripts/lava-token.txt (+1/-0)
build-scripts/lava-user.txt (+1/-0)
build-scripts/post-build-lava.py (+102/-0)
To merge this branch: bzr merge lp:~fgiff/linaro-android-build-tools/linaro-android-connect-android-build-to-lava
Reviewer Review Type Date Requested Status
Paul Sokolovsky 2011-07-26 Approve on 2011-07-26
Paul Larson (community) Approve on 2011-07-26
James Westby (community) 2011-07-26 Approve on 2011-07-26
Paul Larson 2011-07-26 Pending
Alexander Sack 2011-07-26 Pending
Michael Hudson-Doyle 2011-07-26 Pending
Zach Pfeffer 2011-07-26 Pending
Review via email: mp+69279@code.launchpad.net

This proposal supersedes a proposal from 2011-07-06.

Description of the change

Connect the Android Builder to LAVA

Updated to connect to LAVA and submit jobs.

To post a comment you must log in.
Frans Gifford (fgiff) wrote : Posted in a previous version of this proposal

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 reviewer pfefferz
 reviewer plars
 reviewer asac
 reviewer mwhudson
 reviewer james-w
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJOFLjuAAoJEAeFjc2hVnFUUQYIANMRXQ6uQXGEtf0p85a0d1Lv
2WAEZvQt1Fmv3ty3x9MH9DJmss8q92Et3hiOMR/tgVzLqpJdgMF6S3+5DkOst43u
MjnhkXi74PLpAskUmr9SRn3cKWPASzQ8z2PEVmMQQ2nyDUPcLU/OVjYYH9//lpwF
6rid9k9D2UukbVieWyV8wLntm66CoJMFhifyyitt9MFZqdoLp/soOdGFVPq5iLvs
1PtTrYbaENya1GXx4onW1mbetjQkbKmjcyS+bd7RJmj5iY4RfLTlxDwIO/yxxzT1
wTuXrvhBBt60XwPRKCxxqYcjDXPcUQ2S41fY51dyDWeKqrea/B2PZ4sGZFkJPwU=
=iRKu
-----END PGP SIGNATURE-----

James Westby (james-w) wrote : Posted in a previous version of this proposal

18 + if job_name == "linaro-android_leb-panda":

I would think that this would be better using TARGET_PRODUCT wouldn't it?

That way we don't have to edit the script for every job added.

It would mean basing the stream name on the job name as I suggested, but I don't
know if Paul L. thinks that's a good idea.

30 + "job_name": "''' + build_url + '''",

You might prefer string interpolation rather than concatenation:

  print """{"job_name": "%(build_url)s"}""" % dict(build_url=build_url)

or to do it using the json module:

  import json
  config = {"job_name": build_url}
  print json.dumps(config)

I think that they will be a bit easier to modify, and this is going to
see plenty of modifications.

Up to you though.

Lastly convention is for four space indent rather than two.

Thanks,

James

review: Approve
James Westby (james-w) wrote : Posted in a previous version of this proposal

Oh, and I think there should be a toggle so that we can turn off submission if we want
to while testing. That should maybe be decided by the caller though?

Thanks,

James

Paul Sokolovsky (pfalcon) wrote : Posted in a previous version of this proposal

I agree about usage of json module. My idea was also that this script should take values it needs from the environment, not on command line, then it would be more flexible. For example, regarding toggle James talks about - I guess, for starters, we'd want to enable submission feature just for few selected jobs, so the script could check for LAVA_SUBMIT=1 var. Later, we may want to have it enabled by default and have switch to disable it instead. This all can be done in the script itself.

Anyway, current idea is to provide you with a sandbox, where you can test/tweak such integration issues (more info in email).

review: Approve
Frans Gifford (fgiff) wrote : Posted in a previous version of this proposal

OK, I've updated to:

 use the JSON module.
 use a LAVA_SUBMIT environment variable to toggle output.
 read environment variables from env instead of command line.
 use 4-space indent.

I'll try it out in the sandbox now.

James Westby (james-w) wrote : Posted in a previous version of this proposal

72 + if os.environ.get("LAVA_SUBMIT") > 0:
73 + main()

I think you need to wrap the get in an int() to get LAVA_SUBMIT=0 to work,
as it returns a string.

I think this is ok to land and hook in to the build.

Thanks,

James

review: Approve
Paul Sokolovsky (pfalcon) wrote : Posted in a previous version of this proposal

On Thu, 07 Jul 2011 14:56:37 -0000
James Westby <email address hidden> wrote:

> I think this is ok to land and hook in to the build.

I've created ec2-184-73-120-120.compute-1.amazonaws.com sandbox for
Frans to use, and it is set up to fetch binutils from his branch. So, I
guess it makes sense to let him do complete integration work (up to
some milestone of course) in his branch (he can test it right away), and
then merge it, so I skip merging this patch now.

--
Best Regards,
Paul

Michael Hudson-Doyle (mwhudson) wrote : Posted in a previous version of this proposal

I share the concern that defining the mapping between thing that was built and the tests to run and the stream to submit to by hardcoding it in this file is not that great, but we don't really have anything else yet and it's easy to change to something better later. Otherwise, looks fine enough to me.

review: Approve
James Westby (james-w) wrote :

117 + if os.environ.get("LAVA_SUBMIT") > 0:

I think that having this repeated in the caller and the callee is unnecessary.
I'd delete one of the checks to save confusion later.

103 + lava_token=os.environ.get("LAVA_TOKEN")

This isn't good, as it will allow anyone to impersonate you to LAVA as
the build config is public.

It's ok for getting something working, but we'll have to revisit this before
we call it done.

Thanks,

James

review: Approve
Paul Sokolovsky (pfalcon) wrote :

I'd appreciate if assignments syntax could be fixed - they should go with spaced around "=".

> lava_token=os.environ.get("LAVA_TOKEN")

Are we ok to specify LAVA_TOKEN in publicly available build config?

review: Needs Fixing
300. By Frans Gifford on 2011-07-26

Syntax cleanup

Frans Gifford (fgiff) wrote :

> > lava_token=os.environ.get("LAVA_TOKEN")
>
> Are we ok to specify LAVA_TOKEN in publicly available build config?

Lava tokens are cheap to create and revoke. Other than hard-coding the token into the build script, I don't see any other way to do this unless Paul Larson knows.

Paul Larson (pwlars) wrote :

I agree with what James and Paul pointed out, you don't want the token to be publicly visible to anyone. Maybe you could load it from a config file on the machine or something? Please fix this, and if you already have a token that's visible externally, I would recommend removing it and creating a new onw.

Approving for my part because it looks ok otherwise, but please fix Paul's need-fix before going forward with it.

review: Approve
James Westby (james-w) wrote :

On Tue, 26 Jul 2011 15:19:36 -0000, Frans Gifford <email address hidden> wrote:
>
> > > lava_token=os.environ.get("LAVA_TOKEN")
> >
> > Are we ok to specify LAVA_TOKEN in publicly available build config?
>
> Lava tokens are cheap to create and revoke. Other than hard-coding the
> token into the build script, I don't see any other way to do this
> unless Paul Larson knows.

We can do it at a different time so that it doesn't run on the slave and
we don't have to transmit it across from the master.

The master is able to keep some information private, so we can make use
of that.

Thanks,

James

301. By Frans Gifford on 2011-07-26

Use files on server to authenticate to lava rather than environment vars

Frans Gifford (fgiff) wrote :

Added a couple of files: lava-user.txt and lava-token.txt to hold the lava username and token. They're populated with my username and token at the moment. Ideally we'll set up a server account on lava for this. The LAVA_USER and LAVA_TOKEN environment variables are still usable in case we can populate these from Jenkins which would probably be cleaner.

302. By Frans Gifford on 2011-07-26

Use device_type so validation can load balance boards

Paul Sokolovsky (pfalcon) wrote :

301 won't help much. As we discussed during initial BP consideration, there's really no "server", there's only transient build slave instance which doesn't have access to any special files, unless those files were pushed to it first somehow (with Jenkins plugin?).

So, let's just treat this work as solving the most important part of it - actually set up a link between android build and LAVA (including settling protocols, etc.). Bu it needs more work (maybe complete rework) to improve security and reliability. Let's discuss this hopefully at the Connect.

Merge and will be deployed now.

review: Approve
James Westby (james-w) wrote :

On Tue, 26 Jul 2011 17:31:22 -0000, Frans Gifford <email address hidden> wrote:
> Added a couple of files: lava-user.txt and lava-token.txt to hold the
> lava username and token. They're populated with my username and token
> at the moment. Ideally we'll set up a server account on lava for this.

Yes, please do this.

I don't think we should commit the credentials to the branch
though. Paul, how would we deploy those credentials if they weren't in
the branch?

Thanks,

James

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'build-scripts/build-android'
2--- build-scripts/build-android 2011-06-03 16:29:06 +0000
3+++ build-scripts/build-android 2011-07-26 18:03:52 +0000
4@@ -51,3 +51,8 @@
5 toolchain-build/build-sysroot.sh out/target/product/`ls -1 out/target/product | head -n1` /tmp/sysroot
6 tar -cj -C /tmp/sysroot -f out/sysroot.tar.bz2 .
7 fi
8+
9+if test -n "$LAVA_SUBMIT"; then
10+ "${BUILD_SCRIPT_ROOT}"/post-build-lava.py
11+fi
12+
13
14=== added file 'build-scripts/lava-token.txt'
15--- build-scripts/lava-token.txt 1970-01-01 00:00:00 +0000
16+++ build-scripts/lava-token.txt 2011-07-26 18:03:52 +0000
17@@ -0,0 +1,1 @@
18+fyrko3ky6wufoo0rqge9tkmm0nsc06fgt0s6edc67aqdefjm7f7h0ji0aln5uh54yd8o3alcjxktnaa62mob5x6fa2n0htk7mgiwyt6fd0o22aj1lp0zc7biocr0aa6m
19
20=== added file 'build-scripts/lava-user.txt'
21--- build-scripts/lava-user.txt 1970-01-01 00:00:00 +0000
22+++ build-scripts/lava-user.txt 2011-07-26 18:03:52 +0000
23@@ -0,0 +1,1 @@
24+fgiff
25
26=== added file 'build-scripts/post-build-lava.py'
27--- build-scripts/post-build-lava.py 1970-01-01 00:00:00 +0000
28+++ build-scripts/post-build-lava.py 2011-07-26 18:03:52 +0000
29@@ -0,0 +1,102 @@
30+#!/usr/bin/env python
31+import os
32+import json
33+import xmlrpclib
34+
35+def main():
36+ """Script entry point: return some JSON based on calling args.
37+ We should be called from Jenkins and expect the following to
38+ be defined: $TARGET_PRODUCT $JOB_NAME $BUILD_NUMBER $BUILD_URL"""
39+
40+ # Target product name, user defined, e.g. pandaboard
41+ target_product = os.environ.get("TARGET_PRODUCT")
42+ # Job name, defined by android-build, e.g. linaro-android_leb-panda
43+ job_name = os.environ.get("JOB_NAME")
44+ # Build number, defined by android-build, e.g. 61
45+ build_number = os.environ.get("BUILD_NUMBER")
46+ # Build url, defined by android-build, e.g.
47+ # https://android-build.linaro.org/jenkins/job/linaro-android_leb-panda/61/
48+ build_url = os.environ.get("BUILD_URL")
49+ test_plan = os.environ.get("LAVA_TEST_PLAN")
50+ if test_plan == None:
51+ test_plan = "test_android_0xbench"
52+
53+ # Board-specific parameters
54+ if target_product == "pandaboard":
55+ # Board type to test on. Validation does the load balancing.
56+ test_target = "panda"
57+ test_stream = "/anonymous/android-daily/"
58+ elif target_product == "beagleboard":
59+ test_target = "beaglexm"
60+ test_stream = "/anonymous/android-daily/"
61+ #elif target_product == ---> Add other boards here
62+ else:
63+ # We don't know how to test this job, so skip testing.
64+ print "Don't know how to test this board. Skip testing."
65+ return
66+
67+ config = json.dumps( { "job_name": build_url,
68+ "image_type": "android",
69+ "device_type": test_target,
70+ "timeout": 18000,
71+ "actions": [
72+ {
73+ "command": "deploy_linaro_android_image",
74+ "parameters":
75+ {
76+ "boot":
77+build_url + "artifact/build/out/target/product/" + target_product + "/boot.tar.bz2",
78+ "system":
79+build_url + "artifact/build/out/target/product/" + target_product + "/system.tar.bz2",
80+ "data":
81+build_url + "artifact/build/out/target/product/" + target_product + "/userdata.tar.bz2"
82+ },
83+ "metadata":
84+ {
85+ "android.name": job_name,
86+ "android.build": build_number,
87+ "android.url": build_url
88+ }
89+ },
90+ {
91+ "command": "boot_linaro_android_image"
92+ },
93+ {
94+ "command": test_plan
95+ },
96+ {
97+ "command": "submit_results",
98+ "parameters":
99+ {
100+ "result_disk": "sdcard",
101+ "server": "http://validation.linaro.org/lava-server/dashboard",
102+ "stream": test_stream
103+ }
104+ }
105+ ]
106+}, indent=4)
107+
108+ print config
109+ lava_user=os.environ.get("LAVA_USER")
110+ if lava_user == None:
111+ f = open('lava-user.txt')
112+ lava_user = f.read().strip()
113+ f.close()
114+
115+ lava_token=os.environ.get("LAVA_TOKEN")
116+ if lava_token == None:
117+ f = open('lava-token.txt')
118+ lava_token = f.read().strip()
119+ f.close()
120+
121+ lava_server=os.environ.get("LAVA_SERVER")
122+ if lava_server == None:
123+ lava_server = "validation.linaro.org/lava-server/RPC2/"
124+
125+ server = xmlrpclib.ServerProxy("https://%(lava_user)s:%(lava_token)s@%(lava_server)s" % \
126+ dict(lava_user=lava_user, lava_token=lava_token, lava_server=lava_server))
127+ server.scheduler.submit_job(config)
128+
129+if __name__ == "__main__":
130+ main()
131+

Subscribers

People subscribed via source and target branches