search

canonical / operator

Pure Python framework for writing Juju charms
Apache License 2.0
246 stars 119 forks source link

Test harness relations cannot be initialized with data #457

Closed stub42 closed 1 year ago

stub42 commented 3 years ago

A test relation is initialized using harness.add_relation_unit and harness.update_relation_data. A side effect of this needing two calls is the relation-joined handlers get called between the two calls, and will always find the relation data empty. In real Juju deployments, during relation-joined relation data will already be initialized if the remote end happened to run its relation-joined hooks first.

This won't usually be a problem, as charms needing to read relation data should really be doing it in relation-changed hooks. The exception are relation-joined hooks that want to access Juju maintained relation data such as egress-subnets or ingress-address.

Easiest fix is allowing the initial relation data to be passed into add_relation_unit()

jameinel commented 3 years ago

You can work around this by disabling hooks during add_relation_unit, correct? (And/or doing it before calling begin()) We had that model at one point, but it started having a lot of stuff get pushed into a single api, which potentially grew unwieldy. (especially you can imagine the juju model evolving so that there is more than just relation data, and you also have the per-unit data as well as the overall application data, etc.)

I'm tempted to put this as "if you want to simulate the remote unit coming up before this one, you either use disable_hooks or doing the updates before calling begin()".

On Mon, Nov 23, 2020 at 5:03 AM Stub notifications@github.com wrote:

A test relation is initialized using harness.add_relation_unit and harness.update_relation_data. A side effect of this needing two calls is the relation-joined handlers get called between the two calls, and will always find the relation data empty. In real Juju deployments, during relation-joined relation data will already be initialized if the remote end happened to run its relation-joined hooks first.

This won't usually be a problem, as charms needing to read relation data should really be doing it in relation-changed hooks. The exception are relation-joined hooks that want to access Juju maintained relation data such as egress-subnets or ingress-address.

Easiest fix is allowing the initial relation data to be passed into add_relation_unit()

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/457, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7JECWUYOYB65OQJ4NTSRIXPZANCNFSM4T7IM22Q .

stub42 commented 3 years ago

I believe disabling hooks during add_relation_unit will mean the relation-joined hook will never be called? You will only get the relation-changed hook, unless you emit the relation-joined event manually and there is no API to construct relation events.

benhoyt commented 1 year ago

I agree with John's comment:

I'm tempted to put this as "if you want to simulate the remote unit coming up before this one, you either use disable_hooks or doing the updates before calling begin()".

The API isn't exactly lovely right now with having the two calls, but we probably won't change this at this point.