From fc08d2e63cd33f579f39ba14325f906b8324d85a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Stephan=20M=C3=BCller?= Date: Tue, 30 Apr 2019 16:19:39 +0200 Subject: [PATCH] mgr/dashboard: Watch for pool pg's increase and decrease MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Now when creating or editing a pool the background task will watch increasing and decreasing pg's. The executing task will also use a progress to show the progress. If the change was not executed through the dashboard, there is no task to show, but the frontend will make sure to inform the user about the change that is going on in order to stop the user from doing actions on the changing pool. Fixes: https://tracker.ceph.com/issues/39482 Signed-off-by: Stephan Müller --- src/pybind/mgr/dashboard/controllers/pool.py | 35 +++- .../pool-list/pool-list.component.spec.ts | 171 +++++++++++------- .../pool/pool-list/pool-list.component.ts | 7 + .../frontend/src/app/ceph/pool/pool.ts | 7 +- .../shared/services/task-list.service.spec.ts | 11 +- .../app/shared/services/task-list.service.ts | 7 +- src/pybind/mgr/dashboard/tests/test_pool.py | 89 +++++++++ 7 files changed, 254 insertions(+), 73 deletions(-) create mode 100644 src/pybind/mgr/dashboard/tests/test_pool.py diff --git a/src/pybind/mgr/dashboard/controllers/pool.py b/src/pybind/mgr/dashboard/controllers/pool.py index 5edfb91bba4..0a574595d21 100644 --- a/src/pybind/mgr/dashboard/controllers/pool.py +++ b/src/pybind/mgr/dashboard/controllers/pool.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import +import time import cherrypy from . import ApiController, RESTController, Endpoint, ReadPermission, Task @@ -9,7 +10,7 @@ from ..security import Scope from ..services.ceph_service import CephService from ..services.rbd import RbdConfiguration from ..services.exception import handle_send_command_error -from ..tools import str_to_bool +from ..tools import str_to_bool, TaskManager def pool_task(name, metadata, wait_for=2.0): @@ -81,6 +82,7 @@ class Pool(RESTController): def set(self, pool_name, flags=None, application_metadata=None, configuration=None, **kwargs): self._set_pool_values(pool_name, application_metadata, flags, True, kwargs) RbdConfiguration(pool_name).set_configuration(configuration) + self._wait_for_pgs(pool_name) @pool_task('create', {'pool_name': '{pool}'}) @handle_send_command_error('pool') @@ -92,6 +94,7 @@ class Pool(RESTController): rule=rule_name) self._set_pool_values(pool, application_metadata, flags, False, kwargs) RbdConfiguration(pool).set_configuration(configuration) + self._wait_for_pgs(pool) def _set_pool_values(self, pool, application_metadata, flags, update_existing, kwargs): update_name = False @@ -151,6 +154,36 @@ class Pool(RESTController): reset_arg(arg, '0') reset_arg('compression_algorithm', 'unset') + def _wait_for_pgs(self, pool_name): + """ + Keep the task waiting for until all pg changes are complete + :param pool_name: The name of the pool. + :type pool_name: string + """ + current_pool = self._get(pool_name) + initial_pgs = int(current_pool['pg_placement_num']) + int(current_pool['pg_num']) + self._pg_wait_loop(current_pool, initial_pgs) + + def _pg_wait_loop(self, pool, initial_pgs): + """ + Compares if all pg changes are completed, if not it will call itself + until all changes are completed. + :param pool: The dict that represents a pool. + :type pool: dict + :param initial_pgs: The pg and pg_num count before any change happened. + :type initial_pgs: int + """ + if 'pg_num_target' in pool: + target = int(pool['pg_num_target']) + int(pool['pg_placement_num_target']) + current = int(pool['pg_placement_num']) + int(pool['pg_num']) + if current != target: + max_diff = abs(target - initial_pgs) + diff = max_diff - abs(target - current) + percentage = int(round(diff / float(max_diff) * 100)) + TaskManager.current_task().set_progress(percentage) + time.sleep(4) + self._pg_wait_loop(self._get(pool['pool_name']), initial_pgs) + @RESTController.Resource() @ReadPermission def configuration(self, pool_name): diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts index 3920939a897..b4ffdd0f0e2 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts @@ -27,18 +27,18 @@ describe('PoolListComponent', () => { let fixture: ComponentFixture; let poolService: PoolService; - const addPool = (pools, name, id) => { - const pool = new Pool(name); - pool.pool = id; - pool.pg_num = 256; - pools.push(pool); + const createPool = (name, id): Pool => { + return _.merge(new Pool(name), { + pool: id, + pg_num: 256, + pg_placement_num: 256, + pg_num_target: 256, + pg_placement_num_target: 256 + }); }; - const setUpPools = (pools) => { - addPool(pools, 'a', 0); - addPool(pools, 'b', 1); - addPool(pools, 'c', 2); - component.pools = pools; + const getPoolList = (): Pool[] => { + return [createPool('a', 0), createPool('b', 1), createPool('c', 2)]; }; configureTestBed({ @@ -58,6 +58,7 @@ describe('PoolListComponent', () => { component = fixture.componentInstance; component.permissions.pool.read = true; poolService = TestBed.get(PoolService); + spyOn(poolService, 'getList').and.callFake(() => of(getPoolList())); fixture.detectChanges(); }); @@ -69,6 +70,12 @@ describe('PoolListComponent', () => { expect(component.columns.every((column) => Boolean(column.prop))).toBeTruthy(); }); + it('returns pool details correctly', () => { + const pool = { prop1: 1, cdIsBinary: true, prop2: 2, cdExecuting: true, prop3: 3 }; + const expected = { prop1: 1, prop2: 2, prop3: 3 }; + expect(component.getPoolDetails(pool)).toEqual(expected); + }); + describe('monAllowPoolDelete', () => { let configurationService: ConfigurationService; @@ -166,7 +173,6 @@ describe('PoolListComponent', () => { }); describe('handling of executing tasks', () => { - let pools: Pool[]; let summaryService: SummaryService; const addTask = (name: string, pool: string) => { @@ -179,10 +185,6 @@ describe('PoolListComponent', () => { beforeEach(() => { summaryService = TestBed.get(SummaryService); summaryService['summaryDataSource'].next({ executing_tasks: [], finished_tasks: [] }); - pools = []; - setUpPools(pools); - spyOn(poolService, 'getList').and.callFake(() => of(pools)); - fixture.detectChanges(); }); it('gets all pools without executing pools', () => { @@ -279,39 +281,11 @@ describe('PoolListComponent', () => { }); describe('transformPoolsData', () => { - it('transforms pools data correctly', () => { - const pools = [ - { - stats: { - bytes_used: { latest: 5, rate: 0, rates: [] }, - max_avail: { latest: 15, rate: 0, rates: [] }, - rd_bytes: { latest: 6, rate: 4, rates: [[0, 2], [1, 6]] } - }, - pg_status: { 'active+clean': 8, down: 2 } - } - ]; - const expected = [ - { - cdIsBinary: true, - pg_status: '8 active+clean, 2 down', - stats: { - bytes_used: { latest: 5, rate: 0, rates: [] }, - max_avail: { latest: 15, rate: 0, rates: [] }, - rd: { latest: 0, rate: 0, rates: [] }, - rd_bytes: { latest: 6, rate: 4, rates: [2, 6] }, - wr: { latest: 0, rate: 0, rates: [] }, - wr_bytes: { latest: 0, rate: 0, rates: [] } - }, - usage: 0.25 - } - ]; - expect(component.transformPoolsData(pools)).toEqual(expected); - }); + let pool: Pool; - it('transforms pools data correctly if stats are missing', () => { - const pools = [{}]; - const expected = [ - { + const getPoolData = (o) => [ + _.merge( + _.merge(createPool('a', 0), { cdIsBinary: true, pg_status: '', stats: { @@ -323,15 +297,85 @@ describe('PoolListComponent', () => { wr_bytes: { latest: 0, rate: 0, rates: [] } }, usage: 0 - } - ]; - expect(component.transformPoolsData(pools)).toEqual(expected); + }), + o + ) + ]; + + beforeEach(() => { + pool = createPool('a', 0); + }); + + it('transforms pools data correctly', () => { + pool = _.merge(pool, { + stats: { + bytes_used: { latest: 5, rate: 0, rates: [] }, + max_avail: { latest: 15, rate: 0, rates: [] }, + rd_bytes: { latest: 6, rate: 4, rates: [[0, 2], [1, 6]] } + }, + pg_status: { 'active+clean': 8, down: 2 } + }); + expect(component.transformPoolsData([pool])).toEqual( + getPoolData({ + pg_status: '8 active+clean, 2 down', + stats: { + bytes_used: { latest: 5, rate: 0, rates: [] }, + max_avail: { latest: 15, rate: 0, rates: [] }, + rd_bytes: { latest: 6, rate: 4, rates: [2, 6] } + }, + usage: 0.25 + }) + ); + }); + + it('transforms pools data correctly if stats are missing', () => { + expect(component.transformPoolsData([pool])).toEqual(getPoolData({})); }); it('transforms empty pools data correctly', () => { - const pools = undefined; - const expected = undefined; - expect(component.transformPoolsData(pools)).toEqual(expected); + expect(component.transformPoolsData(undefined)).toEqual(undefined); + expect(component.transformPoolsData([])).toEqual([]); + }); + + it('shows not marked pools in progress if pg_num does not match pg_num_target', () => { + const pools = [ + _.merge(pool, { + pg_num: 32, + pg_num_target: 16, + pg_placement_num: 32, + pg_placement_num_target: 16 + }) + ]; + expect(component.transformPoolsData(pools)).toEqual( + getPoolData({ + cdExecuting: 'Updating', + pg_num: 32, + pg_num_target: 16, + pg_placement_num: 32, + pg_placement_num_target: 16 + }) + ); + }); + + it('shows marked pools in progress as defined by task', () => { + const pools = [ + _.merge(pool, { + pg_num: 32, + pg_num_target: 16, + pg_placement_num: 32, + pg_placement_num_target: 16, + cdExecuting: 'Updating 50%' + }) + ]; + expect(component.transformPoolsData(pools)).toEqual( + getPoolData({ + cdExecuting: 'Updating 50%', + pg_num: 32, + pg_num_target: 16, + pg_placement_num: 32, + pg_placement_num_target: 16 + }) + ); }); }); @@ -365,17 +409,7 @@ describe('PoolListComponent', () => { }); }); - describe('getPoolDetails', () => { - it('returns pool details corretly', () => { - const pool = { prop1: 1, cdIsBinary: true, prop2: 2, cdExecuting: true, prop3: 3 }; - const expected = { prop1: 1, prop2: 2, prop3: 3 }; - - expect(component.getPoolDetails(pool)).toEqual(expected); - }); - }); - describe('getSelectionTiers', () => { - let pools: Pool[]; const setSelectionTiers = (tiers: number[]) => { component.selection.selected = [ { @@ -387,18 +421,17 @@ describe('PoolListComponent', () => { }; beforeEach(() => { - pools = []; - setUpPools(pools); + component.pools = getPoolList(); }); it('should select multiple existing cache tiers', () => { setSelectionTiers([0, 1, 2]); - expect(component.selectionCacheTiers).toEqual(pools); + expect(component.selectionCacheTiers).toEqual(getPoolList()); }); it('should select correct existing cache tier', () => { setSelectionTiers([0]); - expect(component.selectionCacheTiers).toEqual([{ pg_num: 256, pool: 0, pool_name: 'a' }]); + expect(component.selectionCacheTiers).toEqual([createPool('a', 0)]); }); it('should not select cache tier if id is invalid', () => { @@ -413,9 +446,9 @@ describe('PoolListComponent', () => { it('should be able to selected one pool with multiple tiers, than with a single tier, than with no tiers', () => { setSelectionTiers([0, 1, 2]); - expect(component.selectionCacheTiers).toEqual(pools); + expect(component.selectionCacheTiers).toEqual(getPoolList()); setSelectionTiers([0]); - expect(component.selectionCacheTiers).toEqual([{ pg_num: 256, pool: 0, pool_name: 'a' }]); + expect(component.selectionCacheTiers).toEqual([createPool('a', 0)]); setSelectionTiers([]); expect(component.selectionCacheTiers).toEqual([]); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts index efed900ec42..7ff390c6cc8 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts @@ -248,6 +248,13 @@ export class PoolListComponent implements OnInit { const avail = stats.bytes_used.latest + stats.max_avail.latest; pool['usage'] = avail > 0 ? stats.bytes_used.latest / avail : avail; + if ( + !pool.cdExecuting && + pool.pg_num + pool.pg_placement_num !== pool.pg_num_target + pool.pg_placement_num_target + ) { + pool['cdExecuting'] = 'Updating'; + } + ['rd_bytes', 'wr_bytes'].forEach((stat) => { pool.stats[stat].rates = pool.stats[stat].rates.map((point) => point[1]); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool.ts index 013718d7ffd..0808a89dcfe 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool.ts @@ -21,6 +21,12 @@ export class Pool { min_read_recency_for_promote: number; target_max_objects: number; pg_num: number; + pg_num_target: number; + pg_num_pending: number; + pg_placement_num: number; + pg_placement_num_target: number; + pg_autoscale_mode: string; + pg_status: string; type: string; pool_name: string; cache_min_evict_age: number; @@ -57,7 +63,6 @@ export class Pool { last_change: string; min_write_recency_for_promote: number; read_tier: number; - pg_status: string; stats?: PoolStats; cdIsBinary?: boolean; configuration: { source: number; name: string; value: string }[]; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-list.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-list.service.spec.ts index 0c6465994c0..b984b31fabc 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-list.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-list.service.spec.ts @@ -62,9 +62,10 @@ describe('TaskListService', () => { expect(service).toBeTruthy(); }); - const addTask = (name: string, itemName: string) => { + const addTask = (name: string, itemName: string, progress?: number) => { const task = new ExecutingTask(); task.name = name; + task.progress = progress; task.metadata = { name: itemName }; tasks.push(task); summaryService.addRunningTask(task); @@ -85,6 +86,14 @@ describe('TaskListService', () => { expectItemTasks(list[3], 'Creating'); }); + it('shows progress of current task if any above 0', () => { + addTask('test/edit', 'd', 97); + addTask('test/edit', 'e', 0); + expect(list.length).toBe(5); + expectItemTasks(list[3], 'Updating 97%'); + expectItemTasks(list[4], 'Updating'); + }); + it('gets all items with one executing items', () => { addTask('test/create', 'a'); expect(list.length).toBe(3); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-list.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-list.service.ts index 20d5a962e36..5db78347652 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-list.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/task-list.service.ts @@ -90,7 +90,12 @@ export class TaskListService implements OnDestroy { if (tasks.length === 0) { return; } - return tasks.map((task) => this.taskMessageService.getRunningText(task)).join(', '); + return tasks + .map((task) => { + const progress = task.progress ? ` ${task.progress}%` : ''; + return this.taskMessageService.getRunningText(task) + progress; + }) + .join(', '); } ngOnDestroy() { diff --git a/src/pybind/mgr/dashboard/tests/test_pool.py b/src/pybind/mgr/dashboard/tests/test_pool.py new file mode 100644 index 00000000000..54fef139826 --- /dev/null +++ b/src/pybind/mgr/dashboard/tests/test_pool.py @@ -0,0 +1,89 @@ +# -*- coding: utf-8 -*- +# pylint: disable=protected-access +import mock + +from . import ControllerTestCase +from ..controllers.pool import Pool + + +class MockTask(object): + percentages = [] + + def set_progress(self, percentage): + self.percentages.append(percentage) + + +class PoolControllerTest(ControllerTestCase): + @classmethod + def setup_server(cls): + Pool._cp_config['tools.authenticate.on'] = False + cls.setup_controllers([Pool]) + + def test_creation(self): + self._post('/api/pool', { + 'pool': 'test-pool', + 'pool_type': 1, + 'pg_num': 64 + }) + self.assertStatus(202) + self.assertJsonBody({'name': 'pool/create', 'metadata': {'pool_name': 'test-pool'}}) + + @mock.patch('dashboard.controllers.pool.Pool._get') + def test_wait_for_pgs_without_waiting(self, _get): + _get.side_effect = [{ + 'pool_name': 'test-pool', + 'pg_num': 32, + 'pg_num_target': 32, + 'pg_placement_num': 32, + 'pg_placement_num_target': 32 + }] + pool = Pool() + pool._wait_for_pgs('test-pool') + self.assertEqual(_get.call_count, 1) + + @mock.patch('dashboard.controllers.pool.Pool._get') + @mock.patch('dashboard.tools.TaskManager.current_task') + def test_wait_for_pgs_with_waiting(self, taskMock, _get): + task = MockTask() + taskMock.return_value = task + _get.side_effect = [{ + 'pool_name': 'test-pool', + 'pg_num': 64, + 'pg_num_target': 32, + 'pg_placement_num': 64, + 'pg_placement_num_target': 64 + }, { + 'pool_name': 'test-pool', + 'pg_num': 63, + 'pg_num_target': 32, + 'pg_placement_num': 62, + 'pg_placement_num_target': 32 + }, { + 'pool_name': 'test-pool', + 'pg_num': 48, + 'pg_num_target': 32, + 'pg_placement_num': 48, + 'pg_placement_num_target': 32 + }, { + 'pool_name': 'test-pool', + 'pg_num': 48, + 'pg_num_target': 32, + 'pg_placement_num': 33, + 'pg_placement_num_target': 32 + }, { + 'pool_name': 'test-pool', + 'pg_num': 33, + 'pg_num_target': 32, + 'pg_placement_num': 32, + 'pg_placement_num_target': 32 + }, { + 'pool_name': 'test-pool', + 'pg_num': 32, + 'pg_num_target': 32, + 'pg_placement_num': 32, + 'pg_placement_num_target': 32 + }] + pool = Pool() + pool._wait_for_pgs('test-pool') + self.assertEqual(_get.call_count, 6) + self.assertEqual(task.percentages, [0, 5, 50, 73, 98]) -- 2.39.5