From 50a4592efb39902b994470a229dff7b37810727b Mon Sep 17 00:00:00 2001
From: czl389 <czl389@163.com>
Date: Sun, 6 Sep 2020 00:48:32 +0800
Subject: [PATCH] Fix cinder-manage cluster remove raising NoSuchOptError

When executing 'cinder-manage cluster remove' script, an error occurs in
the call 'fetch_func_args(fn)':
oslo_config.cfg.NoSuchOptError:no such option cluster_name in group
[DEFAULT].

The cause is that the call 'fetch_func_args(fn)' is trying to fetch
value of argument 'cluster_name' of fn from CONF.cluster_name, but
corresponding argument registered in CONF is 'cluster-name' actually.

We fix this bug by changing argument name registered in CONF, if
positional argument contains '-':
1.put it into automatic conversion from '- to '_', so that keep it the
 same name as handler function argument.
2.add metavar if it doesn't already exist, and it will maintain the help
 output.

Change-Id: I3f2a04b4c8e7bafe6e38fc86a69192dd3a94c0c5
Closes-Bug: #1894381
---
 cinder/cmd/manage.py                          |  5 +++
 cinder/tests/unit/test_cmd.py                 | 37 +++++++++++++++++++
 ...cluster-remove-raising-nosuchopterror.yaml |  6 +++
 3 files changed, 48 insertions(+)
 create mode 100644 releasenotes/notes/bug-1894381-fix-cinder-manage-cluster-remove-raising-nosuchopterror.yaml

diff --git a/cinder/cmd/manage.py b/cinder/cmd/manage.py
index d8de35084a5..2a9cc0081fa 100644
--- a/cinder/cmd/manage.py
+++ b/cinder/cmd/manage.py
@@ -122,6 +122,11 @@ def _get_non_shared_target_hosts(ctxt):
 
 # Decorators for actions
 def args(*args, **kwargs):
+    args = list(args)
+    if not args[0].startswith('-') and '-' in args[0]:
+        kwargs.setdefault('metavar', args[0])
+        args[0] = args[0].replace('-', '_')
+
     def _decorator(func):
         func.__dict__.setdefault('args', []).insert(0, (args, kwargs))
         return func
diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py
index 49665ea601a..b9f7fe55700 100644
--- a/cinder/tests/unit/test_cmd.py
+++ b/cinder/tests/unit/test_cmd.py
@@ -951,6 +951,19 @@ class TestCinderManageCmd(test.TestCase):
             self.assertDictEqual(expected,
                                  cinder_manage.fetch_func_args(my_func))
 
+    def test_args_decorator(self):
+        @cinder_manage.args('host-name')
+        @cinder_manage.args('cluster-name', metavar='cluster')
+        @cinder_manage.args('--debug')
+        def my_func():
+            pass
+
+        expected = [
+            (['host_name'], {'metavar': 'host-name'}),
+            (['cluster_name'], {'metavar': 'cluster'}),
+            (['--debug'], {})]
+        self.assertEqual(expected, my_func.args)
+
     @mock.patch('cinder.context.get_admin_context')
     @mock.patch('cinder.db.cluster_get_all')
     def tests_cluster_commands_list(self, get_all_mock, get_admin_mock,
@@ -1108,6 +1121,30 @@ class TestCinderManageCmd(test.TestCase):
         exit = cluster_commands.rename(False, 'cluster', 'new_cluster')
         self.assertEqual(2, exit)
 
+    @mock.patch('cinder.objects.Cluster.get_by_id')
+    @mock.patch('cinder.context.get_admin_context')
+    def test_main_remove_cluster(self, get_admin_mock, get_cluster_mock):
+        script_name = 'cinder-manage'
+        sys.argv = [script_name, 'cluster', 'remove', 'abinary', 'acluster']
+
+        cinder_manage.CONF = cfg.ConfigOpts()
+        cinder_manage.main()
+
+        expected_argument = (['cluster_name'],
+                             {'type': str,
+                              'help': 'Cluster to delete.',
+                              'metavar': 'cluster-name'})
+        self.assertTrue(expected_argument in
+                        cinder_manage.CONF.category.action_fn.args)
+        self.assertTrue(hasattr(cinder_manage.CONF.category, 'cluster_name'))
+        get_admin_mock.assert_called_with()
+        get_cluster_mock.assert_called_with(get_admin_mock.return_value,
+                                            None, name='acluster',
+                                            binary='abinary',
+                                            get_services=False)
+        cluster = get_cluster_mock.return_value
+        cluster.destroy.assert_called()
+
     @mock.patch('oslo_config.cfg.ConfigOpts.register_cli_opt')
     def test_main_argv_lt_2(self, register_cli_opt):
         script_name = 'cinder-manage'
diff --git a/releasenotes/notes/bug-1894381-fix-cinder-manage-cluster-remove-raising-nosuchopterror.yaml b/releasenotes/notes/bug-1894381-fix-cinder-manage-cluster-remove-raising-nosuchopterror.yaml
new file mode 100644
index 00000000000..d9b75d4dc1b
--- /dev/null
+++ b/releasenotes/notes/bug-1894381-fix-cinder-manage-cluster-remove-raising-nosuchopterror.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    `Bug #1894381 <https://bugs.launchpad.net/cinder/+bug/1894381>`_:
+    Fix the bug that cinder-manage cluster remove
+    does not work and an error NoSuchOptError occurs.
\ No newline at end of file