This version of the patch is a drop of my "as I was buildduty" WIP. Few notes for anyone who would want to pick this up: * verify.py does NOT do any checks if it is racing with another verify.py or if buildbot is up. * panda relay reboots have a high probability of failing if being used with more than one device on the same relay board at once. * status output merely checks what is the most basic state per the foopy itself, does not even do a ping to the foopy, so is delayed data but real fast!

Comment on attachment 746742 [details] [diff] [review] [diff] [review] [tools] v0.1 ---------AUTOMATIC COMMENT--------- -- filter pep8-callek-june-2013 -- ---------AUTOMATIC COMMENT--------- $pep8 <dir> --diff --max-line-length=159 --show-source < attachment.diff /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:20:1: E302 expected 2 blank lines, found 1 def use_json(fn): ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:51:1: E302 expected 2 blank lines, found 1 @per_device ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:53:4: E111 indentation is not a multiple of four device_dir = '/builds/%s' % device ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:54:4: E111 indentation is not a multiple of four stat_buildbot = "not running" ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:55:4: E111 indentation is not a multiple of four stat_disabled = "enabled" ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:56:4: E111 indentation is not a multiple of four stat_error = "" ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:57:4: E111 indentation is not a multiple of four with hide('stdout', 'stderr', 'running'): ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:58:8: E111 indentation is not a multiple of four with cd(device_dir): ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:70:4: E111 indentation is not a multiple of four print "%s - %12s - %10s - %s" % (device, stat_buildbot, stat_disabled, repr(stat_error[:65].replace('\r\n', '--'))) ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:72:1: E302 expected 2 blank lines, found 1 @per_device ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:75:3: E111 indentation is not a multiple of four if 'tegra' in device: ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:76:7: E111 indentation is not a multiple of four run("SUT_NAME=%s python /builds/sut_tools/tegra_powercycle.py %s && sleep 1" % (device, device)) ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:77:3: E111 indentation is not a multiple of four else: ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:78:7: E111 indentation is not a multiple of four bank, relay = json['relayid'].split(":") ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:79:7: E111 indentation is not a multiple of four run("python /builds/sut_tools/relay.py powercycle %s %s %s ; sleep 5" % (json['relayhost'], bank, relay)) ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:80:3: E111 indentation is not a multiple of four print OK, "Powercycled %s" % device ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:84:1: E303 too many blank lines (3) @per_device ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:100:1: E302 expected 2 blank lines, found 1 @per_device ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:103:8: E111 indentation is not a multiple of four run('touch /builds/%s/disabled.flg' % device) ^ /tmp/tmp.gOx1vw6v8P/buildfarm/maintenance/foopy_fabric.py:106:1: E302 expected 2 blank lines, found 1 @per_device ^

Landed working patch in https://hg.mozilla.org/build/tools/rev/3af3a21056e4 An outstanding TODO, per callek: I note one of my pains with that script atm, is that the panda reboot doesn't do _any_ retrying... which it probably needs since the relayboards only allow one socket connection to it at a time. (that said, I haven't had to use it much fat all for pandas, so may not be worth investing in)

I've created an action ("watcher_version") to report the watcher version per device. Please note prior to version 1.16, the Watcher does not report a version number. I'm having some difficulties testing it at the moment, not sure if it is a problem with my code, or we are just having network/device problems at the moment, will continue to test. Let me know if you have any feedback! Thanks, Pete

Comment on attachment 799398 [details] [diff] [review] An action to show the watcher version per device Review of attachment 799398 [details] [diff] [review]: ----------------------------------------------------------------- haven't tested this action, but at a skim it looks good. So as long as it has been tested go ahead and land

Can someone please grab the bug? This will get it out of the queue of unowned Buildduty bugs and remove the need to triage it.

I made a fix for my previous patch that was not quite working properly, but the output is pretty untidy. Therefore I'll upload this now, as I am about to go on PTO, and will improve the layout when I am back - but feel free to take a look already if you like, as it is working, but the layout is a mess.

I've moved the method into sut_lib, added a caller from sut_tools, and changed the fabric implementation to use the sut_tools utility method. Callek - maybe you can help me a little - might be nice to remove the output from the device manager, so the only output is the version info, rather than also all the connection messages, and the output showing which command is being run on the foopy. Would you be able to help me with this? Thanks buddy! Apart from that, it is totally working, so worst case, you just run it against all, wait for it to finish, and then just grep / ack / ag etc to get the output lines. Of course will be nicer when this extra filter step is not needed. Thanks, Pete

