From 187be0ac22be5d2d1c42d45e9299e62cfc3796ae Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Tue, 14 May 2019 18:35:54 +0200 Subject: [PATCH] Microversion 2.73: Support adding the reason behind a server lock This patch adds a new parameter ``--reason`` to ``openstack server lock`` command and ``--locked``, ``unlocked`` filtering parameters to ``openstack server list`` command. This can help users to provide a reason when locking the server and to filter instances based on their locked value from 2.73 microversion. Implements blueprint add-locked-reason Depends-On: https://review.opendev.org/#/c/661785/ Change-Id: Ib2714f98b24d47e570da8a6c231e765acd2ff595 --- lower-constraints.txt | 2 +- openstackclient/compute/v2/server.py | 47 +++++- .../tests/unit/compute/v2/test_server.py | 149 +++++++++++++++++- ...bp-add-locked-reason-425efd2def1144f1.yaml | 13 ++ requirements.txt | 2 +- 5 files changed, 206 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/bp-add-locked-reason-425efd2def1144f1.yaml diff --git a/lower-constraints.txt b/lower-constraints.txt index 43d724a1e..f38782794 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -100,7 +100,7 @@ python-mimeparse==1.6.0 python-mistralclient==3.1.0 python-muranoclient==0.8.2 python-neutronclient==6.7.0 -python-novaclient==10.0.0 +python-novaclient==14.1.0 python-octaviaclient==1.3.0 python-rsdclient==0.1.0 python-saharaclient==1.4.0 diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index cb9f8d43e..b37b8c529 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -1136,6 +1136,21 @@ class ListServer(command.Lister): " The provided time should be an ISO 8061 formatted time." " ex 2016-03-04T06:27:59Z .") ) + lock_group = parser.add_mutually_exclusive_group() + lock_group.add_argument( + '--locked', + action='store_true', + default=False, + help=_('Only display locked servers. ' + 'Requires ``--os-compute-api-version`` 2.73 or greater.'), + ) + lock_group.add_argument( + '--unlocked', + action='store_true', + default=False, + help=_('Only display unlocked servers. ' + 'Requires ``--os-compute-api-version`` 2.73 or greater.'), + ) return parser def take_action(self, parsed_args): @@ -1190,6 +1205,18 @@ class ListServer(command.Lister): 'deleted': parsed_args.deleted, 'changes-since': parsed_args.changes_since, } + support_locked = (compute_client.api_version >= + api_versions.APIVersion('2.73')) + if not support_locked and (parsed_args.locked or parsed_args.unlocked): + msg = _('--os-compute-api-version 2.73 or greater is required to ' + 'use the (un)locked filter option.') + raise exceptions.CommandError(msg) + elif support_locked: + # Only from 2.73. + if parsed_args.locked: + search_opts['locked'] = True + if parsed_args.unlocked: + search_opts['locked'] = False LOG.debug('search options: %s', search_opts) if search_opts['changes-since']: @@ -1374,16 +1401,28 @@ class LockServer(command.Command): nargs='+', help=_('Server(s) to lock (name or ID)'), ) + parser.add_argument( + '--reason', + metavar='', + default=None, + help=_("Reason for locking the server(s). Requires " + "``--os-compute-api-version`` 2.73 or greater.") + ) return parser def take_action(self, parsed_args): compute_client = self.app.client_manager.compute + support_reason = compute_client.api_version >= api_versions.APIVersion( + '2.73') + if not support_reason and parsed_args.reason: + msg = _('--os-compute-api-version 2.73 or greater is required to ' + 'use the --reason option.') + raise exceptions.CommandError(msg) for server in parsed_args.server: - utils.find_resource( - compute_client.servers, - server, - ).lock() + serv = utils.find_resource(compute_client.servers, server) + (serv.lock(reason=parsed_args.reason) if support_reason + else serv.lock()) # FIXME(dtroyer): Here is what I want, how with argparse/cliff? diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index c30af8fb8..c71a31cec 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -90,7 +90,14 @@ class TestServer(compute_fakes.TestComputev2): for s in servers: method = getattr(s, method_name) - method.assert_called_with() + if method_name == 'lock': + version = self.app.client_manager.compute.api_version + if version >= api_versions.APIVersion('2.73'): + method.assert_called_with(reason=None) + else: + method.assert_called_with() + else: + method.assert_called_with() self.assertIsNone(result) @@ -2210,6 +2217,80 @@ class TestServerList(TestServer): self.assertEqual(self.columns, columns) self.assertEqual(tuple(self.data), tuple(data)) + def test_server_list_with_locked_pre_v273(self): + + arglist = [ + '--locked' + ] + verifylist = [ + ('locked', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + ex = self.assertRaises(exceptions.CommandError, + self.cmd.take_action, + parsed_args) + self.assertIn( + '--os-compute-api-version 2.73 or greater is required', str(ex)) + + def test_server_list_with_locked_v273(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--locked' + ] + verifylist = [ + ('locked', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + self.search_opts['locked'] = True + self.servers_mock.list.assert_called_with(**self.kwargs) + + self.assertEqual(self.columns, columns) + self.assertEqual(tuple(self.data), tuple(data)) + + def test_server_list_with_unlocked_v273(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--unlocked' + ] + verifylist = [ + ('unlocked', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + self.search_opts['locked'] = False + self.servers_mock.list.assert_called_with(**self.kwargs) + + self.assertEqual(self.columns, columns) + self.assertEqual(tuple(self.data), tuple(data)) + + def test_server_list_with_locked_and_unlocked_v273(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--locked', + '--unlocked' + ] + verifylist = [ + ('locked', True), + ('unlocked', True) + ] + + ex = self.assertRaises( + utils.ParserException, + self.check_parser, self.cmd, arglist, verifylist) + self.assertIn('Argument parse failed', str(ex)) + def test_server_list_with_flavor(self): arglist = [ @@ -2336,6 +2417,72 @@ class TestServerLock(TestServer): def test_server_lock_multi_servers(self): self.run_method_with_servers('lock', 3) + def test_server_lock_with_reason(self): + server = compute_fakes.FakeServer.create_one_server() + arglist = [ + server.id, + '--reason', "blah", + ] + verifylist = [ + ('reason', "blah"), + ('server', [server.id]) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + ex = self.assertRaises(exceptions.CommandError, + self.cmd.take_action, + parsed_args) + self.assertIn( + '--os-compute-api-version 2.73 or greater is required', str(ex)) + + +class TestServerLockV273(TestServerLock): + + def setUp(self): + super(TestServerLockV273, self).setUp() + + self.server = compute_fakes.FakeServer.create_one_server( + methods=self.methods) + + # This is the return value for utils.find_resource() + self.servers_mock.get.return_value = self.server + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + + # Get the command object to test + self.cmd = server.LockServer(self.app, None) + + def test_server_lock_with_reason(self): + arglist = [ + self.server.id, + '--reason', "blah", + ] + verifylist = [ + ('reason', "blah"), + ('server', [self.server.id]) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + self.servers_mock.get.assert_called_with(self.server.id) + self.server.lock.assert_called_with(reason="blah") + + def test_server_lock_multi_servers_with_reason(self): + server2 = compute_fakes.FakeServer.create_one_server( + methods=self.methods) + arglist = [ + self.server.id, server2.id, + '--reason', "choo..choo", + ] + verifylist = [ + ('reason', "choo..choo"), + ('server', [self.server.id, server2.id]) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + self.assertEqual(2, self.servers_mock.get.call_count) + self.server.lock.assert_called_with(reason="choo..choo") + self.assertEqual(2, self.server.lock.call_count) + class TestServerMigrate(TestServer): diff --git a/releasenotes/notes/bp-add-locked-reason-425efd2def1144f1.yaml b/releasenotes/notes/bp-add-locked-reason-425efd2def1144f1.yaml new file mode 100644 index 000000000..e9f6cc6d9 --- /dev/null +++ b/releasenotes/notes/bp-add-locked-reason-425efd2def1144f1.yaml @@ -0,0 +1,13 @@ +--- +features: + - Add ``--reason`` option to the ``server lock`` command to specify a reason + when locking a server. + Requires ``–os-compute-api-version`` 2.73 or greater. + - Add ``--locked`` option to the ``server list`` command to list only locked + servers. + Requires ``–os-compute-api-version`` 2.73 or greater. + - Add ``--unlocked`` option to the ``server list`` command list only unlocked + servers. + Requires ``–os-compute-api-version`` 2.73 or greater. + [Blueprint `add-locked-reason `_] + \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index aa5debf4e..3db6caef0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,5 +13,5 @@ oslo.i18n>=3.15.3 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0 python-glanceclient>=2.8.0 # Apache-2.0 python-keystoneclient>=3.17.0 # Apache-2.0 -python-novaclient>=10.0.0 # Apache-2.0 +python-novaclient>=14.1.0 # Apache-2.0 python-cinderclient>=3.3.0 # Apache-2.0