Support compute service force down/up

Aims to evacuate servers from compute host as soon as possible,
operators might set the compute service force down manually.
Novaclient support the behavior, this patch support it in OSC.

Change-Id: I22ff1c5d670c449771fdcb3f4f39cd82f428531a
Closes-Bug: #1589348

doc/source/command-objects/compute-service.rst

@@ -58,6 +58,7 @@ Set service command
     os compute service set
         [--enable | --disable]
         [--disable-reason <reason>]
+        [--up | --down]
         <host> <service>
 
 .. _compute-service-set:
@@ -73,6 +74,14 @@ Set service command
 
     Reason for disabling the service (in quotes). Should be used with --disable option.
 
+.. option:: --up
+
+    Force up service
+
+.. option:: --down
+
+    Force down service
+
 .. describe:: <host>
 
     Name of host

openstackclient/compute/v2/service.py

@@ -20,6 +20,7 @@ from osc_lib import exceptions
 from osc_lib import utils
 
 from openstackclient.i18n import _
+from openstackclient.i18n import _LE
 
 
 class DeleteService(command.Command):
@@ -127,6 +128,17 @@ class SetService(command.Command):
             help=_("Reason for disabling the service (in quotas). "
                    "Should be used with --disable option.")
         )
+        up_down_group = parser.add_mutually_exclusive_group()
+        up_down_group.add_argument(
+            '--up',
+            action='store_true',
+            help=_('Force up service'),
+        )
+        up_down_group.add_argument(
+            '--down',
+            action='store_true',
+            help=_('Force down service'),
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -139,20 +151,45 @@ class SetService(command.Command):
                     "--disable specified.")
             raise exceptions.CommandError(msg)
 
+        result = 0
         enabled = None
-        if parsed_args.enable:
-            enabled = True
-        if parsed_args.disable:
-            enabled = False
+        try:
+            if parsed_args.enable:
+                enabled = True
+            if parsed_args.disable:
+                enabled = False
 
-        if enabled is None:
-            return
-        elif enabled:
-            cs.enable(parsed_args.host, parsed_args.service)
-        else:
-            if parsed_args.disable_reason:
-                cs.disable_log_reason(parsed_args.host,
-                                      parsed_args.service,
-                                      parsed_args.disable_reason)
-            else:
-                cs.disable(parsed_args.host, parsed_args.service)
+            if enabled is not None:
+                if enabled:
+                    cs.enable(parsed_args.host, parsed_args.service)
+                else:
+                    if parsed_args.disable_reason:
+                        cs.disable_log_reason(parsed_args.host,
+                                              parsed_args.service,
+                                              parsed_args.disable_reason)
+                    else:
+                        cs.disable(parsed_args.host, parsed_args.service)
+        except Exception:
+            status = "enabled" if enabled else "disabled"
+            self.log.error(_LE("Failed to set service status to %s"), status)
+            result += 1
+
+        force_down = None
+        try:
+            if parsed_args.down:
+                force_down = True
+            if parsed_args.up:
+                force_down = False
+            if force_down is not None:
+                cs.force_down(parsed_args.host, parsed_args.service,
+                              force_down=force_down)
+        except Exception:
+            state = "down" if force_down else "up"
+            self.log.error(_LE("Failed to set service state to %s"), state)
+            result += 1
+
+        if result > 0:
+            msg = _("Compute service %(service)s of host %(host)s failed to "
+                    "set.") % {"service": parsed_args.service,
+                               "host": parsed_args.host}
+            raise exceptions.CommandError(msg)

openstackclient/tests/compute/v2/test_service.py

@@ -13,6 +13,8 @@
 #   under the License.
 #
 
+import mock
+
 from osc_lib import exceptions
 
 from openstackclient.compute.v2 import service
@@ -225,8 +227,12 @@ class TestServiceSet(TestService):
             ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
-                          parsed_args)
+        try:
+            self.cmd.take_action(parsed_args)
+            self.fail("CommandError should be raised.")
+        except exceptions.CommandError as e:
+            self.assertEqual("Cannot specify option --disable-reason without "
+                             "--disable specified.", str(e))
 
     def test_service_set_enable_with_disable_reason(self):
         reason = 'earthquake'
@@ -243,5 +249,93 @@ class TestServiceSet(TestService):
             ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
-                          parsed_args)
+        try:
+            self.cmd.take_action(parsed_args)
+            self.fail("CommandError should be raised.")
+        except exceptions.CommandError as e:
+            self.assertEqual("Cannot specify option --disable-reason without "
+                             "--disable specified.", str(e))
+
+    def test_service_set_state_up(self):
+        arglist = [
+            '--up',
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('up', True),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+        self.service_mock.force_down.assert_called_once_with(
+            self.service.host, self.service.binary, force_down=False)
+        self.assertNotCalled(self.service_mock.enable)
+        self.assertNotCalled(self.service_mock.disable)
+        self.assertIsNone(result)
+
+    def test_service_set_state_down(self):
+        arglist = [
+            '--down',
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('down', True),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+        self.service_mock.force_down.assert_called_once_with(
+            self.service.host, self.service.binary, force_down=True)
+        self.assertNotCalled(self.service_mock.enable)
+        self.assertNotCalled(self.service_mock.disable)
+        self.assertIsNone(result)
+
+    def test_service_set_enable_and_state_down(self):
+        arglist = [
+            '--enable',
+            '--down',
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('enable', True),
+            ('down', True),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+        self.service_mock.enable.assert_called_once_with(
+            self.service.host, self.service.binary)
+        self.service_mock.force_down.assert_called_once_with(
+            self.service.host, self.service.binary, force_down=True)
+        self.assertIsNone(result)
+
+    def test_service_set_enable_and_state_down_with_exception(self):
+        arglist = [
+            '--enable',
+            '--down',
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('enable', True),
+            ('down', True),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        with mock.patch.object(self.cmd.log, 'error') as mock_log:
+            with mock.patch.object(self.service_mock, 'enable',
+                                   side_effect=Exception()):
+                self.assertRaises(exceptions.CommandError,
+                                  self.cmd.take_action, parsed_args)
+                mock_log.assert_called_once_with(
+                    "Failed to set service status to %s", "enabled")
+                self.service_mock.force_down.assert_called_once_with(
+                    self.service.host, self.service.binary, force_down=True)

releasenotes/notes/bug-1589348-4a612a4efc7ed0e5.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - Add options ``--up`` and ``--down`` for compute v2 ``compute service set``
+    command to support force up/down compute service.
+    [Bug `1589348 <https://bugs.launchpad.net/python-openstackclient/+bug/1589348>`_]