Merge pull request ceph#62457 from rhcs-dashboard/nvmeof-upgrade-bug

mgr/dashboard: upgrading nvmeof doesn't update configuration

Author: nizamial09

src/pybind/mgr/cephadm/services/nvmeof.py

@@ -171,7 +171,7 @@ def get_set_cmd_dicts(out: str) -> List[dict]:
                         '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

src/pybind/mgr/dashboard/services/nvmeof_conf.py

@@ -55,13 +55,19 @@ def add_gateway(cls, name, service_url, group, daemon_name):
         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,

src/pybind/mgr/dashboard/tests/__init__.py

@@ -73,6 +73,10 @@ def mock_kv_store(cls):
     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):

src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py

@@ -1,10 +1,13 @@
 import errno
+import json
+import unittest
 from unittest.mock import MagicMock
 
 import pytest
 from mgr_module import CLICommand, HandleCommandResult
 
 from ..services.nvmeof_cli import NvmeofCLICommand
+from ..tests import CLICommandTestMixin
 
 
 @pytest.fixture(scope="class", name="sample_command")
@@ -85,3 +88,176 @@ def test_command_return_empty_cmd_result(self, base_call_return_none_mock, sampl
         assert result.stdout == ''
         assert result.stderr == ''
         base_call_return_none_mock.assert_called_once()
+
+
+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'], {}
+        )