Comment 18 for bug 1654116

I haven't dug particularly deeply. However if I do
 charm pull ceilometer
I get: cs:ceilometer-24
And then dig into the contents of:
 charmhelpers/contrib/peerstorage/__init__.py

I see that it has a function:
def leader_get():

which looks like it is supposed to be a compatibility function, so that old versions of the charm can just use peer relation settings to coordinate, and new versions of the charm can use leader-set and leader-get directly. However, if I'm reading through the code it does:

  if attribute:
    ...
        # If attribute not present in leader db, check if this unit has set
        # the attribute in the peer relation
        if not leader_settings:
            peer_setting = _relation_get(attribute=attribute, unit=local_unit(),
                                         rid=rid)
            if peer_setting:
                leader_set(settings={attribute: peer_setting})
                leader_settings = peer_setting

Nothing in there does any sort of check for "is_leader". It is possible that the original call to "leader_get" itself is done inside a "is_leader" check, but this function appears to be trying to "look for settings that might have been set by a leader, but fall back to a peer relation, and if found in a peer relation set them in the leader settings" which would only be safe to do *as* the leader.

Note also that peerstorage.__init__.py has a "relation_get" function which directly calls "leader_get" which (as noted) might try to call leader_set.

Similarly the function "peer_retrieve" tries to call relation_get, which may call leader_get which may ultimately call 'leader_set'.

There may be other issues, but the code as written appears to be unsafe to use.

It may be that a better writing would trap inside "leader_get" and only attempt to do a migration if "is_leader()" returned True?