Merge ~yoshikadokawa/charm-prometheus-openstack-exporter:lp1936375 into charm-prometheus-openstack-exporter:master

[Yoshi Kadokawa]

use snap layer's library to refresh channel

snap layer's install function also consider's if snap resources are attached, which is necessary in an offline environment.

LP: #1936375

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

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

[James Troup (elmo)]

I'm not going to lie, this feels like a bug in the snap layer? I'm OK with this as a workaround if it unblocks you, but is there a bug for the snap layer?

review: Needs Information

[Yoshi Kadokawa (yoshikadokawa)]

> I'm not going to lie, this feels like a bug in the snap layer? I'm OK with
> this as a workaround if it unblocks you, but is there a bug for the snap
> layer?

I believe you are saying that this is a charmhelpers bug since snap_refresh() function comes from charmhelpers library. Therefore, my MR is replacing the charmhelpers function with the one from snap-layer.
And indeed, I agree that this is a charmhelpers bug, I have just raised a bug for this one.
https://bugs.launchpad.net/charm-helpers/+bug/1941711

However, since most of the recent charms are created with reactive framework, and this prometheus-openstack-exporter is a reactive charm as well, so using snap-layer would be better I believe.

[Paul Goins (vultaire)]

This is a +1 from me.

Indeed, the charmhelpers.fetch.snap module appears to really be a somewhat thin shim around the snap CLI; I can't find any references to resources in that module.

The snap layer's install function is more appropriate here.

review: Approve

[Xav Paice (xavpaice)]

LGTM

review: Approve

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

Change successfully merged at revision 2495a1833a10aadb0eb91128140d8d2216c9bc73

diff --git a/src/reactive/openstack_exporter.py b/src/reactive/openstack_exporter.py
index 9d8cdb3..6f330ec 100644
--- a/src/reactive/openstack_exporter.py
+++ b/src/reactive/openstack_exporter.py
@@ -22,7 +22,6 @@ import os
 from charmhelpers.contrib.charmsupport import nrpe
 from charmhelpers.core import hookenv, host, unitdata
 from charmhelpers.core.templating import render
-from charmhelpers.fetch.snap import snap_refresh
 
 from charms.layer import snap
 from charms.reactive import (
@@ -89,7 +88,7 @@ def snap_channel_changed():
     hookenv.status_set("maintenance", "Snap channel changed")
     config = hookenv.config()
     channel = config.get("snap_channel", "stable")
-    snap_refresh(SNAP_NAME, "--channel", channel)
+    snap.install(SNAP_NAME, channel=channel, force_dangerous=False)
     hookenv.status_set("active", "Snap refreshed")
 
 
diff --git a/src/tests/unit/test_reactive_openstack_exporter.py b/src/tests/unit/test_reactive_openstack_exporter.py
index 9f406e7..520a32d 100644
--- a/src/tests/unit/test_reactive_openstack_exporter.py
+++ b/src/tests/unit/test_reactive_openstack_exporter.py
@@ -93,8 +93,8 @@ class TestOpenstackExporterContext(unittest.TestCase):
         return openstack_exporter.config_paths(key)["target"]
 
     def test_basic_config(self, _mock_hookenv_config, *args):
-        exp_cert = "fortytwo"
-        exp_cert64 = base64.encodestring(exp_cert.encode("utf-8")).decode()
+        exp_cert = b"fortytwo"
+        exp_cert64 = base64.encodebytes(exp_cert).decode()
         config = self.def_config
         config.update(
             {
@@ -140,7 +140,7 @@ class TestOpenstackExporterContext(unittest.TestCase):
         act_keys = act_creds.keys()
         self.assertSetEqual(set(exp_keys), set(act_keys))
         cacert_file = self._config_file(openstack_exporter.CACERT)
-        self.assertEqual(exp_cert, open(cacert_file).read())
+        self.assertEqual(exp_cert.decode(), open(cacert_file).read())
 
     @mock.patch("reactive.openstack_exporter.host.service_running", return_value=True)
     @mock.patch("reactive.openstack_exporter.host.service_restart")