mgr/dashboard: iSCSI targets not available if any gateway is down

Fixes: https://tracker.ceph.com/issues/42687
Signed-off-by: Ricardo Marques <rimarques@suse.com>

diff --git a/src/pybind/mgr/dashboard/controllers/iscsi.py b/src/pybind/mgr/dashboard/controllers/iscsi.py
index f3146879b50d90eef9240e5cfe92903d9ff81bb4..93189685d73aa3f36e9ea1d42c39108ccf4b8000 100644 (file)
--- 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 2060b81b8674ee09a0dab315e943f4bdb6c5b305..3b25328d653194aa055c50e1ad4478aedd02ac70 100644 (file)
--- 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 13ca22d266377e37bd0dc96c31fc0225699a349d..43e4124a19003e7a12e4e51d2cf6b0bd51218ae9 100644 (file)
--- 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 (file)
index 0000000..14a6fd0
--- /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 (file)
index 0000000..75cb8a5
--- /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 44994663e6562be8bd3d200364740f28232ef790..9b42296f00130b57a547fbd0440c840fa2947d2a 100755 (executable)
--- 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 d28a49543e9e0d0843230b59540c1de4bfbf6393..67d2118aa541cf315bf5c8dbef206f22be61f5b7 100644 (file)
--- 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 294e6a9aaeefaba67f6860d96ec1b7c51cc1b5d0..e37a030f244e0dd4dc248a714b747f1dfb71bb56 100644 (file)
--- 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',