canonical / s3-integrator

An integrator charm for handling s3 credentials
https://charmhub.io/s3-integrator
Apache License 2.0
1 stars 1 forks source link

s3-integrator should also observe `relation-created` events #32

Open phvalguima opened 5 months ago

phvalguima commented 5 months ago

The following setup:

juju deploy app1
juju deploy app2
juju deploy s3-integrator --config .... # some config

# And relating
juju integrate app1 s3-integrator
juju integrate app2 s3-integrator

Should result in both app1 and app2 receiving the same data. However, what happens is that only the first really related app gets the data whereas the second gets an empty "data: {}" field.

Looking into the code, I think we are not watching for new apps being attached (i.e. the -created events). We should essentially call the _on_config_changed logic, but focusing only on the new app at every -created.

github-actions[bot] commented 5 months ago

https://warthogs.atlassian.net/browse/DPE-4212

taurus-forever commented 5 months ago

@phvalguima can you please post here the summary of our long talk in MM. Just for the history. Tnx!

phvalguima commented 5 months ago

To summarize our internal discussions here, most of the use cases do not need s3-integrator with more than one relation. That also contemplates the cases where we have multi-cluster deployments:

Now, there is one use-case raised by @deusebio which is not yet clear if it establishes the need for single s3-integrator / multiple relations:

use-case where a s3 bucket is shared by multiple apps. Spark write logs to an s3 buckets that needs to be read by spark history server. Therefore a s3 integrator (provider) is related to both spark-history-server (requirer) and configuration hub (requirer)

In case @deusebio's case is kept and we will use a single s3-integrator charm for that, then we need to:

  1. remove the limit from metadata.yaml, if it was added by then
  2. Review and merge #33

If this is not the case, then this bug can be closed.