From 5656db92df3cbfb970636908ed19b6ef26b79882 Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Tue, 23 Aug 2022 11:23:34 -0300 Subject: [PATCH] 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 | 2 +- actions/change-osd-weight | 1 - src/charm.py | 21 ++++++++++++++++ src/ops_actions/__init__.py | 15 +++++++++++ .../ops_actions}/change_osd_weight.py | 25 ++++++++++--------- tox.ini | 1 - unit_tests/test_action_change_osd_weight.py | 24 ++++++++++-------- unit_tests/test_utils.py | 10 +++++++- 8 files changed, 73 insertions(+), 26 deletions(-) delete mode 120000 actions/change-osd-weight create mode 100644 src/ops_actions/__init__.py rename {actions => src/ops_actions}/change_osd_weight.py (61%) mode change 100755 => 100644 diff --git a/actions/__init__.py b/actions/__init__.py index 9b088de8..26092e0f 100644 --- a/actions/__init__.py +++ b/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. diff --git a/actions/change-osd-weight b/actions/change-osd-weight deleted file mode 120000 index 07705325..00000000 --- a/actions/change-osd-weight +++ /dev/null @@ -1 +0,0 @@ -change_osd_weight.py \ No newline at end of file diff --git a/src/charm.py b/src/charm.py index 6a3cf8ac..18d99131 100755 --- a/src/charm.py +++ b/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) diff --git a/src/ops_actions/__init__.py b/src/ops_actions/__init__.py new file mode 100644 index 00000000..b8d2de33 --- /dev/null +++ b/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 diff --git a/actions/change_osd_weight.py b/src/ops_actions/change_osd_weight.py old mode 100755 new mode 100644 similarity index 61% rename from actions/change_osd_weight.py rename to src/ops_actions/change_osd_weight.py index 1732f010..cc12cf94 --- a/actions/change_osd_weight.py +++ b/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'}) diff --git a/tox.ini b/tox.ini index 597f5d1e..b22b7bb2 100644 --- a/tox.ini +++ b/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 diff --git a/unit_tests/test_action_change_osd_weight.py b/unit_tests/test_action_change_osd_weight.py index e0bff653..49db6796 100644 --- a/unit_tests/test_action_change_osd_weight.py +++ b/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")]) diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py index ce139de5..538aec0d 100644 --- a/unit_tests/test_utils.py +++ b/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()