From 6de09f131074294b71e47ab0e168036a1fcc35fe Mon Sep 17 00:00:00 2001 From: Tiago Melo Date: Mon, 7 Sep 2020 09:47:19 +0000 Subject: [PATCH] mgr/dashboard: Allow editing iSCSI targets with initiators logged-in Fixes: https://tracker.ceph.com/issues/47393 Signed-off-by: Tiago Melo --- src/pybind/mgr/dashboard/controllers/iscsi.py | 99 +++++++++++-------- .../mgr/dashboard/services/iscsi_client.py | 9 ++ src/pybind/mgr/dashboard/tests/test_iscsi.py | 88 +++++++++++++++++ 3 files changed, 157 insertions(+), 39 deletions(-) diff --git a/src/pybind/mgr/dashboard/controllers/iscsi.py b/src/pybind/mgr/dashboard/controllers/iscsi.py index cb5e657cbb5..ce81768072a 100644 --- 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 b82a51a3d4a..cde4f7a6b73 100644 --- 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 962ffeb6846..49dfc81d78c 100644 --- 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]