Merge ~canonical-is-bootstack/charm-grafana:bug/1876327 into charm-grafana:master

Proposed by Alvaro Uria
Status: Merged
Merged at revision: 12c78db6b7baa4072cdf4f7d39737c29c0ef510f
Proposed branch: ~canonical-is-bootstack/charm-grafana:bug/1876327
Merge into: charm-grafana:master
Diff against target: 36 lines (+13/-2)
2 files modified
metadata.yaml (+1/-1)
reactive/grafana.py (+12/-1)
Reviewer Review Type Date Requested Status
Xav Paice (community) Approve
BootStack Reviewers Pending
Review via email: mp+383866@code.launchpad.net

Commit message

Fix grafana-dashboards deserialization

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Alvaro Uria (aluria) wrote :

Manually tested in the charmlab and works as expected. Functional tests can't be included because the prometheus-libvirt-exporter fix is not merged yet.

Revision history for this message
Xav Paice (xavpaice) wrote :

LGTM. Not sure I follow the comment on the prometheus-libvirt-exporter fix, but functional tests on master aren't working for me due to the bundle not being found (environment vars need fixing).

review: Approve
Revision history for this message
Alvaro Uria (aluria) wrote :

Sorry, by "Functional tests can't be included because the prometheus-libvirt-exporter fix is not merged yet" I meant that the MP lacks for a test case regarding this fix.

The reason is that the only project that supports the grafana-dashboard interface in the LMA stack (maintained by BootStack) is prometheus-libvirt-exporter. However that project has a MP pending to include a dashboard (the one that would be shared with Grafana when the "grafana-dashboard" interface is used).

Revision history for this message
Alvaro Uria (aluria) wrote :

Regarding func tests not working for you, in tests/functional/test_deploy.py, there is a snippet:
"""
CHARM_BUILD_DIR = os.getenv('CHARM_BUILD_DIR')
JUJU_REPOSITORY = os.getenv('JUJU_REPOSITORY')
if CHARM_BUILD_DIR is not None:
    GRAFANA_BUILD_DIR = os.path.join(CHARM_BUILD_DIR.rstrip('/'), "grafana")
elif JUJU_REPOSITORY is not None:
    GRAFANA_BUILD_DIR = os.path.join(JUJU_REPOSITORY.rstrip('/'), "builds", "grafana")
else:
    GRAFANA_BUILD_DIR = "/tmp/charm-builds/grafana"
BUNDLE_PATH = os.path.join(GRAFANA_BUILD_DIR, "..", "bundle.yaml")
OVERLAY_PATH = os.path.join(GRAFANA_BUILD_DIR, "..", "overlay.yaml")
"""

"charm build" from charmtools uses /tmp/charm-builds/grafana when no env var is found. Please paste here the failure or file a bug and I will look into it.

Thank you.

Revision history for this message
Xav Paice (xavpaice) wrote :

the tests not working for me was down to env vars- the change from JUJU_REPOSITORY to CHARM_BUILD_DIR - it might be good in a future change to tidy that and remove all references to the JUJU_REPOSITORY, but if there's no errors with those vars (like there was in my env) then it's fine for now.

Revision history for this message
Alvaro Uria (aluria) wrote :

Now, both prometheus-libvirt-exporter and telegraf support the grafana-dashboards interface. This fix has been tested together with those charms and dashboards are loaded as expected.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/metadata.yaml b/metadata.yaml
index 54efd4f..fac2022 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -5,8 +5,8 @@ description: |
5 Grafana is the leading graph and dashboard builder for visualizing5 Grafana is the leading graph and dashboard builder for visualizing
6 time series metrics.6 time series metrics.
7series:7series:
8 - bionic
9 - focal8 - focal
9 - bionic
10 - xenial10 - xenial
11tags:11tags:
12 - misc12 - misc
diff --git a/reactive/grafana.py b/reactive/grafana.py
index 303b10e..c324b41 100644
--- a/reactive/grafana.py
+++ b/reactive/grafana.py
@@ -888,5 +888,16 @@ def hpwgen(passwd, salt):
888 'endpoint.dashboards.requests')888 'endpoint.dashboards.requests')
889def import_dashboards(dashboards):889def import_dashboards(dashboards):
890 for request in dashboards.new_requests:890 for request in dashboards.new_requests:
891 success, reason = import_dashboard(request.dashboard, request.name)891 try:
892 dash = request.dashboard
893 if isinstance(request.dashboard, str):
894 dash = json.loads(request.dashboard)
895 success, reason = import_dashboard(dash, request.name)
896 except json.decoder.JSONDecodeError as error:
897 success, reason = False, str(error)
898
899 hookenv.log(
900 "import_dashboards via grafana-dashboards: {} [{}]".format(success, reason),
901 hookenv.DEBUG,
902 )
892 request.respond(success, reason)903 request.respond(success, reason)

Subscribers

People subscribed via source and target branches

to all changes: