mgr/dashboard: upgrading nvmeof doesn't update configuration

Happens from 19.2.0 to any of the latest upgrade

During upgrade I get
```
Failed to set Dashboard config for nvmeof: dashboard nvmeof-gateway-add failed: JSON array/object not allowed {"prefix": "dashboard nvmeof-gateway-add", "name": "nvmeof.rbd", "group": null, "daemon_name": "nvmeof.rbd.ceph-node-01.irpssg"} retval: -22
```

which is fixed by handling the group_name when its not there in spec.

And the other error was
```
Failed to set Dashboard config for nvmeof: dashboard nvmeof-gateway-add failed: Traceback (most recent call last): File "/usr/share/ceph/mgr/mgr_module.py", line 1864, in _handle_command return CLICommand.COMMANDS[cmd['prefix']].call(self, cmd, inbuf) File "/usr/share/ceph/mgr/mgr_module.py", line 499, in call return self.func(mgr, **kwargs) File "/usr/share/ceph/mgr/mgr_module.py", line 535, in check return func(*args, **kwargs) File "/usr/share/ceph/mgr/dashboard/services/nvmeof_cli.py", line 28, in add_nvmeof_gateway NvmeofGatewaysConfig.add_gateway(name, service_url, group, daemon_name) File "/usr/share/ceph/mgr/dashboard/services/nvmeof_conf.py", line 61, in add_gateway gateway['daemon_name'] = daemon_name TypeError: 'str' object does not support item assignment retval: -22
```

which is fixed by properly updating the config to the newer format that
is available in newer version

Fixes: https://tracker.ceph.com/issues/70629
Signed-off-by: Nizamudeen A <nia@redhat.com>
(cherry picked from commit 6e9ac798e354aa37b41708aae099095baec0061d)

 Conflicts:
	src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py
 -  kept test file and only added the relavant test files

diff --git a/src/pybind/mgr/cephadm/services/nvmeof.py b/src/pybind/mgr/cephadm/services/nvmeof.py
index 78789515a4262c6698f0231ea7a1ddd48ac76f8f..f3eb623d61a2d29bb62f7d94e8c7641e2b5e571e 100644 (file)
--- a/src/pybind/mgr/cephadm/services/nvmeof.py
+++ b/src/pybind/mgr/cephadm/services/nvmeof.py
@@ -105,7 +105,7 @@ class NvmeofService(CephService):
                         'prefix': 'dashboard nvmeof-gateway-add',
                         'inbuf': service_url,
                         'name': service_name,
-                        'group': spec.group,
+                        'group': spec.group if spec.group else '',
                         'daemon_name': dd.name()
                     })
             return cmd_dicts

diff --git a/src/pybind/mgr/dashboard/services/nvmeof_conf.py b/src/pybind/mgr/dashboard/services/nvmeof_conf.py
index 170f98c70d18f2e69c888b40f32cb1be098381f2..6b89c586e99f334b05b577dc75bef3b6c983021c 100644 (file)
--- a/src/pybind/mgr/dashboard/services/nvmeof_conf.py
+++ b/src/pybind/mgr/dashboard/services/nvmeof_conf.py
@@ -55,13 +55,19 @@ class NvmeofGatewaysConfig(object):
         config = cls.get_gateways_config()
 
         if name in config.get('gateways', {}):
