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 6db5acc3e8..d10af2cad9 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 `_]