Check to see if it's not using the upstream snap (#93)

* block units if upstream snap is installed via resource
* not run config-changed when upstream resource is used
* added func test showing that units go into blocked state if upstream resource is added.
* used snap cli to query snaps installed on functional tests

Closes: #94

src/charm.py

@@ -23,7 +23,7 @@
     WaitingStatus,
 )
 
-from service import SNAP_NAME, get_installed_snap_service, snap_install_or_refresh
+from service import SNAP_NAME, UPSTREAM_SNAP, get_installed_snap_service, snap_install_or_refresh
 
 logger = logging.getLogger(__name__)
 
@@ -168,8 +168,11 @@ def _configure(self, _: ops.HookEvent) -> None:
         """
         self.install()
 
-        snap_service = get_installed_snap_service(SNAP_NAME)
+        upstream_snap = get_installed_snap_service(UPSTREAM_SNAP)
+        if upstream_snap.present:
+            return
 
+        snap_service = get_installed_snap_service(SNAP_NAME)
         # if cos is not related then we should block and not run anything
         if not self.model.relations.get("cos-agent"):
             snap_service.stop()
@@ -214,6 +217,17 @@ def _on_collect_unit_status(self, event: ops.CollectStatusEvent) -> None:
         if not self.model.relations.get("cos-agent"):
             event.add_status(BlockedStatus("Grafana Agent is not related"))
 
+        upstream_snap = get_installed_snap_service(UPSTREAM_SNAP)
+
+        # this is necessary when doing a charm upgrade coming from revision 27
+        if upstream_snap.present:
+            event.add_status(
+                BlockedStatus(
+                    "golang-openstack-exporter detected. Please see: "
+                    "https://charmhub.io/openstack-exporter#known-issues"
+                )
+            )
+
         snap_service = get_installed_snap_service(SNAP_NAME)
 
         if not snap_service.present:

src/service.py

@@ -9,6 +9,7 @@
 logger = getLogger(__name__)
 
 SNAP_NAME = "charmed-openstack-exporter"
+UPSTREAM_SNAP = "golang-openstack-exporter"
 
 
 class SnapService:
@@ -93,9 +94,9 @@ def remove_upstream_snap() -> None:
     Raises an exception on error.
     """
     try:
-        snap.remove(["golang-openstack-exporter"])
+        snap.remove([UPSTREAM_SNAP])
     except snap.SnapError as e:
-        logger.error("failed to remove golang-openstack-exporter snap: %s", str(e))
+        logger.error("failed to remove %s snap: %s", UPSTREAM_SNAP, str(e))
         raise e
 
 

tests/integration/tests/charm_tests/openstack_exporter.py

@@ -3,6 +3,7 @@
 #
 # See LICENSE file for licensing details.
 
+import json
 import logging
 import subprocess
 import tempfile
@@ -12,7 +13,7 @@
 import yaml
 from zaza import model
 
-from charm import CLOUD_NAME, OS_CLIENT_CONFIG, SNAP_NAME
+from charm import CLOUD_NAME, OS_CLIENT_CONFIG, SNAP_NAME, UPSTREAM_SNAP
 
 logger = logging.getLogger(__name__)
 
@@ -85,15 +86,15 @@ def test_configure_port(self):
 
     def test_configure_snap_channel(self):
         """Test changing the snap channel."""
-        snap_info_before = get_snap_exporter_info()
+        snap_info_before = get_snap_info(SNAP_NAME)
         self.assertEqual(snap_info_before["channel"], "latest/stable")
 
         # Change snap channel
         new_channel = "latest/edge"
         model.set_application_config(APP_NAME, {"snap_channel": new_channel})
         model.block_until_all_units_idle()
 
-        snap_info_after = get_snap_exporter_info()
+        snap_info_after = get_snap_info(SNAP_NAME)
         self.assertEqual(snap_info_after["channel"], new_channel)
 
         model.reset_application_config(APP_NAME, ["snap_channel"])
