Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cross-model dashboards #29

Merged
merged 19 commits into from
Oct 7, 2021
Merged

Fix cross-model dashboards #29

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d29d1d3
Fix cross-model dashboards
rbarry82 Sep 22, 2021
6a19b3b
Use JInja <3.x, import Jinja at runtime so clients don't need it
rbarry82 Sep 24, 2021
8f18001
Make linters happy
rbarry82 Sep 24, 2021
08090ea
The consumer should work with multiple prometheuses
rbarry82 Sep 24, 2021
f936d31
Add a test for multiple monitoring targets
rbarry82 Sep 24, 2021
f8adda3
Make mypy quiet about jinja
rbarry82 Sep 24, 2021
d635d23
Make mypy quiet about jinja
rbarry82 Sep 24, 2021
08088d8
Magic dashboard discovery
rbarry82 Sep 24, 2021
b6f824d
Throw an exception and return if there are no prometheus relations
rbarry82 Sep 28, 2021
4ae2328
Handle upgrades
rbarry82 Sep 28, 2021
342461a
Fix charm upgrades
rbarry82 Sep 29, 2021
b8d6839
Final updates
rbarry82 Sep 29, 2021
1ebec13
Don't invalidate dashboards without monitoring
rbarry82 Sep 29, 2021
ddb6a98
Use the right lib version
rbarry82 Sep 29, 2021
719c06f
Fix stupid conversion error
rbarry82 Sep 30, 2021
f952deb
Merge branch 'main' into fix/cross-model-relations
rbarry82 Oct 5, 2021
a93b4e9
Merge branch 'fix/cross-model-relations' of github.com:rbarry82/grafa…
rbarry82 Oct 5, 2021
ca45110
Update tests
rbarry82 Oct 5, 2021
841b5f6
Make mypy quiet
rbarry82 Oct 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 27 additions & 15 deletions lib/charms/grafana_k8s/v0/grafana_dashboard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from jinja2.exceptions import TemplateSyntaxError
from ops.charm import CharmBase, RelationBrokenEvent, RelationChangedEvent
from ops.framework import EventBase, EventSource, Object, ObjectEvents, StoredState
from ops.model import Relation, Unit
from ops.model import Relation

# The unique Charmhub library identifier, never change it
LIBID = "c49eb9c7dfef40c7b6235ebd67010a3f"
Expand Down Expand Up @@ -149,18 +149,23 @@ def add_dashboard(self, data: str, rel_id=None) -> None:
self._stored.dashboard_templates[rel_id] = base64.b64encode(
zlib.compress(data.encode(), 9)
).decode()

# After moving to the assumption that model_identifier will be set from
# the other side of the relation for Prometheus, this is here only to
# ensure that there is actually a monitoring relationship before we
# send dashboard data
prom_rel = self.framework.model.get_relation(self._stored.event_relation)

try:
prom_unit = prom_rel.units.pop()
prom_rel.units.pop()
except (IndexError, AttributeError):
error_message = ("Waiting for a {} relation to send dashboard data").format(
self._stored.event_relation
)
self.on.dashboard_status_changed.emit(error_message=error_message, valid=False)
return

self._update_dashboards(data, rel_id, prom_unit)
self._update_dashboards(data, rel_id)

