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: Improve notifications for osd nearfull, full #44024

Merged
merged 1 commit into from Jan 19, 2022
Merged

mgr/dashboard: Improve notifications for osd nearfull, full #44024

merged 1 commit into from Jan 19, 2022

Conversation

aaSharma14
Copy link
Contributor

@aaSharma14 aaSharma14 commented Nov 19, 2021 •
edited

This PR adds some visual hints for osds that are near full or full

Fixes: https://tracker.ceph.com/issues/53334
Signed-off-by: Aashish Sharma aasharma@redhat.com

Landing Page > Capacity > Warning
Screenshot from 2021-11-19 13-29-26

Landing Page > Capacity > Danger
Screenshot from 2021-11-19 13-29-14

Landing Page > OSD card
Screenshot from 2022-01-10 15-18-30

OSD Page > usage bar > warning
Screenshot from 2021-11-19 13-10-10

OSD Page > usage bar > danger
Screenshot from 2021-11-19 13-10-28

@aaSharma14 aaSharma14 added this to In progress in Dashboard via automation Nov 19, 2021
@alfonsomthd
Copy link
Contributor

@aaSharma14 Provided tha there is 1 more line in "Landing Page > OSD card" and the Landing Page is responsive,
can you check that in different screen sizes (mobile, iPad, ...) the view is not broken?

epuertat
epuertat requested changes Nov 19, 2021
View reviewed changes
Copy link
Member

@epuertat epuertat left a comment

Choose a reason for hiding this comment

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

Looks really nice from a UI perspective @aaSharma14! Really happy to see this happening. You still need to refine how you get the data from to decide what color to paint everything. Let's discuss this next Monday.

src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard/health/health.component.ts Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard/osd-data.pipe.ts Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard/osd-data.pipe.ts Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/shared/components/usage-bar/usage-bar.component.html Outdated Show resolved Hide resolved
Dashboard automation moved this from In progress to Review in progress Nov 19, 2021
@aaSharma14
Copy link
Contributor Author

@aaSharma14 Provided tha there is 1 more line in "Landing Page > OSD card" and the Landing Page is responsive, can you check that in different screen sizes (mobile, iPad, ...) the view is not broken?

@alfonsomthd , I checked and the view is not broken. Thanks

@aaSharma14 aaSharma14 force-pushed the fix-53334-master branch 2 times, most recently from 3252d50 to 5e4d6c2 Compare November 22, 2021 11:46
epuertat
epuertat requested changes Nov 22, 2021
View reviewed changes
Copy link
Member

@epuertat epuertat left a comment

Choose a reason for hiding this comment

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

We are using different shades of red:
image

src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard/health/health.component.ts Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/shared/components/usage-bar/usage-bar.component.ts Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard/health/health.component.ts Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard/health/health.component.html Outdated Show resolved Hide resolved
@aaSharma14 aaSharma14 force-pushed the fix-53334-master branch 2 times, most recently from 4c8fddb to 492f182 Compare November 23, 2021 07:04
@avanthakkar
Copy link
Contributor

jenkins test dashboard

@avanthakkar
Copy link
Contributor

jenkins test dashboard cephadm

@aaSharma14 aaSharma14 requested review from a team and epuertat December 3, 2021 09:26
@github-actions github-actions bot added the tests label Dec 3, 2021
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard

@aaSharma14
Copy link
Contributor Author

jenkins test make check arm64

@aaSharma14
Copy link
Contributor Author

jenkins retest this please

pereman2
pereman2 approved these changes Jan 10, 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.

there are some minor tox issues, a part from that, LGETM

avanthakkar
avanthakkar approved these changes Jan 10, 2022
View reviewed changes
Copy link
Contributor

@avanthakkar avanthakkar left a comment

Choose a reason for hiding this comment

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

LGTM!

@aaSharma14
Copy link
Contributor Author

jenkins retest this please

@aaSharma14
Copy link
Contributor Author

jenkins test dashboard cephadm

@alfonsomthd alfonsomthd moved this from Review in progress to Ready-to-merge in Dashboard Jan 17, 2022
epuertat
epuertat approved these changes Jan 18, 2022
View reviewed changes
src/pybind/mgr/dashboard/controllers/osd.py Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard/health/health.component.scss Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/controllers/osd.py
@@ -18,6 +18,7 @@
from . import APIDoc, APIRouter, CreatePermission, DeletePermission, Endpoint, \
EndpointDoc, ReadPermission, RESTController, Task, UpdatePermission, \
allow_empty_body
from ._version import APIVersion
Copy link
Member

Choose a reason for hiding this comment

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

This should be imported in the line above. ._version is a module encapsulated in __init__.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think then we will have to change this in other 10-12 files as well to be consistent

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts Outdated Show resolved Hide resolved
qa/tasks/mgr/dashboard/test_health.py
@@ -112,6 +112,7 @@ def test_minimal_health(self):
JObj({
'in': int,
'up': int,
'state': JList(str)
Copy link
Member

Choose a reason for hiding this comment

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

This technically means we're altering the format of the /health endpoint, right? Since it's additive, perhaps we should increase only the minor version, but unfortunately we don't have yet the code in place for dealing with multiple minor versions... What do you think @aaSharma14 @alfonsomthd @avanthakkar @nizamial09 ?

Dashboard automation moved this from Ready-to-merge to Reviewer approved Jan 18, 2022
@aaSharma14
Copy link
Contributor Author

jenkins retest this please

@aaSharma14
Copy link
Contributor Author

jenkins test make check

mgr/dashboard: Improve notifications for osd nearfull, full
f771cd4 
This PR adds some visual hints for osds that are near full or full

Fixes: https://tracker.ceph.com/issues/53334
Signed-off-by: Aashish Sharma <aasharma@redhat.com>
epuertat
epuertat approved these changes Jan 19, 2022
View reviewed changes
Copy link
Member

@epuertat epuertat left a comment •
edited

Choose a reason for hiding this comment

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

LGTM! Thank you @aaSharma14 !

@epuertat epuertat moved this from Reviewer approved to Ready-to-merge in Dashboard Jan 19, 2022
@epuertat epuertat merged commit 2791a2b into ceph:master Jan 19, 2022
10 checks passed
Dashboard automation moved this from Ready-to-merge to Done Jan 19, 2022
@epuertat epuertat deleted the fix-53334-master branch January 19, 2022 13:03
@epuertat epuertat added the needs-quincy-backport backport required for quincy label Jan 19, 2022
@epuertat epuertat mentioned this pull request Jan 28, 2022
Merged
14 tasks
@epuertat epuertat removed the needs-quincy-backport backport required for quincy label Jan 31, 2022
@aaSharma14 aaSharma14 mentioned this pull request Feb 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nizamial09 nizamial09 nizamial09 approved these changes

@alfonsomthd alfonsomthd alfonsomthd approved these changes

@avanthakkar avanthakkar avanthakkar approved these changes

@pereman2 pereman2 pereman2 approved these changes

@epuertat epuertat epuertat approved these changes

@pcuzner pcuzner Awaiting requested review from pcuzner

Assignees

@sunilangadi2 sunilangadi2

Labels
api-change dashboard feature pybind tests
Projects
Ceph-Dashboard
Archived in project
Dashboard
  
Done
Milestone
No milestone
7 participants
@aaSharma14 @alfonsomthd @avanthakkar @pereman2 @epuertat @nizamial09 @sunilangadi2