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_v2: Initial submission of a web-based management UI (replacement for the existing dashboard) #20103
Conversation
why not merge those RESTful thing from i saw @liewegas @jcsp what's your opinion? thanks! [1] https://github.com/ceph/ceph/tree/master/src/pybind/mgr/dashboard |
@runsisi thank you for your feedback. As far as I understand it, the current dashboard does not use a REST API at all, the Python backend code performs the data collection and most of rendering here. The original In our case, the dashboard_v2 REST API is not intended to become an "official/stable" REST API endpoint - it's primary purpose is to serve the needs of the WebUI. Right now, we try to create a REST API that closely resembles the one used by openATTIC, in order to minimize the porting effort on our side. The Regarding Flask vs CherryPy vs Pecan: AFAIR, @jcsp already commented on the subject of using different Python modules on the ceph-devel mailing list. We chose CherryPy primarily because it is used by the existing dashboard and it fits our needs. However, we're still quite early in the process of the dashboard_v2 backend. If there are strong arguments for it, we'd be fine with switching to another framework at some point. For now and the sake of making progress, we'd rather stick to using CherryPy, though. |
c77ba1a
to
2ebb335
Compare
294e973
to
0d984a4
Compare
6ef48b7
to
da2caea
Compare
jenkins render docs |
Doc render available at http://docs.ceph.com/ceph-prs/20103/ |
cafd5bb
to
1efdf5c
Compare
OK, we've now concluded our first milestone (feature parity with the existing dashboard) and cleaned up the commit history by rebasing/squashing many commits. We'd be grateful for reviews/comments! |
dashboard_v2 teuthology suite results: |
@rdias While you're at it, can you schedule a /2000 rados run? |
I agree with @runsisi about the multiple different frameworks used, and would love to see reducing them to just 1. Most everything in the Ceph ecosystem (build services, ceph-installer, shaman, etc...) is Pecan based, which means there is a lot of know-how in our team, and I am strongly in favor of using it. Having browsed through the initial PR, it seems unreasonable to ask for that transition from the get-go. I would eventually like to have an open discussion to understand what the framework of choice should be moving forward, and what that means for other frameworks being used in the source tree. As an in-tree project which is choosing Angular, how would it look if another dashboard/app chooses React? There is value in uniformity of frameworks. Again, not meaning to derail the PR moving forward. Hope the framework discussion can be picked up later! |
Signed-off-by: Patrick Nawracay <pnawracay@suse.com>
Also: * Disabled API tests in tox. * No longer start a vstart.sh cluster in tox. Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
* Added new CMake flag `WITH_MGR_DASHBOARD_V2_FRONTEND`: Build the mgr/dashboard_v2 frontend using `npm install && npm run build` * Set this flag to `OFF` when building packages. * Removed creation of the frontend from `vstart.sh` Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
With this change, we avoid the disabling/enabling of the ceph-mgr module being tested for each test function declared in each test case. Now the ceph-mgr module being tested is disabled/enabled only once for each test case. Signed-off-by: Ricardo Dias <rdias@suse.com>
We moved the dashboard_v2 test cases that required a real ceph cluster to run into qa/tasks/mgr with the necessary adaptations. These tests can now be run in teuthology using the cephfs_test_runner task. Signed-off-by: Ricardo Dias <rdias@suse.com>
Signed-off-by: Ricardo Dias <rdias@suse.com>
Signed-off-by: Ricardo Dias <rdias@suse.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, this passed a rados suite and works as advertised.
@@ -888,6 +906,7 @@ cmake .. \ | |||
-DWITH_EMBEDDED=OFF \ | |||
-DWITH_MANPAGE=ON \ | |||
-DWITH_PYTHON3=ON \ | |||
-DWITH_MGR_DASHBOARD_V2_FRONTEND=OFF \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why OFF, though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The frontend build requires internet access which is not available in packaging build services, such as OBS. Because of this reason we build the frontend when generating the tarball and include the result files in the tarball.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rjfd yeah, i have the same question. since make-dist
is updated to include the generated files under src/pybind/mgr/dashboard_v2/frontend/dist
, we can safely enable this option in debian/rules
and ceph.spec.in
, i think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the WITH_MGR_DASHBOARD_V2_FRONTEND
enables/disables the frontend compilation, if we enable it during the packaging build (inside OBS, or other packaging service) it will require nodejs and npm as system dependencies, and will try to download all nodejs packages frontend dependencies when running npm install
(https://github.com/ceph/ceph/pull/20103/files#diff-bb5b00ad365f9278caa7bfa071fc7ce3R15). This will fail for two reasons:
a) usually the nodejs and npm system dependencies are too old, that's why we have to upgrade node and npm in install-deps.sh
b) packaging services don't allow internet access during build and npm install
will fail
This is the reason why we include the frontend compiled files in the tarball to avoid the above problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's counter-intuitive for the dashboard to be set to "OFF" in the packaging. There is an analogous situation with Boost, which is downloaded by make-dist. During the build cmake checks for the existence of a Boost-specific file, and only attempts the download if that file doesn't exist. See https://github.com/ceph/ceph/blob/master/cmake/modules/BuildBoost.cmake#L132-L159 for the implementation.
If doing that is too much trouble, could we at least rename the option to something more descriptive (and non-negative) like "-DWITH_MGR_DASHBOARD_V2_NODEJS_DEPS_IN_TARBALL=ON"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If doing that is too much trouble, could we at least rename the option to something more descriptive (and non-negative) like "-
DWITH_MGR_DASHBOARD_V2_NODEJS_DEPS_IN_TARBALL=ON"
From a POV of cmake, this flag disables or enables the build of the frontend. Changing this cmake flag has no effect on the tarball creation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smithfarm I agree with your concern, and I think we can even drop the cmake option, but that will require another round of build testing that takes some time. Can I open a PR after this one is merged dropping the cmake option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rjfd Yes, I was going to say that this concern of mine should not block merge of this PR.
@@ -5,7 +5,7 @@ export DESTDIR=$(CURDIR)/debian/tmp | |||
|
|||
export DEB_HOST_ARCH ?= $(shell dpkg-architecture -qDEB_HOST_ARCH) | |||
|
|||
extraopts += -DUSE_CRYPTOPP=OFF -DWITH_OCF=ON -DWITH_LTTNG=ON -DWITH_PYTHON3=ON -DWITH_EMBEDDED=OFF | |||
extraopts += -DUSE_CRYPTOPP=OFF -DWITH_OCF=ON -DWITH_LTTNG=ON -DWITH_PYTHON3=ON -DWITH_EMBEDDED=OFF -DWITH_MGR_DASHBOARD_V2_FRONTEND=OFF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why OFF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same reason as above
8eab5a2
to
5449857
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with the vstart+qa changes + the UI code itself has already been peer reviewed.
Installed latest centos RPMs in my test environment and it all looks good! 👍 👍 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just skimmed through the change, lgtm in general.
mgr/dashboard_v2: Initial submission of a web-based management UI (replacement for the existing dashboard) Reviewed-by: Nathan Cutler <ncutler@suse.com> Reviewed-by: John Spray <john.spray@redhat.com> Reviewed-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 23eb14d) Signed-off-by: Ernesto Puerta <epuertat@redhat.com> Conflicts: ceph.spec.in install-deps.sh qa/tasks/mgr/mgr_test_case.py qa/tasks/mgr/test_prometheus.py src/pybind/CMakeLists.txt src/vstart.sh Conflict-solving summary: ceph.spec.in: - Add make-check distro-conditional Python BuildRequires: CherryPy, tox, coverage and bcrypt. - Add ceph-mgr package Python Requires: bcrypt install-deps.sh - Add ensure_min_npm_version qa/tasks/mgr/mgr_test_case.py - Pick setUpClass method from remote qa/tasks/mgr/test_prometheus.py - Delete src/pybind/CMakeLists.txt - Add add_subdirectory(mgr) src/vstart.sh - Bring DASH_V2_URLS and dashboard_v2 config setting.
mgr/dashboard_v2: Initial submission of a web-based management UI (replacement for the existing dashboard) Reviewed-by: Nathan Cutler <ncutler@suse.com> Reviewed-by: John Spray <john.spray@redhat.com> Reviewed-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 23eb14d) Signed-off-by: Ernesto Puerta <epuertat@redhat.com> Conflicts: ceph.spec.in install-deps.sh qa/tasks/mgr/mgr_test_case.py qa/tasks/mgr/test_prometheus.py src/pybind/CMakeLists.txt src/vstart.sh Conflict-solving summary: ceph.spec.in: - Add make-check distro-conditional Python BuildRequires: CherryPy, tox, coverage and bcrypt. - Add ceph-mgr package Python Requires: bcrypt install-deps.sh - Add ensure_min_npm_version qa/tasks/mgr/mgr_test_case.py - Pick setUpClass method from remote qa/tasks/mgr/test_prometheus.py - Delete src/pybind/CMakeLists.txt - Add add_subdirectory(mgr) src/vstart.sh - Bring DASH_V2_URLS and dashboard_v2 config setting.
mgr/dashboard_v2: Initial submission of a web-based management UI (replacement for the existing dashboard) Reviewed-by: Nathan Cutler <ncutler@suse.com> Reviewed-by: John Spray <john.spray@redhat.com> Reviewed-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 23eb14d) Signed-off-by: Ernesto Puerta <epuertat@redhat.com> Conflicts: ceph.spec.in install-deps.sh qa/tasks/mgr/mgr_test_case.py qa/tasks/mgr/test_prometheus.py src/pybind/CMakeLists.txt src/vstart.sh Conflict-solving summary: ceph.spec.in: - Add make-check distro-conditional Python BuildRequires: CherryPy, tox, coverage and bcrypt. - Add ceph-mgr package Python Requires: bcrypt install-deps.sh - Add ensure_min_npm_version qa/tasks/mgr/mgr_test_case.py - Pick setUpClass method from remote qa/tasks/mgr/test_prometheus.py - Delete src/pybind/CMakeLists.txt - Add add_subdirectory(mgr) src/vstart.sh - Bring DASH_V2_URLS and dashboard_v2 config setting.
Merge pull request ceph#20103 from openattic/wip-mgr-dashboard_v2 Reviewed-by: Nathan Cutler <ncutler@suse.com> Reviewed-by: John Spray <john.spray@redhat.com> Reviewed-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 23eb14d) Signed-off-by: Ernesto Puerta <epuertat@redhat.com> Conflicts: ceph.spec.in install-deps.sh qa/tasks/mgr/mgr_test_case.py qa/tasks/mgr/test_prometheus.py src/pybind/CMakeLists.txt src/vstart.sh Conflict-solving summary: ceph.spec.in: - Add make-check distro-conditional Python BuildRequires: CherryPy, tox, coverage and bcrypt. - Add ceph-mgr package Python Requires: bcrypt install-deps.sh - Add ensure_min_npm_version qa/tasks/mgr/mgr_test_case.py - Pick setUpClass method from remote qa/tasks/mgr/test_prometheus.py - Delete src/pybind/CMakeLists.txt - Add add_subdirectory(mgr) src/vstart.sh - Bring DASH_V2_URLS and dashboard_v2 config setting.
Merge pull request ceph#20103 from openattic/wip-mgr-dashboard_v2 Reviewed-by: Nathan Cutler <ncutler@suse.com> Reviewed-by: John Spray <john.spray@redhat.com> Reviewed-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 23eb14d) Signed-off-by: Ernesto Puerta <epuertat@redhat.com> Conflicts: ceph.spec.in install-deps.sh qa/tasks/mgr/mgr_test_case.py qa/tasks/mgr/test_prometheus.py src/pybind/CMakeLists.txt src/vstart.sh Conflict-solving summary: ceph.spec.in: - Add make-check distro-conditional Python BuildRequires: CherryPy, tox, coverage and bcrypt. - Add ceph-mgr package Python Requires: bcrypt install-deps.sh - Add ensure_min_npm_version qa/tasks/mgr/mgr_test_case.py - Pick setUpClass method from remote qa/tasks/mgr/test_prometheus.py - Delete src/pybind/CMakeLists.txt - Add add_subdirectory(mgr) src/vstart.sh - Bring DASH_V2_URLS and dashboard_v2 config setting.
Merge pull request ceph#20103 from openattic/wip-mgr-dashboard_v2 Reviewed-by: Nathan Cutler <ncutler@suse.com> Reviewed-by: John Spray <john.spray@redhat.com> Reviewed-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 23eb14d) Signed-off-by: Ernesto Puerta <epuertat@redhat.com> Conflicts: ceph.spec.in install-deps.sh qa/tasks/mgr/mgr_test_case.py qa/tasks/mgr/test_prometheus.py src/pybind/CMakeLists.txt src/vstart.sh Conflict-solving summary: ceph.spec.in: - Add make-check distro-conditional Python BuildRequires: CherryPy, tox, coverage and bcrypt. - Add ceph-mgr package Python Requires: bcrypt install-deps.sh - Add ensure_min_npm_version qa/tasks/mgr/mgr_test_case.py - Pick setUpClass method from remote qa/tasks/mgr/test_prometheus.py - Delete src/pybind/CMakeLists.txt - Add add_subdirectory(mgr) src/vstart.sh - Bring DASH_V2_URLS and dashboard_v2 config setting.
Merge pull request ceph#20103 from openattic/wip-mgr-dashboard_v2 Reviewed-by: Nathan Cutler <ncutler@suse.com> Reviewed-by: John Spray <john.spray@redhat.com> Reviewed-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 23eb14d) Signed-off-by: Ernesto Puerta <epuertat@redhat.com> Conflicts: ceph.spec.in install-deps.sh qa/tasks/mgr/mgr_test_case.py qa/tasks/mgr/test_prometheus.py src/pybind/CMakeLists.txt src/vstart.sh Conflict-solving summary: ceph.spec.in: - Add make-check distro-conditional Python BuildRequires: CherryPy, tox, coverage and bcrypt. - Add ceph-mgr package Python Requires: bcrypt install-deps.sh - Add ensure_min_npm_version qa/tasks/mgr/mgr_test_case.py - Pick setUpClass method from remote qa/tasks/mgr/test_prometheus.py - Delete src/pybind/CMakeLists.txt - Add add_subdirectory(mgr) src/vstart.sh - Bring DASH_V2_URLS and dashboard_v2 config setting.
Merge pull request ceph#20103 from openattic/wip-mgr-dashboard_v2 Reviewed-by: Nathan Cutler <ncutler@suse.com> Reviewed-by: John Spray <john.spray@redhat.com> Reviewed-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 23eb14d) Signed-off-by: Ernesto Puerta <epuertat@redhat.com> Conflicts: ceph.spec.in install-deps.sh qa/tasks/mgr/mgr_test_case.py qa/tasks/mgr/test_prometheus.py src/pybind/CMakeLists.txt src/vstart.sh Conflict-solving summary: ceph.spec.in: - Add make-check distro-conditional Python BuildRequires: CherryPy, tox, coverage and bcrypt. - Add ceph-mgr package Python Requires: bcrypt install-deps.sh - Add ensure_min_npm_version qa/tasks/mgr/mgr_test_case.py - Pick setUpClass method from remote qa/tasks/mgr/test_prometheus.py - Delete src/pybind/CMakeLists.txt - Add add_subdirectory(mgr) src/vstart.sh - Bring DASH_V2_URLS and dashboard_v2 config setting.
The original Ceph Manager Dashboard as shipped with Ceph "Luminous" started out as a simple read-only view into various run-time information and performance data of a Ceph cluster, without authentication or any administrative functionality.
However, there is a growing demand for adding more web-based management capabilities, to make it easier for administrators that prefer a WebUI over the command line.
This module aims at becoming a "plug-in-replacement" for the existing dashboard, to provide the foundation for creating a native, built-in web based monitoring and administration application for Ceph.
The code and architecture of this module is derived from and inspired by the openATTIC Ceph management and monitoring tool.
The WebUI implementation is built using Angular and TypeScript. See the screen shot gallery for some impressions of the look and feel of the UI.
A lof of the backend code (based on CherryPy has been ported from the original dashboard, and will be extended in the future to support more than just read-only functionality. See the
HACKING.rst
file for the developer documentation.The development is actively driven by the team behind openATTIC at SUSE. The intention is to reuse existing openATTIC code where possible, while adapting it to the different environment.
This PR represents the first milestone (feature parity with the original dashboard, plus a simple authentication mechanism).
The porting and migration of the existing openATTIC and dashboard functionality will be done in future pull requests.