From eccd943acc9526311fcfc318280b4be51c42a566 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Mon, 6 Jun 2016 17:51:56 +0800 Subject: [PATCH] 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 --- .../command-objects/compute-service.rst | 9 ++ openstackclient/compute/v2/service.py | 67 +++++++++--- .../tests/compute/v2/test_service.py | 102 +++++++++++++++++- .../notes/bug-1589348-4a612a4efc7ed0e5.yaml | 5 + 4 files changed, 164 insertions(+), 19 deletions(-) create mode 100644 releasenotes/notes/bug-1589348-4a612a4efc7ed0e5.yaml diff --git a/doc/source/command-objects/compute-service.rst b/doc/source/command-objects/compute-service.rst index 7fe92ad313..f43b5fa636 100644 --- a/doc/source/command-objects/compute-service.rst +++ b/doc/source/command-objects/compute-service.rst @@ -58,6 +58,7 @@ Set service command os compute service set [--enable | --disable] [--disable-reason ] + [--up | --down] .. _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:: Name of host diff --git a/openstackclient/compute/v2/service.py b/openstackclient/compute/v2/service.py index 304657836c..18631837fa 100644 --- a/openstackclient/compute/v2/service.py +++ b/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) diff --git a/openstackclient/tests/compute/v2/test_service.py b/openstackclient/tests/compute/v2/test_service.py index 3f7413408e..b360c9dce4 100644 --- a/openstackclient/tests/compute/v2/test_service.py +++ b/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) diff --git a/releasenotes/notes/bug-1589348-4a612a4efc7ed0e5.yaml b/releasenotes/notes/bug-1589348-4a612a4efc7ed0e5.yaml new file mode 100644 index 0000000000..cee2cbf05d --- /dev/null +++ b/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 `_]