Don't include libvirtd on pause/resume

libvirt needs to be running even when the pause action runs, this allows
commands like "virsh secret-list" run and having it running is safe
enough from a nova-compute unit point of view, because it won't touch
the already running instances.

Change-Id: Idec9e2b7c6275c4c5485942574120289552f2955
Closes-Bug: 1802917

hooks/nova_compute_utils.py

@@ -830,11 +830,16 @@ def assess_status(configs):
     @param configs: a templating.OSConfigRenderer() object
     @returns None - this function is executed for its side-effect
     """
-    assess_status_func(configs)()
+    if is_unit_paused_set():
+        services_to_check = services_to_pause_or_resume()
+    else:
+        services_to_check = services()
+
+    assess_status_func(configs, services_to_check)()
     os_application_version_set(VERSION_PACKAGE)
 
 
-def assess_status_func(configs):
+def assess_status_func(configs, services_=None):
     """Helper function to create the function that will assess_status() for
     the unit.
     Uses charmhelpers.contrib.openstack.utils.make_assess_status_func() to
@@ -852,7 +857,7 @@ def assess_status_func(configs):
     required_interfaces.update(get_optional_relations())
     return make_assess_status_func(
         configs, required_interfaces,
-        services=services(), ports=None)
+        services=services_ or services(), ports=None)
 
 
 def pause_unit_helper(configs):
@@ -875,6 +880,10 @@ def resume_unit_helper(configs):
     _pause_resume_helper(resume_unit, configs)
 
 
+def services_to_pause_or_resume():
+    return list(set(services()) - {libvirt_daemon()})
+
+
 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(...)
@@ -884,8 +893,9 @@ def _pause_resume_helper(f, configs):
     """
     # 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(),
+
+    f(assess_status_func(configs, services_to_pause_or_resume()),
+      services=services_to_pause_or_resume(),
       ports=None)
 
 

tests/basic_deployment.py

@@ -648,6 +648,20 @@ class NovaBasicDeployment(OpenStackAmuletDeployment):
         assert u.wait_on_action(action_id), "Pause action failed."
         assert u.status_get(sentry_unit)[0] == "maintenance"
 
+        # check that on resume libvirt wasn't stopped (LP: #1802917)
+        cmd = ('cd hooks; python3 -c '
+               '"from nova_compute_utils import libvirt_daemon; '
+               'print(libvirt_daemon())"')
+        daemon, code = sentry_unit.run(cmd)
+
+        if self.series == "trusty":
+            cmd = 'service %s status' % daemon
+        else:
+            cmd = 'systemctl status %s' % daemon
+
+        output, code = sentry_unit.run(cmd)
+        assert code == 0, '%s, exit code: %d, output: %s' % (cmd, code, output)
+
         action_id = u.run_action(sentry_unit, "resume")
         assert u.wait_on_action(action_id), "Resume action failed."
         assert u.status_get(sentry_unit)[0] == "active"

unit_tests/test_nova_compute_utils.py

@@ -754,12 +754,35 @@ class NovaComputeUtilsTests(CharmTestCase):
             set_shmmax=True,
         )
 
-    def test_assess_status(self):
+    @patch.object(utils, 'is_unit_paused_set')
+    @patch.object(utils, 'services')
+    def test_assess_status(self, services, mock_is_paused):
+        services.return_value = 's1'
+        mock_is_paused.return_value = False
         with patch.object(utils, 'assess_status_func') as asf:
             callee = MagicMock()
             asf.return_value = callee
             utils.assess_status('test-config')
-            asf.assert_called_once_with('test-config')
+            asf.assert_called_once_with('test-config', 's1')
+            callee.assert_called_once_with()
+            self.os_application_version_set.assert_called_with(
+                utils.VERSION_PACKAGE
+            )
+
+    @patch.object(utils, 'os_release')
+    @patch.object(utils, 'is_unit_paused_set')
+    @patch.object(utils, 'services')
+    def test_assess_status_paused(self, services, mock_is_paused,
+                                  mock_os_release):
+        services.return_value = ['qemu-kvm', 'libvirtd', 'nova-compute']
+        mock_is_paused.return_value = True
+        mock_os_release.return_value = 'pike'
+        with patch.object(utils, 'assess_status_func') as asf:
+            callee = MagicMock()
+            asf.return_value = callee
+            utils.assess_status('test-config')
+            asf.assert_called_once_with('test-config',
+                                        ['nova-compute', 'qemu-kvm'])
             callee.assert_called_once_with()
             self.os_application_version_set.assert_called_with(
                 utils.VERSION_PACKAGE
@@ -774,7 +797,7 @@ class NovaComputeUtilsTests(CharmTestCase):
                                 make_assess_status_func,
                                 services,
                                 REQUIRED_INTERFACES):
-        services.return_value = 's1'
+        services.return_value = ['s1']
         REQUIRED_INTERFACES.copy.return_value = {'test-interface': True}
         get_optional_relations.return_value = {'optional': False}
         test_interfaces = {
@@ -784,7 +807,7 @@ class NovaComputeUtilsTests(CharmTestCase):
         utils.assess_status_func('test-config')
         # ports=None whilst port checks are disabled.
         make_assess_status_func.assert_called_once_with(
-            'test-config', test_interfaces, services='s1', ports=None)
+            'test-config', test_interfaces, services=['s1'], ports=None)
 
     def test_pause_unit_helper(self):
         with patch.object(utils, '_pause_resume_helper') as prh:
@@ -794,16 +817,21 @@ class NovaComputeUtilsTests(CharmTestCase):
             utils.resume_unit_helper('random-config')
             prh.assert_called_once_with(utils.resume_unit, 'random-config')
 
+    @patch.object(utils, 'os_release')
+    @patch.object(utils, 'is_unit_paused_set')
     @patch.object(utils, 'services')
-    def test_pause_resume_helper(self, services):
+    def test_pause_resume_helper(self, services, mock_is_paused,
+                                 mock_os_release):
         f = MagicMock()
-        services.return_value = 's1'
+        services.return_value = ['s1']
+        mock_is_paused.return_value = False
+        mock_os_release.return_value = 'queens'
         with patch.object(utils, 'assess_status_func') as asf:
             asf.return_value = 'assessor'
             utils._pause_resume_helper(f, 'some-config')
-            asf.assert_called_once_with('some-config')
+            asf.assert_called_once_with('some-config', ['s1'])
             # ports=None whilst port checks are disabled.
-            f.assert_called_once_with('assessor', services='s1', ports=None)
+            f.assert_called_once_with('assessor', services=['s1'], ports=None)
 
     @patch.object(utils, 'check_call')
     @patch.object(utils, 'check_output')