def _on_grafana_dashboard_relation_changed(self, event: RelationChangedEvent) -> None:
"""Watch for changes so we know if there's an error to signal back to the parent charm.
Expand All @@ -171,7 +176,8 @@ def _on_grafana_dashboard_relation_changed(self, event: RelationChangedEvent) ->
if not self.charm.unit.is_leader():
return

data = json.loads(event.relation.data[event.app].get("event", "{}"))
rel = self.framework.model.get_relation(self.name, event.relation.id)
data = json.loads(rel.data[event.app].get("event", "{}"))

if not data:
return
Expand All @@ -194,17 +200,11 @@ def _on_grafana_dashboard_relation_changed(self, event: RelationChangedEvent) ->
)
)

def _update_dashboards(self, data: str, rel_id: int, prom_unit: Unit) -> None:
def _update_dashboards(self, data: str, rel_id: int) -> None:
"""Update the dashboards in the relation data bucket."""
if not self.charm.unit.is_leader():
return

prom_identifier = "{}_{}_{}".format(
prom_unit._backend.model_name,
prom_unit._backend.model_uuid,
prom_unit.app.name,
)

prom_target = "{} [ {} / {} ]".format(
self.charm.app.name.capitalize(),
self.charm.model.name,
Expand All @@ -218,7 +218,6 @@ def _update_dashboards(self, data: str, rel_id: int, prom_unit: Unit) -> None:
# It's completely ridiculous to add a UUID, but if we don't have some
# pseudo-random value, this never makes it across 'juju set-state'
stored_data = {
"monitoring_identifier": prom_identifier,
"monitoring_target": prom_target,
"monitoring_query": prom_query,
"template": base64.b64encode(zlib.compress(data.encode(), 9)).decode(),
Expand Down Expand Up @@ -325,6 +324,7 @@ def __init__(self, charm: CharmBase, name: str) -> None:
super().__init__(charm, name)
self.charm = charm
self.name = name
self.source_relation = "grafana-source"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is invariant - consider moving to class variable.

events = self.charm.on[name]

self._stored.set_default(
Expand All @@ -351,17 +351,29 @@ def _on_grafana_dashboard_relation_changed(self, event: RelationChangedEvent) ->
if not self.charm.unit.is_leader():
return

rel = event.relation
rel = self.framework.model.get_relation(self.name, event.relation.id)

data = (
json.loads(event.relation.data[event.app].get("dashboards", {}))
if event.relation.data[event.app].get("dashboards", {})
json.loads(rel.data[event.app].get("dashboards", {}))
if rel.data[event.app].get("dashboards", {})
else None
)
if not data:
logger.info("No dashboard data found in relation")
return

# Figure out our Prometheus relation and template the query

prom_rel = self.framework.model.get_relation(self.source_relation)
prom_unit = next(iter(prom_rel.units))
prom_identifier = "{}_{}_{}".format(
self.charm.model.name,
self.charm.model.uuid,
prom_unit.app.name,
)

data["monitoring_identifier"] = prom_identifier

# Get rid of this now that we passed through to the other side
data.pop("uuid", None)

Expand Down
5 changes: 0 additions & 5 deletions tests/test_dashboard_consumer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ def test_consumer_sets_dashboard_data(self):
self.harness.get_relation_data(rel_id, self.harness.model.app.name)["dashboards"]
)
return_data = {
"monitoring_identifier": "testing_abcdefgh-1234_monitoring",
"monitoring_target": "Consumer-tester [ testing / abcdefgh-1234 ]",
"monitoring_query": "juju_model='testing',juju_model_uuid='abcdefgh-1234',juju_application='consumer-tester'",
"template": "\n\n",
Expand All @@ -109,7 +108,6 @@ def test_consumer_can_remove_dashboard(self):
self.harness.get_relation_data(rel_id, self.harness.model.app.name)["dashboards"]
)
return_data = {
"monitoring_identifier": "testing_abcdefgh-1234_monitoring",
"monitoring_target": "Consumer-tester [ testing / abcdefgh-1234 ]",
"monitoring_query": "juju_model='testing',juju_model_uuid='abcdefgh-1234',juju_application='consumer-tester'",
"template": "\n\n",
Expand All @@ -121,7 +119,6 @@ def test_consumer_can_remove_dashboard(self):
self.assertEqual(return_data, data)
self.harness.charm.consumer.remove_dashboard()
return_data = {
"monitoring_identifier": "testing_abcdefgh-1234_monitoring",
"monitoring_target": "Consumer-tester [ testing / abcdefgh-1234 ]",
"monitoring_query": "juju_model='testing',juju_model_uuid='abcdefgh-1234',juju_application='consumer-tester'",
"template": "\n\n",
Expand All @@ -143,7 +140,6 @@ def test_consumer_resends_dashboard_after_monitoring_established(self):
self.harness.get_relation_data(rel_id, self.harness.model.app.name)["dashboards"]
)
return_data = {
"monitoring_identifier": "testing_abcdefgh-1234_monitoring",
"monitoring_target": "Consumer-tester [ testing / abcdefgh-1234 ]",
"monitoring_query": "juju_model='testing',juju_model_uuid='abcdefgh-1234',juju_application='consumer-tester'",
"template": "\n\n",
Expand All @@ -169,7 +165,6 @@ def test_consumer_invalidates_dashboard_after_monitoring_established_then_broken
self.harness.get_relation_data(rel_id, self.harness.model.app.name)["dashboards"]
)
return_data = {
"monitoring_identifier": "testing_abcdefgh-1234_monitoring",
"monitoring_target": "Consumer-tester [ testing / abcdefgh-1234 ]",
"monitoring_query": "juju_model='testing',juju_model_uuid='abcdefgh-1234',juju_application='consumer-tester'",
"template": "\n\n",
Expand Down
19 changes: 11 additions & 8 deletions tests/test_dashboard_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@
"""

