mgr/dashboard: Allow editing iSCSI targets with initiators logged-in

Fixes: https://tracker.ceph.com/issues/47393
Signed-off-by: Tiago Melo <tmelo@suse.com>

diff --git a/src/pybind/mgr/dashboard/controllers/iscsi.py b/src/pybind/mgr/dashboard/controllers/iscsi.py
index cb5e657cbb5e2b5c6dc8b59ed6bafb13118d72de..ce81768072ae578423eb4c0aea7d3edf8da188fa 100644 (file)
--- a/src/pybind/mgr/dashboard/controllers/iscsi.py
+++ b/src/pybind/mgr/dashboard/controllers/iscsi.py
@@ -384,35 +384,48 @@ class IscsiTarget(RESTController):
         deleted_groups = []
         for group_id in list(target_config['groups'].keys()):
             if IscsiTarget._group_deletion_required(target, new_target_iqn, new_target_controls,
-                                                    new_groups, group_id, new_clients,
-                                                    new_disks):
+                                                    new_groups, group_id):
                 deleted_groups.append(group_id)
                 IscsiClient.instance(gateway_name=gateway_name).delete_group(target_iqn,
                                                                              group_id)
+            else:
+                group = IscsiTarget._get_group(new_groups, group_id)
+
+                old_group_disks = set(target_config['groups'][group_id]['disks'].keys())
+                new_group_disks = {'{}/{}'.format(x['pool'], x['image']) for x in group['disks']}
+                local_deleted_disks = list(old_group_disks - new_group_disks)
+
+                old_group_members = set(target_config['groups'][group_id]['members'])
+                new_group_members = set(group['members'])
+                local_deleted_members = list(old_group_members - new_group_members)
+
+                if local_deleted_disks or local_deleted_members:
+                    IscsiClient.instance(gateway_name=gateway_name).update_group(
+                        target_iqn, group_id, local_deleted_members, local_deleted_disks)
             TaskManager.current_task().inc_progress(task_progress_inc)
         deleted_clients = []
         deleted_client_luns = []
         for client_iqn, client_config in target_config['clients'].items():
             if IscsiTarget._client_deletion_required(target, new_target_iqn, new_target_controls,
-                                                     new_clients, client_iqn,
-                                                     deleted_groups):
+                                                     new_clients, client_iqn):
                 deleted_clients.append(client_iqn)
                 IscsiClient.instance(gateway_name=gateway_name).delete_client(target_iqn,
                                                                               client_iqn)
             else:
                 for image_id in list(client_config.get('luns', {}).keys()):
                     if IscsiTarget._client_lun_deletion_required(target, client_iqn, image_id,
-                                                                 new_clients):
+                                                                 new_clients, new_groups):
                         deleted_client_luns.append((client_iqn, image_id))
                         IscsiClient.instance(gateway_name=gateway_name).delete_client_lun(
                             target_iqn, client_iqn, image_id)
             TaskManager.current_task().inc_progress(task_progress_inc)
         for image_id in target_config['disks']:
             if IscsiTarget._target_lun_deletion_required(target, new_target_iqn,
-                                                         new_target_controls,
-                                                         new_disks, image_id):
+                                                         new_target_controls, new_disks, image_id):
                 all_clients = target_config['clients'].keys()
-                not_deleted_clients = [c for c in all_clients if c not in deleted_clients]
+                not_deleted_clients = [c for c in all_clients if c not in deleted_clients
+                                       and not IscsiTarget._client_in_group(target['groups'], c)
+                                       and not IscsiTarget._client_in_group(new_groups, c)]
                 for client_iqn in not_deleted_clients:
                     client_image_ids = target_config['clients'][client_iqn]['luns'].keys()
                     for client_image_id in client_image_ids:
@@ -447,28 +460,12 @@ class IscsiTarget(RESTController):
 
     @staticmethod
     def _group_deletion_required(target, new_target_iqn, new_target_controls,
-                                 new_groups, group_id, new_clients, new_disks):
+                                 new_groups, group_id):
         if IscsiTarget._target_deletion_required(target, new_target_iqn, new_target_controls):
             return True
         new_group = IscsiTarget._get_group(new_groups, group_id)
         if not new_group:
             return True
-        old_group = IscsiTarget._get_group(target['groups'], group_id)
-        if new_group != old_group:
-            return True
-        # Check if any client inside this group has changed
-        for client_iqn in new_group['members']:
-            if IscsiTarget._client_deletion_required(target, new_target_iqn, new_target_controls,
-                                                     new_clients, client_iqn,
-                                                     []):
-                return True
-        # Check if any disk inside this group has changed
-        for disk in new_group['disks']:
-            image_id = '{}/{}'.format(disk['pool'], disk['image'])
-            if IscsiTarget._target_lun_deletion_required(target, new_target_iqn,
-                                                         new_target_controls,
-                                                         new_disks, image_id):
-                return True
         return False
 
     @staticmethod
