Merge ~brettmilford/charm-grafana:lp-1893320 into charm-grafana:master

[Brett Milford]

Catch ConnectionError and update constraints

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

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

[Drew Freiberger (afreiberger)]

I commented on bug that I'm okay with the implications of the revert of 7540b, as the ds_name can't be changed in the UI and have any of our dashboards work properly, anyways.

I would like to know if this change survives an upgrade-charm well before approving.

[Brett Milford (brettmilford)]

Here is what I've tested.

I deployed the grafana portion of the LMA stack like so:

https://pastebin.canonical.com/p/b4TqNcF3nc/

Upgraded charm to the local repository built with the patch.

juju upgrade-charm grafana --path ./.build/grafana

And monitored for issues - none were seen.

Also tested upgrading the charm from an error state after changing prometheus listen port. --force-units is required so that the new charm code can be delivered and executed.

[Drew Freiberger (afreiberger)]

Thanks. As I was reviewing this again, I'd love to see a blocked status set in the exception handling. I realize there may not be one in the exception handling just below, but since you're returning without failing, the operator won't know that dashboards didn't update or that there's an error in the log to review.

[Paul Goins (vultaire)]

+1 from me; one more review from the BootStack team needed for merge.

I did leave some review comments; my final comment suggests adding a source code comment for clarification and noting reason for adding the test in this MR. If you can spare the time, please consider doing that.

review: Approve

[Paul Goins (vultaire)]

Thanks Brett!

[Drew Freiberger (afreiberger)]

LGTM +1

review: Approve

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

Change successfully merged at revision 7929c5b8ef30a3f4e61b5aa55d6600dca62110a0

diff --git a/src/reactive/grafana.py b/src/reactive/grafana.py
index d6718eb..04b7c27 100644
--- a/src/reactive/grafana.py
+++ b/src/reactive/grafana.py
@@ -701,7 +701,7 @@ def check_datasource(ds):
     rows = query.fetchall()
     ds_name = "{} - {}".format(ds["service_name"], ds["description"])
     for row in rows:
-        if row[1] == ds["type"] and row[3] == ds["url"]:
+        if row[1] == ds["type"] and row[2] == ds_name:
             hookenv.log("Datasource already exist, updating: {}".format(ds_name))
             stmt, values = generate_query(ds, row[4], id=row[0])
             print(stmt, values)
@@ -880,7 +880,16 @@ def generate_prometheus_dashboards(gf_adminpasswd, ds):
     config = hookenv.config()
 
     # https://prometheus.io/docs/prometheus/latest/querying/api/#querying-label-values
-    response = requests.get("{}/api/v1/label/__name__/values".format(ds["url"]))
+    try:
+        response = requests.get("{}/api/v1/label/__name__/values".format(ds["url"]))
+    except requests.exceptions.ConnectionError as e:
+        hookenv.log(
+            "Exception while trying to reach prometheus API: {}".format(e),
+            "ERROR",
+        )
+        hookenv.status_set("blocked", "Exception reaching prometheus API whilst updating dashboards")
+        return
+
     if response.status_code != 200:
         hookenv.log(
             "Could not reach prometheus API status code: "
diff --git a/src/tests/functional/tests/test_grafana.py b/src/tests/functional/tests/test_grafana.py
index 4fa664a..5832fe0 100644
--- a/src/tests/functional/tests/test_grafana.py
+++ b/src/tests/functional/tests/test_grafana.py
@@ -251,6 +251,17 @@ class CharmOperationTest(BaseGrafanaTest):
         self.assertTrue(port in crontab["Stdout"])
         self.verify_iterative_backups()
 
+    def test_15_grafana_datasource_updates(self):
+        """
+        Change the port on the associated datasource.
+        Ensures the model returns to idle after changing the prometheus port post deployment.
+        As per LP #1893320
+        """
+        datasource_application = "prometheus"
+        port_config = {"web-listen-port": "1234"}
+        model.set_application_config(datasource_application, port_config)
+        model.block_until_all_units_idle()
+
 
 class SnappedGrafanaTest(BaseGrafanaTest):
     """Verify Grafana installed as a snap."""