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: Add support for blinking enclosure LEDs #31851

Merged
merged 1 commit into from Dec 13, 2019
Merged

mgr/dashboard: Add support for blinking enclosure LEDs #31851

merged 1 commit into from Dec 13, 2019

Conversation

votdev
Copy link
Member

@votdev votdev commented Nov 25, 2019 •
edited

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

Peek 2019-11-25 14-30

Signed-off-by: Volker Theile vtheile@suse.com

Testing

You can easily test this PR by using the test_orchestrator. Check out #31757 how to use it.

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • 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 backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@capri1989
Copy link
Contributor

Thanks a lot and I really like it. Could we instead of showing a percentage in the task list have a clock counter that decreases down to zero? Once you've triggered the command, the percentage is kinda relative

@votdev
Copy link
Member Author

votdev commented Nov 25, 2019

Thanks a lot and I really like it. Could we instead of showing a percentage in the task list have a clock counter that decreases down to zero? Once you've triggered the command, the percentage is kinda relative

The current task implementation (backend and frontend) does not support that. I suggest to open a tracker issue to do this in a follow up action. I think @Devp00l can say more about this as he's the developer who has implemented it.

@capri1989
Copy link
Contributor

Thanks a lot and I really like it. Could we instead of showing a percentage in the task list have a clock counter that decreases down to zero? Once you've triggered the command, the percentage is kinda relative

The current task implementation (backend and frontend) does not support that. I suggest to open a tracker issue to do this in a follow up action. I think @Devp00l can say more about this as he's the developer who has implemented it.

Ok, sounds good to me. I'll open a new tracker issue for that

@bk201 bk201 mentioned this pull request Nov 27, 2019
Merged
5 tasks
@votdev votdev force-pushed the issue_42609_device_lights branch 2 times, most recently from e276273 to 9623a94 Compare November 28, 2019 16:12
@votdev votdev changed the title [After #30921] mgr/dashboard: Add support for blinking enclosure LEDs Add support for blinking enclosure LEDs Nov 29, 2019
@votdev votdev force-pushed the issue_42609_device_lights branch 3 times, most recently from ac2a1bb to 2faa53d Compare November 29, 2019 15:57
@sebastian-philipp
Copy link
Contributor

cc @pcuzner

@votdev votdev force-pushed the issue_42609_device_lights branch 5 times, most recently from 68b3c08 to 9b035f3 Compare December 3, 2019 09:55
@votdev
Copy link
Member Author

votdev commented Dec 3, 2019

jenkins test make check

1 similar comment
@LenzGr
Copy link
Contributor

LenzGr commented Dec 3, 2019

jenkins test make check

@votdev
Copy link
Member Author

votdev commented Dec 4, 2019

jenkins test make check arm64

@votdev votdev marked this pull request as ready for review December 4, 2019 07:13
@votdev votdev requested a review from a team as a code owner December 4, 2019 07:13
@bk201
Copy link
Contributor

bk201 commented Dec 10, 2019

I understand it's not possible to turn on any light on a virtual disk, but the device path seems to have a redundant /dev/ prefix. Maybe it should be fixed in ssh-orchestrator:

@bk201 - can you please submit an issue about this on the tracker?

Create the issue: https://tracker.ceph.com/issues/43223.

@callithea
Copy link
Member

jenkins test dashboard backend

@votdev votdev force-pushed the issue_42609_device_lights branch 3 times, most recently from ba0f121 to fb20d0f Compare December 10, 2019 12:26
@s0nea
Copy link
Member

s0nea commented Dec 10, 2019 •
edited by callithea

ceph dashboard backend tests failure -> https://tracker.ceph.com/issues/43227

@callithea
Copy link
Member

jenkins test dashboard backend

callithea
callithea reviewed Dec 10, 2019
View reviewed changes
Copy link
Member

@callithea callithea left a comment

Choose a reason for hiding this comment

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

make check failed due to mgr-dashboard-frontend-unittests errors

src/app/ceph/cluster/inventory/inventory-devices/inventory-devices.component.ts(12,1): error TS6133: 'NotificationType' is declared but its value is never read.
src/app/ceph/cluster/inventory/inventory-devices/inventory-devices.component.ts(66,13): error TS6138: Property 'notificationService' is declared but its value is never read.```

@votdev
Copy link
Member Author

votdev commented Dec 11, 2019

jenkins test dashboard

@votdev
Copy link
Member Author

votdev commented Dec 11, 2019

jenkins test make check arm64

LenzGr
LenzGr approved these changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@LenzGr LenzGr left a comment

Choose a reason for hiding this comment

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

Tested this locally using the test orchestrator together with the dummy data provided by #32182 - LGTM!

bk201
bk201 approved these changes Dec 12, 2019
View reviewed changes
Copy link
Contributor

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

Because of the on-wait-off logic, a connection to backend will be held until the LED is off.
Maybe we can move the logic into orchestrator or use task manager (which causes redundant tasks displayed) for a improvement in the future?

@votdev
Copy link
Member Author

votdev commented Dec 12, 2019 •
edited

Because of the on-wait-off logic, a connection to backend will be held until the LED is off.
Maybe we can move the logic into orchestrator or use task manager (which causes redundant tasks displayed) for a improvement in the future?

This is caused by using the progress module. My previous implementation has used the Dashboard task feature which will return immediately. Seems we can not have both.

@votdev
Copy link
Member Author

votdev commented Dec 12, 2019

Will revert back to using Dashboard Task implementation instead of using progress module because using the latter one will not fork the request and return immediatelly; thus we will run into a timeout when the delay time is greater than the request timeout.

tspmelo
tspmelo approved these changes Dec 12, 2019
View reviewed changes
Copy link
Contributor

@tspmelo tspmelo left a comment

Choose a reason for hiding this comment

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

was not able to test it, but code lgtm.

@votdev
Copy link
Member Author

votdev commented Dec 12, 2019

jenkins test dashboard

1 similar comment
@callithea
Copy link
Member

jenkins test dashboard

@callithea
Copy link
Member

jenkins test dashboard backend

@bk201 bk201 mentioned this pull request Dec 13, 2019
Merged
3 tasks
@callithea
Copy link
Member

callithea commented Dec 13, 2019 •
edited

@callithea callithea merged commit 481e320 into ceph:master Dec 13, 2019
@votdev votdev deleted the issue_42609_device_lights branch December 13, 2019 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebastian-philipp sebastian-philipp sebastian-philipp left review comments

@epuertat epuertat epuertat left review comments

@LenzGr LenzGr LenzGr approved these changes

@tspmelo tspmelo tspmelo approved these changes

@bk201 bk201 bk201 approved these changes

@callithea callithea callithea approved these changes

Assignees
No one assigned
Labels
dashboard feature needs-review
Projects
None yet
Milestone
No milestone
9 participants
@votdev @capri1989 @sebastian-philipp @LenzGr @callithea @bk201 @s0nea @tspmelo @epuertat