@@ -480,29 +477,45 @@ class IscsiTarget(RESTController):
 
     @staticmethod
     def _client_deletion_required(target, new_target_iqn, new_target_controls,
-                                  new_clients, client_iqn, deleted_groups):
+                                  new_clients, client_iqn):
         if IscsiTarget._target_deletion_required(target, new_target_iqn, new_target_controls):
             return True
         new_client = IscsiTarget._get_client(new_clients, client_iqn)
         if not new_client:
             return True
-        # Check if client belongs to a groups that has been deleted
-        for group in target['groups']:
-            if group['group_id'] in deleted_groups and client_iqn in group['members']:
+        return False
+
+    @staticmethod
+    def _client_in_group(groups, client_iqn):
+        for group in groups:
+            if client_iqn in group['members']:
                 return True
         return False
 
     @staticmethod
-    def _client_lun_deletion_required(target, client_iqn, image_id, new_clients):
+    def _client_lun_deletion_required(target, client_iqn, image_id, new_clients, new_groups):
         new_client = IscsiTarget._get_client(new_clients, client_iqn)
         if not new_client:
             return True
+
+        # Disks inherited from groups must be considered
+        was_in_group = IscsiTarget._client_in_group(target['groups'], client_iqn)
+        is_in_group = IscsiTarget._client_in_group(new_groups, client_iqn)
+
+        if not was_in_group and is_in_group:
+            return True
+
+        if is_in_group:
+            return False
+
         new_lun = IscsiTarget._get_disk(new_client.get('luns', []), image_id)
         if not new_lun:
             return True
+
         old_client = IscsiTarget._get_client(target['clients'], client_iqn)
         if not old_client:
             return False
+
         old_lun = IscsiTarget._get_disk(old_client.get('luns', []), image_id)
         return new_lun != old_lun
 
@@ -673,15 +686,9 @@ class IscsiTarget(RESTController):
 
         target_config = config['targets'][target_iqn]
         target = IscsiTarget._config_to_target(target_iqn, config)
-        deleted_groups = []
-        for group_id in list(target_config['groups'].keys()):
-            if IscsiTarget._group_deletion_required(target, new_target_iqn, new_target_controls,
-                                                    new_groups, group_id, new_clients,
-                                                    new_disks):
-                deleted_groups.append(group_id)
         for client_iqn in list(target_config['clients'].keys()):
             if IscsiTarget._client_deletion_required(target, new_target_iqn, new_target_controls,
-                                                     new_clients, client_iqn, deleted_groups):
+                                                     new_clients, client_iqn):
                 client_info = IscsiClient.instance(gateway_name=gateway).get_clientinfo(target_iqn,
                                                                                         client_iqn)
                 if client_info.get('state', {}).get('LOGGED_IN', []):
@@ -807,8 +814,15 @@ class IscsiTarget(RESTController):
                     pool = lun['pool']
                     image = lun['image']
                     image_id = '{}/{}'.format(pool, image)
+                    # Disks inherited from groups must be considered
+                    group_disks = []
+                    for group in groups:
+                        if client_iqn in group['members']:
+                            group_disks = ['{}/{}'.format(x['pool'], x['image'])
+                                           for x in group['disks']]
                     if not target_config or client_iqn not in target_config['clients'] or \
-                            image_id not in target_config['clients'][client_iqn]['luns']:
+                            (image_id not in target_config['clients'][client_iqn]['luns']
+                             and image_id not in group_disks):
                         IscsiClient.instance(gateway_name=gateway_name).create_client_lun(
                             target_iqn, client_iqn, image_id)
                 TaskManager.current_task().inc_progress(task_progress_inc)
@@ -818,7 +832,14 @@ class IscsiTarget(RESTController):
                 image_ids = []
                 for disk in group['disks']:
                     image_ids.append('{}/{}'.format(disk['pool'], disk['image']))
-                if not target_config or group_id not in target_config['groups']:
+
+                if target_config and group_id in target_config['groups']:
+                    old_members = target_config['groups'][group_id]['members']
+                    old_disks = target_config['groups'][group_id]['disks'].keys()
+
+                if not target_config or group_id not in target_config['groups'] or \
+                        list(set(group['members']) - set(old_members)) or \
+                        list(set(image_ids) - set(old_disks)):
                     IscsiClient.instance(gateway_name=gateway_name).create_group(
                         target_iqn, group_id, members, image_ids)
                 TaskManager.current_task().inc_progress(task_progress_inc)

diff --git a/src/pybind/mgr/dashboard/services/iscsi_client.py b/src/pybind/mgr/dashboard/services/iscsi_client.py
index b82a51a3d4a68f62719bd0e156a67fbc1487fd0d..cde4f7a6b73e375cd04e7202ecdb19a75a389137 100644 (file)
--- a/src/pybind/mgr/dashboard/services/iscsi_client.py
+++ b/src/pybind/mgr/dashboard/services/iscsi_client.py
@@ -205,6 +205,15 @@ class IscsiClient(RestClient):
             'disks': ','.join(image_ids)
         })
 
