From 58253a0002f8722abecaaf58161f6494fbe0eaa0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alfonso=20Mart=C3=ADnez?= Date: Tue, 23 Mar 2021 11:14:11 +0100 Subject: [PATCH] mgr/dashboard: fix error notification shown when no rgw daemons are running. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit - Adapted code to changes introduced in: https://github.com/ceph/ceph/pull/40220 - Improved error handling. - Increased test coverage. - Some refactoring. - Simplified documentation about setting default daemon host and port. Fixes: https://tracker.ceph.com/issues/49655 Signed-off-by: Alfonso Martínez --- doc/mgr/dashboard.rst | 5 +- .../dashboard/controllers/perf_counters.py | 35 +++++ src/pybind/mgr/dashboard/controllers/rgw.py | 26 ++-- .../nfs/nfs-form/nfs-form.component.spec.ts | 2 +- .../core/context/context.component.spec.ts | 34 ++--- .../src/app/core/context/context.component.ts | 4 - .../app/shared/api/rgw-bucket.service.spec.ts | 8 +- .../app/shared/api/rgw-daemon.service.spec.ts | 47 ++++++- .../src/app/shared/api/rgw-daemon.service.ts | 33 ++++- .../app/shared/api/rgw-site.service.spec.ts | 3 +- .../app/shared/api/rgw-user.service.spec.ts | 16 +-- .../frontend/src/testing/unit-test-helper.ts | 23 +++- src/pybind/mgr/dashboard/rest_client.py | 11 +- .../mgr/dashboard/services/ceph_service.py | 27 ++-- .../mgr/dashboard/services/rgw_client.py | 104 ++++++--------- src/pybind/mgr/dashboard/tests/__init__.py | 45 +++++++ .../mgr/dashboard/tests/test_rest_client.py | 12 +- src/pybind/mgr/dashboard/tests/test_rgw.py | 126 +++++++++++++++--- .../mgr/dashboard/tests/test_rgw_client.py | 47 +++---- 19 files changed, 409 insertions(+), 199 deletions(-) diff --git a/doc/mgr/dashboard.rst b/doc/mgr/dashboard.rst index efa458a7cf4..1044b18d528 100644 --- a/doc/mgr/dashboard.rst +++ b/doc/mgr/dashboard.rst @@ -410,9 +410,8 @@ have to do to get the Object Gateway management functionality working. The dashboard will try to automatically determine the host and port from the Ceph Manager's service map. -If multiple zones are used, it will automatically determine the host within the -master zone group and master zone. This should be sufficient for most setups, -but in some circumstances you might want to set the host and port manually:: +In case of having several Object Gateways, you might want to set +the default one by setting its host and port manually:: $ ceph dashboard set-rgw-api-host $ ceph dashboard set-rgw-api-port diff --git a/src/pybind/mgr/dashboard/controllers/perf_counters.py b/src/pybind/mgr/dashboard/controllers/perf_counters.py index 96aa08590ac..fca8e294ef5 100644 --- a/src/pybind/mgr/dashboard/controllers/perf_counters.py +++ b/src/pybind/mgr/dashboard/controllers/perf_counters.py @@ -1,6 +1,8 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import +from typing import Any, Dict + import cherrypy from .. import mgr @@ -80,6 +82,39 @@ class OsdPerfCounter(PerfCounter): class RgwPerfCounter(PerfCounter): service_type = 'rgw' + def get(self, service_id: str) -> Dict[str, Any]: + svc_data = CephService.get_service_data_by_metadata_id(self.service_type, service_id) + service_map_id = svc_data['service_map_id'] + schema_dict = mgr.get_perf_schema(self.service_type, service_map_id) + try: + schema = schema_dict["{}.{}".format(self.service_type, service_map_id)] + except KeyError as e: + raise cherrypy.HTTPError(404, "{0} not found".format(e)) + counters = [] + + for key, value in sorted(schema.items()): + counter = dict() + counter['name'] = str(key) + counter['description'] = value['description'] + # pylint: disable=W0212 + if mgr._stattype_to_str(value['type']) == 'counter': + counter['value'] = CephService.get_rate( + self.service_type, service_map_id, key) + counter['unit'] = mgr._unit_to_str(value['units']) + else: + counter['value'] = mgr.get_latest( + self.service_type, service_map_id, key) + counter['unit'] = '' + counters.append(counter) + + return { + 'service': { + 'type': self.service_type, + 'id': svc_data['id'] + }, + 'counters': counters + } + @ApiController('perf_counters/rbd-mirror', Scope.RBD_MIRRORING) @ControllerDoc("Rgw Mirroring Perf Counters Management API", "RgwMirrorPerfCounter") diff --git a/src/pybind/mgr/dashboard/controllers/rgw.py b/src/pybind/mgr/dashboard/controllers/rgw.py index 9e41e5a4d69..96e6dd13d8f 100644 --- a/src/pybind/mgr/dashboard/controllers/rgw.py +++ b/src/pybind/mgr/dashboard/controllers/rgw.py @@ -11,7 +11,7 @@ from ..rest_client import RequestException from ..security import Permission, Scope from ..services.auth import AuthManager, JwtManager from ..services.ceph_service import CephService -from ..services.rgw_client import RgwClient +from ..services.rgw_client import NoRgwDaemonsException, RgwClient from ..tools import json_str_to_object, str_to_bool from . import ApiController, BaseController, ControllerDoc, Endpoint, \ EndpointDoc, ReadPermission, RESTController, allow_empty_body @@ -48,11 +48,9 @@ class Rgw(BaseController): @ReadPermission @EndpointDoc("Display RGW Status", responses={200: RGW_SCHEMA}) - def status(self): + def status(self) -> dict: status = {'available': False, 'message': None} try: - if not CephService.get_service_list('rgw'): - raise LookupError('No RGW service is running.') instance = RgwClient.admin_instance() # Check if the service is online. try: @@ -76,7 +74,7 @@ class Rgw(BaseController): instance.userid) raise RequestException(msg) status['available'] = True - except (RequestException, LookupError) as ex: + except (DashboardException, RequestException, NoRgwDaemonsException) as ex: status['message'] = str(ex) # type: ignore return status @@ -86,22 +84,25 @@ class Rgw(BaseController): class RgwDaemon(RESTController): @EndpointDoc("Display RGW Daemons", responses={200: [RGW_DAEMON_SCHEMA]}) - def list(self): - # type: () -> List[dict] - daemons = [] - instance = RgwClient.admin_instance() + def list(self) -> List[dict]: + daemons: List[dict] = [] + try: + instance = RgwClient.admin_instance() + except NoRgwDaemonsException: + return daemons + for hostname, server in CephService.get_service_map('rgw').items(): for service in server['services']: metadata = service['metadata'] # extract per-daemon service data and health daemon = { - 'id': service['id'], + 'id': metadata['id'], 'version': metadata['ceph_version'], 'server_hostname': hostname, 'zonegroup_name': metadata['zonegroup_name'], 'zone_name': metadata['zone_name'], - 'default': instance.daemon.name == service['id'] + 'default': instance.daemon.name == metadata['id'] } daemons.append(daemon) @@ -144,7 +145,8 @@ class RgwRESTController(RESTController): result = json_str_to_object(result) return result except (DashboardException, RequestException) as e: - raise DashboardException(e, http_status_code=500, component='rgw') + http_status_code = e.status if isinstance(e, DashboardException) else 500 + raise DashboardException(e, http_status_code=http_status_code, component='rgw') @ApiController('/rgw/site', Scope.RGW) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/nfs/nfs-form/nfs-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/nfs/nfs-form/nfs-form.component.spec.ts index 660b9777a8c..4ebc1288de3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/nfs/nfs-form/nfs-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/nfs/nfs-form/nfs-form.component.spec.ts @@ -47,6 +47,7 @@ describe('NfsFormComponent', () => { component = fixture.componentInstance; httpTesting = TestBed.inject(HttpTestingController); activatedRoute = TestBed.inject(ActivatedRoute); + RgwHelper.selectDaemon(); fixture.detectChanges(); httpTesting.expectOne('api/nfs-ganesha/daemon').flush([ @@ -57,7 +58,6 @@ describe('NfsFormComponent', () => { httpTesting.expectOne('ui-api/nfs-ganesha/fsals').flush(['CEPH', 'RGW']); httpTesting.expectOne('ui-api/nfs-ganesha/cephx/clients').flush(['admin', 'fs', 'rgw']); httpTesting.expectOne('ui-api/nfs-ganesha/cephfs/filesystems').flush([{ id: 1, name: 'a' }]); - RgwHelper.getCurrentDaemon(); httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`).flush(['test', 'dev']); const user_dev = { suspended: 0, diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/context/context.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/context/context.component.spec.ts index adffb6f1075..9512e3183ab 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/context/context.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/context/context.component.spec.ts @@ -5,14 +5,13 @@ import { RouterTestingModule } from '@angular/router/testing'; import { of } from 'rxjs'; -import { RgwDaemon } from '~/app/ceph/rgw/models/rgw-daemon'; import { Permissions } from '~/app/shared/models/permissions'; import { AuthStorageService } from '~/app/shared/services/auth-storage.service'; import { FeatureTogglesMap, FeatureTogglesService } from '~/app/shared/services/feature-toggles.service'; -import { configureTestBed } from '~/testing/unit-test-helper'; +import { configureTestBed, RgwHelper } from '~/testing/unit-test-helper'; import { ContextComponent } from './context.component'; describe('ContextComponent', () => { @@ -26,17 +25,7 @@ describe('ContextComponent', () => { let ftMap: FeatureTogglesMap; let httpTesting: HttpTestingController; - const getDaemonList = () => { - const daemonList: RgwDaemon[] = []; - for (let daemonIndex = 1; daemonIndex <= 3; daemonIndex++) { - const rgwDaemon = new RgwDaemon(); - rgwDaemon.id = `daemon${daemonIndex}`; - rgwDaemon.default = daemonIndex === 2; - rgwDaemon.zonegroup_name = `zonegroup${daemonIndex}`; - daemonList.push(rgwDaemon); - } - return daemonList; - }; + const daemonList = RgwHelper.getDaemonList(); configureTestBed({ declarations: [ContextComponent], @@ -59,9 +48,6 @@ describe('ContextComponent', () => { getFeatureTogglesSpy.and.returnValue(of(ftMap)); fixture = TestBed.createComponent(ContextComponent); component = fixture.componentInstance; - fixture.detectChanges(); - const req = httpTesting.expectOne('api/rgw/daemon'); - req.flush(getDaemonList()); }); it('should create', () => { @@ -75,7 +61,10 @@ describe('ContextComponent', () => { it('should select the default daemon', fakeAsync(() => { component.isRgwRoute = true; + fixture.detectChanges(); tick(); + const req = httpTesting.expectOne('api/rgw/daemon'); + req.flush(daemonList); fixture.detectChanges(); const selectedDaemon = fixture.debugElement.nativeElement.querySelector( '.ctx-bar-selected-rgw-daemon' @@ -85,22 +74,27 @@ describe('ContextComponent', () => { const availableDaemons = fixture.debugElement.nativeElement.querySelectorAll( '.ctx-bar-available-rgw-daemon' ); - expect(availableDaemons.length).toEqual(getDaemonList().length); + expect(availableDaemons.length).toEqual(daemonList.length); expect(availableDaemons[0].textContent).toEqual(' daemon1 ( zonegroup1 ) '); + component.ngOnDestroy(); })); it('should select the chosen daemon', fakeAsync(() => { component.isRgwRoute = true; - component.onDaemonSelection(getDaemonList()[2]); + fixture.detectChanges(); tick(); + const req = httpTesting.expectOne('api/rgw/daemon'); + req.flush(daemonList); fixture.detectChanges(); - + component.onDaemonSelection(daemonList[2]); expect(routerNavigateByUrlSpy).toHaveBeenCalledTimes(1); + fixture.detectChanges(); + tick(); expect(routerNavigateSpy).toHaveBeenCalledTimes(1); - const selectedDaemon = fixture.debugElement.nativeElement.querySelector( '.ctx-bar-selected-rgw-daemon' ); expect(selectedDaemon.textContent).toEqual(' daemon3 ( zonegroup3 ) '); + component.ngOnDestroy(); })); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/context/context.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/context/context.component.ts index 5d1d1e34f76..3aea5686808 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/context/context.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/context/context.component.ts @@ -44,10 +44,6 @@ export class ContextComponent implements OnInit, OnDestroy { .pipe(filter((event: Event) => event instanceof NavigationEnd)) .subscribe(() => (this.isRgwRoute = this.router.url.includes(this.rgwUrlPrefix))) ); - // Select default daemon: - this.rgwDaemonService.list().subscribe((daemons: RgwDaemon[]) => { - this.rgwDaemonService.selectDefaultDaemon(daemons); - }); // Set daemon list polling only when in RGW route: this.subs.add( this.timerService diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts index e0787d06a92..3f2bae5f48a 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts @@ -16,6 +16,7 @@ describe('RgwBucketService', () => { beforeEach(() => { service = TestBed.inject(RgwBucketService); httpTesting = TestBed.inject(HttpTestingController); + RgwHelper.selectDaemon(); }); afterEach(() => { @@ -28,14 +29,12 @@ describe('RgwBucketService', () => { it('should call list', () => { service.list().subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/bucket?${RgwHelper.DAEMON_QUERY_PARAM}&stats=true`); expect(req.request.method).toBe('GET'); }); it('should call get', () => { service.get('foo').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/bucket/foo?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('GET'); }); @@ -44,7 +43,6 @@ describe('RgwBucketService', () => { service .create('foo', 'bar', 'default', 'default-placement', false, 'COMPLIANCE', '10', '0') .subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/bucket?bucket=foo&uid=bar&zonegroup=default&placement_target=default-placement&lock_enabled=false&lock_mode=COMPLIANCE&lock_retention_period_days=10&lock_retention_period_years=0&${RgwHelper.DAEMON_QUERY_PARAM}` ); @@ -55,7 +53,6 @@ describe('RgwBucketService', () => { service .update('foo', 'bar', 'baz', 'Enabled', 'Enabled', '1', '223344', 'GOVERNANCE', '0', '1') .subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/bucket/foo?${RgwHelper.DAEMON_QUERY_PARAM}&bucket_id=bar&uid=baz&versioning_state=Enabled&mfa_delete=Enabled&mfa_token_serial=1&mfa_token_pin=223344&lock_mode=GOVERNANCE&lock_retention_period_days=0&lock_retention_period_years=1` ); @@ -64,7 +61,6 @@ describe('RgwBucketService', () => { it('should call delete, with purgeObjects = true', () => { service.delete('foo').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/bucket/foo?${RgwHelper.DAEMON_QUERY_PARAM}&purge_objects=true` ); @@ -73,7 +69,6 @@ describe('RgwBucketService', () => { it('should call delete, with purgeObjects = false', () => { service.delete('foo', false).subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/bucket/foo?${RgwHelper.DAEMON_QUERY_PARAM}&purge_objects=false` ); @@ -85,7 +80,6 @@ describe('RgwBucketService', () => { service.exists('foo').subscribe((resp) => { result = resp; }); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/bucket?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('GET'); req.flush(['foo', 'bar']); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-daemon.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-daemon.service.spec.ts index 8aa4ea0a480..6f96c6ab51a 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-daemon.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-daemon.service.spec.ts @@ -1,12 +1,27 @@ import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; -import { TestBed } from '@angular/core/testing'; +import { fakeAsync, TestBed, tick } from '@angular/core/testing'; -import { configureTestBed } from '~/testing/unit-test-helper'; +import { of } from 'rxjs'; + +import { RgwDaemon } from '~/app/ceph/rgw/models/rgw-daemon'; +import { configureTestBed, RgwHelper } from '~/testing/unit-test-helper'; import { RgwDaemonService } from './rgw-daemon.service'; describe('RgwDaemonService', () => { let service: RgwDaemonService; let httpTesting: HttpTestingController; + let selectDaemonSpy: jasmine.Spy; + + const daemonList = RgwHelper.getDaemonList(); + const retrieveDaemonList = (reqDaemonList: RgwDaemon[], daemon: RgwDaemon) => { + service + .request((params) => of(params)) + .subscribe((params) => expect(params.get('daemon_name')).toBe(daemon.id)); + const listReq = httpTesting.expectOne('api/rgw/daemon'); + listReq.flush(reqDaemonList); + tick(); + expect(service['selectedDaemon'].getValue()).toEqual(daemon); + }; configureTestBed({ providers: [RgwDaemonService], @@ -15,6 +30,7 @@ describe('RgwDaemonService', () => { beforeEach(() => { service = TestBed.inject(RgwDaemonService); + selectDaemonSpy = spyOn(service, 'selectDaemon').and.callThrough(); httpTesting = TestBed.inject(HttpTestingController); }); @@ -26,15 +42,38 @@ describe('RgwDaemonService', () => { expect(service).toBeTruthy(); }); - it('should call list', () => { + it('should get daemon list', () => { service.list().subscribe(); const req = httpTesting.expectOne('api/rgw/daemon'); + req.flush(daemonList); expect(req.request.method).toBe('GET'); + expect(service['daemons'].getValue()).toEqual(daemonList); }); - it('should call get', () => { + it('should get daemon ', () => { service.get('foo').subscribe(); const req = httpTesting.expectOne('api/rgw/daemon/foo'); expect(req.request.method).toBe('GET'); }); + + it('should call request and not select any daemon from empty daemon list', fakeAsync(() => { + expect(() => retrieveDaemonList([], null)).toThrowError('No RGW daemons found!'); + expect(selectDaemonSpy).toHaveBeenCalledTimes(0); + })); + + it('should call request and select default daemon from daemon list', fakeAsync(() => { + retrieveDaemonList(daemonList, daemonList[1]); + expect(selectDaemonSpy).toHaveBeenCalledTimes(1); + expect(selectDaemonSpy).toHaveBeenCalledWith(daemonList[1]); + })); + + it('should call request and select first daemon from daemon list that has no default', fakeAsync(() => { + const noDefaultDaemonList = daemonList.map((daemon) => { + daemon.default = false; + return daemon; + }); + retrieveDaemonList(noDefaultDaemonList, noDefaultDaemonList[0]); + expect(selectDaemonSpy).toHaveBeenCalledTimes(1); + expect(selectDaemonSpy).toHaveBeenCalledWith(noDefaultDaemonList[0]); + })); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-daemon.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-daemon.service.ts index 67244e7de0e..bc81dacf47e 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-daemon.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-daemon.service.ts @@ -1,8 +1,9 @@ import { HttpClient, HttpParams } from '@angular/common/http'; import { Injectable } from '@angular/core'; -import { Observable, ReplaySubject } from 'rxjs'; -import { mergeMap, take, tap } from 'rxjs/operators'; +import _ from 'lodash'; +import { BehaviorSubject, Observable, of, throwError } from 'rxjs'; +import { mergeMap, retryWhen, take, tap } from 'rxjs/operators'; import { RgwDaemon } from '~/app/ceph/rgw/models/rgw-daemon'; import { cdEncode } from '~/app/shared/decorators/cd-encode'; @@ -13,9 +14,9 @@ import { cdEncode } from '~/app/shared/decorators/cd-encode'; }) export class RgwDaemonService { private url = 'api/rgw/daemon'; - private daemons = new ReplaySubject(1); + private daemons = new BehaviorSubject([]); daemons$ = this.daemons.asObservable(); - private selectedDaemon = new ReplaySubject(1); + private selectedDaemon = new BehaviorSubject(null); selectedDaemon$ = this.selectedDaemon.asObservable(); constructor(private http: HttpClient) {} @@ -24,6 +25,9 @@ export class RgwDaemonService { return this.http.get(this.url).pipe( tap((daemons: RgwDaemon[]) => { this.daemons.next(daemons); + if (_.isEmpty(this.selectedDaemon.getValue())) { + this.selectDefaultDaemon(daemons); + } }) ); } @@ -36,7 +40,11 @@ export class RgwDaemonService { this.selectedDaemon.next(daemon); } - selectDefaultDaemon(daemons: RgwDaemon[]): RgwDaemon { + private selectDefaultDaemon(daemons: RgwDaemon[]): RgwDaemon { + if (daemons.length === 0) { + return null; + } + for (const daemon of daemons) { if (daemon.default) { this.selectDaemon(daemon); @@ -44,11 +52,24 @@ export class RgwDaemonService { } } - throw new Error('No default RGW daemon found.'); + this.selectDaemon(daemons[0]); + return daemons[0]; } request(next: (params: HttpParams) => Observable) { return this.selectedDaemon.pipe( + mergeMap((daemon: RgwDaemon) => + // If there is no selected daemon, retrieve daemon list (default daemon will be selected) + // and try again if daemon list is not empty. + _.isEmpty(daemon) + ? this.list().pipe(mergeMap((daemons) => throwError(!_.isEmpty(daemons)))) + : of(daemon) + ), + retryWhen((error) => + error.pipe( + mergeMap((hasToRetry) => (hasToRetry ? error : throwError('No RGW daemons found!'))) + ) + ), take(1), mergeMap((daemon: RgwDaemon) => { let params = new HttpParams(); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-site.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-site.service.spec.ts index d11501b3511..fa769d88b73 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-site.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-site.service.spec.ts @@ -16,6 +16,7 @@ describe('RgwSiteService', () => { beforeEach(() => { service = TestBed.inject(RgwSiteService); httpTesting = TestBed.inject(HttpTestingController); + RgwHelper.selectDaemon(); }); afterEach(() => { @@ -28,7 +29,6 @@ describe('RgwSiteService', () => { it('should contain site endpoint in GET request', () => { service.get().subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`${service['url']}?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('GET'); }); @@ -36,7 +36,6 @@ describe('RgwSiteService', () => { it('should add query param in GET request', () => { const query = 'placement-targets'; service.get(query).subscribe(); - RgwHelper.getCurrentDaemon(); httpTesting.expectOne( `${service['url']}?${RgwHelper.DAEMON_QUERY_PARAM}&query=placement-targets` ); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts index e85fe9d1138..7884f2385f2 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts @@ -18,6 +18,7 @@ describe('RgwUserService', () => { beforeEach(() => { service = TestBed.inject(RgwUserService); httpTesting = TestBed.inject(HttpTestingController); + RgwHelper.selectDaemon(); }); afterEach(() => { @@ -33,7 +34,6 @@ describe('RgwUserService', () => { service.list().subscribe((resp) => { result = resp; }); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('GET'); req.flush([]); @@ -45,7 +45,6 @@ describe('RgwUserService', () => { service.list().subscribe((resp) => { result = resp; }); - RgwHelper.getCurrentDaemon(); let req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('GET'); req.flush(['foo', 'bar']); @@ -63,35 +62,30 @@ describe('RgwUserService', () => { it('should call enumerate', () => { service.enumerate().subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('GET'); }); it('should call get', () => { service.get('foo').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/user/foo?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('GET'); }); it('should call getQuota', () => { service.getQuota('foo').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/user/foo/quota?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('GET'); }); it('should call update', () => { service.update('foo', { xxx: 'yyy' }).subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/user/foo?${RgwHelper.DAEMON_QUERY_PARAM}&xxx=yyy`); expect(req.request.method).toBe('PUT'); }); it('should call updateQuota', () => { service.updateQuota('foo', { xxx: 'yyy' }).subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/user/foo/quota?${RgwHelper.DAEMON_QUERY_PARAM}&xxx=yyy` ); @@ -100,21 +94,18 @@ describe('RgwUserService', () => { it('should call create', () => { service.create({ foo: 'bar' }).subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}&foo=bar`); expect(req.request.method).toBe('POST'); }); it('should call delete', () => { service.delete('foo').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne(`api/rgw/user/foo?${RgwHelper.DAEMON_QUERY_PARAM}`); expect(req.request.method).toBe('DELETE'); }); it('should call createSubuser', () => { service.createSubuser('foo', { xxx: 'yyy' }).subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/user/foo/subuser?${RgwHelper.DAEMON_QUERY_PARAM}&xxx=yyy` ); @@ -123,7 +114,6 @@ describe('RgwUserService', () => { it('should call deleteSubuser', () => { service.deleteSubuser('foo', 'bar').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/user/foo/subuser/bar?${RgwHelper.DAEMON_QUERY_PARAM}` ); @@ -132,7 +122,6 @@ describe('RgwUserService', () => { it('should call addCapability', () => { service.addCapability('foo', 'bar', 'baz').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/user/foo/capability?${RgwHelper.DAEMON_QUERY_PARAM}&type=bar&perm=baz` ); @@ -141,7 +130,6 @@ describe('RgwUserService', () => { it('should call deleteCapability', () => { service.deleteCapability('foo', 'bar', 'baz').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/user/foo/capability?${RgwHelper.DAEMON_QUERY_PARAM}&type=bar&perm=baz` ); @@ -150,7 +138,6 @@ describe('RgwUserService', () => { it('should call addS3Key', () => { service.addS3Key('foo', { xxx: 'yyy' }).subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/user/foo/key?${RgwHelper.DAEMON_QUERY_PARAM}&key_type=s3&xxx=yyy` ); @@ -159,7 +146,6 @@ describe('RgwUserService', () => { it('should call deleteS3Key', () => { service.deleteS3Key('foo', 'bar').subscribe(); - RgwHelper.getCurrentDaemon(); const req = httpTesting.expectOne( `api/rgw/user/foo/key?${RgwHelper.DAEMON_QUERY_PARAM}&key_type=s3&access_key=bar` ); diff --git a/src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts b/src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts index c45f480a700..f26164fc8ba 100644 --- a/src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts +++ b/src/pybind/mgr/dashboard/frontend/src/testing/unit-test-helper.ts @@ -388,15 +388,24 @@ export class IscsiHelper { } export class RgwHelper { - static readonly DAEMON_NAME = 'daemon1'; - static readonly DAEMON_QUERY_PARAM = `daemon_name=${RgwHelper.DAEMON_NAME}`; + static readonly daemons = RgwHelper.getDaemonList(); + static readonly DAEMON_QUERY_PARAM = `daemon_name=${RgwHelper.daemons[0].id}`; + + static getDaemonList() { + const daemonList: RgwDaemon[] = []; + for (let daemonIndex = 1; daemonIndex <= 3; daemonIndex++) { + const rgwDaemon = new RgwDaemon(); + rgwDaemon.id = `daemon${daemonIndex}`; + rgwDaemon.default = daemonIndex === 2; + rgwDaemon.zonegroup_name = `zonegroup${daemonIndex}`; + daemonList.push(rgwDaemon); + } + return daemonList; + } - static getCurrentDaemon() { - const rgwDaemon = new RgwDaemon(); - rgwDaemon.id = this.DAEMON_NAME; - rgwDaemon.default = true; + static selectDaemon() { const service = TestBed.inject(RgwDaemonService); - service.selectDaemon(rgwDaemon); + service.selectDaemon(this.daemons[0]); } } diff --git a/src/pybind/mgr/dashboard/rest_client.py b/src/pybind/mgr/dashboard/rest_client.py index 4550c092e8a..90cfd41ad77 100644 --- a/src/pybind/mgr/dashboard/rest_client.py +++ b/src/pybind/mgr/dashboard/rest_client.py @@ -445,7 +445,7 @@ class RestClient(object): "{}" # TODO remove .format(self.client_name, resp.status_code, pf( resp.content)), - resp.status_code, + self._handle_response_status_code(resp.status_code), resp.content) except ConnectionError as ex: if ex.args: @@ -506,13 +506,20 @@ class RestClient(object): logger.exception(msg) raise RequestException(msg) + @staticmethod + def _handle_response_status_code(status_code: int) -> int: + """ + Method to be overridden by subclasses that need specific handling. + """ + return status_code + @staticmethod def api(path, **api_kwargs): def call_decorator(func): def func_wrapper(self, *args, **kwargs): method = api_kwargs.get('method', None) resp_structure = api_kwargs.get('resp_structure', None) - args_name = inspect.getargspec(func).args + args_name = inspect.getfullargspec(func).args args_dict = dict(zip(args_name[1:], args)) for key, val in kwargs.items(): args_dict[key] = val diff --git a/src/pybind/mgr/dashboard/services/ceph_service.py b/src/pybind/mgr/dashboard/services/ceph_service.py index fa97b33ea34..7b2aa5fde55 100644 --- a/src/pybind/mgr/dashboard/services/ceph_service.py +++ b/src/pybind/mgr/dashboard/services/ceph_service.py @@ -67,23 +67,30 @@ class CephService(object): return [svc for _, svcs in service_map.items() for svc in svcs['services']] @classmethod - def get_service(cls, service_name, service_id): + def get_service_data_by_metadata_id(cls, + service_type: str, + metadata_id: str) -> Optional[Dict[str, Any]]: for server in mgr.list_servers(): for service in server['services']: - if service['type'] == service_name: - inst_id = service['id'] - if inst_id == service_id: - metadata = mgr.get_metadata(service_name, inst_id) - status = mgr.get_daemon_status(service_name, inst_id) + if service['type'] == service_type: + metadata = mgr.get_metadata(service_type, service['id']) + if metadata_id == metadata['id']: return { - 'id': inst_id, - 'type': service_name, + 'id': metadata['id'], + 'service_map_id': str(service['id']), + 'type': service_type, 'hostname': server['hostname'], - 'metadata': metadata, - 'status': status + 'metadata': metadata } return None + @classmethod + def get_service(cls, service_type: str, metadata_id: str) -> Optional[Dict[str, Any]]: + svc_data = cls.get_service_data_by_metadata_id(service_type, metadata_id) + if svc_data: + svc_data['status'] = mgr.get_daemon_status(svc_data['type'], svc_data['service_map_id']) + return svc_data + @classmethod def get_pool_list(cls, application=None): osd_map = mgr.get('osd_map') diff --git a/src/pybind/mgr/dashboard/services/rgw_client.py b/src/pybind/mgr/dashboard/services/rgw_client.py index 4c3e2821c9c..598d05ee39d 100644 --- a/src/pybind/mgr/dashboard/services/rgw_client.py +++ b/src/pybind/mgr/dashboard/services/rgw_client.py @@ -22,6 +22,11 @@ except ImportError: logger = logging.getLogger('rgw_client') +class NoRgwDaemonsException(Exception): + def __init__(self): + super().__init__('No RGW service is running.') + + class NoCredentialsException(RequestException): def __init__(self): super(NoCredentialsException, self).__init__( @@ -42,61 +47,23 @@ class RgwDaemon: def _get_daemons() -> Dict[str, RgwDaemon]: """ Retrieve RGW daemon info from MGR. - Note, the service id of the RGW daemons may differ depending on the setup. - Example 1: - { - ... - 'services': { - 'rgw': { - 'daemons': { - 'summary': '', - '0': { - ... - 'addr': '[2001:db8:85a3::8a2e:370:7334]:49774/1534999298', - 'metadata': { - 'frontend_config#0': 'civetweb port=7280', - } - ... - } - } - } - } - } - Example 2: - { - ... - 'services': { - 'rgw': { - 'daemons': { - 'summary': '', - 'rgw': { - ... - 'addr': '192.168.178.3:49774/1534999298', - 'metadata': { - 'frontend_config#0': 'civetweb port=8000', - } - ... - } - } - } - } - } """ service_map = mgr.get('service_map') if not dict_contains_path(service_map, ['services', 'rgw', 'daemons']): - raise LookupError('No RGW found') + raise NoRgwDaemonsException + daemons = {} daemon_map = service_map['services']['rgw']['daemons'] for key in daemon_map.keys(): if dict_contains_path(daemon_map[key], ['metadata', 'frontend_config#0']): daemon = _determine_rgw_addr(daemon_map[key]) - daemon.name = key + daemon.name = daemon_map[key]['metadata']['id'] daemon.zonegroup_name = daemon_map[key]['metadata']['zonegroup_name'] daemons[daemon.name] = daemon logger.info('Found RGW daemon with configuration: host=%s, port=%d, ssl=%s', daemon.host, daemon.port, str(daemon.ssl)) if not daemons: - raise LookupError('No RGW daemon found') + raise NoRgwDaemonsException return daemons @@ -229,6 +196,11 @@ class RgwClient(RestClient): got_keys_from_config: bool userid: str + @staticmethod + def _handle_response_status_code(status_code: int) -> int: + # Do not return auth error codes (so they are not handled as ceph API user auth errors). + return 404 if status_code in [401, 403] else status_code + @staticmethod def _get_daemon_connection_info(daemon_name: str) -> dict: try: @@ -238,6 +210,10 @@ class RgwClient(RestClient): # Legacy string values. access_key = Settings.RGW_API_ACCESS_KEY secret_key = Settings.RGW_API_SECRET_KEY + except KeyError as error: + raise DashboardException(msg='Credentials not found for RGW Daemon: {}'.format(error), + http_status_code=404, + component='rgw') return {'access_key': access_key, 'secret_key': secret_key} @@ -261,6 +237,9 @@ class RgwClient(RestClient): def instance(userid: Optional[str] = None, daemon_name: Optional[str] = None) -> 'RgwClient': # pylint: disable=too-many-branches + + RgwClient._daemons = _get_daemons() + # The API access key and secret key are mandatory for a minimal configuration. if not (Settings.RGW_API_ACCESS_KEY and Settings.RGW_API_SECRET_KEY): logger.warning('No credentials found, please consult the ' @@ -268,9 +247,6 @@ class RgwClient(RestClient): 'dashboard.') raise NoCredentialsException() - if not RgwClient._daemons: - RgwClient._daemons = _get_daemons() - if not daemon_name: # Select default daemon if configured in settings: if Settings.RGW_API_HOST and Settings.RGW_API_PORT: @@ -280,9 +256,12 @@ class RgwClient(RestClient): daemon_name = daemon.name break if not daemon_name: - raise LookupError('No RGW daemon found with host: {}, port: {}'.format( - Settings.RGW_API_HOST, - Settings.RGW_API_PORT)) + raise DashboardException( + msg='No RGW daemon found with user-defined host: {}, port: {}'.format( + Settings.RGW_API_HOST, + Settings.RGW_API_PORT), + http_status_code=404, + component='rgw') # Select 1st daemon: else: daemon_name = next(iter(RgwClient._daemons.keys())) @@ -349,7 +328,12 @@ class RgwClient(RestClient): secret_key, daemon_name, user_id=None): - daemon = RgwClient._daemons[daemon_name] + try: + daemon = RgwClient._daemons[daemon_name] + except KeyError as error: + raise DashboardException(msg='RGW Daemon not found: {}'.format(error), + http_status_code=404, + component='rgw') ssl_verify = Settings.RGW_API_SSL_VERIFY self.admin_path = Settings.RGW_API_ADMIN_RESOURCE self.service_url = build_url(host=daemon.host, port=daemon.port) @@ -366,10 +350,11 @@ class RgwClient(RestClient): self.userid = self._get_user_id(self.admin_path) if self.got_keys_from_config \ else user_id except RequestException as error: - # Avoid dashboard GUI redirections caused by status code (403, ...): - http_status_code = 400 if 400 <= error.status_code < 500 else error.status_code - raise DashboardException(msg='Error connecting to Object Gateway.', - http_status_code=http_status_code, + msg = 'Error connecting to Object Gateway' + if error.status_code == 404: + msg = '{}: {}'.format(msg, str(error)) + raise DashboardException(msg=msg, + http_status_code=error.status_code, component='rgw') self.daemon = daemon @@ -377,7 +362,7 @@ class RgwClient(RestClient): daemon.name, daemon.host, daemon.port, daemon.ssl, ssl_verify) @RestClient.api_get('/', resp_structure='[0] > (ID & DisplayName)') - def is_service_online(self, request=None): + def is_service_online(self, request=None) -> bool: """ Consider the service as online if the response contains the specified keys. Nothing more is checked here. @@ -412,12 +397,12 @@ class RgwClient(RestClient): @RestClient.api_get('/{admin_path}/metadata/user?key={userid}', resp_structure='data > system') - def _is_system_user(self, admin_path, userid, request=None): + def _is_system_user(self, admin_path, userid, request=None) -> bool: # pylint: disable=unused-argument response = request() return strtobool(response['data']['system']) - def is_system_user(self): + def is_system_user(self) -> bool: return self._is_system_user(self.admin_path, self.userid) @RestClient.api_get( @@ -577,12 +562,11 @@ class RgwClient(RestClient): request(data=data, headers=headers) except RequestException as error: msg = str(error) - if error.status_code == 403: + if mfa_delete and mfa_token_serial and mfa_token_pin \ + and 'AccessDenied' in error.content.decode(): msg = 'Bad MFA credentials: {}'.format(msg) - # Avoid dashboard GUI redirections caused by status code (403, ...): - http_status_code = 400 if 400 <= error.status_code < 500 else error.status_code raise DashboardException(msg=msg, - http_status_code=http_status_code, + http_status_code=error.status_code, component='rgw') @RestClient.api_get('/{bucket_name}?object-lock') diff --git a/src/pybind/mgr/dashboard/tests/__init__.py b/src/pybind/mgr/dashboard/tests/__init__.py index 9ea4585b25f..72d5204b658 100644 --- a/src/pybind/mgr/dashboard/tests/__init__.py +++ b/src/pybind/mgr/dashboard/tests/__init__.py @@ -6,6 +6,7 @@ import json import logging import threading import time +from unittest.mock import Mock import cherrypy from cherrypy._cptools import HandlerWrapperTool @@ -278,3 +279,47 @@ class ControllerTestCase(helper.CPWebCase): if msg is None: msg = 'expected %r to be in %r' % (data, json_body) self._handlewebError(msg) + + +class Stub: + """Test class for returning predefined values""" + + @classmethod + def get_mgr_no_services(cls): + mgr.get = Mock(return_value={}) + + +class RgwStub(Stub): + + @classmethod + def get_daemons(cls): + mgr.get = Mock(return_value={'services': {'rgw': {'daemons': { + '5297': { + 'addr': '192.168.178.3:49774/1534999298', + 'metadata': { + 'frontend_config#0': 'beast port=8000', + 'id': 'daemon1', + 'zonegroup_name': 'zonegroup1', + 'zone_name': 'zone1' + } + }, + '5398': { + 'addr': '[2001:db8:85a3::8a2e:370:7334]:49774/1534999298', + 'metadata': { + 'frontend_config#0': 'civetweb port=8002', + 'id': 'daemon2', + 'zonegroup_name': 'zonegroup2', + 'zone_name': 'zone2' + } + } + }}}}) + + @classmethod + def get_settings(cls): + settings = { + 'RGW_API_HOST': '', + 'RGW_API_PORT': 0, + 'RGW_API_ACCESS_KEY': 'fake-access-key', + 'RGW_API_SECRET_KEY': 'fake-secret-key', + } + mgr.get_module_option = Mock(side_effect=settings.get) diff --git a/src/pybind/mgr/dashboard/tests/test_rest_client.py b/src/pybind/mgr/dashboard/tests/test_rest_client.py index ba71b4f153e..2df6763f98f 100644 --- a/src/pybind/mgr/dashboard/tests/test_rest_client.py +++ b/src/pybind/mgr/dashboard/tests/test_rest_client.py @@ -14,6 +14,13 @@ from .. import mgr from ..rest_client import RequestException, RestClient +class RestClientTestClass(RestClient): + """RestClient subclass for testing purposes.""" + @RestClient.api_get('/') + def fake_endpoint_method_with_annotation(self, request=None) -> bool: + pass + + class RestClientTest(unittest.TestCase): def setUp(self): settings = {'REST_REQUESTS_TIMEOUT': 45} @@ -56,7 +63,10 @@ class RestClientDoRequestTest(unittest.TestCase): @classmethod def setUpClass(cls): cls.mock_requests = patch('requests.Session').start() - cls.rest_client = RestClient('localhost', 8000, 'UnitTest') + cls.rest_client = RestClientTestClass('localhost', 8000, 'UnitTest') + + def test_endpoint_method_with_annotation(self): + self.assertEqual(self.rest_client.fake_endpoint_method_with_annotation(), None) def test_do_request_exception_no_args(self): self.mock_requests().get.side_effect = requests.exceptions.ConnectionError() diff --git a/src/pybind/mgr/dashboard/tests/test_rgw.py b/src/pybind/mgr/dashboard/tests/test_rgw.py index 5201f8d1115..bf56464c063 100644 --- a/src/pybind/mgr/dashboard/tests/test_rgw.py +++ b/src/pybind/mgr/dashboard/tests/test_rgw.py @@ -1,11 +1,10 @@ -try: - import mock -except ImportError: - import unittest.mock as mock +from unittest.mock import Mock, call, patch from .. import mgr -from ..controllers.rgw import Rgw, RgwUser -from . import ControllerTestCase # pylint: disable=no-name-in-module +from ..controllers.rgw import Rgw, RgwDaemon, RgwUser +from ..rest_client import RequestException +from ..services.rgw_client import RgwClient +from . import ControllerTestCase, RgwStub # pylint: disable=no-name-in-module class RgwControllerTestCase(ControllerTestCase): @@ -14,20 +13,111 @@ class RgwControllerTestCase(ControllerTestCase): Rgw._cp_config['tools.authenticate.on'] = False # pylint: disable=protected-access cls.setup_controllers([Rgw], '/test') + def setUp(self) -> None: + RgwStub.get_daemons() + RgwStub.get_settings() + + @patch.object(RgwClient, '_get_user_id', Mock(return_value='fake-user')) + @patch.object(RgwClient, 'is_service_online', Mock(return_value=True)) + @patch.object(RgwClient, '_is_system_user', Mock(return_value=True)) + def test_status_available(self): + self._get('/test/api/rgw/status') + self.assertStatus(200) + self.assertJsonBody({'available': True, 'message': None}) + + @patch.object(RgwClient, '_get_user_id', Mock(return_value='fake-user')) + @patch.object(RgwClient, 'is_service_online', Mock( + side_effect=RequestException('My test error'))) + def test_status_online_check_error(self): + self._get('/test/api/rgw/status') + self.assertStatus(200) + self.assertJsonBody({'available': False, + 'message': 'My test error'}) + + @patch.object(RgwClient, '_get_user_id', Mock(return_value='fake-user')) + @patch.object(RgwClient, 'is_service_online', Mock(return_value=False)) + def test_status_not_online(self): + self._get('/test/api/rgw/status') + self.assertStatus(200) + self.assertJsonBody({'available': False, + 'message': "Failed to connect to the Object Gateway's Admin Ops API."}) + + @patch.object(RgwClient, '_get_user_id', Mock(return_value='fake-user')) + @patch.object(RgwClient, 'is_service_online', Mock(return_value=True)) + @patch.object(RgwClient, '_is_system_user', Mock(return_value=False)) + def test_status_not_system_user(self): + self._get('/test/api/rgw/status') + self.assertStatus(200) + self.assertJsonBody({'available': False, + 'message': 'The system flag is not set for user "fake-user".'}) + def test_status_no_service(self): - mgr.list_servers.return_value = [] + RgwStub.get_mgr_no_services() self._get('/test/api/rgw/status') self.assertStatus(200) self.assertJsonBody({'available': False, 'message': 'No RGW service is running.'}) +class RgwDaemonControllerTestCase(ControllerTestCase): + @classmethod + def setup_server(cls): + RgwDaemon._cp_config['tools.authenticate.on'] = False # pylint: disable=protected-access + cls.setup_controllers([RgwDaemon], '/test') + + @patch('dashboard.services.rgw_client.RgwClient._get_user_id', Mock( + return_value='dummy_admin')) + def test_list(self): + RgwStub.get_daemons() + RgwStub.get_settings() + mgr.list_servers.return_value = [{ + 'hostname': 'host1', + 'services': [{'id': 'daemon1', 'type': 'rgw'}, {'id': 'daemon2', 'type': 'rgw'}] + }] + mgr.get_metadata.side_effect = [ + { + 'ceph_version': 'ceph version master (dev)', + 'id': 'daemon1', + 'zonegroup_name': 'zg1', + 'zone_name': 'zone1' + }, + { + 'ceph_version': 'ceph version master (dev)', + 'id': 'daemon2', + 'zonegroup_name': 'zg2', + 'zone_name': 'zone2' + }] + self._get('/test/api/rgw/daemon') + self.assertStatus(200) + self.assertJsonBody([{ + 'id': 'daemon1', + 'version': 'ceph version master (dev)', + 'server_hostname': 'host1', + 'zonegroup_name': 'zg1', + 'zone_name': 'zone1', 'default': True + }, + { + 'id': 'daemon2', + 'version': 'ceph version master (dev)', + 'server_hostname': 'host1', + 'zonegroup_name': 'zg2', + 'zone_name': 'zone2', + 'default': False + }]) + + def test_list_empty(self): + RgwStub.get_mgr_no_services() + self._get('/test/api/rgw/daemon') + self.assertStatus(200) + self.assertJsonBody([]) + + class RgwUserControllerTestCase(ControllerTestCase): @classmethod def setup_server(cls): RgwUser._cp_config['tools.authenticate.on'] = False # pylint: disable=protected-access cls.setup_controllers([RgwUser], '/test') - @mock.patch('dashboard.controllers.rgw.RgwRESTController.proxy') + @patch('dashboard.controllers.rgw.RgwRESTController.proxy') def test_user_list(self, mock_proxy): mock_proxy.side_effect = [{ 'count': 3, @@ -37,11 +127,11 @@ class RgwUserControllerTestCase(ControllerTestCase): self._get('/test/api/rgw/user?daemon_name=dummy-daemon') self.assertStatus(200) mock_proxy.assert_has_calls([ - mock.call('dummy-daemon', 'GET', 'user?list', {}) + call('dummy-daemon', 'GET', 'user?list', {}) ]) self.assertJsonBody(['test1', 'test2', 'test3']) - @mock.patch('dashboard.controllers.rgw.RgwRESTController.proxy') + @patch('dashboard.controllers.rgw.RgwRESTController.proxy') def test_user_list_marker(self, mock_proxy): mock_proxy.side_effect = [{ 'count': 3, @@ -56,12 +146,12 @@ class RgwUserControllerTestCase(ControllerTestCase): self._get('/test/api/rgw/user') self.assertStatus(200) mock_proxy.assert_has_calls([ - mock.call(None, 'GET', 'user?list', {}), - mock.call(None, 'GET', 'user?list', {'marker': 'foo:bar'}) + call(None, 'GET', 'user?list', {}), + call(None, 'GET', 'user?list', {'marker': 'foo:bar'}) ]) self.assertJsonBody(['test1', 'test2', 'test3', 'admin']) - @mock.patch('dashboard.controllers.rgw.RgwRESTController.proxy') + @patch('dashboard.controllers.rgw.RgwRESTController.proxy') def test_user_list_duplicate_marker(self, mock_proxy): mock_proxy.side_effect = [{ 'count': 3, @@ -81,7 +171,7 @@ class RgwUserControllerTestCase(ControllerTestCase): self._get('/test/api/rgw/user') self.assertStatus(500) - @mock.patch('dashboard.controllers.rgw.RgwRESTController.proxy') + @patch('dashboard.controllers.rgw.RgwRESTController.proxy') def test_user_list_invalid_marker(self, mock_proxy): mock_proxy.side_effect = [{ 'count': 3, @@ -101,8 +191,8 @@ class RgwUserControllerTestCase(ControllerTestCase): self._get('/test/api/rgw/user') self.assertStatus(500) - @mock.patch('dashboard.controllers.rgw.RgwRESTController.proxy') - @mock.patch.object(RgwUser, '_keys_allowed') + @patch('dashboard.controllers.rgw.RgwRESTController.proxy') + @patch.object(RgwUser, '_keys_allowed') def test_user_get_with_keys(self, keys_allowed, mock_proxy): keys_allowed.return_value = True mock_proxy.return_value = { @@ -116,8 +206,8 @@ class RgwUserControllerTestCase(ControllerTestCase): self.assertInJsonBody('keys') self.assertInJsonBody('swift_keys') - @mock.patch('dashboard.controllers.rgw.RgwRESTController.proxy') - @mock.patch.object(RgwUser, '_keys_allowed') + @patch('dashboard.controllers.rgw.RgwRESTController.proxy') + @patch.object(RgwUser, '_keys_allowed') def test_user_get_without_keys(self, keys_allowed, mock_proxy): keys_allowed.return_value = False mock_proxy.return_value = { diff --git a/src/pybind/mgr/dashboard/tests/test_rgw_client.py b/src/pybind/mgr/dashboard/tests/test_rgw_client.py index f12fa4aa8d5..edc10071f35 100644 --- a/src/pybind/mgr/dashboard/tests/test_rgw_client.py +++ b/src/pybind/mgr/dashboard/tests/test_rgw_client.py @@ -1,33 +1,20 @@ # -*- coding: utf-8 -*- # pylint: disable=too-many-public-methods -import unittest +from unittest import TestCase +from unittest.mock import Mock, patch -try: - from unittest.mock import MagicMock, patch -except ImportError: - from mock import MagicMock, patch # type: ignore - -from ..services.rgw_client import NoCredentialsException, RgwClient, \ - RgwDaemon, _parse_frontend_config +from ..exceptions import DashboardException +from ..services.rgw_client import NoCredentialsException, \ + NoRgwDaemonsException, RgwClient, _parse_frontend_config from ..settings import Settings -from . import KVStoreMockMixin # pylint: disable=no-name-in-module - - -def _get_daemons_stub(): - daemon = RgwDaemon() - daemon.host = 'rgw.1.myorg.com' - daemon.port = 8000 - daemon.ssl = True - daemon.name = 'rgw.1.myorg.com' - daemon.zonegroup_name = 'zonegroup2-realm1' - return {daemon.name: daemon} +from . import KVStoreMockMixin, RgwStub # pylint: disable=no-name-in-module -@patch('dashboard.services.rgw_client._get_daemons', _get_daemons_stub) -@patch('dashboard.services.rgw_client.RgwClient._get_user_id', MagicMock( +@patch('dashboard.services.rgw_client.RgwClient._get_user_id', Mock( return_value='dummy_admin')) -class RgwClientTest(unittest.TestCase, KVStoreMockMixin): +class RgwClientTest(TestCase, KVStoreMockMixin): def setUp(self): + RgwStub.get_daemons() self.mock_kv_store() self.CONFIG_KEY_DICT.update({ 'RGW_API_ACCESS_KEY': 'klausmustermann', @@ -44,6 +31,12 @@ class RgwClientTest(unittest.TestCase, KVStoreMockMixin): instance = RgwClient.admin_instance() self.assertFalse(instance.session.verify) + def test_no_daemons(self): + RgwStub.get_mgr_no_services() + with self.assertRaises(NoRgwDaemonsException) as cm: + RgwClient.admin_instance() + self.assertIn('No RGW service is running.', str(cm.exception)) + def test_no_credentials(self): self.CONFIG_KEY_DICT.update({ 'RGW_API_ACCESS_KEY': '', @@ -55,12 +48,12 @@ class RgwClientTest(unittest.TestCase, KVStoreMockMixin): def test_default_daemon_wrong_settings(self): self.CONFIG_KEY_DICT.update({ - 'RGW_API_HOST': 'localhost', + 'RGW_API_HOST': '172.20.0.2', 'RGW_API_PORT': '7990', }) - with self.assertRaises(LookupError) as cm: + with self.assertRaises(DashboardException) as cm: RgwClient.admin_instance() - self.assertIn('No RGW daemon found with host:', str(cm.exception)) + self.assertIn('No RGW daemon found with user-defined host:', str(cm.exception)) @patch.object(RgwClient, '_get_daemon_zone_info') def test_get_placement_targets_from_zone(self, zone_info): @@ -83,7 +76,7 @@ class RgwClientTest(unittest.TestCase, KVStoreMockMixin): instance = RgwClient.admin_instance() expected_result = { - 'zonegroup': 'zonegroup2-realm1', + 'zonegroup': 'zonegroup1', 'placement_targets': [ { 'name': 'default-placement', @@ -111,7 +104,7 @@ class RgwClientTest(unittest.TestCase, KVStoreMockMixin): self.assertEqual([], instance.get_realms()) -class RgwClientHelperTest(unittest.TestCase): +class RgwClientHelperTest(TestCase): def test_parse_frontend_config_1(self): self.assertEqual(_parse_frontend_config('beast port=8000'), (8000, False)) -- 2.39.5