Add master_manager_launch script which launches master_manager scripts for each master on a host.

infra/tools/master_manager_launcher/desired_state_parser.py

+#!/usr/bin/python
+# Copyright 2015 Google Inc. All Rights Reserved.
+# pylint: disable=F0401
+
+"""Parse, validate and query the desired master state json."""
+
+import bisect
+import json
+import logging
+import operator
+import os
+
+from infra.libs.buildbot import master
+from infra.libs.time_functions import timestamp
+from infra.services.master_lifecycle import buildbot_state
+
+
+LOGGER = logging.getLogger(__name__)
+
+
+def load_desired_state_file(filename):  # pragma: no cover

[agable, 2015/05/06 23:29:30]

Feels odd. Not finding the file is exception-worthy. Finding the file and having
it totally empty is fine. Finding the file and having it invalid json results in
None, which must then be handled by the caller.

[ghost stip (do not use), 2015/05/07 06:50:57]

Done.

+  with open(filename) as f:
+    desired_state = json.load(f)
+  if not desired_master_state_is_valid(desired_state):
+    return None
+  return desired_state
+
+
+def desired_master_state_is_valid(desired_state):
+  """Verify that the desired_master_state file is valid."""
+  now = timestamp.utcnow_ts()
+
+  for mastername, states in desired_state.iteritems():
+    # Verify desired_state and transition_time_utc are present.
+    for k in ('desired_state', 'transition_time_utc'):
+      if not all(k in state for state in states):
+        LOGGER.error(
+            'one or more states for master %s do not contain %s', mastername, k)
+        return False
+
+    # Verify the list is properly sorted.
+    sorted_states = sorted(

[agable, 2015/05/06 23:29:30]

Why does it matter if they're sorted in the file when they have timestamps
anyway?

[ghost stip (do not use), 2015/05/07 06:50:57]

Because a human editing / reading the file would be *very* confused if a later
state was added in the middle of the file somewhere. The parser is very strict
here to strongly discourage people from doing that.

[agable, 2015/05/07 17:59:31]

Fair enough, that makes sense.

+        states, key=operator.itemgetter('transition_time_utc'))
+    if sorted_states != states:
+      LOGGER.error('master %s does not have states sorted by timestamp',
+          mastername)
+      LOGGER.error('should be:\n%s', json.dumps(sorted_states, indent=2))
+      return False
+
+    # Verify desired_state and timestamp are valid.
+    for state in states:
+      if (state['desired_state'] not in
+          buildbot_state.STATES['desired_buildbot_state']):
+        LOGGER.error(
+            'desired_state \'%s\' is not one of %s',
+            state['desired_state'],
+            buildbot_state.STATES['desired_buildbot_state'])
+        return False
+
+      if not isinstance(state['transition_time_utc'], (int, float)):
+        LOGGER.error(
+            'transition_time_utc \'%s\' is not an int or float',
+            state['transition_time_utc'])
+        return False
+
+    # Verify there is at least one state in the past.
+    if not get_master_state(states, now=now):

[agable, 2015/05/06 23:29:30]

Why? Why can't I create the file containing just a single future state that I'd
like it to end up in?

[iannucci, 2015/05/06 23:48:05]

Because then the script doesn't know what state it's supposed to be in in the
mean time.

timestamp / state
0 / offline
1 / online

is different than
0 / drained
1 / online

If you just gave it `1 / online`, it wouldn't be able to take any action until
'1', because it doesn't know what the pre-1 state should be.

[ghost stip (do not use), 2015/05/07 06:50:57]

Right, this is a discussion Robbie and I had in mountain view last tuesday.
Basically, leave master_manager as a very simple script which only has a single,
fully defined goal. It raises an exception if you give it an action to perform
in the future (as that leaves its current goal underdefined).
master_manager_launcher determines what goal to give master_manager based on a
list of goal states over time.

this has a hidden benefit: we can now have a script which generates CLs to
update a rolling 'restart schedule' for each master, similar to the sheriff
rotation system. a human can even edit the file to postpone or remove actions in
the future.

[agable, 2015/05/07 17:59:31]

I don't see how my proposal goes against your discussion with Robbie. The script
is still simple, and has a single fully defined goal: "Oh, the only timestamp I
care about is in the future, so I do nothing." I think it is ugly for a person
who wants to force a master to do something to have to put *two* entries in a
file, one of which contains a meaningless timestamp arbitrarily far (or recent)
in the past.

