Merge ~peter-sabaini/charm-grafana:test-fixes-race-cleanup into charm-grafana:master

[Peter Sabaini]

Functional test fixes

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

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

dd9e6e4... by Peter Sabaini on 2021-01-29

Don't hardcode default port

When resetting datasource config

c651bd8... by Peter Sabaini on 2021-01-29

Fix port param

[Drew Freiberger (afreiberger)]

The focal-snap bundle failed on the upgrade tests oddly:

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

bionic-snap had no such trouble, and the rest of tests run.

2021-01-29 21:12:10 [INFO] ======================================================================
2021-01-29 21:12:10 [INFO] FAIL: test_50_snap_upgrade (tests.test_grafana.SnappedGrafanaTest)
2021-01-29 21:12:10 [INFO] Test a snap upgrade from 6 to 7.
2021-01-29 21:12:10 [INFO] ----------------------------------------------------------------------
2021-01-29 21:12:10 [INFO] Traceback (most recent call last):
2021-01-29 21:12:10 [INFO] File "./tests/test_grafana.py", line 310, in test_50_snap_upgrade
2021-01-29 21:12:10 [INFO] "PACKAGE UPGRADE REQUIRES MANUAL INTERVENTION" in status_message
2021-01-29 21:12:10 [INFO] AssertionError: False is not true
2021-01-29 21:12:10 [INFO] ======================================================================
2021-01-29 21:12:10 [INFO] FAIL: test_51_snap_downgrade_block (tests.test_grafana.SnappedGrafanaTest)
2021-01-29 21:12:10 [INFO] Test that after the previous upgrade, downgrades are blocked.
2021-01-29 21:12:10 [INFO] ----------------------------------------------------------------------
2021-01-29 21:12:10 [INFO] Traceback (most recent call last):
2021-01-29 21:12:10 [INFO] File "./tests/test_grafana.py", line 324, in test_51_snap_downgrade_block
2021-01-29 21:12:10 [INFO] self.assertTrue("PACKAGE DOWNGRADES ARE NOT SUPPORTED" in status_message)
2021-01-29 21:12:10 [INFO] AssertionError: False is not true
2021-01-29 21:12:10 [INFO] ----------------------------------------------------------------------

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:c651bd884a0c37a975d8149d80ffd4a4839fe26e
https://jenkins.canonical.com/bootstack/job/lp-charm-grafana-ci/1/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/2/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/62/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-grafana-ci/1//rebuild

review: Needs Fixing (continuous-integration)

[Alvaro Uria (aluria)]

this is not actually the fault of this MP, but a URL has a wrong format [1], which makes unit tests fail.

The other error is this one (test_get_installed_package_version):
"""
subprocess.CalledProcessError: Command 'apt list --installed | grep python3-pip' returned non-zero exit status 1.
"""

1. https://git.launchpad.net/~peter-sabaini/charm-grafana/tree/src/tests/unit/test_grafana.py?h=test-fixes-race-cleanup#n46

review: Needs Fixing

[Drew Freiberger (afreiberger)]

Regarding the python3-pip apt list command:

https://git.launchpad.net/~peter-sabaini/charm-grafana/tree/src/lib/charms/layer/grafana.py?h=test-fixes-race-cleanup#n181

Update this to subprocess.getoutput() instead of check_output to toss out the exception handling if it's uninstalled. Or add a try: except: return False kind of thing to handle these package install exceptions.

Unmerged commits

c651bd8... by Peter Sabaini on 2021-01-29

Fix port param

dd9e6e4... by Peter Sabaini on 2021-01-29

Don't hardcode default port

When resetting datasource config

bb58345... by Peter Sabaini on 2021-01-29

Functional test fixes

Fix races on file contents checking. Add cleanup to restore SUT after
config changes

