Allow name as positional argument
Make "magnum cluster-create" and "magnum cluster-template-create" accept name as a positional like the other magnum subcommands. Implements: blueprint positional-name Change-Id: Ie505ef8ea7e4c13ed2795cde826b9822d094aaeb
This commit is contained in:
parent
e6329a75f9
commit
cb8edde0b6
|
@ -35,6 +35,16 @@ from six import moves
|
|||
from magnumclient.i18n import _
|
||||
|
||||
|
||||
NAME_DEPRECATION_BASE = ('%sThe --name parameter is deprecated and '
|
||||
'will be removed in a future release. Use the '
|
||||
'<name> positional parameter %s.')
|
||||
|
||||
NAME_DEPRECATION_HELP = NAME_DEPRECATION_BASE % ('', 'instead')
|
||||
|
||||
NAME_DEPRECATION_WARNING = NAME_DEPRECATION_BASE % (
|
||||
'WARNING: ', 'to avoid seeing this message')
|
||||
|
||||
|
||||
def deprecation_message(preamble, new_name):
|
||||
msg = ('%s This parameter is deprecated and will be removed in a future '
|
||||
'release. Use --%s instead.' % (preamble, new_name))
|
||||
|
@ -49,6 +59,14 @@ class MissingArgs(Exception):
|
|||
super(MissingArgs, self).__init__(msg)
|
||||
|
||||
|
||||
class DuplicateArgs(Exception):
|
||||
"""More than one of the same argument type was passed."""
|
||||
def __init__(self, param, dupes):
|
||||
msg = _('Duplicate "%(param)s" arguments: %(dupes)s') % {
|
||||
'param': param, 'dupes': ", ".join(dupes)}
|
||||
super(DuplicateArgs, self).__init__(msg)
|
||||
|
||||
|
||||
def validate_args(fn, *args, **kwargs):
|
||||
"""Check that the supplied args are sufficient for calling a function.
|
||||
|
||||
|
@ -82,6 +100,13 @@ def validate_args(fn, *args, **kwargs):
|
|||
raise MissingArgs(missing)
|
||||
|
||||
|
||||
def validate_name_args(positional_name, optional_name):
|
||||
if optional_name:
|
||||
print(NAME_DEPRECATION_WARNING)
|
||||
if positional_name and optional_name:
|
||||
raise DuplicateArgs("<name>", (positional_name, optional_name))
|
||||
|
||||
|
||||
def deprecated(message):
|
||||
"""Decorator for marking a call as deprecated by printing a given message.
|
||||
|
||||
|
|
|
@ -662,7 +662,11 @@ class OpenStackMagnumShell(object):
|
|||
)
|
||||
|
||||
self._check_deprecation(args.func, argv)
|
||||
args.func(self.cs, args)
|
||||
try:
|
||||
args.func(self.cs, args)
|
||||
except (cliutils.DuplicateArgs, cliutils.MissingArgs):
|
||||
self.do_help(args)
|
||||
raise
|
||||
|
||||
if profiler and args.profile:
|
||||
trace_id = profiler.get().get_base_id()
|
||||
|
|
|
@ -52,8 +52,9 @@ class TestCommandLineArgument(utils.TestCase):
|
|||
]
|
||||
|
||||
_deprecated_warning = [
|
||||
'.*(WARNING: The \-\-[a-z\-]*-id parameter is deprecated)+',
|
||||
'.*(Use the \-\-[a-z\-]* parameter to avoid seeing this message)+'
|
||||
'.*(WARNING: The \-\-[a-z\-]* parameter is deprecated)+',
|
||||
('.*(Use the [\<\-a-z\-\>]* (positional )*parameter to avoid seeing '
|
||||
'this message)+')
|
||||
]
|
||||
|
||||
_few_argument_error = [
|
||||
|
|
|
@ -133,7 +133,7 @@ class ShellTest(shell_test_base.TestCommandLineArgument):
|
|||
mock_ct = mock.MagicMock()
|
||||
mock_ct.uuid = 'xxx'
|
||||
mock_get.return_value = mock_ct
|
||||
self._test_arg_success('cluster-create --name test '
|
||||
self._test_arg_success('cluster-create test '
|
||||
'--cluster-template xxx '
|
||||
'--node-count 123 --timeout 15')
|
||||
expected_args = self._get_expected_args_create('xxx', name='test',
|
||||
|
@ -151,7 +151,7 @@ class ShellTest(shell_test_base.TestCommandLineArgument):
|
|||
keypair='x')
|
||||
mock_create.assert_called_with(**expected_args)
|
||||
|
||||
self._test_arg_success('cluster-create --name test '
|
||||
self._test_arg_success('cluster-create test '
|
||||
'--cluster-template xxx')
|
||||
expected_args = self._get_expected_args_create('xxx',
|
||||
name='test')
|
||||
|
@ -178,13 +178,20 @@ class ShellTest(shell_test_base.TestCommandLineArgument):
|
|||
|
||||
@mock.patch('magnumclient.v1.cluster_templates.ClusterTemplateManager.get')
|
||||
@mock.patch('magnumclient.v1.clusters.ClusterManager.create')
|
||||
def test_cluster_create_deprecation_warnings(self, mock_create, mock_get):
|
||||
def test_cluster_create_deprecation_warnings(self, mock_create,
|
||||
mock_get):
|
||||
self._test_arg_failure('cluster-create --cluster-template xxx '
|
||||
'--keypair-id x',
|
||||
self._deprecated_warning)
|
||||
self.assertTrue(mock_create.called)
|
||||
self.assertTrue(mock_get.called)
|
||||
|
||||
self._test_arg_failure('cluster-create --cluster-template xxx '
|
||||
'--name foo ',
|
||||
self._deprecated_warning)
|
||||
self.assertTrue(mock_create.called)
|
||||
self.assertTrue(mock_get.called)
|
||||
|
||||
@mock.patch('magnumclient.v1.cluster_templates.ClusterTemplateManager.get')
|
||||
@mock.patch('magnumclient.v1.clusters.ClusterManager.create')
|
||||
def test_cluster_create_deprecation_errors(self,
|
||||
|
@ -215,15 +222,41 @@ class ShellTest(shell_test_base.TestCommandLineArgument):
|
|||
|
||||
@mock.patch('magnumclient.v1.cluster_templates.ClusterTemplateManager.get')
|
||||
@mock.patch('magnumclient.v1.clusters.ClusterManager.create')
|
||||
def test_cluster_create_success_only_clustertemplate_arg(self,
|
||||
mock_create,
|
||||
mock_get):
|
||||
def _test_cluster_create_success(self, cmd, expected_args, expected_kwargs,
|
||||
mock_create, mock_get):
|
||||
mock_ct = mock.MagicMock()
|
||||
mock_ct.uuid = 'xxx'
|
||||
mock_get.return_value = mock_ct
|
||||
self._test_arg_success('cluster-create --cluster-template xxx')
|
||||
expected_args = self._get_expected_args_create('xxx')
|
||||
mock_create.assert_called_with(**expected_args)
|
||||
self._test_arg_success(cmd)
|
||||
expected = self._get_expected_args_create(*expected_args,
|
||||
**expected_kwargs)
|
||||
mock_create.assert_called_with(**expected)
|
||||
|
||||
def test_cluster_create_success_only_clustertemplate_arg(self):
|
||||
self._test_cluster_create_success(
|
||||
'cluster-create --cluster-template xxx',
|
||||
['xxx'],
|
||||
{})
|
||||
|
||||
@mock.patch('magnumclient.v1.cluster_templates.ClusterTemplateManager.get')
|
||||
@mock.patch('magnumclient.v1.clusters.ClusterManager.create')
|
||||
def test_cluster_create_success_only_positional_name(self,
|
||||
mock_create,
|
||||
mock_get):
|
||||
self._test_cluster_create_success(
|
||||
'cluster-create foo --cluster-template xxx',
|
||||
['xxx'],
|
||||
{'name': 'foo'})
|
||||
|
||||
@mock.patch('magnumclient.v1.cluster_templates.ClusterTemplateManager.get')
|
||||
@mock.patch('magnumclient.v1.clusters.ClusterManager.create')
|
||||
def test_cluster_create_success_only_optional_name(self,
|
||||
mock_create,
|
||||
mock_get):
|
||||
self._test_cluster_create_success(
|
||||
'cluster-create --name foo --cluster-template xxx',
|
||||
['xxx'],
|
||||
{'name': 'foo'})
|
||||
|
||||
@mock.patch('magnumclient.v1.clusters.ClusterManager.create')
|
||||
def test_cluster_create_failure_only_name(self, mock_create):
|
||||
|
@ -270,6 +303,12 @@ class ShellTest(shell_test_base.TestCommandLineArgument):
|
|||
self._invalid_value_error)
|
||||
mock_create.assert_not_called()
|
||||
|
||||
@mock.patch('magnumclient.v1.clusters.ClusterManager.create')
|
||||
def test_cluster_create_failure_duplicate_name(self, mock_create):
|
||||
self._test_arg_failure('cluster-create foo --name bar',
|
||||
self._mandatory_arg_error)
|
||||
mock_create.assert_not_called()
|
||||
|
||||
@mock.patch('magnumclient.v1.clusters.ClusterManager.delete')
|
||||
def test_cluster_delete_success(self, mock_delete):
|
||||
self._test_arg_success('cluster-delete xxx')
|
||||
|
|
|
@ -462,6 +462,33 @@ class ShellTest(shell_test_base.TestCommandLineArgument):
|
|||
labels={'key1': 'val1', 'key2': 'val2'})
|
||||
mock_create.assert_called_with(**expected_args)
|
||||
|
||||
@mock.patch(
|
||||
'magnumclient.v1.cluster_templates.ClusterTemplateManager.create')
|
||||
def test_cluster_template_create_success_only_positional_name(self,
|
||||
mock_create):
|
||||
self._test_arg_success('cluster-template-create '
|
||||
'test '
|
||||
'--labels key1=val1,key2=val2 '
|
||||
'--keypair-id test_keypair '
|
||||
'--external-network-id test_net '
|
||||
'--image-id test_image '
|
||||
'--coe swarm '
|
||||
'--server-type vm')
|
||||
expected_args = \
|
||||
self._get_expected_args(name='test', image_id='test_image',
|
||||
keypair_id='test_keypair', coe='swarm',
|
||||
external_network_id='test_net',
|
||||
server_type='vm',
|
||||
labels={'key1': 'val1', 'key2': 'val2'})
|
||||
mock_create.assert_called_with(**expected_args)
|
||||
|
||||
@mock.patch(
|
||||
'magnumclient.v1.cluster_templates.ClusterTemplateManager.create')
|
||||
def test_cluster_template_create_failure_duplicate_name(self, mock_create):
|
||||
self._test_arg_failure('cluster-template-create '
|
||||
'foo --name test', self._mandatory_arg_error)
|
||||
mock_create.assert_not_called()
|
||||
|
||||
@mock.patch(
|
||||
'magnumclient.v1.cluster_templates.ClusterTemplateManager.create')
|
||||
def test_cluster_template_create_failure_few_arg(self, mock_create):
|
||||
|
@ -598,6 +625,14 @@ class ShellTest(shell_test_base.TestCommandLineArgument):
|
|||
external_network_id='public')
|
||||
mock_create.assert_called_with(**expected_args)
|
||||
|
||||
self._test_arg_failure(required_args + '--name foo',
|
||||
self._deprecated_warning)
|
||||
expected_args = \
|
||||
self._get_expected_args(image_id='test', coe='test',
|
||||
name='foo',
|
||||
external_network_id='public')
|
||||
mock_create.assert_called_with(**expected_args)
|
||||
|
||||
@mock.patch('magnumclient.v1.cluster_templates.ClusterTemplateManager.get')
|
||||
def test_cluster_template_show_success(self, mock_show):
|
||||
self._test_arg_success('cluster-template-show xxx')
|
||||
|
|
|
@ -33,9 +33,16 @@ def _show_cluster_template(cluster_template):
|
|||
|
||||
|
||||
@utils.deprecation_map(DEPRECATING_PARAMS)
|
||||
@utils.arg('positional_name',
|
||||
metavar='<name>',
|
||||
nargs='?',
|
||||
default=None,
|
||||
help=_('Name of the cluster template to create.'))
|
||||
@utils.arg('--name',
|
||||
metavar='<name>',
|
||||
help=_('Name of the cluster template to create.'))
|
||||
default=None,
|
||||
help=(_('Name of the cluster template to create. %s') %
|
||||
utils.NAME_DEPRECATION_HELP))
|
||||
@utils.arg('--image-id',
|
||||
dest='image',
|
||||
required=True,
|
||||
|
@ -169,8 +176,12 @@ def _show_cluster_template(cluster_template):
|
|||
'floating ip or not.'))
|
||||
def do_cluster_template_create(cs, args):
|
||||
"""Create a cluster template."""
|
||||
args.command = 'cluster-template-create'
|
||||
|
||||
utils.validate_name_args(args.positional_name, args.name)
|
||||
|
||||
opts = {}
|
||||
opts['name'] = args.name
|
||||
opts['name'] = args.positional_name or args.name
|
||||
opts['flavor_id'] = args.flavor
|
||||
opts['master_flavor_id'] = args.master_flavor
|
||||
opts['image_id'] = args.image
|
||||
|
|
|
@ -82,9 +82,16 @@ def do_cluster_list(cs, args):
|
|||
|
||||
|
||||
@utils.deprecation_map(DEPRECATING_PARAMS)
|
||||
@utils.arg('positional_name',
|
||||
metavar='<name>',
|
||||
nargs='?',
|
||||
default=None,
|
||||
help=_('Name of the cluster to create.'))
|
||||
@utils.arg('--name',
|
||||
metavar='<name>',
|
||||
help=_('Name of the cluster to create.'))
|
||||
default=None,
|
||||
help=(_('Name of the cluster to create. %s') %
|
||||
utils.NAME_DEPRECATION_HELP))
|
||||
@utils.arg('--cluster-template',
|
||||
required=True,
|
||||
metavar='<cluster_template>',
|
||||
|
@ -122,10 +129,13 @@ def do_cluster_list(cs, args):
|
|||
'is 60 minutes.'))
|
||||
def do_cluster_create(cs, args):
|
||||
"""Create a cluster."""
|
||||
args.command = 'cluster-create'
|
||||
|
||||
utils.validate_name_args(args.positional_name, args.name)
|
||||
|
||||
cluster_template = cs.cluster_templates.get(args.cluster_template)
|
||||
opts = dict()
|
||||
opts['name'] = args.name
|
||||
opts['name'] = args.positional_name or args.name
|
||||
opts['cluster_template_id'] = cluster_template.uuid
|
||||
opts['keypair'] = args.keypair
|
||||
opts['node_count'] = args.node_count
|
||||
|
|
Loading…
Reference in New Issue