mgr/dashboard: perform daemon actions #44609

pereman2 · 2022-01-17T09:29:15Z

Ceph.2.mp4

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

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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

nizamial09

@pereman2: Didn't yet tested this manually but found some minor improvements in the UI section. Also I think its good to use pre-commit hook before commiting so that it could take care of most of the linting issues in code. You can find out how to setup pre-commit hook in ceph-dev..

...rontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.html

.../frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.ts

src/pybind/mgr/dashboard/frontend/src/app/shared/api/daemon.service.ts

nizamial09 · 2022-01-17T13:59:59Z

Forgot one thing: when the daemon is already in a running state and you select the daemon, instead of showing the Start Action, you can disable the Start Action and show the Stop Action first. That way you don't need to click the drop down action again. Similar to when you select a Pool and it shows Edit right away in the action instead of Create. @pereman2

pereman2 · 2022-01-17T14:28:43Z

@nizamial09 thanks! I should've opened this as draft since it was mostly to show alfonso the frontend test :p

...tend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.spec.ts

pereman2 · 2022-01-19T09:52:49Z

jenkins test make check

nizamial09 · 2022-01-20T08:56:34Z

@pereman2 I just saw we already have a tracker dedicated for this. Instead of using the new one you created, shall we use that? https://tracker.ceph.com/issues/50322

pereman2 · 2022-01-20T08:58:07Z

@pereman2 I just saw we already have a tracker dedicated for this. Instead of using the new one you created, shall we use that? https://tracker.ceph.com/issues/50322

Let's use that one, I'll close the current one.

pereman2 · 2022-01-21T08:09:02Z

jenkins test make check

src/pybind/mgr/dashboard/controllers/daemon.py

.../frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.ts

epuertat

LGTM! But I think we should start covering all day-2 ops with e2e, so here it comes the challenge:

Given that you already have experience with BDD/Behave/Cucumber
When you implement this new day-2 operation
Then it'd be awesome if a new BDD e2e test is added 😻

If you want to take the challenge, Cypress has a Cucumber-like library.

epuertat · 2022-01-21T16:59:09Z

.../frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.ts

+      {
+        permission: 'update',
+        icon: Icons.deploy,
+        click: () => this.daemonAction('redeploy'),


In the docs I see reconfig and/or redeploy?

Not sure if tableActions support tool-tips, but wouldn't be worthy to explain what redeploy and reconfig do (start and stop are self-explanatory though)?

I thought it was self-explanatory both restart and redeploy. Restart is stop and start I'd say, and redeploy recreates the systemd unit files, stops and restarts the service again. Reconfig I haven't checked since I thought it would increase the complexity of this pr rn

epuertat · 2022-01-21T18:20:53Z

src/pybind/mgr/dashboard/controllers/daemon.py

+class Daemon(RESTController):
+    @raise_if_no_orchestrator([OrchFeature.DAEMON_ACTION])
+    @handle_orchestrator_error('daemon')
+    @RESTController.MethodMap(version=APIVersion.EXPERIMENTAL)


Is experimental the new black? :P Now serious: I see the point of always starting with experimental version, but then we'll have to force ourselves to periodically review and freeze endpoints as stable.

nizamial09

Looks almost good to me apart from some minor hiccups.

I have an rgw service deployed in such a way that 2 daemons are in a same host and 1 in other. When I try to select a daemon in the first host, both of the daemons in that host gets selected.
https://user-images.githubusercontent.com/71764184/150755853-40673a7d-58dc-4111-8aa7-d1a64b8ceeae.mp4

Also, whenever I try to stop an rgw daemon it goes into an error state (same happening while using cephadm commands) so maybe we need to raise a bug or talk with cephadm folks...

.../frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.ts

nizamial09

LGTM! Awesome work @pereman2
One last request from me and I think that'll be it.

src/pybind/mgr/dashboard/frontend/cypress/integration/orchestrator/06-daemons.e2e-spec.ts

nizamial09 · 2022-01-27T10:14:11Z

jenkins test dashboard cephadm

nizamial09 · 2022-01-27T10:14:19Z

jenkins test dashboard

epuertat · 2022-01-27T15:18:05Z

Moved to RTB @pereman2 , but please squash commits (it'd facilitate backporting) and add the Fixes: to the commit message too.

epuertat

Looks mostly good, but some comments from my end:

When I stop a service it enters "error" state. Shouldn't it be "stopped" instead? "Error" is something unexpected, and no action that we allow users to do should directly result in an error state (IMHO we should prevent that). @adk3798 any clue on this?

I also see the mon/mgr actions are grayed out? Aren't they supported?
Another interesting behaviour I observed is that when performing multiple actions (start/stop/redeploy), no more than 6 events are displayed, but they're not the latest 6, but the first six (2 events from 1 hour ago are there, while I just did many restarts and they aren't displayed: the issue is probably in the back-end code). Additionally, I'd expect to see the events in reverse chronological order (newer at the top). Does that make sense?:

When I check the service events, I get:

Any clue about if that's expected?

Just a nit UI question: could we add some padding between icons and buttons (I know it's not specific of this PR, but let's incrementally improve the UI?)

This is a sample from the Patternfly website:

pereman2 · 2022-01-28T08:10:58Z

Looks mostly good, but some comments from my end:

When I stop a service it enters "error" state. Shouldn't it be "stopped" instead? "Error" is something unexpected, and no action that we allow users to do should directly result in an error state (IMHO we should prevent that). @adk3798 any clue on this?

2022-01-28 06:10:34,210 7fd195dfeb80 DEBUG /usr/bin/docker: Error: No such object: ceph-00000000-0000-0000-0000-0000deadbeef-crash-7b37b3131b1a this is what I see in cephadm.log

I also see the mon/mgr actions are grayed out? Aren't they supported?

Do we want to support this? If you kill a mon or mgr you risk making the dashboard unusable.

Another interesting behaviour I observed is that when performing multiple actions (start/stop/redeploy), no more than 6 events are displayed, but they're not the latest 6, but the first six (2 events from 1 hour ago are there, while I just did many restarts and they aren't displayed: the issue is probably in the back-end code). Additionally, I'd expect to see the events in reverse chronological order (newer at the top). Does that make sense?:

Events come from cephadm we don't modify them.
Fixed:

When I check the service events, I get:

Any clue about if that's expected?

Those are service events not daemon events which come from a different datasource.

Just a nit UI question: could we add some padding between icons and buttons (I know it's not specific of this PR, but let's incrementally improve the UI?)

This is a sample from the Patternfly website:

done too:

Signed-off-by: Pere Diaz Bou <pdiazbou@redhat.com> Fixes: https://tracker.ceph.com/issues/50322

epuertat · 2022-01-31T21:37:39Z

jenkins test api

epuertat · 2022-01-31T21:37:55Z

jenkins test make check

epuertat · 2022-01-31T21:38:13Z

jenkins test dashboard cephadm

pereman2 · 2022-02-01T09:34:52Z

jenkins test make check

pereman2 · 2022-02-01T14:00:19Z

jenkins test make check

pereman2 · 2022-02-01T14:01:32Z

jenkins test api

pereman2 requested a review from a team January 17, 2022 09:29

pereman2 requested a review from a team as a code owner January 17, 2022 09:29

pereman2 requested review from avanthakkar and callithea and removed request for a team January 17, 2022 09:29

github-actions bot added dashboard monitoring pybind labels Jan 17, 2022

pereman2 force-pushed the daemon_action branch from e2f8e87 to 2c1c3c3 Compare January 17, 2022 09:30

pereman2 requested review from alfonsomthd, epuertat and nizamial09 January 17, 2022 09:30

github-actions bot added this to In progress in Dashboard Jan 17, 2022

pereman2 requested review from Waadkh7 and aaSharma14 January 17, 2022 09:30

pereman2 force-pushed the daemon_action branch from 2c1c3c3 to c56d5e8 Compare January 17, 2022 09:42

nizamial09 requested changes Jan 17, 2022

View reviewed changes

Dashboard automation moved this from In progress to Review in progress Jan 17, 2022

alfonsomthd requested changes Jan 18, 2022

View reviewed changes

...tend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.spec.ts Outdated Show resolved Hide resolved

github-actions bot added the api-change label Jan 18, 2022

pereman2 force-pushed the daemon_action branch from a8b69bc to 748deae Compare January 21, 2022 07:24

alfonsomthd approved these changes Jan 21, 2022

View reviewed changes

src/pybind/mgr/dashboard/controllers/daemon.py Outdated Show resolved Hide resolved

.../frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.ts Show resolved Hide resolved

pereman2 force-pushed the daemon_action branch from 748deae to 987cd6d Compare January 21, 2022 14:24

epuertat approved these changes Jan 21, 2022

View reviewed changes

nizamial09 reviewed Jan 24, 2022

View reviewed changes

.../frontend/src/app/ceph/cluster/services/service-daemon-list/service-daemon-list.component.ts Outdated Show resolved Hide resolved