@@ -105,7 +106,7 @@ def test_snap_as_resource(self):
         by the user changing the snap channel charm configuration.
         """
         # snap from snapstore won't have "x" in the revision
-        snap_info_before = get_snap_exporter_info()
+        snap_info_before = get_snap_info(SNAP_NAME)
         self.assertNotIn("x", snap_info_before["revision"])
 
         # juju cannot access resources at /tmp, so we create a tmp folder at the current directory
@@ -119,15 +120,15 @@ def test_snap_as_resource(self):
             model.block_until_all_units_idle()
 
             # local installed snaps will have "x" in the revision
-            snap_info_resource = get_snap_exporter_info()
+            snap_info_resource = get_snap_info(SNAP_NAME)
             self.assertIn("x", snap_info_resource["revision"])
 
             # changing channel won't affect the snap installed as resource
             # Change snap channel
             new_channel = "latest/beta"
             model.set_application_config(APP_NAME, {"snap_channel": new_channel})
             model.block_until_all_units_idle()
-            channel_after = get_snap_exporter_info()["channel"]
+            channel_after = get_snap_info(SNAP_NAME)["channel"]
             self.assertNotEqual(channel_after, new_channel)
             self.assertEqual(channel_after, snap_info_resource["channel"])
 
@@ -138,7 +139,7 @@ def test_snap_as_resource(self):
             model.attach_resource(APP_NAME, "openstack-exporter", str(temp_file_path))
             model.block_until_all_units_idle()
 
-            snap_info_after = get_snap_exporter_info()
+            snap_info_after = get_snap_info(SNAP_NAME)
             self.assertNotIn("x", snap_info_after["revision"])
             self.assertEqual(snap_info_after["channel"], new_channel)
 
@@ -256,18 +257,46 @@ def test_openstack_exporter_snap_down(self):
         )
         self.assertEqual(self.leader_unit.workload_status_message, "")
 
+    def test_upstream_exporter_as_resource(self):
+        """Test using the upstream golang as resource will block the unit."""
+        # juju cannot access resources at /tmp, so we create a tmp folder at the current directory
+        with tempfile.TemporaryDirectory(dir="./") as tmpdir:
+            tmp_path = Path(tmpdir)
+            download_cmd = (
+                f"wget -q -P {tmpdir} https://github.com/canonical/openstack-exporter-operator/"
+                "releases/download/rev2/golang-openstack-exporter_amd64.snap"
+            )
+            subprocess.run(download_cmd.split(), check=False)
+
+            resource_file = tmp_path / "golang-openstack-exporter_amd64.snap"
+            model.attach_resource(APP_NAME, "openstack-exporter", str(resource_file))
+            model.block_until_unit_wl_status(
+                self.leader_unit_entity_id, "blocked", timeout=STATUS_TIMEOUT
+            )
+            snaps_installed = [snap["name"] for snap in get_snaps_installed()]
+            self.assertIn(UPSTREAM_SNAP, snaps_installed)
+            expected_msg = (
+                "golang-openstack-exporter detected. Please see: "
+                "https://charmhub.io/openstack-exporter#known-issues"
+            )
+            self.assertEqual(self.leader_unit.workload_status_message, expected_msg)
+
+            # passing an empty file will unblock the unit
+            temp_file_path = Path(tmpdir) / f"{SNAP_NAME}.snap"
+            temp_file_path.touch()
+            model.attach_resource(APP_NAME, "openstack-exporter", str(temp_file_path))
+            model.block_until_unit_wl_status(
+                self.leader_unit_entity_id, "active", timeout=STATUS_TIMEOUT
+            )
+
+
+def get_snaps_installed() -> dict[str, str]:
+    """Get the snaps installed in the leader unit."""
+    cmd = "curl -sS --unix-socket /run/snapd.socket http://localhost/v2/snaps"
+    return json.loads(model.run_on_leader(APP_NAME, cmd).get("Stdout", ""))["result"]
+
 
-def get_snap_exporter_info() -> dict[str, str]:
-    """Get the openstack exporter snap information.
-
-    The snap list expected format as input is:
-    Name                        Version            Rev    Tracking       Publisher   Notes
-    charmed-openstack-exporter  1.7.0-26-g3be9ddb  4      latest/stable  canonical✓  -
-    """
-    results = model.run_on_leader(APP_NAME, f"snap list {SNAP_NAME}").get("Stdout", "").strip()
-    snap_info = results.splitlines()[1].split()
-    return {
-        "version": snap_info[VERSION_COLUMN],
-        "revision": snap_info[REVISION_COLUMN],
-        "channel": snap_info[CHANNEL_COLUMN],
-    }
+def get_snap_info(snap_name: str) -> list[dict[str, str]]:
+    """Get the snap information."""
+    snaps = get_snaps_installed()
+    return [snap for snap in snaps if snap["name"] == snap_name][0]

tests/unit/test_charm.py

@@ -37,7 +37,11 @@ def teardown_method(self, _):
     def test_config_changed(self, config, mocker):
         mock_get_installed_snap_service = mocker.patch("charm.get_installed_snap_service")
         mock_snap_service = mocker.Mock(spec_set=SnapService)
-        mock_get_installed_snap_service.return_value = mock_snap_service
+        mock_upstream_service = mocker.Mock(spec_set=SnapService)
+        mock_upstream_service.present = False
+        mock_get_installed_snap_service.side_effect = (
+            lambda snap: mock_snap_service if snap == SNAP_NAME else mock_upstream_service
+        )
 
         mocker.patch("charm.OpenstackExporterOperatorCharm.install")
 
@@ -81,6 +85,16 @@ def test_config_changed_snap_channel(self, mock_install, _):
         self.harness.update_config({"snap_channel": "latest/edge"})
         mock_install.assert_called_once()
 
+    @mock.patch("charm.get_installed_snap_service")
+    @mock.patch("charm.OpenstackExporterOperatorCharm.install")
+    def test_config_changed_upstream_resource(self, mock_install, mocked_installed_snap):
+        """Test config change when using the upstream resource."""
+        mocked_upstream = mock.MagicMock()
+        mocked_upstream.present = True
+        mocked_installed_snap.return_value = mocked_upstream
+        self.harness.update_config({"snap_channel": "latest/edge"})
+        mock_install.assert_not_called()
+
     @mock.patch("charm.OpenstackExporterOperatorCharm.get_resource", return_value="")
     @mock.patch("charm.snap_install_or_refresh")
     def test_install_snap(self, mock_install, _):

tests/unit/test_service.py

@@ -53,7 +53,7 @@ def test_snap_install_or_refresh_exception_raises(mock_snap, mock_workaround):
 def test_remove_upstream_snap(mock_snap):
     """Test remove upstream snap function."""
     service.remove_upstream_snap()
-    mock_snap.remove.assert_called_with(["golang-openstack-exporter"])
+    mock_snap.remove.assert_called_with([service.UPSTREAM_SNAP])
 
 
 @mock.patch("service.snap.remove")