-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support relation aliases and multiple relations #30
Comments
Several connections can be created using alias for the application itself: juju deploy redis-k8s redis-cache
juju deploy redis-k8s redis-broker
juju deploy my-app
juju relate redis-cache my-app
juju relate redis-broker my-app In this case, information about the relation can be accessed on for relation in self.model.relations["<metadata-relation-name>"]:
print(relation.app.name) This would print: 'redis-cache' and 'redis-broker'. I'm trying to figure out if there is a better way to obtain this information and I'll update accordingly. I've been trying this out with the current redis-k8s charm and a small charm I use for development and testing. Would this cover the use case? @mthaddon @arturo-seijas |
Hi Folks, We think this will be possible, and will confirm the code changes needed to make it happen (from our side). However, we also think it'll be a little sub-optimal as it'll mean we need to manually keep track of which one is the cache and which one is the broker. If you're working on a new library, we just wanted to confirm at what point you'd be committed to one approach or another? This will help us prioritise the work to confirm exactly whether the approach above will work and how important it might be to have dedicated relations for these. Thanks, Tom |
Hi Raul, I've managed to cover our use case following your advice. Thank you for that. Unfortunately, I don't find the solution very intuitive from a library consumer's perspective. An example of the resulting code to retrieve the relation data is as follows:
This doesn't look as intuitive as having a unique listener for each relation defined in the metadata. Also, I needed to discriminate the relations based on the alias, which isn't visible for the outside.
For the reasons above I think that it'd be worth to align this charm library with the expected behaviour, that is, supporting the multiple relations defined in the metadata file. Thanks |
An alternative to iterating over all relations could be to use the application peer databag to store mapping information: def _on_database_created(self, event: RelationCreatedEvent):
# As juju leader:
# Get the remote app name
remote_redis_name = event.relation.app.name
# Create an entry on the application databag (app_databag = self.model.get_relation(PEER_RELATION).data[self.app])
self.app_databag[remote_redis_name] = {} I think that for older relations this is the correct approach. The main issue is that the current redis relation library uses Once the new relation is in place, there will be the possibility of declaring event handlers for each relation. See here As per the unit test, I'm afraid that's how it's going to look like regardless of relation implementation. This is a current peer relation test: # At test setUp():
self._peer_relation = "redis-peers"
self.harness.add_relation(self._peer_relation, self.harness.charm.app.name)
# Relation test:
rel = self.harness.charm.model.get_relation(self._peer_relation)
# Trigger peer_relation_joined/changed
self.harness.add_relation_unit(rel.id, "redis-k8s/1")
# Simulate an update to the application databag made by the leader unit
self.harness.update_relation_data(rel.id, "redis-k8s", {key: value, key:value}) The only significant difference is that instead of calling |
Just to reiterate on the current status of this issue:
|
Hi @zmraul. From what I see in that example, aliases would cover our needs but still wouldn't reflect the specific required relations in the metadata file. What I had in mind is supporting multiple relations for the same interface:
Note that in our scenario these redis relations correspond to completely different components |
Thanks for the details @arturo-seijas! I guess I confused the concept of alias for this specific scenario. For this specific scenario, you can use the new library that Raúl mentioned and use something like the code below in charm.py: self.broker = DatabaseRequires(
self, "redis-broker", database_name="0"
)
self.framework.observe(
self.broker.on.database_created, self._on_broker_database_created
)
self.cache = DatabaseRequires(
self, "redis-cache", database_name="0"
)
self.framework.observe(
self.cache.on.database_created, self._on_cache_database_created
) The database name field may not be so useful for a scenario with Redis, as it uses the predefined 0 to 15 indexes as the database identifiers. One detail is that the new relations using that library are not yet implemented in the Redis charm. |
Right now the redis client library does not support adding aliases to the relations and hence, multiple relations with the same consumer charm. The relation event name is hardcoded:
We request this change because we've identified use cases in which two different redis clusters need to be consumed.
The text was updated successfully, but these errors were encountered: