Review `ceph-dashboard` charm for listing

charmhub requests review requests
1

Hi,

I’d like to propose our ceph-dashboard charm for listing. It’s been in use for a while as part of the OpenStack Charmers offering, and is being developed under the OpenStack umbrella.

I’ve filled out the form as per below, please let me know if there’s any questions.

Metadata links

  • project repository:

https://opendev.org/openstack/charm-ceph-dashboard Mirrored at https://github.com/openstack/charm-ceph-dashboard

CI Links

  • Code linting or-style checks:

We’re using flake8, tox.ini:

https://opendev.org/openstack/charm-ceph-dashboard/src/branch/master/tox.ini

  • Release automation:

Build automation info is kept in https://github.com/openstack-charmers/charmed-openstack-info

  • Unit test implementation:

https://opendev.org/openstack/charm-ceph-dashboard/src/branch/master/unit_tests

  • Unit test results:

https://zuul.opendev.org/t/openstack/buildset/d74010eb63ea49e98e19ef4fd81cd43f

  • Integration test implementation:

https://opendev.org/openstack/charm-ceph-dashboard/src/branch/master/tests https://github.com/openstack-charmers/zaza-openstack-tests

  • Integration test results:

Documentation Links

  • Usage:

Usage is documented here: https://ubuntu.com/ceph/docs/install-dashboard

  • Contribution:

Developer info is kept as part of the OpenStack Charm Guide: https://docs.openstack.org/charm-guide/latest/

  • Licensing:

https://opendev.org/openstack/charm-ceph-dashboard/src/branch/master/LICENSE

  • Publisher:

We can be reached at openstack-discuss@lists.openstack.org

2

Hi @chrome0 ,

Thank you for submitting a review request! @ca-scribner, can you please assist Peter with the charm review? Feel free to ping @review-coordinators if there are any questions.

Best, Simon

3

@0x12b I’m probably not the best to help atm. I’m out of office till late October. Is there someone else you have in mind?

4

Hah! Yeah, that makes no sense then. Thanks for letting me know! How about @medib instead?

5

There is an initial issue with the charms from the open stack team.

The publisher is not clear, as who is the group which provides the charm? It is unclear if it is from canonical or from the openinfra foundation.

@billy-olsen it might be a good opportunity to cover this question for this review.

6

and … please allow me to repeat my remark about the appearance on the detail page on charmhub ref to the two “lonely” lines:

Enable dashboard for Ceph
Enable the ceph dashboard on the ceph mon units

How about adding an information about getting started at least there?

7

Wrt to docs we’ve just merged the necessary metadata.yaml update, should be reflected in charmhub shortly

8

@medib Please proceed by ticking off the checklist from Reviewing charms in this topic. :slight_smile:

9

Hi @chrome0,

This review did not go through a PR, but here is a preliminary review:

  1. Intended functionality: TBC
    • I didn’t manage to deploy the bundle jammy-yoga.yaml to test it. Do you perhaps have a demo of it in action? or some script with all the pre-requisites prior to deploying the bundle?
  2. Charmhub.io charm detail page: OK
    • Could you add a link to the deployment guide of a “ceph cluster” (which is listed as a pre-requisite to using this charm)?
  3. Source repository: OK
  4. Coding conventions: OK
  5. Release automation implementation: TBC
    • Could you share the pipeline where this happens?
  6. Unit tests implementation: OK
  7. Unit tests results: OK
  8. Installation test implemented (could be part of the integration test): OK
  9. Installation test results: OK
  10. Integration tests implemented: OK
  11. Integration test results: OK
  12. Documentation for usage: OK
    • Could you add a link to the deployment guide of a “ceph cluster” (which is listed as a pre-requisite to using this charm)?
  13. Documentation for contributing: MISSING
  14. Licensing statement: MISSING in README

Could you:

  • make a PR?
  • take a look at: 1, 2, 5, 12, 13, 14?
  • share with me where are the CI definition files invoked from?
  • fix: [testenv:func-dev]

Thank you

5 Likes
10

Hi @medib,

make a PR?

The OpenStack dev model does not lend itself well to creating arbitrary branches. I’ve therefore created a branch from the Github mirror, hope this works too:

Intended functionality: TBC

I presume you were referring to tests/bundles/jammy-yoga.yaml? Typically these would be deployed in CI in our test environment. Typically Ceph will be deployed to a MAAS substrate, but an OpenStack undercloud would also work (this is what we’re using in testing), but e.g. LXD provider will not.

Deploying from a testing bundle should look something like this: https://asciinema.org/a/5I1lqFFpVzS2l0lVCtmgzV43e

But note that this bundle is e.g. not relating vault to ceph-dashboard (it’s explicitly done in a functest) so SSL certs are not set up.

Could you add a link to the deployment guide

Added one

Could you share the pipeline where this happens?

The charm is first uploaded via mirroring git repos into Launchpad and then built via LP recipes – here’s a list of the recipes that are in effect:

https://code.launchpad.net/~openstack-charmers/charm-ceph-dashboard/+git/charm-ceph-dashboard/+charm-recipes

Documentation for contributing

It’s linked from the charm-guide:

https://docs.openstack.org/charm-guide/latest/community/index.html

Licensing statement

Could you please elaborate the ask here? Similar to the other OpenStack projects we’re licensed under the Apache2.0 license and have a statement in the repo: https://opendev.org/openstack/charm-ceph-dashboard/src/branch/master/LICENSE

fix: [testenv:func-dev]

Done in https://review.opendev.org/c/openstack/charm-ceph-dashboard/+/896010

11

Ref the licensing statement, the ask is to have a statement in the README, such as in https://github.com/canonical/pgbouncer-k8s-operator#license I admit it is not critical, because the license file is present and millions of projects implement the same practise. If you find the time add it to the next commit on the README.md, it would make a sound impression though.

A propos readme.md. Had a quick look on the contribution guidelines - I need to admit that also for me it is not obvious how to find documentation about how to contribute to the project. I see that deployment, architecture, concepts are all relevant for contributing to the charm development. However, a contribution guideline in the sense of https://gist.github.com/briandk/3d2e8b3ec8daf5a27a62 I also did not find. If it exists, how about adding such link to the README - along with a licensing statement?

12

Hi,

I’ve added some details to the README: https://review.opendev.org/c/openstack/charm-ceph-dashboard/+/900612

Please let me know if this works for you.

Thanks, peter.

13

Hello, yes, ref. Contributing and License it looks perfectly fine, links work.

14

Hi @chrome0,

Sorry for the delay, I was on vacations :slight_smile:

Great work and thanks for addressing the feedback points! I also reviewed and approved your PR and left a few minor comments - nothing blocking.

Here is the review:

  1. Intended functionality: OK
  2. Charmhub.io charm detail page: OK
  3. Source repository: OK
  4. Coding conventions: OK
  5. Release automation implementation: OK
  6. Unit tests implementation: OK
  7. Unit tests results: OK
  8. Installation test implemented (could be part of the integration test): OK
  9. Installation test results: OK
  10. Integration tests implemented: OK
  11. Integration test results: OK
  12. Documentation for usage: OK
  13. Documentation for contributing: OK
  14. Licensing statement: OK

You should be good to go :slight_smile:

Thanks, Mehdi

15

Thanks @medib!

hello @odysseus-k could we ask you please to switch the charm to listed?

Thanks, Michael

16

Hi,

This is now done.

Thanks,

Odysseus

1 Like