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

Proposed by Frans Gifford
Status: Merged
Approved by: Paul Sokolovsky
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 Approve
Paul Larson (community) Approve
James Westby (community) Approve
Paul Larson Pending
Alexander Sack Pending
Michael Hudson-Doyle Pending
Zach Pfeffer 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.
Revision history for this message
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-----

Revision history for this message
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
Revision history for this message
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

Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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
Revision history for this message
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

Revision history for this message
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
Revision history for this message
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
Revision history for this message
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

Syntax cleanup

Revision history for this message
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.

Revision history for this message
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
Revision history for this message
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

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

Revision history for this message
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

Use device_type so validation can load balance boards

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

Subscribers

People subscribed via source and target branches