Comment on attachment 807236 [details] [diff] [review] An action to show the watcher version per device wrong attachment!

Comment on attachment 807408 [details] [diff] [review] An action to show the watcher version per device Review of attachment 807408 [details] [diff] [review]: ----------------------------------------------------------------- while I am not too familiar with this code, this looks sane to get the watcher version, some of my feedback might be lack of education. ::: buildfarm/maintenance/foopy_fabric.py @@ +62,4 @@ > run("python /builds/sut_tools/relay.py powercycle %s %s %s ; sleep 5" % (json['relayhost'], bank, relay)) > print OK, "Powercycled %s" % device > > +@per_device what does @per_device mean? @@ +64,5 @@ > > +@per_device > +def watcher_version(device): > + with show('running'): > + run("python /builds/sut_tools/watcher_version.py -t %s" % (device)) where will this print out to? stdout? ::: lib/python/sut_lib/watcher.py @@ +5,5 @@ > +from mozdevice import droid > + > +def getWatcherVersion(tegra): > + try: > + results = droid.DroidSUT(tegra)._runCmds([{'cmd': 'exec su -c "cat /data/data/com.mozilla.watcher/files/version.txt"'}]).split('\n') using droid/droidsut with exec su seems sort of hairy, but I assume this works well. ::: sut_tools/watcher_version.py @@ +16,5 @@ > +oSummary = None > +defaultOptions = { > + 'bbpath': ('-p', '--bbpath', '/builds', > + 'Path where the Tegra buildbot slave clients can be found'), > + 'tegra': ('-t', '--tegra', None, this is hard coded to a tegra, what about the pandas?

(In reply to Joel Maher (:jmaher) from comment #9) > Comment on attachment 807408 [details] [diff] [review] > An action to show the watcher version per device > > Review of attachment 807408 [details] [diff] [review]: > ----------------------------------------------------------------- > > while I am not too familiar with this code, this looks sane to get the > watcher version, some of my feedback might be lack of education. > > ::: buildfarm/maintenance/foopy_fabric.py > @@ +62,4 @@ > > run("python /builds/sut_tools/relay.py powercycle %s %s %s ; sleep 5" % (json['relayhost'], bank, relay)) > > print OK, "Powercycled %s" % device > > > > +@per_device > > what does @per_device mean? Its an annotation for the other code that tells it to run against each device on a host, rather than once per host. > @@ +64,5 @@ > > > > +@per_device > > +def watcher_version(device): > > + with show('running'): > > + run("python /builds/sut_tools/watcher_version.py -t %s" % (device)) > > where will this print out to? stdout? Yes. > ::: lib/python/sut_lib/watcher.py > @@ +5,5 @@ > > +from mozdevice import droid > > + > > +def getWatcherVersion(tegra): > > + try: > > + results = droid.DroidSUT(tegra)._runCmds([{'cmd': 'exec su -c "cat /data/data/com.mozilla.watcher/files/version.txt"'}]).split('\n') > > using droid/droidsut with exec su seems sort of hairy, but I assume this > works well. I agree with the reservation here, I'd rather use devicemanagerSUT than droid.DroidSUT if at all possible without too much hassle. > ::: sut_tools/watcher_version.py > @@ +16,5 @@ > > +oSummary = None > > +defaultOptions = { > > + 'bbpath': ('-p', '--bbpath', '/builds', > > + 'Path where the Tegra buildbot slave clients can be found'), > > + 'tegra': ('-t', '--tegra', None, > > this is hard coded to a tegra, what about the pandas? Like Joel I prefer the newer-syntax of --device rather than --tegra. (now off to do my own review rather than tack onto jmaher's, but the coming r- will be at the least for this --device rather than --tegra syntax choice.)

Comment on attachment 807408 [details] [diff] [review] An action to show the watcher version per device Review of attachment 807408 [details] [diff] [review]: ----------------------------------------------------------------- ::: sut_tools/watcher_version.py @@ +31,5 @@ > + if options.tegra is None: > + for f in os.listdir(options.bbpath): > + if os.path.isdir(os.path.join(options.bbpath, f)) \ > + and 'tegra-' in f.lower(): > + tegras.append(f) not really a fan of doing the hardcoded path checking here... I'd rather force us to pass a device in options for now.

Looks like this one slipped off my radar at some point, assigning to myself so it doesn't happen again...

We no longer use foopies, therefore no need for foopy_fabric