diff --git a/magnumclient/common/cliutils.py b/magnumclient/common/cliutils.py index d2876433..8db1cb1d 100644 --- a/magnumclient/common/cliutils.py +++ b/magnumclient/common/cliutils.py @@ -35,14 +35,20 @@ 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 ' - ' positional parameter %s.') +DEPRECATION_BASE = ('%sThe --%s parameter is deprecated and ' + 'will be removed in a future release. Use the ' + '<%s> positional parameter %s.') -NAME_DEPRECATION_HELP = NAME_DEPRECATION_BASE % ('', 'instead') +NAME_DEPRECATION_HELP = DEPRECATION_BASE % ('', 'name', 'name', 'instead') -NAME_DEPRECATION_WARNING = NAME_DEPRECATION_BASE % ( - 'WARNING: ', 'to avoid seeing this message') +NAME_DEPRECATION_WARNING = DEPRECATION_BASE % ( + 'WARNING: ', 'name', 'name', 'to avoid seeing this message') + +CLUSTER_DEPRECATION_HELP = DEPRECATION_BASE % ('', 'cluster', 'cluster', + 'instead') + +CLUSTER_DEPRECATION_WARNING = DEPRECATION_BASE % ( + 'WARNING: ', 'cluster', 'cluster', 'to avoid seeing this message') def deprecation_message(preamble, new_name): @@ -107,6 +113,14 @@ def validate_name_args(positional_name, optional_name): raise DuplicateArgs("", (positional_name, optional_name)) +def validate_cluster_args(positional_cluster, optional_cluster): + if optional_cluster: + print(CLUSTER_DEPRECATION_WARNING) + if positional_cluster and optional_cluster: + raise DuplicateArgs("", (positional_cluster, + optional_cluster)) + + def deprecated(message): """Decorator for marking a call as deprecated by printing a given message. diff --git a/magnumclient/tests/v1/shell_test_base.py b/magnumclient/tests/v1/shell_test_base.py index d221518f..89f6195c 100644 --- a/magnumclient/tests/v1/shell_test_base.py +++ b/magnumclient/tests/v1/shell_test_base.py @@ -51,9 +51,9 @@ class TestCommandLineArgument(utils.TestCase): ".*?^Try 'magnum help ", ] - _duplicate_name_arg_error = [ + _duplicate_arg_error = [ '.*?^usage: ', - '.*?^error: (Duplicate "" arguments:)', + '.*?^error: (Duplicate "<.*>" arguments:)', ".*?^Try 'magnum help ", ] diff --git a/magnumclient/tests/v1/test_certificates_shell.py b/magnumclient/tests/v1/test_certificates_shell.py index 2599f4ca..806130b9 100644 --- a/magnumclient/tests/v1/test_certificates_shell.py +++ b/magnumclient/tests/v1/test_certificates_shell.py @@ -14,7 +14,7 @@ import mock -from magnumclient.common import cliutils as utils +from magnumclient.common import cliutils from magnumclient.tests.v1 import shell_test_base from magnumclient.v1 import certificates_shell @@ -28,8 +28,7 @@ class ShellTest(shell_test_base.TestCommandLineArgument): mockbay.status = "CREATE_COMPLETE" mockbay.uuid = "xxx" mock_bay_get.return_value = mockbay - self._test_arg_success('ca-show ' - '--bay xxx') + self._test_arg_success('ca-show --bay xxx') expected_args = {} expected_args['cluster_uuid'] = mockbay.uuid mock_cert_get.assert_called_once_with(**expected_args) @@ -41,12 +40,48 @@ class ShellTest(shell_test_base.TestCommandLineArgument): mockcluster.status = "CREATE_COMPLETE" mockcluster.uuid = "xxx" mock_cluster_get.return_value = mockcluster - self._test_arg_success('ca-show ' - '--cluster xxx') + self._test_arg_success('ca-show xxx') expected_args = {} expected_args['cluster_uuid'] = mockcluster.uuid mock_cert_get.assert_called_once_with(**expected_args) + @mock.patch('magnumclient.v1.clusters.ClusterManager.get') + @mock.patch('magnumclient.v1.certificates.CertificateManager.get') + def test_positional_cluster_bay_ca_show_success(self, mock_cert_get, + mock_cluster_get): + mockcluster = mock.MagicMock() + mockcluster.status = "CREATE_COMPLETE" + mockcluster.uuid = "xxx" + mock_cluster_get.return_value = mockcluster + self._test_arg_success('ca-show xxx --bay not-found') + expected_args = {} + expected_args['cluster_uuid'] = mockcluster.uuid + mock_cert_get.assert_called_once_with(**expected_args) + + @mock.patch('magnumclient.v1.clusters.ClusterManager.get') + @mock.patch('magnumclient.v1.certificates.CertificateManager.get') + def test_cluster_bay_ca_show_success(self, mock_cert_get, + mock_cluster_get): + mockcluster = mock.MagicMock() + mockcluster.status = "CREATE_COMPLETE" + mockcluster.uuid = "xxx" + mock_cluster_get.return_value = mockcluster + self._test_arg_success('ca-show --cluster xxx --bay not-found') + expected_args = {} + expected_args['cluster_uuid'] = mockcluster.uuid + mock_cert_get.assert_called_once_with(**expected_args) + + @mock.patch('magnumclient.v1.clusters.ClusterManager.get') + @mock.patch('magnumclient.v1.certificates.CertificateManager.get') + def test_ca_show_failure_duplicate_arg(self, mock_cert_get, + mock_cluster_get): + self.assertRaises(cliutils.DuplicateArgs, + self._test_arg_failure, + 'ca-show foo --cluster foo', + self._duplicate_arg_error) + mock_cert_get.assert_not_called() + mock_cluster_get.assert_not_called() + @mock.patch('os.path.isfile') @mock.patch('magnumclient.v1.bays.BayManager.get') @mock.patch('magnumclient.v1.certificates.CertificateManager.create') @@ -135,11 +170,10 @@ class ShellTest(shell_test_base.TestCommandLineArgument): @mock.patch('magnumclient.v1.certificates.CertificateManager.get') def test_ca_show_failure_with_invalid_field(self, mock_cert_get, mock_cluster_get): - _error_msg = [".*?^--cluster or --bay"] - self.assertRaises(utils.MissingArgs, + self.assertRaises(cliutils.MissingArgs, self._test_arg_failure, 'ca-show', - _error_msg) + self._few_argument_error) mock_cert_get.assert_not_called() mock_cluster_get.assert_not_called() diff --git a/magnumclient/tests/v1/test_clusters_shell.py b/magnumclient/tests/v1/test_clusters_shell.py index 214670ae..c2ad7395 100644 --- a/magnumclient/tests/v1/test_clusters_shell.py +++ b/magnumclient/tests/v1/test_clusters_shell.py @@ -320,7 +320,7 @@ class ShellTest(shell_test_base.TestCommandLineArgument): self._test_arg_failure, 'cluster-create foo --name bar ' '--cluster-template xxx', - self._duplicate_name_arg_error) + self._duplicate_arg_error) mock_create.assert_not_called() @mock.patch('magnumclient.v1.clusters.ClusterManager.delete') diff --git a/magnumclient/v1/certificates_shell.py b/magnumclient/v1/certificates_shell.py index 078a3a87..f9530323 100644 --- a/magnumclient/v1/certificates_shell.py +++ b/magnumclient/v1/certificates_shell.py @@ -43,12 +43,20 @@ def _get_target_uuid(cs, args): required=False, metavar='', help=_('ID or name of the bay.')) -@utils.arg('--cluster', - required=False, +@utils.arg('postional_cluster', metavar='', + nargs='?', + default=None, help=_('ID or name of the cluster.')) +@utils.arg('--cluster', + metavar='', + default=None, + help=(_('ID or name of the cluster. %s') % + utils.CLUSTER_DEPRECATION_HELP)) def do_ca_show(cs, args): """Show details about the CA certificate for a bay or cluster.""" + utils.validate_cluster_args(args.postional_cluster, args.cluster) + args.cluster = args.postional_cluster or args.cluster opts = { 'cluster_uuid': _get_target_uuid(cs, args) }