Losing k8s-master causes prometheus manual-jobs duplicate

Thanks for the detailed report and reproduction steps!

I think this will need to be fixed in the prometheus charm. The code that injects the UUID[1] is part of the to_json function that's invoked by prometheus[2]. This is supposed to "ensure uniqueness" i.e. prevent multiple requests from clashing. A side effect of this is that when different units request the same job, the prometheus charm will generate multiple copies of that job.

The request ID that's used is part of the request-response pattern from charms.reactive. It is stored in relation data, and relation data is unit-scoped. Updating that pattern to allow re-use of a request ID across units would be doable but challenging. I strongly believe that, instead, this is a case where the request ID is being misused by the receiving side. Perhaps the relation ID should be used instead, which seems to be unique per application?

I am adding prometheus to this issue, but also leaving it open against kubernetes-master, since I would like another engineer from our team to look at this.

[1]: https://github.com/juju-solutions/interface-prometheus-manual/blob/3f775242c16d53243c993d7ba0c896169ad1639e/common.py#L36
[2]: https://git.launchpad.net/charm-prometheus2/tree/src/reactive/prometheus.py#n748

When I looked at the code from a charm-prometheus2 support perspective, my take was that the kubernetes-master charm will set the prometheus manual jobs up on leader-elected[1], but, the demoted leader does not stop advertising it's jobs.

Because the charm-prometheus2 is architected to monitor each unit of a related application, this is why the tagging of the request UUID to ensure each unit that is announcing to be monitored is monitored individually. Imagine 100 units of telegraf with an additional manual-job metric for a per-unit counter.

In the instance of kubernetes-master charm providing details for monitoring a single pod (per manual-job) behind the 3 HA k8s-master, there is only need for one monitoring endpoint for kubernetes-master (announced to be monitored through the kubeapi-load-balancer, IIRC), but multiple units advertising to prometheus to be monitored, the data is duplicated.

I believe the solution is to add a function to remove the prometheus manual scrape job details from the relation @when_not('leadership.is_leader'). basically, the opposite of this function when leadership is lost:

[1]: https://github.com/charmed-kubernetes/charm-kubernetes-master/blob/master/reactive/kubernetes_master.py#L3128

I've marked this bug with wishlist against charm-prometheus2, as we could implement a scope variable in the relation that would allow for unit or app based monitors to be defined. This likely won't make it into the charm until the rewrites into the operator framework which is not yet on the roadmap.

Removing and re-adding the kubernetes-master/prometheus relationship cleared the issue for now.

how about this one? Please give me any advice

put timestamp from register_prometheus_jobs in kubernetes-master charm

https://pastebin.ubuntu.com/p/btZxVBHxn8/

and relation provider

https://pastebin.ubuntu.com/p/hD7Jh5x3qf/

then take that timestamp in prometheus charm like below, skip data with old timestamp, take newer one.

then we can take manual_jobs from later leader.

https://pastebin.ubuntu.com/p/Rthx7yWsSz/

I think it is not possible we can detect trigger when leader is down ( maybe im wrong )

A better solution for this would be to use application-level relation data, but that does require newer version of Juju to support, and currently doesn't have support available in charmhelpers or reactive.

Having the non-leader K8s unit clear its request from the relation data could still end up with duplicate data in the case of a stuck unit, but it would work in other cases without changes to the interface or promteheus2 charm.

Adding a timestamp would lead to complications when one side or the other is an older version of the charm which doesn't know about the timestamp, but could probably be handled gracefully enough to at least end up with the current behavior.

However, it looks like the interface could be changed to use the relation ID as the request ID instead of relying on the default UUID creation, as George suggested. That way, the existing de-duplication should work as-is and should even handle stuck non-leaders since the rest of the request body should be identical.

To add to my last comment, in theory changing the request ID to be the relation ID should be as simple as adding a `request_id=relation.relation_id` param to [1] and I *think* it might even handle upgrades fine, too, since it selects the request to update based on the `job_name` field rather than the request ID. However, I'm probably missing something in how that would actually be handled on one side or the other if the request ID for an existing request is changed, so that would definitely need some testing.

[1]: https://github.com/juju-solutions/interface-prometheus-manual/blob/master/provides.py#L37-L41

I've uploaded pull request

[1] https://github.com/juju/charm-helpers/pull/566
[2] https://github.com/juju-solutions/charms.reactive/pull/232
[3] https://github.com/juju-solutions/interface-prometheus-manual/pull/3

Please review if anyone is interested in this case

Note that pull/3 has been superseded by pull/6
https://github.com/juju-solutions/interface-prometheus-manual/pull/6