OF should gracefully handle blank application names in relation-broken hooks

[pengale]

See the discussion here: https://bugs.launchpad.net/juju/+bug/1960934

In a relation-broken hook, Juju does not set JUJU_REMOTE_APP. This is Working as Intended for Juju, but it raises an exception in the framework, when iterating through relations.

We need to refactor so that we handle the empty value without throwing the exception.

[jameinel]

I could be convinced to set JUJU_REMOTE_APP but you'd still want to handle the case in OF for versions of Juju that don't have that set.

…

On Tue, Feb 15, 2022 at 3:36 PM Pen Gale ***@***.***> wrote: See the discussion here: https://bugs.launchpad.net/juju/+bug/1960934 In a relation-broken hook, Juju does not set JUJU_REMOTE_APP. This is Working as Intended for Juju, but it raises an exception in the framework, when iterating through relations. We need to refactor so that we handle the empty value without throwing the exception. — Reply to this email directly, view it on GitHub <#693>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABRQ7J3PYBB5AK7XMNUEMDU3K2MLANCNFSM5OPUSLHQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[claudiubelu]

I want to mention that I've encountered this type of errors before [1], and that they still happen for the nginx-ingress-integrator charm when a related application is removed (the charm iterates over its relations in order to establish what Kubernetes Services and Ingress Resources need to be created / removed).

[1] finos/legend-juju-gitlab-integrator#6

[rwcarlsen]

If someone is e.g. using the remote app name to access relation data (relation.data[relation.app].get(...)), what should happen? Should we just have relation.data[relation.app] return an empty dictionary and log an error? Or something else...

[rwcarlsen]

According to the traceback (why does python reverse the compound word convention from c/c++?) in the linked juju issue, and relation.data[relation.app].get(...) is failing in the .get(...) call. And the relation-get hook tool is failing with an empty app name. This means that relation.app is an existing instance with an empty app name. I've traced this issue to

operator/ops/model.py

Line 1642 in e3bc30b

return os.environ['JUJU_REMOTE_APP']

where we end up returning an empty string instead of None. The calling code for this function only checks for None (not empty string) as a special case. I would argue that this is a bug that we can fix easily. However, this doesn't really resolve the question I asked above - users will instead just get a keyerror for e.g. relation.data[relation.app] because now relation.app will be None. If the actual relation data still does exist and relation-get calls can succeed, how can ops get access to the (gone?) remote app's name?