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

@@ -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_.

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.

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")