Add pause/resume actions and sync charm-helpers

Adds pause and resume unit to the charm such that the
charm stays paused during maintenance operations.

Change-Id: Id545477313bde717e1c941f62e6348f3c0656ab3
Partial-Bug: 1558642

actions.yaml

@@ -1,3 +1,7 @@
+pause:
+  description: Pause the cinder unit.  This action will stop cinder services.
+resume:
+  descrpition: Resume the cinder unit.  This action will start cinder services.
 git-reinstall:
   description: Reinstall cinder from the openstack-origin-git repositories.
 openstack-upgrade:

actions/actions.py

@@ -0,0 +1,48 @@
+#!/usr/bin/python
+
+import os
+import sys
+
+sys.path.append('hooks/')
+
+from charmhelpers.core.hookenv import action_fail
+from cinder_utils import (
+    pause_unit_helper,
+    resume_unit_helper,
+    register_configs,
+)
+
+
+def pause(args):
+    """Pause the Ceilometer services.
+    @raises Exception should the service fail to stop.
+    """
+    pause_unit_helper(register_configs())
+
+
+def resume(args):
+    """Resume the Ceilometer services.
+    @raises Exception should the service fail to start."""
+    resume_unit_helper(register_configs())
+
+
+# A dictionary of all the defined actions to callables (which take
+# parsed arguments).
+ACTIONS = {"pause": pause, "resume": resume}
+
+
+def main(args):
+    action_name = os.path.basename(args[0])
+    try:
+        action = ACTIONS[action_name]
+    except KeyError:
+        return "Action %s undefined" % action_name
+    else:
+        try:
+            action(args)
+        except Exception as e:
+            action_fail(str(e))
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))

actions/pause

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

actions/resume

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

hooks/cinder_hooks.py

