Add ability to run observed event ONLY on Juju leader

[marcoppenheimer]

Reasoning

It's very common to have the first part of any event handler to be if not self.charm.unit.is_leader(): return Usually this is because the handler sets app data, but there can be other reasons why you might want to only run on leader.

Although it's small, it would be nice to have some functionality to avoid this.

(IDEA) - Maybe a decorator?

I have a pretty crappy decorator that I wrote that looks roughly like the following:

def leader_check(func: Callable) -> Any:
    def check_unit_leader(*args, **kwargs):
        if not kwargs["event"].unit.is_leader():
            return
        else:
            return func(*args, **kwargs)

    return check_unit_leader

@leader_check
def _on_relation_joined(self, event):
    set_app_data()

but that doesn't run on every event. It would be nice if we could reduce the leader-checks overall in some way.

[rwcarlsen]

The event.unit attribute (which is only on relation events) refers to the remote application unit that triggered the event. You would probably want kwargs["event"].framework.model.unit.is_leader(). @PietroPasotti and others have spent some time thinking about similar needs/issues. We'll keep this suggestion on our radar - it's a good thought.

[sed-i]

There are scenarios in which only some parts are leader-guarded, for example:

    def _set_sources(self, rel: Relation):
        """Inform the consumer about the source configuration."""
        self._set_unit_details(rel)

        if not self._charm.unit.is_leader():
            return

        logger.debug("Setting Grafana data sources: %s", self._scrape_data)
        rel.data[self._charm.app]["grafana_source_data"] = json.dumps(self._scrape_data)

So we'd end up either using both the decorator and the explicit form, or, refactor the function into two function, perhaps artificially.

At some point I tried to generalize this concept to precondition decorators:

    @defer_unless(network_ready, WaitingStatus("Waiting for ip address"))
    def _on_alertmanager_pebble_ready(self, event: ops.charm.PebbleReadyEvent):
        # ...

    @defer_unless(network_ready, WaitingStatus("Waiting for ip address"))
    @skip_unless(config_valid, BlockedStatus("PagerDuty service key missing"))
    def _on_config_changed(self, event: ops.charm.ConfigChangedEvent):
        # ...

but this was abandoned due to insufficient flexibility.

[rwcarlsen]

There are definitely subtleties. I think there is potential in this space to provide some helpful tools. I'm just not sure what they should look like yet.

[Abuelodelanada]

Having something like this in the framework will help us a lot, for instance in grafana-k8s-operator charm we are checking for leadership a lot:

➜  grafana-k8s-operator git:(main) git grep "is_leader()" | egrep "src/|grafana"
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:            if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:            if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if not self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_dashboard.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_source.py:        if not self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_source.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_source.py:        if self._charm.unit.is_leader():
lib/charms/grafana_k8s/v0/grafana_source.py:        if self._charm.unit.is_leader():
src/charm.py:        if self.unit.is_leader() and not self._stored.k8s_service_patched:
src/charm.py:        if not self.unit.is_leader():
src/charm.py:        if not self.unit.is_leader():

May be we are not going to avoid all those checks because in some of them we execute code before, but definitely will help charm authors a lot.

[marcoppenheimer]

The event.unit attribute (which is only on relation events) refers to the remote application unit that triggered the event. You would probably want kwargs["event"].framework.model.unit.is_leader(). @PietroPasotti and others have spent some time thinking about similar needs/issues. We'll keep this suggestion on our radar - it's a good thought.

Yeah it was a crappy solution, decorator probably wouldn't work, but overall I hope the intent was clear.

[PietroPasotti]

Something I'd not be 'too opposed to' is this pattern:

# (in __init__):
if self.unit.is_leader():
    self.framework.observe(foo, self._foo_if_leader)
else: 
    self.framework.observe(foo, self._foo_if_not_leader)

# regardless:
self.framework.observe(bar, self._on_bar)

If you have some events (E.g. relation stuff) you really only care about if you're a leader...

Regarding the fact that leader/nonleader is a common branching point in charms, yeah... I had a half baked design on 'branching charms' which I might need to revisit...

[benhoyt]

I'm not in favour of decorators and suchlike for this. I think ops should reflect the Juju model pretty directly, and I think we should use the simple and straight-forward version:

if not self.charm.unit.is_leader():
  return

If charmers are using this a ton and really want to make their own decorator for this, ops won't stop them.