-            existing_gateways = config['gateways'][name]
-            for gateway in existing_gateways:
-                if 'daemon_name' not in gateway:
-                    gateway['daemon_name'] = daemon_name
-                    break
-                if gateway['service_url'] == service_url:
-                    return
+            # the nvmeof dashboard config used in v19.2.0 saves the below
+            # to a dict. Converting that to a list so that the upgrade
+            # properly migrate it to the newer format, and also keep it empty.
+            if isinstance(config['gateways'][name], dict):
+                config['gateways'][name] = []
+            else:
+                existing_gateways = config['gateways'][name]
+                for gateway in existing_gateways:
+                    if 'daemon_name' not in gateway:
+                        gateway['daemon_name'] = daemon_name
+                        break
+                    if gateway['service_url'] == service_url:
+                        return
 
         new_gateway = {
             'service_url': service_url,

diff --git a/src/pybind/mgr/dashboard/tests/__init__.py b/src/pybind/mgr/dashboard/tests/__init__.py
index 3061fe9dc93167ec2e126f383bd520825640540f..41dc300e9bcdc667ac84529f5430dd6b1f82f9f6 100644 (file)
--- a/src/pybind/mgr/dashboard/tests/__init__.py
+++ b/src/pybind/mgr/dashboard/tests/__init__.py
@@ -73,6 +73,10 @@ class KVStoreMockMixin(object):
     def get_key(cls, key):
         return cls.CONFIG_KEY_DICT.get(key, None)
 
+    @classmethod
+    def set_key(cls, key, value):
+        cls.CONFIG_KEY_DICT[key] = value
+
 
 # pylint: disable=protected-access
 class CLICommandTestMixin(KVStoreMockMixin):

diff --git a/src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py b/src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py
new file mode 100644 (file)
index 0000000..901bdbd
--- /dev/null
+++ b/src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py
@@ -0,0 +1,177 @@
+import json
+import unittest
+
+from ..tests import CLICommandTestMixin
+
+
+class TestNVMeoFConfCLI(unittest.TestCase, CLICommandTestMixin):
+    def setUp(self):
+        self.mock_kv_store()
+
+    def test_cli_add_gateway(self):
+
+        self.exec_cmd(
+            'nvmeof-gateway-add',
+            name='nvmeof.pool.group',
+            inbuf='http://nvmf:port',
+            daemon_name='nvmeof_daemon',
+            group='group'
+        )
+
+        config = json.loads(self.get_key('_nvmeof_config'))
+        self.assertEqual(
+            config['gateways'], {
+                'nvmeof.pool.group': [{
+                    'group': 'group',
+                    'daemon_name': 'nvmeof_daemon',
+                    'service_url': 'http://nvmf:port'
+                }]
+            }
+        )
+
+    def test_cli_migration_from_legacy_config(self):
+        legacy_config = json.dumps({
+            'gateways': {
+                'nvmeof.pool': {
+                    'service_url': 'http://nvmf:port'
+                }
+            }
+        })
+        self.set_key('_nvmeof_config', legacy_config)
+
+        self.exec_cmd(
+            'nvmeof-gateway-add',
+            name='nvmeof.pool',
+            inbuf='http://nvmf:port',
+            daemon_name='nvmeof_daemon',
+            group=''
+        )
+
+        config = json.loads(self.get_key('_nvmeof_config'))
+        self.assertEqual(
+            config['gateways'], {
+                'nvmeof.pool': [{
+                    'daemon_name': 'nvmeof_daemon',
+                    'group': '',
+                    'service_url': 'http://nvmf:port'
+                }]
+            }
+        )
+
+    def test_cli_add_gw_to_existing(self):
+        # add first gw
+        self.exec_cmd(
+            'nvmeof-gateway-add',
+            name='nvmeof.pool',
+            inbuf='http://nvmf:port',
+            daemon_name='nvmeof_daemon',
+            group=''
+        )
+
+        # add another daemon to the first gateway
+        self.exec_cmd(
+            'nvmeof-gateway-add',
+            name='nvmeof.pool',
+            inbuf='http://nvmf-2:port',
+            daemon_name='nvmeof_daemon_2',
+            group=''
+        )
+
+        config = json.loads(self.get_key('_nvmeof_config'))
+
+        # make sure its appended to the existing gateway
+        self.assertEqual(
+            config['gateways'], {
+                'nvmeof.pool': [{
+                    'daemon_name': 'nvmeof_daemon',
+                    'group': '',
+                    'service_url': 'http://nvmf:port'
+                }, {
+                    'daemon_name': 'nvmeof_daemon_2',
+                    'group': '',
+                    'service_url': 'http://nvmf-2:port'
+                }]
+            }
+        )
+
+    def test_cli_add_new_gw(self):
+        # add first config
+        self.exec_cmd(
+            'nvmeof-gateway-add',
+            name='nvmeof.pool',
+            inbuf='http://nvmf:port',
+            daemon_name='nvmeof_daemon',
+            group=''
+        )
+
+        # add another gateway
+        self.exec_cmd(
+            'nvmeof-gateway-add',
+            name='nvmeof2.pool.group',
+            inbuf='http://nvmf-2:port',
+            daemon_name='nvmeof_daemon_2',
+            group='group'
+        )
+
+        config = json.loads(self.get_key('_nvmeof_config'))
+
+        # make sure its added as a new entry
+        self.assertEqual(
+            config['gateways'], {
+                'nvmeof.pool': [{
+                    'daemon_name': 'nvmeof_daemon',
+                    'group': '',
+                    'service_url': 'http://nvmf:port'
+                }],
+                'nvmeof2.pool.group': [{
+                    'daemon_name': 'nvmeof_daemon_2',
+                    'group': 'group',
+                    'service_url': 'http://nvmf-2:port'
+                }]
+            }
+        )
+
+    def test_cli_rm_gateway(self):
+        self.test_cli_add_gateway()
+        self.exec_cmd('nvmeof-gateway-rm', name='nvmeof.pool.group')
+
+        config = json.loads(self.get_key('_nvmeof_config'))
+        self.assertEqual(
+            config['gateways'], {}
+        )
+
+    def test_cli_rm_daemon_from_gateway(self):
+        self.test_cli_add_gw_to_existing()
+        self.exec_cmd(
+            'nvmeof-gateway-rm',
+            name='nvmeof.pool',
+            daemon_name='nvmeof_daemon'
+        )
+
+        config = json.loads(self.get_key('_nvmeof_config'))
+        self.assertEqual(
+            config['gateways'], {
+                'nvmeof.pool': [{
+                    'daemon_name': 'nvmeof_daemon_2',
+                    'group': '',
+                    'service_url': 'http://nvmf-2:port'
+                }]
+            }
+        )
+
+    def test_cli_legacy_config_rm(self):
+        legacy_config = json.dumps({
+            'gateways': {
+                'nvmeof.pool': {
+                    'service_url': 'http://nvmf:port'
+                }
+            }
+        })
+        self.set_key('_nvmeof_config', legacy_config)
+
+        self.exec_cmd('nvmeof-gateway-rm', name='nvmeof.pool')
+
+        config = json.loads(self.get_key('_nvmeof_config'))
+        self.assertEqual(
+            config['gateways'], {}
+        )