@@ -27,8 +27,7 @@ from cinder_utils import (
     setup_ipv6,
     check_db_initialised,
     filesystem_mounted,
-    required_interfaces,
+    assess_status,
-    check_optional_relations,
 )
 
 from charmhelpers.core.hookenv import (
@@ -54,7 +53,6 @@ from charmhelpers.fetch import (
 
 from charmhelpers.core.host import (
     lsb_release,
-    restart_on_change,
     service_reload,
     umount,
 )
@@ -66,7 +64,8 @@ from charmhelpers.contrib.openstack.utils import (
     openstack_upgrade_available,
     sync_db_with_multi_ipv6_addresses,
     os_release,
-    set_os_workload_status,
+    is_unit_paused_set,
+    pausable_restart_on_change as restart_on_change,
 )
 
 from charmhelpers.contrib.storage.linux.ceph import (
@@ -165,7 +164,7 @@ def config_changed():
                              upgrade_nonce=uuid.uuid4())
 
     # overwrite config is not in conf file. so We can't use restart_on_change
-    if config_value_changed('overwrite'):
+    if config_value_changed('overwrite') and not is_unit_paused_set():
         service_restart('cinder-volume')
 
     CONFIGS.write_all()
@@ -367,6 +366,7 @@ def ceph_changed(relation_id=None):
         CONFIGS.write(ceph_config_file())
         # Ensure that cinder-volume is restarted since only now can we
         # guarantee that ceph resources are ready.
+        if not is_unit_paused_set():
             service_restart('cinder-volume')
     else:
         send_request_if_needed(get_ceph_request())
@@ -511,6 +511,7 @@ def configure_https():
 
     # TODO: improve this by checking if local CN certs are available
     # first then checking reload status (see LP #1433114).
+    if not is_unit_paused_set():
         service_reload('apache2', restart_on_failure=True)
 
     for rid in relation_ids('identity-service'):
@@ -557,5 +558,4 @@ if __name__ == '__main__':
         hooks.execute(sys.argv)
     except UnregisteredHookError as e:
         juju_log('Unknown hook {} - skipping.'.format(e))
-    set_os_workload_status(CONFIGS, required_interfaces(),
+    assess_status(CONFIGS)
-                           charm_func=check_optional_relations)

hooks/cinder_utils.py

@@ -54,7 +54,7 @@ from charmhelpers.contrib.hahelpers.cluster import (
 from charmhelpers.contrib.storage.linux.utils import (
     is_block_device,
     zap_disk,
-    is_device_mounted
+    is_device_mounted,
 )
 
 from charmhelpers.contrib.storage.linux.lvm import (
@@ -85,6 +85,10 @@ from charmhelpers.contrib.openstack.utils import (
     git_pip_venv_dir,
     os_release,
     set_os_workload_status,
+    make_assess_status_func,
+    pause_unit,
+    resume_unit,
+    is_unit_paused_set,
 )
 
 from charmhelpers.core.decorators import (
@@ -578,6 +582,7 @@ def check_db_initialised():
                 local_unit() not in init_id):
             log("Restarting cinder services following db initialisation",
                 level=DEBUG)
+            if not is_unit_paused_set():
                 for svc in enabled_services():
                     service_restart(svc)
 
@@ -596,6 +601,8 @@ def migrate_database():
     log("Notifying peer(s) that db is initialised and restarting services",
         level=DEBUG)
     for r_id in relation_ids('cluster'):
+
+        if not is_unit_paused_set():
             for svc in enabled_services():
                 service_restart(svc)
 
@@ -644,6 +651,7 @@ def do_openstack_upgrade(configs):
     [service_stop(s) for s in services()]
     if is_elected_leader(CLUSTER_RES):
         migrate_database()
+    if not is_unit_paused_set():
         [service_start(s) for s in services()]
 
 
@@ -830,6 +838,7 @@ def git_post_install(projects_yaml):
     render('git.upstart', '/etc/init/cinder-volume.conf',
            cinder_volume_context, perms=0o644, templates_dir=templates_dir)
 
+    if not is_unit_paused_set():
         service_restart('tgtd')
 
         [service_restart(s) for s in services()]
@@ -861,3 +870,70 @@ def check_optional_relations(configs):
         return status_get()
     else:
         return 'unknown', 'No optional relations'
+
+
+def assess_status(configs):
+    """Assess status of current unit
+    Decides what the state of the unit should be based on the current
+    configuration.
+    SIDE EFFECT: calls set_os_workload_status(...) which sets the workload
+    status of the unit.
+    Also calls status_set(...) directly if paused state isn't complete.
+    @param configs: a templating.OSConfigRenderer() object
+    @returns None - this function is executed for its side-effect
+    """
+    assess_status_func(configs)()
+
+
+def assess_status_func(configs):
+    """Helper function to create the function that will assess_status() for
+    the unit.
+    Uses charmhelpers.contrib.openstack.utils.make_assess_status_func() to
+    create the appropriate status function and then returns it.
+    Used directly by assess_status() and also for pausing and resuming
+    the unit.
+
+    NOTE(ajkavanagh) ports are not checked due to race hazards with services
+    that don't behave sychronously w.r.t their service scripts.  e.g.
+    apache2.
+    @param configs: a templating.OSConfigRenderer() object
+    @return f() -> None : a function that assesses the unit's workload status
+    """
+    return make_assess_status_func(
+        configs, REQUIRED_INTERFACES,
+        charm_func=check_optional_relations,
+        services=services(), ports=None)
+
+
+def pause_unit_helper(configs):
+    """Helper function to pause a unit, and then call assess_status(...) in
+    effect, so that the status is correctly updated.
+    Uses charmhelpers.contrib.openstack.utils.pause_unit() to do the work.
+    @param configs: a templating.OSConfigRenderer() object
+    @returns None - this function is executed for its side-effect
+    """
+    _pause_resume_helper(pause_unit, configs)
+
+
+def resume_unit_helper(configs):
+    """Helper function to resume a unit, and then call assess_status(...) in
+    effect, so that the status is correctly updated.
+    Uses charmhelpers.contrib.openstack.utils.resume_unit() to do the work.
+    @param configs: a templating.OSConfigRenderer() object
+    @returns None - this function is executed for its side-effect
+    """
+    _pause_resume_helper(resume_unit, configs)
+
+
+def _pause_resume_helper(f, configs):
+    """Helper function that uses the make_assess_status_func(...) from
+    charmhelpers.contrib.openstack.utils to create an assess_status(...)
+    function that can be used with the pause/resume of the unit
+    @param f: the function to be used with the assess_status(...) function
+    @returns None - this function is executed for its side-effect
+    """
+    # TODO(ajkavanagh) - ports= has been left off because of the race hazard
+    # that exists due to service_start()
+    f(assess_status_func(configs),
+      services=services(),
+      ports=None)

tests/basic_deployment.py

@@ -3,6 +3,9 @@
 import amulet
 import os
 import yaml
+import time
+import json
+import subprocess
 
 from charmhelpers.contrib.openstack.amulet.deployment import (
     OpenStackAmuletDeployment
@@ -139,6 +142,32 @@ class CinderBasicDeployment(OpenStackAmuletDeployment):
         # Authenticate admin with glance endpoint
         self.glance = u.authenticate_glance_admin(self.keystone)
 
+    def _run_action(self, unit_id, action, *args):
+        command = ["juju", "action", "do", "--format=json", unit_id, action]
+        command.extend(args)
+        print("Running command: %s\n" % " ".join(command))
+        output = subprocess.check_output(command)
+        output_json = output.decode(encoding="UTF-8")
+        data = json.loads(output_json)
+        action_id = data[u'Action queued with id']
+        return action_id
+
+    def _wait_on_action(self, action_id):
+        command = ["juju", "action", "fetch", "--format=json", action_id]
+        while True:
+            try:
+                output = subprocess.check_output(command)
+            except Exception as e:
+                print(e)
+                return False
+            output_json = output.decode(encoding="UTF-8")
+            data = json.loads(output_json)
+            if data[u"status"] == "completed":
+                return True
+            elif data[u"status"] == "failed":
+                return False
+            time.sleep(2)
+
     def _extend_cinder_volume(self, vol_id, new_size=2):
         """Extend an existing cinder volume size.
 
@@ -709,3 +738,20 @@ class CinderBasicDeployment(OpenStackAmuletDeployment):
             sleep_time = 0
 
         self.d.configure(juju_service, set_default)
+
+    def test_910_pause_and_resume(self):
+        """The services can be paused and resumed. """
+        u.log.debug('Checking pause and resume actions...')
+        unit_name = "cinder/0"
+        unit = self.d.sentry.unit[unit_name]
+
+        assert u.status_get(unit)[0] == "active"
+
+        action_id = self._run_action(unit_name, "pause")
+        assert self._wait_on_action(action_id), "Pause action failed."
+        assert u.status_get(unit)[0] == "maintenance"
+
+        action_id = self._run_action(unit_name, "resume")
+        assert self._wait_on_action(action_id), "Resume action failed."
+        assert u.status_get(unit)[0] == "active"
+        u.log.debug('OK')

unit_tests/test_actions.py

@@ -0,0 +1,72 @@
+from mock import patch, mock
+import os
+
+os.environ['JUJU_UNIT_NAME'] = 'cinder'
+
+from test_utils import RESTART_MAP
+
+with patch('cinder_utils.register_configs') as register_configs:
+    with patch('cinder_utils.restart_map') as restart_map:
+        restart_map.return_value = RESTART_MAP
+        register_configs.return_value = 'test-config'
+        import actions
+
+from test_utils import (
+    CharmTestCase
+)
+
+
+class PauseTestCase(CharmTestCase):
+
+    def setUp(self):
+        super(PauseTestCase, self).setUp(
+            actions, ["pause_unit_helper"])
+
+    def test_pauses_services(self):
+        actions.pause([])
+        self.pause_unit_helper.assert_called_once_with('test-config')
+
+
+class ResumeTestCase(CharmTestCase):
+
+    def setUp(self):
+        super(ResumeTestCase, self).setUp(
+            actions, ["resume_unit_helper"])
+
+    def test_pauses_services(self):
+        actions.resume([])
+        self.resume_unit_helper.assert_called_once_with('test-config')
+
+
+class MainTestCase(CharmTestCase):
+
+    def setUp(self):
+        super(MainTestCase, self).setUp(actions, ["action_fail"])
+
+    def test_invokes_action(self):
+        dummy_calls = []
+
+        def dummy_action(args):
+            dummy_calls.append(True)
+
+        with mock.patch.dict(actions.ACTIONS, {"foo": dummy_action}):
+            actions.main(["foo"])
+        self.assertEqual(dummy_calls, [True])
+
+    def test_unknown_action(self):
+        """Unknown actions aren't a traceback."""
+        exit_string = actions.main(["foo"])
+        self.assertEqual("Action foo undefined", exit_string)
+
+    def test_failing_action(self):
+        """Actions whose traceback trigger action_fail() calls."""
+        dummy_calls = []
+
+        self.action_fail.side_effect = dummy_calls.append
+
+        def dummy_action(args):
+            raise ValueError("uh oh")
+
+        with mock.patch.dict(actions.ACTIONS, {"foo": dummy_action}):
+            actions.main(["foo"])
+        self.assertEqual(dummy_calls, ["uh oh"])

unit_tests/test_cinder_utils.py

@@ -925,3 +925,49 @@ class TestCinderUtils(CharmTestCase):
             'messaging': ['amqp'],
         }
         self.assertEqual(cinder_utils.required_interfaces(), expected)
+
+    def test_assess_status(self):
+        with patch.object(cinder_utils, 'assess_status_func') as asf:
+            callee = MagicMock()
+            asf.return_value = callee
+            cinder_utils.assess_status('test-config')
+            asf.assert_called_once_with('test-config')
+            callee.assert_called_once_with()
+
+    @patch.object(cinder_utils, 'check_optional_relations')
+    @patch.object(cinder_utils, 'REQUIRED_INTERFACES')
+    @patch.object(cinder_utils, 'services')
+    @patch.object(cinder_utils, 'make_assess_status_func')
+    def test_assess_status_func(self,
+                                make_assess_status_func,
+                                services,
+                                REQUIRED_INTERFACES,
+                                check_optional_relations):
+        services.return_value = 's1'
+        cinder_utils.assess_status_func('test-config')
+        # ports=None whilst port checks are disabled.
+        make_assess_status_func.assert_called_once_with(
+            'test-config', REQUIRED_INTERFACES,
+            charm_func=check_optional_relations,
+            services='s1', ports=None)
+
+    def test_pause_unit_helper(self):
+        with patch.object(cinder_utils, '_pause_resume_helper') as prh:
+            cinder_utils.pause_unit_helper('random-config')
+            prh.assert_called_once_with(cinder_utils.pause_unit,
+                                        'random-config')
+        with patch.object(cinder_utils, '_pause_resume_helper') as prh:
+            cinder_utils.resume_unit_helper('random-config')
+            prh.assert_called_once_with(cinder_utils.resume_unit,
+                                        'random-config')
+
+    @patch.object(cinder_utils, 'services')
+    def test_pause_resume_helper(self, services):
+        f = MagicMock()
+        services.return_value = 's1'
+        with patch.object(cinder_utils, 'assess_status_func') as asf:
+            asf.return_value = 'assessor'
+            cinder_utils._pause_resume_helper(f, 'some-config')
+            asf.assert_called_once_with('some-config')
+            # ports=None whilst port checks are disabled.
+            f.assert_called_once_with('assessor', services='s1', ports=None)