[ghost stip (do not use), 2015/05/07 19:49:39]

There is only one case where a person who wants to restart a master *in the
future* has to put two entries. As you'll see, this makes sense:

Case 1: The master already has entries in the desired_master_state file and the
person wants to do something in the future. The person only adds one entry, in
the future, because the present case is already taken care of by previous
entries.

Case 2: The master is not in the desired_master_state file (perhaps it is new)
and wants to do an action now. The person only adds one entry with a timestamp
of 'slightly in the past', and the action is immediately performed.

Case 3: The master is not in the desired_master_state file (perhaps it is new)
and wants to do an action in the future. This is particularly rare, as it means
that someone has made a new master and wants to do something to it later. The
person adds two entries: the 'meaningless timestamp' describing the current
state, and the action in the future.

The reason this makes sense is that it obviates all kinds of special-case logic
for when the present is undefined. What if someone says 'offline' in the future,
the master is running right now, and you kill it? Do we keep the script running
and *then* bring it offline in the future? Do we keep it offline because "that's
what they want later anyway?"

What if someone says 'drained' in the future, and it's offline now. Do we start
it just to keep it drained, or keep it offline?

All these kinds of weird edge cases go away if you just *tell* it what you want
for the present state (most likely 'running' or 'offline'), and you only have to
do this once for when the master isn't in the system yet *and* you want
something in the future.

[agable, 2015/05/08 00:14:53]

Case 1.5: The file used to contain a bunch of entries, but has been cleaned out
for the sake of readability. Goto Case 2 or 3.

On 2015/05/07 at 19:49:39, stip wrote:
I think these "what do we do" objections are contrived. The answer is always
"nothing". If the only action is in the future, we *do nothing*, exactly as
though the file were empty and completely unmanaged by the master-manager. There
are no corner cases here.

[ghost stip (do not use), 2015/05/08 00:22:13]

You can't clean it out for readability fully, because the script (and presubmit)
won't allow you to delete all entries -- you must have at least one describing a
state in the past.
we'll see how it plays out, but we can always tear out logic in the future if
it's not working out

+      LOGGER.error(
+          'master %s does not have a state older than %s', mastername, now)
+      return False
+
+  return True
+
+
+def get_master_state(states, now=None):
+  """Returns the latest state earlier than the current (or specified) time.
+
+  If there are three items, each with transition times of 100, 200 and 300:
+    * calling when 'now' is 50 will return None
+    * calling when 'now' is 150 will return the first item
+    * calling when 'now' is 400 will return the third item
+  """
+  now = now or timestamp.utcnow_ts()
+
+  times = [x['transition_time_utc'] for x in states]
+  index = bisect.bisect_left(times, now)
+  if index:
+    return states[index - 1]
+  return None
+
+
+def get_masters_for_host(desired_state, build_dir, hostname):
+  """Identify which masters on this host should be managed.
+
+  Returns two lists: triggered_masters and ignored_masters.

[agable, 2015/05/06 23:29:30]

Any reason they're sets instead of lists? Ordering isn't important here afaict.

[ghost stip (do not use), 2015/05/07 06:50:57]

TypeError: unhashable type: 'dict'

ignored_masters has been made a set (since it's just strings)

+
+  triggered_masters is a list of dicts. Each dict is the full dict from
+  mastermap.py with two extra keys: 'fulldir' (the absolute path to the master
+  directory), and 'states' (a list of desired states sorted by transition time,
+  pulled from the desired states file).
+
+  ignored_masters is merely a list of 'dirname' strings (ex: master.chromium).

[agable, 2015/05/06 23:29:30]

Docstring should explain why they're ignored

[ghost stip (do not use), 2015/05/07 06:50:57]

Done.

+  """
+  triggered_masters = []
+  ignored_masters = []
+  for master_dict in master.get_mastermap_for_host(
+      build_dir, hostname):
+    if master_dict['dirname'] in desired_state:
+      if master_dict['internal']:
+        master_dir = os.path.abspath(os.path.join(
+          build_dir, os.pardir, 'build_internal', 'masters',
+          master_dict['dirname']))
+      else:
+        master_dir = os.path.abspath(os.path.join(
+          build_dir, 'masters', master_dict['dirname']))
+      master_dict['fulldir'] = master_dir
+      master_dict['states'] = desired_state[master_dict['dirname']]
+
+      triggered_masters.append(master_dict)
+    else:
+      ignored_masters.append(master_dict['dirname'])
+  return triggered_masters, ignored_masters