Rewrite the 'change-osd-weight' to use the op framework

This patchset changes a single action, 'change-osd-weight' so that
it's implemented with the operator framework.

Change-Id: Ia11885a2096b6e4b1ecda5caea38939e17098e1d

actions/__init__.py

@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd
+# Copyright 2022 Canonical Ltd
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.

actions/change-osd-weight

@@ -1 +0,0 @@
-change_osd_weight.py

src/charm.py

@@ -7,6 +7,8 @@ import ops_openstack.core
 import ceph_hooks as hooks
 import ceph_metrics
 
+import ops_actions
+
 
 class CephMonCharm(ops_openstack.core.OSBaseCharm):
 
@@ -66,12 +68,31 @@ class CephMonCharm(ops_openstack.core.OSBaseCharm):
     def on_nrpe_relation(self, event):
         hooks.upgrade_nrpe_config()
 
+    # Actions.
+
+    def _observe_action(self, on_action, callable):
+        def _make_method(fn):
+            return lambda _, event: fn(event)
+
+        method_name = 'on_' + str(on_action.event_kind)
+        method = _make_method(callable)
+        # In addition to being a method, the action callbacks _must_ have
+        # the same '__name__' as their attribute name (this is how lookups
+        # work in the operator framework world).
+        method.__name__ = method_name
+
+        inst = type(self)
+        setattr(inst, method_name, method)
+        self.framework.observe(on_action, getattr(self, method_name))
+
     def __init__(self, *args):
         super().__init__(*args)
         self._stored.is_started = True
         fw = self.framework
 
         self.metrics_endpoint = ceph_metrics.CephMetricsEndpointProvider(self)
+        self._observe_action(self.on.change_osd_weight_action,
+                             ops_actions.change_osd_weight.change_osd_weight)
 
         fw.observe(self.on.install, self.on_install)
         fw.observe(self.on.config_changed, self.on_config)

src/ops_actions/__init__.py

@@ -0,0 +1,15 @@
+# Copyright 2022 Canonical Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from . import change_osd_weight   # noqa: F401

src/ops_actions/change_osd_weight.py

@@ -16,25 +16,26 @@
 
 """Changes the crush weight of an OSD."""
 
-from charmhelpers.core.hookenv import function_fail, function_get, log
-from charms_ceph.utils import reweight_osd
+import charms_ceph.utils as ceph_utils
+import logging
 
 
-def crush_reweight(osd_num, new_weight):
+logger = logging.getLogger(__name__)
+
+
+def change_osd_weight(event) -> None:
     """Run reweight_osd to change OSD weight."""
+    osd_num = event.params.get("osd")
+    new_weight = event.params.get("weight")
     try:
-        result = reweight_osd(str(osd_num), str(new_weight))
+        result = ceph_utils.reweight_osd(str(osd_num), str(new_weight))
     except Exception as e:
-        log(e)
-        function_fail("Reweight failed due to exception")
+        logger.warn(e)
+        event.fail("Reweight failed due to exception")
         return
 
     if not result:
-        function_fail("Reweight failed to complete")
+        event.fail("Reweight failed to complete")
         return
 
-
-if __name__ == "__main__":
-    osd_num = function_get("osd")
-    new_weight = function_get("weight")
-    crush_reweight(osd_num, new_weight)
+    event.set_results({'message': 'success'})

tox.ini

@@ -86,7 +86,6 @@ basepython = python3
 deps = flake8==3.9.2
        charm-tools==2.8.4
 commands = flake8 {posargs} unit_tests tests actions files src
-           charm-proof
 
 [testenv:cover]
 # Technique based heavily upon

unit_tests/test_action_change_osd_weight.py

@@ -13,25 +13,29 @@
 
 """Tests for reweight_osd action."""
 
-from actions import change_osd_weight as action
 import unittest.mock as mock
-from test_utils import CharmTestCase
+from test_utils import CharmTestCase, MockActionEvent
+from ops.testing import Harness
+
+with mock.patch('charmhelpers.contrib.hardening.harden.harden') as mock_dec:
+    mock_dec.side_effect = (lambda *dargs, **dkwargs: lambda f:
+                            lambda *args, **kwargs: f(*args, **kwargs))
+    # src.charm imports ceph_hooks, so we need to workaround the inclusion
+    # of the 'harden' decorator.
+    from src.charm import CephMonCharm
 
 
 class ReweightTestCase(CharmTestCase):
     """Run tests for action."""
 
     def setUp(self):
-        """Init mocks for test cases."""
-        super(ReweightTestCase, self).setUp(
-            action, ["function_get", "function_fail"]
-        )
+        self.harness = Harness(CephMonCharm)
 
-    @mock.patch("actions.change_osd_weight.reweight_osd")
+    @mock.patch("ops_actions.change_osd_weight.ceph_utils.reweight_osd")
     def test_reweight_osd(self, _reweight_osd):
         """Test reweight_osd action has correct calls."""
         _reweight_osd.return_value = True
-        osd_num = 4
-        new_weight = 1.2
-        action.crush_reweight(osd_num, new_weight)
+        self.harness.begin()
+        self.harness.charm.on_change_osd_weight_action(
+            MockActionEvent({'osd': 4, 'weight': 1.2}))
         _reweight_osd.assert_has_calls([mock.call("4", "1.2")])

unit_tests/test_utils.py

@@ -17,7 +17,7 @@ import unittest
 import os
 import yaml
 
-from unittest.mock import patch
+from unittest.mock import patch, MagicMock
 
 
 def load_config():
@@ -157,3 +157,11 @@ class TestLeaderSettings(object):
         elif attr in self.settings:
             return self.settings[attr]
         return None
+
+
+class MockActionEvent:
+
+    def __init__(self, params={}):
+        self.params = params
+        self.fail = MagicMock()
+        self.set_results = MagicMock()