Skip to content
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

Include and serve static prometheus web UI in admin portal #3303

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

tsatam
Copy link
Collaborator

@tsatam tsatam commented Nov 30, 2023

Which issue this PR addresses:

Fixes ARO-4823

What this PR does / why we need it:

Bundles static Prometheus Web UI in ARO Admin Portal, and serves the bundled UI rather than proxying the in-cluster Prometheus UI. The Prometheus API is proxied the same as it has been.

This allows ARO SRE to continue using the Prometheus UI on OCP 4.13+ clusters, which remove the in-cluster bundled UI.

Note that this change introduces a few regressions and concerns:

  • The version of the Prometheus Web UI we bundle and serve is now decoupled from the actual Prometheus API version running on any given ARO cluster. This should hopefully not cause any issues, but is something to keep in mind. I did not validate that this newest Prometheus UI works against incredibly old (e.g. 4.4) clusters.
  • The version of the Prometheus Web UI is now vendored into the RP codebase, and we will need to keep it up to date. There are no automated processes or tooling introduced in this PR to do so.
  • The Prometheus Web UI expects to be served by the Prometheus service itself, which performs simple string replacement on the HTML content before serving (see https://github.com/prometheus/prometheus/blob/6d80b30990bc297d95b5c844e118c4011fad8054/web/web.go#L392C7-L392C7). This is not being performed on the UI served by the admin portal. This could be introduced in a future change.
    • The most noticeable artifact of this is the title of the Prometheus web page, which now just displays TITLE_PLACEHOLDER.

Test plan for issue:

This change was tested manually. This change was run locally as well as in int.

  • Local test against 4.10 cluster (before Prometheus introduced URL routing to the original React app)
    • Note that this change will introduce the Prometheus subroutes (e.g. /graph) even on <4.11 clusters.
  • Local test against 4.12 cluster (after the routing change)
  • Local test against 4.13 cluster (no more built-in UI)
  • Int test against 4.12 cluster
  • Int test against 4.13 cluster

This change, as well as an ARO 4.13 cluster, are currently live in the int environment. If you have access to the environment (see https://msazure.visualstudio.com/AzureRedHatOpenShift/_wiki/wikis/ARO.wiki/494771/INT-Environment#) you can validate this yourself as well.

Is there any documentation that needs to be updated for this PR?

A simple summary of this change is included in docs/admin-portal.md.

bennerv
bennerv previously approved these changes Dec 6, 2023
Copy link
Collaborator

@bennerv bennerv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - this makes sense.

Can we get some accompanying docs on how to get the static prometheus files?

@tsatam tsatam changed the title {WIP} Include and serve static prometheus web UI in admin portal Include and serve static prometheus web UI in admin portal Dec 7, 2023
@cadenmarchese cadenmarchese merged commit f3f4d1b into Azure:master Dec 15, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants