From 255888fef3ddd04dcb3db85feb6326fdf396b54f Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Fri, 30 Sep 2022 08:05:17 -0400 Subject: [PATCH] Rewrite get_health action with the Operator framework Change-Id: I68645a3d00c0622c7701c8177bcd510c3092afe4 --- actions/ceph_ops.py | 14 ------------ actions/get-health | 1 - src/charm.py | 2 ++ src/ops_actions/__init__.py | 1 + {actions => src/ops_actions}/get_health.py | 17 ++++++++------ unit_tests/test_actions_mon.py | 5 ----- unit_tests/test_ceph_actions.py | 26 ++++++++++++++++++++++ 7 files changed, 39 insertions(+), 27 deletions(-) delete mode 120000 actions/get-health rename {actions => src/ops_actions}/get_health.py (71%) diff --git a/actions/ceph_ops.py b/actions/ceph_ops.py index a71c6869..10cc8ba0 100755 --- a/actions/ceph_ops.py +++ b/actions/ceph_ops.py @@ -83,20 +83,6 @@ def get_versions_report(): return json.dumps(report, indent=4) -def get_health(): - """ - Returns the output of 'ceph health'. - - On error, 'unknown' is returned. - """ - try: - value = check_output(['ceph', 'health']).decode('UTF-8') - return value - except CalledProcessError as e: - action_fail(str(e)) - return 'Getting health failed, health unknown' - - def pool_get(): """ Returns a key from a pool using 'ceph osd pool get'. diff --git a/actions/get-health b/actions/get-health deleted file mode 120000 index 9c8a8000..00000000 --- a/actions/get-health +++ /dev/null @@ -1 +0,0 @@ -get_health.py \ No newline at end of file diff --git a/src/charm.py b/src/charm.py index c16cffaa..6a07a3b8 100755 --- a/src/charm.py +++ b/src/charm.py @@ -156,6 +156,8 @@ class CephMonCharm(ops_openstack.core.OSBaseCharm): self._observe_action( self.on.create_erasure_profile_action, ops_actions.create_erasure_profile.create_erasure_profile_action) + self._observe_action(self.on.get_health_action, + ops_actions.get_health.get_health_action) 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 index 8711e196..54aaec57 100644 --- a/src/ops_actions/__init__.py +++ b/src/ops_actions/__init__.py @@ -17,4 +17,5 @@ from . import ( # noqa: F401 copy_pool, create_crush_rule, create_erasure_profile, + get_health, ) diff --git a/actions/get_health.py b/src/ops_actions/get_health.py similarity index 71% rename from actions/get_health.py rename to src/ops_actions/get_health.py index d1e0da48..b148c954 100755 --- a/actions/get_health.py +++ b/src/ops_actions/get_health.py @@ -14,15 +14,18 @@ # See the License for the specific language governing permissions and # limitations under the License. -from subprocess import CalledProcessError +from subprocess import check_output, CalledProcessError +import logging -from ceph_ops import get_health -from charmhelpers.core.hookenv import log, action_set, action_fail -if __name__ == '__main__': +logger = logging.getLogger(__name__) + + +def get_health_action(event): try: - action_set({'message': get_health()}) + event.set_results( + {'message': check_output(['ceph', 'health']).decode('UTF-8')}) except CalledProcessError as e: - log(e) - action_fail( + logger.warning(e) + event.fail( "ceph health failed with message: {}".format(str(e))) diff --git a/unit_tests/test_actions_mon.py b/unit_tests/test_actions_mon.py index ff54db0f..a09a7b0a 100644 --- a/unit_tests/test_actions_mon.py +++ b/unit_tests/test_actions_mon.py @@ -49,11 +49,6 @@ class OpsTestCase(CharmTestCase): "action_fail", "open"]) - def test_get_health(self): - actions.get_health() - cmd = ['ceph', 'health'] - self.check_output.assert_called_once_with(cmd) - def test_get_version_report_ok(self): def _call_rslt(): with open('unit_tests/ceph_ls_node.json') as f: diff --git a/unit_tests/test_ceph_actions.py b/unit_tests/test_ceph_actions.py index 514b5dbb..21520390 100644 --- a/unit_tests/test_ceph_actions.py +++ b/unit_tests/test_ceph_actions.py @@ -230,3 +230,29 @@ class CreateErasureProfileTestCase(test_utils.CharmTestCase): coding_chunks=None, helper_chunks=2, scalar_mds='jerasure', failure_domain='disk', device_class=None ) + + +class GetHealthTestCase(test_utils.CharmTestCase): + """Run tests for action.""" + + def setUp(self): + self.harness = Harness(CephMonCharm) + self.harness.begin() + self.addCleanup(self.harness.cleanup) + + @mock.patch('ops_actions.get_health.check_output') + def test_get_health_action(self, mock_check_output): + mock_check_output.return_value = b'yay' + event = test_utils.MockActionEvent({}) + self.harness.charm.on_get_health_action(event) + event.set_results.assert_called_once_with(({'message': 'yay'})) + + @mock.patch('ops_actions.get_health.check_output') + def test_get_health_action_error(self, mock_check_output): + mock_check_output.side_effect = subprocess.CalledProcessError( + 1, 'test') + event = test_utils.MockActionEvent({}) + self.harness.charm.on_get_health_action(event) + event.fail.assert_called_once_with( + 'ceph health failed with message: ' + "Command 'test' returned non-zero exit status 1.")