Skip to content
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

Jump to bottom

mgr/dashboard: rbd-mirror improvements #49292

Merged
merged 1 commit into from Dec 19, 2022
Merged

mgr/dashboard: rbd-mirror improvements #49292

merged 1 commit into from Dec 19, 2022

Conversation

aaSharma14
Copy link
Contributor

@aaSharma14 aaSharma14 commented Dec 7, 2022 •
edited

Fixes: https://tracker.ceph.com/issues/58297
This PR intends to solve the below mentioned workflow issues-

  1. RBD images in dashboard shows default mirroring as journal which should not be the case
  2. snapshot based mirroring schedule Interval got disabled to edit which should not be the case
  3. unable to create snapshot of an image using dashboard which should not be the case
  4. provide snapshot schedule info in a new column
  5. dashboard doesn't allow importing peer bootstrap key to be imported for subsequent pools which should not be the case

Screenshot from 2022-12-07 13-28-58

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@aaSharma14 aaSharma14 requested a review from a team as a code owner December 7, 2022 05:31
@aaSharma14 aaSharma14 added this to In progress in Dashboard via automation Dec 7, 2022
@sunilangadi2 sunilangadi2 self-requested a review December 7, 2022 05:49
sunilangadi2
sunilangadi2 reviewed Dec 7, 2022
View reviewed changes
Copy link

@sunilangadi2 sunilangadi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @aaSharma14
Can you please add some screenshots and test result of those scenarios in this PR?

@aaSharma14
Copy link
Contributor Author

Hi @aaSharma14 Can you please add some screenshots and test result of those scenarios in this PR?

Hi @sunilangadi2 , I think screenshots will be of no use here as these are workflow related improvements. I guess testing the workflow on a running cluster should be better.

@aaSharma14 aaSharma14 force-pushed the rbd-mirror-improvements branch 3 times, most recently from 33d5f36 to 6307e21 Compare December 8, 2022 05:45
@aaSharma14
Copy link
Contributor Author

jenkins test make check

pereman2
pereman2 approved these changes Dec 12, 2022
View reviewed changes
Copy link
Contributor

@pereman2 pereman2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that having the label set to unknown when mirroring mode is disabled on the image is misleading. You might think it is enabled but something happened. What if we set it to disabled or just blank?

image

I just noticed whenever you click on "Configure RBD Mirroring" it will fire a notification with a message of "[object Object]"
image

Dashboard automation moved this from In progress to Reviewer approved Dec 12, 2022
@avanthakkar avanthakkar changed the title Rbd mirror improvements mgr/dashboard: rbd-mirror improvements Dec 13, 2022
@avanthakkar
Copy link
Contributor

jenkins retest this please

Pegonzal
Pegonzal reviewed Dec 14, 2022
View reviewed changes
Copy link
Contributor

@Pegonzal Pegonzal left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding make check:

[lint:prettier] src/app/ceph/block/rbd-form/rbd-form.component.ts
[lint:prettier] src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts
[lint:eslint ] Lint errors found in the listed files.
[lint:eslint ]
[lint:eslint ]
[lint:eslint ] Linting "ceph-dashboard"...
[lint:eslint ]
[lint:eslint ] /home/pegonzal/ceph/ceph/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts
[lint:eslint ] 647:1 error Trailing spaces not allowed no-trailing-spaces
[lint:eslint ]
[lint:eslint ] ✖ 1 problem (1 error, 0 warnings)
[lint:eslint ] 1 error and 0 warnings potentially fixable with the --fix option.
[lint:eslint ]
[lint:tsc ] - ng-block-ui/http [es2015/esm2015] (git+https://github.com/kuuurt13/ng-block-ui.git)
[lint:tsc ] - ng-block-ui/router [es2015/esm2015] (git+https://github.com/kuuurt13/ng-block-ui.git)
[lint:tsc ] src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts(100,19): error TS2554: Expected 12 arguments, but got 11.

@aaSharma14 aaSharma14 force-pushed the rbd-mirror-improvements branch 2 times, most recently from ea0ecb2 to b328dda Compare December 16, 2022 07:52
@aaSharma14
Copy link
Contributor Author

I think that having the label set to unknown when mirroring mode is disabled on the image is misleading. You might think it is enabled but something happened. What if we set it to disabled or just blank?
Makes Sense, Changed it to Disabled

image

I just noticed whenever you click on "Configure RBD Mirroring" it will fire a notification with a message of "[object Object]" image

This hapens just in ceph-dev env as we donot have orchestrator configured there, In a Cephadm cluster we don't get this error

@aaSharma14
mgr/dashboard: Rbd Mirroring improvements
a85ed94 
1.RBD images in dashboard shows default mirroring as journal
2.snapshot based mirroring schedule Interval got disabled to edit
3.unable to create snapshot of an image using dashboard
4.provide snapshot schedule info in a new column
5.dashboard doesn't allow importing peer bootstrap key to be imported for subsequent pools

Fixes: https://tracker.ceph.com/issues/58297
Signed-off-by: Aashish Sharma <aasharma@redhat.com>
(cherry picked from commit 1a37c5e001e0bbc2720e6dfabf6ba04442acabb7)
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard cephadm

@aaSharma14
Copy link
Contributor Author

jenkins test windows

@aaSharma14
Copy link
Contributor Author

jenkins test make check arm64

@aaSharma14 aaSharma14 merged commit 1edef55 into ceph:main Dec 19, 2022
10 of 13 checks passed
Dashboard automation moved this from Reviewer approved to Done Dec 19, 2022
@aaSharma14 aaSharma14 deleted the rbd-mirror-improvements branch December 19, 2022 06:49
@aaSharma14 aaSharma14 mentioned this pull request Dec 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sunilangadi2 sunilangadi2 sunilangadi2 left review comments

@Pegonzal Pegonzal Pegonzal left review comments

@pereman2 pereman2 pereman2 approved these changes

@nizamial09 nizamial09 nizamial09 approved these changes

@avanthakkar avanthakkar Awaiting requested review from avanthakkar

Assignees
No one assigned
Labels
dashboard pybind
Projects
Ceph-Dashboard
Archived in project
Dashboard
  
Done
Milestone
No milestone
6 participants
@aaSharma14 @avanthakkar @sunilangadi2 @pereman2 @nizamial09 @Pegonzal