Refactor SetService --enable/disable option

This patch changes the following:

1. --enable/disable option should follow the rules
   in the doc below:
   http://docs.openstack.org/developer/python-openstackclient/command-options.html#boolean-options

2. "--disable-resion" is specified but not "--disable",
   an exception is raised instead of igoring
   "--disable-reason" option.

Change-Id: I92e9234111e661bfe7119a8e19389a87c874ab0c
This commit is contained in:
Tang Chen 2016-05-25 11:12:57 +08:00
parent 81718beb8f
commit ac1d86c343
4 changed files with 57 additions and 49 deletions

View File

@ -63,7 +63,7 @@ Set service command
.. _compute-service-set:
.. option:: --enable
Enable service (default)
Enable service
.. option:: --disable
@ -71,8 +71,7 @@ Set service command
.. option:: --disable-reason <reason>
Reason for disabling the service (in quotes). Note that when the service
is enabled, this option is ignored.
Reason for disabling the service (in quotes). Should be used with --disable option.
.. describe:: <host>

View File

@ -16,6 +16,7 @@
"""Service action implementations"""
from openstackclient.common import command
from openstackclient.common import exceptions
from openstackclient.common import utils
from openstackclient.i18n import _
@ -110,23 +111,20 @@ class SetService(command.Command):
enabled_group = parser.add_mutually_exclusive_group()
enabled_group.add_argument(
"--enable",
dest="enabled",
default=True,
action="store_true",
help=_("Enable a service (default)")
help=_("Enable service")
)
enabled_group.add_argument(
"--disable",
dest="enabled",
action="store_false",
help=_("Disable a service")
action="store_true",
help=_("Disable service")
)
parser.add_argument(
"--disable-reason",
default=None,
metavar="<reason>",
help=_("Reason for disabling the service (in quotas). Note that "
"when the service is enabled, this option is ignored.")
help=_("Reason for disabling the service (in quotas). "
"Should be used with --disable option.")
)
return parser
@ -134,16 +132,26 @@ class SetService(command.Command):
compute_client = self.app.client_manager.compute
cs = compute_client.services
if not parsed_args.enabled:
if (parsed_args.enable or not parsed_args.disable) and \
parsed_args.disable_reason:
msg = _("Cannot specify option --disable-reason without "
"--disable specified.")
raise exceptions.CommandError(msg)
enabled = None
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)
else:
if parsed_args.disable_reason:
msg = _("argument --disable-reason has been ignored")
self.log.info(msg)
cs.enable(parsed_args.host, parsed_args.service)

View File

@ -13,8 +13,7 @@
# under the License.
#
import mock
from openstackclient.common import exceptions
from openstackclient.compute.v2 import service
from openstackclient.tests.compute.v2 import fakes as compute_fakes
@ -128,6 +127,23 @@ class TestServiceSet(TestService):
self.cmd = service.SetService(self.app, None)
def test_set_nothing(self):
arglist = [
self.service.host,
self.service.binary,
]
verifylist = [
('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_not_called()
self.service_mock.disable.assert_not_called()
self.service_mock.disable_log_reason.assert_not_called()
self.assertIsNone(result)
def test_service_set_enable(self):
arglist = [
'--enable',
@ -135,7 +151,7 @@ class TestServiceSet(TestService):
self.service.binary,
]
verifylist = [
('enabled', True),
('enable', True),
('host', self.service.host),
('service', self.service.binary),
]
@ -156,7 +172,7 @@ class TestServiceSet(TestService):
self.service.binary,
]
verifylist = [
('enabled', False),
('disable', True),
('host', self.service.host),
('service', self.service.binary),
]
@ -179,7 +195,7 @@ class TestServiceSet(TestService):
self.service.binary,
]
verifylist = [
('enabled', False),
('disable', True),
('disable_reason', reason),
('host', self.service.host),
('service', self.service.binary),
@ -203,24 +219,13 @@ class TestServiceSet(TestService):
self.service.binary,
]
verifylist = [
('enabled', True),
('disable_reason', reason),
('host', self.service.host),
('service', self.service.binary),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
with mock.patch.object(self.cmd.log, 'info') as mock_log:
result = self.cmd.take_action(parsed_args)
msg = "argument --disable-reason has been ignored"
mock_log.assert_called_once_with(msg)
self.service_mock.enable.assert_called_with(
self.service.host,
self.service.binary
)
self.assertIsNone(result)
self.assertRaises(exceptions.CommandError, self.cmd.take_action,
parsed_args)
def test_service_set_enable_with_disable_reason(self):
reason = 'earthquake'
@ -231,21 +236,11 @@ class TestServiceSet(TestService):
self.service.binary,
]
verifylist = [
('enabled', True),
('enable', True),
('disable_reason', reason),
('host', self.service.host),
('service', self.service.binary),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
with mock.patch.object(self.cmd.log, 'info') as mock_log:
result = self.cmd.take_action(parsed_args)
msg = "argument --disable-reason has been ignored"
mock_log.assert_called_once_with(msg)
self.service_mock.enable.assert_called_with(
self.service.host,
self.service.binary
)
self.assertIsNone(result)
self.assertRaises(exceptions.CommandError, self.cmd.take_action,
parsed_args)

View File

@ -0,0 +1,6 @@
---
upgrade:
- An exception is not raised by command ``service set`` when nothing
specified. Instead, the service is not enabled by default. And if
``--disable-resion`` is specified but not ``--disable``, an
exception will be raised.