Allowing two requirers for the same relation

[dimaqq]

Re: two Requirers for the same relation. We've got a check for that in ops/framework.py _track(...) And that fails with

12345678    def _track(self, obj: 'Serializable'):
        """Track object and ensure it is the only object created using its handle path."""
        if obj is self:
            # Framework objects don't track themselves
            return
        if obj.handle.path in self.framework._objects:
>           raise RuntimeError(f'two objects claiming to be {obj.handle.path} have been created')
E           RuntimeError: two objects claiming to be TempoCoordinatorCharm/TracingEndpointRequirer[self-charm-tracing] have been created

I'll hack in this area a bit, but I think that ultimately I should either:

• try to update cos-lib and then temp-coordinator (seems convoluted), or
• take some other charm with a COS integration test, add tracing data test, try to upgrade that charm

Let me sleep on this decision and maybe let's discuss this on Monday at the stand-up.

I have an idea how we ended up here. Perhaps that the original intention for event paths was for Object's to be nested, so that we could have:

SomeCharm/SomeOtherLib/TracingEndpointRequirer[...] side by side with
SomeCharm/Tracing/TracingEndpointRequirer[...]

That would make sense for Objects that don't have exclusive ownership of a thing, like a relation.

However, we don't allow that -- both the type hint and the internal code require Object's init argument to be BaseCharm, not another Object. So charmers and I end up with a flattened observer bush (name?) and not a tree:

SomeCharm/SomeOtherLib
SomeCharm/TracingEndpointRequirer[...]
SomeCharm/Tracing
SomeCharm/TracingEndpointRequirer[...]

Let's discuss this in Frankfurt.

Summary: https://gist.github.com/benhoyt/06f5b38961609edf3c60ad7470e84573

[dimaqq]

To do

• [ ] think of a case where two requirers target the same relation
• [ ] exact code sample
• [ ] socialise and understand if that's a real issue

PoC

A charm has two workload containers, each served by a charm lib. Each needs the CA cert, and there's a single relation to get the CA cert. Each charm lib instantiates a CertificateRequirer.

[tonyandrewmeyer]

However, we don't allow that -- both the type hint and the internal code require Object's init argument to be BaseCharm, not another Object.

Could you elaborate more on this? Object's __init__ is:

def __init__(self, parent: Framework | Object, key: str | None):
        self.framework: Framework = None  # type: ignore
        self.handle: Handle = None  # type: ignore

        kind = self.handle_kind
        if isinstance(parent, Framework):
            self.framework = parent
            # Avoid Framework instances having a circular reference to themselves.
            if self.framework is self:
                self.framework = weakref.proxy(self.framework)
            self.handle = Handle(None, kind, typing.cast('str', key))
        else:
            self.framework = parent.framework
            self.handle = Handle(parent, kind, typing.cast('str', key))
        self.framework._track(self)  # type: ignore

        # TODO Detect conflicting handles here.

I assume you're meaning the parent argument, but that can either be a Framework or another Object. Isn't passing an Object the kind of nesting that you're wanting here? There's no requirement for BaseCharm (presumably a typo for CharmBase) here - the framework doesn't know about charms (a symptom of how overly generic it is, I feel). Where are you seeing a CharmBase argument requirement?

[dimaqq]

I gave it a go in ops[tracing] vendored charm libs, and so far I could not get it to work.

How do I fix this?

In a charm lib SomeObject.__init__(), the set of events the charm is allowed to listen on (derived from metadata.yaml) is available on the charm instance, but not on the framework instance:

(Pdb) charm.on
<scenario._runtime.Runtime._wrap.<locals>.WrappedEvents: charm_tracing_relation_broken, charm_tracing_relation_changed, charm_tracing_relation_created, charm_tracing_relation_departed, charm_tracing_relation_joined, collect_app_status, collect_metrics, collect_unit_status, config_changed, install, leader_elected, leader_settings_changed, post_series_upgrade, pre_series_upgrade, receive_ca_cert_relation_broken, receive_ca_cert_relation_changed, receive_ca_cert_relation_created, receive_ca_cert_relation_departed, receive_ca_cert_relation_joined, remove, secret_changed, secret_expired, secret_remove, secret_rotate, start, stop, update_status, upgrade_charm>
(Pdb) self.framework.on
<ops.framework.FrameworkEvents: commit, pre_commit>

Perhaps that's simply an oversight in Scenario run-time though.

Other

Today, all(?) the charm libs expect a BaseCharm instance and not an Object of Framework instance, perhaps due to the limitation above.

Thus it takes quite a few source code changes to update the charm lib object tree to the "nested way of doing things", see https://github.com/dimaqq/operator/pull/46

[dimaqq]

Fixing self.framework.on would probably require major surgery.

[dimaqq]

Recapping earlier conversation, my and @tonyandrewmeyer's guts tell us that it's probably not worth the effort.