DASHBOARD_RENDERED = """
"name": "testing_abcdefgh-1234_monitoring"
"name": "testing_abcdefgh-1234_source"
"label": "Consumer-tester [ testing / abcdefgh-1234 ]"
"query": "label_values(up{ juju_model='testing',juju_model_uuid='abcdefgh-1234',juju_application='consumer-tester' }, juju_unit)"
"""

MODEL_INFO = {"name": "testing", "uuid": "abcdefgh-1234"}

SOURCE_DATA = {
"monitoring_identifier": "testing_abcdefgh-1234_monitoring",
"monitoring_target": "Consumer-tester [ testing / abcdefgh-1234 ]",
"monitoring_query": "juju_model='testing',juju_model_uuid='abcdefgh-1234',juju_application='consumer-tester'",
"template": DASHBOARD_TMPL,
Expand All @@ -42,7 +43,6 @@
}

OTHER_SOURCE_DATA = {
"monitoring_identifier": "testing_abcdefgh-2345_monitoring",
"monitoring_target": "Consumer-tester [ testing / abcdefgh-2345 ]",
"monitoring_query": "juju_model='testing',juju_model_uuid='abcdefgh-2345',juju_application='consumer-tester'",
"template": DASHBOARD_TMPL,
Expand Down Expand Up @@ -79,6 +79,7 @@ def version(self):
class TestDashboardProvider(unittest.TestCase):
def setUp(self):
self.harness = Harness(ProviderCharm)
self.harness.set_model_info(name=MODEL_INFO["name"], uuid=MODEL_INFO["uuid"])
self.addCleanup(self.harness.cleanup)
self.harness.set_leader(True)
self.harness.begin()
Expand All @@ -91,15 +92,17 @@ def setup_charm_relations(self, multi=False):
created.
"""
self.harness.charm.grafana_provider._stored.active_sources = [
{"source-name": "testing_abcdefgh-1234_monitoring"},
{"source-name": "testing_abcdefgh-2345_monitoring"},
{"source-name": "testing_abcdefgh-1234_source"},
{"source-name": "testing_abcdefgh-2345_source"},
]

rel_ids = []
self.assertEqual(self.harness.charm._stored.dashboard_events, 0)
source_rel_id = self.harness.add_relation("grafana-source", "source")
self.harness.add_relation_unit(source_rel_id, "source/0")
rel_id = self.harness.add_relation("grafana-dashboard", "consumer")
rel_ids.append(rel_id)
self.harness.add_relation_unit(rel_id, "consumer/0")
rel_ids.append(rel_id)
self.harness.update_relation_data(
rel_id,
"consumer",
Expand Down Expand Up @@ -135,9 +138,9 @@ def test_provider_notifies_on_new_dashboards(self):
self.assertEqual(
stored,
{
"target": "testing_abcdefgh-1234_monitoring",
"target": "testing_abcdefgh-1234_source",
"data": {
"monitoring_identifier": "testing_abcdefgh-1234_monitoring",
"monitoring_identifier": "testing_abcdefgh-1234_source",
"monitoring_target": "Consumer-tester [ testing / abcdefgh-1234 ]",
"monitoring_query": "juju_model='testing',juju_model_uuid='abcdefgh-1234',juju_application='consumer-tester'",
"template": DASHBOARD_TMPL,
Expand Down