mgr/prometheus: Make prometheus standby behaviour configurable #43464
Conversation
avanthakkar
requested review from
a team,
pereman2,
nizamial09 and
p-se
and removed request for
a team,
pereman2 and
nizamial09
There was a problem hiding this comment.
LGTM: it seems inspired by the Dashboard approach (except for the 301 redirection) and it's not breaking the default behaviour. However, as Prometheus will always scrape the /metrics endpoint, have you checked whether this behaves as intended (I assume it returns 4xx-5xx in the /metrics as well)?
rsommer
force-pushed
the
wip-prometheus-standby-behaviour
branch
from
adf9520
to
0d3dc00
Compare
|
jenkins test api |
|
@p-se can you please check this PR? Thanks! |
|
@rsommer this is ready for merging, but can you please squash all these commits? And, if you like to see this landing in Pacific, a tracker issue would be required (and the subsequent "Fixes: https://tracker..." in the commit message and the PR description). Thanks! |
Enable config settings to modify standby's behaviour on the index page This makes the standby discoverable by reverse proxy or loadbalancer setups. Testing for the empty response of the '/metrics' endpoint would trigger metric collection on the active manager instance. The newly added configuration options settings standby_behaviour and standby_error_status_code are documented and flagged as runtime, as modifying both settings has an immediate effect (no restart required). Co-authored-by: Ernesto Puerta <37327689+epuertat@users.noreply.github.com> Signed-off-by: Roland Sommer <rol@ndsommer.de> Fixes: https://tracker.ceph.com/issues/53229
rsommer
force-pushed
the
wip-prometheus-standby-behaviour
branch
from
0d3dc00
to
c1570f8
Compare
|
Squashed, tracker ticket created and pull request updated. Hope this is all good now ;) |
|
Thank you very much @rsommer ! Merged! |
| }) | ||
|
|
||
| module = self | ||
|
|
||
| class Root(object): | ||
| @cherrypy.expose | ||
| def index(self) -> str: | ||
| active_uri = module.get_active_uri() | ||
| return '''<!DOCTYPE html> | ||
| standby_behaviour = module.get_module_option('standby_behaviour') |
There was a problem hiding this comment.
This line introduces a regression, as None is returned if nothing is set for standby_behaviour. Proposed fix in #43904.
There was a problem hiding this comment.
As far as I understood (because I had the default value in there in the beginning, see #43464 (comment)) this should have been handled by the default value set in the option definition?
The dashboard module provides configuration options to configure the standby behaviour for better integration with reverse proxys or loadbalancers. Adding this to the prometheus module simplifys a similiar setup for the prometheus endpoint.
Fixes: https://tracker.ceph.com/issues/53229
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox