797868 - add a pdu cycle to the sut_tools for reboots instead of dm.reboot

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

yet another controversial bug, but if we want panda boards to be automated we either need to take this or somebody else needs to debug.  

This adds relay.py which :ted wrote to powercycle the panda boards.  This also instruments the sut_tools code to use relay.py as a default and fall back to the dm.reboot.  In the current implementation it will fail to cycle if the device is not a panda board, so this conveniently resolves backwards compatibility with the tegras :)

One place this doesn't change things is inside of cleanup.py.  We do a dm.uninstallAppAndReboot() which reboots the device.  Of course this assumes that the device is in a state to reboot via sutagent which sometimes it won't.  In my final patch in bug 781341 this is reworked to only use dm.uninstallApp() which doesn't reboot and we will pdu_cycle iff we have uninstalled.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: add pdu_cycle to sut_tools and use it instead of dm.reboot for panda boards (1.0)

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 667983 [details] [diff] [review]
add pdu_cycle to sut_tools and use it instead of dm.reboot for panda boards (1.0)

Review of attachment 667983 [details] [diff] [review]:
-----------------------------------------------------------------

Besides being bitrotted, I'm not a fan of the csv use here, I'm also not a fan of splitting up the decision to dm.reboot vs pdu/relay cycle throughout so many files. So I will attach a file in a bit, f? to kim for her opinions, and r? to joel instead.

I recognize my patch will bitrot Bug 781341 as well, but I THIS patch will seperate from a new patch I'll be writing there in the same way this bug and that bug were already seperated.

Comment 3

[Justin Wood (:Callek)]

Attachment: do relay-reboot for pandas, adds pandas to devices.json

I didn't get around to staging this tonight, as I planned, will do that tomorrow on the tegra-staging machines, for sanity there. Will stage with kim's panda foopies once I roll in a Bug 781341 and then compare against the changes she has there, to be sure I don't regress any of her local changes.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 674988 [details] [diff] [review]
do relay-reboot for pandas, adds pandas to devices.json

Review of attachment 674988 [details] [diff] [review]:
-----------------------------------------------------------------

I think the soft_reboot name is confusing.  At first I saw it and really liked it, but then when I saw the implementation it does a hard reboot for pandas.  This is nice in that the same reboot method solves the problem for both scenarios.  Maybe we should call it device_reboot instead?

is relay.py the same as what I uploaded (a copy of :ted's repo)?

::: buildfarm/mobile/devices.json
@@ +1640,5 @@
>          "pduid": ".AA3"
> +    },
> +    "panda-0022": {
> +        "foopy": "foopy33",
> +        "relayhost": "panda-relay-06.build.mozilla.org",

isn't it panda-relay-006 now?

@@ +1641,5 @@
> +    },
> +    "panda-0022": {
> +        "foopy": "foopy33",
> +        "relayhost": "panda-relay-06.build.mozilla.org",
> +        "relayid": "1:1"

I don't like the bank/relay being bundled together- I always think we will need them in the future.

@@ +1722,5 @@
> +    "panda-0050": {
> +        "foopy": "foopy34",
> +        "relayhost": "panda-relay-04.build.mozilla.org",
> +        "relayid": "2:1"
> +    }, 

nit: whitespace

::: sut_tools/sut_lib.py
@@ +493,5 @@
>  
> +def soft_reboot(device, dm, *args, **kwargs):
> +    """
> +    Use the softest/kindest reboot method we think we should use.
> +    

nit: whitespace

Comment 5

[Justin Wood (:Callek)]

Comment on attachment 674988 [details] [diff] [review]
do relay-reboot for pandas, adds pandas to devices.json

Review of attachment 674988 [details] [diff] [review]:
-----------------------------------------------------------------

> I think the soft_reboot name is confusing. At first I saw it and really liked it, but then when I saw
> the implementation it does a hard reboot for pandas. This is nice in that the same reboot method solves
> the problem for both scenarios. Maybe we should call it device_reboot instead?

Discussed on IRC, with reboot_device named as such here, theres no good name as it stands, we will likely want to refactor/rename some of these functions to make it all clearer for the next person to need to touch this code though, in the future.

> is relay.py the same as what I uploaded (a copy of :ted's repo)?

Yes, I skimmed it, and see no glaring problems, so happy to take it as-is.

::: buildfarm/mobile/devices.json
@@ +1640,5 @@
>          "pduid": ".AA3"
> +    },
> +    "panda-0022": {
> +        "foopy": "foopy33",
> +        "relayhost": "panda-relay-06.build.mozilla.org",

Yes it is zero-padded one more, fixed up (also *this* relay was wrong in our configs, kmoir pointed it out and I fixed locally)

@@ +1641,5 @@
> +    },
> +    "panda-0022": {
> +        "foopy": "foopy33",
> +        "relayhost": "panda-relay-06.build.mozilla.org",
> +        "relayid": "1:1"

Discussed on IRC, we're ok with this for now, but in the future might abstract out to something like "pduType": and have "pduHost" and "pduID" as consistent values, so that we can interchange as needed.

::: sut_tools/config.py
@@ +88,5 @@
>  proxyPort = calculatePort()
>  refWidth  = 1600 # x
>  refHeight = 1200 # y
> +deviceName = os.path.basename(cwd)
> +deviceIP = sys.argv[1]

Just to say, many of these sut_* scripts dely on `cwd` for now, and instead of hard-coding IP in the json/csv or needing to calc IP for every panda at least once per script run, I went with calculating deviceName based on the os.path.basename which forces us to use 'panda-*' or 'tegra-*' for all devices.

Alternatively we could magically inherit from os.env (like updateSUT does) if this is deemed too much of a pain.

::: sut_tools/sut_lib.py
@@ +501,5 @@
> +        # Using devicemanager for reboots on pandas doesn't work reliably
> +        if reboot_relay(device):
> +            return True
> +        else:
> +            log.warn("Error while attempting to reboot %s via Relay Board." % device)

Per IRC discussion we'll bubble this warning up to TBPL with
            log.warn("Automation Error: Unable to reboot %s via Relay Board." % device)

Comment 6

[Kim Moir [:kmoir] ET]

Comment on attachment 674988 [details] [diff] [review]
do relay-reboot for pandas, adds pandas to devices.json

The the only feedback I have is that that relay 6 should be relay 2 need to be fixed in the devices.json and the relayhost need to be padded with an extra 0 we discussed in IRC yesterday.  Also, I have foopy 36 attached with more pandas but they can be added in later patch. Sorry for the delay, I assumed our IRC conversation was feedback :-)

Comment 7

[Justin Wood (:Callek)]

Attachment: interdiff of touch-up fixes

Minor needed fixes.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 675917 [details] [diff] [review]
interdiff of touch-up fixes

Review of attachment 675917 [details] [diff] [review]:
-----------------------------------------------------------------

small and simple!

Comment 9

[Justin Wood (:Callek)]

Deployed 2012-10-29 ~3:00 PT

http://hg.mozilla.org/build/tools/rev/a6974cf7947f