diff --git a/src/tests/functional/tests/test_grafana.py b/src/tests/functional/tests/test_grafana.py
index d0e54d4..5709ca5 100644
--- a/src/tests/functional/tests/test_grafana.py
+++ b/src/tests/functional/tests/test_grafana.py
@@ -1,4 +1,5 @@
 """Encapsulate prometheus-openstack-exporter testing."""
+import asyncio
 import json
 import logging
 import time
@@ -103,6 +104,21 @@ class BaseGrafanaTest(unittest.TestCase):
         number_of_backups_end = backups_end["Stdout"].count(".json")
         self.assertTrue(number_of_backups_end > number_of_backups_start)
 
+    def file_pattern_test(self, app, pattern, file, message):
+        """Test if remote file matches a pattern.
+
+        Retry pattern matching for 30s before giving up and failing test with message
+        """
+        try:
+            model.block_until_file_matches_re(
+                app,
+                file,
+                pattern,
+                timeout=30,
+            )
+        except asyncio.exceptions.TimeoutError:
+            self.fail(message)
+
 
 class CharmOperationTest(BaseGrafanaTest):
     """Verify operations."""
@@ -228,12 +244,15 @@ class CharmOperationTest(BaseGrafanaTest):
             {"dashboards_backup_schedule": "* * * * *"},
         )
         model.block_until_all_units_idle()
-        crontab = model.run_on_unit(
-            self.lead_unit_name,
-            "cat /etc/cron.d/juju-dashboards-backup",
+        expect_pat = "dashboards_backup -d {} -p {}".format(
+            DEFAULT_BACKUP_DIRECTORY, DEFAULT_API_PORT
+        )
+        self.file_pattern_test(
+            self.application_name,
+            expect_pat,
+            "/etc/cron.d/juju-dashboards-backup",
+            "Cronjob doesn't match pattern {}".format(expect_pat),
         )
-        self.assertTrue(DEFAULT_BACKUP_DIRECTORY in crontab["Stdout"])
-        self.assertTrue(DEFAULT_API_PORT in crontab["Stdout"])
         self.verify_iterative_backups()
 
     def test_14_grafana_dashboard_backup_port_change(self):
@@ -245,11 +264,19 @@ class CharmOperationTest(BaseGrafanaTest):
         model.block_until_all_units_idle()
         model.set_application_config(self.application_name, {"port": port})
         model.block_until_all_units_idle()
-        crontab = model.run_on_unit(
-            self.lead_unit_name, "cat /etc/cron.d/juju-dashboards-backup"
+        expect_pat = "dashboards_backup -d {} -p {}".format(
+            DEFAULT_BACKUP_DIRECTORY, port
+        )
+        self.file_pattern_test(
+            self.application_name,
+            expect_pat,
+            "/etc/cron.d/juju-dashboards-backup",
+            "Cronjob doesn't match pattern {}".format(expect_pat),
         )
-        self.assertTrue(port in crontab["Stdout"])
         self.verify_iterative_backups()
+        # Cleanup: restore orig. port to make test re-runnable
+        model.set_application_config(self.application_name, {"port": DEFAULT_API_PORT})
+        model.block_until_all_units_idle()
 
     def test_15_grafana_datasource_updates(self):
         """Change the port on the associated datasource.
@@ -263,6 +290,9 @@ class CharmOperationTest(BaseGrafanaTest):
         port_config = {"web-listen-port": "1234"}
         model.set_application_config(datasource_application, port_config)
         model.block_until_all_units_idle()
+        # Cleanup: reset to default port
+        model.reset_application_config(datasource_application, ["web-listen-port"])
+        model.block_until_all_units_idle()
 
 
 class SnappedGrafanaTest(BaseGrafanaTest):
@@ -301,6 +331,7 @@ class SnappedGrafanaTest(BaseGrafanaTest):
         model.set_application_config(  # set it back to 6
             self.application_name, {"snap_channel": "6/stable"}
         )
+        model.block_until_unit_wl_status(self.lead_unit_name, "active")
         model.block_until_all_units_idle()
 
     def test_52_latest(self):