uniter: lifecycle handling

uniter: lifecycle handling

This remains WIP just for the moment, largely because it lacks tests (ha!).

Things to note and perhaps comment on at this stage:

* The modeStarting type is, I *think*, a Good Thing, because it allows me to
  focus on smaller parts of the problem at a time. I believe it will start
  to really pull its weight when I add relations; unless you feel there's
  something fundamentally wrong with the concept/approach, please be
  indulgent of its existence for a CL or two :).

* Man, lifecycle handling is a lot of code. This is particularly noticeable
  in ModeHookError and ModeConflicted, which now share enough lifecycle and
  watch manipulation logic that I'm very tempted to carve out a parametrized
  modeAwaitResolved to keep it in. I'm really not sure whether this is
  wantonly premature abstraction or sensible factoring of fiddly logic into
  one place only, but I kinda still lean towards the former; this would most
  probably change if we got *another* distinct error mode.

...but, really, anything that springs to mind is fair game; absent comments,
I expect to propose something pretty close to this at some stage tomorrow.

https://code.launchpad.net/~fwereade/juju-core/uniter-unit-lifecycle/+merge/126558

(do not edit description out of merge proposal)

[fwereade, 2012-09-26 22:00:08]

Please take a look.

https://codereview.appspot.com/6575053/diff/1/worker/uniter/modes.go
File worker/uniter/modes.go (right):

https://codereview.appspot.com/6575053/diff/1/worker/uniter/modes.go#newcode214
worker/uniter/modes.go:214: // ...otherwise, shut down.
This should be "if no relationers exist, shut down", so that we fall through to
ensuring hooks. Er, maybe. I'll know better soon.

[niemeyer, 2012-09-27 01:04:50]

This is a great code spike. It enables us to have a pretty clear picture of what
has to be done and where. It also gives some comfort that the mode concept is
flexible enough to handle further abuse.

That said, I think it's time to step back and clean it up a bit by understanding
what we're doing. We have a lot of logic repetition, we have state machines
within the state machine (only watch bar after the first event of watching foo),
we're handling events which are global in a local way (either the whole unit is
supposed to die or not), etc.

I think pretty much every watcher we have can be setup upfront, and be made
available via the Uniter interface. We also don't have to offer the modes such a
low-level interface. A few examples to get your mind ticking:

    case <-uniter.tombDying():

This is closed when the unit tomb dies. It's probably just a dumb method that
returns u.tomb.Dying(), but it's good to follow the pattern.

    case <-uniter.unitDying():

This is closed when either the service or the unit state change and become
dying.

    case ch := <-uniter.charmUpgrade():

This fires once every time the service properties are found to be modified (via
a watcher) *and* the charm URL is found to have changed.

    case config := <-uniter.configChanged():

This fires once every time the service configuration has changed. To support it,
we'd also have:

    config := <-uniter.currentConfig()

in the same loop that feeds configChanged. This means we avoid the current back
and forth to handle the config-changed hook, and can manipulate its execution
synchronously.

None of these channels should be closed, so we don't have to  worry about ok
repeatedly.

You see where this is going? Except for tombDying none of these channels *have*
to be consumed, which means it's totally fine for a mode to care only about its
own business and ignore everything else that it's someone else's problem.

I suspect this would not only avoid the significant burden that the code spike
is currently exposing by the introduction of lifecycle support, but it'd also
simplify the currently existing logic.