822534 - Coerce manage_foopy fabric script to accept individual devices as well.

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task)

Description

[Justin Wood (:Callek)]

Attachment: [tools] v1

So this does not (yet) switch over any actions to accept per_device, but pulls out data so we have all that is in devices.json exposed to the script.

It also follows the following rules:

-H --host
  Specify multiple (or "all") or at least one device
-D --device
  Specify multiple (or "all") or at least one device

Where if you specify a host, all devices will get used for a per_device action
If you specify a device its whole host will get used for a per_host action
If you specify a combination, we'll use all devices on the hosts specified, and if just a per_device action only those devices, otherwise we'll union those hosts with the hosts specified. (if that makes sense)

Future expansion here can allow XOR (e.g. -H -foopy05) or even Role (-R production) etc. But that is less important ;-)

This also comes with the following assumptions:
* We don't want a high value of -j to escalate to a DOS on a given host, so we will treat all outgoing device-specific host actions as -j1 on a per-host-basis (as in, when stop_cp is doable on a per-device basis, we won't run 13 stop_cp's on the same host at the same time.)
** This will inflate per_device actions in terms of end-to-end time, but I feel this is safer for our collective sanity.
* We only ever need the foopy name itself for host specific actions
* We will be planning to connect to a remote host for every per_device action in the near-future (we'd need to add other logic if this no longer holds)

Any questions/issues fire-away.

Comment 1

[Justin Wood (:Callek)]

Comment on attachment 693218 [details] [diff] [review]
[tools] v1

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

::: buildfarm/maintenance/manage_foopies.py
@@ +111,1 @@
>          parser.error("You need to specify a foopy via -H")

self-nit: "...via -H or a device via -D")

@@ +114,5 @@
>      
>      for action in actions:
> +        fn = run_action_on_foopy
> +        genArg = lambda x: x
> +        if hasattr(action, "per_device"):

...actually this *never* does what we want -- it fails because actions is grabbed from optparse but we're checking for an attribute defined on the action-function itself.

This is also an issue in manage_masters.py.

I am leaving the review open on you for the general approach, will need to touch this up though.

@@ +116,5 @@
> +        fn = run_action_on_foopy
> +        genArg = lambda x: x
> +        if hasattr(action, "per_device"):
> +            fn = run_action_on_devices
> +            genArg = lambda x: selected[x]

self-comment meant to be: dict([x, selected[x]]) (so we end up passing in a one-key dict with the foopy-to-use as the only key, whose value itself is a dict of devices)

Comment 2

[Armen [:armenzg]]

Comment on attachment 693218 [details] [diff] [review]
[tools] v1

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

f=armenzg

Over all it looks good. Shoot review once you're happy with the patch.

Comment 3

[Justin Wood (:Callek)]

Attachment: [tools] v2

(In reply to Justin Wood (:Callek) from comment #1)
> Comment on attachment 693218 [details] [diff] [review]
> [tools] v1
>
> @@ +116,5 @@
> > +        fn = run_action_on_foopy
> > +        genArg = lambda x: x
> > +        if hasattr(action, "per_device"):
> > +            fn = run_action_on_devices
> > +            genArg = lambda x: selected[x]
> 
> self-comment meant to be: dict([x, selected[x]]) (so we end up passing in a
> one-key dict with the foopy-to-use as the only key, whose value itself is a
> dict of devices)

This ended up getting changed to have a "host" key and "devices" key, where devices is teh dict of devices.

I'll also attach a POC patch (which will be part of another bug) which does the switch to per-device ;-) [and adds another action that can be used per-device]

Comment 4

[Justin Wood (:Callek)]

Attachment: [tools] POC

This is the proof of concept patch to layer on top of the v2 patch here. It is not going to land as part of this bug, but shows the general direction of what v2 is doing.

Comment 5

[Armen [:armenzg]]

Comment on attachment 694064 [details] [diff] [review]
[tools] POC

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

LGTM.

Comment 6

[Justin Wood (:Callek)]

Comment on attachment 694063 [details] [diff] [review]
[tools] v2

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