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

mgr/dashboard: list services and daemons #33531

Merged
merged 3 commits into from Mar 5, 2020
Merged

Conversation

bk201
Copy link
Contributor

@bk201 bk201 commented Feb 25, 2020

  • Display services and daemons in the cluster/services page.
  • Display daemons in the cluster/hosts/host-detail page (Daemons tab).

This PR also partially addresses https://tracker.ceph.com/issues/43165:
The endpoint /api/orchestrator/service is removed.

Create new endpoints:

  • /api/service: listing all services in the Ceph cluster.
  • /api/service/<service_name>/daemons: listing daemons for a
    service. e.g. daemons of OSD.
  • /api/host/<hostname>/daemons: listing daemons of a host.

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

Signed-off-by: Kiefer Chang kiefer.chang@suse.com

How to test

This PR needs to be tested in cephadm spawned cluster.

Screenshots

Cluster->Services
demo_2020-02-20-services
Cluster->Hosts (Daemons tab)
demo_2020-02-20-hosts

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

Copy link
Contributor

@sebastian-philipp sebastian-philipp left a comment

Choose a reason for hiding this comment

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

ack

@votdev
Copy link
Member

votdev commented Feb 25, 2020

Is it possible to add service information to dummy_data.json? This will make it easier to test the PR and such things in future.

@LenzGr LenzGr added this to the octopus milestone Feb 26, 2020
@bk201 bk201 requested a review from a team as a code owner February 27, 2020 07:54
@bk201
Copy link
Contributor Author

bk201 commented Feb 27, 2020

Is it possible to add service information to dummy_data.json? This will make it easier to test the PR and such things in future.

Added. I also refactor test_orchestrator a bit.

@@ -36,7 +36,7 @@ def get_device_osd_map():
}
:rtype: dict
"""
result = {}
result: dict = {}
Copy link
Member

Choose a reason for hiding this comment

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

This is PY3 type hinting syntax. Can we use that or should we use # type: dict instead? This may be a concern when backporting Octopus PRs to a PY2 based release.

@sebastian-philipp What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for bringing this up. Orchestrator integration will not be backported, but considering we are encouraged to add typing info in Dashboard (are we?), we should have a guideline to follow.

PY3 type hinting syntax will make backport a nightmare.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In today's standup we agree:

  • Use the PY3-only syntax if the codes are not going to be backported.
  • Stick to the comments way # type: xxxx if codes are going to be backported potentially.
  • If you don't know, stick to the comments way.

@bk201
Copy link
Contributor Author

bk201 commented Feb 27, 2020

jenkins test dashboard

1 similar comment
@callithea
Copy link
Member

jenkins test dashboard

- Display services and daemons in the cluster/services page.
- Display daemons in the cluster/hosts/host-detail page (Daemons tab).

This PR also partially addresses https://tracker.ceph.com/issues/43165:
The endpoint `/api/orchestrator/service` is removed.

Create new endpoints:
  - `/api/service`: listing all services in the Ceph cluster.
  - `/api/service/<service_name>/daemons`: listing daemons for a
    service. e.g. daemons of OSD.
  - `/api/host/<hostname>/daemons`: listing daemons of a host.

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

Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
- Refactor list_daemons: return ceph-xxx processes on host.
- Refactor describe_service: return services by grouping ceph-xxx
  processes on host.
- Add dummy data.

Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
@tspmelo
Copy link
Contributor

tspmelo commented Mar 2, 2020

jenkins test make check

@tspmelo
Copy link
Contributor

tspmelo commented Mar 2, 2020

jenkins test dashboard

1 similar comment
@bk201
Copy link
Contributor Author

bk201 commented Mar 3, 2020

jenkins test dashboard

@jschmid1
Copy link
Contributor

jschmid1 commented Mar 3, 2020

there was a column newly added with #33553 (SPEC) which you might include here?

@bk201
Copy link
Contributor Author

bk201 commented Mar 3, 2020

there was a column newly added with #33553 (SPEC) which you might include here?

Sure. But need this PR to display the value: #33685

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.

lgtm

@bk201
Copy link
Contributor Author

bk201 commented Mar 4, 2020

there was a column newly added with #33553 (SPEC) which you might include here?

Sure. But need this PR to display the value: #33685

Let's add the Placement column in another PR and get this merged first.
Created an issue to track: https://tracker.ceph.com/issues/44404

@liewegas
Copy link
Member

liewegas commented Mar 4, 2020

The spec_presence was changed to an actual spec in #33667, merged yesterday

@liewegas
Copy link
Member

liewegas commented Mar 5, 2020

these tests need to be fixed:

    def test_ps(self):
        ret = self._orch_cmd("ps")
        self.assertIn("ceph-mgr", ret)

    def test_ps_json(self):
        ret = self._orch_cmd("ps", "--format", "json")
        self.assertIsInstance(json.loads(ret), list)
        self.assertIn("ceph-mgr", ret)

now that the ps implementation is not bonkers it should check for mgr not ceph-mgr

I didn't realize test_orchestator was fabricating a result for ps aux... eek!

http://pulpito.ceph.com/sage-2020-03-05_03:05:51-rados-wip-sage2-testing-2020-03-04-1543-distro-basic-smithi/4826342

The ps output names daemons like 'type.foo', e.g., 'mgr.x'.  Now that
the test_orchestrator impl is less bonkers this needs to be adjusted to
match reality.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas liewegas merged commit 8a84dde into ceph:master Mar 5, 2020
@bk201 bk201 deleted the wip-44221 branch March 11, 2020 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants