From 1ef7fc33f31b51f24d13f2d7aef1f2af1aa2a69b Mon Sep 17 00:00:00 2001 From: Ricardo Marques Date: Fri, 22 Nov 2019 18:33:22 +0000 Subject: [PATCH] mgr/dashboard: iSCSI targets not available if any gateway is down Fixes: https://tracker.ceph.com/issues/42687 Signed-off-by: Ricardo Marques --- src/pybind/mgr/dashboard/controllers/iscsi.py | 131 ++++++++++++------ .../iscsi-target-discovery-modal.component.ts | 2 +- .../iscsi-target-list.component.ts | 23 ++- .../shared/pipes/not-available.pipe.spec.ts | 32 +++++ .../app/shared/pipes/not-available.pipe.ts | 16 +++ .../src/app/shared/pipes/pipes.module.ts | 4 + .../mgr/dashboard/services/tcmu_service.py | 2 + src/pybind/mgr/dashboard/tests/test_iscsi.py | 44 ++++-- 8 files changed, 200 insertions(+), 54 deletions(-) create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/not-available.pipe.spec.ts create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/not-available.pipe.ts diff --git a/src/pybind/mgr/dashboard/controllers/iscsi.py b/src/pybind/mgr/dashboard/controllers/iscsi.py index f3146879b50..93189685d73 100644 --- a/src/pybind/mgr/dashboard/controllers/iscsi.py +++ b/src/pybind/mgr/dashboard/controllers/iscsi.py @@ -32,18 +32,13 @@ class IscsiUi(BaseController): @ReadPermission def status(self): status = {'available': False} - gateways = IscsiGatewaysConfig.get_gateways_config()['gateways'] - if not gateways: - status['message'] = 'There are no gateways defined' + try: + gateway = get_available_gateway() + except DashboardException as e: + status['message'] = str(e) return status try: - for gateway in gateways: - try: - IscsiClient.instance(gateway_name=gateway).ping() - except RequestException: - status['message'] = 'Gateway {} is inaccessible'.format(gateway) - return status - config = IscsiClient.instance().get_config() + config = IscsiClient.instance(gateway_name=gateway).get_config() if config['version'] < IscsiUi.REQUIRED_CEPH_ISCSI_CONFIG_MIN_VERSION or \ config['version'] > IscsiUi.REQUIRED_CEPH_ISCSI_CONFIG_MAX_VERSION: status['message'] = 'Unsupported `ceph-iscsi` config version. ' \ @@ -68,14 +63,17 @@ class IscsiUi(BaseController): @Endpoint() @ReadPermission def version(self): + gateway = get_available_gateway() + config = IscsiClient.instance(gateway_name=gateway).get_config() return { - 'ceph_iscsi_config_version': IscsiClient.instance().get_config()['version'] + 'ceph_iscsi_config_version': config['version'] } @Endpoint() @ReadPermission def settings(self): - settings = IscsiClient.instance().get_settings() + gateway = get_available_gateway() + settings = IscsiClient.instance(gateway_name=gateway).get_settings() if 'target_controls_limits' in settings: target_default_controls = settings['target_default_controls'] for ctrl_k, ctrl_v in target_default_controls.items(): @@ -105,8 +103,11 @@ class IscsiUi(BaseController): portals = [] gateways_config = IscsiGatewaysConfig.get_gateways_config() for name in gateways_config['gateways']: - ip_addresses = IscsiClient.instance(gateway_name=name).get_ip_addresses() - portals.append({'name': name, 'ip_addresses': ip_addresses['data']}) + try: + ip_addresses = IscsiClient.instance(gateway_name=name).get_ip_addresses() + portals.append({'name': name, 'ip_addresses': ip_addresses['data']}) + except RequestException: + pass return sorted(portals, key=lambda p: '{}.{}'.format(p['name'], p['ip_addresses'])) @Endpoint() @@ -183,16 +184,24 @@ class Iscsi(BaseController): @Endpoint('GET', 'discoveryauth') @ReadPermission def get_discoveryauth(self): - return self._get_discoveryauth() + gateway = get_available_gateway() + return self._get_discoveryauth(gateway) @Endpoint('PUT', 'discoveryauth') @UpdatePermission def set_discoveryauth(self, user, password, mutual_user, mutual_password): - IscsiClient.instance().update_discoveryauth(user, password, mutual_user, mutual_password) - return self._get_discoveryauth() - - def _get_discoveryauth(self): - config = IscsiClient.instance().get_config() + gateway = get_available_gateway() + config = IscsiClient.instance(gateway_name=gateway).get_config() + gateway_names = list(config['gateways'].keys()) + validate_rest_api(gateway_names) + IscsiClient.instance(gateway_name=gateway).update_discoveryauth(user, + password, + mutual_user, + mutual_password) + return self._get_discoveryauth(gateway) + + def _get_discoveryauth(self, gateway): + config = IscsiClient.instance(gateway_name=gateway).get_config() user = config['discovery_auth']['username'] password = config['discovery_auth']['password'] mutual_user = config['discovery_auth']['mutual_username'] @@ -213,7 +222,8 @@ def iscsi_target_task(name, metadata, wait_for=2.0): class IscsiTarget(RESTController): def list(self): - config = IscsiClient.instance().get_config() + gateway = get_available_gateway() + config = IscsiClient.instance(gateway_name=gateway).get_config() targets = [] for target_iqn in config['targets'].keys(): target = IscsiTarget._config_to_target(target_iqn, config) @@ -222,7 +232,8 @@ class IscsiTarget(RESTController): return targets def get(self, target_iqn): - config = IscsiClient.instance().get_config() + gateway = get_available_gateway() + config = IscsiClient.instance(gateway_name=gateway).get_config() if target_iqn not in config['targets']: raise cherrypy.HTTPError(404) target = IscsiTarget._config_to_target(target_iqn, config) @@ -231,7 +242,8 @@ class IscsiTarget(RESTController): @iscsi_target_task('delete', {'target_iqn': '{target_iqn}'}) def delete(self, target_iqn): - config = IscsiClient.instance().get_config() + gateway = get_available_gateway() + config = IscsiClient.instance(gateway_name=gateway).get_config() if target_iqn not in config['targets']: raise DashboardException(msg='Target does not exist', code='target_does_not_exist', @@ -240,11 +252,15 @@ class IscsiTarget(RESTController): raise DashboardException(msg='Target does not exist', code='target_does_not_exist', component='iscsi') - target_info = IscsiClient.instance().get_targetinfo(target_iqn) - if target_info['num_sessions'] > 0: - raise DashboardException(msg='Target has active sessions', - code='target_has_active_sessions', - component='iscsi') + portal_names = list(config['targets'][target_iqn]['portals'].keys()) + validate_rest_api(portal_names) + if portal_names: + portal_name = portal_names[0] + target_info = IscsiClient.instance(gateway_name=portal_name).get_targetinfo(target_iqn) + if target_info['num_sessions'] > 0: + raise DashboardException(msg='Target has active sessions', + code='target_has_active_sessions', + component='iscsi') IscsiTarget._delete(target_iqn, config, 0, 100) @iscsi_target_task('create', {'target_iqn': '{target_iqn}'}) @@ -256,12 +272,13 @@ class IscsiTarget(RESTController): clients = clients or [] groups = groups or [] - config = IscsiClient.instance().get_config() + gateway = get_available_gateway() + config = IscsiClient.instance(gateway_name=gateway).get_config() if target_iqn in config['targets']: raise DashboardException(msg='Target already exists', code='target_already_exists', component='iscsi') - settings = IscsiClient.instance().get_settings() + settings = IscsiClient.instance(gateway_name=gateway).get_settings() IscsiTarget._validate(target_iqn, target_controls, portals, disks, groups, settings) IscsiTarget._create(target_iqn, target_controls, acl_enabled, auth, portals, disks, @@ -276,7 +293,8 @@ class IscsiTarget(RESTController): clients = IscsiTarget._sorted_clients(clients) groups = IscsiTarget._sorted_groups(groups) - config = IscsiClient.instance().get_config() + gateway = get_available_gateway() + config = IscsiClient.instance(gateway_name=gateway).get_config() if target_iqn not in config['targets']: raise DashboardException(msg='Target does not exist', code='target_does_not_exist', @@ -285,7 +303,11 @@ class IscsiTarget(RESTController): raise DashboardException(msg='Target IQN already in use', code='target_iqn_already_in_use', component='iscsi') - settings = IscsiClient.instance().get_settings() + settings = IscsiClient.instance(gateway_name=gateway).get_settings() + new_portal_names = {p['host'] for p in portals} + old_portal_names = set(config['targets'][target_iqn]['portals'].keys()) + deleted_portal_names = list(old_portal_names - new_portal_names) + validate_rest_api(deleted_portal_names) IscsiTarget._validate(new_target_iqn, target_controls, portals, disks, groups, settings) config = IscsiTarget._delete(target_iqn, config, 0, 50, new_target_iqn, target_controls, portals, disks, clients, groups) @@ -504,15 +526,8 @@ class IscsiTarget(RESTController): code='target_control_invalid_max', component='iscsi') - for portal in portals: - gateway_name = portal['host'] - try: - IscsiClient.instance(gateway_name=gateway_name).ping() - except RequestException: - raise DashboardException(msg='iSCSI REST Api not available for gateway ' - '{}'.format(gateway_name), - code='ceph_iscsi_rest_api_not_available_for_gateway', - component='iscsi') + portal_names = [p['host'] for p in portals] + validate_rest_api(portal_names) for disk in disks: pool = disk['pool'] @@ -829,6 +844,12 @@ class IscsiTarget(RESTController): # During task execution, additional info is not available if IscsiTarget._is_executing(target_iqn): return + # If any portal is down, additional info is not available + for portal in target['portals']: + try: + IscsiClient.instance(gateway_name=portal['host']).ping() + except RequestException: + return gateway_name = target['portals'][0]['host'] try: target_info = IscsiClient.instance(gateway_name=gateway_name).get_targetinfo( @@ -881,3 +902,31 @@ class IscsiTarget(RESTController): portals_by_host[host] = [] portals_by_host[host].append(ip) return portals_by_host + + +def get_available_gateway(): + gateways = IscsiGatewaysConfig.get_gateways_config()['gateways'] + if not gateways: + raise DashboardException(msg='There are no gateways defined', + code='no_gateways_defined', + component='iscsi') + for gateway in gateways: + try: + IscsiClient.instance(gateway_name=gateway).ping() + return gateway + except RequestException: + pass + raise DashboardException(msg='There are no gateways available', + code='no_gateways_available', + component='iscsi') + + +def validate_rest_api(gateways): + for gateway in gateways: + try: + IscsiClient.instance(gateway_name=gateway).ping() + except RequestException: + raise DashboardException(msg='iSCSI REST Api not available for gateway ' + '{}'.format(gateway), + code='ceph_iscsi_rest_api_not_available_for_gateway', + component='iscsi') diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-discovery-modal/iscsi-target-discovery-modal.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-discovery-modal/iscsi-target-discovery-modal.component.ts index 2060b81b867..3b25328d653 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-discovery-modal/iscsi-target-discovery-modal.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-discovery-modal/iscsi-target-discovery-modal.component.ts @@ -116,7 +116,7 @@ export class IscsiTargetDiscoveryModalComponent implements OnInit { this.bsModalRef.hide(); }, () => { - this.bsModalRef.hide(); + this.discoveryForm.setErrors({ cdSubmitButton: true }); } ); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts index 13ca22d2663..43e4124a190 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts @@ -17,6 +17,7 @@ import { CdTableSelection } from '../../../shared/models/cd-table-selection'; import { FinishedTask } from '../../../shared/models/finished-task'; import { Permission } from '../../../shared/models/permissions'; import { CephReleaseNamePipe } from '../../../shared/pipes/ceph-release-name.pipe'; +import { NotAvailablePipe } from '../../../shared/pipes/not-available.pipe'; import { AuthStorageService } from '../../../shared/services/auth-storage.service'; import { SummaryService } from '../../../shared/services/summary.service'; import { TaskListService } from '../../../shared/services/task-list.service'; @@ -61,6 +62,7 @@ export class IscsiTargetListComponent implements OnInit, OnDestroy { private iscsiService: IscsiService, private taskListService: TaskListService, private cephReleaseNamePipe: CephReleaseNamePipe, + private notAvailablePipe: NotAvailablePipe, private summaryservice: SummaryService, private modalService: BsModalService, private taskWrapper: TaskWrapperService, @@ -79,7 +81,9 @@ export class IscsiTargetListComponent implements OnInit, OnDestroy { permission: 'update', icon: Icons.edit, routerLink: () => `/block/iscsi/targets/edit/${this.selection.first().target_iqn}`, - name: this.actionLabels.EDIT + name: this.actionLabels.EDIT, + disable: () => !this.selection.first() || !_.isUndefined(this.getDeleteDisableDesc()), + disableDesc: () => this.getEditDisableDesc() }, { permission: 'delete', @@ -113,6 +117,7 @@ export class IscsiTargetListComponent implements OnInit, OnDestroy { { name: this.i18n('# Sessions'), prop: 'info.num_sessions', + pipe: this.notAvailablePipe, flexGrow: 1 } ]; @@ -152,8 +157,24 @@ export class IscsiTargetListComponent implements OnInit, OnDestroy { } } + getEditDisableDesc(): string | undefined { + const first = this.selection.first(); + if (first && first.cdExecuting) { + return first.cdExecuting; + } + if (first && _.isUndefined(first['info'])) { + return this.i18n('Unavailable gateway(s)'); + } + } + getDeleteDisableDesc(): string | undefined { const first = this.selection.first(); + if (first && first.cdExecuting) { + return first.cdExecuting; + } + if (first && _.isUndefined(first['info'])) { + return this.i18n('Unavailable gateway(s)'); + } if (first && first['info'] && first['info']['num_sessions']) { return this.i18n('Target has active sessions'); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/not-available.pipe.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/not-available.pipe.spec.ts new file mode 100644 index 00000000000..14a6fd064ce --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/not-available.pipe.spec.ts @@ -0,0 +1,32 @@ +import { TestBed } from '@angular/core/testing'; + +import { I18n } from '@ngx-translate/i18n-polyfill'; + +import { configureTestBed, i18nProviders } from '../../../testing/unit-test-helper'; +import { NotAvailablePipe } from './not-available.pipe'; + +describe('NotAvailablePipe', () => { + let pipe: NotAvailablePipe; + + configureTestBed({ + providers: [i18nProviders] + }); + + beforeEach(() => { + const i18n = TestBed.get(I18n); + pipe = new NotAvailablePipe(i18n); + }); + + it('create an instance', () => { + expect(pipe).toBeTruthy(); + }); + + it('transforms not available', () => { + expect(pipe.transform('')).toBe('n/a'); + }); + + it('transforms number', () => { + expect(pipe.transform(0)).toBe(0); + expect(pipe.transform(1)).toBe(1); + }); +}); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/not-available.pipe.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/not-available.pipe.ts new file mode 100644 index 00000000000..75cb8a5cf0c --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/not-available.pipe.ts @@ -0,0 +1,16 @@ +import { Pipe, PipeTransform } from '@angular/core'; + +import { I18n } from '@ngx-translate/i18n-polyfill'; + +@Pipe({ + name: 'notAvailable' +}) +export class NotAvailablePipe implements PipeTransform { + constructor(private i18n: I18n) {} + transform(value: any): any { + if (value === '') { + return this.i18n('n/a'); + } + return value; + } +} diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/pipes.module.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/pipes.module.ts index 44994663e65..9b42296f001 100755 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/pipes.module.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/pipes.module.ts @@ -20,6 +20,7 @@ import { IscsiBackstorePipe } from './iscsi-backstore.pipe'; import { JoinPipe } from './join.pipe'; import { LogPriorityPipe } from './log-priority.pipe'; import { MillisecondsPipe } from './milliseconds.pipe'; +import { NotAvailablePipe } from './not-available.pipe'; import { OrdinalPipe } from './ordinal.pipe'; import { RbdConfigurationSourcePipe } from './rbd-configuration-source.pipe'; import { RelativeDatePipe } from './relative-date.pipe'; @@ -49,6 +50,7 @@ import { UpperFirstPipe } from './upper-first.pipe'; RoundPipe, OrdinalPipe, MillisecondsPipe, + NotAvailablePipe, IopsPipe, UpperFirstPipe, RbdConfigurationSourcePipe, @@ -75,6 +77,7 @@ import { UpperFirstPipe } from './upper-first.pipe'; RoundPipe, OrdinalPipe, MillisecondsPipe, + NotAvailablePipe, IopsPipe, UpperFirstPipe, RbdConfigurationSourcePipe, @@ -100,6 +103,7 @@ import { UpperFirstPipe } from './upper-first.pipe'; OrdinalPipe, IopsPipe, MillisecondsPipe, + NotAvailablePipe, UpperFirstPipe ] }) diff --git a/src/pybind/mgr/dashboard/services/tcmu_service.py b/src/pybind/mgr/dashboard/services/tcmu_service.py index d28a49543e9..67d2118aa54 100644 --- a/src/pybind/mgr/dashboard/services/tcmu_service.py +++ b/src/pybind/mgr/dashboard/services/tcmu_service.py @@ -12,6 +12,8 @@ class TcmuService(object): images = {} for service in CephService.get_service_list(SERVICE_TYPE): metadata = service['metadata'] + if metadata is None: + continue status = service['status'] hostname = service['hostname'] diff --git a/src/pybind/mgr/dashboard/tests/test_iscsi.py b/src/pybind/mgr/dashboard/tests/test_iscsi.py index 294e6a9aaee..e37a030f244 100644 --- a/src/pybind/mgr/dashboard/tests/test_iscsi.py +++ b/src/pybind/mgr/dashboard/tests/test_iscsi.py @@ -3,13 +3,14 @@ import copy import errno import json +import unittest try: import mock except ImportError: import unittest.mock as mock -from . import CmdException, ControllerTestCase, CLICommandTestMixin +from . import CmdException, ControllerTestCase, CLICommandTestMixin, KVStoreMockMixin from .. import mgr from ..controllers.iscsi import Iscsi, IscsiTarget from ..services.iscsi_client import IscsiClient @@ -17,16 +18,7 @@ from ..services.orchestrator import OrchClient from ..rest_client import RequestException -class IscsiTest(ControllerTestCase, CLICommandTestMixin): - - @classmethod - def setup_server(cls): - OrchClient.instance().available = lambda: False - mgr.rados.side_effect = None - # pylint: disable=protected-access - Iscsi._cp_config['tools.authenticate.on'] = False - IscsiTarget._cp_config['tools.authenticate.on'] = False - cls.setup_controllers([Iscsi, IscsiTarget]) +class IscsiTestCli(unittest.TestCase, CLICommandTestMixin): def setUp(self): self.mock_kv_store() @@ -69,6 +61,36 @@ class IscsiTest(ControllerTestCase, CLICommandTestMixin): } }) + +class IscsiTestController(ControllerTestCase, KVStoreMockMixin): + + @classmethod + def setup_server(cls): + OrchClient.instance().available = lambda: False + mgr.rados.side_effect = None + # pylint: disable=protected-access + Iscsi._cp_config['tools.authenticate.on'] = False + IscsiTarget._cp_config['tools.authenticate.on'] = False + cls.setup_controllers([Iscsi, IscsiTarget]) + + def setUp(self): + self.mock_kv_store() + self.CONFIG_KEY_DICT['_iscsi_config'] = ''' + { + "gateways": { + "node1": { + "service_url": "https://admin:admin@10.17.5.1:5001" + }, + "node2": { + "service_url": "https://admin:admin@10.17.5.2:5001" + } + } + } + ''' + # pylint: disable=protected-access + IscsiClientMock._instance = IscsiClientMock() + IscsiClient.instance = IscsiClientMock.instance + def test_enable_discoveryauth(self): discoveryauth = { 'user': 'myiscsiusername', -- 2.39.5