Allow enabling prometheus metrics #67
Conversation
The test suite currently uses an f-string, which are only supported beginning with Python 3.6. On some systems, tox attempts to run the tests using Python 3.5, causing a test failure from invalid syntax. This change fixes the issue by ensuring that tox always uses Python 3.6.
This is required for building and deploying the charm on newer versions of charmcraft and/or Ubuntu.
The docker registry can be configured to provide prometheus metrics, but the charm currently does not have the option to enable this option. This change adds the ability to enable this option, using the `prometheus-metrics` and `debug-port` charm configuration options. There is no way to only enable prometheus metrics, so enabling the configuration option also enables other debug URLs such as /debug/vars. It is expected that the other debugging URLs will be less relevant to users of this charm, and so `prometheus-metrics` was chosen as the name of the configuration option. This change also adds a small bit of fanciness by updating the ports displayed in the output of `juju status` dynamically based on their values and the value of the `prometheus-metrics` option. Fixes https://bugs.launchpad.net/layer-docker-registry/+bug/2034475
The unit tests for this project use GitHub Actions, which install a single version of Python during execution. The previous change to use python 3.6 for tests does not work in such an environment, and so this commit undos that one.
lib/charms/layer/docker_registry.py
Outdated
| # debug https://docs.docker.com/registry/configuration/#debug | ||
| # The debug server always listens on port 5001, with debug-port used |
There was a problem hiding this comment.
Fun, looks like the registry was recently donated to the CNCF (as noted at https://docs.docker.com/registry/). Would you like all the URLs updated in this PR, or in a separate one? I've changed this URL specifically since it's directly related to the changes.
There was a problem hiding this comment.
Ah... just the one is fine.
| # https://docs.docker.com/registry/deploying/#customize-the-published-port | ||
| cmd = ['docker', 'run', '-d', '-p', '{}:5000'.format(port), | ||
| '--restart', 'unless-stopped'] | ||
| '-p', '{}:5001'.format(debug_port), '--restart', 'unless-stopped'] |
There was a problem hiding this comment.
Ahh, okay so we're port-forwarding the debug-port into the hard-coded port 5001 from above
There was a problem hiding this comment.
Correct, I based this off the registry-port config option.
wheelhouse.txt
Outdated
| calver | ||
| hatchling |
There was a problem hiding this comment.
sorry, why are these packages included? Neither are used.
| calver | |
| hatchling |
There was a problem hiding this comment.
Okay, I resolved an issue in the layer-docker which was requiring these packages. Please try without them @shanepelletier
There was a problem hiding this comment.
Ah, I was pretty sure I needed them for something. I've removed them now, thanks for fixing the root cause!
The docker registry can be configured to provide prometheus metrics, but
the charm currently does not have the option to enable this option. This
change adds the ability to enable this option, using the
prometheus-metricsanddebug-portcharm configuration options. Thereis no way to only enable prometheus metrics, so enabling the
configuration option also enables other debug URLs such as /debug/vars.
It is expected that the other debugging URLs will be less relevant to
users of this charm, and so
prometheus-metricswas chosen as the nameof the configuration option.
This change also adds a small bit of fanciness by updating the ports
displayed in the output of
juju statusdynamically based on theirvalues and the value of the
prometheus-metricsoption.Fixes https://bugs.launchpad.net/layer-docker-registry/+bug/2034475