New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dashboard: support configuring block mirroring pools and peers #25210

Merged
merged 9 commits into from Jan 4, 2019

Conversation

Projects
None yet
6 participants
@dillaman
Copy link
Contributor

dillaman commented Nov 21, 2018

Fixes: https://tracker.ceph.com/issues/24275

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

@dillaman dillaman force-pushed the dillaman:wip-dashboard-rbd-mirroring branch from 10e09be to 9006b34 Nov 22, 2018

@votdev

This comment has been minimized.

Copy link
Contributor

votdev commented Nov 22, 2018

Using config file /ceph/src/pybind/mgr/dashboard/.pylintrc
************* Module dashboard.controllers.rbd_mirroring
C:363, 0: Exactly one space required after comma
                mode_enum = dict([[x[1],x[0]] for x in
                                       ^ (bad-whitespace)
E: 23,15: handle_rados_error('rbd-mirroring') is not callable (not-callable)
E: 24,15: handle_rbd_error() is not callable (not-callable)
E:412,18: Undefined variable 'cherrypy' (undefined-variable)

Please fix lint errors.

@votdev

This comment has been minimized.

Copy link
Contributor

votdev commented Nov 22, 2018

Please fix JS lint errors

ERROR: /ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/mirroring/overview/overview.component.ts[27, 73]: Property 'i18n' is declared but its value is never read.
ERROR: /ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/mirroring/pool-edit-mode-modal/pool-edit-mode-modal.component.ts[36, 13]: Property 'rbdService' is declared but its value is never read.
ERROR: /ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/mirroring/pool-edit-mode-modal/pool-edit-mode-modal.component.ts[38, 13]: Property 'fb' is declared but its value is never read.
ERROR: /ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/mirroring/pool-edit-mode-modal/pool-edit-mode-modal.component.ts[39, 13]: Property 'taskWrapper' is declared but its value is never read.
ERROR: /ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/mirroring/pool-edit-peer-modal/pool-edit-peer-modal.component.ts[29, 13]: Property 'rbdService' is declared but its value is never read.
ERROR: /ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/mirroring/pool-edit-peer-modal/pool-edit-peer-modal.component.ts[31, 13]: Property 'fb' is declared but its value is never read.
ERROR: /ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/mirroring/pool-edit-peer-modal/pool-edit-peer-modal.component.ts[32, 13]: Property 'taskWrapper' is declared but its value is never read.
ERROR: /ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/mirroring/pool-list/pool-list.component.ts[62, 71]: == should be ===

Some of the JS unit tests also fail.

@dillaman dillaman force-pushed the dillaman:wip-dashboard-rbd-mirroring branch 5 times, most recently from 06fd503 to 377cca5 Dec 5, 2018

undefined,
() => this.editModeForm.setErrors({ cdSubmitButton: true }),
() => {
this.rbdMirroringService.refresh();

This comment has been minimized.

@dillaman

dillaman Dec 6, 2018

Author Contributor

Is there a proper way to invoke a callback once the API call has completed successfully? I want to ensure that the UI gets properly updated w/o the need to click the refresh buttons. Right now this seems racy in that sometimes it works and sometimes it doesn't.

This comment has been minimized.

@tspmelo

tspmelo Dec 6, 2018

Contributor

refresh() needs to return an Observable.

Changing refresh to the following should work:

refresh() {
  this.http
    .get('api/block/mirroring/summary')
    .pipe(tap((data) => this.summaryDataSource.next(data)));
}

then you need to subscribe after each call:

Suggested change Beta
this.rbdMirroringService.refresh();
this.rbdMirroringService.refresh().subscribe((data) => {
//It's finished
});

This comment has been minimized.

@dillaman

dillaman Dec 6, 2018

Author Contributor

... it turns out that the controller endpoint for "/api/block/mirroring/summary" will cache the view for up to 1 second, which is why I am sometimes getting stale results. I guess that cache needs to be removed or I need a way to force-refresh.

@dillaman dillaman force-pushed the dillaman:wip-dashboard-rbd-mirroring branch from 377cca5 to 479ea62 Dec 6, 2018

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 6, 2018

screenshot from 2018-12-06 14-43-31
screenshot from 2018-12-06 14-43-58
screenshot from 2018-12-06 14-44-26
screenshot from 2018-12-06 14-44-38
screenshot from 2018-12-06 14-45-04
screenshot from 2018-12-06 14-45-36
screenshot from 2018-12-06 14-45-58

@dillaman dillaman force-pushed the dillaman:wip-dashboard-rbd-mirroring branch from 479ea62 to 022e23e Dec 6, 2018

@dillaman dillaman changed the title [WIP] dashboard: support configuring block mirroring pools and peers dashboard: support configuring block mirroring pools and peers Dec 6, 2018

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 6, 2018

Ready for another round of review. It still requires a change to the controller to invalidate the @ViewCache when data is updated.

@callithea callithea added needs-review and removed needs-rebase labels Dec 7, 2018

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 7, 2018

... fixed the issue w/ the stale view cache within the RBD mirroring controller.

@dillaman dillaman removed the DNM label Dec 7, 2018

dillaman added some commits Nov 13, 2018

mgr/dashboard: move RBD mirroring controller endpoint
The current summary page has been moved to 'api/block/mirroring/summary'.
Eventually multiple RBD mirroring-related task endpoints will be
added.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
mgr/dashboard: show all pools on block mirroring page
Pools that do not have mirroring enabled are now shown with a
"disabled" status.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
mgr/dashboard: backend REST APIs for block mirroring management
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
mgr/dashboard: expose mirror peer uuids in summary response
Currently only a single peer is supported, so this data will be used to
enable/disable the peer action drop-downs.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
pybind/rbd: fixed empty result handling in mirror_peer_get_attributes
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
mgr/dashboard: reset the block mirroring cached views upon update
When manipulating the mirroring state, ensure that the cached
views are reset to avoid returning stale data to the UI.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
}

validateMonAddr(control: AbstractControl) {
if (!control.value.match(/^[,; ]*([\w\-_\[\]]+(:[\d]+)?[,; ]*)*$/)) {

This comment has been minimized.

@ricardoasmarques

ricardoasmarques Dec 14, 2018

Member

Shouldn't we also accept IP addresses?

screenshot from 2018-12-14 11-17-48

This comment has been minimized.

@dillaman

dillaman Dec 14, 2018

Author Contributor

Thx -- fixed.

@dillaman dillaman force-pushed the dillaman:wip-dashboard-rbd-mirroring branch from 83dfe00 to d52e727 Dec 14, 2018

[ngClass]="{'has-error': editPeerForm.showError('clusterName', formDir)}">
<label class="control-label"
for="clusterName">
<span i18n>Cluster Name</span>

This comment has been minimized.

@ricardoasmarques

ricardoasmarques Dec 16, 2018

Member

nit: Required fields must have a * next to the label (use <span class="required"></span>):

screenshot from 2018-12-16 22-41-19

[ngClass]="{'has-error': editPeerForm.showError('monAddr', formDir)}">
<label class="control-label"
for="monAddr">
<span i18n>Monitor Address</span>

This comment has been minimized.

@ricardoasmarques

ricardoasmarques Dec 16, 2018

Member

nit: we accept multiple addresses so s/Monitor Address/Monitor Addresses/

@ricardoasmarques

This comment has been minimized.

Copy link
Member

ricardoasmarques commented Dec 16, 2018

Just for my learning, can we set the Mode on erasure Pools?

If no, I wonder if we should exclude these pools from the list, or at least show an error before-hand.

screenshot from 2018-12-16 23-06-27

@ricardoasmarques

This comment has been minimized.

Copy link
Member

ricardoasmarques commented Dec 16, 2018

... the rest lgtm

@dillaman dillaman force-pushed the dillaman:wip-dashboard-rbd-mirroring branch from d52e727 to eb65a25 Dec 17, 2018

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 17, 2018

@ricardoasmarques
Copy link
Member

ricardoasmarques left a comment

lgtm

dillaman added some commits Nov 16, 2018

mgr/dashboard: support editing block mirror pools
Fixes: https://tracker.ceph.com/issues/24275
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
mgr/dashboard: filter EC pools from mirroring pool list
RBD mirroring is configured on the metadata pool which is required
to be a replicated pool.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

@dillaman dillaman force-pushed the dillaman:wip-dashboard-rbd-mirroring branch from eb65a25 to fa9cf2d Dec 17, 2018

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 17, 2018

i18n strings updated

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 17, 2018

jenkins test docs

1 similar comment
@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 17, 2018

jenkins test docs

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 18, 2018

jenkins test dashboard

1 similar comment
@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 19, 2018

jenkins test dashboard

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Dec 19, 2018

@tspmelo Any other changes? the jenkins "dashboard tests" builders are finally working again.

@ricardoasmarques ricardoasmarques merged commit 6959767 into ceph:master Jan 4, 2019

6 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
ceph dashboard tests ceph dashboard tests succeeded
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@dillaman dillaman deleted the dillaman:wip-dashboard-rbd-mirroring branch Jan 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment