From 1e3095124e131f266f24720f4c6187f8e0345706 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Thu, 11 May 2017 12:26:05 -0400 Subject: [PATCH] Change assess_status() method to be deferred and run once. This change makes the assess_status() method to be deferred until the end of the hook execution. This is to ensure that the (expensive) assess_status() functionality is only run once, even though it may be called multiple times from reactive handlers that need to update the status on the charm. This uses the hookenv.atexit() function to queue an function that calls _assess_status() on the charm singleton after all the reactive handlers have run. If no handler calls the assess_status() method then the _assess_status() 'real' method won't be called for that hook invocation. Change-Id: I5d405446761a646585dfa1c446009e4374c01000 --- README.md | 14 +++++++++++--- charms_openstack/charm.py | 17 ++++++++++++++++- unit_tests/test_charms_openstack_charm.py | 18 +++++++++++++++--- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index bf76989..e7830d0 100644 --- a/README.md +++ b/README.md @@ -282,14 +282,22 @@ state, and override the `assess_status()` method to a NOP. The `assess_status()` method on `OpenStackCharm` provides a helper to enable the charm author to provide workload status. By default: + * The actual assessment of status is deferred until the all of the reactive + handlers have had a chance to execute (according to their conditions), just + before the charm hook exits. The real `assess_status()` method is actually + `_assess_status()` and the `assess_status()` method simply sets up an + `atexit()` hook to defer the operation. This means that you can call + `assess_status()` multiple times BUT it will actually only be invoked at the + end of the charm hook execution. If you _need_ to actually run + assess_status() at the point in the handler, then call `_assess_status()`. * The install method provides the maintenance status. * The `layer-openstack` layer provides a hook for `update-status` which calls the `assess_status()` function on the charm class. - * The `assess_status()` method uses various attributes of the class to provide + * The `_assess_status()` method uses various attributes of the class to provide a default mechanism for assessing the workload status of the charm/unit. The latter is extremely useful for determining the workload status. The -`assess_status()` method does the following checks: +`_assess_status()` method does the following checks: 1. The unit checks if it is paused. (Not yet available as a feature). 2. The unit checks the relations to see if they are connected and available. @@ -340,7 +348,7 @@ class. The return value from the method is: ### Not checking services are running -By default, the `assess_status()` method checks that the services declared in +By default, the `_assess_status()` method checks that the services declared in the class attribute `services` (list of strings) are checked to ensure that they are running. Additionally, the ports declared in the class attribute `api_ports` are also checked for being _listened on_. diff --git a/charms_openstack/charm.py b/charms_openstack/charm.py index 8509d98..1e0ce68 100644 --- a/charms_openstack/charm.py +++ b/charms_openstack/charm.py @@ -568,6 +568,7 @@ class OpenStackCharm(object): self.__adapters_instance = None self.__interfaces = interfaces or [] self.__options = None + self.__run_assess_status = False @property def adapters_instance(self): @@ -901,7 +902,7 @@ class OpenStackCharm(object): """ pass - def assess_status(self): + def _assess_status(self): """Assess the status of the unit and set the status and a useful message as appropriate. @@ -939,6 +940,20 @@ class OpenStackCharm(object): # No state was particularly set, so assume the unit is active hookenv.status_set('active', 'Unit is ready') + def assess_status(self): + """This is a deferring version of _assess_status that only runs during + exit. This method can be called multiple times, but it will ensure that + the _assess_status() is only called once at the end of the charm after + all handlers have completed. + """ + if not self.__run_assess_status: + self.__run_assess_status = True + + def atexit_assess_status(): + hookenv.log("Running _assess_status()", level=hookenv.DEBUG) + self._assess_status() + hookenv.atexit(atexit_assess_status) + def custom_assess_status_check(self): """Override this function in a derived class if there are any other status checks that need to be done that aren't about relations, etc. diff --git a/unit_tests/test_charms_openstack_charm.py b/unit_tests/test_charms_openstack_charm.py index 935f048..d7eef82 100644 --- a/unit_tests/test_charms_openstack_charm.py +++ b/unit_tests/test_charms_openstack_charm.py @@ -1442,6 +1442,18 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): for call in self.render.call_args_list: self.assertTrue(call[1]['context']) + def test_deferred_assess_status(self): + self.patch_object(chm.hookenv, 'atexit') + s = self.target.singleton + self.patch_target('_assess_status') + s.assess_status() + self._assess_status.assert_not_called() + self.atexit.assert_called_once_with(mock.ANY) + self.atexit.reset_mock() + s.assess_status() + self.atexit.assert_not_called() + self._assess_status.assert_not_called() + def test_assess_status_active(self): self.patch_object(chm.hookenv, 'status_set') self.patch_object(chm.hookenv, 'application_version_set') @@ -1451,7 +1463,7 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): self.patch_target('custom_assess_status_check', return_value=(None, None)) self.patch_target('check_services_running', return_value=(None, None)) - self.target.assess_status() + self.target._assess_status() self.status_set.assert_called_once_with('active', 'Unit is ready') self.application_version_set.assert_called_once_with(mock.ANY) # check all the check functions got called @@ -1466,7 +1478,7 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): # patch out _ows_check_if_paused self.patch_object(chm.os_utils, '_ows_check_if_paused', return_value=('paused', '123')) - self.target.assess_status() + self.target._assess_status() self.status_set.assert_called_once_with('paused', '123') self.application_version_set.assert_called_once_with(mock.ANY) self._ows_check_if_paused.assert_called_once_with( @@ -1525,7 +1537,7 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): self.assertEqual(self.target.check_interfaces(), ('blocked', "'rel1' incomplete, 'rel2' missing")) # check that the assess_status give the same result - self.target.assess_status() + self.target._assess_status() self.status_set.assert_called_once_with( 'blocked', "'rel1' incomplete, 'rel2' missing")