+    @RestClient.api_put('/api/hostgroup/{target_iqn}/{group_name}')
+    def update_group(self, target_iqn, group_name, members, image_ids, request=None):
+        logger.debug("iSCSI[%s] Updating group: %s/%s", self.gateway_name, target_iqn, group_name)
+        return request({
+            'action': 'remove',
+            'members': ','.join(members),
+            'disks': ','.join(image_ids)
+        })
+
     @RestClient.api_delete('/api/hostgroup/{target_iqn}/{group_name}')
     def delete_group(self, target_iqn, group_name, request=None):
         logger.debug("[%s] Deleting group: %s/%s", self.gateway_name, target_iqn, group_name)

diff --git a/src/pybind/mgr/dashboard/tests/test_iscsi.py b/src/pybind/mgr/dashboard/tests/test_iscsi.py
index 962ffeb684665e4c513acf6d1500ee664e03960a..49dfc81d78c1031713ecbbf1bf67786d362a3ad4 100644 (file)
--- a/src/pybind/mgr/dashboard/tests/test_iscsi.py
+++ b/src/pybind/mgr/dashboard/tests/test_iscsi.py
@@ -517,6 +517,74 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         response['clients'].pop(0)
         self._update_iscsi_target(create_request, update_request, 200, None, response)
 
+    @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
+    def test_add_image_to_group_with_client_logged_in(self, _validate_image_mock):
+        client_info = {
+            'alias': '',
+            'ip_address': [],
+            'state': {'LOGGED_IN': ['node1']}
+        }
+        new_disk = {"pool": "rbd", "image": "lun1"}
+        # pylint: disable=protected-access
+        IscsiClientMock._instance.clientinfo = client_info
+        target_iqn = "iqn.2003-01.com.redhat.iscsi-gw:iscsi-igw21"
+        create_request = copy.deepcopy(iscsi_target_request)
+        create_request['target_iqn'] = target_iqn
+        update_request = copy.deepcopy(create_request)
+        update_request['new_target_iqn'] = target_iqn
+        update_request['groups'][0]['disks'].append(new_disk)
+        response = copy.deepcopy(iscsi_target_response)
+        response['target_iqn'] = target_iqn
+        response['groups'][0]['disks'].insert(0, new_disk)
+        for client in response['clients']:
+            client['info'] = client_info
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
+
+    @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
+    def test_add_image_to_initiator_with_client_logged_in(self, _validate_image_mock):
+        client_info = {
+            'alias': '',
+            'ip_address': [],
+            'state': {'LOGGED_IN': ['node1']}
+        }
+        new_disk = {"pool": "rbd", "image": "lun2"}
+        # pylint: disable=protected-access
+        IscsiClientMock._instance.clientinfo = client_info
+        target_iqn = "iqn.2003-01.com.redhat.iscsi-gw:iscsi-igw22"
+        create_request = copy.deepcopy(iscsi_target_request)
+        create_request['target_iqn'] = target_iqn
+        update_request = copy.deepcopy(create_request)
+        update_request['new_target_iqn'] = target_iqn
+        update_request['clients'][0]['luns'].append(new_disk)
+        response = copy.deepcopy(iscsi_target_response)
+        response['target_iqn'] = target_iqn
+        response['clients'][0]['luns'].append(new_disk)
+        for client in response['clients']:
+            client['info'] = client_info
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
+
+    @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
+    def test_remove_image_from_group_with_client_logged_in(self, _validate_image_mock):
+        client_info = {
+            'alias': '',
+            'ip_address': [],
+            'state': {'LOGGED_IN': ['node1']}
+        }
+        # pylint: disable=protected-access
+        IscsiClientMock._instance.clientinfo = client_info
+        target_iqn = "iqn.2003-01.com.redhat.iscsi-gw:iscsi-igw23"
+        create_request = copy.deepcopy(iscsi_target_request)
+        create_request['target_iqn'] = target_iqn
+        update_request = copy.deepcopy(create_request)
+        update_request['new_target_iqn'] = target_iqn
+        update_request['groups'][0]['disks'] = []
+        response = copy.deepcopy(iscsi_target_response)
+        response['target_iqn'] = target_iqn
+        response['groups'][0]['disks'] = []
+        for client in response['clients']:
+            client['info'] = client_info
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
+
     def _update_iscsi_target(self, create_request, update_request, update_response_code,
                              update_response, response):
         self._task_post('/api/iscsi/target', create_request)
@@ -839,6 +907,26 @@ class IscsiClientMock(object):
             target_config['groups'][group_name]['disks'][image_id] = {}
         target_config['groups'][group_name]['members'] = members
 
+    def update_group(self, target_iqn, group_name, members, image_ids):
+        target_config = self.config['targets'][target_iqn]
+        group = target_config['groups'][group_name]
+        old_members = group['members']
+        disks = group['disks']
+        target_config['groups'][group_name] = {
+            "disks": {},
+            "members": []
+        }
+
+        for image_id in disks.keys():
+            if image_id not in image_ids:
+                target_config['groups'][group_name]['disks'][image_id] = {}
+
+        new_members = []
+        for member_iqn in old_members:
+            if member_iqn not in members:
+                new_members.append(member_iqn)
+        target_config['groups'][group_name]['members'] = new_members
+
     def delete_group(self, target_iqn, group_name):
         target_config = self.config['targets'][target_iqn]
         del target_config['groups'][group_name]