diff --git a/doc/source/command-objects/compute-service.rst b/doc/source/command-objects/compute-service.rst
index bda64c811d..71225bc37a 100644
--- a/doc/source/command-objects/compute-service.rst
+++ b/doc/source/command-objects/compute-service.rst
@@ -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>
diff --git a/openstackclient/compute/v2/service.py b/openstackclient/compute/v2/service.py
index 6093b8ce17..e7966a8a1a 100644
--- a/openstackclient/compute/v2/service.py
+++ b/openstackclient/compute/v2/service.py
@@ -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)
diff --git a/openstackclient/tests/compute/v2/test_service.py b/openstackclient/tests/compute/v2/test_service.py
index 7a5a840f05..cf53497878 100644
--- a/openstackclient/tests/compute/v2/test_service.py
+++ b/openstackclient/tests/compute/v2/test_service.py
@@ -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)
diff --git a/releasenotes/notes/service-set-option-61772a8940ad0778.yaml b/releasenotes/notes/service-set-option-61772a8940ad0778.yaml
new file mode 100644
index 0000000000..02c7fcba2e
--- /dev/null
+++ b/releasenotes/notes/service-set-option-61772a8940ad0778.yaml
@@ -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.