From 4d3f996e7ce5385688f6e570b8ac73327bac9f44 Mon Sep 17 00:00:00 2001
From: Rui Chen <chenrui.momo@gmail.com>
Date: Mon, 14 Mar 2016 16:53:51 +0800
Subject: [PATCH] Log hint when --enable present with --disable-reason
--enable and --disable-reason should be mutually exclusive in "compute
service set" command, but now when they are present at the same time,
--disable-reason would be ignored silently. Fix these and add some hints
about --disable-reason argument is ignored in this situation.
Change-Id: I43254b6bc40fcae4fd0dc3457f26fad84c267072
Closes-Bug: #1556801
---
.../command-objects/compute-service.rst | 5 +-
openstackclient/compute/v2/service.py | 9 ++-
.../tests/compute/v2/test_service.py | 73 ++++++++++++++++---
3 files changed, 74 insertions(+), 13 deletions(-)
diff --git a/doc/source/command-objects/compute-service.rst b/doc/source/command-objects/compute-service.rst
index db75bef426..bda64c811d 100644
--- a/doc/source/command-objects/compute-service.rst
+++ b/doc/source/command-objects/compute-service.rst
@@ -36,7 +36,7 @@ List service command
.. _compute-service-list:
.. option:: --host <host>
- List services on specified host (name only)
+ List services on specified host (name only)
.. option:: --service <service>
@@ -71,7 +71,8 @@ Set service command
.. option:: --disable-reason <reason>
- Reason for disabling the service (in quotes)
+ Reason for disabling the service (in quotes). Note that when the service
+ is enabled, this option is ignored.
.. describe:: <host>
diff --git a/openstackclient/compute/v2/service.py b/openstackclient/compute/v2/service.py
index 68037c9445..2b51af3d64 100644
--- a/openstackclient/compute/v2/service.py
+++ b/openstackclient/compute/v2/service.py
@@ -17,6 +17,7 @@
from openstackclient.common import command
from openstackclient.common import utils
+from openstackclient.i18n import _ # noqa
class DeleteService(command.Command):
@@ -117,8 +118,8 @@ class SetService(command.Command):
"--disable-reason",
default=None,
metavar="<reason>",
- help="Reason for disabling the service (in quotas)"
- )
+ help="Reason for disabling the service (in quotas). Note that "
+ "when the service is enabled, this option is ignored.")
return parser
def take_action(self, parsed_args):
@@ -133,4 +134,8 @@ class SetService(command.Command):
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 2feaf1566f..db09720457 100644
--- a/openstackclient/tests/compute/v2/test_service.py
+++ b/openstackclient/tests/compute/v2/test_service.py
@@ -14,6 +14,7 @@
#
import copy
+import mock
from openstackclient.compute.v2 import service
from openstackclient.tests.compute.v2 import fakes as compute_fakes
@@ -138,14 +139,14 @@ class TestServiceSet(TestService):
def test_service_set_enable(self):
arglist = [
+ '--enable',
compute_fakes.service_host,
compute_fakes.service_binary,
- '--enable',
]
verifylist = [
+ ('enabled', True),
('host', compute_fakes.service_host),
('service', compute_fakes.service_binary),
- ('enabled', True),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -159,14 +160,14 @@ class TestServiceSet(TestService):
def test_service_set_disable(self):
arglist = [
+ '--disable',
compute_fakes.service_host,
compute_fakes.service_binary,
- '--disable',
]
verifylist = [
+ ('enabled', False),
('host', compute_fakes.service_host),
('service', compute_fakes.service_binary),
- ('enabled', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -181,17 +182,16 @@ class TestServiceSet(TestService):
def test_service_set_disable_with_reason(self):
reason = 'earthquake'
arglist = [
+ '--disable',
+ '--disable-reason', reason,
compute_fakes.service_host,
compute_fakes.service_binary,
- '--disable',
- '--disable-reason',
- reason
]
verifylist = [
+ ('enabled', False),
+ ('disable_reason', reason),
('host', compute_fakes.service_host),
('service', compute_fakes.service_binary),
- ('enabled', False),
- ('disable_reason', reason)
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -203,3 +203,58 @@ class TestServiceSet(TestService):
reason
)
self.assertIsNone(result)
+
+ def test_service_set_only_with_disable_reason(self):
+ reason = 'earthquake'
+ arglist = [
+ '--disable-reason', reason,
+ compute_fakes.service_host,
+ compute_fakes.service_binary,
+ ]
+ verifylist = [
+ ('enabled', True),
+ ('disable_reason', reason),
+ ('host', compute_fakes.service_host),
+ ('service', compute_fakes.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(
+ compute_fakes.service_host,
+ compute_fakes.service_binary
+ )
+ self.assertIsNone(result)
+
+ def test_service_set_enable_with_disable_reason(self):
+ reason = 'earthquake'
+ arglist = [
+ '--enable',
+ '--disable-reason', reason,
+ compute_fakes.service_host,
+ compute_fakes.service_binary,
+ ]
+ verifylist = [
+ ('enabled', True),
+ ('disable_reason', reason),
+ ('host', compute_fakes.service_host),
+ ('service', compute_fakes.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(
+ compute_fakes.service_host,
+ compute_fakes.service_binary
+ )
+ self.assertIsNone(result)