Sorry, NOT LGTM as it stands. I think it's a matter of:
select {
case <- u.f.UnitDying():
...
default:
}
...at appropriate points in the modes which *can* run before we're
started, and short-circuiting to ModeTerminating only out of those. Once
relations and hook errors could be involved, the proposed approach
becomes, er, risky at best.
https://codereview.appspot.com/12517043/diff/1/worker/uniter/modes.go#newcode348
worker/uniter/modes.go:348: return modeAbideDyingLoop(u)
I don't think this is right. If we've had a hook error we're in an
unknown state, and subsequent hooks are also likely to fail; and if we
fail a -broken hook we won't depart the relation; and if a unit goes
away without departing all its relations, those relations (and their
services) will become unkillable. Which would be a Bad Thing.
Sorry, NOT LGTM as it stands. I think it's a matter of:
select {
case <- u.f.UnitDying():
...
default:
}
...at appropriate points in the modes which *can* run before we're
started, and short-circuiting to ModeTerminating only out of those. Once
relations and hook errors could be involved, the proposed approach
becomes, er, risky at best.
https:/ /codereview. appspot. com/12517043/ diff/1/ worker/ uniter/ modes.go uniter/ modes.go (right):
File worker/
https:/ /codereview. appspot. com/12517043/ diff/1/ worker/ uniter/ modes.go# newcode291 uniter/ modes.go: 291: return ModeTerminating, nil
worker/
Hitherto, were we in ModeAbide, we must have been started.
https:/ /codereview. appspot. com/12517043/ diff/1/ worker/ uniter/ modes.go# newcode348 uniter/ modes.go: 348: return modeAbideDyingL oop(u)
worker/
I don't think this is right. If we've had a hook error we're in an
unknown state, and subsequent hooks are also likely to fail; and if we
fail a -broken hook we won't depart the relation; and if a unit goes
away without departing all its relations, those relations (and their
services) will become unkillable. Which would be a Bad Thing.
https:/ /codereview. appspot. com/12517043/