Review `prometheus-k8s` for listing

The Prometheus K8s charm deploys Prometheus, the de facto standard for single-node metrics ingestion, storage and visualization.

Review PR

Metadata Links

CI Links

Documentation Links

Publisher: Simon Aronsson simon.aronsson@canonical.com, Canonical Observability Team


Review comments are to be directed to the observability core team, meaning @sed-i, @crucible, @lucabello, @jose, or sina.

Hi @0x12b ! Apologies for the delay here - caught up with some other things this week, but I’ll make sure this is assigned by Monday at the latest and we get the review completed next week.

(More differentiating text in this paragraph to be ignored).

Listing requirements

  • The charm does what it is meant to do, per the demo or tutorial. *Note: I skipped doing this specifically for the review, but this charm is part of the Ops K8s Tutorial, so I and many others in Charm Tech have validated it regularly).
  • The charm’s page on Charmhub provides a quality impression. The overall appearance looks good and the documentation looks reasonable.
  • Automated releasing to unstable channels exists
  • Integration tests exist, are run on every change to the default branch, and are passing. At minimum, the tests verify that the charm can be deployed and ends up in a success state, and that the charm can be integrated with at least one example for each ‘provides’ and ‘requires’ specified (including optional, excluding tracing) ending up in a success state.

If the charm provides an interface library, the library’s module docstring must contain the following information:

  • the interface(s) this library is for, and if it takes care of one or both of the providing/requiring sides
  • guidance on how to start when using the library to implement their end of the interface

If the charm provides a general library, the library’s module docstring must contain the following information:

  • the purpose of the library
  • the intended audience for the library: is this library intended for use only by the charm or the charming team, or is it a public library intended for anyone to use in their charm?
  • guidance on how to start using the library

Best practices

The following best practices are recommended for all charms, and are also required for listing.

  • Use absolute paths in subprocesses to prevent security issues.
  • Execute processes directly rather than via the shell.
  • Name the repository using the pattern <charm name>-operator for a single charm, or <base charm name>-operators when the repository will hold multiple related charms. For the charm name, see {external+charmcraft:ref}Charmcraft | Specify a name <specify-a-name>.
  • Set the requires-python version in your pyproject.toml so that tooling will detect any use of Python features not available in the versions you support.
  • When using the charm plugin with charmcraft, ensure that you set strict dependencies to true. For example:
  • Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies.
  • Ensure that the pyproject.toml and the lock file are committed to version control, so that exact versions of charms can be reproduced.
  • Capture output to stdout and stderr in your charm and use the logging and warning functionality to send messages to the charm user, rather than rely on Juju capturing output. In particular, you should avoid print() calls, and ensure that any subprocess calls capture output.
  • Do not build log strings yourself: allow the logger to do this for you as required. That is:
  • Ensure that log messages are clear, meaningful, and provide enough information for the user to troubleshoot any issues. Avoid spurious logging. For instance, try not to log when event handlers are called, as the Juju controller does this automatically.
  • Never log credentials or other sensitive information.
  • All charms should provide the commands configured by the Charmcraft profile, to allow easy testing across the charm ecosystem. It’s fine to tweak the configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides.
  • Write your charm to be stateless, where possible.
  • Libraries should never mutate the status of a unit or application. Instead, use return values, or raise exceptions and let them bubble back up to the charm for the charm author to handle as they see fit.
  • If you’re setting up a git repository: name it using the pattern <charm name>-operator. For the charm name, see :ref:specify-a-name.
  • For resources that are binary files, provide binaries for all the CPU architectures you intend to support.
  • Prefer lowercase alphanumeric names, and use hyphens (-) to separate words. For charms that have already standardised on underscores, it is not necessary to change them, and it is better to be consistent within a charm then to have some action names be dashed and some be underscored.
  • Just like Juju, a charm is an opinionated tool. Configure the application with the best defaults (ideally the application is deployable without providing any configuration at deploy time), and only expose application configuration options when necessary.
  • Prefer lowercase alphanumeric names, separated with dashes if required. For charms that have already standardised on underscores, it is not necessary to change them, and it is better to be consistent within a charm then to have some config names be dashed and some be underscored.
  • Documentation links should apply to the charm, and not to the application that is being charmed. Assume that the user already has basic competency in the use of the application.
  • The name should be slug-oriented (ASCII lowercase letters, numbers, and hyphens) and follow the pattern <workload name in full>[<function>][-k8s]. For example, argo-server-k8s.
  • Always include the optional key, rather than relying on the default value to indicate that the relation is required. Although this field is not enforced by Juju, including it makes it clear to users (and other tools) whether the relation is required.

Non-blocking notes:

  • update-ca-certificates is run without an absolute path - we’ve made exceptions for this already with other charms.
  • Is something ensuring that you’re alerted when a dependency has a security update? I couldn’t see an explicit Dependabot config, or something like Renovate.
  • There’s one case of building log lines with format(), which shouldn’t be done.
  • We’re trying to standardise the tox command names, and part of that is “format” rather than “fmt”. This is the one least likely to be used in bulk, so not a big deal, but it would be nice to change it.
  • We prefer dashed config names rather than underscored ones, but I assume the charm has been around too long to change that now (particularly since there’s already a deprecated config option).

Thanks @0x12b !

@adityagoel28 this one is good for listing, thanks!

1 Like

Hi @tony-meyer,

I have listed the charm, but charms will not show up unless they are defaulted to a channel that has a stable release.

I did some investigation and your two options would be to change the default track to 1 (which I can do). Option two would be to release a revision to latest/stable.

Thanks,
Aditya

@sed-i, @crucible, @lucabello, @jose (and maybe @0x12b) I assume this is related to dropping ‘latest’. I imagine you want 1 to be the default track, but please organise whatever adjustments you prefer.

Track 1 was already supposed to be our default, not sure how it slipped through the cracks! I just updated it to be the default track though :slight_smile:

1 Like

The charm should be visible after publishing a new revision to the default track. Do let me know in case of any issues.

1 Like