Using app relation data instead of unit relation data

[xtrusia]

As LP:1899706, solution for the issue is using application relation data instead of unit relation data.

This fix needs latest charms.reactive and charmhelpers.

This fix abandons request-reponse pattern which looks like use only for this.
johnca mentioned this in [2]

I know this code may not good enough.

Any advices are welcome.

Thanks.

[1] https://bugs.launchpad.net/charm-kubernetes-master/+bug/1899706
[2] juju-solutions/charms.reactive@92a8ee4

[jameinel]

Mostly what I have are high level comments. Hopefully this helps to think about how we want to do changes like this. I'm not wedded to any of the details, but I would like to encourage discussion.

• I'm a little surprised changes like this don't come alongside any sort of test changes, but if there hasn't been any infrastructure originally written to have tests, that probably doesn't mean you need to add it now

• I am quite concerned about compatibility, especially wrt heterogenous charms. (Having to have everyone upgrade to the new version of the protocol or nothing works is very hard on the community of charms.)

• I don't have a comment on the Requester pattern vs normal Endpoints. If you were guided in this direction, that seems fine.

• I feel like the original bug could be fixed in a couple of ways, and I wanted to make sure they were considered

  1. You could use a peer relation or a 'leader-set/leader-get' to coordinate the UUID. So both the old leader and the new leader would see that a UUID was already selected and just re-use it in the relation data. I'd recommend doing something like this anyway, as it is the safest way to provide a stable UUID over the long term.
  2. You could do this as application data, and then the leader could read if that relation already had a UUID, and if not set one that the next leader would preserve. Presumably the older Prometheus side would ignore it, but it would still let you have a stable UUID. (iow, check relation-get --app -r RELID uuid and use that to relation-set -r RELID uuid=X.) That has the nice property of being compatible.
  3. I think having a way of migrating to a new system where you prefer application data, and if application data is set, you could ignore unit data, but in the short term you would provide both sets of data would be the best for compatibility.

[jameinel]

My concern with this change is as follows:

1. Changing prometheus requires to only pay attention to app values means that it breaks compatibility with any charm that hasn't been updated yet.
2. Setting the request Job's request ID to a uuid4() at this point means that every time this gets reevaluated then all jobs get new random UUIDs (if I'm reading it correctly).
   In which case, why would we want the uuid at all?

[jameinel]

I don't know the details of what the content needs to be, so I haven't looked over this for correctness at all.

[xtrusia]

I tried to match the same format as previous(when using request-response pattern) one. so just copied this function from that pattern.

[xtrusia]

Mostly what I have are high level comments. Hopefully this helps to think about how we want to do changes like this. I'm not wedded to any of the details, but I would like to encourage discussion.

• I'm a little surprised changes like this don't come alongside any sort of test changes, but if there hasn't been any infrastructure originally written to have tests, that probably doesn't mean you need to add it now

• I am quite concerned about compatibility, especially wrt heterogenous charms. (Having to have everyone upgrade to the new version of the protocol or nothing works is very hard on the community of charms.)

• I don't have a comment on the Requester pattern vs normal Endpoints. If you were guided in this direction, that seems fine.

• I feel like the original bug could be fixed in a couple of ways, and I wanted to make sure they were considered

  1. You could use a peer relation or a 'leader-set/leader-get' to coordinate the UUID. So both the old leader and the new leader would see that a UUID was already selected and just re-use it in the relation data. I'd recommend doing something like this anyway, as it is the safest way to provide a stable UUID over the long term.
  2. You could do this as application data, and then the leader could read if that relation already had a UUID, and if not set one that the next leader would preserve. Presumably the older Prometheus side would ignore it, but it would still let you have a stable UUID. (iow, check relation-get --app -r RELID uuid and use that to relation-set -r RELID uuid=X.) That has the nice property of being compatible.
  3. I think having a way of migrating to a new system where you prefer application data, and if application data is set, you could ignore unit data, but in the short term you would provide both sets of data would be the best for compatibility.

Yes Cory guided me to remove request-response pattern as there is no other charms uses this.
and about UUID, I tried this in the beginning with kubernetes-master charm, if the leader is dead there was no way to get info about it. and k-master charm should be changed if i select about uuid. so I discussed this with Cory and he mentioned application relation data would be helpful for this issue at the moment.

Thanks.

[aieri]

I would like to comment as a user of this interface through the prometheus2 charm. With the COS-Lite now being in GA, we are limiting as much as possible any work on the prometheus2 charm, which is effectively deprecated and will only continue to be used during the transition phase. This patch introduces a significant change in the way the relation would work, and I would therefore not be in favor of incorporating it in the prometheus2 charm at this stage. I believe focus should rather be given to enabling the move to the COS as soon as possible.