Hook -relation-broken is broken with charms.reactive

[Stuart Bishop]

On 20 December 2015 at 02:13, Cory Johns <cory.johns at canonical.com> wrote:
> On Sat, Dec 19, 2015 at 3:45 AM, Stuart Bishop <stuart.bishop at canonical.com>
> wrote:
>>
>> Which unit name would you use? -relation-departed gets run multiple
>> times, once for each unit. -relation-broken gets run once, after all
>> the -departed hooks have been run.
>
>
> That can't be right.  Surely it must run at least once on each unit still
> remaining in the relation.  Every relation consists of two end-points, and

A relation consists of several end-points. A relation is between two
services, which means it is between several units (several on the
'local' side, and several on the 'remote' side). It is not a
conversation between two units, where two units are passing messages
between each other, but a conference involving several units, where
each unit is yelling at every other unit. I find the
relations-are-conversations metaphor breaks down as soon as you have
two or more units to a service.

>From the Juju docs:

""
[name]-relation-broken indicates that the current relation is no
longer valid, and that the charm's software must be configured as
though the relation had never existed. It will only be called after
every necessary -departed hook has been run; if it's being executed,
you can be sure that no remote units are currently known locally. It
is important to note that the -broken hook might run even if no other
units have ever joined the relation. This is not a bug: even if no
remote units have ever joined, the fact of the unit's participation
can be detected in other hooks via the relation-ids tool, and the
-broken hook needs to execute to give the charm an opportunity to
clean up any optimistically-generated configuration.
"""

A unit appears in the relation. For every unit in the remote service,
the relation-joined and relation-changed hooks are triggered. If a new
unit is added to the remote service, the relation-joined and
relation-changed hooks are triggered. If a unit in the remote service
departs, the relation-departed hook is triggered. If the relation is
destroyed, the relation-departed hook is triggered once for each unit
in the remote service and the relation-broken hook triggered a single
time, after all the relation-departed hooks have been triggered.


> even if the implementation of Juju means that it's difficult or impossible
> for the agent to populate that variable, there's still an objectively
> correct value to put there.  And it doesn't seem unreasonable to have
> expected Juju to do so, like it does for every relation hook.

> Also, as I referenced previously, I'm pretty sure that I could reconstruct
> the expected value by saving the list of departing unit(s) in the charm and
> comparing that to the list during the -broken hook.  But my point was that

I don't think there is a correct value to put there. In the -broken
hook, the there will be zero units in the remote service. At this
point, there is no longer a remote service at all. The only thing you
would be doing is setting $REMOTE_UNIT to the last unit to depart,
which is arbitrary.


> I'm not sure it's worth doing that because I'm not sure I see a use-case for
> this hook that couldn't just as easily be done with the -departed hook
> alone.

relation-departed is when a remote *unit* has gone.
relation-broken is when the remote *service* has gone (or never
appeared in the first place)

In theory, you can use it to clean up resources required by the remote
service. For example, you could shut down daemons no longer required
or destroy the database used by the remote service. In theory, you
don't do this from the relation-departed hook, because the remote
service still exists (even if it has no units) and new units could be
added.

In practice, I don't think I've ever seen this done and suspect
relation-broken hooks are unnecessary atavisms like 'start' and 'stop'
hooks. In particular, cleaning up for $SERVICE in relation-broken is
tricky as Juju does not tell us what $SERVICE actually was - I think
all you know is the former relation id and relation name.



>> As far as the reactive framework is concerned, I don't think it fits
>> as a handler on a RelationBase subclass. It would work fine as a
>> 'standard' parameterless handler. Maybe you want some magic to destroy
>> conversations and such from the now defunct and useless relation
>> object.
>
>
> I'm not sure I understand what you mean here.  I don't see the need for any
> "magic" here.  During a relation hook, charms.reactive needs to be able to
> determine what two units the relation applied to in order to know what
> conversations they may have been participating in and those conversations'
> associated states.  If it can't determine that for -broken, then -broken
> can't be used with charms.reactive.

By magic I meant you may want to destroy any remaining conversations.
If the charm does not implement the relation-departed hook, or the
RelationBase.depart() method not called, you are going to end up with
garbage in unitdata.kv() for conversations in a relation that no
longer exists. We could probably ignore this garbage.

The relation-broken hook does not have a remote unit as context
(unlike relation-joined, relation-changed and relation-departed). It
could still be used in charms.reactive though, just like 'install',
'config-changed', 'leader-elected' and any other hook that does not
have a remote unit.

class FooRelation(BaseRelation):
    scope = scopes.GLOBAL
    @hook('{provides:foo}-relation-changed')
    def configure(self):
        [...]

@hook('{provides:foo}-relation-broken')
def foo_cleanup():
    [...]


> But, again, my question is whether it is even useful to do anything with
> -broken in reactive charms?

It is just as necessary to handle relation-broken hooks in reactive
charms as it is to handle relation-broken hooks in non-reactive
charms. Which is to say, perhaps ;) My personal advice is never use
relation-joined or relation-broken hooks, as I find they do nothing
but add unnecessary complexity. I don't know if charms.reactive should
be that opinionated though.

-- 
Stuart Bishop <stuart.bishop at canonical.com>