Optional parameter not saved in RelationMeta class

[hemanthnakkina]

Charm authors can mark some of the relations as optional using 'optional' attribute in relations section (provides/requires/peers) in metadata.yaml [1], however the parameter is not saved in RelationMeta class.

The operator charms can perform some actions like updating configuration files, db sync once all the relations are established. However there could be cases where some relations are not mandatory and can be marked as Optional in metadata.yaml. Since the information is not saved in RelationMeta class, currently it is mandatory to have all the relations established.

[1] https://juju.is/docs/sdk/metadata-reference

[rwcarlsen]

I wasn't around when this design decision was made. But if I had to hazard a guess, because charms already need to be hard-coded to understand required-ness of relations anyway it was deemed unnecessary. Charms will usually need to have relation-specific code that deals with how to treat each particular relation w.r.t optional/required. Flipping a relation from not optional to optional in metadata.yaml is not something that I envision can/should be done without various corresponding changes to the charm code. For example, if a relation is required, I'd expect various activities a charm does to wait until that relation is created. And if it is then changed to optional, that code probably needs to be changed to not necessarily wait. In summary, I think that optional/required annotations in metadata.yaml are more like hints to juju of how the charm code actually behaves - not the other way around.

If you could provide a more specific use case where the current behavior isn't working well - it is very possible I'm not being imaginative enough.

[benhoyt]

@jameinel We have all the other relation metadata fields exposed in RelationMeta (see here), so I don't see any harm in adding the optional field there as well for completeness. Do you?

[jameinel]

Juju itself doesn't do anything with the field other than curry it to various APIs. (It exists in the all watcher, and we track it through various api calls, but I couldn't find any logic that behaves differently based on whether it is set or not.) As such, it exists in metadata.yaml solely as an informative message (and potentially a charm could interpret it however it wants to.)

Is this the right place to express it. In theory, it might be useful for a human to see the value (and metadata.yaml is more visible to a human than something deep inside charm.py). But ultimately the charm.py is the code that needs to know what to do when a relation is or isn't established.

I'm tempted to say that Juju should be deprecating it in metadata.yaml if we are going to actually change behavior. (I don't think it shows up in juju status or juju info, etc.

If it is something that we want to expose to humans (via charmhub, or info, etc) then I agree that we shouldn't have 2 copies of it (something in metadata.yaml that is for humans, and a code path in charm.py that doesn't pay attention to that value.)

The MySQL charm does declare some requires as optional: https://github.com/canonical/mysql-operator/blob/e4532cf5a8957b94a808634c920f4d5689078f04/metadata.yaml#L47

However juju info doesn't say anything about that field:

$ juju info mysql
...
relations:
  provides:
    cos-agent: cos_agent
    database: mysql_client
    db-router: mysql-router
    mysql: mysql
    shared-db: mysql-shared
  requires:
    certificates: tls-certificates
    s3-parameters: s3

Nor do I see anything about optional on the web page: https://charmhub.io/mysql/integrations

I feel like it is something we should either commit to and actually expose, or deprecate.

I'm tempted to do the latter, because to Juju every relation is optional, and if we aren't surfacing it to humans, then having code paths in charm.py that may or may not be in sync with metadata.yaml is not helpful.

[benhoyt]

Thanks, @jameinel.

@hemanthnakkina Can you describe how you use the optional field in Openstack charms, and point to some examples? I'm tempted to just expose this, seeing it is in metadata.yaml already, but would like some more concrete info.

[hemanthnakkina]

It is common in openstack charms that all the relations are not pre-requisite to start workload service. And so the workload can be started even when the optional relations are not yet integrated and in some cases optional relations may not be required in certain deployments.

Taking charm-keystone-k8s as an example [1], keystone-k8s requires database and ingress-public integrations to be done to configure and run the workload keystone service. However there are 2 optional relations

1. ingress-internal integration is ONLY required when underlying networks exist to separate internal and public traffic.
2. amqp integration is required ONLY when telemetry is enabled in the deployment.

All the sunbeam charms uses ops_sunbeam library [2] where most of the logic is handled. The charm registers all the relation handlers at the start up with callback typically configure_charm() in most cases and on event trigger the callback is invoked. configure_charm wait for ALL mandatory relations to be connected and ready before writing the configuration on the workload container and starting the service. At this point of time, the workload status is changed to Active [3] without waiting for status of optional relations. At any point of time when optional relations are integrated, the workload configuration is updated and service gets restarted.

Currently the charm maintains the mandatory relations in a variable which will be used further to check if all mandatory relations are ready [4] [5] [6]. It will be great if charm can identify the optional and mandatory relations from Relation metadata fields.

[1] https://opendev.org/openstack/charm-keystone-k8s/src/branch/main/metadata.yaml#L33-L46 [2] https://opendev.org/openstack/charm-ops-sunbeam [3] https://opendev.org/openstack/charm-ops-sunbeam/src/branch/main/ops_sunbeam/charm.py#L480-L483 [4] https://opendev.org/openstack/charm-ops-sunbeam/src/branch/main/ops_sunbeam/charm.py#L339-L354 [5] https://opendev.org/openstack/charm-ops-sunbeam/src/branch/main/ops_sunbeam/charm.py#L232-L236 [6] https://opendev.org/openstack/charm-ops-sunbeam/src/branch/main/ops_sunbeam/charm.py#L144

[benhoyt]

Got it, thanks for the context and code links. I think the explicit list of mandatory relations is fine too, but seeing we already support the "optional: true" field in metadata.yaml, I think it's reasonable to expose this in ops.RelationMeta.

We could argue whether optional should have been there in the first place, but that discussion is water under the bridge at this point, and I think it doesn't make sense -- and is costly -- to deprecate it at this point.

So let's do this! I'll put a (fairly trivial) PR shortly.

[benhoyt]

This is done now and will be in the next version of ops (which we'll release toward the end of October).