Merge ~tcuthbert/charm-grafana:master into charm-grafana:master

[Thomas Cuthbert]

Newer versions of Grafana require a unique uid for each datasource

As per, https://rt.admin.canonical.com/Ticket/Display.html?id=126670#txn-2984750, we need to ensure that each Grafana Data Source has a unique uid, per organisation.

The way I have implemented this is to just hash the datasource URL, which is unique, and converting it to a 32bit integer as SQLite3 overflows with Python's 64bit ints.

unit-grafana-2: 05:26:06 DEBUG unit.grafana/2.grafana-source-relation-changed OverflowError: Python int too large to convert to SQLite INTEGER

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

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

[Joel Sing (jsing)]

LGTM, but see comments inline.

review: Approve (+1)

[Adam Dyess (addyess)]

LGTM also

review: Approve

[Alvaro Uria (aluria)]

In general, change looks good. However, functional tests fail because bionic-snap and focal-snap gate tests specify "snap_channel: 6/stable". I think that line should be removed and use the default (7/stable).

Several comments:
* Change in Grafana v7 was added here [1]
* data_source table schemas for v6 and v7 show that v7 has "uid" (see [2])
* Default APT package is v7
* Default snap_channel is 7/stable

This change will break install_method=snap and snap_channel=6/edge or snap_channel=stable (which is latest/stable and currently points to 6/stable revision).

The path forward will be to:
* merge and release (once the func tests that use install_method=snap are fixed) in cs:~llama-charmers/grafana
* For stable/20.08 release, we will make latest/stable match 7/stable (otherwise, this fix would need to track which version of grafana is in use, and if it is v6, then skip adding "uid" in generate_query function)

1. https://github.com/grafana/grafana/pull/23585
2. https://pastebin.ubuntu.com/p/jGb72gDzzZ/

review: Needs Fixing

[Alvaro Uria (aluria)]

I've reviewed the change and it lgtm. However, we still need to support Grafana v6, which is available via snap. I've suggested a MP to merge against your repo [1]

OTOH, once it is merged (if you agree), I would recommend rebasing from master as I see conflicts with the main branch. I did so locally and also had to add an extra relation in base.yaml, and all tests passed (see [2]).

Let me know what you think. With these changes, I think it would be ready to merge.

Thank you.

1. https://code.launchpad.net/~aluria/charm-grafana/+git/grafana-charm-1/+merge/387944
2. https://pastebin.ubuntu.com/p/zRCPjGp7NH/

review: Needs Fixing

[Xav Paice (xavpaice)]

Known issue is causing test failure:

2020-07-28 17:42:36 [INFO] ======================================================================
2020-07-28 17:42:36 [INFO] FAIL: test_11_dashboard_upgrade (tests.test_grafana.CharmOperationTest)
2020-07-28 17:42:36 [INFO] Test upgraded dashboards are imported.
2020-07-28 17:42:36 [INFO] ----------------------------------------------------------------------
2020-07-28 17:42:36 [INFO] Traceback (most recent call last):
2020-07-28 17:42:36 [INFO] File "./tests/test_grafana.py", line 156, in test_11_dashboard_upgrade
2020-07-28 17:42:36 [INFO] self.assertTrue(chk(dashes))
2020-07-28 17:42:36 [INFO] AssertionError: False is not true
2020-07-28 17:42:36 [INFO] ----------------------------------------------------------------------

This issue is unrelated to this change, mentioning here just for completeness.

LGTM, +1

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 0eb6f0de35c7076946e0b82da8e20a292555bc8f

diff --git a/reactive/grafana.py b/reactive/grafana.py
index 4434b17..502d7c8 100644
--- a/reactive/grafana.py
+++ b/reactive/grafana.py
@@ -934,6 +934,18 @@ def generate_query(ds, is_default, id=None, org_id=1):
             dtime,
         ]
 
+        config = hookenv.config()
+        if config["install_method"] == "apt" or config[
+            "snap_channel"
+        ].startswith("7/"):
+            fields.append("uid")
+            # Grafana's SQLite3 database has a unique constraint on the org_id
+            # and uid field, so generate one based on a hash of the data source
+            # url which is unique.
+            # The 32 bit mask is necessary because Python's 64 bit ints are too
+            # large for SQLite3.
+            values.append(hash(ds["url"]) & 0xFFFFFFFF)
+
         # Used in connecting a mysql source
         if "database" in ds:
             hookenv.log("Adding fields for the database")