Dashboard automation moved this from Review in progress to Reviewer approved Jan 27, 2022

nizamial09 approved these changes Jan 27, 2022

View reviewed changes

src/pybind/mgr/dashboard/frontend/cypress/integration/orchestrator/06-daemons.e2e-spec.ts Outdated Show resolved Hide resolved

nizamial09 added the needs-quincy-backport backport required for quincy label Jan 27, 2022

epuertat moved this from Reviewer approved to Ready-to-merge in Dashboard Jan 27, 2022

pereman2 force-pushed the daemon_action branch from 83fd2e4 to 42f43ca Compare January 27, 2022 15:26

epuertat reviewed Jan 27, 2022

View reviewed changes

pereman2 force-pushed the daemon_action branch from 42f43ca to a302ac1 Compare January 28, 2022 08:10

mgr/dashboard: perform daemon actions on cluster->services

239f884

Signed-off-by: Pere Diaz Bou <pdiazbou@redhat.com> Fixes: https://tracker.ceph.com/issues/50322

pereman2 force-pushed the daemon_action branch from a302ac1 to 239f884 Compare January 28, 2022 10:44

pereman2 requested a review from epuertat January 28, 2022 10:53

epuertat merged commit 8505861 into ceph:master Feb 2, 2022
10 checks passed

Dashboard automation moved this from Ready-to-merge to Done Feb 2, 2022

epuertat deleted the daemon_action branch February 2, 2022 13:07

epuertat assigned sunilangadi2 Feb 3, 2022

epuertat mentioned this pull request Feb 4, 2022

quincy: dashboard: 2nd backport batch #44899

Merged

epuertat removed the needs-quincy-backport backport required for quincy label Feb 4, 2022

epuertat added the feature label Feb 23, 2022

nizamial09 mentioned this pull request Mar 1, 2022

pacific: mgr/dashboard: perform daemon actions #45203

Merged

mgr/dashboard: perform daemon actions #44609

mgr/dashboard: perform daemon actions #44609

pereman2 commented Jan 17, 2022 •

edited

nizamial09 left a comment •

edited

nizamial09 commented Jan 17, 2022 •

edited

pereman2 commented Jan 17, 2022

pereman2 commented Jan 19, 2022

nizamial09 commented Jan 20, 2022

pereman2 commented Jan 20, 2022

pereman2 commented Jan 21, 2022

epuertat left a comment

epuertat Jan 21, 2022

pereman2 Jan 27, 2022

epuertat Jan 21, 2022

nizamial09 left a comment •

edited

nizamial09 left a comment

nizamial09 commented Jan 27, 2022

nizamial09 commented Jan 27, 2022

epuertat commented Jan 27, 2022

epuertat left a comment

pereman2 commented Jan 28, 2022

epuertat commented Jan 31, 2022

epuertat commented Jan 31, 2022

epuertat commented Jan 31, 2022

pereman2 commented Feb 1, 2022

pereman2 commented Feb 1, 2022

pereman2 commented Feb 1, 2022

mgr/dashboard: perform daemon actions #44609

mgr/dashboard: perform daemon actions #44609

Conversation

pereman2 commented Jan 17, 2022 • edited

Checklist

nizamial09 left a comment • edited

Choose a reason for hiding this comment

nizamial09 commented Jan 17, 2022 • edited

pereman2 commented Jan 17, 2022

pereman2 commented Jan 19, 2022

nizamial09 commented Jan 20, 2022

pereman2 commented Jan 20, 2022

pereman2 commented Jan 21, 2022

epuertat left a comment

Choose a reason for hiding this comment

epuertat Jan 21, 2022

Choose a reason for hiding this comment

pereman2 Jan 27, 2022

Choose a reason for hiding this comment

epuertat Jan 21, 2022

Choose a reason for hiding this comment

nizamial09 left a comment • edited

Choose a reason for hiding this comment

nizamial09 left a comment

Choose a reason for hiding this comment

nizamial09 commented Jan 27, 2022

nizamial09 commented Jan 27, 2022

epuertat commented Jan 27, 2022

epuertat left a comment

Choose a reason for hiding this comment

pereman2 commented Jan 28, 2022

epuertat commented Jan 31, 2022

epuertat commented Jan 31, 2022

epuertat commented Jan 31, 2022

pereman2 commented Feb 1, 2022

pereman2 commented Feb 1, 2022

pereman2 commented Feb 1, 2022

pereman2 commented Jan 17, 2022 •

edited

nizamial09 left a comment •

edited

nizamial09 commented Jan 17, 2022 •

edited

nizamial09 left a comment •

edited